Rewrite how we deal with nested FrameProperties
authorMatt Woodrow <mwoodrow@mozilla.com>
Tue, 24 Oct 2017 22:54:29 +1300
changeset 685681 1307b5c1322045962a28f750101d73586f75ccf3
parent 685680 912061a6ab946b861a6d85219a25e831f8782535
child 685682 50e8ae90137a2d2164a2e9d383c660bf43c6ad2b
push id86010
push userbmo:ethlin@mozilla.com
push dateWed, 25 Oct 2017 00:44:42 +0000
milestone58.0a1
Rewrite how we deal with nested FrameProperties
layout/base/FrameProperties.h
layout/painting/nsDisplayList.h
--- a/layout/base/FrameProperties.h
+++ b/layout/base/FrameProperties.h
@@ -147,17 +147,16 @@ public:
   template<typename T>
   using Descriptor = const FramePropertyDescriptor<T>*;
   using UntypedDescriptor = const FramePropertyDescriptorUntyped*;
 
   template<typename T>
   using PropertyType = typename detail::FramePropertyTypeHelper<T>::Type;
 
   explicit FrameProperties()
-    : mInDeleteAll(false)
   {
   }
 
   ~FrameProperties()
   {
     MOZ_ASSERT(mProperties.Length() == 0, "forgot to delete properties");
   }
 
@@ -284,24 +283,22 @@ public:
       }
     }
   }
 
   /**
    * Remove and destroy all property values for the frame.
    */
   void DeleteAll(const nsIFrame* aFrame) {
-    mozilla::DebugOnly<size_t> len = mProperties.Length();
-    mInDeleteAll = true;
-    for (auto& prop : mProperties) {
+    nsTArray<PropertyValue> toDelete;
+    toDelete.SwapElements(mProperties);
+    for (auto& prop : toDelete) {
       prop.DestroyValueFor(aFrame);
-      MOZ_ASSERT(mProperties.Length() == len);
     }
-    mInDeleteAll = false;
-    mProperties.Clear();
+    MOZ_ASSERT(mProperties.IsEmpty(), "a property dtor added new properties");
   }
 
   size_t SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const {
     // We currently report only the shallow size of the mProperties array.
     // As for the PropertyValue entries: we don't need to measure the mProperty
     // field of because it always points to static memory, and we can't measure
     // mValue because the type is opaque.
     // XXX Can we do better, e.g. with a method on the descriptor?
@@ -369,19 +366,16 @@ private:
    * Stores a property descriptor/value pair.
    */
   struct PropertyValue {
     PropertyValue() : mProperty(nullptr), mValue(nullptr) {}
     PropertyValue(UntypedDescriptor aProperty, void* aValue)
       : mProperty(aProperty), mValue(aValue) {}
 
     void DestroyValueFor(const nsIFrame* aFrame) {
-      if (!mProperty) {
-        return;
-      }
       if (mProperty->mDestructor) {
         mProperty->mDestructor(mValue);
       } else if (mProperty->mDestructorWithFrame) {
         mProperty->mDestructorWithFrame(aFrame, mValue);
       }
     }
 
     UntypedDescriptor mProperty;
@@ -401,17 +395,16 @@ private:
       return a == b.mProperty;
     }
     bool Equals(const PropertyValue& a, UntypedDescriptor b) const {
       return a.mProperty == b;
     }
   };
 
   nsTArray<PropertyValue> mProperties;
-  bool mInDeleteAll;
 };
 
 
 inline void*
 FrameProperties::GetInternal(UntypedDescriptor aProperty,
                              bool* aFoundResult) const
 {
   MOZ_ASSERT(aProperty, "Null property?");
@@ -471,42 +464,30 @@ FrameProperties::RemoveInternal(UntypedD
     return nullptr;
   }
 
   if (aFoundResult) {
     *aFoundResult = true;
   }
 
   void* result = mProperties.ElementAt(index).mValue;
-
-  if (mInDeleteAll) {
-    mProperties.ElementAt(index).mProperty = nullptr;
-    mProperties.ElementAt(index).mValue = nullptr;
-  } else {
-    mProperties.RemoveElementAt(index);
-  }
+  mProperties.RemoveElementAt(index);
 
   return result;
 }
 
 inline void
 FrameProperties::DeleteInternal(UntypedDescriptor aProperty,
                                 const nsIFrame* aFrame)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aProperty, "Null property?");
 
   auto index = mProperties.IndexOf(aProperty, 0, PropertyComparator());
   if (index != nsTArray<PropertyValue>::NoIndex) {
-    PropertyValue pv = mProperties.ElementAt(index);
-    if (mInDeleteAll) {
-      mProperties.ElementAt(index).mProperty = nullptr;
-      mProperties.ElementAt(index).mValue = nullptr;
-    } else {
-      mProperties.RemoveElementAt(index);
-    }
-    pv.DestroyValueFor(aFrame);
+    mProperties.ElementAt(index).DestroyValueFor(aFrame);
+    mProperties.RemoveElementAt(index);
   }
 }
 
 } // namespace mozilla
 
 #endif /* FRAMEPROPERTIES_H_ */
