2011-06-01 Julien Chaffraix <jchaffraix@codeaurora.org>
authorjchaffraix@webkit.org <jchaffraix@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 2 Jun 2011 00:53:01 +0000 (00:53 +0000)
committerjchaffraix@webkit.org <jchaffraix@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 2 Jun 2011 00:53:01 +0000 (00:53 +0000)
        Reviewed by Simon Fraser.

        CSSStyleSheet#insertRule doesn't work well with imported stylesheets
        https://bugs.webkit.org/show_bug.cgi?id=56981

        Test that a combination of insertRule and @import works properly.

        * fast/css/import-and-insert-rule-no-update-expected.txt: Added.
        * fast/css/import-and-insert-rule-no-update.html: Added.
        * fast/css/resources/red.css: Added.
        (div):
        * fast/css/resources/redimport.css: Added.
2011-06-01  Julien Chaffraix  <jchaffraix@codeaurora.org>

        Reviewed by Simon Fraser.

        CSSStyleSheet#insertRule doesn't work well with imported stylesheets
        https://bugs.webkit.org/show_bug.cgi?id=56981

        Test: fast/css/import-and-insert-rule-no-update.html

        The bug arises from the fact that <link> element did not know about a programmatically
        loading sheet (using insertRule and @import) and would thus never call removePendingSheet.
        This is needed to make sure our style selector contains an up-to-date list of stylesheets.

        The gist of the patch adds a way for style sheet owner element to know if we are
        programmatically loading a style sheet. This is needed as <link> keeps the information
        about that last loaded stylesheet.

        * css/CSSImportRule.cpp:
        (WebCore::CSSImportRule::insertedIntoParent): Call startLoadingDynamicSheet
        on our parent style sheet instead of directly adding a pending style sheet.

        * css/CSSStyleSheet.cpp:
        (WebCore::CSSStyleSheet::startLoadingDynamicSheet): Call startLoadingDynamicSheet
        on our owner element if we have one.

        * css/CSSStyleSheet.h:
        * dom/Node.h:
        (WebCore::Node::startLoadingDynamicSheet): Added common implementation of
        startLoadingDynamicSheet, which should never be called.

        * dom/StyleElement.cpp:
        (WebCore::StyleElement::startLoadingDynamicSheet):
        * dom/StyleElement.h:
        Common implementation of startLoadingDynamicSheet for style elements.

        * html/HTMLLinkElement.cpp:
        (WebCore::HTMLLinkElement::startLoadingDynamicSheet):
        * html/HTMLLinkElement.h:
        Use the HTMLLinkElement plumbing to make sure we call addRemovePendingSheet.

        * html/HTMLStyleElement.h:
        (WebCore::HTMLStyleElement::startLoadingDynamicSheet):
        * svg/SVGStyleElement.h:
        (WebCore::SVGStyleElement::startLoadingDynamicSheet):
        Forward the call to StyleElement.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@87867 268f45cc-cd09-0410-ab3c-d52691b4dbfc

