Bug 1130400. Part 2. Use a reflow callback to set the position of xul popups. r=enndeakin
authorTimothy Nikkel <tnikkel@gmail.com>
Thu, 19 Mar 2015 23:55:33 -0500
changeset 234711 4867d8464f88b6b9e15d42056452c75d171b979c
parent 234710 43f2f38859d029ce9bb238ed3dd64ea3275d61fd
child 234712 25c0b4f618fbd5397ae0b4f6a3a4dd53ed6b12dd
push id28454
push userphilringnalda@gmail.com
push dateSat, 21 Mar 2015 19:32:28 +0000
treeherdermozilla-central@f949be6cd23e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersenndeakin
bugs1130400
milestone39.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 1130400. Part 2. Use a reflow callback to set the position of xul popups. r=enndeakin During reflow our frame, or any ancestors, may not yet be placed at their final position. So calculating the screen position of our popup during our reflow could produce incorrect results. Instead post a reflow callback to do it after reflow is finished.
layout/xul/nsMenuPopupFrame.cpp
layout/xul/nsMenuPopupFrame.h
--- a/layout/xul/nsMenuPopupFrame.cpp
+++ b/layout/xul/nsMenuPopupFrame.cpp
@@ -453,18 +453,21 @@ nsMenuPopupFrame::LayoutPopup(nsBoxLayou
 
   // if the size changed then set the bounds to be the preferred size
   bool sizeChanged = (mPrefSize != prefSize);
   if (sizeChanged) {
     SetBounds(aState, nsRect(0, 0, prefSize.width, prefSize.height), false);
     mPrefSize = prefSize;
   }
 
+  bool needCallback = false;
+
   if (shouldPosition) {
     SetPopupPosition(aAnchor, false, aSizedToPopup);
+    needCallback = true;
   }
 
   nsRect bounds(GetRect());
   Layout(aState);
 
   // if the width or height changed, readjust the popup position. This is a
   // special case for tooltips where the preferred height doesn't include the
   // real height for its inline element, but does once it is laid out.
@@ -472,16 +475,17 @@ nsMenuPopupFrame::LayoutPopup(nsBoxLayou
   if (!aParentMenu) {
     nsSize newsize = GetSize();
     if (newsize.width > bounds.width || newsize.height > bounds.height) {
       // the size after layout was larger than the preferred size,
       // so set the preferred size accordingly
       mPrefSize = newsize;
       if (isOpen) {
         SetPopupPosition(aAnchor, false, aSizedToPopup);
+        needCallback = true;
       }
     }
   }
 
   nsPresContext* pc = PresContext();
   nsView* view = GetView();
 
   if (sizeChanged) {
@@ -523,16 +527,37 @@ nsMenuPopupFrame::LayoutPopup(nsBoxLayou
       return;
     }
 #endif
 
     // If there are no transitions, fire the popupshown event right away.
     nsCOMPtr<nsIRunnable> event = new nsXULPopupShownEvent(GetContent(), pc);
     NS_DispatchToCurrentThread(event);
   }
+
+  if (needCallback && !mReflowCallbackData.mPosted) {
+    pc->PresShell()->PostReflowCallback(this);
+    mReflowCallbackData.MarkPosted(aAnchor, aSizedToPopup);
+  }
+}
+
+bool
+nsMenuPopupFrame::ReflowFinished()
+{
+  SetPopupPosition(mReflowCallbackData.mAnchor, false, mReflowCallbackData.mSizedToPopup);
+
+  mReflowCallbackData.Clear();
+
+  return false;
+}
+
+void
+nsMenuPopupFrame::ReflowCallbackCanceled()
+{
+  mReflowCallbackData.Clear();
 }
 
 nsIContent*
 nsMenuPopupFrame::GetTriggerContent(nsMenuPopupFrame* aMenuPopupFrame)
 {
   while (aMenuPopupFrame) {
     if (aMenuPopupFrame->mTriggerContent)
       return aMenuPopupFrame->mTriggerContent;
@@ -1938,16 +1963,21 @@ nsMenuPopupFrame::MoveToAttributePositio
 
   if (NS_SUCCEEDED(err1) && NS_SUCCEEDED(err2))
     MoveTo(xpos, ypos, false);
 }
 
 void
 nsMenuPopupFrame::DestroyFrom(nsIFrame* aDestructRoot)
 {
+  if (mReflowCallbackData.mPosted) {
+    PresContext()->PresShell()->CancelReflowCallback(this);
+    mReflowCallbackData.Clear();
+  }
+
   nsMenuFrame* menu = do_QueryFrame(GetParent());
   if (menu) {
     // clear the open attribute on the parent menu
     nsContentUtils::AddScriptRunner(
       new nsUnsetAttrRunnable(menu->GetContent(), nsGkAtoms::open));
   }
 
   ClearPopupShownDispatcher();
--- a/layout/xul/nsMenuPopupFrame.h
+++ b/layout/xul/nsMenuPopupFrame.h
@@ -147,17 +147,18 @@ public:
 protected:
   virtual ~nsXULPopupShownEvent() { }
 
 private:
   nsCOMPtr<nsIContent> mPopup;
   nsRefPtr<nsPresContext> mPresContext;
 };
 
-class nsMenuPopupFrame MOZ_FINAL : public nsBoxFrame, public nsMenuParent
+class nsMenuPopupFrame MOZ_FINAL : public nsBoxFrame, public nsMenuParent,
+                                   public nsIReflowCallback
 {
 public:
   NS_DECL_QUERYFRAME_TARGET(nsMenuPopupFrame)
   NS_DECL_QUERYFRAME
   NS_DECL_FRAMEARENA_HELPERS
 
   explicit nsMenuPopupFrame(nsStyleContext* aContext);
 
@@ -394,16 +395,20 @@ public:
       mPopupShownDispatcher->CancelListener();
       mPopupShownDispatcher = nullptr;
       return true;
     }
 
     return false;
   }
 
+  // nsIReflowCallback
+  virtual bool ReflowFinished() MOZ_OVERRIDE;
+  virtual void ReflowCallbackCanceled() MOZ_OVERRIDE;
+
 protected:
 
   // returns the popup's level.
   nsPopupLevel PopupLevel(bool aIsNoAutoHide) const;
 
   // redefine to tell the box system not to move the views.
   virtual void GetLayoutFlags(uint32_t& aFlags) MOZ_OVERRIDE;
 
@@ -521,16 +526,38 @@ protected:
   int8_t mPopupAlignment;
   int8_t mPopupAnchor;
   int8_t mPosition;
 
   // One of PopupBoxObject::ROLLUP_DEFAULT/ROLLUP_CONSUME/ROLLUP_NO_CONSUME
   uint8_t mConsumeRollupEvent;
   FlipType mFlip; // Whether to flip
 
+  struct ReflowCallbackData {
+    ReflowCallbackData() :
+      mPosted(false),
+      mAnchor(nullptr),
+      mSizedToPopup(false)
+    {}
+    void MarkPosted(nsIFrame* aAnchor, bool aSizedToPopup) {
+      mPosted = true;
+      mAnchor = aAnchor;
+      mSizedToPopup = aSizedToPopup;
+    }
+    void Clear() {
+      mPosted = false;
+      mAnchor = nullptr;
+      mSizedToPopup = false;
+    }
+    bool mPosted;
+    nsIFrame* mAnchor;
+    bool mSizedToPopup;
+  };
+  ReflowCallbackData mReflowCallbackData;
+
   bool mIsOpenChanged; // true if the open state changed since the last layout
   bool mIsContextMenu; // true for context menus
   // true if we need to offset the popup to ensure it's not under the mouse
   bool mAdjustOffsetForContextMenu;
   bool mGeneratedChildren; // true if the contents have been created
 
   bool mMenuCanOverlapOSBar;    // can we appear over the taskbar/menubar?
   bool mShouldAutoPosition; // Should SetPopupPosition be allowed to auto position popup?