Bug 1353187 - Give frame properties the const-ness semantics of member variables. r=dholbert

This makes it so that, given a |const nsIFrame*|, a caller can retrieve
properties but not set or remove them, but with an |nsIFrame*| all
operations are allowed.  I believe this is sensible since properties act
as extended member variables for things that are needed rarely, and
these are the const-ness semantics of member variables.

This also avoids the need for const_cast<nsIFrame*> to cast away const
in the following patch, which guards property access with a frame state
bit.

MozReview-Commit-ID: IJ9JnGzdH51

--HG--
extra : transplant_source : %D4%DF%04%91_q%E6%CF%B3N%82%2C%A5%CB0%3A%B6%810%ED

layout/base/FramePropertyTable.cpp

@@ -13,7 +13,7 @@ namespace mozilla {
 
 void
 FramePropertyTable::SetInternal(
-  const nsIFrame* aFrame, UntypedDescriptor aProperty, void* aValue)
+  nsIFrame* aFrame, UntypedDescriptor aProperty, void* aValue)
 {
   MOZ_ASSERT(NS_IsMainThread());
   NS_ASSERTION(aFrame, "Null frame?");
@@ -111,7 +111,7 @@ FramePropertyTable::GetInternal(
 
 void*
 FramePropertyTable::RemoveInternal(
-  const nsIFrame* aFrame, UntypedDescriptor aProperty, bool* aFoundResult)
+  nsIFrame* aFrame, UntypedDescriptor aProperty, bool* aFoundResult)
 {
   MOZ_ASSERT(NS_IsMainThread());
   NS_ASSERTION(aFrame, "Null frame?");
@@ -176,7 +176,7 @@ FramePropertyTable::RemoveInternal(
 
 void
 FramePropertyTable::DeleteInternal(
-  const nsIFrame* aFrame, UntypedDescriptor aProperty)
+  nsIFrame* aFrame, UntypedDescriptor aProperty)
 {
   MOZ_ASSERT(NS_IsMainThread());
   NS_ASSERTION(aFrame, "Null frame?");
@@ -206,7 +206,7 @@ FramePropertyTable::DeleteAllForEntry(Entry* aEntry)
 }
 
 void
-FramePropertyTable::DeleteAllFor(const nsIFrame* aFrame)
+FramePropertyTable::DeleteAllFor(nsIFrame* aFrame)
 {
   NS_ASSERTION(aFrame, "Null frame?");
 

layout/base/FramePropertyTable.h

@@ -165,7 +165,7 @@ public:
    * is destroyed.
    */
   template<typename T>
-  void Set(const nsIFrame* aFrame, Descriptor<T> aProperty,
+  void Set(nsIFrame* aFrame, Descriptor<T> aProperty,
            PropertyType<T> aValue)
   {
     void* ptr = ReinterpretHelper<T>::ToPointer(aValue);
@@ -228,7 +228,7 @@ public:
    * 'property value is null'.
    */
   template<typename T>
-  PropertyType<T> Remove(const nsIFrame* aFrame, Descriptor<T> aProperty,
+  PropertyType<T> Remove(nsIFrame* aFrame, Descriptor<T> aProperty,
                          bool* aFoundResult = nullptr)
   {
     void* ptr = RemoveInternal(aFrame, aProperty, aFoundResult);
@@ -241,7 +241,7 @@ public:
    * property, nothing happens.
    */
   template<typename T>
-  void Delete(const nsIFrame* aFrame, Descriptor<T> aProperty)
+  void Delete(nsIFrame* aFrame, Descriptor<T> aProperty)
   {
     DeleteInternal(aFrame, aProperty);
   }
@@ -249,7 +249,7 @@ public:
    * Remove and destroy all property values for a frame. This requires one
    * hashtable lookup (using the frame as the key).
    */
-  void DeleteAllFor(const nsIFrame* aFrame);
+  void DeleteAllFor(nsIFrame* aFrame);
   /**
    * Remove and destroy all property values for all frames.
    */
@@ -258,16 +258,16 @@ public:
   size_t SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
 
 protected:
-  void SetInternal(const nsIFrame* aFrame, UntypedDescriptor aProperty,
+  void SetInternal(nsIFrame* aFrame, UntypedDescriptor aProperty,
                    void* aValue);
 
   void* GetInternal(const nsIFrame* aFrame, UntypedDescriptor aProperty,
                     bool* aFoundResult);
 
-  void* RemoveInternal(const nsIFrame* aFrame, UntypedDescriptor aProperty,
+  void* RemoveInternal(nsIFrame* aFrame, UntypedDescriptor aProperty,
                        bool* aFoundResult);
 
-  void DeleteInternal(const nsIFrame* aFrame, UntypedDescriptor aProperty);
+  void DeleteInternal(nsIFrame* aFrame, UntypedDescriptor aProperty);
 
   template<typename T>
   struct ReinterpretHelper
@@ -392,22 +392,21 @@ protected:
 };
 
 /**
- * This class encapsulates the properties of a frame.
+ * The FrameProperties/ConstFrameProperties class encapsulates the
+ * properties of a frame.
+ *
+ * However, since frame properties are like member variables, we have
+ * different versions for whether the frame is |const|, sharing a common
+ * base class.
+ *
+ * CVnsIFrame is either |nsIFrame| or |const nsIFrame|.
  */
-class FrameProperties {
+template<class CVnsIFrame>
+class FramePropertiesBase {
 public:
   template<typename T> using Descriptor = FramePropertyTable::Descriptor<T>;
   template<typename T> using PropertyType = FramePropertyTable::PropertyType<T>;
 
-  FrameProperties(FramePropertyTable* aTable, const nsIFrame* aFrame)
-    : mTable(aTable), mFrame(aFrame) {}
-
-  template<typename T>
-  void Set(Descriptor<T> aProperty, PropertyType<T> aValue) const
-  {
-    mTable->Set(mFrame, aProperty, aValue);
-  }
-
   template<typename T>
   bool Has(Descriptor<T> aProperty) const
   {
@@ -420,21 +419,49 @@ public:
   {
     return mTable->Get(mFrame, aProperty, aFoundResult);
   }
+
+protected:
+  FramePropertiesBase(FramePropertyTable* aTable, CVnsIFrame* aFrame)
+    : mTable(aTable), mFrame(aFrame) {}
+
+  FramePropertyTable* const mTable;
+  CVnsIFrame* const mFrame;
+};
+
+class ConstFrameProperties : public FramePropertiesBase<const nsIFrame> {
+public:
+  ConstFrameProperties(FramePropertyTable* aTable, const nsIFrame* aFrame)
+    : FramePropertiesBase(aTable, aFrame)
+  {
+  }
+};
+
+class FrameProperties : public FramePropertiesBase<nsIFrame> {
+public:
+  FrameProperties(FramePropertyTable* aTable, nsIFrame* aFrame)
+    : FramePropertiesBase(aTable, aFrame)
+  {
+  }
+
+  template<typename T>
+  void Set(Descriptor<T> aProperty, PropertyType<T> aValue) const
+  {
+    mTable->Set(mFrame, aProperty, aValue);
+  }
+
   template<typename T>
   PropertyType<T> Remove(Descriptor<T> aProperty,
                          bool* aFoundResult = nullptr) const
   {
     return mTable->Remove(mFrame, aProperty, aFoundResult);
   }
+
   template<typename T>
-  void Delete(Descriptor<T> aProperty)
+  void Delete(Descriptor<T> aProperty) const
   {
     mTable->Delete(mFrame, aProperty);
   }
 
-private:
-  FramePropertyTable* mTable;
-  const nsIFrame* mFrame;
 };
 
 } // namespace mozilla

layout/generic/nsFrame.cpp

@@ -1702,7 +1702,7 @@ nsIFrame::GetVisibility() const
   }
 
   bool isSet = false;
-  FrameProperties props = Properties();
+  ConstFrameProperties props = Properties();
   uint32_t visibleCount = props.Get(VisibilityStateProperty(), &isSet);
 
   MOZ_ASSERT(isSet, "Should have a VisibilityStateProperty value "

layout/generic/nsIFrame.h

@@ -577,6 +577,7 @@ public:
   using Visibility = mozilla::Visibility;
 
   typedef mozilla::FrameProperties FrameProperties;
+  typedef mozilla::ConstFrameProperties ConstFrameProperties;
   typedef mozilla::layers::Layer Layer;
   typedef mozilla::layout::FrameChildList ChildList;
   typedef mozilla::layout::FrameChildListID ChildListID;
@@ -3251,10 +3252,14 @@ public:
     return mContent == aParentContent;
   }
 
-  FrameProperties Properties() const {
+  FrameProperties Properties() {
     return FrameProperties(PresContext()->PropertyTable(), this);
   }
 
+  ConstFrameProperties Properties() const {
+    return ConstFrameProperties(PresContext()->PropertyTable(), this);
+  }
+
   /**
    * Return true if and only if this frame obeys visibility:hidden.
    * if it does not, then nsContainerFrame will hide its view even though

layout/generic/nsTextFrame.cpp

@@ -3204,7 +3204,7 @@ protected:
   const nsStyleText*              mTextStyle;
   const nsTextFragment*           mFrag;
   const nsIFrame*                 mLineContainer;
-  const nsTextFrame*              mFrame;
+  nsTextFrame*                    mFrame;
   gfxSkipCharsIterator            mStart;  // Offset in original and transformed string
   const gfxSkipCharsIterator      mTempIterator;
 

layout/mathml/nsMathMLmtableFrame.cpp

@@ -167,7 +167,7 @@ FindCellProperty(const nsIFrame* aCellFrame,
   nsTArray<int8_t>* propertyData = nullptr;
 
   while (currentFrame) {
-    FrameProperties props = currentFrame->Properties();
+    ConstFrameProperties props = currentFrame->Properties();
     propertyData = props.Get(aFrameProperty);
     bool frameIsTable = (currentFrame->GetType() == nsGkAtoms::tableFrame);
 

layout/tables/nsTableFrame.cpp

@@ -2794,11 +2794,17 @@ nsTableFrame::GetUsedMargin() const
 NS_DECLARE_FRAME_PROPERTY_DELETABLE(TableBCProperty, BCPropertyData)
 
 BCPropertyData*
-nsTableFrame::GetBCProperty(bool aCreateIfNecessary) const
+nsTableFrame::GetBCProperty() const
+{
+  return Properties().Get(TableBCProperty());
+}
+
+BCPropertyData*
+nsTableFrame::GetOrCreateBCProperty()
 {
   FrameProperties props = Properties();
   BCPropertyData* value = props.Get(TableBCProperty());
-  if (!value && aCreateIfNecessary) {
+  if (!value) {
     value = new BCPropertyData();
     props.Set(TableBCProperty(), value);
   }
@@ -4247,7 +4253,7 @@ nsTableFrame::AddBCDamageArea(const TableArea& aValue)
 
   SetNeedToCalcBCBorders(true);
   // Get the property
-  BCPropertyData* value = GetBCProperty(true);
+  BCPropertyData* value = GetOrCreateBCProperty();
   if (value) {
 #ifdef DEBUG
     VerifyNonNegativeDamageRect(value->mDamageArea);
@@ -4287,7 +4293,7 @@ nsTableFrame::SetFullBCDamageArea()
 
   SetNeedToCalcBCBorders(true);
 
-  BCPropertyData* value = GetBCProperty(true);
+  BCPropertyData* value = GetOrCreateBCProperty();
   if (value) {
     value->mDamageArea = TableArea(0, 0, GetColCount(), GetRowCount());
   }

layout/tables/nsTableFrame.h

@@ -807,7 +807,8 @@ protected:
 
   void SetBorderCollapse(bool aValue);
 
-  BCPropertyData* GetBCProperty(bool aCreateIfNecessary = false) const;
+  BCPropertyData* GetBCProperty() const;
+  BCPropertyData* GetOrCreateBCProperty();
   void SetFullBCDamageArea();
   void CalcBCBorders();