Bug 1597160 - Remove nsChangeHint_UpdateWidgetProperties. r=mstange
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 20 Nov 2019 02:37:06 +0000
changeset 502770 53d116eb2adb6d25ba5ef3664127c2f204652466
parent 502769 79821df172391d2d9ab224951b36bd8856df0fb1
child 502771 ed9b896fd383049c143c41fe28547d7b37c051c0
push id36824
push usercsabou@mozilla.com
push dateWed, 20 Nov 2019 16:17:49 +0000
treeherdermozilla-central@01032a31b2e2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange
bugs1597160, 1597120
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1597160 - Remove nsChangeHint_UpdateWidgetProperties. r=mstange :ntim was about to add another widget-affecting property (pointer-events) to menupopups to replace the mousethrough attribute, see bug 1597120. But pointer-events is inherited, and thus changing pointer-events on any element would cause a change list of length == the number of descendants of the element, which is not amazing. So I suggested using DidSetComputedStyle instead, as this is fairly popup-specific, but we may as well be consistent and do the same everywhere. This removes the code to handle the -moz-window-* properties on the root frame, as I haven't seen any usage of them (we always set them in panel or menupopup). Differential Revision: https://phabricator.services.mozilla.com/D53377
layout/base/RestyleManager.cpp
layout/base/nsChangeHint.h
layout/generic/nsFrame.cpp
layout/generic/nsIFrame.h
layout/style/nsStyleStruct.cpp
layout/xul/nsMenuPopupFrame.cpp
layout/xul/nsMenuPopupFrame.h
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -558,17 +558,16 @@ nsCString RestyleManager::ChangeHintToSt
                          "NeutralChange",
                          "InvalidateRenderingObservers",
                          "ReflowChangesSizeOrPosition",
                          "UpdateComputedBSize",
                          "UpdateUsesOpacity",
                          "UpdateBackgroundPosition",
                          "AddOrRemoveTransform",
                          "ScrollbarChange",
-                         "UpdateWidgetProperties",
                          "UpdateTableCellSpans",
                          "VisibilityChange"};
   static_assert(nsChangeHint_AllHints ==
                     static_cast<uint32_t>((1ull << ArrayLength(names)) - 1),
                 "Name list doesn't match change hints.");
   uint32_t hint =
       aHint & static_cast<uint32_t>((1ull << ArrayLength(names)) - 1);
   uint32_t rest =
@@ -1778,19 +1777,16 @@ void RestyleManager::ProcessRestyledFram
             }
           }
         }
       }
       if ((hint & nsChangeHint_UpdateCursor) && !didUpdateCursor) {
         presContext->PresShell()->SynthesizeMouseMove(false);
         didUpdateCursor = true;
       }
-      if (hint & nsChangeHint_UpdateWidgetProperties) {
-        frame->UpdateWidgetProperties();
-      }
       if (hint & nsChangeHint_UpdateTableCellSpans) {
         frameConstructor->UpdateTableCellSpans(content);
       }
       if (hint & nsChangeHint_VisibilityChange) {
         frame->UpdateVisibleDescendantsState();
       }
     }
   }
--- a/layout/base/nsChangeHint.h
+++ b/layout/base/nsChangeHint.h
@@ -220,51 +220,45 @@ enum nsChangeHint : uint32_t {
    * In most cases, this is equivalent to nsChangeHint_ReconstructFrame. But
    * in some special cases where the change is really targeting the viewport's
    * scrollframe, this is instead equivalent to nsChangeHint_AllReflowHints
    * (because the viewport always has an associated scrollframe).
    */
   nsChangeHint_ScrollbarChange = 1 << 26,
 
   /**
-   * Indicates that nsIFrame::UpdateWidgetProperties needs to be called.
-   * This is used for -moz-window-* properties.
-   */
-  nsChangeHint_UpdateWidgetProperties = 1 << 27,
-
-  /**
    *  Indicates that there has been a colspan or rowspan attribute change
    *  on the cells of a table.
    */
-  nsChangeHint_UpdateTableCellSpans = 1 << 28,
+  nsChangeHint_UpdateTableCellSpans = 1 << 27,
 
   /**
    * Indicates that the visiblity property changed.
    * This change hint is used for skip restyling for animations on
    * visibility:hidden elements in the case where the elements have no visible
    * descendants.
    */
-  nsChangeHint_VisibilityChange = 1u << 29,
+  nsChangeHint_VisibilityChange = 1u << 28,
 
   // IMPORTANT NOTE: When adding a new hint, you will need to add it to
   // one of:
   //
   //   * nsChangeHint_Hints_NeverHandledForDescendants
   //   * nsChangeHint_Hints_AlwaysHandledForDescendants
   //   * nsChangeHint_Hints_SometimesHandledForDescendants
   //
   // and you also may need to handle it in NS_HintsNotHandledForDescendantsIn.
   //
   // Please also add it to RestyleManager::ChangeHintToString and
   // modify nsChangeHint_AllHints below accordingly.
 
   /**
    * Dummy hint value for all hints. It exists for compile time check.
    */
-  nsChangeHint_AllHints = uint32_t((1ull << 30) - 1),
+  nsChangeHint_AllHints = uint32_t((1ull << 29) - 1),
 };
 
 // Redefine these operators to return nothing. This will catch any use
 // of these operators on hints. We should not be using these operators
 // on nsChangeHints
 inline void operator<(nsChangeHint s1, nsChangeHint s2) {}
 inline void operator>(nsChangeHint s1, nsChangeHint s2) {}
 inline void operator!=(nsChangeHint s1, nsChangeHint s2) {}
