Log Message
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.
Modified Paths
- trunk/LayoutTests/ChangeLog
- trunk/LayoutTests/fast/dom/MutationObserver/parser-mutations.html
- trunk/Source/WebCore/ChangeLog
- trunk/Source/WebCore/dom/Document.cpp
- trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (234943 => 234944)
--- trunk/LayoutTests/ChangeLog 2018-08-16 18:21:52 UTC (rev 234943)
+++ trunk/LayoutTests/ChangeLog 2018-08-16 18:25:02 UTC (rev 234944)
@@ -1,3 +1,22 @@
+2018-08-16 Ryosuke Niwa <[email protected]>
+
+ 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 <[email protected]>
Add Experimental Feature support for SourceBuffer.changeType()
Added: trunk/LayoutTests/fast/custom-elements/perform-microtask-checkpoint-before-construction-expected.txt (0 => 234944)
--- trunk/LayoutTests/fast/custom-elements/perform-microtask-checkpoint-before-construction-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/custom-elements/perform-microtask-checkpoint-before-construction-expected.txt 2018-08-16 18:25:02 UTC (rev 234944)
@@ -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
+
Added: trunk/LayoutTests/fast/custom-elements/perform-microtask-checkpoint-before-construction.html (0 => 234944)
--- trunk/LayoutTests/fast/custom-elements/perform-microtask-checkpoint-before-construction.html (rev 0)
+++ trunk/LayoutTests/fast/custom-elements/perform-microtask-checkpoint-before-construction.html 2018-08-16 18:25:02 UTC (rev 234944)
@@ -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=""
+<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=""
+<script src=""
+<script src=""
+</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>
Modified: trunk/LayoutTests/fast/dom/MutationObserver/parser-mutations.html (234943 => 234944)
--- trunk/LayoutTests/fast/dom/MutationObserver/parser-mutations.html 2018-08-16 18:21:52 UTC (rev 234943)
+++ trunk/LayoutTests/fast/dom/MutationObserver/parser-mutations.html 2018-08-16 18:25:02 UTC (rev 234944)
@@ -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>
Modified: trunk/Source/WebCore/ChangeLog (234943 => 234944)
--- trunk/Source/WebCore/ChangeLog 2018-08-16 18:21:52 UTC (rev 234943)
+++ trunk/Source/WebCore/ChangeLog 2018-08-16 18:25:02 UTC (rev 234944)
@@ -1,3 +1,28 @@
+2018-08-16 Ryosuke Niwa <[email protected]>
+
+ 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 <[email protected]>
Re-introduce assertion removed in r234890
Modified: trunk/Source/WebCore/dom/Document.cpp (234943 => 234944)
--- trunk/Source/WebCore/dom/Document.cpp 2018-08-16 18:21:52 UTC (rev 234943)
+++ trunk/Source/WebCore/dom/Document.cpp 2018-08-16 18:25:02 UTC (rev 234944)
@@ -5427,6 +5427,9 @@
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)
Modified: trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp (234943 => 234944)
--- trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp 2018-08-16 18:21:52 UTC (rev 234943)
+++ trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp 2018-08-16 18:25:02 UTC (rev 234944)
@@ -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 @@
{
// 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);
_______________________________________________ webkit-changes mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-changes
