Allow render tree building before loading stylesheet elements
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 6 Mar 2017 12:19:43 +0000 (12:19 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 6 Mar 2017 12:19:43 +0000 (12:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=169079
Source/WebCore:

<rdar://problem/30865709>

Reviewed by Andreas Kling.

Currently we don't resolve style or construct renderers if there are any pending
stylesheet loads. This patch enables style and renderer constuction up to the
first encountered loading style element.

This is a step toward allowing incremental rendering for in-body stylesheets.

* dom/Document.cpp:
(WebCore::Document::needsStyleRecalc):

    Ensure scrolling to anchor eventually happens.

* dom/Document.h:
(WebCore::Document::isIgnoringPendingStylesheets):
* dom/InlineStyleSheetOwner.cpp:
(WebCore::InlineStyleSheetOwner::createSheet):
(WebCore::InlineStyleSheetOwner::sheetLoaded):
(WebCore::InlineStyleSheetOwner::startLoadingDynamicSheet):
* dom/ProcessingInstruction.cpp:
(WebCore::ProcessingInstruction::checkStyleSheet):
(WebCore::ProcessingInstruction::sheetLoaded):
(WebCore::ProcessingInstruction::removedFrom):
* html/HTMLLinkElement.cpp:
(WebCore::HTMLLinkElement::addPendingSheet):
(WebCore::HTMLLinkElement::removePendingSheet):
* style/StyleScope.cpp:
(WebCore::Style::Scope::addPendingSheet):
(WebCore::Style::Scope::removePendingSheet):

    Track pending sheet nodes in a map so we can test if a given node has a pending sheet.
    This is also more robust in general.

(WebCore::Style::Scope::hasProcessingInstructionWithPendingSheet):
(WebCore::Style::Scope::updateActiveStyleSheets):
* style/StyleScope.h:
(WebCore::Style::Scope::hasPendingSheet):
(WebCore::Style::Scope::hasPendingSheets):
(WebCore::Style::Scope::addPendingSheet): Deleted.
* style/StyleTreeResolver.cpp:
(WebCore::Style::TreeResolver::styleForElement):

    Instead of global test for placeholder construction check the status of the pending sheet flag.

(WebCore::Style::hasLoadingStylesheet):
(WebCore::Style::TreeResolver::resolveComposedTree):

    Set a flag when encountering a pending top level sheet during DOM traversal.

(WebCore::Style::TreeResolver::resolve):
* style/StyleTreeResolver.h:
* svg/graphics/SVGImage.cpp:
(WebCore::SVGImage::dataChanged):

    Ensure SVG images have layout before getting the size.

LayoutTests:

Reviewed by Andreas Kling.

Ensure that style is synchronized after adding a stylesheet dynamically by doing an additional test.
Otherwise the class/attr invalidation test may as we don't know about the new stylesheet yet.
This is functionally fine (future synchronization would invalidate the style) but messes up the test
trying to verify class/attr change invalidation specifically.

* fast/css/style-invalidation-attribute-change-descendants-expected.txt:
* fast/css/style-invalidation-attribute-change-descendants.html:
* fast/css/style-invalidation-class-change-descendants-expected.txt:
* fast/css/style-invalidation-class-change-descendants.html:

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

16 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/css/style-invalidation-attribute-change-descendants-expected.txt
LayoutTests/fast/css/style-invalidation-attribute-change-descendants.html
LayoutTests/fast/css/style-invalidation-class-change-descendants-expected.txt
LayoutTests/fast/css/style-invalidation-class-change-descendants.html
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/dom/InlineStyleSheetOwner.cpp
Source/WebCore/dom/ProcessingInstruction.cpp
Source/WebCore/html/HTMLLinkElement.cpp
Source/WebCore/style/StyleScope.cpp
Source/WebCore/style/StyleScope.h
Source/WebCore/style/StyleTreeResolver.cpp
Source/WebCore/style/StyleTreeResolver.h
Source/WebCore/svg/graphics/SVGImage.cpp

index 79b4a10..7a81bf3 100644 (file)
@@ -1,3 +1,20 @@
+2017-03-06  Antti Koivisto  <antti@apple.com>
+
+        Allow render tree building before loading stylesheet elements
+        https://bugs.webkit.org/show_bug.cgi?id=169079
+
+        Reviewed by Andreas Kling.
+
+        Ensure that style is synchronized after adding a stylesheet dynamically by doing an additional test.
+        Otherwise the class/attr invalidation test may as we don't know about the new stylesheet yet.
+        This is functionally fine (future synchronization would invalidate the style) but messes up the test
+        trying to verify class/attr change invalidation specifically.
+
+        * fast/css/style-invalidation-attribute-change-descendants-expected.txt:
+        * fast/css/style-invalidation-attribute-change-descendants.html:
+        * fast/css/style-invalidation-class-change-descendants-expected.txt:
+        * fast/css/style-invalidation-class-change-descendants.html:
+
 2017-03-05  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         Unreviewed GTK+ gardening. Rebaseline fast/css/css2-system-fonts.html after r213267.
index 0a74a67..8cef742 100644 (file)
@@ -170,6 +170,10 @@ PASS testStyleChangeType("target", "NoStyleChange") is true
 PASS testStyleChangeType("inert", "NoStyleChange") is true
 PASS hasExpectedStyle is true
 Inserting stylesheet '[myattr3] target { color:rgb(12, 0, 0); }'
+PASS testStyleChangeType("root", "NoStyleChange") || testStyleChangeType("root", "InlineStyleChange") is true
+PASS testStyleChangeType("target", "NoStyleChange") is true
+PASS testStyleChangeType("inert", "NoStyleChange") is true
+PASS hasExpectedStyle is true
 Setting attribute 'myattr3' value ''
 PASS testStyleChangeType("root", "NoStyleChange") || testStyleChangeType("root", "InlineStyleChange") is true
 PASS testStyleChangeType("target", "InlineStyleChange") is true
index 82f9da5..bed82fc 100644 (file)
@@ -276,6 +276,9 @@ dynamicSheet.innerHTML = "[myattr3] target { color:rgb(12, 0, 0); }"
 debug("Inserting stylesheet '" + dynamicSheet.innerHTML + "'");
 document.head.appendChild(dynamicSheet);
 
+testStyleInvalidation("NoStyleChange");
+checkStyle(11);
+
 setAttribute('myattr3', '');
 testStyleInvalidation("InlineStyleChange");
 checkStyle(12);
index f885fa5..87a0085 100644 (file)
@@ -73,6 +73,10 @@ PASS testStyleChangeType("target", "NoStyleChange") is true
 PASS testStyleChangeType("inert", "NoStyleChange") is true
 PASS hasExpectedStyle is true
 Inserting stylesheet 'root.dynamicStyle target { color:rgb(6, 6, 6); }'
+PASS testStyleChangeType("root", "NoStyleChange") || testStyleChangeType("root", "InlineStyleChange") is true
+PASS testStyleChangeType("target", "NoStyleChange") is true
+PASS testStyleChangeType("inert", "NoStyleChange") is true
+PASS hasExpectedStyle is true
 Adding class dynamicStyle
 PASS testStyleChangeType("root", "NoStyleChange") || testStyleChangeType("root", "InlineStyleChange") is true
 PASS testStyleChangeType("target", "InlineStyleChange") is true
index a5926c2..02a9846 100644 (file)
@@ -167,13 +167,16 @@ checkStyle(0);
 
 removeClass('dynamicStyle');
 testStyleInvalidation("NoStyleChange");
-checkStyle(0)
+checkStyle(0);
 
 var dynamicSheet = document.createElement("style");
 dynamicSheet.innerHTML = "root.dynamicStyle target { color:rgb(6, 6, 6); }"
 debug("Inserting stylesheet '" + dynamicSheet.innerHTML + "'");
 document.head.appendChild(dynamicSheet);
 
+testStyleInvalidation("NoStyleChange");
+checkStyle(0);
+
 addClass('dynamicStyle');
 testStyleInvalidation("InlineStyleChange");
 checkStyle(6);
index 716ad2b..6f5fad6 100644 (file)
@@ -1,3 +1,65 @@
+2017-03-06  Antti Koivisto  <antti@apple.com>
+
+        Allow render tree building before loading stylesheet elements
+        https://bugs.webkit.org/show_bug.cgi?id=169079
+        <rdar://problem/30865709>
+
+        Reviewed by Andreas Kling.
+
+        Currently we don't resolve style or construct renderers if there are any pending
+        stylesheet loads. This patch enables style and renderer constuction up to the
+        first encountered loading style element.
+
+        This is a step toward allowing incremental rendering for in-body stylesheets.
+
+        * dom/Document.cpp:
+        (WebCore::Document::needsStyleRecalc):
+
+            Ensure scrolling to anchor eventually happens.
+
+        * dom/Document.h:
+        (WebCore::Document::isIgnoringPendingStylesheets):
+        * dom/InlineStyleSheetOwner.cpp:
+        (WebCore::InlineStyleSheetOwner::createSheet):
+        (WebCore::InlineStyleSheetOwner::sheetLoaded):
+        (WebCore::InlineStyleSheetOwner::startLoadingDynamicSheet):
+        * dom/ProcessingInstruction.cpp:
+        (WebCore::ProcessingInstruction::checkStyleSheet):
+        (WebCore::ProcessingInstruction::sheetLoaded):
+        (WebCore::ProcessingInstruction::removedFrom):
+        * html/HTMLLinkElement.cpp:
+        (WebCore::HTMLLinkElement::addPendingSheet):
+        (WebCore::HTMLLinkElement::removePendingSheet):
+        * style/StyleScope.cpp:
+        (WebCore::Style::Scope::addPendingSheet):
+        (WebCore::Style::Scope::removePendingSheet):
+
+            Track pending sheet nodes in a map so we can test if a given node has a pending sheet.
+            This is also more robust in general.
+
+        (WebCore::Style::Scope::hasProcessingInstructionWithPendingSheet):
+        (WebCore::Style::Scope::updateActiveStyleSheets):
+        * style/StyleScope.h:
+        (WebCore::Style::Scope::hasPendingSheet):
+        (WebCore::Style::Scope::hasPendingSheets):
+        (WebCore::Style::Scope::addPendingSheet): Deleted.
+        * style/StyleTreeResolver.cpp:
+        (WebCore::Style::TreeResolver::styleForElement):
+
+            Instead of global test for placeholder construction check the status of the pending sheet flag.
+
+        (WebCore::Style::hasLoadingStylesheet):
+        (WebCore::Style::TreeResolver::resolveComposedTree):
+
+            Set a flag when encountering a pending top level sheet during DOM traversal.
+
+        (WebCore::Style::TreeResolver::resolve):
+        * style/StyleTreeResolver.h:
+        * svg/graphics/SVGImage.cpp:
+        (WebCore::SVGImage::dataChanged):
+
+            Ensure SVG images have layout before getting the size.
+
 2017-03-06  Vanessa Chipirrás Navalón  <vchipirras@igalia.com>
 
         [GStreamer] Adopt nullptr
index 9c52536..f4b7561 100644 (file)
@@ -1854,7 +1854,20 @@ bool Document::needsStyleRecalc() const
     if (pageCacheState() != NotInPageCache)
         return false;
 
-    return m_pendingStyleRecalcShouldForce || childNeedsStyleRecalc() || styleScope().hasPendingUpdate();
+    if (m_pendingStyleRecalcShouldForce)
+        return true;
+
+    if (childNeedsStyleRecalc())
+        return true;
+
+    if (styleScope().hasPendingUpdate())
+        return true;
+
+    // Ensure this happens eventually as it is currently in resolveStyle. This can be removed if the code moves.
+    if (m_gotoAnchorNeededAfterStylesheetsLoad && !styleScope().hasPendingSheets())
+        return true;
+
+    return false;
 }
 
 void Document::updateStyleIfNeeded()
index 77e1f34..ff192b3 100644 (file)
@@ -484,6 +484,7 @@ public:
     CSSFontSelector& fontSelector() { return m_fontSelector; }
 
     WEBCORE_EXPORT bool haveStylesheetsLoaded() const;
+    bool isIgnoringPendingStylesheets() const { return m_ignorePendingStylesheets; }
 
     WEBCORE_EXPORT StyleSheetList& styleSheets();
 
index a252632..5037d2c 100644 (file)
@@ -157,7 +157,7 @@ void InlineStyleSheetOwner::createSheet(Element& element, const String& text)
     Document& document = element.document();
     if (m_sheet) {
         if (m_sheet->isLoading() && m_styleScope)
-            m_styleScope->removePendingSheet();
+            m_styleScope->removePendingSheet(element);
         clearSheet();
     }
 
@@ -178,7 +178,7 @@ void InlineStyleSheetOwner::createSheet(Element& element, const String& text)
         return;
 
     if (m_styleScope)
-        m_styleScope->addPendingSheet();
+        m_styleScope->addPendingSheet(element);
 
     auto cacheKey = makeInlineStyleSheetCacheKey(text, element);
     if (cacheKey) {
@@ -228,21 +228,21 @@ bool InlineStyleSheetOwner::isLoading() const
     return m_sheet && m_sheet->isLoading();
 }
 
-bool InlineStyleSheetOwner::sheetLoaded(Element&)
+bool InlineStyleSheetOwner::sheetLoaded(Element& element)
 {
     if (isLoading())
         return false;
 
     if (m_styleScope)
-        m_styleScope->removePendingSheet();
+        m_styleScope->removePendingSheet(element);
 
     return true;
 }
 
-void InlineStyleSheetOwner::startLoadingDynamicSheet(Element&)
+void InlineStyleSheetOwner::startLoadingDynamicSheet(Element& element)
 {
     if (m_styleScope)
-        m_styleScope->addPendingSheet();
+        m_styleScope->addPendingSheet(element);
 }
 
 void InlineStyleSheetOwner::clearCache()
index 92c3322..92e8893 100644 (file)
@@ -129,12 +129,17 @@ void ProcessingInstruction::checkStyleSheet()
                 m_cachedSheet = nullptr;
             }
 