@@ -335,18 +329,17 @@ inline nsChangeHint operator^=(nsChangeH
   (nsChangeHint_BorderStyleNoneChange | nsChangeHint_ChildrenOnlyTransform |  \
    nsChangeHint_ScrollbarChange | nsChangeHint_InvalidateRenderingObservers | \
    nsChangeHint_RecomputePosition | nsChangeHint_UpdateBackgroundPosition |   \
    nsChangeHint_UpdateComputedBSize | nsChangeHint_UpdateContainingBlock |    \
    nsChangeHint_UpdateEffects | nsChangeHint_UpdateOpacityLayer |             \
    nsChangeHint_UpdateOverflow | nsChangeHint_UpdateParentOverflow |          \
    nsChangeHint_UpdatePostTransformOverflow |                                 \
    nsChangeHint_UpdateTableCellSpans | nsChangeHint_UpdateTransformLayer |    \
-   nsChangeHint_UpdateUsesOpacity | nsChangeHint_AddOrRemoveTransform |       \
-   nsChangeHint_UpdateWidgetProperties)
+   nsChangeHint_UpdateUsesOpacity | nsChangeHint_AddOrRemoveTransform)
 
 // The change hints that are sometimes considered to be handled for descendants.
 #define nsChangeHint_Hints_SometimesHandledForDescendants           \
   (nsChangeHint_ClearAncestorIntrinsics | nsChangeHint_NeedReflow | \
    nsChangeHint_ReflowChangesSizeOrPosition)
 
 static_assert(!(nsChangeHint_Hints_AlwaysHandledForDescendants &
                 nsChangeHint_Hints_NeverHandledForDescendants) &&
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -11020,52 +11020,16 @@ gfx::Matrix nsIFrame::ComputeWidgetTrans
         "-moz-window-transform does not describe a 2D transform, "
         "but only 2d transforms are supported");
     return gfx::Matrix();
   }
 
   return result2d;
 }
 
