DevTools: do not use range indices outside of InspectorStyleSheet.

Source/core/inspector/InspectorCSSAgent.cpp

 /*
  * Copyright (C) 2010, Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
  *     notice, this list of conditions and the following disclaimer in the
@@ skipping 269 matching lines @@
 private:
     RefPtrWillBeMember<InspectorStyleSheetBase> m_styleSheet;
     unsigned m_ruleIndex;
     unsigned m_propertyIndex;
     String m_text;
     String m_oldStyleText;
     String m_newStyleText;
     bool m_overwrite;
 };
 
-class InspectorCSSAgent::SetRuleSelectorAction final : public InspectorCSSAgent::StyleSheetAction {
-    WTF_MAKE_NONCOPYABLE(SetRuleSelectorAction);
+class InspectorCSSAgent::SetRuleSelectorOrMediaAction final : public InspectorCSSAgent::StyleSheetAction {
+    WTF_MAKE_NONCOPYABLE(SetRuleSelectorOrMediaAction);
 public:
-    SetRuleSelectorAction(InspectorStyleSheet* styleSheet, unsigned ruleIndex, const String& selector)
-        : InspectorCSSAgent::StyleSheetAction("SetRuleSelector")
+    SetRuleSelectorOrMediaAction(bool isMedia, InspectorStyleSheet* styleSheet, const SourceRange& range, const String& text)
+        : InspectorCSSAgent::StyleSheetAction("SetRuleSelectorOrMediaAction")
         , m_styleSheet(styleSheet)
-        , m_ruleIndex(ruleIndex)
-        , m_selector(selector)
+        , m_isMedia(isMedia)
+        , m_range(range)
+        , m_text(text)
+        , m_cssRule(nullptr)
     {
     }
 
     virtual bool perform(ExceptionState& exceptionState) override
     {
-        m_oldSelector = m_styleSheet->ruleSelector(m_ruleIndex, exceptionState);
-        if (exceptionState.hadException())
-            return false;
         return redo(exceptionState);
     }
 
     virtual bool undo(ExceptionState& exceptionState) override
     {
-        return m_styleSheet->setRuleSelector(m_ruleIndex, m_oldSelector, exceptionState);
+        if (m_isMedia)
+            return m_styleSheet->setMediaRuleText(m_newRange, m_oldText, nullptr, nullptr, exceptionState);
+        return m_styleSheet->setRuleSelector(m_newRange, m_oldText, nullptr, nullptr, exceptionState);
     }
 
     virtual bool redo(ExceptionState& exceptionState) override
     {
-        return m_styleSheet->setRuleSelector(m_ruleIndex, m_selector, exceptionState);
+        if (m_isMedia)
+            m_cssRule = m_styleSheet->setMediaRuleText(m_range, m_text, &m_newRange, &m_oldText, exceptionState);
+        else
+            m_cssRule = m_styleSheet->setRuleSelector(m_range, m_text, &m_newRange, &m_oldText, exceptionState);
+        return m_cssRule;
+    }
+
+    CSSRule* takeRule()
+    {
+        CSSRule* result = m_cssRule.get();
+        m_cssRule = nullptr;
+        return result;
     }
 
     DEFINE_INLINE_VIRTUAL_TRACE()
     {
         visitor->trace(m_styleSheet);
+        visitor->trace(m_cssRule);
         InspectorCSSAgent::StyleSheetAction::trace(visitor);
     }
 
 private:
     RefPtrWillBeMember<InspectorStyleSheet> m_styleSheet;
-    unsigned m_ruleIndex;
-    String m_selector;
-    String m_oldSelector;
-};
-
-class InspectorCSSAgent::SetMediaTextAction final : public InspectorCSSAgent::StyleSheetAction {
-    WTF_MAKE_NONCOPYABLE(SetMediaTextAction);
-public:
-    SetMediaTextAction(InspectorStyleSheet* styleSheet, unsigned ruleIndex, const String& text)
-        : InspectorCSSAgent::StyleSheetAction("SetMediaText")
-        , m_styleSheet(styleSheet)
-        , m_ruleIndex(ruleIndex)
-        , m_text(text)
-    {
-    }
-
-    virtual bool perform(ExceptionState& exceptionState) override
-    {
-        m_oldText = m_styleSheet->mediaRuleText(m_ruleIndex, exceptionState);
-        if (exceptionState.hadException())
-            return false;
-        return redo(exceptionState);
-    }
-
-    virtual bool undo(ExceptionState& exceptionState) override
-    {
-        return m_styleSheet->setMediaRuleText(m_ruleIndex, m_oldText, exceptionState);
-    }
-
-    virtual bool redo(ExceptionState& exceptionState) override
-    {
-        return m_styleSheet->setMediaRuleText(m_ruleIndex, m_text, exceptionState);
-    }
-
-    DEFINE_INLINE_VIRTUAL_TRACE()
-    {
-        visitor->trace(m_styleSheet);
-        InspectorCSSAgent::StyleSheetAction::trace(visitor);
-    }
-
-private:
-    RefPtrWillBeMember<InspectorStyleSheet> m_styleSheet;
-    unsigned m_ruleIndex;
+    bool m_isMedia;
+    SourceRange m_range;
     String m_text;
     String m_oldText;
+    SourceRange m_newRange;
+    RefPtrWillBeMember<CSSRule> m_cssRule;
 };
 
 class InspectorCSSAgent::AddRuleAction final : public InspectorCSSAgent::StyleSheetAction {
     WTF_MAKE_NONCOPYABLE(AddRuleAction);
 public:
     AddRuleAction(InspectorStyleSheet* styleSheet, const String& ruleText, const SourceRange& location)
         : InspectorCSSAgent::StyleSheetAction("AddRule")
         , m_styleSheet(styleSheet)
         , m_ruleText(ruleText)
         , m_location(location)
     {
     }
 
     virtual bool perform(ExceptionState& exceptionState) override
     {
         return redo(exceptionState);
     }
 
     virtual bool undo(ExceptionState& exceptionState) override
     {
-        return m_styleSheet->deleteRule(m_newOrdinal, m_oldText, exceptionState);
+        return m_styleSheet->deleteRule(m_addedRange, exceptionState);
     }
 
     virtual bool redo(ExceptionState& exceptionState) override
     {
-        if (!m_styleSheet->getText(&m_oldText))
-            return false;
-        CSSStyleRule* cssStyleRule = m_styleSheet->addRule(m_ruleText, m_location, exceptionState);
+        m_cssRule = m_styleSheet->addRule(m_ruleText, m_location, &m_addedRange, exceptionState);
         if (exceptionState.hadException())
             return false;
-        m_newOrdinal = m_styleSheet->indexOf(cssStyleRule);
         return true;
     }
 
-    unsigned newRuleOrdinal() { return m_newOrdinal; }
+    CSSStyleRule* takeRule()
+    {
+        CSSStyleRule* result = m_cssRule.get();
+        m_cssRule = nullptr;

[lushnikov, 2015/06/17 09:56:22]

FYI: this looks shaggy as this might kill the "result" CSSStyleRule; though I
understand that
this doesn't happen due to a particular way you call this method from the
outside.

[pfeldman, 2015/06/17 11:05:08]

Done.

+        return result;
+    }
 
     DEFINE_INLINE_VIRTUAL_TRACE()
     {
         visitor->trace(m_styleSheet);
+        visitor->trace(m_cssRule);
         InspectorCSSAgent::StyleSheetAction::trace(visitor);
     }
 
 private:
     RefPtrWillBeMember<InspectorStyleSheet> m_styleSheet;
-    unsigned m_newOrdinal;
+    RefPtrWillBeMember<CSSStyleRule> m_cssRule;
     String m_ruleText;
     String m_oldText;
     SourceRange m_location;
+    SourceRange m_addedRange;
 };
 
 // static
 CSSStyleRule* InspectorCSSAgent::asCSSStyleRule(CSSRule* rule)
 {
     if (!rule || rule->type() != CSSRule::STYLE_RULE)
         return nullptr;
     return toCSSStyleRule(rule);
 }
 
@@ skipping 549 matching lines @@
 void InspectorCSSAgent::setRuleSelector(ErrorString* errorString, const String& styleSheetId, const RefPtr<JSONObject>& range, const String& selector, RefPtr<TypeBuilder::CSS::CSSRule>& result)
 {
     InspectorStyleSheet* inspectorStyleSheet = assertInspectorStyleSheetForId(errorString, styleSheetId);
     if (!inspectorStyleSheet) {
         *errorString = "Stylesheet not found";
         return;
     }
     SourceRange selectorRange;
     if (!jsonRangeToSourceRange(errorString, inspectorStyleSheet, range, &selectorRange))
         return;
-    unsigned ruleIndex = 0;
-    if (!inspectorStyleSheet->findRuleBySelectorRange(selectorRange, &ruleIndex)) {
-        *errorString = "Source range didn't match any rule selector source range";
-        return;
-    }
 
     TrackExceptionState exceptionState;
-    bool success = m_domAgent->history()->perform(adoptRefWillBeNoop(new SetRuleSelectorAction(inspectorStyleSheet, ruleIndex, selector)), exceptionState);
+    RefPtrWillBeRawPtr<SetRuleSelectorOrMediaAction> action = adoptRefWillBeNoop(new SetRuleSelectorOrMediaAction(false, inspectorStyleSheet, selectorRange, selector));
+    bool success = m_domAgent->history()->perform(action, exceptionState);
     if (success) {
-        CSSStyleRule* rule = inspectorStyleSheet->ruleAt(ruleIndex);
+        CSSStyleRule* rule = InspectorCSSAgent::asCSSStyleRule(action->takeRule());
         result = inspectorStyleSheet->buildObjectForRule(rule, buildMediaListChain(rule));
     }
     *errorString = InspectorDOMAgent::toErrorString(exceptionState);
 }
 
 void InspectorCSSAgent::setMediaText(ErrorString* errorString, const String& styleSheetId, const RefPtr<JSONObject>& range, const String& text, RefPtr<TypeBuilder::CSS::CSSMedia>& result)
 {
     InspectorStyleSheet* inspectorStyleSheet = assertInspectorStyleSheetForId(errorString, styleSheetId);
     if (!inspectorStyleSheet) {
         *errorString = "Stylesheet not found";
         return;
     }
     SourceRange textRange;
     if (!jsonRangeToSourceRange(errorString, inspectorStyleSheet, range, &textRange))
         return;
-    unsigned ruleIndex = 0;
-    if (!inspectorStyleSheet->findMediaRuleByRange(textRange, &ruleIndex)) {
-        *errorString = "Source range didn't match any media rule source range";
-        return;
-    }
 
     TrackExceptionState exceptionState;
-    bool success = m_domAgent->history()->perform(adoptRefWillBeNoop(new SetMediaTextAction(inspectorStyleSheet, ruleIndex, text)), exceptionState);
+    RefPtrWillBeRawPtr<SetRuleSelectorOrMediaAction> action = adoptRefWillBeNoop(new SetRuleSelectorOrMediaAction(true, inspectorStyleSheet, textRange, text));
+    bool success = m_domAgent->history()->perform(action, exceptionState);
     if (success) {
-        CSSMediaRule* rule = inspectorStyleSheet->mediaRuleAt(ruleIndex);
+        CSSMediaRule* rule = InspectorCSSAgent::asCSSMediaRule(action->takeRule());
         String sourceURL = rule->parentStyleSheet()->contents()->baseURL();
         if (sourceURL.isEmpty())
             sourceURL = InspectorDOMAgent::documentURLString(rule->parentStyleSheet()->ownerDocument());
         result = buildMediaObject(rule->media(), MediaListSourceMediaRule, sourceURL, rule->parentStyleSheet());
     }
     *errorString = InspectorDOMAgent::toErrorString(exceptionState);
 }
 
 void InspectorCSSAgent::createStyleSheet(ErrorString* errorString, const String& frameId, TypeBuilder::CSS::StyleSheetId* outStyleSheetId)
 {
@@ skipping 30 matching lines @@
         return;
 
     TrackExceptionState exceptionState;
     RefPtrWillBeRawPtr<AddRuleAction> action = adoptRefWillBeNoop(new AddRuleAction(inspectorStyleSheet, ruleText, ruleLocation));
     bool success = m_domAgent->history()->perform(action, exceptionState);
     if (!success) {
         *errorString = InspectorDOMAgent::toErrorString(exceptionState);
         return;
     }
 
-    unsigned ruleOrdinal = action->newRuleOrdinal();
-    CSSStyleRule* rule = inspectorStyleSheet->ruleAt(ruleOrdinal);
+    CSSStyleRule* rule = action->takeRule();
     result = inspectorStyleSheet->buildObjectForRule(rule, buildMediaListChain(rule));
 }
 
 void InspectorCSSAgent::forcePseudoState(ErrorString* errorString, int nodeId, const RefPtr<JSONArray>& forcedPseudoClasses)
 {
     Element* element = m_domAgent->assertElement(errorString, nodeId);
     if (!element)
         return;
 
     unsigned forcedPseudoState = computePseudoClassMask(forcedPseudoClasses.get());
@@ skipping 503 matching lines @@
     visitor->trace(m_invalidatedDocuments);
     visitor->trace(m_nodeToInspectorStyleSheet);
     visitor->trace(m_documentToViaInspectorStyleSheet);
 #endif
     visitor->trace(m_inspectorUserAgentStyleSheet);
     InspectorBaseAgent::trace(visitor);
 }
 
 } // namespace blink