16 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/css/import-and-insert-rule-no-update-expected.txt [new file with mode: 0644]
LayoutTests/fast/css/import-and-insert-rule-no-update.html [new file with mode: 0644]
LayoutTests/fast/css/resources/red.css [new file with mode: 0644]
LayoutTests/fast/css/resources/redimport.css [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/CSSImportRule.cpp
Source/WebCore/css/CSSStyleSheet.cpp
Source/WebCore/css/CSSStyleSheet.h
Source/WebCore/dom/Node.h
Source/WebCore/dom/StyleElement.cpp
Source/WebCore/dom/StyleElement.h
Source/WebCore/html/HTMLLinkElement.cpp
Source/WebCore/html/HTMLLinkElement.h
Source/WebCore/html/HTMLStyleElement.h
Source/WebCore/svg/SVGStyleElement.h

index 3dca075..9abd1b2 100644 (file)
@@ -1,3 +1,18 @@
+2011-06-01  Julien Chaffraix  <jchaffraix@codeaurora.org>
+
+        Reviewed by Simon Fraser.
+
+        CSSStyleSheet#insertRule doesn't work well with imported stylesheets
+        https://bugs.webkit.org/show_bug.cgi?id=56981
+
+        Test that a combination of insertRule and @import works properly.
+
+        * fast/css/import-and-insert-rule-no-update-expected.txt: Added.
+        * fast/css/import-and-insert-rule-no-update.html: Added.
+        * fast/css/resources/red.css: Added.
+        (div):
+        * fast/css/resources/redimport.css: Added.
+
 2011-06-01  Abhishek Arya  <inferno@chromium.org>
 
         Reviewed by Alexey Proskuryakov.
diff --git a/LayoutTests/fast/css/import-and-insert-rule-no-update-expected.txt b/LayoutTests/fast/css/import-and-insert-rule-no-update-expected.txt
new file mode 100644 (file)
index 0000000..474d8cb
--- /dev/null
@@ -0,0 +1,3 @@
+Test for bug 56981: CSSStyleSheet#insertRule doesn't work well with imported stylesheets
+You should see one PASS below.
+PASS
diff --git a/LayoutTests/fast/css/import-and-insert-rule-no-update.html b/LayoutTests/fast/css/import-and-insert-rule-no-update.html
new file mode 100644 (file)
index 0000000..aab5af0
--- /dev/null
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<html><head>
+<link rel="stylesheet" href="resources/redimport.css"/>
+</head><body>
+<div>Test for bug <a href="https://bugs.webkit.org/show_bug.cgi?id=56981">56981</a>: CSSStyleSheet#insertRule doesn't work well with imported stylesheets</div>
+<div>You should see one PASS below.</div>
+<div id="testArea"></div>
+<script>
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+    layoutTestController.waitUntilDone();
+}
+
+var remainingTests = 20;
+
+function test() {
+    try {
+        var testArea = document.getElementById("testArea");
+        if (getComputedStyle(testArea).backgroundColor == "rgb(0, 128, 0)") {
+            testArea.innerHTML = 'PASS';
+            remainingTests = 0;
+        } else {
+            if (--remainingTests)
+                testArea.innerHTML = 'FAIL, backgroundColor was ' + getComputedStyle(testArea).backgroundColor;
+        }
+    } catch (e) {
+        testArea.innerHTML = 'FAIL, exception raised (' + e.message + ')';
+        remainingTests = 0;
+    }
+    if (!remainingTests)
+        window.setTimeout(test, 25);
+    if (window.layoutTestController)
+        layoutTestController.notifyDone();
+}
+
+window.onload = function() {
+    document.styleSheets[0].insertRule('@import "green.css";', 1);
+    // We need to wait some time to let the stylesheet load before testing.
+    window.setTimeout(test, 25);
+};
+</script>
+</body></html>
diff --git a/LayoutTests/fast/css/resources/red.css b/LayoutTests/fast/css/resources/red.css
new file mode 100644 (file)
index 0000000..cdd3498
--- /dev/null
@@ -0,0 +1 @@
+div { background-color: red }
diff --git a/LayoutTests/fast/css/resources/redimport.css b/LayoutTests/fast/css/resources/redimport.css
new file mode 100644 (file)
index 0000000..078d5ed
--- /dev/null
@@ -0,0 +1 @@
+@import url(red.css);
index 0ebc241..6520863 100644 (file)
@@ -1,3 +1,49 @@
+2011-06-01  Julien Chaffraix  <jchaffraix@codeaurora.org>
+
+        Reviewed by Simon Fraser.
+
+        CSSStyleSheet#insertRule doesn't work well with imported stylesheets
+        https://bugs.webkit.org/show_bug.cgi?id=56981
+
+        Test: fast/css/import-and-insert-rule-no-update.html
+
+        The bug arises from the fact that <link> element did not know about a programmatically
+        loading sheet (using insertRule and @import) and would thus never call removePendingSheet.
+        This is needed to make sure our style selector contains an up-to-date list of stylesheets.
+
+        The gist of the patch adds a way for style sheet owner element to know if we are
+        programmatically loading a style sheet. This is needed as <link> keeps the information
+        about that last loaded stylesheet.
+
+        * css/CSSImportRule.cpp:
+        (WebCore::CSSImportRule::insertedIntoParent): Call startLoadingDynamicSheet
+        on our parent style sheet instead of directly adding a pending style sheet.
+
+        * css/CSSStyleSheet.cpp:
+        (WebCore::CSSStyleSheet::startLoadingDynamicSheet): Call startLoadingDynamicSheet
+        on our owner element if we have one.
+
+        * css/CSSStyleSheet.h:
+        * dom/Node.h:
+        (WebCore::Node::startLoadingDynamicSheet): Added common implementation of
+        startLoadingDynamicSheet, which should never be called.
+
+        * dom/StyleElement.cpp:
+        (WebCore::StyleElement::startLoadingDynamicSheet):
+        * dom/StyleElement.h:
+        Common implementation of startLoadingDynamicSheet for style elements.
+
+        * html/HTMLLinkElement.cpp:
+        (WebCore::HTMLLinkElement::startLoadingDynamicSheet):
+        * html/HTMLLinkElement.h:
+        Use the HTMLLinkElement plumbing to make sure we call addRemovePendingSheet.
+
+        * html/HTMLStyleElement.h:
+        (WebCore::HTMLStyleElement::startLoadingDynamicSheet):
+        * svg/SVGStyleElement.h:
+        (WebCore::SVGStyleElement::startLoadingDynamicSheet):
+        Forward the call to StyleElement.
+
 2011-06-01  Levi Weintraub  <leviw@chromium.org>
 
         Reviewed by Eric Seidel.