+            if (m_loading) {
+                m_loading = false;
+                document().styleScope().removePendingSheet(*this);
+            }
+
             String url = document().completeURL(href).string();
             if (!dispatchBeforeLoadEvent(url))
                 return;
 
             m_loading = true;
-            document().styleScope().addPendingSheet();
+            document().styleScope().addPendingSheet(*this);
 
 #if ENABLE(XSLT)
             if (m_isXSL) {
@@ -154,7 +159,7 @@ void ProcessingInstruction::checkStyleSheet()
             else {
                 // The request may have been denied if (for example) the stylesheet is local and the document is remote.
                 m_loading = false;
-                document().styleScope().removePendingSheet();
+                document().styleScope().removePendingSheet(*this);
 #if ENABLE(XSLT)
                 if (m_isXSL)
                     document().styleScope().flushPendingUpdate();
@@ -176,7 +181,7 @@ bool ProcessingInstruction::isLoading() const
 bool ProcessingInstruction::sheetLoaded()
 {
     if (!isLoading()) {
-        document().styleScope().removePendingSheet();
+        document().styleScope().removePendingSheet(*this);
 #if ENABLE(XSLT)
         if (m_isXSL)
             document().styleScope().flushPendingUpdate();
@@ -276,7 +281,7 @@ void ProcessingInstruction::removedFrom(ContainerNode& insertionPoint)
 
     if (m_loading) {
         m_loading = false;
-        document().styleScope().removePendingSheet();
+        document().styleScope().removePendingSheet(*this);
     }
 
     document().styleScope().didChangeActiveStyleSheetCandidates();
index d6ac449..c8b80ec 100644 (file)
@@ -551,7 +551,7 @@ void HTMLLinkElement::addPendingSheet(PendingSheetType type)
     if (m_pendingSheetType == InactiveSheet)
         return;
     ASSERT(m_styleScope);
-    m_styleScope->addPendingSheet();
+    m_styleScope->addPendingSheet(*this);
 }
 
 void HTMLLinkElement::removePendingSheet()
@@ -569,7 +569,7 @@ void HTMLLinkElement::removePendingSheet()
         return;
     }
 
-    m_styleScope->removePendingSheet();
+    m_styleScope->removePendingSheet(*this);
 }
 
 } // namespace WebCore
