Bug 160797 – customElements.define should retrieve lifecycle callbacks

• Component: DOM
• Status: RESOLVED FIXED

[Ryosuke Niwa]

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.

[Ryosuke Niwa]

Created attachment 285886 [details]
Fixes the behavior

[Ryosuke Niwa]

Created attachment 285887 [details]
Fixes the behavior

[Build Bot]

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]

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 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]

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 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]

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 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]

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 on attachment 285887 [details]
Fixes the behavior

I need to fix the sequence<T> support for iterators first.

[Ryosuke Niwa]

Created attachment 286160 [details]
Patch

[Ryosuke Niwa]

Created attachment 286163 [details]
Added one more test case

[Chris Dumez]

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]

Committed r204540: <http://trac.webkit.org/changeset/204540>

[Myles C. Maxfield]

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]

Re-opened since this is blocked by bug 160932

[Myles C. Maxfield]

(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]

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]

Created attachment 286261 [details]
Patch for landing

[Ryosuke Niwa]

Created attachment 286262 [details]
Patch for landing

[Ryosuke Niwa]

Committed r204553: <http://trac.webkit.org/changeset/204553>

[Ryosuke Niwa]

<rdar://problem/28090374