Bug 78290

Summary: Implement end-of-task delivery for MutationObservers
Product: WebKit Reporter: Adam Klein <adamk>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, darin, efidler, haraken, japhet, jochen, mjs, mrobinson, ojan, rafaelw, rniwa, sam, schenney, syoichi, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 68729, 85161    
Attachments:
Description Flags
WIP Patch none

Description Adam Klein 2012-02-09 15:23:50 PST
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.
Comment 1 Ryosuke Niwa 2012-05-26 09:43:56 PDT
Can we just modify RunLoop implementations (e.g. http://trac.webkit.org/browser/trunk/Source/WebCore/platform/mac/RunLoopMac.mm) files for each port ?
Comment 2 Darin Adler 2012-05-26 16:57:08 PDT
(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.
Comment 3 Ryosuke Niwa 2012-06-15 13:33:28 PDT
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.
Comment 4 Adam Klein 2012-06-15 15:42:21 PDT
Created attachment 147915 [details]
WIP Patch
Comment 5 Adam Klein 2012-06-15 16:12:50 PDT
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.
Comment 6 Adam Barth 2012-06-15 20:31:54 PDT
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?
Comment 7 Adam Klein 2012-06-16 10:02:58 PDT
(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.
Comment 8 Adam Klein 2013-01-08 14:45:04 PST
Ryosuke, you mentioned you were thinking of taking this on, so unassigning myself in case you want to take ownership.
Comment 9 Stephen Chenney 2013-04-09 17:05:27 PDT
Marked LayoutTest bugs, bugs with Chromium IDs, and some others as WontFix. Test failure bugs still are trackable via TestExpectations or disabled unit tests.
Comment 10 Adam Klein 2013-04-09 17:06:39 PDT
This bug applies to _non_-Chromium platforms
Comment 11 Stephen Chenney 2013-04-09 18:04:13 PDT
Marking WontFix as we have a Chromium bug.
Comment 12 Adam Klein 2013-04-09 18:09:18 PDT
Reopening one more time, sorry for the noise...
Comment 13 Ryosuke Niwa 2016-04-11 01:37:52 PDT
This has definitely been implemented.