This is an archived snapshot of W3C's public bugzilla bug tracker, decommissioned in April 2019. Please see the home page for more details.

Bug 22459 - [Custom]: Consider explicitly emptying callback queue in more cases
Summary: [Custom]: Consider explicitly emptying callback queue in more cases
Status: RESOLVED FIXED
Alias: None
Product: WebAppsWG
Classification: Unclassified
Component: HISTORICAL - Component Model (show other bugs)
Version: unspecified
Hardware: PC All
: P2 normal
Target Milestone: ---
Assignee: Dimitri Glazkov
QA Contact: public-webapps-bugzilla
URL:
Whiteboard:
Keywords:
: 22446 (view as bug list)
Depends on:
Blocks: 14968
  Show dependency treegraph
 
Reported: 2013-06-25 17:24 UTC by Dimitri Glazkov
Modified: 2013-08-26 17:57 UTC (History)
9 users (show)

See Also:


Attachments

Description Dimitri Glazkov 2013-06-25 17:24:13 UTC
Right now, only createElement and a few others trigger delivery of callbacks.

For the optimal developer experience, I think we should also do it in the following places (and possibly more):

Document.importNode
Document.execCommand
Node.cloneNode
ShadowRoot.innerHTML
Element.innerHTML
Element.outerHTML
Element.insertAdjacentHTML
Comment 1 Daniel Buchner 2013-06-25 18:01:49 UTC
(In reply to comment #0)
> Right now, only createElement and a few others trigger delivery of callbacks.
> 
> For the optimal developer experience, I think we should also do it in the
> following places (and possibly more):
> 
> Document.importNode
> Document.execCommand
> Node.cloneNode
> ShadowRoot.innerHTML
> Element.innerHTML
> Element.outerHTML
> Element.insertAdjacentHTML

Yes, this is a much needed addition, you read our collective Web-Component-minded minds! - http://www.youtube.com/watch?v=QzBmQMyYDBk
Comment 2 Dimitri Glazkov 2013-06-25 20:48:08 UTC
*** Bug 22446 has been marked as a duplicate of this bug. ***
Comment 3 Daniel Freedman 2013-06-25 20:55:35 UTC
If you have Document.importNode, then it seems likely Document.adoptNode would be needed.
Comment 4 Dominic Cooney 2013-06-25 23:55:23 UTC
How does this set of methods deliver the "optimal developer experience"? I think it would help to evaluate whether a given method is in the set or not if you broke that down into what properties you're trying to maintain.
Comment 5 Dominic Cooney 2013-06-26 02:13:22 UTC
(I started this on bug 22426 but I think it is more relevant here:)

I think callback-queue-draining is a blunt instrument that will make web developers miserable.

Is it not reentrant? Then you basically have to code for the asynchronous case anyway, because there might be another lifecycle dispatch loop operating at the top level--it would be simpler to just make everything asynchronous.

Is it reentrant? Then you get consistency problems where you can't trust some callbacks that are stale because the stack is unwinding.

(Aside: One way you could avoid a lot of teh suck with reentrancy is not to provide the state in the callback, but just notify that the state changed. For example instead of inserted and removed as separate callbacks, just one callback that says "in document may be different." Instead of attribute values, one callback that says "this attribute may have a different value." Then the element can poll. It may even make sense to coalesce changes that have not been delivered yet.)

Here's my thinking.

A custom element's "affect" (behavior, appearance, etc.) depends on its state, which includes:

- Whether it is in the tree.
- What its attributes are.
- JavaScript state (eg in properties).

(Interestingly what is *not* included here is the state of the tree around the custom element. That can be observed, and changes reacted to with Mutation Observers, but you're not proposing any new behavior, and no synchronous behavior, related to that.)

We desire custom elements that, when their state changes, their "affect" is immediately changed.

One observation is that the most important "affect" is how a custom element appears to script--its JavaScript properties, and what its methods do.

What about this:

Each custom element has its own lifecycle callbacks queue. There's a method to flush the queue. And maybe there's sugar, in the spec or provided by frameworks, to wrap the methods and properties of a custom element which flush that queue.
Comment 6 Erik Arvidsson 2013-06-26 19:58:11 UTC
If there is ever a case where an elements interface is not correct we have failed.

The problem is that methods/accessors will at some point depend on some initialization that should have happened in the constructor/createCallback. We therefore need to ensure that the createCallback is called before anyone can access the element.
Comment 7 Dimitri Glazkov 2013-06-26 21:57:08 UTC
I spent a bit of time thinking about this and I agree that this is challenging.

My intuition is that we must keep exploring the perceived-as-synchronous solution until we either find a sensible compromise or are certain there isn't one.

The "perceived-as-synchronous" is defined thusly: as far as the author is concerned, all operations look like they synchronously, measured by the likelihood of their non-synchronousness being ever detected. The goal is to approach (though not necessarily reach) zero.

I started documenting a few interesting cases we've thought up so far:
https://gist.github.com/dglazkov/5872110
Comment 8 Dimitri Glazkov 2013-06-26 21:58:25 UTC
Also, there's good discussion on bug 21969 that is not to be missed.
Comment 9 Dimitri Glazkov 2013-06-26 22:33:57 UTC
Another thought: the callbacks are explicitly about modifying internal state of a custom element in response to external changes. The author of the callback is _bound_ to encounter weirdness should they try and access or modify things that are external to the custom element. Might be worth putting this as informative text into the spec.
Comment 10 Dominic Cooney 2013-07-03 03:14:16 UTC
Here is a proposal:

Rename some of the callbacks to better reflect their purpose. readyCallback => createdCallback, insertedCallback => enteredDocumentCallback, removedCallback => leftDocumentCallback. (attributeChangedCallback remains as-is.)

Bring __proto__ swizzling, the applicability of :unresolved, and invoking the createdCallback together so that they occur at the same point in time. This means the custom element author can treat the createdCallback like a constructor and does not have to deal with handling calls on the prototype before createdCallback gets a chance to run. Authors can rely on prototype instanceof checks, querySelector with :unresolved/:not(:unresolved), and having run the constructor giving consistent results.

When the createdCallback returns, if the element is in the document, the enteredDocumentCallback must be called at that point. This means authors can assume that their elements are created outside of the document, then added to it, and keep any "in document" handling in the enteredDocumentCallback and not blur it into createdCallback.

The attributeChanged callback is more specifically typed as

callback void (DOMString name, DOMString oldValue);

When the attribute is being added, the oldValue is null. Authors can use getAttribute to retrieve the new value. Other cases: These arguments get exactly what you'd expect.

Instead of specifying a set of methods where callbacks are processed, specify that callbacks are processed at *every* method. It is a readily available optimization for browser vendors to only process callbacks at methods which may schedule callbacks.

Lastly, the queue of callbacks should become more elaborate to support recursion better. There is no recursion guard. Instead of a global queue of callbacks, each element has a local queue of callbacks pertaining just to that element. As script calls into DOM, a callback scope is pushed onto the stack. As a callback is scheduled with an element, the *element* is noted in the callback scope. When the callback scope exits (ie right before returning to script), the elements are processed; "processing" an element means invoking the callbacks in its queue. Note that there is a queue *per element*, not *per element, scope pair.* Processing a given element's queue may be resumed in the case of recursion.

I plan to implement this in Blink to get feedback from web developers.
Comment 11 Daniel Buchner 2013-07-03 15:39:14 UTC
(In reply to comment #10)
> Here is a proposal:
> 
> Rename some of the callbacks to better reflect their purpose. readyCallback
> => createdCallback, insertedCallback => enteredDocumentCallback,
> removedCallback => leftDocumentCallback. (attributeChangedCallback remains
> as-is.)

Not sure the callback names need to be more verbose, I'll posit this isn't a novice API folks will happen upon, thus the 2 second thought/look-up on MDN or Web Platform docs are not much of a hurdle. I would stay away from churning anything at this level, it just isn't worth it, IMO of course. (FWIW, I always liked create vs ready, but in the end, it probably doesn't matter)

> When the createdCallback returns, if the element is in the document, the
> enteredDocumentCallback must be called at that point. This means authors can
> assume that their elements are created outside of the document, then added
> to it, and keep any "in document" handling in the enteredDocumentCallback
> and not blur it into createdCallback.

Makes sense to me.

> The attributeChanged callback is more specifically typed as
> 
> callback void (DOMString name, DOMString oldValue);
> 
> When the attribute is being added, the oldValue is null. Authors can use
> getAttribute to retrieve the new value. Other cases: These arguments get
> exactly what you'd expect.

1) I'm not sure if there's any empirical reason not to just pass the new value (certainly not speed/perf right?) 2) Are those the "arguments you'd expect"? How do we know this, and why is another arg so 'unexpected'?
Comment 12 Dimitri Glazkov 2013-07-03 15:50:22 UTC
(In reply to comment #10)
> Here is a proposal:
> 
> Rename some of the callbacks to better reflect their purpose. readyCallback
> => createdCallback, insertedCallback => enteredDocumentCallback,
> removedCallback => leftDocumentCallback. (attributeChangedCallback remains
> as-is.)

Let's file this as a separate bug.

> Bring __proto__ swizzling, the applicability of :unresolved, and invoking
> the createdCallback together so that they occur at the same point in time.
> This means the custom element author can treat the createdCallback like a
> constructor and does not have to deal with handling calls on the prototype
> before createdCallback gets a chance to run. Authors can rely on prototype
> instanceof checks, querySelector with :unresolved/:not(:unresolved), and
> having run the constructor giving consistent results.

Ditto.
 
> When the createdCallback returns, if the element is in the document, the
> enteredDocumentCallback must be called at that point. This means authors can
> assume that their elements are created outside of the document, then added
> to it, and keep any "in document" handling in the enteredDocumentCallback
> and not blur it into createdCallback.

This is already the case in the spec, right?

https://dvcs.w3.org/hg/webcomponents/raw-file/tip/spec/custom/index.html#dfn-element-upgrade-algorithm

> 
> The attributeChanged callback is more specifically typed as
> 
> callback void (DOMString name, DOMString oldValue);
> 
> When the attribute is being added, the oldValue is null. Authors can use
> getAttribute to retrieve the new value. Other cases: These arguments get
> exactly what you'd expect.

But... the new value might not be the value that _was_ the new value at the time the callback was queued?

> Instead of specifying a set of methods where callbacks are processed,
> specify that callbacks are processed at *every* method. It is a readily
> available optimization for browser vendors to only process callbacks at
> methods which may schedule callbacks.

+1
 
> Lastly, the queue of callbacks should become more elaborate to support
> recursion better. There is no recursion guard. Instead of a global queue of
> callbacks, each element has a local queue of callbacks pertaining just to
> that element. As script calls into DOM, a callback scope is pushed onto the
> stack. As a callback is scheduled with an element, the *element* is noted in
> the callback scope. When the callback scope exits (ie right before returning
> to script), the elements are processed; "processing" an element means
> invoking the callbacks in its queue. Note that there is a queue *per
> element*, not *per element, scope pair.* Processing a given element's queue
> may be resumed in the case of recursion.

+1

> I plan to implement this in Blink to get feedback from web developers.

My gut feeling is that the callback ordering is the first order of business (and the topic of the bug). Callback naming is mostly cosmetics and can be done after. We still have several of machinery issues to resolve before then, like bug 20488.
Comment 13 Dimitri Glazkov 2013-07-03 22:37:58 UTC
Filed bug 22564 for readyCallback rename, bug 22565 for inserted/removedCallback rename, and bug 22566 for changing the timing of setting the prototype.
Comment 14 Dominic Cooney 2013-07-04 02:16:18 UTC
(In reply to comment #13)
> Filed bug 22564 for readyCallback rename, bug 22565 for
> inserted/removedCallback rename, and bug 22566 for changing the timing of
> setting the prototype.

+Let's resume the attributeChangedCallback argument discussion on bug 22426?

Now the topic of this bug is unadulterated callback ordering/recursion/etc.
Comment 15 Dominic Cooney 2013-07-09 01:35:12 UTC
I took a shot at implementing this in Blink; next we'll see what some web developers think.

Here is some implementor feedback:

The processing stack is a curious structure; callback queues enter the processing stack but then migrate within it. But they only migrate upwards because the callback queues are processed exhaustively (although that processing might be interrupted if the callback queue migrates up, then resumed.)

I'm not 100% sure how this interacts with microtasks now. Since the DOM-modifying methods push and pop from the processing stack, I think this means there's nothing to do at microtask checkpoints any more. Does that sound right?

One last minor question: When it is time to schedule a callback, should the callback queue migrate to the top of the processing stack even if that element did not register that callback? For example in this case:

Assume x-a, x-b and x-c:
- x-a has an enteredDocument callback that grabs x-c and sets an attribute
- x-b & x-c have an enteredDocument callback that logs
- x-c *has* an attributeChangedCallback.

Given:

document.body.innerHTML = '<x-a></x-a><x-b></x-b><x-c></x-c>'

I think processing stack should produce this ordering:

enter innerHTML
  x-a enteredDocument, callback enters setAttribute
    x-c enteredDocument  <-- x-c has been pulled to processing stack T.O.S.
    x-c attributeChanged
  leave setAttribute
  x-b enteredDocument
leave innerHTML

Now imagine x-c doesn't have an attributeChanged callback. Is the order the same (minus x-c attributeChanged?) Or is it

enter innerHTML
  x-a enteredDocument, callback calls setAttribute but no recursion
  x-b enteredDocument
  x-c enteredDocument
leave innerHTML
Comment 16 Scott Miles 2013-07-09 02:00:17 UTC
Imo, the algorithm should be naive and not try to resolve these timing issues.

I would expect something like:

  x-a enteredDocument
    x-c attributeChanged
  x-b enteredDocument
  x-c enteredDocument

I do realize that at 'attributeChanged' time, 'x-c' is ignorant about being in an 'entered' state, but the callbacks have to occur serially and not having simultaneous state knowledge is the price we pay for wanting to be synchronous-like.
Comment 17 Dominic Cooney 2013-07-09 02:02:30 UTC
(In reply to comment #16)
> Imo, the algorithm should be naive and not try to resolve these timing
> issues.
> 
> I would expect something like:
> 
>   x-a enteredDocument
>     x-c attributeChanged
>   x-b enteredDocument
>   x-c enteredDocument
> 
> I do realize that at 'attributeChanged' time, 'x-c' is ignorant about being
> in an 'entered' state, but the callbacks have to occur serially and not
> having simultaneous state knowledge is the price we pay for wanting to be
> synchronous-like.

If I changed enteredCallback to createdCallback, would you change your mind? It is easier for the implementation to order the callbacks as uniformly as possible.
Comment 18 Scott Miles 2013-07-09 02:04:27 UTC
`createdCallback` is special, since it's our only chance to be constructor-like.

So yes, it's important in our model that `createdCallback` be the first signal received. This is so far an invariant in our suggestions.
Comment 19 Dominic Cooney 2013-07-09 02:14:39 UTC
(In reply to comment #18)
> `createdCallback` is special, since it's our only chance to be
> constructor-like.

And enteredDocument and leftDocument callback are also special; my understanding was that you wanted these in matched pairs. Consider this modified example:

Assume x-a, x-b and x-c:
- x-a has an enteredDocument callback that grabs x-b removes it
- x-b have enteredDocument, leftDocument callbacks which log

Given:

document.body.innerHTML = '<x-a></x-a><x-b></x-b>'

I think processing stack should produce this ordering:

enter innerHTML
  x-a enteredDocument, callback removes x-b
    x-b enteredDocument  <-- note x-b is not in the document at this point
    x-b leftDocument     <-- ...however the *last* callback received is sensible
  leave remove
leave innerHTML

If I understand your new proposal, the methods would be:

enter innerHTML
  x-a enteredDocument, callback removes x-b
    x-b leftDocument     <-- state is correct, but not a matched pair
  leave remove
  x-b enteredDocument    <-- note x-b is not in the document at this point
leave innerHTML

Did I get that right?
Comment 20 Scott Miles 2013-07-09 02:15:41 UTC
To be clear, my initial comment was to suggest that you not take special pains to resolve ordering problems (apart from createdCallback going first).

If the ordering you suggested is natural for your implementation, that's fine. My intention was to say, do whatever is easiest to implement.
Comment 21 Scott Miles 2013-07-09 02:22:17 UTC
It's illogical to receive a 'left' signal before 'entered', that's true.

I admit to not being aware of all the ins and outs of this algorithm. I probably should have kept my mouth shut. 

I only meant to say we didn't need special processing in the specific case you originally described, in the hopes this made things easier on you.
Comment 22 Dominic Cooney 2013-07-09 02:43:07 UTC
(In reply to comment #21)
> It's illogical to receive a 'left' signal before 'entered', that's true.
> 
> I admit to not being aware of all the ins and outs of this algorithm. I
> probably should have kept my mouth shut. 

I value your feedback. Maybe the best thing is to try last week's proposal with actual components? It should be in tomorrow's Chrome Canary.

> I only meant to say we didn't need special processing in the specific case
> you originally described, in the hopes this made things easier on you.

This specific case is probably a corner case. It is the kind of thing where the implementation must do something but it is easy to do either one. It is literally moving one line of code to three lines earlier in a couple of places.

For now I've implemented the one where a Custom Element's callback queue is only scheduled earlier if there *is* a callback to run.
Comment 23 Elliott Sprehn 2013-07-09 03:25:20 UTC
(In reply to comment #19)
> (In reply to comment #18)
> > `createdCallback` is special, since it's our only chance to be
> > constructor-like.
> 
> And enteredDocument and leftDocument callback are also special; my
> understanding was that you wanted these in matched pairs. Consider this
> modified example:
> 
> Assume x-a, x-b and x-c:
> - x-a has an enteredDocument callback that grabs x-b removes it
> - x-b have enteredDocument, leftDocument callbacks which log
> 
> Given:
> 
> ...
> 
> Did I get that right?

No, callback execution always resumes from where it left off in the queue. You can never have a leftDocument before the associated enteredDocument(), but you _can_ have an enteredDocument() at a point where you're no longer in the document, or a leftDocument() where you're already back in the document.

All operations just push more callbacks to run. When you exit a method like innerHTML() you start running all callbacks from the oldest to the newest. If any callback adds more callbacks they just end up at the end of the line.

Your logging output doesn't match the discussion for how this works.
Comment 24 Elliott Sprehn 2013-07-09 03:29:15 UTC
(In reply to comment #23)
> ... 
> Your logging output doesn't match the discussion for how this works.

Err, my confusion, I see there's some new proposal going on here!
Comment 25 Dimitri Glazkov 2013-08-15 20:53:39 UTC
https://dvcs.w3.org/hg/webcomponents/rev/55851441bbea
Comment 26 Dimitri Glazkov 2013-08-15 20:54:23 UTC
I probably messed up something. This is a massive patch. Domininc et al, PTAL? :)