Introduce CustomElementRegistry#whenDefined() method

Introduce CustomElementRegistry#whenDefined() method

This patch introduces |CustomElementRegistry#whenDefined()| as specified in [1],
and makes |CustomElementRegistry::define()| to resolve a promise associated to
newly defined name.

[1] // https://html.spec.whatwg.org/multipage/scripting.html#dom-customelementsregistry-whendefined

BUG=594918
TEST=LayoutTests/custom-elements/spec/custom-elements-registry/when_defined.html

Committed: https://crrev.com/52523d4e58d99cdb768791bf9ac532c917522460
Cr-Commit-Position: refs/heads/master@{#399171}

[yosin_UTC9, 2016-06-09 10:01:15]

Description was changed from

==========
2016-06-09T18:55:30

BUG=594918
==========

to

==========
Introduce CustomElementRegistry#whenDefined() method

This patch introduces |CustomElementRegistry#whenDefined()| as specified in [1],
and makes |CustomElementRegistry::define()| to resolve a promise associated to
newly defined name.

[1] //
https://html.spec.whatwg.org/multipage/scripting.html#dom-customelementsregis...


BUG=594918
TEST=LayoutTests/custom-elements/spec/custom-elements-registry/when_defined.html
==========

[yosin_UTC9, 2016-06-09 10:05:17]

yosin@chromium.org changed reviewers:
+ dominicc@chromium.org, kojii@chromium.org

[yosin_UTC9, 2016-06-09 10:05:17]

PTAL

Implement CustomElementsRegistry#whenDefined() in "CustomElementsRegistry.cpp".

I think it is OK to using ScriptPromise in core/ since other parts use it, e.g.
css/FontSet, html/ImageData.h.

[dominicc (has gone to gerrit), 2016-06-10 06:20:57]

lgtm

Thanks for working on this; very nice.

Some comments inline.

https://codereview.chromium.org/2048343002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/custom-elements/spec/custom-elements-registry/when_defined.html
(right):

https://codereview.chromium.org/2048343002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/custom-elements/spec/custom-elements-registry/when_defined.html:40:
}).catch(reason => { throw reason }));
I think the spec additionally requires that the same (===) Promise is returned
by whenDefined. We should do that *at least* before the name is defined; after
it is defined I think what you have is better.

Could you:

1. file a spec issue indicating that we're doing this--different promise after
defined?
2. write a test that multiple calls to whenDefined are === before something is
defined?

https://codereview.chromium.org/2048343002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/custom/CustomElementsRegistry.cpp
(right):

https://codereview.chromium.org/2048343002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/custom/CustomElementsRegistry.cpp:40: } //
namespace
Blank lines before }, after namespace { maybe? That seems to be practice. Why do
this namespace thing and not static in blink::?

[yosin_UTC9, 2016-06-10 07:41:00]

On 2016/06/10 at 06:20:57, dominicc wrote:
> lgtm
> 
> Thanks for working on this; very nice.
> 
> Some comments inline.
> 
>
https://codereview.chromium.org/2048343002/diff/1/third_party/WebKit/LayoutTe...
> File
third_party/WebKit/LayoutTests/custom-elements/spec/custom-elements-registry/when_defined.html
(right):
> 
>
https://codereview.chromium.org/2048343002/diff/1/third_party/WebKit/LayoutTe...
>
third_party/WebKit/LayoutTests/custom-elements/spec/custom-elements-registry/when_defined.html:40:
}).catch(reason => { throw reason }));
> I think the spec additionally requires that the same (===) Promise is returned
by whenDefined. We should do that *at least* before the name is defined; after
it is defined I think what you have is better.
> 
> Could you:
> 
> 1. file a spec issue indicating that we're doing this--different promise after
defined?
The spec is correct and we follow the spec. ScriptPromiseResolver::promise()
returns same Promise instance.

Here is summary:
 - After defined: spec says whenDefined() returns a new promise
 - Before defined: spec says same promise for multiple calls of whenDefined()

> 2. write a test that multiple calls to whenDefined are === before something is
defined?
Done.

[yosin_UTC9, 2016-06-10 07:52:28]

Thanks for review!

Updated to follow comments.

Committing...

https://codereview.chromium.org/2048343002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/custom-elements/spec/custom-elements-registry/when_defined.html
(right):

https://codereview.chromium.org/2048343002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/custom-elements/spec/custom-elements-registry/when_defined.html:40:
}).catch(reason => { throw reason }));
On 2016/06/10 at 06:20:57, dominicc wrote:
> I think the spec additionally requires that the same (===) Promise is returned
by whenDefined. We should do that *at least* before the name is defined; after
it is defined I think what you have is better.
> 
> Could you:
> 
> 1. file a spec issue indicating that we're doing this--different promise after
defined?