index d0345dc..66da360 100644 (file)
@@ -72,6 +72,7 @@ Scope::Scope(ShadowRoot& shadowRoot)
 
 Scope::~Scope()
 {
+    ASSERT(m_nodesWithPendingSheets.isEmpty());
 }
 
 bool Scope::shouldUseSharedUserAgentShadowTreeStyleResolver() const
@@ -171,14 +172,20 @@ void Scope::setSelectedStylesheetSetName(const String& name)
     didChangeActiveStyleSheetCandidates();
 }
 
+void Scope::addPendingSheet(const Node& node)
+{
+    ASSERT(!m_nodesWithPendingSheets.contains(&node));
+
+    m_nodesWithPendingSheets.add(&node);
+}
+
 // This method is called whenever a top-level stylesheet has finished loading.
-void Scope::removePendingSheet()
+void Scope::removePendingSheet(const Node& node)
 {
-    // Make sure we knew this sheet was pending, and that our count isn't out of sync.
-    ASSERT(m_pendingStyleSheetCount > 0);
+    ASSERT(m_nodesWithPendingSheets.contains(&node));
 
-    m_pendingStyleSheetCount--;
-    if (m_pendingStyleSheetCount)
+    m_nodesWithPendingSheets.remove(&node);
+    if (!m_nodesWithPendingSheets.isEmpty())
         return;
 
     didChangeActiveStyleSheetCandidates();
@@ -187,6 +194,21 @@ void Scope::removePendingSheet()
         m_document.didRemoveAllPendingStylesheet();
 }
 
