Bug 1345696 part 6 - Fix cycle collection for font-face rule. r=mccr8,heycam draft
authorXidorn Quan <me@upsuper.org>
Tue, 28 Mar 2017 16:11:44 +1100
changeset 553423 f588ab575681db370b2fcb09d865ff17e87d752a
parent 553419 fa5d3343dfa81593b4d26691a9593435cdcc69cc
child 553424 54baffc4da262dabfc2ea9ac79a020098410b0e4
push id51630
push userxquan@mozilla.com
push dateThu, 30 Mar 2017 01:15:08 +0000
reviewersmccr8, heycam
bugs1345696
milestone55.0a1
Bug 1345696 part 6 - Fix cycle collection for font-face rule. r=mccr8,heycam Having Servo's FontFaceRule owning Gecko's nsCSSFontFaceRule object opens an untracked edge, because we generally don't track the reference through Servo objects, so there is a chance that nsCSSFontFaceRule can form a undroppable reference cycle. This patch adds a workaround that we track @font-face rule twice in CSS rule list. This is not perfect, but the idea is that, if someone wants to put some reference on the @font-face rule, the rule itself and its parent list should have been constructed in the CSSOM tree. If they are not there, we are probably safe from cycle reference. For @font-face rule, that assumption isn't strictly true. Script can still get a font-face rule via layout inspector's nsIDOMFontFace::rule. If some script puts an object which references the stylesheet of the rule or any of its parent rule / rule list into font-face rule's expando, we would leak again... But as far as that is an internal interface, we are probably safe? MozReview-Commit-ID: DDMJh3mxDCH
layout/style/ServoCSSRuleList.cpp
--- a/layout/style/ServoCSSRuleList.cpp
+++ b/layout/style/ServoCSSRuleList.cpp
@@ -37,16 +37,23 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(Se
   tmp->DropAllRules();
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END_INHERITED(dom::CSSRuleList)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(ServoCSSRuleList,
                                                   dom::CSSRuleList)
   tmp->EnumerateInstantiatedRules([&](css::Rule* aRule) {
     if (!aRule->IsCCLeaf()) {
       NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mRules[i]");
       cb.NoteXPCOMChild(aRule);
+      // Note about @font-face rule again, since there is an indirect
+      // owning edge through Servo's struct that FontFaceRule in Servo
+      // owns a Gecko nsCSSFontFaceRule object.
+      if (aRule->Type() == nsIDOMCSSRule::FONT_FACE_RULE) {
+        NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mRawRules[i]");
+        cb.NoteXPCOMChild(aRule);
+      }
     }
   });
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 void
 ServoCSSRuleList::SetParentRule(css::GroupRule* aParentRule)
 {
   mParentRule = aParentRule;
@@ -134,16 +141,17 @@ DropRule(already_AddRefed<css::Rule> aRu
 
 void
 ServoCSSRuleList::DropAllRules()
 {
   EnumerateInstantiatedRules([](css::Rule* rule) {
     DropRule(already_AddRefed<css::Rule>(rule));
   });
   mRules.Clear();
+  mRawRules = nullptr;
 }
 
 void
 ServoCSSRuleList::DropReference()
 {
   mStyleSheet = nullptr;
   mParentRule = nullptr;
   DropAllRules();