Bug 1368249 - Create nsIFrame::AddProperty API for use when the property is known to not already exist, and use to optimize some call sites. r=mats
authorJonathan Kew <jkew@mozilla.com>
Sun, 28 May 2017 13:16:55 +0100
changeset 409176 ac905e4037e15b594d90703d87874ac0a91f557f
parent 409175 db3cc5f0ecaebd15f5eb04c4387fe7dbe36d87f7
child 409177 0dbfdf0816b63b01f1076f118aad79b29a3cef22
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats
bugs1368249
milestone55.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 1368249 - Create nsIFrame::AddProperty API for use when the property is known to not already exist, and use to optimize some call sites. r=mats
layout/base/FrameProperties.cpp
layout/base/FrameProperties.h
layout/base/RestyleManager.cpp
layout/base/nsCSSFrameConstructor.cpp
layout/generic/ReflowInput.cpp
layout/generic/nsFrame.cpp
layout/generic/nsIFrame.h
--- a/layout/base/FrameProperties.cpp
+++ b/layout/base/FrameProperties.cpp
@@ -28,16 +28,28 @@ FrameProperties::SetInternal(UntypedDesc
   }
 
   mProperties.AppendElement(PropertyValue(aProperty, aValue));
 #ifdef DEBUG
   mMaxLength = std::max<uint32_t>(mMaxLength, mProperties.Length());
 #endif
 }
 
+void
+FrameProperties::AddInternal(UntypedDescriptor aProperty, void* aValue)
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  MOZ_ASSERT(aProperty, "Null property?");
+
+  mProperties.AppendElement(PropertyValue(aProperty, aValue));
+#ifdef DEBUG
+  mMaxLength = std::max<uint32_t>(mMaxLength, mProperties.Length());
+#endif
+}
+
 void*
 FrameProperties::RemoveInternal(UntypedDescriptor aProperty, bool* aFoundResult)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aProperty, "Null property?");
 
   nsTArray<PropertyValue>::index_type index =
     mProperties.IndexOf(aProperty, 0, PropertyComparator());
