DevTools: refactor InspectorStyleSheet

DevTools: refactor InspectorStyleSheet

Introduce InspectorStyleSheetBase, inherit InspectorStyleSheet and InspectorStyleSheetForInlineStyle from it.

BUG=341506
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169550

[lushnikov, 2014-03-17 16:48:08]

please take a look

[lushnikov, 2014-03-17 16:48:08]

please take a look

[lushnikov, 2014-03-18 15:49:48]

Please, take a look

[vsevik, 2014-03-18 16:25:00]

lgtm

https://chromiumcodereview.appspot.com/202103002/diff/20001/Source/core/inspe...
File Source/core/inspector/InspectorStyleSheet.h (right):

https://chromiumcodereview.appspot.com/202103002/diff/20001/Source/core/inspe...
Source/core/inspector/InspectorStyleSheet.h:182: virtual CSSStyleRule*
ruleForId(const InspectorCSSId&) const = 0;
This should be on one subclass only.

https://chromiumcodereview.appspot.com/202103002/diff/20001/Source/core/inspe...
Source/core/inspector/InspectorStyleSheet.h:184: virtual InspectorCSSId
ruleId(CSSStyleRule*) const = 0;
ditto

https://chromiumcodereview.appspot.com/202103002/diff/20001/Source/core/inspe...
Source/core/inspector/InspectorStyleSheet.h:200: virtual
PassOwnPtr<Vector<unsigned> > lineEndings() const = 0;
Can we have only one implementation?

[lushnikov, 2014-03-18 16:32:15]

https://chromiumcodereview.appspot.com/202103002/diff/20001/Source/core/inspe...
File Source/core/inspector/InspectorStyleSheet.h (right):

https://chromiumcodereview.appspot.com/202103002/diff/20001/Source/core/inspe...
Source/core/inspector/InspectorStyleSheet.h:182: virtual CSSStyleRule*
ruleForId(const InspectorCSSId&) const = 0;
On 2014/03/18 16:25:01, vsevik wrote:
> This should be on one subclass only.

Done.

https://chromiumcodereview.appspot.com/202103002/diff/20001/Source/core/inspe...
Source/core/inspector/InspectorStyleSheet.h:184: virtual InspectorCSSId
ruleId(CSSStyleRule*) const = 0;
On 2014/03/18 16:25:01, vsevik wrote:
> ditto

Done.

https://chromiumcodereview.appspot.com/202103002/diff/20001/Source/core/inspe...
Source/core/inspector/InspectorStyleSheet.h:200: virtual
PassOwnPtr<Vector<unsigned> > lineEndings() const = 0;
On 2014/03/18 16:25:01, vsevik wrote:
> Can we have only one implementation?

Sure, I'll do it in a follow-up patch if you don't mind.

[lushnikov, 2014-03-18 16:34:06]

The CQ bit was checked by lushnikov@chromium.org

