Bug 78290 – Implement end-of-task delivery for MutationObservers

• Component: DOM
• Status: RESOLVED FIXED

[Adam Klein]

For a wide swathe of DOM mutations, end-of-microtask delivery (when the JS stack winds down to zero) is sufficient, since it will handle any DOM mutation made from JavaScript.

But DOM can also be changed through, e.g., user input in a contentEditable element. The layout test fast/mutation/end-of-task-delivery.html exercises just such a case. It passes in Chromium and fails elsewhere, because Chromium delivers mutations not just at the end of every microtask, but also at the end of every run through its main event loop.  From Source/WebKit/chromium/src/WebKit.cpp:

class EndOfTaskRunner : public WebThread::TaskObserver {
public:
    virtual void didProcessTask()
    {
        WebCore::WebKitMutationObserver::deliverAllMutations();
    }
};

...

void initialize(...)
{
    ... 
    s_endOfTaskRunner = new EndOfTaskRunner;
    currentThread->addTaskObserver(s_endOfTaskRunner);
    ...
}

This is "easy" for Chromium because it has its own custom event loop implementation.  This bug is asking for something similar (in behavior, at least) on non-Chromium platforms.

The chief requirement is that, if DOM is mutated in a given task, any mutation observers are notified of that mutation before the next task (e.g., a paint, or a timer scheduled with setTimeout) gets a chance to run.

[Ryosuke Niwa]

Can we just modify RunLoop implementations (e.g. http://trac.webkit.org/browser/trunk/Source/WebCore/platform/mac/RunLoopMac.mm) files for each port ?

[Darin Adler]

(In reply to comment #1)
> Can we just modify RunLoop implementations (e.g. http://trac.webkit.org/browser/trunk/Source/WebCore/platform/mac/RunLoopMac.mm) files for each port ?

That’s used for WebKit2 in all ports that support it, but not in other ports. For example, I don’t think that’s used for Mac WebKit1.

[Ryosuke Niwa]

One alternative to having each port implement this end of task timing is to add some hook in the editing code (e.g. EditCommand::apply, unapply, reapply) to trigger end-of-task delivery given that the editing is the only situation (in addition to modifications due to Objective-C, C++, etc... APIs) in which the end of task delivery is needed.

[Adam Klein]

Created attachment 147915 [details]
WIP Patch

[Adam Klein]

This patch is shockingly simple: it hooks EventDispatcher::dispatchEvent with a counter representing the number of times it's been re-entered. When the counter goes to zero, it checks to see if script is currently executing. If it is, nothing happens (the existing end-of-microtask stuff will take care of handling delivery). But if no script is running, mutations are delivered. This allows, for example, the Mac port to pass fast/mutation/end-of-task-delivery.html.

But I feel like I'm likely missing something by taking this "emulation" approach.

[Adam Barth]

Comment on attachment 147915 [details]
WIP Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=147915&action=review

> Source/WebCore/ChangeLog:11
> +        outermost invocation of EventDispatcher::dispatchEvent. Since editing
> +        (via input events) is the one way to make DOM mutations without
> +        invoking script, ensuring that mutations are delivered after every

What about DOM mutations via the embedder API or via plug-ins?

[Adam Klein]

(In reply to comment #6)
> (From update of attachment 147915 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=147915&action=review
> 
> > Source/WebCore/ChangeLog:11
> > +        outermost invocation of EventDispatcher::dispatchEvent. Since editing
> > +        (via input events) is the one way to make DOM mutations without
> > +        invoking script, ensuring that mutations are delivered after every
> 
> What about DOM mutations via the embedder API or via plug-ins?

Mutations via embedder were definitely the reason we wanted to run this every task, but it occurred to Ryosuke and I that we might be able to either punt or special case those. But plugins seem like a bigger problem, since the embedder won't know what they're doing but the page should be able to find out.

It seems this approach is probably not going to fly, sadly.

[Adam Klein]

Ryosuke, you mentioned you were thinking of taking this on, so unassigning myself in case you want to take ownership.

[Stephen Chenney]

Marked LayoutTest bugs, bugs with Chromium IDs, and some others as WontFix. Test failure bugs still are trackable via TestExpectations or disabled unit tests.

[Adam Klein]

This bug applies to _non_-Chromium platforms

[Stephen Chenney]

Marking WontFix as we have a Chromium bug.

[Adam Klein]

Reopening one more time, sorry for the noise...

[Ryosuke Niwa]

This has definitely been implemented.