--- a/layout/base/FrameProperties.h
+++ b/layout/base/FrameProperties.h
@@ -171,16 +171,27 @@ public:
   void Set(Descriptor<T> aProperty, PropertyType<T> aValue,
            const nsIFrame* aFrame)
   {
     void* ptr = ReinterpretHelper<T>::ToPointer(aValue);
     SetInternal(aProperty, ptr, aFrame);
   }
 
   /**
+   * Add a property value; the descriptor MUST NOT already be present.
+   */
+  template<typename T>
+  void Add(Descriptor<T> aProperty, PropertyType<T> aValue)
+  {
+    MOZ_ASSERT(!Has(aProperty), "duplicate frame property");
+    void* ptr = ReinterpretHelper<T>::ToPointer(aValue);
+    AddInternal(aProperty, ptr);
+  }
+
+  /**
    * @return true if @aProperty is set. This requires a linear search through the
    * properties of the frame.
    *
    * In most cases, this shouldn't be used outside of assertions, because if
    * you're doing a lookup anyway it would be far more efficient to call Get()
    * or Remove() and check the aFoundResult outparam to find out whether the
    * property is set. Legitimate non-assertion uses include:
    *
@@ -194,17 +205,18 @@ public:
    * The HasSkippingBitCheck variant doesn't test NS_FRAME_HAS_PROPERTIES
    * on aFrame, so it is safe to call after aFrame has been destroyed as
    * long as, since that destruction happened, it isn't possible for a
    * new frame to have been created and the same property added.
    */
   template<typename T>
   bool Has(Descriptor<T> aProperty) const
   {
-    return mProperties.IndexOf(aProperty, 0, PropertyComparator()) != nsTArray<PropertyValue>::NoIndex;
+    return mProperties.IndexOf(aProperty, 0, PropertyComparator())
+           != nsTArray<PropertyValue>::NoIndex;
   }
 
   /**
    * Get a property value. This requires a linear search through
    * the properties of the frame. If the frame has no such property,
    * returns zero-filled result, which means null for pointers and
    * zero for integers and floating point types.
    * @param aFoundResult if non-null, receives a value 'true' iff
@@ -263,16 +275,18 @@ private:
   // Prevent copying of FrameProperties; we should always return/pass around
   // references to it, not copies!
   FrameProperties(const FrameProperties&) = delete;
   FrameProperties& operator=(const FrameProperties&) = delete;
 
   void SetInternal(UntypedDescriptor aProperty, void* aValue,
                    const nsIFrame* aFrame);
 
+  void AddInternal(UntypedDescriptor aProperty, void* aValue);
+
   inline void*
   GetInternal(UntypedDescriptor aProperty, bool* aFoundResult) const;
 
   void* RemoveInternal(UntypedDescriptor aProperty, bool* aFoundResult);
 
   void DeleteInternal(UntypedDescriptor aProperty, const nsIFrame* aFrame);
 
   template<typename T>
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -798,19 +798,20 @@ RecomputePosition(nsIFrame* aFrame)
         NS_ASSERTION(newOffsets.left == -newOffsets.right &&
                      newOffsets.top == -newOffsets.bottom,
                      "ComputeRelativeOffsets should return valid results");
 
         // ReflowInput::ApplyRelativePositioning would work here, but
         // since we've already checked mPosition and aren't changing the frame's
         // normal position, go ahead and add the offsets directly.
         // First, we need to ensure that the normal position is stored though.
-        nsPoint normalPosition = cont->GetNormalPosition();
-        if (!cont->GetProperty(nsIFrame::NormalPositionProperty())) {
-          cont->SetProperty(nsIFrame::NormalPositionProperty(),
+        bool hasProperty;
+        nsPoint normalPosition = cont->GetNormalPosition(&hasProperty);
+        if (!hasProperty) {
+          cont->AddProperty(nsIFrame::NormalPositionProperty(),
                             new nsPoint(normalPosition));
         }
         cont->SetPosition(normalPosition +
                           nsPoint(newOffsets.left, newOffsets.top));
       }
     }
 
     return true;
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -6172,17 +6172,17 @@ AddGenConPseudoToFrame(nsIFrame* aOwnerF
 
   // FIXME(emilio): Remove this property, and use the frame of the generated
   // content itself to tear the content down? It should be quite simpler.
 
   nsIFrame::ContentArray* value =
     aOwnerFrame->GetProperty(nsIFrame::GenConProperty());
   if (!value) {
     value = new nsIFrame::ContentArray;
-    aOwnerFrame->SetProperty(nsIFrame::GenConProperty(), value);
+    aOwnerFrame->AddProperty(nsIFrame::GenConProperty(), value);
   }
   value->AppendElement(aContent);
 }
 
 /**
  * Return true if the frame construction item pointed to by aIter will
  * create a frame adjacent to a line boundary in the frame tree, and that
  * line boundary is induced by a content node adjacent to the frame's
--- a/layout/generic/ReflowInput.cpp
+++ b/layout/generic/ReflowInput.cpp
@@ -1015,17 +1015,17 @@ ReflowInput::ComputeRelativeOffsets(Writ
 
   // Convert the offsets to physical coordinates and store them on the frame
   aComputedOffsets = offsets.GetPhysicalMargin(aWM);
   nsMargin* physicalOffsets =
     aFrame->GetProperty(nsIFrame::ComputedOffsetProperty());
   if (physicalOffsets) {
     *physicalOffsets = aComputedOffsets;
   } else {
-    aFrame->SetProperty(nsIFrame::ComputedOffsetProperty(),
+    aFrame->AddProperty(nsIFrame::ComputedOffsetProperty(),
                         new nsMargin(aComputedOffsets));
   }
 }
 
 /* static */ void
 ReflowInput::ApplyRelativePositioning(nsIFrame* aFrame,
                                             const nsMargin& aComputedOffsets,
                                             nsPoint* aPosition)
