Bug 188189 - Perform a microtask checkpoint before creating a custom element
Summary: Perform a microtask checkpoint before creating a custom element
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL: http://w3c-test.org/custom-elements/m...
Keywords: InRadar
: 188284 (view as bug list)
Depends on:
Blocks: 154907
  Show dependency treegraph
 
Reported: 2018-07-31 00:31 PDT by Frédéric Wang (:fredw)
Modified: 2018-11-30 10:24 PST (History)
16 users (show)

See Also:


Attachments
Fixes the bug (11.85 KB, patch)
2018-08-14 15:49 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-sierra (2.62 MB, application/zip)
2018-08-14 16:35 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-sierra-wk2 (2.85 MB, application/zip)
2018-08-14 17:06 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews123 for ios-simulator-wk2 (2.59 MB, application/zip)
2018-08-14 17:25 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews115 for mac-sierra (3.07 MB, application/zip)
2018-08-14 17:35 PDT, Build Bot
no flags Details
Fixed the test (13.11 KB, patch)
2018-08-14 18:57 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Frédéric Wang (:fredw) 2018-07-31 00:31:45 PDT
From https://html.spec.whatwg.org/multipage/parsing.html#create-an-element-for-the-token (step 6.2):

If will execute script is true, then: If the JavaScript execution context stack is empty, then perform a microtask checkpoint.
Comment 1 Ryosuke Niwa 2018-08-01 17:54:41 PDT
Are there any WPT tests for this?
Comment 2 Radar WebKit Bug Importer 2018-08-01 22:42:48 PDT
<rdar://problem/42843022>
Comment 3 Frédéric Wang (:fredw) 2018-08-02 01:49:38 PDT
(In reply to Ryosuke Niwa from comment #1)
> Are there any WPT tests for this?

I think it is microtasks-and-constructors.html which we currently fails.

In addition to comment 0, the test and https://github.com/whatwg/html/issues/2381 also refer to microtask during element upgrade, but I can't find where it in the current version of the spec.
Comment 5 Ryosuke Niwa 2018-08-14 15:36:28 PDT
*** Bug 188284 has been marked as a duplicate of this bug. ***
Comment 6 Ryosuke Niwa 2018-08-14 15:49:24 PDT
Created attachment 347118 [details]
Fixes the bug
Comment 7 Build Bot 2018-08-14 16:35:00 PDT
Comment on attachment 347118 [details]
Fixes the bug

Attachment 347118 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/8860603

New failing tests:
fast/dom/MutationObserver/parser-mutations.html
Comment 8 Build Bot 2018-08-14 16:35:02 PDT
Created attachment 347127 [details]
Archive of layout-test-results from ews101 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 9 Build Bot 2018-08-14 17:06:15 PDT
Comment on attachment 347118 [details]
Fixes the bug

Attachment 347118 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/8860946

New failing tests:
fast/dom/MutationObserver/parser-mutations.html
Comment 10 Build Bot 2018-08-14 17:06:17 PDT
Created attachment 347133 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 11 Build Bot 2018-08-14 17:25:50 PDT
Comment on attachment 347118 [details]
Fixes the bug

Attachment 347118 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/8860804

New failing tests:
fast/dom/MutationObserver/parser-mutations.html
Comment 12 Build Bot 2018-08-14 17:25:52 PDT
Created attachment 347135 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4
Comment 13 Build Bot 2018-08-14 17:35:06 PDT
Comment on attachment 347118 [details]
Fixes the bug

Attachment 347118 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/8861012

New failing tests:
fast/dom/MutationObserver/parser-mutations.html
Comment 14 Build Bot 2018-08-14 17:35:08 PDT
Created attachment 347138 [details]
Archive of layout-test-results from ews115 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 15 Ryosuke Niwa 2018-08-14 18:57:58 PDT
Created attachment 347143 [details]
Fixed the test
Comment 16 Geoffrey Garen 2018-08-16 10:55:47 PDT
Comment on attachment 347143 [details]
Fixed the test

r=me
Comment 17 Ryosuke Niwa 2018-08-16 10:57:58 PDT
Comment on attachment 347143 [details]
Fixed the test

Thanks!
Comment 18 WebKit Commit Bot 2018-08-16 11:25:08 PDT
Comment on attachment 347143 [details]
Fixed the test

Clearing flags on attachment: 347143

Committed r234944: <https://trac.webkit.org/changeset/234944>
Comment 19 WebKit Commit Bot 2018-08-16 11:25:10 PDT
All reviewed patches have been landed.  Closing bug.
Comment 20 Frédéric Wang (:fredw) 2018-08-22 02:59:44 PDT
Comment on attachment 347143 [details]
Fixed the test

View in context: https://bugs.webkit.org/attachment.cgi?id=347143&action=review

> Source/WebCore/dom/Document.cpp:5430
> +    // FIXME: Schdule a task to fire DOMContentLoaded event instead. See webkit.org/b/82931

nit: Schedule

> LayoutTests/ChangeLog:13
> +        * fast/custom-elements/perform-microtask-checkpoint-before-construction.html: Added.

Does it make sense to export this to the upstream WPT repo?
Comment 21 Ryosuke Niwa 2018-08-22 23:09:43 PDT
Comment on attachment 347143 [details]
Fixed the test

View in context: https://bugs.webkit.org/attachment.cgi?id=347143&action=review

>> LayoutTests/ChangeLog:13
>> +        * fast/custom-elements/perform-microtask-checkpoint-before-construction.html: Added.
> 
> Does it make sense to export this to the upstream WPT repo?

Yes.
Comment 22 darshan 2018-11-23 22:18:59 PST
Created pull request https://github.com/web-platform-tests/wpt/pull/14221,

Second test is failing as of now.
Comment 23 Ryosuke Niwa 2018-11-23 22:33:55 PST
(In reply to darshan from comment #22)
> Created pull request https://github.com/web-platform-tests/wpt/pull/14221,
> 
> Second test is failing as of now.

Against which STP or trunk revision did you test this?
Comment 24 darshan 2018-11-23 23:32:33 PST
(In reply to Ryosuke Niwa from comment #23)
> (In reply to darshan from comment #22)
> > Created pull request https://github.com/web-platform-tests/wpt/pull/14221,
> > 
> > Second test is failing as of now.
> 
> Against which STP or trunk revision did you test this?
I did ./wpt serve in repo and open the test in chromium, I am using Chromium 70.0.3538.77
Comment 25 Ryosuke Niwa 2018-11-24 01:12:25 PST
(In reply to darshan from comment #24)
> (In reply to Ryosuke Niwa from comment #23)
> > (In reply to darshan from comment #22)
> > > Created pull request https://github.com/web-platform-tests/wpt/pull/14221,
> > > 
> > > Second test is failing as of now.
> > 
> > Against which STP or trunk revision did you test this?
> I did ./wpt serve in repo and open the test in chromium, I am using Chromium
> 70.0.3538.77

Okay, that's not too surprising because you're testing against Blink. Note that Chromium no longer uses WebKit. It uses an alternative engine called Blink, which was forked in 2013.
Comment 26 darshan 2018-11-24 01:34:03 PST
(In reply to Ryosuke Niwa from comment #25)
> (In reply to darshan from comment #24)
> > (In reply to Ryosuke Niwa from comment #23)
> > > (In reply to darshan from comment #22)
> > > > Created pull request https://github.com/web-platform-tests/wpt/pull/14221,
> > > > 
> > > > Second test is failing as of now.
> > > 
> > > Against which STP or trunk revision did you test this?
> > I did ./wpt serve in repo and open the test in chromium, I am using Chromium
> > 70.0.3538.77
> 
> Okay, that's not too surprising because you're testing against Blink. Note
> that Chromium no longer uses WebKit. It uses an alternative engine called
> Blink, which was forked in 2013.

Okay, so it means we can merge this PR in master?
Comment 27 Frédéric Wang (:fredw) 2018-11-24 02:18:37 PST
(In reply to darshan from comment #26)
> (In reply to Ryosuke Niwa from comment #25)
> > (In reply to darshan from comment #24)
> > > (In reply to Ryosuke Niwa from comment #23)
> > > > (In reply to darshan from comment #22)
> > > > > Created pull request https://github.com/web-platform-tests/wpt/pull/14221,
> > > > > 
> > > > > Second test is failing as of now.
> > > > 
> > > > Against which STP or trunk revision did you test this?
> > > I did ./wpt serve in repo and open the test in chromium, I am using Chromium
> > > 70.0.3538.77
> > 
> > Okay, that's not too surprising because you're testing against Blink. Note
> > that Chromium no longer uses WebKit. It uses an alternative engine called
> > Blink, which was forked in 2013.
> 
> Okay, so it means we can merge this PR in master?

@Darshan: Thanks for the PR. This bug is fixed in WebKit. If you are using a trunk build of WebKit then all the tests should pass. As Ryosuke said, Chromium does not use WebKit anymore so behavior might be different. Let's continue the discussion on github WPT.
Comment 28 darshan 2018-11-30 10:24:32 PST
Created new PR https://github.com/web-platform-tests/wpt/pull/14314,
Closed old PR(https://github.com/web-platform-tests/wpt/pull/14221), as I messed git history