Fix memory error during selector matching due to getMatchedCSSRules.

Source/core/css/ElementRuleCollector.cpp

Index: Source/core/css/ElementRuleCollector.cpp
diff --git a/Source/core/css/ElementRuleCollector.cpp b/Source/core/css/ElementRuleCollector.cpp
index deff32a41630b5914d77b31d4d7a15aa6c1b6c7d..860f3675153610962392e527b3fc8b6f165bb39f 100644
--- a/Source/core/css/ElementRuleCollector.cpp
+++ b/Source/core/css/ElementRuleCollector.cpp
@@ -31,6 +31,7 @@
 
 #include "core/css/CSSRuleList.h"
 #include "core/css/CSSSelector.h"
+#include "core/css/CSSStyleRule.h"
 #include "core/css/SelectorCheckerFastPath.h"
 #include "core/css/SiblingTraversalStrategies.h"
 #include "core/css/StylePropertySet.h"
@@ -41,7 +42,7 @@
 namespace WebCore {
 
 ElementRuleCollector::ElementRuleCollector(const ElementResolveContext& context,
-    const SelectorFilter& filter, RenderStyle* style)
+    const SelectorFilter& filter, RenderStyle* style, ShouldIncludeStyleSheetInCSSOMWrapper includeStyleSheet)
     : m_context(context)
     , m_selectorFilter(filter)
     , m_style(style)
@@ -51,6 +52,7 @@ ElementRuleCollector::ElementRuleCollector(const ElementResolveContext& context,
     , m_canUseFastReject(m_selectorFilter.parentStackIsConsistent(context.parentNode()))
     , m_sameOriginOnly(false)
     , m_matchingUARules(false)
+    , m_includeStyleSheet(includeStyleSheet)
 { }
 
 ElementRuleCollector::~ElementRuleCollector()
@@ -178,6 +180,46 @@ void ElementRuleCollector::collectMatchingRulesForRegion(const MatchRequest& mat
     }
 }
 
+
+static CSSStyleSheet* findStyleSheet(StyleEngine* styleEngine, StyleRule* rule)
+{
+    // FIXME: StyleEngine has a bunch of different accessors for StyleSheet lists, is this the only one we need to care about?
+    const Vector<RefPtr<CSSStyleSheet> >& stylesheets = styleEngine->activeAuthorStyleSheets();
+    for (size_t i = 0; i < stylesheets.size(); ++i) {
+        CSSStyleSheet* sheet = stylesheets[i].get();
+        for (unsigned j = 0; j < sheet->length(); ++j) {
+            CSSRule* cssRule = sheet->item(j);
+            if (cssRule->type() != CSSRule::STYLE_RULE)
+                continue;
+            CSSStyleRule* cssStyleRule = toCSSStyleRule(cssRule);
+            if (cssStyleRule->styleRule() == rule)
+                return sheet;
+        }
+    }
+    return 0;
+}
+
+void ElementRuleCollector::appendCSSOMWrapperForRule(StyleRule* rule)
+{
+    // FIXME: There should be no codepath that creates a CSSOMWrapper without a parent stylesheet or rule because
+    // then that codepath can lead to the CSSStyleSheet contents not getting correctly copied when the rule is modified
+    // through the wrapper (e.g. rule.selectorText="div"). Right now, the inspector uses the pointers for identity though,
+    // so calling CSSStyleSheet->willMutateRules breaks the inspector.
+    CSSStyleSheet* sheet = m_includeStyleSheet == IncludeStyleSheetInCSSOMWrapper ? findStyleSheet(m_context.element()->document().styleEngine(), rule) : 0;
+    // We need to call willMutateRules so that the StyleSheetContents are copy-on-writed before we create the CSSOMWrappers.
+    // Otherwise, if the sheet is modified through the regular CSSOM, the CSSOMWrappers created here won't be updated
+    // to point to the new copied StyleRules. An alternate solution would be to update all the CSSOMWrappers when
+    // the stylesheet is modified.
+    if (sheet) {
+        sheet->willMutateRules();
+        sheet->didMutateRules();

[esprehn, 2013/10/28 23:54:08]

I think you're supposed to call didMutateRules at the end of this function call
not at the start like this. You do will*() and then did*() but no mutation
happens in between. That'll make maintaining this harder later.

[ojan, 2013/10/28 23:59:58]

This function doesn't actually mutate the rules though. That's the sucky thing
about this bit of code. Really this is just getting rid of the stylesheet
sharing.

+    }
+    RefPtr<CSSRule> cssRule = rule->createCSSOMWrapper(sheet);
+    if (sheet)
+        sheet->registerExtraChildRuleCSSOMWrapper(cssRule);
+    ensureRuleList()->rules().append(cssRule);
+}
+
 void ElementRuleCollector::sortAndTransferMatchedRules()
 {
     if (!m_matchedRules || m_matchedRules->isEmpty())
@@ -187,8 +229,9 @@ void ElementRuleCollector::sortAndTransferMatchedRules()
 
     Vector<MatchedRule, 32>& matchedRules = *m_matchedRules;
     if (m_mode == SelectorChecker::CollectingRules) {
-        for (unsigned i = 0; i < matchedRules.size(); ++i)
-            ensureRuleList()->rules().append(matchedRules[i].ruleData()->rule()->createCSSOMWrapper());
+        for (unsigned i = 0; i < matchedRules.size(); ++i) {
+            appendCSSOMWrapperForRule(matchedRules[i].ruleData()->rule());
+        }
         return;
     }