index 5a08c0b..e86f674 100644 (file)
@@ -144,7 +144,7 @@ void CSSImportRule::insertedIntoParent()
         // removed from the pending sheet count, so let the doc know
         // the sheet being imported is pending.
         if (parentSheet && parentSheet->loadCompleted() && root == parentSheet)
-            parentSheet->document()->addPendingSheet();
+            parentSheet->startLoadingDynamicSheet();
         m_loading = true;
         m_cachedSheet->addClient(this);
     }
index 16c2ba8..2fc3f48 100644 (file)
@@ -230,6 +230,12 @@ void CSSStyleSheet::checkLoaded()
     m_loadCompleted = ownerNode() ? ownerNode()->sheetLoaded() : true;
 }
 
+void CSSStyleSheet::startLoadingDynamicSheet()
+{
+    if (Node* owner = ownerNode())
+        owner->startLoadingDynamicSheet();
+}
+
 Document* CSSStyleSheet::document()
 {
     StyleBase* styleObject = this;
index 062886a..35a4510 100644 (file)
@@ -86,6 +86,7 @@ public:
     virtual bool isLoading();
 
     virtual void checkLoaded();
+    void startLoadingDynamicSheet();
 
     Document* document();
 
index 6490e3b..93da879 100644 (file)
@@ -283,6 +283,7 @@ public:
 
     // For <link> and <style> elements.
     virtual bool sheetLoaded() { return true; }
+    virtual void startLoadingDynamicSheet() { ASSERT_NOT_REACHED(); }
 
     bool hasID() const { return getFlag(HasIDFlag); }
     bool hasClass() const { return getFlag(HasClassFlag); }
index 5b0e2ad..533934c 100644 (file)
@@ -182,4 +182,10 @@ bool StyleElement::sheetLoaded(Document* document)
     return true;
 }
 
+void StyleElement::startLoadingDynamicSheet(Document* document)
+{
+    ASSERT(document);
+    document->addPendingSheet();
+}
+
 }
index 4356c17..17fc648 100644 (file)
@@ -41,6 +41,7 @@ protected:
 
     bool isLoading() const;
     bool sheetLoaded(Document*);
+    void startLoadingDynamicSheet(Document*);
 
     void insertedIntoDocument(Document*, Element*);
     void removedFromDocument(Document*, Element*);
index d2cb074..26191e1 100644 (file)
@@ -472,6 +472,13 @@ bool HTMLLinkElement::sheetLoaded()
     return false;
 }
 
+void HTMLLinkElement::startLoadingDynamicSheet()
+{
+    // We don't support multiple blocking sheets.
+    ASSERT(m_pendingSheetType < Blocking);
+    addPendingSheet(Blocking);
+}
+
 bool HTMLLinkElement::isURLAttribute(Attribute *attr) const
 {
     return attr->name() == hrefAttr;
index dcc7e67..5e1cbd4 100644 (file)
@@ -101,6 +101,7 @@ private:
     virtual void notifyFinished(CachedResource*);
 #endif
     virtual bool sheetLoaded();
+    virtual void startLoadingDynamicSheet();
 
     bool isAlternate() const { return m_disabledState == Unset && m_relAttribute.m_isAlternate; }
     
index c9b5649..af28085 100644 (file)
@@ -55,6 +55,7 @@ private:
 
     virtual bool isLoading() const { return StyleElement::isLoading(); }
     virtual bool sheetLoaded() { return StyleElement::sheetLoaded(document()); }
+    virtual void startLoadingDynamicSheet() { StyleElement::startLoadingDynamicSheet(document()); }
 
     virtual void addSubresourceAttributeURLs(ListHashSet<KURL>&) const;
 
index 9f0ff31..e81c2c8 100644 (file)
@@ -59,6 +59,7 @@ private:
 
     virtual bool isLoading() const { return StyleElement::isLoading(); }
     virtual bool sheetLoaded() { return StyleElement::sheetLoaded(document()); }
+    virtual void startLoadingDynamicSheet() { StyleElement::startLoadingDynamicSheet(document()); }
 };
 
 } // namespace WebCore