Style::Scope::flushPendingUpdate() can replace the entire document in XSLTProcessor...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Oct 2017 00:30:03 +0000 (00:30 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Oct 2017 00:30:03 +0000 (00:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=178715
<rdar://problem/35144665>

Reviewed by Brent Fulgham.

Apply XLS tranforms when a 0s timer fires or the document finishes parsing or loading whichever comes first
instead of in the middle of collecting a list of stylesheets.

* dom/Document.cpp:
(WebCore::Document::Document): Initialize the newly added timer.
(WebCore::Document::implicitClose): Apply any pending XSLT before we fire load events since some of the event
handlers may be expecting to see the document after XSLT had been applied.
(WebCore::Document::scheduleToApplyXSLTransforms): Added.
(WebCore::Document::applyPendingXSLTransformsNowIfScheduled): Added.
(WebCore::Document::applyPendingXSLTransformsTimerFired): Added. Moved the logic to apply XSL transforms from
Style::Scope::collectActiveStyleSheets, and merged applyXSLTransform into this function.
(WebCore::Document::applyXSLTransform): Deleted.
(WebCore::Document::finishedParsing): Apply XSLT right before updating the style. This is where used to apply
inline XSLT and it happens much earlier than implicitClose.
* dom/Document.h:
* dom/ProcessingInstruction.cpp:
(WebCore::ProcessingInstruction::checkStyleSheet): Schedule XSLT in the document instead of flushing pending
stylesheets, which would have synchronously applied XSLT. We can't apply XSLT synchronously here because this
function can be called from a non-script-resilient call stack.
(WebCore::ProcessingInstruction::sheetLoaded): Ditto.
* style/StyleScope.cpp:
(WebCore::Style::Scope::collectXSLTransforms): Added.
(WebCore::Style::Scope::collectActiveStyleSheets): Removed the code to apply XSLT. Skip ProcessingInstructions
that applies XSLT. Also use RefPtr<StyleSheet> instead of a raw pointer to store StyleSheet.
* style/StyleScope.h:
* xml/parser/XMLDocumentParserLibxml2.cpp:
(WebCore::XMLDocumentParser::doEnd): Apply any pending XSLTs synchronously here as the comment suggests.

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

Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/dom/ProcessingInstruction.cpp
Source/WebCore/style/StyleScope.cpp
Source/WebCore/style/StyleScope.h
Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp

index 2f9942a..059cc1d 100644 (file)
@@ -1,3 +1,39 @@
+2017-10-25  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Style::Scope::flushPendingUpdate() can replace the entire document in XSLTProcessor::createDocumentFromSource
+        https://bugs.webkit.org/show_bug.cgi?id=178715
+        <rdar://problem/35144665>
+
+        Reviewed by Brent Fulgham.
+
+        Apply XLS tranforms when a 0s timer fires or the document finishes parsing or loading whichever comes first
+        instead of in the middle of collecting a list of stylesheets.
+
+        * dom/Document.cpp:
+        (WebCore::Document::Document): Initialize the newly added timer.
+        (WebCore::Document::implicitClose): Apply any pending XSLT before we fire load events since some of the event
+        handlers may be expecting to see the document after XSLT had been applied.
+        (WebCore::Document::scheduleToApplyXSLTransforms): Added.
+        (WebCore::Document::applyPendingXSLTransformsNowIfScheduled): Added.
+        (WebCore::Document::applyPendingXSLTransformsTimerFired): Added. Moved the logic to apply XSL transforms from
+        Style::Scope::collectActiveStyleSheets, and merged applyXSLTransform into this function.
+        (WebCore::Document::applyXSLTransform): Deleted.
+        (WebCore::Document::finishedParsing): Apply XSLT right before updating the style. This is where used to apply
+        inline XSLT and it happens much earlier than implicitClose.
+        * dom/Document.h:
+        * dom/ProcessingInstruction.cpp:
+        (WebCore::ProcessingInstruction::checkStyleSheet): Schedule XSLT in the document instead of flushing pending
+        stylesheets, which would have synchronously applied XSLT. We can't apply XSLT synchronously here because this
+        function can be called from a non-script-resilient call stack.
+        (WebCore::ProcessingInstruction::sheetLoaded): Ditto.
+        * style/StyleScope.cpp:
+        (WebCore::Style::Scope::collectXSLTransforms): Added.
+        (WebCore::Style::Scope::collectActiveStyleSheets): Removed the code to apply XSLT. Skip ProcessingInstructions
+        that applies XSLT. Also use RefPtr<StyleSheet> instead of a raw pointer to store StyleSheet.
+        * style/StyleScope.h:
+        * xml/parser/XMLDocumentParserLibxml2.cpp:
+        (WebCore::XMLDocumentParser::doEnd): Apply any pending XSLTs synchronously here as the comment suggests.
+
 2017-10-25  Devin Rousso  <webkit@devinrousso.com>
 
         Web Inspector: Canvas Tab: starting a second recording doesn't show red titlebar if the first recording was empty
index 25d4a5c..975c268 100644 (file)
@@ -469,6 +469,7 @@ Document::Document(Frame* frame, const URL& url, unsigned documentClasses, unsig
     , m_documentCreationTime(MonotonicTime::now())
     , m_scriptRunner(std::make_unique<ScriptRunner>(*this))
     , m_moduleLoader(std::make_unique<ScriptModuleLoader>(*this))
+    , m_applyPendingXSLTransformsTimer(*this, &Document::applyPendingXSLTransformsTimerFired)
     , m_xmlVersion(ASCIILiteral("1.0"))
     , m_constantPropertyMap(std::make_unique<ConstantPropertyMap>(*this))
     , m_documentClasses(documentClasses)
@@ -2725,6 +2726,9 @@ void Document::implicitClose()
     // ramifications, and we need to decide what is the Right Thing To Do(tm)
     RefPtr<Frame> f = frame();
     if (f) {
+        // Apply XSL transforms before load events so that event handlers can access the transformed DOM tree.
+        applyPendingXSLTransformsNowIfScheduled();
+
         if (auto* documentLoader = loader())
             documentLoader->startIconLoading();
 
@@ -5050,18 +5054,43 @@ void Document::popCurrentScript()
 
 #if ENABLE(XSLT)
 
-void Document::applyXSLTransform(ProcessingInstruction* pi)
+void Document::scheduleToApplyXSLTransforms()
 {
-    RefPtr<XSLTProcessor> processor = XSLTProcessor::create();
-    processor->setXSLStyleSheet(downcast<XSLStyleSheet>(pi->sheet()));
-    String resultMIMEType;
-    String newSource;
-    String resultEncoding;
-    if (!processor->transformToString(*this, resultMIMEType, newSource, resultEncoding))
+    if (!m_applyPendingXSLTransformsTimer.isActive())
+        m_applyPendingXSLTransformsTimer.startOneShot(0_s);
+}
+
+void Document::applyPendingXSLTransformsNowIfScheduled()
+{
+    if (!m_applyPendingXSLTransformsTimer.isActive())
         return;
-    // FIXME: If the transform failed we should probably report an error (like Mozilla does).
-    Frame* ownerFrame = frame();
-    processor->createDocumentFromSource(newSource, resultEncoding, resultMIMEType, this, ownerFrame);
+    m_applyPendingXSLTransformsTimer.stop();
+    applyPendingXSLTransformsTimerFired();
+}
+
+void Document::applyPendingXSLTransformsTimerFired()
+{
+    if (parsing())
+        return;
+
+    ASSERT(NoEventDispatchAssertion::isEventAllowedInMainThread());
+    for (auto& processingInstruction : styleScope().collectXSLTransforms()) {
+        ASSERT(processingInstruction->isXSL());
+
+        // Don't apply XSL transforms to already transformed documents -- <rdar://problem/4132806>
+        if (transformSourceDocument() || !processingInstruction->sheet())
+            return;
+
+        auto processor = XSLTProcessor::create();
+        processor->setXSLStyleSheet(downcast<XSLStyleSheet>(processingInstruction->sheet()));
+        String resultMIMEType;
+        String newSource;
+        String resultEncoding;
+        if (!processor->transformToString(*this, resultMIMEType, newSource, resultEncoding))
+            continue;
+        // FIXME: If the transform failed we should probably report an error (like Mozilla does).
+        processor->createDocumentFromSource(newSource, resultEncoding, resultMIMEType, this, frame());
+    }
 }
 
 void Document::setTransformSource(std::unique_ptr<TransformSource> source)
@@ -5243,6 +5272,8 @@ void Document::finishedParsing()
     ASSERT(!scriptableDocumentParser() || m_readyState != Loading);
     setParsing(false);
 
+    Ref<Document> protectedThis(*this);
+
     if (!m_documentTiming.domContentLoadedEventStart)
         m_documentTiming.domContentLoadedEventStart = MonotonicTime::now();
 
@@ -5252,6 +5283,8 @@ void Document::finishedParsing()
         m_documentTiming.domContentLoadedEventEnd = MonotonicTime::now();
 
     if (RefPtr<Frame> frame = this->frame()) {
+        applyPendingXSLTransformsNowIfScheduled();
+
         // FrameLoader::finishedParsing() might end up calling Document::implicitClose() if all
         // resource loads are complete. HTMLObjectElements can start loading their resources from
         // post attach callbacks triggered by resolveStyle(). This means if we parse out an <object>
index 16717e8..4882cb5 100644 (file)
@@ -944,7 +944,8 @@ public:
     void popCurrentScript();
 
 #if ENABLE(XSLT)
-    void applyXSLTransform(ProcessingInstruction* pi);
+    void scheduleToApplyXSLTransforms();
+    void applyPendingXSLTransformsNowIfScheduled();
     RefPtr<Document> transformSourceDocument() { return m_transformSourceDocument; }
     void setTransformSourceDocument(Document* doc) { m_transformSourceDocument = doc; }
 
@@ -1558,8 +1559,11 @@ private:
     Vector<RefPtr<HTMLScriptElement>> m_currentScriptStack;
 
 #if ENABLE(XSLT)
+    void applyPendingXSLTransformsTimerFired();
+
     std::unique_ptr<TransformSource> m_transformSource;
     RefPtr<Document> m_transformSourceDocument;
+    Timer m_applyPendingXSLTransformsTimer;
 #endif
 
     String m_xmlEncoding;
index 758dcc2..2977e0d 100644 (file)
@@ -126,6 +126,7 @@ void ProcessingInstruction::checkStyleSheet()
                 URL finalURL(ParsedURLString, m_localHref);
                 m_sheet = XSLStyleSheet::createEmbedded(this, finalURL);
                 m_loading = false;
+                document().scheduleToApplyXSLTransforms();
             }
 #endif
         } else {
@@ -179,7 +180,7 @@ void ProcessingInstruction::checkStyleSheet()
                 document().styleScope().removePendingSheet(*this);
 #if ENABLE(XSLT)
                 if (m_isXSL)
-                    document().styleScope().flushPendingUpdate();
+                    document().scheduleToApplyXSLTransforms();
 #endif
             }
         }
@@ -202,7 +203,7 @@ bool ProcessingInstruction::sheetLoaded()
             document().styleScope().removePendingSheet(*this);
 #if ENABLE(XSLT)
         if (m_isXSL)
-            document().styleScope().flushPendingUpdate();
+            document().scheduleToApplyXSLTransforms();
 #endif
         return true;
     }
index 5016eeb..beddcf6 100644 (file)
@@ -290,27 +290,31 @@ void Scope::removeStyleSheetCandidateNode(Node& node)
         didChangeActiveStyleSheetCandidates();
 }
 
