Perform a microtask checkpoint before creating a custom element

https://bugs.webkit.org/show_bug.cgi?id=188189 <rdar://problem/42843022> Reviewed by Geoffrey Garen. Source/WebCore: Fixed the bug that the HTML parser was not performing a microtask checkpoint prior to synchronously constructing a custom element in the concept to create an element for a token: https://html.spec.whatwg.org/multipage/parsing.html#creating-and-inserting-nodes:perform-a-microtask-checkpoint Also added a microtask checkpoint before dispatching DOMContentLoaded to work around webkit.org/b/82931 since scheduling a task to fire a DOMContentLoaded event in Document::finishedParsing as the HTML5 spec mandates is a long standing bug with a lot of implications, which is completely outside the scope of this bug fix: https://html.spec.whatwg.org/multipage/parsing.html#stop-parsing Test: fast/custom-elements/perform-microtask-checkpoint-before-construction.html * dom/Document.cpp: (WebCore::Document::finishedParsing): Perform a microtask checkpoint before dispatching DOMContentLoaded here as a workaround for webkit.org/b/82931. * html/parser/HTMLDocumentParser.cpp: (WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder): Perform a microtask checkpoint here to fix the bug. LayoutTests: Added a W3C style testharness.js test for perfoming microtask checkpoint before constructing a custom element synchronously. * fast/custom-elements/perform-microtask-checkpoint-before-construction-expected.txt: Added. * fast/custom-elements/perform-microtask-checkpoint-before-construction.html: Added. * fast/dom/MutationObserver/parser-mutations.html: Fixed the test per new behavior in Document::finishParsing. Because iframe loads synchronously and fires DOMContentLoaded, mutation records are now delivered twice after iframe element is encountered in this test and before script element executes. Concatenate the mutation records arrays to account for this behavioral change. New WebKit behavior matches that of Chrome; namely this test fails both on Chrome Canary 70 and trunk WebKit with this patch without this fix. Canonical link: https://commits.webkit.org/203724@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@234944 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Aug 16, 2018 · ca269f9d8d64a1a9f10fd723531c354659a6166e · ca269f9
1 parent 00a9ef0
commit ca269f9d8d64a1a9f10fd723531c354659a6166e
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,22 @@
+2018-08-16  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Perform a microtask checkpoint before creating a custom element
+        https://bugs.webkit.org/show_bug.cgi?id=188189
+        <rdar://problem/42843022>
+
+        Reviewed by Geoffrey Garen.
+
+        Added a W3C style testharness.js test for perfoming microtask checkpoint before constructing
+        a custom element synchronously.
+
+        * fast/custom-elements/perform-microtask-checkpoint-before-construction-expected.txt: Added.
+        * fast/custom-elements/perform-microtask-checkpoint-before-construction.html: Added.
+        * fast/dom/MutationObserver/parser-mutations.html: Fixed the test per new behavior in Document::finishParsing.
+        Because iframe loads synchronously and fires DOMContentLoaded, mutation records are now delivered twice after
+        iframe element is encountered in this test and before script element executes. Concatenate the mutation records
+        arrays to account for this behavioral change. New WebKit behavior matches that of Chrome; namely this test
+        fails both on Chrome Canary 70 and trunk WebKit with this patch without this fix.
+
 2018-08-15  Jer Noble  <jer.noble@apple.com>
 
         Add Experimental Feature support for SourceBuffer.changeType()

