commit 8614bf2a9ac69a52f070a91e747ecaf6710c60c4
parent a462b9fb10c3e1e3ffbe71b18fd361c6c73bfba2
Author: Emilio Cobos Álvarez <emilio@crisal.io>
Date: Fri, 3 Oct 2025 11:33:41 +0000
Bug 1986711 - Closed menupopups shouldn't have an intrinsic size. r=layout-reviewers,dshin
We already avoid most of the reflow work during Reflow(), but this
code-path can still be expensive. We get it to try to compute the
hypothetical position of a popup, which arguably we should never need to
begin with, but this seems trivial to do here.
Differential Revision: https://phabricator.services.mozilla.com/D267288
Diffstat:
2 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/layout/xul/nsMenuPopupFrame.cpp b/layout/xul/nsMenuPopupFrame.cpp
@@ -500,6 +500,9 @@ void nsMenuPopupFrame::DidSetComputedStyle(ComputedStyle* aOldStyle) {
nscoord nsMenuPopupFrame::IntrinsicISize(const IntrinsicSizeInput& aInput,
IntrinsicISizeType aType) {
+ if (CanSkipLayout()) {
+ return 0;
+ }
nscoord iSize = nsBlockFrame::IntrinsicISize(aInput, aType);
if (!ShouldExpandToInflowParentOrAnchor()) {
return iSize;
@@ -566,6 +569,18 @@ void nsMenuPopupFrame::EnsureActiveMenuListItemIsVisible() {
ScrollFlags::ScrollOverflowHidden | ScrollFlags::ScrollFirstAncestorOnly);
}
+bool nsMenuPopupFrame::CanSkipLayout() const {
+ // If the popup is not open, only do layout while showing or if we're a
+ // menulist.
+ //
+ // The later is needed because the SelectParent code wants to limit the height
+ // of the popup before opening it.
+ //
+ // TODO(emilio): We should consider adding a way to do that more reliably
+ // instead, but this preserves existing behavior.
+ return !IsVisibleOrShowing() && !IsMenuList();
+}
+
void nsMenuPopupFrame::LayoutPopup(nsPresContext* aPresContext,
ReflowOutput& aDesiredSize,
const ReflowInput& aReflowInput,
@@ -577,21 +592,9 @@ void nsMenuPopupFrame::LayoutPopup(nsPresContext* aPresContext,
SchedulePaint();
const bool isOpen = IsOpen();
- if (!isOpen) {
- // If the popup is not open, only do layout while showing or if we're a
- // menulist.
- //
- // This is needed because the SelectParent code wants to limit the height of
- // the popup before opening it.
- //
- // TODO(emilio): We should consider adding a way to do that more reliably
- // instead, but this preserves existing behavior.
- const bool needsLayout = mPopupState == ePopupShowing ||
- mPopupState == ePopupPositioning || IsMenuList();
- if (!needsLayout) {
- RemoveStateBits(NS_FRAME_FIRST_REFLOW);
- return;
- }
+ if (CanSkipLayout()) {
+ RemoveStateBits(NS_FRAME_FIRST_REFLOW);
+ return;
}
// Do a first reflow, with all our content, in order to find our preferred
diff --git a/layout/xul/nsMenuPopupFrame.h b/layout/xul/nsMenuPopupFrame.h
@@ -266,14 +266,15 @@ class nsMenuPopupFrame final : public nsBlockFrame {
return mPopupState == ePopupOpening || mPopupState == ePopupVisible ||
mPopupState == ePopupShown;
}
- bool IsVisible() {
+ bool IsVisible() const {
return mPopupState == ePopupVisible || mPopupState == ePopupShown;
}
- bool IsVisibleOrShowing() {
+ bool IsVisibleOrShowing() const {
return IsOpen() || mPopupState == ePopupPositioning ||
mPopupState == ePopupShowing;
}
bool IsNativeMenu() const { return mIsNativeMenu; }
+ bool CanSkipLayout() const;
bool IsMouseTransparent() const;
// Return true if the popup is for a menulist.