Bug 161423

Summary: Add the check for reentrancy to CustomElementRegistry
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: DOMAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, dbates, esprehn+autocc, kangil.han
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 154907    
Attachments:
Description Flags
Adds the check
none
Updated for ToT
none
Updated for ToT
koivisto: review+, buildbot: commit-queue-
Archive of layout-test-results from ews122 for ios-simulator-elcapitan-wk2 none

Ryosuke Niwa
Reported 2016-08-30 22:09:10 PDT
Add the "element definition is running" flag to CustomElementRegistry: https://html.spec.whatwg.org/multipage/scripting.html#element-definition-is-running and use it in customElements.define.
Attachments
Adds the check (15.46 KB, patch)
2016-08-30 22:17 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Updated for ToT (15.46 KB, patch)
2016-08-30 22:25 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Updated for ToT (15.48 KB, patch)
2016-08-30 23:16 PDT, Ryosuke Niwa 		koivisto: review+
buildbot: commit-queue-
DetailsFormatted DiffDiff
Archive of layout-test-results from ews122 for ios-simulator-elcapitan-wk2 (8.18 MB, application/zip)
2016-08-31 00:48 PDT, Build Bot 		no flags
Details
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2016-08-30 22:17:50 PDT
Created attachment 287482 [details] Adds the check
Ryosuke Niwa
Comment 2 2016-08-30 22:25:42 PDT
Created attachment 287486 [details] Updated for ToT
Ryosuke Niwa
Comment 3 2016-08-30 23:16:01 PDT
Created attachment 287490 [details] Updated for ToT
Ryosuke Niwa
Comment 4 2016-08-30 23:38:11 PDT
Comment on attachment 287490 [details] Updated for ToT View in context: https://bugs.webkit.org/attachment.cgi?id=287490&action=review > LayoutTests/fast/custom-elements/CustomElementRegistry.html:21 > test(function () { > + assert_throws({'name': 'TypeError'}, function () { customElements.define('badname', 1); }, I changed the order of tests to match the order of steps in https://html.spec.whatwg.org/multipage/scripting.html#dom-customelementregistry-define
Build Bot
Comment 5 2016-08-31 00:48:16 PDT
Comment on attachment 287490 [details] Updated for ToT Attachment 287490 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1978874 New failing tests: fast/scrolling/ios/scroll-events-back-forward.html
Build Bot
Comment 6 2016-08-31 00:48:20 PDT
Created attachment 287502 [details] Archive of layout-test-results from ews122 for ios-simulator-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-elcapitan-wk2 Platform: Mac OS X 10.11.5
Antti Koivisto
Comment 7 2016-08-31 12:18:12 PDT
Comment on attachment 287490 [details] Updated for ToT View in context: https://bugs.webkit.org/attachment.cgi?id=287490&action=review > Source/WebCore/dom/CustomElementRegistry.h:53 > +class ElementDefinitionIsRunningTemporaryChange : public TemporaryChange<bool> { > +public: > + ElementDefinitionIsRunningTemporaryChange(CustomElementRegistry&); > +}; I'm not a big fan of inheriting from basic types. Does this really look better than using TemporaryChange<bool> directly?
Ryosuke Niwa
Comment 8 2016-08-31 12:29:35 PDT
(In reply to comment #7) > Comment on attachment 287490 [details] > Updated for ToT > > View in context: > https://bugs.webkit.org/attachment.cgi?id=287490&action=review > > > Source/WebCore/dom/CustomElementRegistry.h:53 > > +class ElementDefinitionIsRunningTemporaryChange : public TemporaryChange<bool> { > > +public: > > + ElementDefinitionIsRunningTemporaryChange(CustomElementRegistry&); > > +}; > > I'm not a big fan of inheriting from basic types. Does this really look > better than using TemporaryChange<bool> directly? Will add bool& member function instead per discussion on IRC.
Ryosuke Niwa
Comment 9 2016-08-31 12:33:03 PDT
Committed r205261: <http://trac.webkit.org/changeset/205261>
Note You need to log in before you can comment on or make changes to this bug.