diff --git a/...tTests/fast/custom-elements/perform-microtask-checkpoint-before-construction-expected.txt b/...tTests/fast/custom-elements/perform-microtask-checkpoint-before-construction-expected.txt
@@ -0,0 +1,4 @@
+
+PASS HTML parser must perform a microtask checkpoint before constructing a custom element 
+PASS HTML parser must perform a microtask checkpoint before constructing a custom element during the adoption agency algorithm 
+
diff --git a/LayoutTests/fast/custom-elements/perform-microtask-checkpoint-before-construction.html b/LayoutTests/fast/custom-elements/perform-microtask-checkpoint-before-construction.html
@@ -0,0 +1,143 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>Custom Elements: create an element for a token must perform a microtask checkpoint</title>
+<meta name="author" title="Ryosuke Niwa" href="mailto:rniwa@webkit.org">
+<meta name="assert" content="When the HTML parser creates an element for a token, it must perform a microtask checkpoint before invoking the constructor">
+<meta name="help" content="https://html.spec.whatwg.org/multipage/parsing.html#create-an-element-for-the-token">
+<meta name="help" content="https://html.spec.whatwg.org/multipage/webappapis.html#perform-a-microtask-checkpoint">
+<meta name="help" content="https://html.spec.whatwg.org/multipage/parsing.html#adoption-agency-algorithm">
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+<script src="../../imported/w3c/web-platform-tests/custom-elements/resources/custom-elements-helpers.js"></script>
+</head>
+<body>
+<div id="log"></div>
+<script>
+
+async function construct_custom_element_in_parser(test, markup)
+{
+    const window = await create_window_in_test(test, `
+<!DOCTYPE html>
+<html>
+<body><script>
+class SomeElement extends HTMLElement {
+    constructor() {
+        super();
+        window.recordsListInConstructor = recordsList.map((records) => records.slice(0));
+    }
+}
+customElements.define('some-element', SomeElement);
+
+const recordsList = [];
+const observer = new MutationObserver((records) => {
+    recordsList.push(records);
+});
+observer.observe(document.body, {childList: true, subtree: true});
+
+window.onload = () => {
+    window.recordsListInDOMContentLoaded = recordsList.map((records) => records.slice(0));
+}
+
+</scr` + `ipt>${markup}</body></html>`);
+    return window;
+}
+
+promise_test(async function () {
+    const contentWindow = await construct_custom_element_in_parser(this, '<b><some-element></b>');
+    const contentDocument = contentWindow.document;
+
+    let recordsList = contentWindow.recordsListInConstructor;
+    assert_true(Array.isArray(recordsList));
+    assert_equals(recordsList.length, 1);
+    assert_true(Array.isArray(recordsList[0]));
+    assert_equals(recordsList[0].length, 1);
+    let record = recordsList[0][0];
+    assert_equals(record.type, 'childList');
+    assert_equals(record.target, contentDocument.body);
+    assert_equals(record.previousSibling, contentDocument.querySelector('script'));
+    assert_equals(record.nextSibling, null);
+    assert_equals(record.removedNodes.length, 0);
+    assert_equals(record.addedNodes.length, 1);
+    assert_equals(record.addedNodes[0], contentDocument.querySelector('b'));
+
+    recordsList = contentWindow.recordsListInDOMContentLoaded;
+    assert_true(Array.isArray(recordsList));
+    assert_equals(recordsList.length, 2);
+    assert_true(Array.isArray(recordsList[1]));
+    assert_equals(recordsList[1].length, 1);
+    record = recordsList[1][0];
+    assert_equals(record.type, 'childList');
+    assert_equals(record.target, contentDocument.querySelector('b'));
+    assert_equals(record.previousSibling, null);
+    assert_equals(record.nextSibling, null);
+    assert_equals(record.removedNodes.length, 0);
+    assert_equals(record.addedNodes.length, 1);
+    assert_equals(record.addedNodes[0], contentDocument.querySelector('some-element'));
+}, 'HTML parser must perform a microtask checkpoint before constructing a custom element');
+
+promise_test(async function () {
+    const contentWindow = await construct_custom_element_in_parser(this, '<b><i>hello</b><some-element>');
+    const contentDocument = contentWindow.document;
+    let recordsList = contentWindow.recordsListInConstructor;
+    assert_true(Array.isArray(recordsList));
+    assert_equals(recordsList.length, 1);
+    assert_true(Array.isArray(recordsList[0]));
+    assert_equals(recordsList[0].length, 4);
+
+    let record = recordsList[0][0];
+    assert_equals(record.type, 'childList');
+    assert_equals(record.target, contentDocument.body);
+    assert_equals(record.previousSibling, contentDocument.querySelector('script'));
+    assert_equals(record.nextSibling, null);
+    assert_equals(record.removedNodes.length, 0);
+    assert_equals(record.addedNodes.length, 1);
+    assert_equals(record.addedNodes[0], contentDocument.querySelector('b'));
+
+    record = recordsList[0][1];
+    assert_equals(record.type, 'childList');
+    assert_equals(record.target, contentDocument.querySelector('b'));
+    assert_equals(record.previousSibling, null);
+    assert_equals(record.nextSibling, null);
+    assert_equals(record.removedNodes.length, 0);
+    assert_equals(record.addedNodes.length, 1);
+    assert_equals(record.addedNodes[0], contentDocument.querySelector('i'));
+
+    record = recordsList[0][2];
+    assert_equals(record.type, 'childList');
+    assert_equals(record.target, contentDocument.querySelector('i'));
+    assert_equals(record.previousSibling, null);
+    assert_equals(record.nextSibling, null);
+    assert_equals(record.removedNodes.length, 0);
+    assert_equals(record.addedNodes.length, 1);
+    assert_equals(record.addedNodes[0].nodeType, Node.TEXT_NODE);
+    assert_equals(record.addedNodes[0].data, "hello");
+
+    record = recordsList[0][3];
+    assert_equals(record.type, 'childList');
+    assert_equals(record.target, contentDocument.body);
+    assert_equals(record.previousSibling, contentDocument.querySelector('b'));
+    assert_equals(record.nextSibling, null);
+    assert_equals(record.removedNodes.length, 0);
+    assert_equals(record.addedNodes.length, 1);
+    assert_equals(record.addedNodes[0], contentDocument.querySelectorAll('i')[1]);
+
+    recordsList = contentWindow.recordsListInDOMContentLoaded;
+    assert_true(Array.isArray(recordsList));
+    assert_equals(recordsList.length, 2);
+    assert_true(Array.isArray(recordsList[1]));
+    assert_equals(recordsList[1].length, 1);
+
+    record = recordsList[1][0];
+    assert_equals(record.type, 'childList');
+    assert_equals(record.target, contentDocument.querySelectorAll('i')[1]);
+    assert_equals(record.previousSibling, null);
+    assert_equals(record.nextSibling, null);
+    assert_equals(record.removedNodes.length, 0);
+    assert_equals(record.addedNodes.length, 1);
+    assert_equals(record.addedNodes[0], contentDocument.querySelector('some-element'));
+}, 'HTML parser must perform a microtask checkpoint before constructing a custom element during the adoption agency algorithm');
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/dom/MutationObserver/parser-mutations.html b/LayoutTests/fast/dom/MutationObserver/parser-mutations.html
@@ -7,8 +7,9 @@
     if (window.testRunner)
         testRunner.dumpAsText();
 
