DevTools: do not use range indices outside of InspectorStyleSheet.

Source/core/inspector/InspectorStyleSheet.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 967 matching lines @@
 }
 
 String InspectorStyleSheet::finalURL() const
 {
     String url = styleSheetURL(m_pageStyleSheet.get());
     return url.isEmpty() ? m_documentURL : url;
 }
 
 bool InspectorStyleSheet::setText(const String& text, ExceptionState& exceptionState)
 {
-    updateText(text);
+    innerSetText(text);
     m_flatRules.clear();
 
     if (listener())
         listener()->willReparseStyleSheet();
 
     {
         // Have a separate scope for clearRules() (bug 95324).
         CSSStyleSheet::RuleMutationScope mutationScope(m_pageStyleSheet.get());
         m_pageStyleSheet->contents()->clearRules();
         m_pageStyleSheet->clearChildRuleCSSOMWrappers();
     }
     {
         CSSStyleSheet::RuleMutationScope mutationScope(m_pageStyleSheet.get());
         m_pageStyleSheet->contents()->parseString(text);
     }
 
     if (listener())
         listener()->didReparseStyleSheet();
     onStyleSheetTextChanged();
     m_pageStyleSheet->ownerDocument()->styleResolverChanged(FullStyleUpdate);
     return true;
 }
 
