Attachment 347118 Details for Bug 188189 – Fixes the bug

[patch] Fixes the bug

bug-188189-20180814154923.patch (text/plain), 11.85 KB, created by Ryosuke Niwa on 2018-08-14 15:49:24 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2018-08-14 15:49:24 PDT

Size: 11.85 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 234862)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,28 @@
>+2018-08-14  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 NOBODY (OOPS!).
>+
>+        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-14  Alex Christensen  <achristensen@webkit.org>
> 
>         Use a Variant instead of a union in CSSSelector
>Index: Source/WebCore/dom/Document.cpp
>===================================================================
>--- Source/WebCore/dom/Document.cpp	(revision 234834)
>+++ Source/WebCore/dom/Document.cpp	(working copy)
>@@ -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: Source/WebCore/html/parser/HTMLDocumentParser.cpp
>===================================================================
>--- Source/WebCore/html/parser/HTMLDocumentParser.cpp	(revision 234834)
>+++ Source/WebCore/html/parser/HTMLDocumentParser.cpp	(working copy)
>@@ -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::runScriptsForPa
>         {
>             // 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);
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 234834)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,17 @@
>+2018-08-14  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 NOBODY (OOPS!).
>+
>+        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.
>+
> 2018-08-13  Joseph Pecoraro  <pecoraro@apple.com>
> 
>         [macOS debug] LayoutTest inspector/worker/resources-in-worker.html is a flaky timeout
>Index: LayoutTests/fast/custom-elements/perform-microtask-checkpoint-before-construction-expected.txt
>===================================================================
>--- LayoutTests/fast/custom-elements/perform-microtask-checkpoint-before-construction-expected.txt	(nonexistent)
>+++ LayoutTests/fast/custom-elements/perform-microtask-checkpoint-before-construction-expected.txt	(working copy)
>@@ -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 
>+
>Index: LayoutTests/fast/custom-elements/perform-microtask-checkpoint-before-construction.html
>===================================================================
>--- LayoutTests/fast/custom-elements/perform-microtask-checkpoint-before-construction.html	(nonexistent)
>+++ LayoutTests/fast/custom-elements/perform-microtask-checkpoint-before-construction.html	(working copy)
>@@ -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>

Actions: View | Formatted Diff | Diff

Attachments on bug 188189: 347118 | 347127 | 347133 | 347135 | 347138 | 347143