RESOLVED FIXED 160797
customElements.define should retrieve lifecycle callbacks
https://bugs.webkit.org/show_bug.cgi?id=160797
Summary customElements.define should retrieve lifecycle callbacks
Ryosuke Niwa
Reported 2016-08-11 20:27:16 PDT
The latest spec mandates that `customElements.define` invoke `Get(constructor, "prototype")` as well as `Get(prototype, callbackName)` for each lifecycle callback. Do that, and also add the support for observedAttributes.
Attachments
Fixes the behavior (127.21 KB, patch)
2016-08-11 20:40 PDT, Ryosuke Niwa
no flags
Fixes the behavior (39.88 KB, patch)
2016-08-11 20:42 PDT, Ryosuke Niwa
no flags
Archive of layout-test-results from ews102 for mac-yosemite (1.11 MB, application/zip)
2016-08-11 21:21 PDT, Build Bot
no flags
Archive of layout-test-results from ews104 for mac-yosemite-wk2 (1.02 MB, application/zip)
2016-08-11 21:32 PDT, Build Bot
no flags
Archive of layout-test-results from ews114 for mac-yosemite (1.30 MB, application/zip)
2016-08-11 21:41 PDT, Build Bot
no flags
Archive of layout-test-results from ews122 for ios-simulator-elcapitan-wk2 (604.98 KB, application/zip)
2016-08-11 21:56 PDT, Build Bot
no flags
Patch (44.62 KB, patch)
2016-08-16 00:25 PDT, Ryosuke Niwa
no flags
Added one more test case (45.88 KB, patch)
2016-08-16 00:51 PDT, Ryosuke Niwa
no flags
Patch for landing (49.92 KB, patch)
2016-08-16 20:31 PDT, Ryosuke Niwa
no flags
Patch for landing (49.93 KB, patch)
2016-08-16 20:39 PDT, Ryosuke Niwa
no flags
Ryosuke Niwa
Comment 1 2016-08-11 20:40:29 PDT
Created attachment 285886 [details] Fixes the behavior
Ryosuke Niwa
Comment 2 2016-08-11 20:42:10 PDT
Created attachment 285887 [details] Fixes the behavior
Build Bot
Comment 3 2016-08-11 21:21:48 PDT
Comment on attachment 285887 [details] Fixes the behavior Attachment 285887 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1855682 Number of test failures exceeded the failure limit.
Build Bot
Comment 4 2016-08-11 21:21:51 PDT
Created attachment 285889 [details] Archive of layout-test-results from ews102 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-yosemite Platform: Mac OS X 10.10.5
Build Bot
Comment 5 2016-08-11 21:32:22 PDT
Comment on attachment 285887 [details] Fixes the behavior Attachment 285887 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1855720 Number of test failures exceeded the failure limit.
Build Bot
Comment 6 2016-08-11 21:32:25 PDT
Created attachment 285890 [details] Archive of layout-test-results from ews104 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Build Bot
Comment 7 2016-08-11 21:41:55 PDT
Comment on attachment 285887 [details] Fixes the behavior Attachment 285887 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1855724 Number of test failures exceeded the failure limit.
Build Bot
Comment 8 2016-08-11 21:41:58 PDT
Created attachment 285891 [details] Archive of layout-test-results from ews114 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-yosemite Platform: Mac OS X 10.10.5
Build Bot
Comment 9 2016-08-11 21:56:53 PDT
Comment on attachment 285887 [details] Fixes the behavior Attachment 285887 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1855750 Number of test failures exceeded the failure limit.
Build Bot
Comment 10 2016-08-11 21:56:56 PDT
Created attachment 285892 [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
Ryosuke Niwa
Comment 11 2016-08-11 23:30:29 PDT
Comment on attachment 285887 [details] Fixes the behavior I need to fix the sequence<T> support for iterators first.
Ryosuke Niwa
Comment 12 2016-08-16 00:25:08 PDT
Ryosuke Niwa
Comment 13 2016-08-16 00:51:45 PDT
Created attachment 286163 [details] Added one more test case
Chris Dumez
Comment 14 2016-08-16 15:26:39 PDT
Comment on attachment 286163 [details] Added one more test case View in context: https://bugs.webkit.org/attachment.cgi?id=286163&action=review r=me with comments > Source/WebCore/bindings/js/JSCustomElementsRegistryCustom.cpp:42 > +static JSObject* getLifecycleCallback(ExecState& state, JSValue prototype, const Identifier& id) At the call sites, you already check that prototype is a JSObject, therefore, could we take a JSObject& here instead of a JSValue? > Source/WebCore/bindings/js/JSCustomElementsRegistryCustom.cpp:47 > + if (callback.isUndefined()) Seems redundant given the isFunction() check below? > Source/WebCore/bindings/js/JSCustomElementsRegistryCustom.cpp:85 > + // FIXME: Check re-entracy here. typo > Source/WebCore/bindings/js/JSCustomElementsRegistryCustom.cpp:127 > + if (attributeChangedCallback) { Can this really be null if there was no exception thrown? Could this simply be an assertion? > Source/WebCore/dom/LifecycleCallbackQueue.cpp:113 > + RELEASE_ASSERT(elementInterface); Is this useful? I mean, it will crash on the next line anyway. > Source/WebCore/dom/LifecycleCallbackQueue.h:30 > +#include "Element.h" Hmm, why do we need to include Element.h here now ? :( We should try and avoid this for the sake of my build time.
Ryosuke Niwa
Comment 15 2016-08-16 16:57:25 PDT
Myles C. Maxfield
Comment 16 2016-08-16 20:12:54 PDT
I think this broke the Windows build. https://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/96729/steps/compile-webkit/logs/stdio/text C:\cygwin\home\buildbot\slave\win-debug\build\Source\WebCore\bindings\js\JSCustomElementsRegistryCustom.cpp(123): error C2236: unexpected token 'struct'. Did you forget a ';'? [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\WebCore\WebCore.vcxproj] C:\cygwin\home\buildbot\slave\win-debug\build\Source\WebCore\bindings\js\JSCustomElementsRegistryCustom.cpp(123): error C2332: 'struct': missing tag name [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\WebCore\WebCore.vcxproj] C:\cygwin\home\buildbot\slave\win-debug\build\Source\WebCore\bindings\js\JSCustomElementsRegistryCustom.cpp(123): error C2513: '': no variable declared before '=' [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\WebCore\WebCore.vcxproj] C:\cygwin\home\buildbot\slave\win-debug\build\Source\WebCore\bindings\js\JSCustomElementsRegistryCustom.cpp(133): error C2332: 'struct': missing tag name [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\WebCore\WebCore.vcxproj] C:\cygwin\home\buildbot\slave\win-debug\build\Source\WebCore\bindings\js\JSCustomElementsRegistryCustom.cpp(133): warning C4094: untagged 'struct' declared no symbols [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\WebCore\WebCore.vcxproj] C:\cygwin\home\buildbot\slave\win-debug\build\Source\WebCore\bindings\js\JSCustomElementsRegistryCustom.cpp(133): error C2143: syntax error: missing ';' before '->' [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\WebCore\WebCore.vcxproj] C:\cygwin\home\buildbot\slave\win-debug\build\Source\WebCore\bindings\js\JSCustomElementsRegistryCustom.cpp(133): error C2059: syntax error: '->' [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\WebCore\WebCore.vcxproj] C:\cygwin\home\buildbot\slave\win-debug\build\Source\WebCore\bindings\js\JSCustomElementsRegistryCustom.cpp(139): error C2332: 'struct': missing tag name [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\WebCore\WebCore.vcxproj] C:\cygwin\home\buildbot\slave\win-debug\build\Source\WebCore\bindings\js\JSCustomElementsRegistryCustom.cpp(139): error C2062: type '' unexpected [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\WebCore\WebCore.vcxproj]
WebKit Commit Bot
Comment 17 2016-08-16 20:15:41 PDT
Re-opened since this is blocked by bug 160932
Myles C. Maxfield
Comment 18 2016-08-16 20:17:21 PDT
(In reply to comment #16) > I think this broke the Windows build. Oh, it looks like you're already working on this at https://trac.webkit.org/changeset/204547
Ryosuke Niwa
Comment 19 2016-08-16 20:17:39 PDT
I've landed two speculative fixes but didn't work. The build failure error makes zero sense to me. There is no keyword "struct" at line 123.
Ryosuke Niwa
Comment 20 2016-08-16 20:31:59 PDT
Created attachment 286261 [details] Patch for landing
Ryosuke Niwa
Comment 21 2016-08-16 20:39:02 PDT
Created attachment 286262 [details] Patch for landing
Ryosuke Niwa
Comment 22 2016-08-16 21:44:29 PDT
Ryosuke Niwa
Comment 23 2016-09-01 16:16:47 PDT
Note You need to log in before you can comment on or make changes to this bug.