Bug 1340771 part 2 - Introduce a WeakFrame class for heap allocated weak frame pointers, stored in a hashtable for fast lookup. r=tn draft
authorMats Palmgren <mats@mozilla.com>
Mon, 27 Feb 2017 17:40:21 +0100
changeset 490076 4c8da6912482dbe8a3e463404bc52d6767eb4b04
parent 490075 0d83b285d402b3453a7a0139ab936d34c9ca5e75
child 490077 559f7a4ffbd50e0704d486328e77ad0340789cbc
push id46987
push usermpalmgren@mozilla.com
push dateMon, 27 Feb 2017 16:44:31 +0000
reviewerstn
bugs1340771
milestone54.0a1
Bug 1340771 part 2 - Introduce a WeakFrame class for heap allocated weak frame pointers, stored in a hashtable for fast lookup. r=tn MozReview-Commit-ID: Ii8rDQfRYKz
layout/base/PresShell.cpp
layout/base/nsIPresShell.h
layout/generic/nsFrame.cpp
layout/generic/nsIFrame.h
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -676,16 +676,32 @@ nsIPresShell::AddWeakFrameInternal(AutoW
   if (aWeakFrame->GetFrame()) {
     aWeakFrame->GetFrame()->AddStateBits(NS_FRAME_EXTERNAL_REFERENCE);
   }
   aWeakFrame->SetPreviousWeakFrame(mWeakFrames);
   mWeakFrames = aWeakFrame;
 }
 
 /* virtual */ void
+nsIPresShell::AddWeakFrameExternal(WeakFrame* aWeakFrame)
+{
+  AddWeakFrameInternal(aWeakFrame);
+}
+
+void
+nsIPresShell::AddWeakFrameInternal(WeakFrame* aWeakFrame)
+{
+  if (aWeakFrame->GetFrame()) {
+    aWeakFrame->GetFrame()->AddStateBits(NS_FRAME_EXTERNAL_REFERENCE);
+  }
+  MOZ_ASSERT(!mHeapWeakFrames.GetEntry(aWeakFrame));
+  mHeapWeakFrames.PutEntry(aWeakFrame);
+}
+
+/* virtual */ void
 nsIPresShell::RemoveWeakFrameExternal(AutoWeakFrame* aWeakFrame)
 {
   RemoveWeakFrameInternal(aWeakFrame);
 }
 
 void
 nsIPresShell::RemoveWeakFrameInternal(AutoWeakFrame* aWeakFrame)
 {
@@ -697,16 +713,29 @@ nsIPresShell::RemoveWeakFrameInternal(Au
   while (nextWeak && nextWeak->GetPreviousWeakFrame() != aWeakFrame) {
     nextWeak = nextWeak->GetPreviousWeakFrame();
   }
   if (nextWeak) {
     nextWeak->SetPreviousWeakFrame(aWeakFrame->GetPreviousWeakFrame());
   }
 }
 
+/* virtual */ void
+nsIPresShell::RemoveWeakFrameExternal(WeakFrame* aWeakFrame)
+{
+  RemoveWeakFrameInternal(aWeakFrame);
+}
+
+void
+nsIPresShell::RemoveWeakFrameInternal(WeakFrame* aWeakFrame)
+{
+  MOZ_ASSERT(mHeapWeakFrames.GetEntry(aWeakFrame));
+  mHeapWeakFrames.RemoveEntry(aWeakFrame);
+}
+
 already_AddRefed<nsFrameSelection>
 nsIPresShell::FrameSelection()
 {
   RefPtr<nsFrameSelection> ret = mSelection;
   return ret.forget();
 }
 
 //----------------------------------------------------------------------
@@ -1361,21 +1390,28 @@ PresShell::Destroy()
     // now dead, we shouldn't be looking up any more properties in that table.
     // We want to do this before we call DetachShell() on the prescontext, so
     // property destructors can usefully call GetPresShell() on the
     // prescontext.
     mPresContext->PropertyTable()->DeleteAll();
   }
 
 