Here is summary:
 - After defined: spec says whenDefined() returns a new promise
 - Before defined: spec says same promise for multiple calls of whenDefined()

> 2. write a test that multiple calls to whenDefined are === before something is
defined?
Done.

https://codereview.chromium.org/2048343002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/custom/CustomElementsRegistry.cpp
(right):

https://codereview.chromium.org/2048343002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/custom/CustomElementsRegistry.cpp:40: } //
namespace
On 2016/06/10 at 06:20:57, dominicc wrote:
> Blank lines before }, after namespace { maybe? That seems to be practice. Why
do this namespace thing and not static in blink::?
Done.
Use static function instead of anonymous namespace, since there is only one
function.

[yosin_UTC9, 2016-06-10 07:52:31]

The CQ bit was checked by yosin@chromium.org

[yosin_UTC9, 2016-06-10 07:52:31]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominicc@chromium.org
Link to the patchset: https://codereview.chromium.org/2048343002/#ps20001
(title: "2016-06-10T16:48:25")

[commit-bot: I haz the power, 2016-06-10 07:52:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2048343002/20001

[commit-bot: I haz the power, 2016-06-10 09:24:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-10 09:24:30]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[yosin_UTC9, 2016-06-10 10:05:39]

The CQ bit was checked by yosin@chromium.org

[yosin_UTC9, 2016-06-10 10:05:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominicc@chromium.org
Link to the patchset: https://codereview.chromium.org/2048343002/#ps40001
(title: "2016-06-10T19:03:11")

[commit-bot: I haz the power, 2016-06-10 10:05:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2048343002/40001

[commit-bot: I haz the power, 2016-06-10 11:03:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-10 11:03:21]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[yosin_UTC9, 2016-06-10 12:30:08]

The CQ bit was checked by yosin@chromium.org

[commit-bot: I haz the power, 2016-06-10 12:30:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2048343002/40001

[yosin_UTC9, 2016-06-10 12:30:39]

Description was changed from

==========
Introduce CustomElementRegistry#whenDefined() method

This patch introduces |CustomElementRegistry#whenDefined()| as specified in [1],
and makes |CustomElementRegistry::define()| to resolve a promise associated to
newly defined name.

[1] //
https://html.spec.whatwg.org/multipage/scripting.html#dom-customelementsregis...


BUG=594918
TEST=LayoutTests/custom-elements/spec/custom-elements-registry/when_defined.html
==========

to

==========
Introduce CustomElementRegistry#whenDefined() method

This patch introduces |CustomElementRegistry#whenDefined()| as specified in [1],
and makes |CustomElementRegistry::define()| to resolve a promise associated to
newly defined name.

[1] //
https://html.spec.whatwg.org/multipage/scripting.html#dom-customelementsregis...

BUG=594918
TEST=LayoutTests/custom-elements/spec/custom-elements-registry/when_defined.html
==========

[commit-bot: I haz the power, 2016-06-10 13:25:32]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-10 13:26:36]

Description was changed from

==========
Introduce CustomElementRegistry#whenDefined() method

This patch introduces |CustomElementRegistry#whenDefined()| as specified in [1],
and makes |CustomElementRegistry::define()| to resolve a promise associated to
newly defined name.

[1] //
https://html.spec.whatwg.org/multipage/scripting.html#dom-customelementsregis...

BUG=594918
TEST=LayoutTests/custom-elements/spec/custom-elements-registry/when_defined.html
==========

to

==========
Introduce CustomElementRegistry#whenDefined() method

This patch introduces |CustomElementRegistry#whenDefined()| as specified in [1],
and makes |CustomElementRegistry::define()| to resolve a promise associated to
newly defined name.

[1] //
https://html.spec.whatwg.org/multipage/scripting.html#dom-customelementsregis...

BUG=594918
TEST=LayoutTests/custom-elements/spec/custom-elements-registry/when_defined.html

Committed: https://crrev.com/52523d4e58d99cdb768791bf9ac532c917522460
Cr-Commit-Position: refs/heads/master@{#399171}
==========

[commit-bot: I haz the power, 2016-06-10 13:26:37]

Patchset 3 (id:??) landed as
https://crrev.com/52523d4e58d99cdb768791bf9ac532c917522460
Cr-Commit-Position: refs/heads/master@{#399171}