WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Fixes the behavior
(39.88 KB, patch)
2016-08-11 20:42 PDT
,
Ryosuke Niwa
no flags
Details
Formatted Diff
Diff
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
Details
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
Details
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
Details
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
Details
Patch
(44.62 KB, patch)
2016-08-16 00:25 PDT
,
Ryosuke Niwa
no flags
Details
Formatted Diff
Diff
Added one more test case
(45.88 KB, patch)
2016-08-16 00:51 PDT
,
Ryosuke Niwa
no flags
Details
Formatted Diff
Diff
Patch for landing
(49.92 KB, patch)
2016-08-16 20:31 PDT
,
Ryosuke Niwa
no flags
Details
Formatted Diff
Diff
Patch for landing
(49.93 KB, patch)
2016-08-16 20:39 PDT
,
Ryosuke Niwa
no flags
Details
Formatted Diff
Diff
Show Obsolete
(9)
View All
Add attachment
proposed patch, testcase, etc.
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
Created
attachment 286160
[details]
Patch
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
Committed
r204540
: <
http://trac.webkit.org/changeset/204540
>
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
Committed
r204553
: <
http://trac.webkit.org/changeset/204553
>
Ryosuke Niwa
Comment 23
2016-09-01 16:16:47 PDT
<
rdar://problem/28090374
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug