Run Mutation Observer and Custom Element callbacks consistently at microtask checkpoint

This introduces one place, Microtask::performCheckpoint, to add checkpoint steps and teleports all calls to MutationObservers::deliverAllMutations to their new home.

It pulls the callback dispatching behavior that was left in CustomElementRegistry into its own class, CustomElementCallbackDispatcher. Custom Elements have evolved and how involve four callbacks with three timing semantics. CustomElementCallbackDispatcher will be the traffic cop when the latter three kinds of callbacks are implemented.

This is fixes two bad behaviors (1. run Custom Element lifecycle callbacks and Mutation Observers in an inconsistent order depending on callsite; 2. allow pending lifecycle callbacks, or mutation events begot by lifecycle callbacks, to persist until the next microtask.)

BUG=234509
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=152982

[dominicc (has gone to gerrit), 2013-05-08 06:07:59]

This assertion is trivially easy to trigger by doing something that queues a
lifecycle callback in a mutation observation.

[dglazkov, 2013-05-08 16:42:00]

Do we need a test for this?

https://codereview.chromium.org/14660019/diff/1/Source/core/dom/CustomElement...
File Source/core/dom/CustomElementRegistry.h (right):

https://codereview.chromium.org/14660019/diff/1/Source/core/dom/CustomElement...
Source/core/dom/CustomElementRegistry.h:101: void
abandonQueuedLifecycleCallbacks();
lifecycle callbacks queue is just begging to be its own class, with all these
methods on it.

[rafaelw, 2013-05-08 18:33:42]

I'd be in favor of just going ahead and abstracting the mutation observers
delivery mechanism into Microtask and making MutationObservers a client of that.

I assume that Lifecycle events will also wants to be suspend-able, the way that
MutationObservers are so they deliver at the same point in code execution
regardless of whether the inspector breaks on a debugger inner-loop.

[dominicc (has gone to gerrit), 2013-05-09 02:10:31]

This was based on abarth's feedback to stabilize the patient before planned
surgery, but just diving into the guts of the thing looks tasty.

That will also make it possible to test this--currently difficult to test
because we want the assertion in there, but that will only crash debug, etc.

I'll keep the issue alive but there will be some sawdust first.

[dominicc (has gone to gerrit), 2013-06-24 13:54:56]

Getting back to this... Dimitri, PTAL

rafaelw: I tried sharing suspending and making an "end of microtask work"
abstraction. It might be workable, but I would like to tackle it when I have an
implementation of other Custom Element callbacks, which will clarify the timing
and ordering of those. They aren't uniform.

For now Custom Element callbacks continue to not support suspension. But I
recognize we must do this.

I also tried adding a "MicrotaskCheckpointWorker" type for polymorphism between
MutationObserver's dispatcher and CustomElementCallbackDispatcher. But it really
wasn't pulling its weight because the performCheckpoint callsite statically
knows who to talk to.

Maybe if there were a lot of expensive steps at the checkpoint the interface
makes sense; perhaps detect we reached a fixpoint invoking fewer steps than we
do now. I think simple is OK for now.

[dglazkov, 2013-06-24 19:20:11]

LGTM.

https://codereview.chromium.org/14660019/diff/11001/Source/core/dom/CustomEle...
File Source/core/dom/CustomElementCallbackDispatcher.h (right):

https://codereview.chromium.org/14660019/diff/11001/Source/core/dom/CustomEle...
Source/core/dom/CustomElementCallbackDispatcher.h:45: static
CustomElementCallbackDispatcher& dispatcher();
instance()?

[rafaelw, 2013-06-24 23:15:49]

lgtm. 

Adam and I talked with Mark Miller last week and have a good starting point for
abstracting this microtask checkpoint to be a general mechanism for
non-mutation-observer clients.

I'll be sending out a strawman shortly.

https://codereview.chromium.org/14660019/diff/11001/Source/core/dom/Microtask...
File Source/core/dom/Microtask.cpp (right):

https://codereview.chromium.org/14660019/diff/11001/Source/core/dom/Microtask...
Source/core/dom/Microtask.cpp:50: workToDo =
CustomElementCallbackDispatcher::dispatcher().dispatch();
Nit: workToDo => anyWorkDone. 

Also, I think it's worth putting a TODO on the while loop that we ultimately
need this to be a FIFO queue.

[dominicc (has gone to gerrit), 2013-06-25 00:45:00]

Thanks dglazkov and rafaelw for review.

On 2013/06/24 23:15:49, rafaelw wrote:
> Adam and I talked with Mark Miller last week and have a good starting point
for
> abstracting this microtask checkpoint to be a general mechanism for
> non-mutation-observer clients.
> 
> I'll be sending out a strawman shortly.

Great! Look forward to reading this.

>
https://codereview.chromium.org/14660019/diff/11001/Source/core/dom/Microtask...
>
> Also, I think it's worth putting a TODO on the while loop that we ultimately
> need this to be a FIFO queue.

I'm not putting a FIXME here, this needs to be thought through more. It is not
clear to me how Custom Elements works with a FIFO given this:

<https://dvcs.w3.org/hg/webcomponents/raw-file/tip/spec/custom/index.html#seri...>

Specifically Step 7, which *prepends* to its queue.

I think having a FIFO that specs mess with (Shotgun! etc.) isn't a sensible
solution. Maybe the FIFO works, but all ready callbacks are one big task and
Custom Elements can mess *just* with the ordering within that task. I guess it
has to group them by document in order to work with suspension. I'd like to see
what you came up with with Mark Miller, see where the Custom Elements spec can
bend (in particular I'm not 100% convinced that it runs ready before
attributeChanged for a given element, but I need to think it through.)

[dominicc (has gone to gerrit), 2013-06-25 00:51:53]

(cont'd)

Also I think Mutation Observers sorts its callbacks by observer creation age?
Guess I'll wait to see your proposal but curious about what the plan is.

[rafaelw1, 2013-06-25 01:00:28]

I wasn't remembering that readyCallbacks want to control their order
as well ;-(. We were wondering if it was worth changing the
MutationObserver delivery ordering in order to avoid *another* inner
priority queue.

Worst case there is a MutationObserverServicer in the queue and a
CustomElementsLifeCycleServicer in the queue. When each runs, it sorts
out it's internal delivery order, dispatches, and when it's done,
re-schedules itself if it already knows there's more work to do.

On Mon, Jun 24, 2013 at 5:51 PM,  <dominicc@chromium.org> wrote:
> (cont'd)
>
> Also I think Mutation Observers sorts its callbacks by observer creation
> age?
> Guess I'll wait to see your proposal but curious about what the plan is.
>
> https://codereview.chromium.org/14660019/

[commit-bot: I haz the power, 2013-06-25 01:49:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dominicc@chromium.org/14660019/18001

[commit-bot: I haz the power, 2013-06-25 03:56:46]

Change committed as 152982