@@ -1040,17 +1040,17 @@ ReflowInput::ApplyRelativePositioning(ns
   }
 
   // Store the normal position
   nsPoint* normalPosition =
     aFrame->GetProperty(nsIFrame::NormalPositionProperty());
   if (normalPosition) {
     *normalPosition = *aPosition;
   } else {
-    aFrame->SetProperty(nsIFrame::NormalPositionProperty(),
+    aFrame->AddProperty(nsIFrame::NormalPositionProperty(),
                         new nsPoint(*aPosition));
   }
 
   const nsStyleDisplay* display = aFrame->StyleDisplay();
   if (NS_STYLE_POSITION_RELATIVE == display->mPosition) {
     *aPosition += nsPoint(aComputedOffsets.left, aComputedOffsets.top);
   } else if (NS_STYLE_POSITION_STICKY == display->mPosition &&
              !aFrame->GetNextContinuation() &&
@@ -2514,17 +2514,17 @@ UpdateProp(nsIFrame* aFrame,
            bool aNeeded,
            nsMargin& aNewValue)
 {
   if (aNeeded) {
     nsMargin* propValue = aFrame->GetProperty(aProperty);
     if (propValue) {
       *propValue = aNewValue;
     } else {
-      aFrame->SetProperty(aProperty, new nsMargin(aNewValue));
+      aFrame->AddProperty(aProperty, new nsMargin(aNewValue));
     }
   } else {
     aFrame->DeleteProperty(aProperty);
   }
 }
 
 void
 SizeComputationInput::InitOffsets(WritingMode aWM,
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -944,36 +944,36 @@ nsFrame::DidSetStyleContext(nsStyleConte
     // calls GetUsed(Margin|Border|Padding)() before the next reflow, we
     // can give an accurate answer.
     // We don't want to set the property if one already exists.
     nsMargin oldValue(0, 0, 0, 0);
     nsMargin newValue(0, 0, 0, 0);
     const nsStyleMargin* oldMargin = aOldStyleContext->PeekStyleMargin();
     if (oldMargin && oldMargin->GetMargin(oldValue)) {
       if ((!StyleMargin()->GetMargin(newValue) || oldValue != newValue) &&
-          !GetProperty(UsedMarginProperty())) {
-        SetProperty(UsedMarginProperty(), new nsMargin(oldValue));
+          !HasProperty(UsedMarginProperty())) {
+        AddProperty(UsedMarginProperty(), new nsMargin(oldValue));
       }
     }
 
     const nsStylePadding* oldPadding = aOldStyleContext->PeekStylePadding();
     if (oldPadding && oldPadding->GetPadding(oldValue)) {
       if ((!StylePadding()->GetPadding(newValue) || oldValue != newValue) &&
-          !GetProperty(UsedPaddingProperty())) {
-        SetProperty(UsedPaddingProperty(), new nsMargin(oldValue));
+          !HasProperty(UsedPaddingProperty())) {
+        AddProperty(UsedPaddingProperty(), new nsMargin(oldValue));
       }
     }
 
     const nsStyleBorder* oldBorder = aOldStyleContext->PeekStyleBorder();
     if (oldBorder) {
       oldValue = oldBorder->GetComputedBorder();
       newValue = StyleBorder()->GetComputedBorder();
       if (oldValue != newValue &&
-          !GetProperty(UsedBorderProperty())) {
-        SetProperty(UsedBorderProperty(), new nsMargin(oldValue));
+          !HasProperty(UsedBorderProperty())) {
+        AddProperty(UsedBorderProperty(), new nsMargin(oldValue));
       }
     }
   }
 
   ImageLoader* imageLoader = PresContext()->Document()->StyleImageLoader();
   imgIRequest *oldBorderImage = aOldStyleContext
     ? aOldStyleContext->StyleBorder()->GetBorderImageRequest()
     : nullptr;
@@ -6564,17 +6564,17 @@ nsIFrame::InvalidateFrameWithRect(const 
   if (HasAnyStateBits(NS_FRAME_HAS_INVALID_RECT)) {
     rect = GetProperty(InvalidationRect());
     MOZ_ASSERT(rect);
   } else {
     if (alreadyInvalid) {
       return;
     }
     rect = new nsRect();
-    SetProperty(InvalidationRect(), rect);
+    AddProperty(InvalidationRect(), rect);
     AddStateBits(NS_FRAME_HAS_INVALID_RECT);
   }
 
   *rect = rect->Union(aRect);
 }
 
 /*static*/ uint8_t nsIFrame::sLayerIsPrerenderedDataKey;
 
@@ -6828,28 +6828,16 @@ nsIFrame::GetNormalRect() const
   nsPoint* normalPosition = GetProperty(NormalPositionProperty());
   if (normalPosition) {
     return nsRect(*normalPosition, GetSize());
   }
   return GetRect();
 }
 
 nsPoint
-nsIFrame::GetNormalPosition() const
-{
-  // It might be faster to first check
-  // StyleDisplay()->IsRelativelyPositionedStyle().
-  nsPoint* normalPosition = GetProperty(NormalPositionProperty());
-  if (normalPosition) {
-    return *normalPosition;
-  }
-  return GetPosition();
-}
-
-nsPoint
 nsIFrame::GetPositionIgnoringScrolling()
 {
   return GetParent() ? GetParent()->GetPositionOfChildIgnoringScrolling(this)
     : GetPosition();
 }
 
 nsRect
 nsIFrame::GetOverflowRect(nsOverflowType aType) const
@@ -8720,17 +8708,17 @@ nsIFrame::GetOverflowAreasProperty()
 
   if (overflow) {
     return overflow; // the property already exists
   }
 
   // The property isn't set yet, so allocate a new rect, set the property,
   // and return the newly allocated rect
   overflow = new nsOverflowAreas;
-  SetProperty(OverflowAreasProperty(), overflow);
+  AddProperty(OverflowAreasProperty(), overflow);
   return overflow;
 }
 
 /** Set the overflowArea rect, storing it as deltas or a separate rect
  * depending on its size in relation to the primary frame rect.
  */
 bool
 nsIFrame::SetOverflowAreas(const nsOverflowAreas& aOverflowAreas)
@@ -9017,17 +9005,17 @@ nsIFrame::FinishAndStoreOverflow(nsOverf
   // Store the passed in overflow area if we are a preserve-3d frame or we have
   // a transform, and it's not just the frame bounds.
   if (hasTransform || Combines3DTransformWithAncestors(disp)) {
     if (!aOverflowAreas.VisualOverflow().IsEqualEdges(bounds) ||
         !aOverflowAreas.ScrollableOverflow().IsEqualEdges(bounds)) {
       nsOverflowAreas* initial =
         GetProperty(nsIFrame::InitialOverflowProperty());
       if (!initial) {
-        SetProperty(nsIFrame::InitialOverflowProperty(),
+        AddProperty(nsIFrame::InitialOverflowProperty(),
                          new nsOverflowAreas(aOverflowAreas));
       } else if (initial != &aOverflowAreas) {
         *initial = aOverflowAreas;
       }
     } else {
       DeleteProperty(nsIFrame::InitialOverflowProperty());
     }
 #ifdef DEBUG
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -1061,19 +1061,22 @@ public:
   }
 
   /**
    * Return frame's rect without relative positioning
    */
   nsRect GetNormalRect() const;
 
   /**
-   * Return frame's position without relative positioning
-   */
-  nsPoint GetNormalPosition() const;
+   * Return frame's position without relative positioning.
+   * If aHasProperty is provided, returns whether the normal position
+   * was stored in a frame property.
+   */
+  inline nsPoint GetNormalPosition(bool* aHasProperty = nullptr) const;
+
   mozilla::LogicalPoint
   GetLogicalNormalPosition(mozilla::WritingMode aWritingMode,
                            const nsSize& aContainerSize) const
   {
     // Subtract the size of this frame from the container size to get
     // the correct position in rtl frames where the origin is on the
     // right instead of the left
     return mozilla::LogicalPoint(aWritingMode,
@@ -3400,26 +3403,39 @@ public:
   bool HasProperty(FrameProperties::Descriptor<T> aProperty) const
   {
     if (mProperties) {
       return mProperties->Has(aProperty);
     }
     return false;
   }
 
+  // Add a property, or update an existing property for the given descriptor.
   template<typename T>
   void SetProperty(FrameProperties::Descriptor<T> aProperty,
                    FrameProperties::PropertyType<T> aValue)
   {
     if (!mProperties) {
       mProperties = mozilla::MakeUnique<FrameProperties>();
     }
     mProperties->Set(aProperty, aValue, this);
   }
 
+  // Unconditionally add a property; use ONLY if the descriptor is known
+  // to NOT already be present.
+  template<typename T>
+  void AddProperty(FrameProperties::Descriptor<T> aProperty,
+                   FrameProperties::PropertyType<T> aValue)
+  {
+    if (!mProperties) {
+      mProperties = mozilla::MakeUnique<FrameProperties>();
+    }
+    mProperties->Add(aProperty, aValue);
+  }
+
   template<typename T>
   FrameProperties::PropertyType<T>
   RemoveProperty(FrameProperties::Descriptor<T> aProperty,
                  bool* aFoundResult = nullptr)
   {
     if (mProperties) {
       return mProperties->Remove(aProperty, aFoundResult);
     }
@@ -4539,9 +4555,27 @@ nsIFrame::IsFrameListSorted(nsFrameList&
     trailingIter.Next();
     iter.Next();
   }
 
   // We made it to the end without returning early, so the list is sorted.
   return true;
 }
 
+// Needs to be defined here rather than nsIFrameInlines.h, because it is used
+// within this header.
+nsPoint
+nsIFrame::GetNormalPosition(bool* aHasProperty) const
+{
+  nsPoint* normalPosition = GetProperty(NormalPositionProperty());
+  if (normalPosition) {
+    if (aHasProperty) {
+      *aHasProperty = true;
+    }
+    return *normalPosition;
+  }
+  if (aHasProperty) {
+    *aHasProperty = false;
+  }
+  return GetPosition();
+}
+
 #endif /* nsIFrame_h___ */