-  NS_WARNING_ASSERTION(!mWeakFrames,
+  NS_WARNING_ASSERTION(!mWeakFrames && mHeapWeakFrames.IsEmpty(),
                        "Weak frames alive after destroying FrameManager");
   while (mWeakFrames) {
     mWeakFrames->Clear(this);
   }
+  nsTArray<WeakFrame*> toRemove(mHeapWeakFrames.Count());
+  for (auto iter = mHeapWeakFrames.Iter(); !iter.Done(); iter.Next()) {
+    toRemove.AppendElement(iter.Get()->GetKey());
+  }
+  for (WeakFrame* weakFrame : toRemove) {
+    weakFrame->Clear(this);
+  }
 
   // Let the style set do its cleanup.
   mStyleSet->Shutdown();
 
   if (mPresContext) {
     // We hold a reference to the pres context, and it holds a weak link back
     // to us. To avoid the pres context having a dangling reference, set its
     // pres shell to nullptr
@@ -2953,16 +2989,27 @@ PresShell::ClearFrameRefs(nsIFrame* aFra
   while (weakFrame) {
     AutoWeakFrame* prev = weakFrame->GetPreviousWeakFrame();
     if (weakFrame->GetFrame() == aFrame) {
       // This removes weakFrame from mWeakFrames.
       weakFrame->Clear(this);
     }
     weakFrame = prev;
   }
+
+  AutoTArray<WeakFrame*, 4> toRemove;
+  for (auto iter = mHeapWeakFrames.Iter(); !iter.Done(); iter.Next()) {
+    WeakFrame* weakFrame = iter.Get()->GetKey();
+    if (weakFrame->GetFrame() == aFrame) {
+      toRemove.AppendElement(weakFrame);
+    }
+  }
+  for (WeakFrame* weakFrame : toRemove) {
+    weakFrame->Clear(this);
+  }
 }
 
 already_AddRefed<gfxContext>
 PresShell::CreateReferenceRenderingContext()
 {
   nsDeviceContext* devCtx = mPresContext->DeviceContext();
   RefPtr<gfxContext> rc;
   if (mPresContext->IsScreen()) {
--- a/layout/base/nsIPresShell.h
+++ b/layout/base/nsIPresShell.h
@@ -74,16 +74,17 @@ class nsFrameSelection;
 class nsFrameManager;
 class nsILayoutHistoryState;
 class nsIReflowCallback;
 class nsIDOMNode;
 class nsCSSFrameConstructor;
 class nsISelection;
 template<class E> class nsCOMArray;
 class AutoWeakFrame;
+class WeakFrame;
 class nsIScrollableFrame;
 class gfxContext;
 class nsIDOMEvent;
 class nsDisplayList;
 class nsDisplayListBuilder;
 class nsPIDOMWindowOuter;
 struct nsPoint;
 class nsINode;
@@ -1189,37 +1190,57 @@ public:
   virtual already_AddRefed<mozilla::gfx::SourceSurface>
   RenderSelection(nsISelection* aSelection,
                   const mozilla::LayoutDeviceIntPoint aPoint,
                   mozilla::LayoutDeviceIntRect* aScreenRect,
                   uint32_t aFlags) = 0;
 
   void AddWeakFrameInternal(AutoWeakFrame* aWeakFrame);
   virtual void AddWeakFrameExternal(AutoWeakFrame* aWeakFrame);
+  void AddWeakFrameInternal(WeakFrame* aWeakFrame);
+  virtual void AddWeakFrameExternal(WeakFrame* aWeakFrame);
 
   void AddWeakFrame(AutoWeakFrame* aWeakFrame)
   {
 #ifdef MOZILLA_INTERNAL_API
     AddWeakFrameInternal(aWeakFrame);
 #else
     AddWeakFrameExternal(aWeakFrame);
 #endif
   }
+  void AddWeakFrame(WeakFrame* aWeakFrame)
+  {
+#ifdef MOZILLA_INTERNAL_API
+    AddWeakFrameInternal(aWeakFrame);
+#else
+    AddWeakFrameExternal(aWeakFrame);
+#endif
+  }
 
   void RemoveWeakFrameInternal(AutoWeakFrame* aWeakFrame);
   virtual void RemoveWeakFrameExternal(AutoWeakFrame* aWeakFrame);
+  void RemoveWeakFrameInternal(WeakFrame* aWeakFrame);
+  virtual void RemoveWeakFrameExternal(WeakFrame* aWeakFrame);
 
   void RemoveWeakFrame(AutoWeakFrame* aWeakFrame)
   {
 #ifdef MOZILLA_INTERNAL_API
     RemoveWeakFrameInternal(aWeakFrame);
 #else
     RemoveWeakFrameExternal(aWeakFrame);
 #endif
   }
+  void RemoveWeakFrame(WeakFrame* aWeakFrame)
+  {
+#ifdef MOZILLA_INTERNAL_API
+    RemoveWeakFrameInternal(aWeakFrame);
+#else
+    RemoveWeakFrameExternal(aWeakFrame);
+#endif
+  }
 
 #ifdef DEBUG
   nsIFrame* GetDrawEventTargetFrame() { return mDrawEventTargetFrame; }
 #endif
 
   /**
    * Stop or restart non synthetic test mouse event handling on *all*
    * presShells.
@@ -1810,18 +1831,21 @@ protected:
 #endif
 
 
   // Count of the number of times this presshell has been painted to a window.
   uint64_t                  mPaintCount;
 
   nsSize                    mScrollPositionClampingScrollPortSize;
 
-  // A list of weak frames. This is a pointer to the last item in the list.
-  AutoWeakFrame*              mWeakFrames;
+  // A list of stack weak frames. This is a pointer to the last item in the list.
+  AutoWeakFrame*            mWeakFrames;
+
+  // A hash table of heap weak frames.
+  nsTHashtable<nsPtrHashKey<WeakFrame>> mHeapWeakFrames;
 
   // Most recent canvas background color.
   nscolor                   mCanvasBackgroundColor;
 
   // Used to force allocation and rendering of proportionally more or
   // less pixels in both dimensions.
   mozilla::Maybe<float>     mResolution;
 
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -401,32 +401,54 @@ nsIFrame::FindCloserFrameForSelection(
   }
 }
 
 void
 nsIFrame::ContentStatesChanged(mozilla::EventStates aStates)
 {
 }
 
+AutoWeakFrame::AutoWeakFrame(const WeakFrame& aOther)
+  : mPrev(nullptr), mFrame(nullptr)
+{
+  Init(aOther.GetFrame());
+}
+
 void
 AutoWeakFrame::Init(nsIFrame* aFrame)
 {
   Clear(mFrame ? mFrame->PresContext()->GetPresShell() : nullptr);
   mFrame = aFrame;
   if (mFrame) {
     nsIPresShell* shell = mFrame->PresContext()->GetPresShell();
     NS_WARNING_ASSERTION(shell, "Null PresShell in AutoWeakFrame!");
     if (shell) {
       shell->AddWeakFrame(this);
     } else {
       mFrame = nullptr;
     }
   }
 }
 
+void
+WeakFrame::Init(nsIFrame* aFrame)
+{
+  Clear(mFrame ? mFrame->PresContext()->GetPresShell() : nullptr);
+  mFrame = aFrame;
+  if (mFrame) {
+    nsIPresShell* shell = mFrame->PresContext()->GetPresShell();
+    MOZ_ASSERT(shell, "Null PresShell in WeakFrame!");
+    if (shell) {
+      shell->AddWeakFrame(this);
+    } else {
+      mFrame = nullptr;
+    }
+  }
+}
+
 nsIFrame*
 NS_NewEmptyFrame(nsIPresShell* aPresShell, nsStyleContext* aContext)
 {
   return new (aPresShell) nsFrame(aContext);
 }
 
 nsFrame::nsFrame(nsStyleContext* aContext)
 {
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -3800,36 +3800,43 @@ public:
 #endif
 };
 
 //----------------------------------------------------------------------
 
 /**
  * AutoWeakFrame can be used to keep a reference to a nsIFrame in a safe way.
  * Whenever an nsIFrame object is deleted, the AutoWeakFrames pointing
- * to it will be cleared.
+ * to it will be cleared.  AutoWeakFrame is for variables on the stack or
+ * in static storage only, there is also a WeakFrame below for heap uses.
  *
  * Create AutoWeakFrame object when it is sure that nsIFrame object
  * is alive and after some operations which may destroy the nsIFrame
  * (for example any DOM modifications) use IsAlive() or GetFrame() methods to
  * check whether it is safe to continue to use the nsIFrame object.
  *
  * @note The usage of this class should be kept to a minimum.
  */
-
-class AutoWeakFrame {
+class WeakFrame;
+class AutoWeakFrame
+{
 public:
-  AutoWeakFrame() : mPrev(nullptr), mFrame(nullptr) { }
-
-  AutoWeakFrame(const AutoWeakFrame& aOther) : mPrev(nullptr), mFrame(nullptr)
+  explicit AutoWeakFrame()
+    : mPrev(nullptr), mFrame(nullptr) {}
+
+  AutoWeakFrame(const AutoWeakFrame& aOther)
+    : mPrev(nullptr), mFrame(nullptr)
   {
     Init(aOther.GetFrame());
   }
 
-  MOZ_IMPLICIT AutoWeakFrame(nsIFrame* aFrame) : mPrev(nullptr), mFrame(nullptr)
+  MOZ_IMPLICIT AutoWeakFrame(const WeakFrame& aOther);
+
+  MOZ_IMPLICIT AutoWeakFrame(nsIFrame* aFrame)
+    : mPrev(nullptr), mFrame(nullptr)
   {
     Init(aFrame);
   }
 
   AutoWeakFrame& operator=(AutoWeakFrame& aOther) {
     Init(aOther.GetFrame());
     return *this;
   }
@@ -3865,20 +3872,83 @@ public:
 
   void SetPreviousWeakFrame(AutoWeakFrame* aPrev) { mPrev = aPrev; }
 
   ~AutoWeakFrame()
   {
     Clear(mFrame ? mFrame->PresContext()->GetPresShell() : nullptr);
   }
 private:
+  // Not available for the heap!
+  void* operator new(size_t) = delete;
+  void* operator new[](size_t) = delete;
+  void operator delete(void*) = delete;
+  void operator delete[](void*) = delete;
+
   void Init(nsIFrame* aFrame);
 
   AutoWeakFrame*  mPrev;
-  nsIFrame*     mFrame;
+  nsIFrame*       mFrame;
+};
+
+/**
+ * @see AutoWeakFrame
+ */
+class WeakFrame
+{
+public:
+  WeakFrame() : mFrame(nullptr) {}
+
+  WeakFrame(const WeakFrame& aOther) : mFrame(nullptr)
+  {
+    Init(aOther.GetFrame());
+  }
+
+  MOZ_IMPLICIT WeakFrame(const AutoWeakFrame& aOther) : mFrame(nullptr)
+  {
+    Init(aOther.GetFrame());
+  }
+
+  MOZ_IMPLICIT WeakFrame(nsIFrame* aFrame) : mFrame(nullptr)
+  {
+    Init(aFrame);
+  }
+
+  ~WeakFrame()
+  {
+    Clear(mFrame ? mFrame->PresContext()->GetPresShell() : nullptr);
+  }
+
+  WeakFrame& operator=(WeakFrame& aOther) {
+    Init(aOther.GetFrame());
+    return *this;
+  }
+
+  WeakFrame& operator=(nsIFrame* aFrame) {
+    Init(aFrame);
+    return *this;
+  }
+
+  nsIFrame* operator->() { return mFrame; }
+  operator nsIFrame*() { return mFrame; }
+
+  void Clear(nsIPresShell* aShell) {
+    if (aShell) {
+      aShell->RemoveWeakFrame(this);
+    }
+    mFrame = nullptr;
+  }
+
+  bool IsAlive() { return !!mFrame; }
+  nsIFrame* GetFrame() const { return mFrame; }
+
+private:
+  void Init(nsIFrame* aFrame);
+
+  nsIFrame* mFrame;
 };
 
 inline bool
 nsFrameList::ContinueRemoveFrame(nsIFrame* aFrame)
 {
   MOZ_ASSERT(!aFrame->GetPrevSibling() || !aFrame->GetNextSibling(),
              "Forgot to call StartRemoveFrame?");
   if (aFrame == mLastChild) {