From 0212f3fee3b27c8559aa6e12af2348d65eb3bcfc Mon Sep 17 00:00:00 2001
From: Miko Mynttinen <mikokm@gmail.com>
Date: Tue, 2 Oct 2018 11:40:16 +0000
Subject: [PATCH] Bug 1494124 - Allow only one rect per frame in
 WeakFrameRegion r=mattwoodrow

Differential Revision: https://phabricator.services.mozilla.com/D7303

--HG--
extra : moz-landing-system : lando
---
 layout/painting/nsDisplayList.cpp | 40 ++++++++++++++-----------------
 layout/painting/nsDisplayList.h   | 38 ++++++++++++++++++++++-------
 2 files changed, 47 insertions(+), 31 deletions(-)

diff --git a/layout/painting/nsDisplayList.cpp b/layout/painting/nsDisplayList.cpp
index 825b1aa66ee7..2664c98326d9 100644
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -2046,46 +2046,42 @@ nsDisplayListBuilder::GetWindowDraggingRegion() const
 }
 
 /**
- * Removes modified frames and rects from |aRegion|.
+ * Removes modified frames and rects from this WeakFrameRegion.
  */
-static void
-RemoveModifiedFramesAndRects(nsDisplayListBuilder::WeakFrameRegion& aRegion)
+void
+nsDisplayListBuilder::WeakFrameRegion::RemoveModifiedFramesAndRects()
 {
-  std::vector<WeakFrame>& frames = aRegion.mFrames;
-  nsTArray<pixman_box32_t>& rects = aRegion.mRects;
-
-  MOZ_ASSERT(frames.size() == rects.Length());
+  MOZ_ASSERT(mFrames.Length() == mRects.Length());
 
   uint32_t i = 0;
-  uint32_t length = frames.size();
+  uint32_t length = mFrames.Length();
 
   while (i < length) {
-    WeakFrame& frame = frames[i];
+    auto& wrapper = mFrames[i];
 
-    if (!frame.IsAlive() || frame->IsFrameModified()) {
-      // To avoid O(n) shifts in the array, move the last element of the array
-      // to the current position and decrease the array length. Moving WeakFrame
-      // inside of the array causes a new WeakFrame to be created and registered
-      // with PresShell. We could avoid this by, for example, using a wrapper
-      // class for WeakFrame, or by storing raw  WeakFrame pointers.
-      frames[i] = frames[length - 1];
-      rects[i] = rects[length - 1];
+    if (!wrapper.mWeakFrame->IsAlive() ||
+        wrapper.mWeakFrame->GetFrame()->IsFrameModified()) {
+      // To avoid multiple O(n) shifts in the array, move the last element of
+      // the array to the current position and decrease the array length.
+      mFrameSet.RemoveEntry(wrapper.mFrame);
+      mFrames[i] = std::move(mFrames[length - 1]);
+      mRects[i] = std::move(mRects[length - 1]);
       length--;
     } else {
       i++;
     }
   }
 
-  frames.resize(length);
-  rects.TruncateLength(length);
+  mFrames.TruncateLength(length);
+  mRects.TruncateLength(length);
 }
 
 void
 nsDisplayListBuilder::RemoveModifiedWindowRegions()
 {
-  RemoveModifiedFramesAndRects(mRetainedWindowDraggingRegion);
-  RemoveModifiedFramesAndRects(mRetainedWindowNoDraggingRegion);
-  RemoveModifiedFramesAndRects(mWindowExcludeGlassRegion);
+  mRetainedWindowDraggingRegion.RemoveModifiedFramesAndRects();
+  mRetainedWindowNoDraggingRegion.RemoveModifiedFramesAndRects();
+  mWindowExcludeGlassRegion.RemoveModifiedFramesAndRects();
 }
 
 void
diff --git a/layout/painting/nsDisplayList.h b/layout/painting/nsDisplayList.h
index 917906bff1c4..a039836002a3 100644
--- a/layout/painting/nsDisplayList.h
+++ b/layout/painting/nsDisplayList.h
@@ -1921,27 +1921,47 @@ public:
    */
   struct WeakFrameRegion
   {
-    std::vector<WeakFrame> mFrames;
+    /**
+     * A wrapper to store WeakFrame and the pointer to the underlying frame.
+     * This is needed because WeakFrame does not store the frame pointer after
+     * the frame has been deleted.
+     */
+    struct WeakFrameWrapper {
+      explicit WeakFrameWrapper(nsIFrame* aFrame)
+        : mWeakFrame(new WeakFrame(aFrame))
+        , mFrame(aFrame)
+      {
+      }
+
+      mozilla::UniquePtr<WeakFrame> mWeakFrame;
+      void* mFrame;
+    };
+
+    nsTHashtable<nsPtrHashKey<void>> mFrameSet;
+    nsTArray<WeakFrameWrapper> mFrames;
     nsTArray<pixman_box32_t> mRects;
 
-    void Add(nsIFrame* aFrame, const nsRect& aRect)
+    template<typename RectType>
+    void Add(nsIFrame* aFrame, const RectType& aRect)
     {
-      mFrames.emplace_back(aFrame);
-      mRects.AppendElement(nsRegion::RectToBox(aRect));
-    }
+      if (mFrameSet.Contains(aFrame)) {
+        return;
+      }
 
-    void Add(nsIFrame* aFrame, const mozilla::gfx::IntRect& aRect)
-    {
-      mFrames.emplace_back(aFrame);
+      mFrameSet.PutEntry(aFrame);
+      mFrames.AppendElement(WeakFrameWrapper(aFrame));
       mRects.AppendElement(nsRegion::RectToBox(aRect));
     }
 
     void Clear()
     {
-      mFrames.clear();
+      mFrameSet.Clear();
+      mFrames.Clear();
       mRects.Clear();
     }
 
+    void RemoveModifiedFramesAndRects();
+
     typedef mozilla::gfx::ArrayView<pixman_box32_t> BoxArrayView;
 
     nsRegion ToRegion() const { return nsRegion(BoxArrayView(mRects)); }