Bug 1340451. Don't use weak frames for reflow callbacks in nsMenuFrame. r=mats
authorTimothy Nikkel <tnikkel@gmail.com>
Sat, 18 Feb 2017 02:13:40 -0600
changeset 372825 6af6e3b0247f89ab98d04f16f294c6b21b9730cc
parent 372824 80e5378cc1c819fa92099e28a5a6629d8a949e52
child 372826 8fc28f2d52d93245ed802f23c15c16290a7c22cc
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats
bugs1340451
milestone54.0a1
Bug 1340451. Don't use weak frames for reflow callbacks in nsMenuFrame. r=mats Weak frames are stored in a linked list, so are not fast. Hence they are not meant to be used when there can be a lot of them active at the same time. When constructing large selects with lots of options we create one for every option. There's no need to use a weak frame for a reflow callback as we can just cancel the reflow callback in the Destroy function.
layout/xul/nsMenuFrame.cpp
layout/xul/nsMenuFrame.h
--- a/layout/xul/nsMenuFrame.cpp
+++ b/layout/xul/nsMenuFrame.cpp
@@ -169,16 +169,17 @@ NS_QUERYFRAME_HEAD(nsMenuFrame)
   NS_QUERYFRAME_ENTRY(nsMenuFrame)
 NS_QUERYFRAME_TAIL_INHERITING(nsBoxFrame)
 
 nsMenuFrame::nsMenuFrame(nsStyleContext* aContext):
   nsBoxFrame(aContext),
     mIsMenu(false),
     mChecked(false),
     mIgnoreAccelTextChange(false),
+    mReflowCallbackPosted(false),
     mType(eMenuType_Normal),
     mBlinkState(0)
 {
 }
 
 nsMenuParent*
 nsMenuFrame::GetMenuParent() const
 {
@@ -191,57 +192,46 @@ nsMenuFrame::GetMenuParent() const
     nsMenuBarFrame* menubar = do_QueryFrame(parent);
     if (menubar) {
       return menubar;
     }
   }
   return nullptr;
 }
 
-class nsASyncMenuInitialization final : public nsIReflowCallback
+bool
+nsMenuFrame::ReflowFinished()
 {
-public:
-  explicit nsASyncMenuInitialization(nsIFrame* aFrame)
-    : mWeakFrame(aFrame)
-  {
-  }
+  mReflowCallbackPosted = false;
 
-  virtual bool ReflowFinished() override
-  {
-    bool shouldFlush = false;
-    nsMenuFrame* menu = do_QueryFrame(mWeakFrame.GetFrame());
-    if (menu) {
-      menu->UpdateMenuType();
-      shouldFlush = true;
-    }
-    delete this;
-    return shouldFlush;
-  }
+  UpdateMenuType();
+  return true;
+}
 
-  virtual void ReflowCallbackCanceled() override
-  {
-    delete this;
-  }
-
-  nsWeakFrame mWeakFrame;
-};
+void
+nsMenuFrame::ReflowCallbackCanceled()
+{
+  mReflowCallbackPosted = false;
+}
 
 void
 nsMenuFrame::Init(nsIContent*       aContent,
                   nsContainerFrame* aParent,
                   nsIFrame*         aPrevInFlow)
 {
   nsBoxFrame::Init(aContent, aParent, aPrevInFlow);
 
   // Set up a mediator which can be used for callbacks on this frame.
   mTimerMediator = new nsMenuTimerMediator(this);
 
   BuildAcceleratorText(false);
-  nsIReflowCallback* cb = new nsASyncMenuInitialization(this);
-  PresContext()->PresShell()->PostReflowCallback(cb);
+  if (!mReflowCallbackPosted) {
+    mReflowCallbackPosted = true;
+    PresContext()->PresShell()->PostReflowCallback(this);
+  }
 }
 
 const nsFrameList&
 nsMenuFrame::GetChildList(ChildListID aListID) const
 {
   if (kPopupList == aListID) {
     nsFrameList* list = GetPopupList();
     return list ? *list : nsFrameList::EmptyList();
@@ -316,16 +306,21 @@ nsMenuFrame::SetInitialChildList(ChildLi
     SetPopupFrame(aChildList);
   }
   nsBoxFrame::SetInitialChildList(aListID, aChildList);
 }
 
 void
 nsMenuFrame::DestroyFrom(nsIFrame* aDestructRoot)
 {
+  if (mReflowCallbackPosted) {
+    PresContext()->PresShell()->CancelReflowCallback(this);
+    mReflowCallbackPosted = false;
+  }
+
   // Kill our timer if one is active. This is not strictly necessary as
   // the pointer to this frame will be cleared from the mediator, but
   // this is done for added safety.
   if (mOpenTimer) {
     mOpenTimer->Cancel();
   }
 
   StopBlinking();
--- a/layout/xul/nsMenuFrame.h
+++ b/layout/xul/nsMenuFrame.h
@@ -13,16 +13,17 @@
 #include "nsIAtom.h"
 #include "nsCOMPtr.h"
 
 #include "nsBoxFrame.h"
 #include "nsFrameList.h"
 #include "nsGkAtoms.h"
 #include "nsMenuParent.h"
 #include "nsXULPopupManager.h"
+#include "nsIReflowCallback.h"
 #include "nsITimer.h"
 #include "mozilla/Attributes.h"
 
 nsIFrame* NS_NewMenuFrame(nsIPresShell* aPresShell, nsStyleContext* aContext);
 nsIFrame* NS_NewMenuItemFrame(nsIPresShell* aPresShell, nsStyleContext* aContext);
 
 class nsIContent;
 
@@ -67,16 +68,17 @@ public:
 private:
   ~nsMenuTimerMediator();
 
   // Pointer to the wrapped frame.
   nsMenuFrame* mFrame;
 };
 
 class nsMenuFrame final : public nsBoxFrame
+                        , public nsIReflowCallback
 {
 public:
   explicit nsMenuFrame(nsStyleContext* aContext);
 
   NS_DECL_QUERYFRAME_TARGET(nsMenuFrame)
   NS_DECL_QUERYFRAME
   NS_DECL_FRAMEARENA_HELPERS
 
@@ -206,16 +208,20 @@ public:
   virtual nsresult GetFrameName(nsAString& aResult) const override
   {
       return MakeFrameName(NS_LITERAL_STRING("Menu"), aResult);
   }
 #endif
 
   static bool IsSizedToPopup(nsIContent* aContent, bool aRequireAlways);
 
+  // nsIReflowCallback
+  virtual bool ReflowFinished() override;
+  virtual void ReflowCallbackCanceled() override;
+
 protected:
   friend class nsMenuTimerMediator;
   friend class nsASyncMenuInitialization;
   friend class nsMenuAttributeChangedEvent;
 
   /**
    * Initialize the popup list to the first popup frame within
    * aChildList. Removes the popup, if any, from aChildList.
@@ -265,16 +271,17 @@ protected:
 #ifdef DEBUG_LAYOUT
   nsresult SetXULDebug(nsBoxLayoutState& aState, nsIFrame* aList, bool aDebug);
 #endif
   nsresult Notify(nsITimer* aTimer);
 
   bool mIsMenu; // Whether or not we can even have children or not.
   bool mChecked;              // are we checked?
   bool mIgnoreAccelTextChange; // temporarily set while determining the accelerator key
+  bool mReflowCallbackPosted;
   nsMenuType mType;
 
   // Reference to the mediator which wraps this frame.
   RefPtr<nsMenuTimerMediator> mTimerMediator;
 
   nsCOMPtr<nsITimer> mOpenTimer;
   nsCOMPtr<nsITimer> mBlinkTimer;