Attachment 346070 Details for Bug 187802 – Patch

[patch] Patch

bug-187802-20180730200102.patch (text/plain), 9.87 KB, created by Frédéric Wang (:fredw) on 2018-07-30 11:01:03 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Frédéric Wang (:fredw)

Created: 2018-07-30 11:01:03 PDT

Size: 9.87 KB

patch

obsolete

>Subversion Revision: 234362
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index b60e0512ab197ebe2d030e98f76863b9981f514b..fe7fe7fd1eef6b0d68eec3c3adbf7a7f96dd9612 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,35 @@
>+2018-07-30  Frederic Wang  <fwang@igalia.com>
>+
>+        ASSERTION !data.customElementReactionQueue() when creating custom element inside an SVG document
>+        https://bugs.webkit.org/show_bug.cgi?id=187802
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Per [1], the XML parser should follow algorithm equivalent to [2] when creating DOM nodes.
>+        This patch adds missing steps in XMLDocumentParser::startElementNs to enqueue/invoke/dequeue
>+        reaction queue when creating the element. It also ensures that the synchronous custom element
>+        flag is properly set when creating the element [3]. This fixes an ASSERTION failure in
>+        when the element is later inserted into the DOM tree. It also makes the autonomous case of
>+        custom-elements/parser/parser-uses-create-an-element-for-a-token-svg.svg pass but
>+        unfortunately this test still times out because of the infrastructure bug 187800. Hence
>+        a separate non-regression test is added instead. Note that the HTML parser does not follow
>+        steps from [2] [3], this will be handled in follow-up commits (e.g. bug 183586).
>+
>+        [1] https://html.spec.whatwg.org/#parsing-xhtml-documents:create-an-element-for-the-token
>+        [2] https://html.spec.whatwg.org/#create-an-element-for-the-token
>+        [3] https://dom.spec.whatwg.org/#concept-create-element
>+
>+        Test: fast/custom-elements/xml-parsing-create-and-element-for-a-token.svg
>+
>+        * dom/Document.cpp:
>+        (WebCore::Document::createElement): Do not create custom element
>+        synchronously when synchronousCustomElements is false as per [3].
>+        * dom/Document.h: Add synchronousCustomElements flag.
>+        * xml/parser/XMLDocumentParserLibxml2.cpp:
>+        (WebCore::XMLDocumentParser::startElementNs): Calculate willExecuteScript and enqueue,
>+        dequeue, invoke element reactions and set synchronous custom elements flag accordingly. Also
>+        add some comments to make steps clearer against the specification [2].
>+
> 2018-07-30  Thibault Saunier  <tsaunier@igalia.com>
> 
>         [GStreamer] Make codecparsers optionnal
>diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
>index d0ebe6ff7f984e723772fe14f3c5144084fbe46b..8a6d8fd4a6dd8e9b37031f82b4814ddded3ac1a3 100644
>--- a/Source/WebCore/dom/Document.cpp
>+++ b/Source/WebCore/dom/Document.cpp
>@@ -1060,15 +1060,19 @@ static Ref<HTMLElement> createFallbackHTMLElement(Document& document, const Qual
> }
> 
> // FIXME: This should really be in a possible ElementFactory class.
>-Ref<Element> Document::createElement(const QualifiedName& name, bool createdByParser)
>+Ref<Element> Document::createElement(const QualifiedName& name, bool createdByParser, bool synchronousCustomElements)
> {
>     RefPtr<Element> element;
> 
>     // FIXME: Use registered namespaces and look up in a hash to find the right factory.
>     if (name.namespaceURI() == xhtmlNamespaceURI) {
>         element = HTMLElementFactory::createKnownElement(name, *this, nullptr, createdByParser);
>-        if (UNLIKELY(!element))
>-            element = createFallbackHTMLElement(*this, name);
>+        if (UNLIKELY(!element)) {
>+            if (synchronousCustomElements)
>+                element = createFallbackHTMLElement(*this, name);
>+            else
>+                element = createUpgradeCandidateElement(*this, name);
>+        }
>     } else if (name.namespaceURI() == SVGNames::svgNamespaceURI)
>         element = SVGElementFactory::createElement(name, *this, createdByParser);
> #if ENABLE(MATHML)
>diff --git a/Source/WebCore/dom/Document.h b/Source/WebCore/dom/Document.h
>index a8d37226bf2c7f5a0169fa414106ae66a0a93c5d..45e820a0251d3f77580836690ce5dc5abc8d106b 100644
>--- a/Source/WebCore/dom/Document.h
>+++ b/Source/WebCore/dom/Document.h
>@@ -437,7 +437,7 @@ public:
>     WEBCORE_EXPORT ExceptionOr<Ref<Attr>> createAttributeNS(const AtomicString& namespaceURI, const String& qualifiedName, bool shouldIgnoreNamespaceChecks = false);
>     WEBCORE_EXPORT ExceptionOr<Ref<Node>> importNode(Node& nodeToImport, bool deep);
>     WEBCORE_EXPORT ExceptionOr<Ref<Element>> createElementNS(const AtomicString& namespaceURI, const String& qualifiedName);
>-    WEBCORE_EXPORT Ref<Element> createElement(const QualifiedName&, bool createdByParser);
>+    WEBCORE_EXPORT Ref<Element> createElement(const QualifiedName&, bool createdByParser, bool synchronousCustomElements = true);
> 
>     static CustomElementNameValidationStatus validateCustomElementName(const AtomicString&);
> 
>diff --git a/Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp b/Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp
>index 80973e98550029636e81b3597f021f8824fedeb2..621f62c42cdb62a86524bfefbbe7ed964986a01d 100644
>--- a/Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp
>+++ b/Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp
>@@ -31,6 +31,9 @@
> #include "CDATASection.h"
> #include "Comment.h"
> #include "CachedResourceLoader.h"
>+#include "CustomElementReactionQueue.h"
>+#include "CustomElementRegistry.h"
>+#include "DOMWindow.h"
> #include "Document.h"
> #include "DocumentFragment.h"
> #include "DocumentType.h"
>@@ -781,7 +784,24 @@ void XMLDocumentParser::startElementNs(const xmlChar* xmlLocalName, const xmlCha
>     m_sawFirstElement = true;
> 
>     QualifiedName qName(prefix, localName, uri);
>-    auto newElement = m_currentNode->document().createElement(qName, true);
>+    // Code below follows steps to create an element for a token.
>+    // See https://html.spec.whatwg.org/#parsing-xhtml-documents:create-an-element-for-the-token
>+    // Step 5.
>+    bool willExecuteScript = false;
>+    if (!m_parsingFragment) {
>+        if (auto* window = m_currentNode->document().domWindow()) {
>+            auto* registry = window->customElementRegistry();
>+            if (UNLIKELY(registry))
>+                willExecuteScript = registry->findInterface(qName);
>+        }
>+    }
>+
>+    std::optional<CustomElementReactionStack> customElementReactionStack;
>+    if (willExecuteScript)
>+        customElementReactionStack.emplace(); // Step 6.
>+
>+    // Step 7 and 8.
>+    auto newElement = m_currentNode->document().createElement(qName, true, willExecuteScript);
> 
>     Vector<Attribute> prefixedAttributes;
>     if (!handleNamespaceAttributes(prefixedAttributes, libxmlNamespaces, numNamespaces)) {
>@@ -797,6 +817,8 @@ void XMLDocumentParser::startElementNs(const xmlChar* xmlLocalName, const xmlCha
>         return;
>     }
> 
>+    customElementReactionStack.reset(); // Step 9.
>+
>     newElement->beginParsingChildren();
> 
>     if (isScriptElement(newElement.get()))
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index f571f0f24b70c197aba31cf0e70537e0e92b38bb..8a18db8c43c625aa56c389491dc0c75077560cc5 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-07-30  Frederic Wang  <fwang@igalia.com>
>+
>+        ASSERTION !data.customElementReactionQueue() when creating custom element inside an SVG document
>+        https://bugs.webkit.org/show_bug.cgi?id=187802
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * TestExpectations: This test no longer crashes but times out because of bug 187800.
>+        * fast/custom-elements/xml-parsing-create-and-element-for-a-token-expected.txt: Added.
>+        * fast/custom-elements/xml-parsing-create-and-element-for-a-token.svg: Added.
>+
> 2018-07-30  Zan Dobersek  <zdobersek@igalia.com>
> 
>         Unreviewed WPE gardening. Adding and adjusting a bunch of expectations
>diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
>index 2c7f6dc1be9e844eb6ce226f01e98c6e6dcd75ba..95657659aa4ff3a3cf5058b92b8def3901e45a05 100644
>--- a/LayoutTests/TestExpectations
>+++ b/LayoutTests/TestExpectations
>@@ -570,7 +570,7 @@ imported/w3c/web-platform-tests/html/semantics/document-metadata/the-meta-elemen
> 
> # WPT tests for custom elements
> webkit.org/b/187800 imported/w3c/web-platform-tests/custom-elements/Document-createElement-svg.svg [ Skip ]
>-webkit.org/b/187802 imported/w3c/web-platform-tests/custom-elements/parser/parser-uses-create-an-element-for-a-token-svg.svg [ Skip ]
>+webkit.org/b/187800 imported/w3c/web-platform-tests/custom-elements/parser/parser-uses-create-an-element-for-a-token-svg.svg [ Skip ]
> webkit.org/b/187805 imported/w3c/web-platform-tests/custom-elements/reactions/with-exceptions.html [ Skip ]
> 
> # selectors
>diff --git a/LayoutTests/fast/custom-elements/xml-parsing-create-and-element-for-a-token-expected.txt b/LayoutTests/fast/custom-elements/xml-parsing-create-and-element-for-a-token-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..654ddf7f17efab58e42fabe385257964ae2c5d41
>--- /dev/null
>+++ b/LayoutTests/fast/custom-elements/xml-parsing-create-and-element-for-a-token-expected.txt
>@@ -0,0 +1 @@
>+This test passes if it does not crash.
>diff --git a/LayoutTests/fast/custom-elements/xml-parsing-create-and-element-for-a-token.svg b/LayoutTests/fast/custom-elements/xml-parsing-create-and-element-for-a-token.svg
>new file mode 100644
>index 0000000000000000000000000000000000000000..3b16db0cfbbd84d9830775f0c036521dfcf365f1
>--- /dev/null
>+++ b/LayoutTests/fast/custom-elements/xml-parsing-create-and-element-for-a-token.svg
>@@ -0,0 +1,16 @@
>+<?xml version="1.0" encoding="utf-8"?>
>+<svg:svg xmlns:svg="http://www.w3.org/2000/svg"
>+         xmlns="http://www.w3.org/1999/xhtml">
>+<svg:text>This test passes if it does not crash.</svg:text>
>+<script><![CDATA[
>+if (testRunner)
>+    testRunner.dumpAsText();
>+class MyElement extends HTMLElement {}
>+customElements.define('my-element', MyElement);
>+]]></script>
>+<my-element></my-element>
>+<element-without-definition></element-without-definition>
>+<script><![CDATA[
>+document.documentElement.insertAdjacentHTML("beforeend", "<my-element></my-element>");
>+]]></script>
>+</svg:svg>

Actions: View | Formatted Diff | Diff

Attachments on bug 187802: 345347 | 345567 | 346067 | 346070 | 346613