+    var mutations = [];
     var observer = new MutationObserver(function(mutations, observer) {
-        window.mutations = mutations;
+        window.mutations = window.mutations.concat(mutations);
     });
     observer.observe(document.body, {childList: true, subtree:true});
 </script>

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,28 @@
+2018-08-16  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Perform a microtask checkpoint before creating a custom element
+        https://bugs.webkit.org/show_bug.cgi?id=188189
+        <rdar://problem/42843022>
+
+        Reviewed by Geoffrey Garen.
+
+        Fixed the bug that the HTML parser was not performing a microtask checkpoint prior to synchronously constructing
+        a custom element in the concept to create an element for a token:
+        https://html.spec.whatwg.org/multipage/parsing.html#creating-and-inserting-nodes:perform-a-microtask-checkpoint
+
+        Also added a microtask checkpoint before dispatching DOMContentLoaded to work around webkit.org/b/82931 since
+        scheduling a task to fire a DOMContentLoaded event in Document::finishedParsing as the HTML5 spec mandates
+        is a long standing bug with a lot of implications, which is completely outside the scope of this bug fix:
+        https://html.spec.whatwg.org/multipage/parsing.html#stop-parsing
+
+        Test: fast/custom-elements/perform-microtask-checkpoint-before-construction.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::finishedParsing): Perform a microtask checkpoint before dispatching DOMContentLoaded here as
+        a workaround for webkit.org/b/82931.
+        * html/parser/HTMLDocumentParser.cpp:
+        (WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder): Perform a microtask checkpoint here to fix the bug.
+
 2018-08-16  Alex Christensen  <achristensen@webkit.org>
 
         Re-introduce assertion removed in r234890

diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
@@ -5427,6 +5427,9 @@ void Document::finishedParsing()
     if (!m_documentTiming.domContentLoadedEventStart)
         m_documentTiming.domContentLoadedEventStart = MonotonicTime::now();
 
+    // FIXME: Schdule a task to fire DOMContentLoaded event instead. See webkit.org/b/82931
+    MicrotaskQueue::mainThreadQueue().performMicrotaskCheckpoint();
+
     dispatchEvent(Event::create(eventNames().DOMContentLoadedEvent, true, false));
 
     if (!m_documentTiming.domContentLoadedEventEnd)

diff --git a/Source/WebCore/html/parser/HTMLDocumentParser.cpp b/Source/WebCore/html/parser/HTMLDocumentParser.cpp
@@ -39,6 +39,7 @@
 #include "HTMLUnknownElement.h"
 #include "JSCustomElementInterface.h"
 #include "LinkLoader.h"
+#include "Microtasks.h"
 #include "NavigationScheduler.h"
 #include "ScriptElement.h"
 #include "ThrowOnDynamicMarkupInsertionCountIncrementer.h"
@@ -213,6 +214,9 @@ void HTMLDocumentParser::runScriptsForPausedTreeBuilder()
         {
             // Prevent document.open/write during reactions by allocating the incrementer before the reactions stack.
             ThrowOnDynamicMarkupInsertionCountIncrementer incrementer(*document());
+
+            MicrotaskQueue::mainThreadQueue().performMicrotaskCheckpoint();
+
             CustomElementReactionStack reactionStack(document()->execState());
             auto& elementInterface = constructionData->elementInterface.get();
             auto newElement = elementInterface.constructElementWithFallback(*document(), constructionData->name);