Fix memory error during selector matching due to getMatchedCSSRules.

Source/core/css/ElementRuleCollector.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 2004-2005 Allan Sandfeld Jensen (kde@carewolf.com)
  * Copyright (C) 2006, 2007 Nicholas Shanks (webkit@nickshanks.com)
  * Copyright (C) 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013 Apple Inc. All rights reserved.
  * Copyright (C) 2007 Alexey Proskuryakov <ap@webkit.org>
  * Copyright (C) 2007, 2008 Eric Seidel <eric@webkit.org>
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
  * Copyright (C) Research In Motion Limited 2011. All rights reserved.
@@ skipping 13 matching lines @@
  * along with this library; see the file COPYING.LIB.  If not, write to
  * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
  * Boston, MA 02110-1301, USA.
  */
 
 #include "config.h"
 #include "core/css/ElementRuleCollector.h"
 
 #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"
 #include "core/css/resolver/StyleResolver.h"
 #include "core/dom/shadow/ShadowRoot.h"
 #include "core/rendering/RenderRegion.h"
 
 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)
     , m_regionForStyling(0)
     , m_pseudoStyleRequest(NOPSEUDO)
     , m_mode(SelectorChecker::ResolvingStyle)
     , m_canUseFastReject(m_selectorFilter.parentStackIsConsistent(context.parentNode()))
     , m_sameOriginOnly(false)
     , m_matchingUARules(false)
+    , m_includeStyleSheet(includeStyleSheet)
 { }
 
 ElementRuleCollector::~ElementRuleCollector()
 {
 }
 
 MatchResult& ElementRuleCollector::matchedResult()
 {
     return m_result;
 }
@@ skipping 107 matching lines @@
     for (unsigned i = 0; i < size; ++i) {
         const CSSSelector* regionSelector = matchRequest.ruleSet->m_regionSelectorsAndRuleSets.at(i).selector;
         if (checkRegionSelector(regionSelector, toElement(m_regionForStyling->node()))) {
             RuleSet* regionRules = matchRequest.ruleSet->m_regionSelectorsAndRuleSets.at(i).ruleSet.get();
             ASSERT(regionRules);
             collectMatchingRules(MatchRequest(regionRules, matchRequest.includeEmptyRules, matchRequest.scope), ruleRange, behaviorAtBoundary, cascadeScope, cascadeOrder);
         }
     }
 }
 
+
+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())
         return;
 
     sortMatchedRules();
 
     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;
     }
 
     // Now transfer the set of matched rules over to our list of declarations.
     for (unsigned i = 0; i < matchedRules.size(); i++) {
         // FIXME: Matching should not modify the style directly.
         const RuleData* ruleData = matchedRules[i].ruleData();
         if (m_style && ruleData->containsUncommonAttributeSelector())
             m_style->setUnique();
         m_result.addMatchedProperties(ruleData->rule()->properties(), ruleData->rule(), ruleData->linkMatchType(), ruleData->propertyWhitelistType(m_matchingUARules));
@@ skipping 119 matching lines @@
     // information about "scope".
     int firstRuleIndex = -1, lastRuleIndex = -1;
     RuleRange ruleRange(firstRuleIndex, lastRuleIndex);
     // FIXME: Verify whether it's ok to ignore CascadeScope here.
     collectMatchingRules(MatchRequest(ruleSet), ruleRange, SelectorChecker::StaysWithinTreeScope);
 
     return m_matchedRules && !m_matchedRules->isEmpty();
 }
 
 } // namespace WebCore