-String InspectorStyleSheet::ruleSelector(unsigned ruleIndex, ExceptionState& exceptionState)
+CSSStyleRule* InspectorStyleSheet::setRuleSelector(const SourceRange& range, const String& text, SourceRange* newRange, String* oldText, ExceptionState& exceptionState)
 {
-    CSSStyleRule* rule = ruleAt(ruleIndex);
-    if (!rule) {
-        exceptionState.throwDOMException(NotFoundError, "No rule was found for the given ID.");
-        return "";
+    if (!verifySelectorText(text)) {
+        exceptionState.throwDOMException(SyntaxError, "Selector or media text is not valid.");
+        return nullptr;
     }
-    return rule->selectorText();
+
+    CSSRule* rule = nullptr;
+    CSSRuleSourceData* sourceData = nullptr;
+    if (!findRuleByHeaderRange(range, &rule, &sourceData) || !sourceData->styleSourceData || rule->type() != CSSRule::STYLE_RULE) {
+        exceptionState.throwDOMException(NotFoundError, "Source range didn't match existing source range");
+        return nullptr;
+    }
+
+    CSSStyleRule* styleRule = InspectorCSSAgent::asCSSStyleRule(rule);
+    styleRule->setSelectorText(text);
+
+    replaceText(sourceData->ruleHeaderRange, text, newRange, oldText);
+    onStyleSheetTextChanged();
+
+    return styleRule;
 }
 
-bool InspectorStyleSheet::setRuleSelector(unsigned ruleIndex, const String& selector, ExceptionState& exceptionState)
+CSSMediaRule* InspectorStyleSheet::setMediaRuleText(const SourceRange& range, const String& text, SourceRange* newRange, String* oldText, ExceptionState& exceptionState)
 {
-    CSSStyleRule* rule = ruleAt(ruleIndex);
-    if (!rule) {
-        exceptionState.throwDOMException(NotFoundError, "No rule was found for the given ID.");
-        return false;
-    }
-    CSSStyleSheet* styleSheet = rule->parentStyleSheet();
-    if (!styleSheet || !ensureParsedDataReady()) {
-        exceptionState.throwDOMException(NotFoundError, "No stylesheet could be found in which to set the selector.");
-        return false;
+    if (!verifyMediaText(text)) {
+        exceptionState.throwDOMException(SyntaxError, "Selector or media text is not valid.");
+        return nullptr;
     }
 
-    if (!verifySelectorText(selector)) {
-        exceptionState.throwDOMException(SyntaxError, "Selector text is not valid.");
-        return false;
+    CSSRule* rule = nullptr;
+    CSSRuleSourceData* sourceData = nullptr;
+    if (!findRuleByHeaderRange(range, &rule, &sourceData) || !sourceData->mediaSourceData || rule->type() != CSSRule::MEDIA_RULE) {
+        exceptionState.throwDOMException(NotFoundError, "Source range didn't match existing source range");
+        return nullptr;
     }
 
-    rule->setSelectorText(selector);
-    RefPtrWillBeRawPtr<CSSRuleSourceData> sourceData = ruleSourceDataAt(ruleIndex);
-    ASSERT(sourceData);
+    CSSMediaRule* mediaRule = InspectorCSSAgent::asCSSMediaRule(rule);
+    mediaRule->media()->setMediaText(text);
 
-    String sheetText = m_parsedStyleSheet->text();
-    sheetText.replace(sourceData->ruleHeaderRange.start, sourceData->ruleHeaderRange.length(), selector);
-    updateText(sheetText);
+    replaceText(sourceData->ruleHeaderRange, text, newRange, oldText);
     onStyleSheetTextChanged();
-    return true;
-}
 
-String InspectorStyleSheet::mediaRuleText(unsigned ruleIndex, ExceptionState& exceptionState)
-{
-    CSSMediaRule* rule = mediaRuleAt(ruleIndex);
-    if (!rule) {
-        exceptionState.throwDOMException(NotFoundError, "No media rule was found for the given ID.");
-        return "";
-    }
-    return rule->media()->mediaText();
-}
-
-bool InspectorStyleSheet::setMediaRuleText(unsigned ruleIndex, const String& text, ExceptionState& exceptionState)
-{
-    CSSMediaRule* rule = mediaRuleAt(ruleIndex);
-    if (!rule) {
-        exceptionState.throwDOMException(NotFoundError, "No media rule was found for the given ID.");
-        return false;
-    }
-    CSSStyleSheet* styleSheet = rule->parentStyleSheet();
-    if (!styleSheet || !ensureParsedDataReady()) {
-        exceptionState.throwDOMException(NotFoundError, "No stylesheet could be found in which to set the media text.");
-        return false;
-    }
-    if (!verifyMediaText(text)) {
-        exceptionState.throwDOMException(SyntaxError, "Media text is not valid.");
-        return false;
-    }
-
-    rule->media()->setMediaText(text);
-    RefPtrWillBeRawPtr<CSSRuleSourceData> sourceData = ruleSourceDataAt(ruleIndex);
-    ASSERT(sourceData && sourceData->mediaSourceData);
-
-    String sheetText = m_parsedStyleSheet->text();
-    sheetText.replace(sourceData->ruleHeaderRange.start, sourceData->ruleHeaderRange.length(), text);
-    updateText(sheetText);
-    onStyleSheetTextChanged();
-    return true;
+    return mediaRule;
 }
 
 unsigned InspectorStyleSheet::ruleIndexBySourceRange(const CSSMediaRule* parentMediaRule, const SourceRange& sourceRange)
 {
     unsigned index = 0;
     for (size_t i = 0; i < m_flatRules.size(); ++i) {
         RefPtrWillBeRawPtr<CSSRule> rule = m_flatRules.at(i);
         if (rule->parentRule() != parentMediaRule)
             continue;
         RefPtrWillBeRawPtr<CSSRuleSourceData> ruleSourceData = m_parsedStyleSheet->ruleSourceDataAt(i);
@@ skipping 30 matching lines @@
     }
     return styleRule;
 }
 
 CSSStyleRule* InspectorStyleSheet::insertCSSOMRuleBySourceRange(const SourceRange& sourceRange, const String& ruleText, ExceptionState& exceptionState)
 {
     int containingRuleIndex = -1;
     unsigned containingRuleLength = 0;
     for (size_t i = 0; i < m_parsedStyleSheet->ruleCount(); ++i) {
         RefPtrWillBeRawPtr<CSSRuleSourceData> ruleSourceData = m_parsedStyleSheet->ruleSourceDataAt(i);
+

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

stray line

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

Done.

         if (ruleSourceData->ruleHeaderRange.start < sourceRange.start && sourceRange.start < ruleSourceData->ruleBodyRange.start) {
             exceptionState.throwDOMException(NotFoundError, "Cannot insert rule inside rule selector.");
             return nullptr;
         }
         if (sourceRange.start < ruleSourceData->ruleBodyRange.start || ruleSourceData->ruleBodyRange.end < sourceRange.start)
             continue;
         if (containingRuleIndex == -1 || containingRuleLength > ruleSourceData->ruleBodyRange.length()) {
             containingRuleIndex = i;
             containingRuleLength = ruleSourceData->ruleBodyRange.length();
         }
     }
     if (containingRuleIndex == -1)
         return insertCSSOMRuleInStyleSheet(sourceRange, ruleText, exceptionState);
+

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

stray line

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

Acknowledged.

     RefPtrWillBeRawPtr<CSSRule> rule = m_flatRules.at(containingRuleIndex);
     if (rule->type() != CSSRule::MEDIA_RULE) {
         exceptionState.throwDOMException(NotFoundError, "Cannot insert rule in non-media rule.");
         return nullptr;
     }
     return insertCSSOMRuleInMediaRule(toCSSMediaRule(rule.get()), sourceRange, ruleText, exceptionState);
 }
 
 bool InspectorStyleSheet::verifyRuleText(const String& ruleText)
 {
@@ skipping 77 matching lines @@
     if (propertyCount != 1)
         return false;
 
     // Check for the property name.
     if (propertyData.at(0).name != bogusPropertyName)
         return false;
 
     return true;
 }
 
-CSSStyleRule* InspectorStyleSheet::addRule(const String& ruleText, const SourceRange& location, ExceptionState& exceptionState)
+CSSStyleRule* InspectorStyleSheet::addRule(const String& ruleText, const SourceRange& location, SourceRange* addedRange, ExceptionState& exceptionState)
 {
-    if (!ensureParsedDataReady()) {
-        exceptionState.throwDOMException(NotFoundError, "Cannot parse style sheet.");
-        return nullptr;
-    }
-
     if (location.start != location.end) {
         exceptionState.throwDOMException(NotFoundError, "Source range must be collapsed.");
         return nullptr;
     }
 
     if (!verifyRuleText(ruleText)) {
         exceptionState.throwDOMException(SyntaxError, "Rule text is not valid.");
         return nullptr;
     }
 
-    String text;
-    bool success = getText(&text);
-    if (!success) {
-        exceptionState.throwDOMException(NotFoundError, "The rule '" + ruleText + "' could not be added.");
+    if (!ensureParsedDataReady()) {
+        exceptionState.throwDOMException(NotFoundError, "Cannot parse style sheet.");
         return nullptr;
     }
+    ensureFlatRules();
 
-    ensureFlatRules();
     CSSStyleRule* styleRule = insertCSSOMRuleBySourceRange(location, ruleText, exceptionState);
     if (exceptionState.hadException())
         return nullptr;
 
-    text.insert(ruleText, location.start);
-
-    updateText(text);
+    replaceText(location, ruleText, addedRange, nullptr);
     m_flatRules.clear();
 
     onStyleSheetTextChanged();
     return styleRule;
 }
 
-bool InspectorStyleSheet::deleteRule(unsigned ruleIndex, const String& oldText, ExceptionState& exceptionState)
+bool InspectorStyleSheet::deleteRule(const SourceRange& range, ExceptionState& exceptionState)
 {
-    RefPtrWillBeRawPtr<CSSStyleRule> rule = ruleAt(ruleIndex);
+    // Find index of CSSRule that entirely belongs to the range.
+    CSSRule* rule = nullptr;

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

RefPtr<CSSRule> rule;

why do you want this to be a raw pointer? 

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

Done.

+    unsigned containingRuleLength = 0;

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

.. = UCHAR_MAX;

to make it clear that you minimize over this parameter.

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

I was consistent with your code below.

+
+    String styleSheetText = m_parsedStyleSheet->text();

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

unused?

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

Done.

+
+    for (size_t i = 0; i < m_flatRules.size() && i < m_parsedStyleSheet->ruleCount(); ++i) {
+        RefPtrWillBeRawPtr<CSSRuleSourceData> ruleSourceData = m_parsedStyleSheet->ruleSourceDataAt(i);
+        unsigned ruleStart = ruleSourceData->ruleHeaderRange.start;
+        unsigned ruleEnd = ruleSourceData->ruleBodyRange.end + 1;
+        bool startBelongs = ruleStart >= range.start && ruleStart < range.end;
+        bool endBelongs = ruleEnd > range.start && ruleEnd <= range.end;
+        if (startBelongs != endBelongs)

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

The two if-statements could be replaced with


if (!startBelongs || !endBelongs)
    continue; 

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

Can't do.

+            break;
+        if (!startBelongs)
+            continue;
+        if (!rule || containingRuleLength > ruleSourceData->ruleBodyRange.length()) {
+            containingRuleLength = ruleSourceData->ruleBodyRange.length();
+            rule = m_flatRules.at(i).get();
+        }
+    }
     if (!rule) {
-        exceptionState.throwDOMException(NotFoundError, "No style rule could be found for the provided ID.");
+        exceptionState.throwDOMException(NotFoundError, "No style rule could be found in given range.");
         return false;
     }
     CSSStyleSheet* styleSheet = rule->parentStyleSheet();
     if (!styleSheet || !ensureParsedDataReady()) {
         exceptionState.throwDOMException(NotFoundError, "No parent stylesheet could be found.");
         return false;
     }
-
-    RefPtrWillBeRawPtr<CSSRuleSourceData> sourceData = ruleSourceDataAt(ruleIndex);
-    if (!sourceData) {
-        exceptionState.throwDOMException(NotFoundError, "No style rule could be found for the provided ID.");
-        return false;
-    }
-
     CSSRule* parentRule = rule->parentRule();
     if (parentRule) {
         if (parentRule->type() != CSSRule::MEDIA_RULE) {
             exceptionState.throwDOMException(NotFoundError, "Cannot remove rule from non-media rule.");
             return false;
         }
         CSSMediaRule* parentMediaRule = toCSSMediaRule(parentRule);
         size_t index = 0;
         while (index < parentMediaRule->length() && parentMediaRule->item(index) != rule)
             ++index;
         ASSERT(index < parentMediaRule->length());
         parentMediaRule->deleteRule(index, exceptionState);
     } else {
         size_t index = 0;
         while (index < styleSheet->length() && styleSheet->item(index) != rule)
             ++index;
         ASSERT(index < styleSheet->length());
         styleSheet->deleteRule(index, exceptionState);
     }
     // |rule| MAY NOT be addressed after this line!
 
     if (exceptionState.hadException())
         return false;
 
-    updateText(oldText);
+    replaceText(range, "", nullptr, nullptr);
     m_flatRules.clear();
     onStyleSheetTextChanged();
     return true;
 }
 
-void InspectorStyleSheet::updateText(const String& newText)
+void InspectorStyleSheet::replaceText(const SourceRange& range, const String& text, SourceRange* newRange, String* oldText)
+{
+    String sheetText = m_parsedStyleSheet->text();
+    if (oldText)
+        *oldText = sheetText.substring(range.start, range.length());
+    sheetText.replace(range.start, range.length(), text);
+    if (newRange)
+        *newRange = SourceRange(range.start, range.start + text.length());
+    innerSetText(sheetText);
+}
+
+void InspectorStyleSheet::innerSetText(const String& newText)
 {
     Element* element = ownerStyleElement();
     if (element)
         m_cssAgent->addEditedStyleElement(DOMNodeIds::idForNode(element), newText);
     else
         m_cssAgent->addEditedStyleSheet(finalURL(), newText);
     m_parsedStyleSheet->setText(newText);
 }
 
 CSSStyleRule* InspectorStyleSheet::ruleAt(unsigned ruleIndex) const
@@ skipping 261 matching lines @@
 {
     ensureFlatRules();
     for (unsigned i = 0, size = m_flatRules.size(); i < size; ++i) {
         CSSStyleRule* styleRule = InspectorCSSAgent::asCSSStyleRule(m_flatRules.at(i).get());
         if (styleRule && styleRule->style() == style)
             return i;
     }
     return UINT_MAX;
 }
 
-bool InspectorStyleSheet::findRuleBySelectorRange(const SourceRange& sourceRange, unsigned* ruleIndex)
+bool InspectorStyleSheet::findRuleByHeaderRange(const SourceRange& sourceRange, CSSRule** pRule, CSSRuleSourceData** pSourceData)
 {
     if (!ensureParsedDataReady())
         return false;
-    for (size_t i = 0; i < ruleCount(); ++i) {
+    ensureFlatRules();
+
+    for (size_t i = 0; i < ruleCount() && i < m_flatRules.size(); ++i) {
         RefPtrWillBeRawPtr<CSSRuleSourceData> ruleSourceData = ruleSourceDataAt(i);
-        if (!ruleSourceData->styleSourceData)
-            continue;
         if (ruleSourceData->ruleHeaderRange.start == sourceRange.start && ruleSourceData->ruleHeaderRange.end == sourceRange.end) {
-            *ruleIndex = i;
+            *pRule = m_flatRules.at(i).get();
+            if (!(*pRule)->parentStyleSheet())
+                return false;
+            *pSourceData = ruleSourceData.get();
             return true;
         }
     }
-    return false;
-}
-
-bool InspectorStyleSheet::findMediaRuleByRange(const SourceRange& sourceRange, unsigned* ruleIndex)
-{
-    if (!ensureParsedDataReady())
-        return false;
-    for (size_t i = 0; i < ruleCount(); ++i) {
-        RefPtrWillBeRawPtr<CSSRuleSourceData> ruleSourceData = ruleSourceDataAt(i);
-        if (!ruleSourceData->mediaSourceData)
-            continue;
-        if (ruleSourceData->ruleHeaderRange.start == sourceRange.start && ruleSourceData->ruleHeaderRange.end == sourceRange.end) {
-            *ruleIndex = i;
-            return true;
-        }
-    }
     return false;
 }
 
 const CSSRuleVector& InspectorStyleSheet::flatRules()
 {
     ensureFlatRules();
     return m_flatRules;
 }
 
 Document* InspectorStyleSheet::ownerDocument() const
@@ skipping 73 matching lines @@
         return false;
 
     String patchedStyleSheetText;
     bool success = styleSheetTextWithChangedStyle(style, text, &patchedStyleSheetText);
     if (!success)
         return false;
 
     TrackExceptionState exceptionState;
     style->setCSSText(text, exceptionState);
     if (!exceptionState.hadException()) {
-        updateText(patchedStyleSheetText);
+        innerSetText(patchedStyleSheetText);
         onStyleSheetTextChanged();
     }
 
     return !exceptionState.hadException();
 }
 
 bool InspectorStyleSheet::styleSheetTextWithChangedStyle(CSSStyleDeclaration* style, const String& newStyleText, String* result)
 {
     if (!style)
         return false;
@@ skipping 196 matching lines @@
 
 DEFINE_TRACE(InspectorStyleSheetForInlineStyle)
 {
     visitor->trace(m_element);
     visitor->trace(m_ruleSourceData);
     visitor->trace(m_inspectorStyle);
     InspectorStyleSheetBase::trace(visitor);
 }
 
 } // namespace blink