WebCore: https://bugs.webkit.org/show_bug.cgi?id=51354
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 22 Dec 2010 14:00:55 +0000 (14:00 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 22 Dec 2010 14:00:55 +0000 (14:00 +0000)
Don't block rendering and script execution on deferred stylesheets

Reviewed by Alexey Proskuryakov.

- Don't add low priority stylesheets to the document pending sheet count.
- Resolve media attribute fully for the link element stylesheet load.

Test: http/tests/local/stylesheet-and-script-load-order-media-print.html

* html/HTMLLinkElement.cpp:
(WebCore::HTMLLinkElement::HTMLLinkElement):
(WebCore::HTMLLinkElement::~HTMLLinkElement):
(WebCore::HTMLLinkElement::setDisabledState):
(WebCore::HTMLLinkElement::process):
(WebCore::HTMLLinkElement::sheetLoaded):
(WebCore::HTMLLinkElement::addPendingSheet):
(WebCore::HTMLLinkElement::removePendingSheet):
* html/HTMLLinkElement.h:

LayoutTests: https://bugs.webkit.org/show_bug.cgi?id=51354
Don't block rendering and script execution on deferred stylesheets

Reviewed by Alexey Proskuryakov.

* http/tests/local/link-stylesheet-load-order-expected.txt:
* http/tests/local/link-stylesheet-load-order-preload-expected.txt:
* http/tests/local/resources/check-stylesheet-not-loaded.js: Added.
* http/tests/local/stylesheet-and-script-load-order-media-print-expected.txt: Added.
* http/tests/local/stylesheet-and-script-load-order-media-print.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/http/tests/local/link-stylesheet-load-order-expected.txt
LayoutTests/http/tests/local/link-stylesheet-load-order-preload-expected.txt
LayoutTests/http/tests/local/resources/check-stylesheet-not-loaded.js [new file with mode: 0644]
LayoutTests/http/tests/local/stylesheet-and-script-load-order-media-print-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/local/stylesheet-and-script-load-order-media-print.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/dom/Document.h
WebCore/html/HTMLLinkElement.cpp
WebCore/html/HTMLLinkElement.h

index 208ff31..6542066 100644 (file)
@@ -1,3 +1,16 @@
+2010-12-20  Antti Koivisto  <antti@apple.com>
+
+        Reviewed by Alexey Proskuryakov.
+
+        https://bugs.webkit.org/show_bug.cgi?id=51354
+        Don't block rendering and script execution on deferred stylesheets
+
+        * http/tests/local/link-stylesheet-load-order-expected.txt:
+        * http/tests/local/link-stylesheet-load-order-preload-expected.txt:
+        * http/tests/local/resources/check-stylesheet-not-loaded.js: Added.
+        * http/tests/local/stylesheet-and-script-load-order-media-print-expected.txt: Added.
+        * http/tests/local/stylesheet-and-script-load-order-media-print.html: Added.
+
 2010-12-22  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r74473.
index 67cfb6d..bf0f9ff 100644 (file)
@@ -8,17 +8,16 @@ media-all.css
 media-screen.css
 media-screen-query-success.css
 media-braille-screen.css
-media-screen-query-fail.css
 no-media-2.css
 media-empty-2.css
 media-all-2.css
 media-screen-2.css
 media-screen-query-success-2.css
 media-braille-screen-2.css
-media-screen-query-fail-2.css
 script.js
 image.jpg
 media-print.css
+media-screen-query-fail.css
 media-aural.css
 media-braille.css
 median-handheld.css
@@ -26,3 +25,4 @@ media-projection.css
 media-tty.css
 media-tv.css
 media-print-2.css
+media-screen-query-fail-2.css
index 6a3df6a..5bd7aee 100644 (file)
@@ -14,12 +14,11 @@ media-all-2.css
 media-screen-2.css
 media-braille-screen-2.css
 media-screen-query-success.css
-media-screen-query-fail.css
 media-screen-query-success-2.css
-media-screen-query-fail-2.css
 script-2.js
 image.jpg
 media-print.css
+media-screen-query-fail.css
 media-aural.css
 media-braille.css
 median-handheld.css
@@ -27,3 +26,4 @@ media-projection.css
 media-tty.css
 media-tv.css
 media-print-2.css
+media-screen-query-fail-2.css
diff --git a/LayoutTests/http/tests/local/resources/check-stylesheet-not-loaded.js b/LayoutTests/http/tests/local/resources/check-stylesheet-not-loaded.js
new file mode 100644 (file)
index 0000000..63eb0a1
--- /dev/null
@@ -0,0 +1 @@
+document.write(document.styleSheets[0] ? "FAIL" : "PASS");
diff --git a/LayoutTests/http/tests/local/stylesheet-and-script-load-order-media-print-expected.txt b/LayoutTests/http/tests/local/stylesheet-and-script-load-order-media-print-expected.txt
new file mode 100644 (file)
index 0000000..84140d8
--- /dev/null
@@ -0,0 +1,3 @@
+Test that script execution doesn't block on print sheet load.
+
+PASS
diff --git a/LayoutTests/http/tests/local/stylesheet-and-script-load-order-media-print.html b/LayoutTests/http/tests/local/stylesheet-and-script-load-order-media-print.html
new file mode 100644 (file)
index 0000000..ff2c334
--- /dev/null
@@ -0,0 +1,13 @@
+<html>
+<head>
+    <script>  
+        if (window.layoutTestController)
+            layoutTestController.dumpAsText();
+    </script>
+    <link rel=stylesheet media=print href="http://127.0.0.1:8000/local/slow-css-pass.cgi">
+</head>
+<body>
+    <p>Test that script execution doesn't block on print sheet load.</p>
+    <script src="http://127.0.0.1:8000/local/resources/check-stylesheet-not-loaded.js"></script>
+</body>
+</html>
index fa0064c..f6aa369 100644 (file)
@@ -1,3 +1,25 @@
+2010-12-20  Antti Koivisto  <antti@apple.com>
+
+        Reviewed by Alexey Proskuryakov.
+
+        https://bugs.webkit.org/show_bug.cgi?id=51354
+        Don't block rendering and script execution on deferred stylesheets
+        
+        - Don't add low priority stylesheets to the document pending sheet count.
+        - Resolve media attribute fully for the link element stylesheet load.
+
+        Test: http/tests/local/stylesheet-and-script-load-order-media-print.html
+
+        * html/HTMLLinkElement.cpp:
+        (WebCore::HTMLLinkElement::HTMLLinkElement):
+        (WebCore::HTMLLinkElement::~HTMLLinkElement):
+        (WebCore::HTMLLinkElement::setDisabledState):
+        (WebCore::HTMLLinkElement::process):
+        (WebCore::HTMLLinkElement::sheetLoaded):
+        (WebCore::HTMLLinkElement::addPendingSheet):
+        (WebCore::HTMLLinkElement::removePendingSheet):
+        * html/HTMLLinkElement.h:
+
 2010-12-22  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r74473.
index 7df0f8e..39072dd 100644 (file)
@@ -1161,10 +1161,10 @@ private:
     RefPtr<DocumentType> m_docType;
     mutable RefPtr<DOMImplementation> m_implementation;
 
-    // Track the number of currently loading top-level stylesheets.  Sheets
-    // loaded using the @import directive are not included in this count.
+    // Track the number of currently loading top-level stylesheets needed for rendering.
+    // Sheets loaded using the @import directive are not included in this count.
     // We use this count of pending sheets to detect when we can begin attaching
-    // elements.
+    // elements and when it is safe to execute scripts.
     int m_pendingStylesheets;
 
     // But sometimes you need to ignore pending stylesheet count to
index a6e5b57..c71544f 100644 (file)
 #include "Attribute.h"
 #include "CachedCSSStyleSheet.h"
 #include "CachedResourceLoader.h"
+#include "CSSStyleSelector.h"
 #include "Document.h"
 #include "Frame.h"
 #include "FrameLoader.h"
 #include "FrameLoaderClient.h"
 #include "FrameTree.h"
+#include "FrameView.h"
 #include "HTMLNames.h"
 #include "HTMLParserIdioms.h"
 #include "MediaList.h"
@@ -54,6 +56,7 @@ inline HTMLLinkElement::HTMLLinkElement(const QualifiedName& tagName, Document*
     , m_disabledState(Unset)
     , m_loading(false)
     , m_createdByParser(createdByParser)
+    , m_pendingSheetType(None)
 {
     ASSERT(hasTagName(linkTag));
 }
@@ -69,9 +72,8 @@ HTMLLinkElement::~HTMLLinkElement()
         m_sheet->clearOwnerNode();
 
     if (m_cachedSheet) {
-        m_cachedSheet->removeClient(this);
-        if (m_loading && !isDisabled() && !isAlternate())
-            document()->removePendingSheet();
+        m_cachedSheet->removeClient(this);    
+        removePendingSheet();
     }
     
 #if ENABLE(LINK_PREFETCH)
@@ -88,15 +90,13 @@ void HTMLLinkElement::setDisabledState(bool _disabled)
         // If we change the disabled state while the sheet is still loading, then we have to
         // perform three checks:
         if (isLoading()) {
-            // Check #1: If the sheet becomes disabled while it was loading, and if it was either
-            // a main sheet or a sheet that was previously enabled via script, then we need
-            // to remove it from the list of pending sheets.
-            if (m_disabledState == Disabled && (!m_relAttribute.m_isAlternate || oldDisabledState == EnabledViaScript))
-                document()->removePendingSheet();
+            // Check #1: The sheet becomes disabled while loading.
+            if (m_disabledState == Disabled)
+                removePendingSheet();
 
             // Check #2: An alternate sheet becomes enabled while it is still loading.
             if (m_relAttribute.m_isAlternate && m_disabledState == EnabledViaScript)
-                document()->addPendingSheet();
+                addPendingSheet(Blocking);
 
             // Check #3: A main sheet becomes enabled while it was still loading and
             // after it was disabled via script.  It takes really terrible code to make this
@@ -104,7 +104,7 @@ void HTMLLinkElement::setDisabledState(bool _disabled)
             // virtualplastic.net, which manages to do about 12 enable/disables on only 3
             // sheets. :)
             if (!m_relAttribute.m_isAlternate && m_disabledState == EnabledViaScript && oldDisabledState == Disabled)
-                document()->addPendingSheet();
+                addPendingSheet(Blocking);
 
             // If the sheet is already loading just bail.
             return;
@@ -223,13 +223,6 @@ void HTMLLinkElement::process()
 #endif
 
     bool acceptIfTypeContainsTextCSS = document()->page() && document()->page()->settings() && document()->page()->settings()->treatsAnyTextCSSLinkAsStylesheet();
-
-    bool mediaIsScreen = true;
-    if (!m_media.isEmpty()) {
-        RefPtr<MediaList> media = MediaList::createAllowingDescriptionSyntax(m_media);
-        MediaQueryEvaluator screenEvaluator("screen", true);
-        mediaIsScreen = screenEvaluator.eval(media.get());
-    }
     
     if (m_disabledState != Disabled && (m_relAttribute.m_isStyleSheet || (acceptIfTypeContainsTextCSS && type.contains("text/css"))) 
         && document()->frame() && m_url.isValid()) {
@@ -237,10 +230,9 @@ void HTMLLinkElement::process()
         String charset = getAttribute(charsetAttr);
         if (charset.isEmpty() && document()->frame())
             charset = document()->frame()->loader()->writer()->encoding();
-
+        
         if (m_cachedSheet) {
-            if (m_loading)
-                document()->removePendingSheet();
+            removePendingSheet();
             m_cachedSheet->removeClient(this);
             m_cachedSheet = 0;
         }
@@ -249,14 +241,22 @@ void HTMLLinkElement::process()
             return;
         
         m_loading = true;
-        
-        // Add ourselves as a pending sheet, but only if we aren't an alternate 
-        // stylesheet.  Alternate stylesheets don't hold up render tree construction.
-        if (!isAlternate())
-            document()->addPendingSheet();
 
-        // Load non-screen stylesheets with low priority so they don't affect normal page loading.
-        ResourceLoadPriority priority = mediaIsScreen ? ResourceLoadPriorityUnresolved : ResourceLoadPriorityVeryLow;
+        bool mediaQueryMatches = true;
+        if (!m_media.isEmpty()) {
+            RefPtr<RenderStyle> documentStyle = CSSStyleSelector::styleForDocument(document());
+            RefPtr<MediaList> media = MediaList::createAllowingDescriptionSyntax(m_media);
+            MediaQueryEvaluator evaluator(document()->frame()->view()->mediaType(), document()->frame(), documentStyle.get());
+            mediaQueryMatches = evaluator.eval(media.get());
+        }
+
+        // Don't hold up render tree construction and script execution on stylesheets
+        // that are not needed for the rendering at the moment.
+        bool blocking = mediaQueryMatches && !isAlternate();
+        addPendingSheet(blocking ? Blocking : NonBlocking);
+
+        // Load stylesheets that are not needed for the rendering immediately with low priority.
+        ResourceLoadPriority priority = blocking ? ResourceLoadPriorityUnresolved : ResourceLoadPriorityVeryLow;
         m_cachedSheet = document()->cachedResourceLoader()->requestCSSStyleSheet(m_url, charset, priority);
         
         if (m_cachedSheet)
@@ -264,8 +264,7 @@ void HTMLLinkElement::process()
         else {
             // The request may have been denied if (for example) the stylesheet is local and the document is remote.
             m_loading = false;
-            if (!isAlternate())
-                document()->removePendingSheet();
+            removePendingSheet();
         }
     } else if (m_sheet) {
         // we no longer contain a stylesheet, e.g. perhaps rel or type was changed
@@ -397,8 +396,8 @@ void HTMLLinkElement::notifyFinished(CachedResource* resource)
 
 bool HTMLLinkElement::sheetLoaded()
 {
-    if (!isLoading() && !isDisabled() && !isAlternate()) {
-        document()->removePendingSheet();
+    if (!isLoading()) {
+        removePendingSheet();
         return true;
     }
     return false;
@@ -448,4 +447,30 @@ void HTMLLinkElement::addSubresourceAttributeURLs(ListHashSet<KURL>& urls) const
         styleSheet->addSubresourceStyleURLs(urls);
 }
 
+void HTMLLinkElement::addPendingSheet(PendingSheetType type)
+{
+    if (type <= m_pendingSheetType)
+        return;
+    m_pendingSheetType = type;
+
+    if (m_pendingSheetType == NonBlocking)
+        return;
+    document()->addPendingSheet();
+}
+
+void HTMLLinkElement::removePendingSheet()
+{
+    PendingSheetType type = m_pendingSheetType;
+    m_pendingSheetType = None;
+
+    if (type == None)
+        return;
+    if (type == NonBlocking) {
+        // Document::removePendingSheet() triggers the style selector recalc for blocking sheets.
+        document()->styleSelectorChanged(RecalcStyleImmediately);
+        return;
+    }
+    document()->removePendingSheet();
+}
+
 }
index f359b8a..bdbdeb5 100644 (file)
@@ -108,6 +108,10 @@ private:
     virtual void addSubresourceAttributeURLs(ListHashSet<KURL>&) const;
 
     virtual void finishParsingChildren();
+    
+    enum PendingSheetType { None, NonBlocking, Blocking };
+    void addPendingSheet(PendingSheetType);
+    void removePendingSheet();
 
 private:
     HTMLLinkElement(const QualifiedName&, Document*, bool createdByParser);
@@ -131,6 +135,8 @@ private:
     RelAttribute m_relAttribute;
     bool m_loading;
     bool m_createdByParser;
+    
+    PendingSheetType m_pendingSheetType;
 };
 
 } //namespace