Changeset 234944 in webkit

Timestamp:

Aug 16, 2018 11:25:02 AM (6 years ago)

Author:

rniwa@webkit.org

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/custom-elements/perform-microtask-checkpoint-before-construction-expected.txt (added)
• LayoutTests/fast/custom-elements/perform-microtask-checkpoint-before-construction.html (added)
• LayoutTests/fast/dom/MutationObserver/parser-mutations.html (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/Document.cpp (modified) (1 diff)
• Source/WebCore/html/parser/HTMLDocumentParser.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/LayoutTests/fast/dom/MutationObserver/parser-mutations.html

@@ -8 +8 @@
         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});

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/dom/Document.cpp

@@ -5428 +5428 @@
         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));
 

trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp

@@ -40 +40 @@
 #include "JSCustomElementInterface.h"
 #include "LinkLoader.h"
+#include "Microtasks.h"
 #include "NavigationScheduler.h"
 #include "ScriptElement.h"
@@ -214 +215 @@
             // 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();