+#if ENABLE(XSLT)
+// FIXME: <https://webkit.org/b/178830> Remove XSLT relaed code from Style::Scope.
+Vector<Ref<ProcessingInstruction>> Scope::collectXSLTransforms()
+{
+    Vector<Ref<ProcessingInstruction>> processingInstructions;
+    for (auto& node : m_styleSheetCandidateNodes) {
+        if (is<ProcessingInstruction>(*node) && downcast<ProcessingInstruction>(*node).isXSL())
+            processingInstructions.append(downcast<ProcessingInstruction>(*node));
+    }
+    return processingInstructions;
+}
+#endif
+
 void Scope::collectActiveStyleSheets(Vector<RefPtr<StyleSheet>>& sheets)
 {
     if (!m_document.settings().authorAndUserStylesEnabled())
         return;
 
     for (auto& node : m_styleSheetCandidateNodes) {
-        StyleSheet* sheet = nullptr;
+        RefPtr<StyleSheet> sheet;
         if (is<ProcessingInstruction>(*node)) {
-            // Processing instruction (XML documents only).
+            if (!downcast<ProcessingInstruction>(*node).isCSS())
+                continue;
             // We don't support linking to embedded CSS stylesheets, see <https://bugs.webkit.org/show_bug.cgi?id=49281> for discussion.
-            ProcessingInstruction& pi = downcast<ProcessingInstruction>(*node);
-            sheet = pi.sheet();
-#if ENABLE(XSLT)
-            // Don't apply XSL transforms to already transformed documents -- <rdar://problem/4132806>
-            if (pi.isXSL() && !m_document.transformSourceDocument()) {
-                // Don't apply XSL transforms until loading is finished.
-                if (!m_document.parsing())
-                    m_document.applyXSLTransform(&pi);
-                return;
-            }
-#endif
+            sheet = downcast<ProcessingInstruction>(*node).sheet();
         } else if (is<HTMLLinkElement>(*node) || is<HTMLStyleElement>(*node) || is<SVGStyleElement>(*node)) {
             Element& element = downcast<Element>(*node);
             AtomicString title = element.attributeWithoutSynchronization(titleAttr);
@@ -364,7 +368,7 @@ void Scope::collectActiveStyleSheets(Vector<RefPtr<StyleSheet>>& sheets)
                 sheet = nullptr;
         }
         if (sheet)
-            sheets.append(sheet);
+            sheets.append(WTFMove(sheet));
     }
 }
 
index a408d8c..0ebfd1b 100644 (file)
@@ -108,6 +108,10 @@ public:
     bool hasPendingUpdate() const { return m_pendingUpdate || m_hasDescendantWithPendingUpdate; }
     void flushPendingUpdate();
 
+#if ENABLE(XSLT)
+    Vector<Ref<ProcessingInstruction>> collectXSLTransforms();
+#endif
+
     StyleResolver& resolver();
     StyleResolver* resolverIfExists();
     void clearResolver();
index 925d4c1..738872e 100644 (file)
@@ -1333,7 +1333,7 @@ void XMLDocumentParser::doEnd()
         document()->setTransformSource(std::make_unique<TransformSource>(doc));
 
         document()->setParsing(false); // Make the document think it's done, so it will apply XSL stylesheets.
-        document()->styleScope().didChangeActiveStyleSheetCandidates();
+        document()->applyPendingXSLTransformsNowIfScheduled();
 
         // styleResolverChanged() call can detach the parser and null out its document.
         // In that case, we just bail out.