-static already_AddRefed<nsIWidget> GetWindowWidget(
-    nsPresContext* aPresContext) {
-  // We want to obtain the widget for the window. We can't use any of these
-  // methods: nsPresContext::GetRootWidget, nsPresContext::GetNearestWidget,
-  // nsIFrame::GetNearestWidget because those deal with child widgets and
-  // there is no parent widget connection between child widgets and the
-  // window widget that contains them.
-  nsCOMPtr<nsISupports> container = aPresContext->Document()->GetContainer();
-  nsCOMPtr<nsIBaseWindow> baseWindow = do_QueryInterface(container);
-  if (!baseWindow) {
-    return nullptr;
-  }
-
-  nsCOMPtr<nsIWidget> mainWidget;
-  baseWindow->GetMainWidget(getter_AddRefs(mainWidget));
-  return mainWidget.forget();
-}
-
-void nsIFrame::UpdateWidgetProperties() {
-  nsPresContext* presContext = PresContext();
-  if (presContext->IsRoot() || !presContext->IsChrome()) {
-    // Don't do anything for documents that aren't the root chrome document.
-    return;
-  }
-  nsIFrame* rootFrame =
-      presContext->FrameConstructor()->GetRootElementStyleFrame();
-  if (this != rootFrame) {
-    // Only the window's root style frame is relevant for widget properties.
-    return;
-  }
-  if (nsCOMPtr<nsIWidget> widget = GetWindowWidget(presContext)) {
-    widget->SetWindowOpacity(StyleUIReset()->mWindowOpacity);
-    widget->SetWindowTransform(ComputeWidgetTransform());
-  }
-}
-
 void nsIFrame::DoUpdateStyleOfOwnedAnonBoxes(ServoRestyleState& aRestyleState) {
   // As a special case, we check for {ib}-split block frames here, rather
   // than have an nsInlineFrame::AppendDirectlyOwnedAnonBoxes implementation
   // that returns them.
   //
   // (If we did handle them in AppendDirectlyOwnedAnonBoxes, we would have to
   // return *all* of the in-flow {ib}-split block frames, not just the first
   // one.  For restyling, we really just need the first in flow, and the other
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -4106,21 +4106,16 @@ class nsIFrame : public nsQueryFrame {
    * Computes a 2D matrix from the -moz-window-transform and
    * -moz-window-transform-origin properties on aFrame.
    * Values that don't result in a 2D matrix will be ignored and an identity
    * matrix will be returned instead.
    */
   Matrix ComputeWidgetTransform();
 
   /**
-   * Applies the values from the -moz-window-* properties to the widget.
-   */
-  virtual void UpdateWidgetProperties();
-
-  /**
    * @return true iff this frame has one or more associated image requests.
    * @see mozilla::css::ImageLoader.
    */
   bool HasImageRequest() const { return mHasImageRequest; }
 
   /**
    * Update this frame's image request state.
    */
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -3750,22 +3750,19 @@ nsChangeHint nsStyleUIReset::CalcDiffere
   if (mUserSelect != aNewData.mUserSelect) {
     hint |= NS_STYLE_HINT_VISUAL;
   }
 
   if (mWindowDragging != aNewData.mWindowDragging) {
     hint |= nsChangeHint_SchedulePaint;
   }
 
-  if (mWindowOpacity != aNewData.mWindowOpacity ||
-      mMozWindowTransform != aNewData.mMozWindowTransform) {
-    hint |= nsChangeHint_UpdateWidgetProperties;
-  }
-
-  if (!hint && mIMEMode != aNewData.mIMEMode) {
+  if (!hint && (mIMEMode != aNewData.mIMEMode ||
+                mWindowOpacity != aNewData.mWindowOpacity ||
+                mMozWindowTransform != aNewData.mMozWindowTransform)) {
     hint |= nsChangeHint_NeutralChange;
   }
 
   return hint;
 }
 
 //-----------------------
 // nsStyleEffects
--- a/layout/xul/nsMenuPopupFrame.cpp
+++ b/layout/xul/nsMenuPopupFrame.cpp
@@ -448,20 +448,35 @@ bool nsMenuPopupFrame::IsLeafDynamic() c
     return sizedToPopup.EqualsLiteral("none");
   }
 
   return (!parentContent->IsElement() ||
           !parentContent->AsElement()->HasAttr(kNameSpaceID_None,
                                                nsGkAtoms::sizetopopup));
 }
 
-void nsMenuPopupFrame::UpdateWidgetProperties() {
-  if (nsIWidget* widget = GetWidget()) {
-    widget->SetWindowOpacity(StyleUIReset()->mWindowOpacity);
-    widget->SetWindowTransform(ComputeWidgetTransform());
+void nsMenuPopupFrame::DidSetComputedStyle(ComputedStyle* aOldStyle) {
+  nsBoxFrame::DidSetComputedStyle(aOldStyle);
+
+  if (!aOldStyle) {
+    return;
+  }
+
+  auto& newUI = *StyleUIReset();
+  auto& oldUI = *aOldStyle->StyleUIReset();
+  if (newUI.mWindowOpacity != oldUI.mWindowOpacity) {
+    if (nsIWidget* widget = GetWidget()) {
+      widget->SetWindowOpacity(newUI.mWindowOpacity);
+    }
+  }
+
+  if (newUI.mMozWindowTransform != oldUI.mMozWindowTransform) {
+    if (nsIWidget* widget = GetWidget()) {
+      widget->SetWindowTransform(ComputeWidgetTransform());
+    }
   }
 }
 
 void nsMenuPopupFrame::LayoutPopup(nsBoxLayoutState& aState,
                                    nsIFrame* aParentMenu, bool aSizedToPopup) {
   if (IsLeaf()) {
     return;
   }
--- a/layout/xul/nsMenuPopupFrame.h
+++ b/layout/xul/nsMenuPopupFrame.h
@@ -245,19 +245,19 @@ class nsMenuPopupFrame final : public ns
   // Ensure that a widget has already been created for this view, and create
   // one if it hasn't. If aRecreate is true, destroys any existing widget and
   // creates a new one, regardless of whether one has already been created.
   void EnsureWidget(bool aRecreate = false);
 
   nsresult CreateWidgetForView(nsView* aView);
   uint8_t GetShadowStyle();
 
-  virtual bool IsLeafDynamic() const override;
+  bool IsLeafDynamic() const override;
 
-  virtual void UpdateWidgetProperties() override;
+  void DidSetComputedStyle(ComputedStyle* aOldStyle) override;
 
   // layout, position and display the popup as needed
   MOZ_CAN_RUN_SCRIPT_BOUNDARY
   void LayoutPopup(nsBoxLayoutState& aState, nsIFrame* aParentMenu,
                    bool aSizedToPopup);
 
   nsView* GetRootViewForPopup(nsIFrame* aStartFrame);