Bug 1466963: Use consistent indentation in DeclarationBlock, and make the copy-constructor private. r?xidorn draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 05 Jun 2018 21:22:14 +0200
changeset 804470 970a8f4c4d34da0421ba473832c1dfbf40509fe8
parent 804469 d4eedc3d0727bc30c556d01c890d13f79f5d50f4
child 804471 749d1d7017634b0ebb9ccf49cb9fc0727b79b684
push id112373
push userbmo:emilio@crisal.io
push dateTue, 05 Jun 2018 22:12:13 +0000
reviewersxidorn
bugs1466963
milestone62.0a1
Bug 1466963: Use consistent indentation in DeclarationBlock, and make the copy-constructor private. r?xidorn MozReview-Commit-ID: JhTymV72jHn
layout/style/DeclarationBlock.cpp
layout/style/DeclarationBlock.h
--- a/layout/style/DeclarationBlock.cpp
+++ b/layout/style/DeclarationBlock.cpp
@@ -14,18 +14,18 @@ namespace mozilla {
 
 /* static */ already_AddRefed<DeclarationBlock>
 DeclarationBlock::FromCssText(const nsAString& aCssText,
                               URLExtraData* aExtraData,
                               nsCompatibility aMode,
                               css::Loader* aLoader)
 {
   NS_ConvertUTF16toUTF8 value(aCssText);
-  RefPtr<RawServoDeclarationBlock>
-      raw = Servo_ParseStyleAttribute(&value, aExtraData, aMode, aLoader).Consume();
+  RefPtr<RawServoDeclarationBlock> raw =
+      Servo_ParseStyleAttribute(&value, aExtraData, aMode, aLoader).Consume();
   RefPtr<DeclarationBlock> decl = new DeclarationBlock(raw.forget());
   return decl.forget();
 }
 
 // TODO: We can make them inline.
 void
 DeclarationBlock::GetPropertyValue(const nsAString& aProperty,
                                    nsAString& aValue) const
--- a/layout/style/DeclarationBlock.h
+++ b/layout/style/DeclarationBlock.h
@@ -23,61 +23,62 @@ namespace mozilla {
 
 namespace css {
 class Declaration;
 class Rule;
 } // namespace css
 
 class DeclarationBlock final
 {
-public:
-  explicit DeclarationBlock(
-    already_AddRefed<RawServoDeclarationBlock> aRaw)
-    : mRaw(aRaw)
-    , mImmutable(false)
-    , mIsDirty(false)
-  {
-    mContainer.mRaw = 0;
-  }
-
-  DeclarationBlock()
-    : DeclarationBlock(Servo_DeclarationBlock_CreateEmpty().Consume()) {}
-
   DeclarationBlock(const DeclarationBlock& aCopy)
     : mRaw(Servo_DeclarationBlock_Clone(aCopy.mRaw).Consume())
     , mImmutable(false)
     , mIsDirty(false)
   {
     mContainer.mRaw = 0;
   }
 
+public:
+  explicit DeclarationBlock(already_AddRefed<RawServoDeclarationBlock> aRaw)
+    : mRaw(aRaw)
+    , mImmutable(false)
+    , mIsDirty(false)
+  {
+    mContainer.mRaw = 0;
+  }
+
+  DeclarationBlock()
+    : DeclarationBlock(Servo_DeclarationBlock_CreateEmpty().Consume())
+  { }
+
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DeclarationBlock)
 
-  inline already_AddRefed<DeclarationBlock> Clone() const
+  already_AddRefed<DeclarationBlock> Clone() const
   {
     return do_AddRef(new DeclarationBlock(*this));
   }
 
   /**
    * Return whether |this| may be modified.
    */
-  bool IsMutable() const {
+  bool IsMutable() const
+  {
     return !mImmutable;
   }
 
   /**
-   * Crash if |this| cannot be modified.
+   * Crash in debug builds if |this| cannot be modified.
    */
-  void AssertMutable() const {
+  void AssertMutable() const
+  {
     MOZ_ASSERT(IsMutable(), "someone forgot to call EnsureMutable");
   }
 
   /**
-   * Mark this declaration as unmodifiable.  It's 'const' so it can
-   * be called from ToString.
+   * Mark this declaration as unmodifiable.
    */
   void SetImmutable() { mImmutable = true; }
 
   /**
    * Return whether |this| has been restyled after modified.
    */
   bool IsDirty() const { return mIsDirty; }
 
@@ -89,57 +90,65 @@ public:
   /**
    * Mark this declaration as not dirty.
    */
   void UnsetDirty() { mIsDirty = false; }
 
   /**
    * Copy |this|, if necessary to ensure that it can be modified.
    */
-  inline already_AddRefed<DeclarationBlock>
-  EnsureMutable()
+  already_AddRefed<DeclarationBlock> EnsureMutable()
   {
     if (!IsDirty()) {
       // In stylo, the old DeclarationBlock is stored in element's rule node tree
       // directly, to avoid new values replacing the DeclarationBlock in the tree
       // directly, we need to copy the old one here if we haven't yet copied.
       // As a result the new value does not replace rule node tree until traversal
       // happens.
+      //
+      // FIXME(emilio): This is a hack for ::first-line and transitions starting
+      // due to CSSOM changes when other transitions are already running. Try
+      // to simplify this setup.
       return Clone();
     }
 
     if (!IsMutable()) {
       return Clone();
     }
+
     return do_AddRef(this);
   }
 
-  void SetOwningRule(css::Rule* aRule) {
+  void SetOwningRule(css::Rule* aRule)
+  {
     MOZ_ASSERT(!mContainer.mOwningRule || !aRule,
                "should never overwrite one rule with another");
     mContainer.mOwningRule = aRule;
   }
 
-  css::Rule* GetOwningRule() const {
+  css::Rule* GetOwningRule() const
+  {
     if (mContainer.mRaw & 0x1) {
       return nullptr;
     }
     return mContainer.mOwningRule;
   }
 
-  void SetHTMLCSSStyleSheet(nsHTMLCSSStyleSheet* aHTMLCSSStyleSheet) {
+  void SetHTMLCSSStyleSheet(nsHTMLCSSStyleSheet* aHTMLCSSStyleSheet)
+  {
     MOZ_ASSERT(!mContainer.mHTMLCSSStyleSheet || !aHTMLCSSStyleSheet,
                "should never overwrite one sheet with another");
     mContainer.mHTMLCSSStyleSheet = aHTMLCSSStyleSheet;
     if (aHTMLCSSStyleSheet) {
       mContainer.mRaw |= uintptr_t(1);
     }
   }
 
-  nsHTMLCSSStyleSheet* GetHTMLCSSStyleSheet() const {
+  nsHTMLCSSStyleSheet* GetHTMLCSSStyleSheet() const
+  {
     if (!(mContainer.mRaw & 0x1)) {
       return nullptr;
     }
     auto c = mContainer;
     c.mRaw &= ~uintptr_t(1);
     return c.mHTMLCSSStyleSheet;
   }
 
@@ -162,33 +171,34 @@ public:
                   sizeof(RawServoDeclarationBlock*),
                   "RefPtr should just be a pointer");
     static_assert(sizeof(RefPtr<RawServoDeclarationBlock>) ==
                   sizeof(RawServoDeclarationBlockStrong),
                   "RawServoDeclarationBlockStrong should be the same as RefPtr");
     return reinterpret_cast<const RawServoDeclarationBlockStrong*>(&mRaw);
   }
 
-  void ToString(nsAString& aResult) const {
+  void ToString(nsAString& aResult) const
+  {
     Servo_DeclarationBlock_GetCssText(mRaw, &aResult);
   }
 
-  uint32_t Count() const {
+  uint32_t Count() const
+  {
     return Servo_DeclarationBlock_Count(mRaw);
   }
 
-  bool GetNthProperty(uint32_t aIndex, nsAString& aReturn) const {
+  bool GetNthProperty(uint32_t aIndex, nsAString& aReturn) const
+  {
     aReturn.Truncate();
     return Servo_DeclarationBlock_GetNthProperty(mRaw, aIndex, &aReturn);
   }
 
-  void GetPropertyValue(const nsAString& aProperty,
-                        nsAString& aValue) const;
-  void GetPropertyValueByID(nsCSSPropertyID aPropID,
-                            nsAString& aValue) const;
+  void GetPropertyValue(const nsAString& aProperty, nsAString& aValue) const;
+  void GetPropertyValueByID(nsCSSPropertyID aPropID, nsAString& aValue) const;
   bool GetPropertyIsImportant(const nsAString& aProperty) const;
   // Returns whether the property was removed.
   bool RemoveProperty(const nsAString& aProperty);
   // Returns whether the property was removed.
   bool RemovePropertyByID(nsCSSPropertyID aProperty);
 
 private:
   ~DeclarationBlock() = default;