heap: Replace raw pointers in CSSRule with a Member

This changes simplifies the tracing logic and avoid needing to manually
call the write barrier on assignments.

Bug: 986235
Change-Id: I2ae7b43241e1777dce4efcba5586ef2e1b0b1143
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2019282
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735187}

third_party/blink/renderer/core/css/css_rule.cc

@@ -31,12 +31,21 @@ struct SameSizeAsCSSRule : public GarbageCollected<SameSizeAsCSSRule>,
                            public ScriptWrappable {
   ~SameSizeAsCSSRule() override;
   unsigned char bitfields;
-  void* pointer_union;
+  Member<ScriptWrappable> member;
+#if !DCHECK_IS_ON()
+  static_assert(sizeof(Member<ScriptWrappable>) == sizeof(void*),
+                "Increasing size of Member increases size of CSSRule");
+#endif  // DCHECK_IS_ON()
 };
 
 static_assert(sizeof(CSSRule) == sizeof(SameSizeAsCSSRule),
               "CSSRule should stay small");
 
+CSSRule::CSSRule(CSSStyleSheet* parent)
+    : has_cached_selector_text_(false),
+      parent_is_rule_(false),
+      parent_(parent) {}
+
 const CSSParserContext* CSSRule::ParserContext(
     SecureContextMode secure_context_mode) const {
   CSSStyleSheet* style_sheet = parentStyleSheet();
@@ -46,25 +55,26 @@ const CSSParserContext* CSSRule::ParserContext(
 
 void CSSRule::SetParentStyleSheet(CSSStyleSheet* style_sheet) {
   parent_is_rule_ = false;
-  parent_style_sheet_ = style_sheet;
-  MarkingVisitor::WriteBarrier(&parent_style_sheet_);
+  parent_ = style_sheet;
 }
 
 void CSSRule::SetParentRule(CSSRule* rule) {
   parent_is_rule_ = true;
-  parent_rule_ = rule;
-  MarkingVisitor::WriteBarrier(&parent_rule_);
+  parent_ = rule;
 }
 
 void CSSRule::Trace(blink::Visitor* visitor) {
-  // This makes the parent link strong, which is different from the
-  // pre-oilpan world, where the parent link is mysteriously zeroed under
-  // some circumstances.
-  if (parent_is_rule_)
-    visitor->Trace(parent_rule_);
-  else
-    visitor->Trace(parent_style_sheet_);
+  visitor->Trace(parent_);
   ScriptWrappable::Trace(visitor);
 }
 
+bool CSSRule::VerifyParentIsCSSRule() const {
+  return !parent_ || parent_->GetWrapperTypeInfo()->IsSubclass(
+                         CSSRule::GetStaticWrapperTypeInfo());
+}
+bool CSSRule::VerifyParentIsCSSStyleSheet() const {
+  return !parent_ || parent_->GetWrapperTypeInfo()->IsSubclass(
+                         CSSStyleSheet::GetStaticWrapperTypeInfo());
+}
+
 }  // namespace blink

third_party/blink/renderer/core/css/css_rule.h

@@ -75,22 +75,19 @@ class CORE_EXPORT CSSRule : public ScriptWrappable {
 
   CSSStyleSheet* parentStyleSheet() const {
     if (parent_is_rule_)
-      return parent_rule_ ? parent_rule_->parentStyleSheet() : nullptr;
-    return parent_style_sheet_;
+      return parent_ ? ParentAsCSSRule()->parentStyleSheet() : nullptr;
+    return ParentAsCSSStyleSheet();
   }
 
   CSSRule* parentRule() const {
-    return parent_is_rule_ ? parent_rule_ : nullptr;
+    return parent_is_rule_ ? ParentAsCSSRule() : nullptr;
   }
 
   // The CSSOM spec states that "setting the cssText attribute must do nothing."
   void setCSSText(const String&) {}
 
  protected:
-  CSSRule(CSSStyleSheet* parent)
-      : has_cached_selector_text_(false),
-        parent_is_rule_(false),
-        parent_style_sheet_(parent) {}
+  CSSRule(CSSStyleSheet* parent);
 
   bool HasCachedSelectorText() const { return has_cached_selector_text_; }
   void SetHasCachedSelectorText(bool has_cached_selector_text) const {
@@ -100,14 +97,27 @@ class CORE_EXPORT CSSRule : public ScriptWrappable {
   const CSSParserContext* ParserContext(SecureContextMode) const;
 
  private:
+  bool VerifyParentIsCSSRule() const;
+  bool VerifyParentIsCSSStyleSheet() const;
+
+  CSSRule* ParentAsCSSRule() const {
+    DCHECK(parent_is_rule_);
+    DCHECK(VerifyParentIsCSSRule());
+    return reinterpret_cast<CSSRule*>(parent_.Get());
+  }
+  CSSStyleSheet* ParentAsCSSStyleSheet() const {
+    DCHECK(!parent_is_rule_);
+    DCHECK(VerifyParentIsCSSStyleSheet());
+    return reinterpret_cast<CSSStyleSheet*>(parent_.Get());
+  }
+
   mutable unsigned char has_cached_selector_text_ : 1;
   unsigned char parent_is_rule_ : 1;
 
-  // These should be Members, but no Members in unions.
-  union {
-    CSSRule* parent_rule_;
-    CSSStyleSheet* parent_style_sheet_;
-  };
+  // parent_ should reference either CSSRule or CSSStyleSheet (both are
+  // descendants of ScriptWrappable). This field should only be accessed
+  // via the getters above (ParentAsCSSRule and ParentAsCSSStyleSheet).
+  Member<ScriptWrappable> parent_;
 };
 
 }  // namespace blink