+bool Scope::hasProcessingInstructionWithPendingSheet()
+{
+    ASSERT(!m_shadowRoot);
+
+    for (auto* child = m_document.firstChild(); child; child = child->nextSibling()) {
+        if (is<Element>(*child))
+            return false;
+        if (m_nodesWithPendingSheets.contains(child)) {
+            ASSERT(is<ProcessingInstruction>(*child));
+            return true;
+        }
+    }
+    return false;
+}
+
 void Scope::addStyleSheetCandidateNode(Node& node, bool createdByParser)
 {
     if (!node.isConnected())
@@ -394,7 +416,7 @@ void Scope::updateActiveStyleSheets(UpdateType updateType)
 
     // Don't bother updating, since we haven't loaded all our style info yet
     // and haven't calculated the style resolver for the first time.
-    if (!m_shadowRoot && !m_didUpdateActiveStyleSheets && m_pendingStyleSheetCount) {
+    if (!m_shadowRoot && !m_didUpdateActiveStyleSheets && hasPendingSheets()) {
         clearResolver();
         return;
     }
index 27e5c45..e1a094f 100644 (file)
@@ -31,6 +31,7 @@
 #include <memory>
 #include <wtf/FastMalloc.h>
 #include <wtf/HashMap.h>
+#include <wtf/HashSet.h>
 #include <wtf/ListHashSet.h>
 #include <wtf/RefPtr.h>
 #include <wtf/Vector.h>
@@ -81,10 +82,11 @@ public:
     void setPreferredStylesheetSetName(const String&);
     void setSelectedStylesheetSetName(const String&);
 
-    void addPendingSheet() { m_pendingStyleSheetCount++; }
-    void removePendingSheet();
-
-    bool hasPendingSheets() const { return m_pendingStyleSheetCount > 0; }
+    void addPendingSheet(const Node&);
+    void removePendingSheet(const Node&);
+    bool hasPendingSheet(const Node& node) const { return m_nodesWithPendingSheets.contains(&node); }
+    bool hasProcessingInstructionWithPendingSheet();
+    bool hasPendingSheets() const { return !m_nodesWithPendingSheets.isEmpty(); }
 
     bool usesStyleBasedEditability() { return m_usesStyleBasedEditability; }
 
@@ -141,15 +143,17 @@ private:
     Vector<RefPtr<StyleSheet>> m_styleSheetsForStyleSheetList;
     Vector<RefPtr<CSSStyleSheet>> m_activeStyleSheets;
 
+
     Timer m_pendingUpdateTimer;
 
     mutable std::unique_ptr<HashSet<const CSSStyleSheet*>> m_weakCopyOfActiveStyleSheetListForFastLookup;
 
-    // Track the number of currently loading top-level stylesheets needed for rendering.
+    // Track the 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 and when it is safe to execute scripts.
-    int m_pendingStyleSheetCount { 0 };
+    HashSet<const Node*> m_nodesWithPendingSheets;
+
     bool m_didUpdateActiveStyleSheets { false };
 
     std::optional<UpdateType> m_pendingUpdate;
index d0fd059..6ed3400 100644 (file)
@@ -126,7 +126,7 @@ void TreeResolver::popScope()
 
 std::unique_ptr<RenderStyle> TreeResolver::styleForElement(Element& element, const RenderStyle& inheritedStyle)
 {
-    if (!m_document.haveStylesheetsLoaded() && !element.renderer()) {
+    if (m_didSeePendingStylesheet && !element.renderer() && !m_document.isIgnoringPendingStylesheets()) {
         m_document.setHasNodesWithPlaceholderStyle();
         return makePlaceholderStyle(m_document);
     }
@@ -359,6 +359,21 @@ static void clearNeedsStyleResolution(Element& element)
         after->setHasValidStyle();
 }
 
+static bool hasLoadingStylesheet(const Style::Scope& styleScope, const Element& element, bool checkDescendants)
+{
+    if (!styleScope.hasPendingSheets())
+        return false;
+    if (styleScope.hasPendingSheet(element))
+        return true;
+    if (!checkDescendants)
+        return false;
+    for (auto& descendant : descendantsOfType<Element>(element)) {
+        if (styleScope.hasPendingSheet(descendant))
+            return true;
+    };
+    return false;
+}
+
 void TreeResolver::resolveComposedTree()
 {
     ASSERT(m_parentStack.size() == 1);
@@ -438,6 +453,10 @@ void TreeResolver::resolveComposedTree()
         }
 
         bool shouldIterateChildren = style && (element.childNeedsStyleRecalc() || change != NoChange);
+
+        if (!m_didSeePendingStylesheet)
+            m_didSeePendingStylesheet = hasLoadingStylesheet(m_document.styleScope(), element, !shouldIterateChildren);
+
         if (!shouldIterateChildren) {
             it.traverseNextSkippingChildren();
             continue;
@@ -463,6 +482,8 @@ std::unique_ptr<Update> TreeResolver::resolve()
     if (!documentElement->childNeedsStyleRecalc() && !documentElement->needsStyleRecalc())
         return nullptr;
 
+    m_didSeePendingStylesheet = m_document.styleScope().hasProcessingInstructionWithPendingSheet();
+
     m_update = std::make_unique<Update>(m_document);
     m_scopeStack.append(adoptRef(*new Scope(m_document)));
     m_parentStack.append(Parent(m_document));
index 331c50b..2a2737a 100644 (file)
@@ -102,6 +102,7 @@ private:
 
     Vector<Ref<Scope>, 4> m_scopeStack;
     Vector<Parent, 32> m_parentStack;
+    bool m_didSeePendingStylesheet { false };
 
     std::unique_ptr<Update> m_update;
 };
index 27cec3d..746113c 100644 (file)
@@ -451,6 +451,8 @@ bool SVGImage::dataChanged(bool allDataReceived)
         loader.activeDocumentLoader()->writer().addData(data()->data(), data()->size());
         loader.activeDocumentLoader()->writer().end();
 
+        frame.document()->updateLayoutIgnorePendingStylesheets();
+
         // Set the intrinsic size before a container size is available.
         m_intrinsicSize = containerSize();
         reportApproximateMemoryCost();