Flush microtask before event handlers are invoked

Flush microtask before event handlers are invoked

This is needed as we want to see custom elements upgraded
in @onload event handler.

BUG=366877
TEST=import-custom-element-onload.html
R=dominicc@chromium.org, haraken@chromium.org

[Hajime Morrita, 2014-04-24 22:18:58]

Dominic: PTAL?

This is a spin out from https://codereview.chromium.org/249563003/.

Haraken: could you rubberstamp binding/ change based on Dominic's judge?

[haraken, 2014-04-25 01:55:22]

bindings/ LGTM.

[dominicc (has gone to gerrit), 2014-04-25 02:14:49]

I think rafaelw should look at this one too.

https://codereview.chromium.org/259503007/diff/10001/LayoutTests/fast/html/im...
File LayoutTests/fast/html/imports/import-custom-element-onload.html (right):

https://codereview.chromium.org/259503007/diff/10001/LayoutTests/fast/html/im...
LayoutTests/fast/html/imports/import-custom-element-onload.html:15: t =
async_test('Custom elements are upgraded when onload is called.');
This isn't clear; use "should be" instead of "are" and "when" is vague...
before? During? After?

https://codereview.chromium.org/259503007/diff/10001/LayoutTests/fast/html/im...
LayoutTests/fast/html/imports/import-custom-element-onload.html:18: {
Again, I think this brace goes on the previous line. JavaScript != C++.

https://codereview.chromium.org/259503007/diff/10001/LayoutTests/fast/html/im...
LayoutTests/fast/html/imports/import-custom-element-onload.html:20:
assert_array_equals(["1", "2", "3"], hello.ids);
Stick to one kind of quote character for string literals unless there's a good
reason to use a different one.

https://codereview.chromium.org/259503007/diff/10001/Source/bindings/v8/V8Abs...
File Source/bindings/v8/V8AbstractEventListener.cpp (right):

https://codereview.chromium.org/259503007/diff/10001/Source/bindings/v8/V8Abs...
Source/bindings/v8/V8AbstractEventListener.cpp:113:
Microtask::performCheckpoint(isolate);
Can you point to the DOM or HTML spec (...or all specs with event listeners?)
that says to do this?

[adamk, 2014-04-25 18:23:31]

https://codereview.chromium.org/259503007/diff/10001/Source/bindings/v8/V8Abs...
File Source/bindings/v8/V8AbstractEventListener.cpp (right):

https://codereview.chromium.org/259503007/diff/10001/Source/bindings/v8/V8Abs...
Source/bindings/v8/V8AbstractEventListener.cpp:113:
Microtask::performCheckpoint(isolate);
On 2014/04/25 02:14:50, dominicc wrote:
> Can you point to the DOM or HTML spec (...or all specs with event listeners?)
> that says to do this?

I'm fairly sure this is not specced, and I'm not clear on where this requirement
came from. The attached bug has not details...

[Hajime Morrita, 2014-04-25 18:34:26]

On 2014/04/25 18:23:31, adamk wrote:
>
https://codereview.chromium.org/259503007/diff/10001/Source/bindings/v8/V8Abs...
> File Source/bindings/v8/V8AbstractEventListener.cpp (right):
> 
>
https://codereview.chromium.org/259503007/diff/10001/Source/bindings/v8/V8Abs...
> Source/bindings/v8/V8AbstractEventListener.cpp:113:
> Microtask::performCheckpoint(isolate);
> On 2014/04/25 02:14:50, dominicc wrote:
> > Can you point to the DOM or HTML spec (...or all specs with event
listeners?)
> > that says to do this?
> 
> I'm fairly sure this is not specced, and I'm not clear on where this
requirement
> came from. The attached bug has not details...

The layout test captures the exact issue I hoped to work.
What I want is that Custom Elements an import are upgraded when onload of the
import is called.

This doesn't come from production/Polymer but I just thought this should work.
So probably this isn't right fix or even isn't worth fixing.

[blink-reviews, 2014-04-25 19:34:20]

The issue is that creating a delivery step here will affect more than
custom elements. If you want this case handled, it needs to be a larger
(spec) discussion about when performing the microtask checkpoint takes
place.


On Fri, Apr 25, 2014 at 11:34 AM, <morrita@chromium.org> wrote:

> On 2014/04/25 18:23:31, adamk wrote:
>
> https://codereview.chromium.org/259503007/diff/10001/Source/bindings/v8/
> V8AbstractEventListener.cpp
>
>> File Source/bindings/v8/V8AbstractEventListener.cpp (right):
>>
>
>
> https://codereview.chromium.org/259503007/diff/10001/Source/bindings/v8/
> V8AbstractEventListener.cpp#newcode113
>
>> Source/bindings/v8/V8AbstractEventListener.cpp:113:
>> Microtask::performCheckpoint(isolate);
>> On 2014/04/25 02:14:50, dominicc wrote:
>> > Can you point to the DOM or HTML spec (...or all specs with event
>>
> listeners?)
>
>> > that says to do this?
>>
>
>  I'm fairly sure this is not specced, and I'm not clear on where this
>>
> requirement
>
>> came from. The attached bug has not details...
>>
>
> The layout test captures the exact issue I hoped to work.
> What I want is that Custom Elements an import are upgraded when onload of
> the
> import is called.
>
> This doesn't come from production/Polymer but I just thought this should
> work.
> So probably this isn't right fix or even isn't worth fixing.
>
> https://codereview.chromium.org/259503007/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[Hajime Morrita, 2014-04-25 22:34:01]

On 2014/04/25 19:34:20, blink-reviews_chromium.org wrote:
> The issue is that creating a delivery step here will affect more than
> custom elements. If you want this case handled, it needs to be a larger
> (spec) discussion about when performing the microtask checkpoint takes
> place.
> 

Yeah, this is valid concern. As I heard any complaint about this so far, I'd
like to let it alone.
Thanks for giving your thoughts folks.

> 
> On Fri, Apr 25, 2014 at 11:34 AM, <mailto:morrita@chromium.org> wrote:
> 
> > On 2014/04/25 18:23:31, adamk wrote:
> >
> > https://codereview.chromium.org/259503007/diff/10001/Source/bindings/v8/
> > V8AbstractEventListener.cpp
> >
> >> File Source/bindings/v8/V8AbstractEventListener.cpp (right):
> >>
> >
> >
> > https://codereview.chromium.org/259503007/diff/10001/Source/bindings/v8/
> > V8AbstractEventListener.cpp#newcode113
> >
> >> Source/bindings/v8/V8AbstractEventListener.cpp:113:
> >> Microtask::performCheckpoint(isolate);
> >> On 2014/04/25 02:14:50, dominicc wrote:
> >> > Can you point to the DOM or HTML spec (...or all specs with event
> >>
> > listeners?)
> >
> >> > that says to do this?
> >>
> >
> >  I'm fairly sure this is not specced, and I'm not clear on where this
> >>
> > requirement
> >
> >> came from. The attached bug has not details...
> >>
> >
> > The layout test captures the exact issue I hoped to work.
> > What I want is that Custom Elements an import are upgraded when onload of
> > the
> > import is called.
> >
> > This doesn't come from production/Polymer but I just thought this should
> > work.
> > So probably this isn't right fix or even isn't worth fixing.
> >
> > https://codereview.chromium.org/259503007/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:blink-reviews+unsubscribe@chromium.org.

[dominicc (has gone to gerrit), 2014-04-25 23:30:36]

Is this the WebComponentsLoaded event? We could just look at adding that.

Naive question: if you want to synchronize Custom Element callbacks and
onload, why don't you specialize the Custom Element Microtask Queue for
imports and have them notify you when the queue is "exiting" back to the
parent queue and schedule onload at that (maybe much later) point?
On Apr 26, 2014 7:34 AM, <morrita@chromium.org> wrote:

>
> On 2014/04/25 19:34:20, blink-reviews_chromium.org wrote:
>
>> The issue is that creating a delivery step here will affect more than
>> custom elements. If you want this case handled, it needs to be a larger
>> (spec) discussion about when performing the microtask checkpoint takes
>> place.
>>
>
>
> Yeah, this is valid concern. As I heard any complaint about this so far,
> I'd
> like to let it alone.
> Thanks for giving your thoughts folks.
>
>
>  On Fri, Apr 25, 2014 at 11:34 AM, <mailto:morrita@chromium.org> wrote:
>>
>
>  > On 2014/04/25 18:23:31, adamk wrote:
>> >
>> > https://codereview.chromium.org/259503007/diff/10001/
>> Source/bindings/v8/
>> > V8AbstractEventListener.cpp
>> >
>> >> File Source/bindings/v8/V8AbstractEventListener.cpp (right):
>> >>
>> >
>> >
>> > https://codereview.chromium.org/259503007/diff/10001/
>> Source/bindings/v8/
>> > V8AbstractEventListener.cpp#newcode113
>> >
>> >> Source/bindings/v8/V8AbstractEventListener.cpp:113:
>> >> Microtask::performCheckpoint(isolate);
>> >> On 2014/04/25 02:14:50, dominicc wrote:
>> >> > Can you point to the DOM or HTML spec (...or all specs with event
>> >>
>> > listeners?)
>> >
>> >> > that says to do this?
>> >>
>> >
>> >  I'm fairly sure this is not specced, and I'm not clear on where this
>> >>
>> > requirement
>> >
>> >> came from. The attached bug has not details...
>> >>
>> >
>> > The layout test captures the exact issue I hoped to work.
>> > What I want is that Custom Elements an import are upgraded when onload
>> of
>> > the
>> > import is called.
>> >
>> > This doesn't come from production/Polymer but I just thought this should
>> > work.
>> > So probably this isn't right fix or even isn't worth fixing.
>> >
>> > https://codereview.chromium.org/259503007/
>> >
>>
>
>  To unsubscribe from this group and stop receiving emails from it, send an
>>
> email
>
>> to mailto:blink-reviews+unsubscribe@chromium.org.
>>
>
>
>
> https://codereview.chromium.org/259503007/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[Hajime Morrita, 2014-04-25 23:49:12]

On 2014/04/25 23:30:36, dominicc wrote:
> Is this the WebComponentsLoaded event? We could just look at adding that.
> 
Hmm? What do you mean? Are you suggesting to add another event? That is
possible.
However, I hope the "load" event being the catch-all type of event that ensures
everything is ready.
In my intuition, WebComponentsLoaded can be fired earlier than load events, not
later.
This probably should be talked at more appropriate forum though.

> Naive question: if you want to synchronize Custom Element callbacks and
> onload, why don't you specialize the Custom Element Microtask Queue for
> imports and have them notify you when the queue is "exiting" back to the
> parent queue and schedule onload at that (maybe much later) point?

Right. Using microtask queue to notify the end of pending element updgrade
sounds better solution actually.

> On Apr 26, 2014 7:34 AM, <mailto:morrita@chromium.org> wrote:
> 
> >
> > On 2014/04/25 19:34:20, http://blink-reviews_chromium.org wrote:
> >
> >> The issue is that creating a delivery step here will affect more than
> >> custom elements. If you want this case handled, it needs to be a larger
> >> (spec) discussion about when performing the microtask checkpoint takes
> >> place.
> >>
> >
> >
> > Yeah, this is valid concern. As I heard any complaint about this so far,
> > I'd
> > like to let it alone.
> > Thanks for giving your thoughts folks.
> >
> >
> >  On Fri, Apr 25, 2014 at 11:34 AM, <mailto:morrita@chromium.org> wrote:
> >>
> >
> >  > On 2014/04/25 18:23:31, adamk wrote:
> >> >
> >> > https://codereview.chromium.org/259503007/diff/10001/
> >> Source/bindings/v8/
> >> > V8AbstractEventListener.cpp
> >> >
> >> >> File Source/bindings/v8/V8AbstractEventListener.cpp (right):
> >> >>
> >> >
> >> >
> >> > https://codereview.chromium.org/259503007/diff/10001/
> >> Source/bindings/v8/
> >> > V8AbstractEventListener.cpp#newcode113
> >> >
> >> >> Source/bindings/v8/V8AbstractEventListener.cpp:113:
> >> >> Microtask::performCheckpoint(isolate);
> >> >> On 2014/04/25 02:14:50, dominicc wrote:
> >> >> > Can you point to the DOM or HTML spec (...or all specs with event
> >> >>
> >> > listeners?)
> >> >
> >> >> > that says to do this?
> >> >>
> >> >
> >> >  I'm fairly sure this is not specced, and I'm not clear on where this
> >> >>
> >> > requirement
> >> >
> >> >> came from. The attached bug has not details...
> >> >>
> >> >
> >> > The layout test captures the exact issue I hoped to work.
> >> > What I want is that Custom Elements an import are upgraded when onload
> >> of
> >> > the
> >> > import is called.
> >> >
> >> > This doesn't come from production/Polymer but I just thought this should
> >> > work.
> >> > So probably this isn't right fix or even isn't worth fixing.
> >> >
> >> > https://codereview.chromium.org/259503007/
> >> >
> >>
> >
> >  To unsubscribe from this group and stop receiving emails from it, send an
> >>
> > email
> >
> >> to mailto:blink-reviews+unsubscribe@chromium.org.
> >>
> >
> >
> >
> > https://codereview.chromium.org/259503007/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:blink-reviews+unsubscribe@chromium.org.