--- a/layout/painting/nsDisplayList.h
+++ b/layout/painting/nsDisplayList.h
@@ -190,27 +190,29 @@ struct AnimatedGeometryRoot
   RefPtr<AnimatedGeometryRoot> mParentAGR;
   bool mIsAsync;
   bool mIsRetained;
 
 protected:
   static void DetachAGR(AnimatedGeometryRoot* aAGR) {
     aAGR->mFrame = nullptr;
     aAGR->mParentAGR = nullptr;
+    NS_RELEASE(aAGR);
   }
   NS_DECLARE_FRAME_PROPERTY_WITH_DTOR(AnimatedGeometryRootCache, AnimatedGeometryRoot, DetachAGR)
 
   AnimatedGeometryRoot(nsIFrame* aFrame, AnimatedGeometryRoot* aParent, bool aIsAsync, bool aIsRetained)
     : mFrame(aFrame)
     , mParentAGR(aParent)
     , mIsAsync(aIsAsync)
     , mIsRetained(aIsRetained)
   {
     MOZ_ASSERT(mParentAGR || mIsAsync, "The root AGR should always be treated as an async AGR.");
     if (mIsRetained) {
+      NS_ADDREF(this);
       aFrame->SetProperty(AnimatedGeometryRootCache(), this);
     }
   }
 
   ~AnimatedGeometryRoot()
   {
     if (mFrame && mIsRetained) {
       mFrame->DeleteProperty(AnimatedGeometryRootCache());
@@ -248,17 +250,18 @@ struct ActiveScrolledRoot {
     if (aIsRetained) {
       asr = f->GetProperty(ActiveScrolledRootCache());
     }
 
     if (!asr) {
       asr = new ActiveScrolledRoot();
 
       if (aIsRetained) {
-        f->SetProperty(ActiveScrolledRootCache(), asr);
+        RefPtr<ActiveScrolledRoot> ref = asr;
+        f->SetProperty(ActiveScrolledRootCache(), ref.forget().take());
       }
     }
     asr->mParent = aParent;
     asr->mScrollableFrame = aScrollableFrame;
     asr->mDepth = aParent ? aParent->mDepth + 1 : 1;
     asr->mRetained = aIsRetained;
 
     return asr.forget();
@@ -302,16 +305,17 @@ private:
       nsIFrame* f = do_QueryFrame(mScrollableFrame);
       f->DeleteProperty(ActiveScrolledRootCache());
     }
   }
 
   static void DetachASR(ActiveScrolledRoot* aASR) {
     aASR->mParent = nullptr;
     aASR->mScrollableFrame = nullptr;
+    NS_RELEASE(aASR);
   }
   NS_DECLARE_FRAME_PROPERTY_WITH_DTOR(ActiveScrolledRootCache, ActiveScrolledRoot, DetachASR)
 
   static uint32_t Depth(const ActiveScrolledRoot* aActiveScrolledRoot) {
     return aActiveScrolledRoot ? aActiveScrolledRoot->mDepth : 0;
   }
 
   uint32_t mDepth;