[commit-bot: I haz the power, 2014-03-18 16:34:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/lushnikov@chromium.org/202103002/40001

[lushnikov, 2014-03-18 16:45:48]

The CQ bit was unchecked by lushnikov@chromium.org

[lushnikov, 2014-03-18 16:46:11]

The CQ bit was checked by lushnikov@chromium.org

[commit-bot: I haz the power, 2014-03-18 16:46:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/lushnikov@chromium.org/202103002/40001

[commit-bot: I haz the power, 2014-03-18 19:10:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-18 19:10:24]

Failed to apply patch for Source/core/inspector/InspectorStyleSheet.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/core/inspector/InspectorStyleSheet.cpp
  Hunk #2 succeeded at 830 with fuzz 1.
  Hunk #3 FAILED at 894.
  1 out of 15 hunks FAILED -- saving rejects to file
Source/core/inspector/InspectorStyleSheet.cpp.rej

Patch:       Source/core/inspector/InspectorStyleSheet.cpp
Index: Source/core/inspector/InspectorStyleSheet.cpp
diff --git a/Source/core/inspector/InspectorStyleSheet.cpp
b/Source/core/inspector/InspectorStyleSheet.cpp
index
8e9081b638948a46c87cc3d49d0fc1684e47dd91..56dee2c07e1a913507f9cd3f2d375a85f23176fe
100644
--- a/Source/core/inspector/InspectorStyleSheet.cpp
+++ b/Source/core/inspector/InspectorStyleSheet.cpp
@@ -504,12 +504,12 @@ static PassRefPtrWillBeRawPtr<CSSRuleList>
asCSSRuleList(CSSRule* rule)
     return nullptr;
 }
 
-PassRefPtr<InspectorStyle> InspectorStyle::create(const InspectorCSSId&
styleId, PassRefPtr<CSSStyleDeclaration> style, InspectorStyleSheet*
parentStyleSheet)
+PassRefPtr<InspectorStyle> InspectorStyle::create(const InspectorCSSId&
styleId, PassRefPtr<CSSStyleDeclaration> style, InspectorStyleSheetBase*
parentStyleSheet)
 {
     return adoptRef(new InspectorStyle(styleId, style, parentStyleSheet));
 }
 
-InspectorStyle::InspectorStyle(const InspectorCSSId& styleId,
PassRefPtr<CSSStyleDeclaration> style, InspectorStyleSheet* parentStyleSheet)
+InspectorStyle::InspectorStyle(const InspectorCSSId& styleId,
PassRefPtr<CSSStyleDeclaration> style, InspectorStyleSheetBase*
parentStyleSheet)
     : m_styleId(styleId)
     , m_style(style)
     , m_parentStyleSheet(parentStyleSheet)
@@ -830,7 +830,62 @@ NewLineAndWhitespace&
InspectorStyle::newLineAndWhitespaceDelimiters() const
 
 Document* InspectorStyle::ownerDocument() const
 {
-    return m_parentStyleSheet->pageStyleSheet() ?
m_parentStyleSheet->pageStyleSheet()->ownerDocument() : 0;
+    return m_parentStyleSheet->ownerDocument();
+}
+
+InspectorStyleSheetBase::InspectorStyleSheetBase(const String& id, Listener*
listener)
+    : m_id(id)
+    , m_listener(listener)
+{
+}
+
+bool InspectorStyleSheetBase::setPropertyText(const InspectorCSSId& id,
unsigned propertyIndex, const String& text, bool overwrite, String* oldText,
ExceptionState& exceptionState)
+{
+    RefPtr<InspectorStyle> inspectorStyle = inspectorStyleForId(id);
+    if (!inspectorStyle) {
+        exceptionState.throwDOMException(NotFoundError, "No property could be
found for the given ID.");
+        return false;
+    }
+
+    bool success = inspectorStyle->setPropertyText(propertyIndex, text,
overwrite, oldText, exceptionState);
+    if (success)
+        fireStyleSheetChanged();
+    return success;
+}
+
+void InspectorStyleSheetBase::fireStyleSheetChanged()
+{
+    if (listener())
+        listener()->styleSheetChanged(this);
+}
+
+PassRefPtr<TypeBuilder::CSS::CSSStyle>
InspectorStyleSheetBase::buildObjectForStyle(CSSStyleDeclaration* style)
+{
+    RefPtr<CSSRuleSourceData> sourceData;
+    if (ensureParsedDataReady())
+        sourceData = ruleSourceDataFor(style);
+
+    InspectorCSSId id = styleId(style);
+    if (id.isEmpty()) {
+        // Any rule coming from User Agent and not from DefaultStyleSheet will
not have id.
+        // See InspectorCSSAgent::buildObjectForRule for details.
+        RefPtr<InspectorStyle> inspectorStyle = InspectorStyle::create(id,
style, this);
+        return inspectorStyle->buildObjectForStyle();
+    }
+    RefPtr<InspectorStyle> inspectorStyle = inspectorStyleForId(id);
+    RefPtr<TypeBuilder::CSS::CSSStyle> result =
inspectorStyle->buildObjectForStyle();
+
+    // Style text cannot be retrieved without stylesheet, so set cssText here.
+    if (sourceData) {
+        String sheetText;
+        bool success = getText(&sheetText);
+        if (success) {
+            const SourceRange& bodyRange = sourceData->ruleBodyRange;
+            result->setCssText(sheetText.substring(bodyRange.start,
bodyRange.end - bodyRange.start));
+        }
+    }
+
+    return result.release();
 }
 
 PassRefPtr<InspectorStyleSheet> InspectorStyleSheet::create(InspectorPageAgent*
pageAgent, InspectorResourceAgent* resourceAgent, const String& id,
PassRefPtr<CSSStyleSheet> pageStyleSheet,
TypeBuilder::CSS::StyleSheetOrigin::Enum origin, const String& documentURL,
Listener* listener)
@@ -839,13 +894,12 @@ PassRefPtr<InspectorStyleSheet>
InspectorStyleSheet::create(InspectorPageAgent*
 }
 
 InspectorStyleSheet::InspectorStyleSheet(InspectorPageAgent* pageAgent,
InspectorResourceAgent* resourceAgent, const String& id,
PassRefPtr<CSSStyleSheet> pageStyleSheet,
TypeBuilder::CSS::StyleSheetOrigin::Enum origin, const String& documentURL,
Listener* listener)
-    : m_pageAgent(pageAgent)
+    : InspectorStyleSheetBase(id, listener)
+    , m_pageAgent(pageAgent)
     , m_resourceAgent(resourceAgent)
-    , m_id(id)
     , m_pageStyleSheet(pageStyleSheet)
     , m_origin(origin)
     , m_documentURL(documentURL)
-    , m_listener(listener)
 {
     m_parsedStyleSheet = adoptPtr(new
ParsedStyleSheet(m_pageStyleSheet.get()));
 }
@@ -869,8 +923,8 @@ String InspectorStyleSheet::finalURL() const
 
 void InspectorStyleSheet::reparseStyleSheet(const String& text)
 {
-    if (m_listener)
-        m_listener->willReparseStyleSheet();
+    if (listener())
+        listener()->willReparseStyleSheet();
 
     {
         // Have a separate scope for clearRules() (bug 95324).
@@ -883,19 +937,14 @@ void InspectorStyleSheet::reparseStyleSheet(const String&
text)
         m_pageStyleSheet->contents()->parseString(text);
     }
 
-    if (m_listener)
-        m_listener->didReparseStyleSheet();
+    if (listener())
+        listener()->didReparseStyleSheet();
     fireStyleSheetChanged();
    
m_pageStyleSheet->ownerDocument()->styleResolverChanged(RecalcStyleImmediately,
FullStyleUpdate);
 }
 
 bool InspectorStyleSheet::setText(const String& text, ExceptionState&
exceptionState)
 {
-    if (!checkPageStyleSheet(exceptionState))
-        return false;
-    if (!m_parsedStyleSheet)
-        return false;
-
     m_parsedStyleSheet->setText(text);
     m_flatRules.clear();
 
@@ -914,8 +963,6 @@ String InspectorStyleSheet::ruleSelector(const
InspectorCSSId& id, ExceptionStat
 
 bool InspectorStyleSheet::setRuleSelector(const InspectorCSSId& id, const
String& selector, ExceptionState& exceptionState)
 {
-    if (!checkPageStyleSheet(exceptionState))
-        return false;
     CSSStyleRule* rule = ruleForId(id);
     if (!rule) {
         exceptionState.throwDOMException(NotFoundError, "No rule was found for
the given ID.");
@@ -950,8 +997,6 @@ static bool checkStyleRuleSelector(Document* document, const
String& selector)
 
 CSSStyleRule* InspectorStyleSheet::addRule(const String& selector,
ExceptionState& exceptionState)
 {
-    if (!checkPageStyleSheet(exceptionState))
-        return 0;
     if (!checkStyleRuleSelector(m_pageStyleSheet->ownerDocument(), selector)) {
         exceptionState.throwDOMException(SyntaxError, "The selector '" +
selector + "' could not be added.");
         return 0;
@@ -998,8 +1043,6 @@ CSSStyleRule* InspectorStyleSheet::addRule(const String&
selector, ExceptionStat
 
 bool InspectorStyleSheet::deleteRule(const InspectorCSSId& id, ExceptionState&
exceptionState)
 {
-    if (!checkPageStyleSheet(exceptionState))
-        return false;
     RefPtrWillBeRawPtr<CSSStyleRule> rule = ruleForId(id);
     if (!rule) {
         exceptionState.throwDOMException(NotFoundError, "No style rule could be
found for the provided ID.");
@@ -1146,49 +1189,6 @@ PassRefPtr<TypeBuilder::CSS::CSSRule>
InspectorStyleSheet::buildObjectForRule(CS
     return result.release();
 }
 
-PassRefPtr<TypeBuilder::CSS::CSSStyle>
InspectorStyleSheet::buildObjectForStyle(CSSStyleDeclaration* style)
-{
-    RefPtr<CSSRuleSourceData> sourceData;
-    if (ensureParsedDataReady())
-        sourceData = ruleSourceDataFor(style);
-
-    InspectorCSSId id = ruleIdByStyle(style);
-    if (id.isEmpty()) {
-        // Any rule coming from User Agent and not from DefaultStyleSheet will
not have id.
-        // See InspectorCSSAgent::buildObjectForRule for details.
-        RefPtr<InspectorStyle> inspectorStyle = InspectorStyle::create(id,
style, this);
-        return inspectorStyle->buildObjectForStyle();
-    }
-    RefPtr<InspectorStyle> inspectorStyle = inspectorStyleForId(id);
-    RefPtr<TypeBuilder::CSS::CSSStyle> result =
inspectorStyle->buildObjectForStyle();
-
-    // Style text cannot be retrieved without stylesheet, so set cssText here.
-    if (sourceData) {
-        String sheetText;
-        bool success = getText(&sheetText);
-        if (success) {
-            const SourceRange& bodyRange = sourceData->ruleBodyRange;
-            result->setCssText(sheetText.substring(bodyRange.start,
bodyRange.end - bodyRange.start));
-        }
-    }
-
-    return result.release();
-}
-
-bool InspectorStyleSheet::setPropertyText(const InspectorCSSId& id, unsigned
propertyIndex, const String& text, bool overwrite, String* oldText,
ExceptionState& exceptionState)
-{
-    RefPtr<InspectorStyle> inspectorStyle = inspectorStyleForId(id);
-    if (!inspectorStyle) {
-        exceptionState.throwDOMException(NotFoundError, "No property could be
found for the given ID.");
-        return false;
-    }
-
-    bool success = inspectorStyle->setPropertyText(propertyIndex, text,
overwrite, oldText, exceptionState);
-    if (success)
-        fireStyleSheetChanged();
-    return success;
-}
-
 bool InspectorStyleSheet::getText(String* result) const
 {
     if (!ensureText())
@@ -1206,12 +1206,6 @@ CSSStyleDeclaration*
InspectorStyleSheet::styleForId(const InspectorCSSId& id) c
     return rule->style();
 }
 
-void InspectorStyleSheet::fireStyleSheetChanged()
-{
-    if (m_listener)
-        m_listener->styleSheetChanged(this);
-}
-
 PassRefPtr<TypeBuilder::CSS::SourceRange>
InspectorStyleSheet::ruleHeaderSourceRange(const…
(message too large)

[lushnikov, 2014-03-19 12:58:31]

The CQ bit was checked by lushnikov@chromium.org

[commit-bot: I haz the power, 2014-03-19 12:58:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/lushnikov@chromium.org/202103002/60001

[commit-bot: I haz the power, 2014-03-19 14:06:44]

Change committed as 169550