Backed out changeset 59c0589220ec (bug 1368249) unused function GetNormalPosition at nsIFrame.h:1073. r=backout
authorSebastian Hengst <archaeopteryx@coole-files.de>
Sun, 28 May 2017 15:38:15 +0200
changeset 585766 db3cc5f0ecaebd15f5eb04c4387fe7dbe36d87f7
parent 585765 59c0589220ecb10b820a5c165e33bfb0f98720a6
child 585767 ac905e4037e15b594d90703d87874ac0a91f557f
push id61188
push usermaglione.k@gmail.com
push dateSun, 28 May 2017 22:11:50 +0000
reviewersbackout
bugs1368249
milestone55.0a1
backs out59c0589220ecb10b820a5c165e33bfb0f98720a6
Backed out changeset 59c0589220ec (bug 1368249) unused function GetNormalPosition at nsIFrame.h:1073. r=backout
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
layout/generic/nsIFrameInlines.h
--- a/layout/base/FrameProperties.cpp
+++ b/layout/base/FrameProperties.cpp
@@ -28,28 +28,16 @@ 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,27 +171,16 @@ 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:
    *
@@ -205,18 +194,17 @@ 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
@@ -275,18 +263,16 @@ 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,20 +798,19 @@ 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.
-        bool hasProperty;
-        nsPoint normalPosition = cont->GetNormalPosition(&hasProperty);
-        if (!hasProperty) {
-          cont->AddProperty(nsIFrame::NormalPositionProperty(),
+        nsPoint normalPosition = cont->GetNormalPosition();
+        if (!cont->GetProperty(nsIFrame::NormalPositionProperty())) {
+          cont->SetProperty(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->AddProperty(nsIFrame::GenConProperty(), value);
+    aOwnerFrame->SetProperty(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->AddProperty(nsIFrame::ComputedOffsetProperty(),
+    aFrame->SetProperty(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->AddProperty(nsIFrame::NormalPositionProperty(),
+    aFrame->SetProperty(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->AddProperty(aProperty, new nsMargin(aNewValue));
+      aFrame->SetProperty(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) &&
-          !HasProperty(UsedMarginProperty())) {
-        AddProperty(UsedMarginProperty(), new nsMargin(oldValue));
+          !GetProperty(UsedMarginProperty())) {
+        SetProperty(UsedMarginProperty(), new nsMargin(oldValue));
       }
     }
 
     const nsStylePadding* oldPadding = aOldStyleContext->PeekStylePadding();
     if (oldPadding && oldPadding->GetPadding(oldValue)) {
       if ((!StylePadding()->GetPadding(newValue) || oldValue != newValue) &&
-          !HasProperty(UsedPaddingProperty())) {
-        AddProperty(UsedPaddingProperty(), new nsMargin(oldValue));
+          !GetProperty(UsedPaddingProperty())) {
+        SetProperty(UsedPaddingProperty(), new nsMargin(oldValue));
       }
     }
 
     const nsStyleBorder* oldBorder = aOldStyleContext->PeekStyleBorder();
     if (oldBorder) {
       oldValue = oldBorder->GetComputedBorder();
       newValue = StyleBorder()->GetComputedBorder();
       if (oldValue != newValue &&
-          !HasProperty(UsedBorderProperty())) {
-        AddProperty(UsedBorderProperty(), new nsMargin(oldValue));
+          !GetProperty(UsedBorderProperty())) {
+        SetProperty(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();
-    AddProperty(InvalidationRect(), rect);
+    SetProperty(InvalidationRect(), rect);
     AddStateBits(NS_FRAME_HAS_INVALID_RECT);
   }
 
   *rect = rect->Union(aRect);
 }
 
 /*static*/ uint8_t nsIFrame::sLayerIsPrerenderedDataKey;
 
@@ -6828,16 +6828,28 @@ 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
@@ -8708,17 +8720,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;
-  AddProperty(OverflowAreasProperty(), overflow);
+  SetProperty(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)
@@ -9005,17 +9017,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) {
-        AddProperty(nsIFrame::InitialOverflowProperty(),
+        SetProperty(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,22 +1061,19 @@ public:
   }
 
   /**
    * Return frame's rect without relative positioning
    */
   nsRect GetNormalRect() 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;
-
+   * Return frame's position without relative positioning
+   */
+  nsPoint GetNormalPosition() 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,
@@ -3403,39 +3400,26 @@ 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);
     }
--- a/layout/generic/nsIFrameInlines.h
+++ b/layout/generic/nsIFrameInlines.h
@@ -175,25 +175,9 @@ nsIFrame::GetInFlowParent()
   if (GetStateBits() & NS_FRAME_OUT_OF_FLOW) {
     nsFrameManager* fm = PresContext()->FrameManager();
     return fm->GetPlaceholderFrameFor(FirstContinuation())->GetParent();
   }
 
   return GetParent();
 }
 
-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