Perform a microtask checkpoint before creating a custom element
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Aug 2018 18:25:02 +0000 (18:25 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Aug 2018 18:25:02 +0000 (18:25 +0000)
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.

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

LayoutTests/ChangeLog
LayoutTests/fast/custom-elements/perform-microtask-checkpoint-before-construction-expected.txt [new file with mode: 0644]
LayoutTests/fast/custom-elements/perform-microtask-checkpoint-before-construction.html [new file with mode: 0644]
LayoutTests/fast/dom/MutationObserver/parser-mutations.html
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/html/parser/HTMLDocumentParser.cpp

index 106a934..444ebfd 100644 (file)
@@ -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/LayoutTests/fast/custom-elements/perform-microtask-checkpoint-before-construction-expected.txt b/LayoutTests/fast/custom-elements/perform-microtask-checkpoint-before-construction-expected.txt
new file mode 100644 (file)
index 0000000..9656bf2
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..f117c88
--- /dev/null
@@ -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>
index edd9093..bb141de 100644 (file)
@@ -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>
index 0d651ba..3291bcd 100644 (file)
@@ -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
index df39f4d..ee74da2 100644 (file)
@@ -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)
index ad2ab6c..43abb5e 100644 (file)
@@ -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);