[Webkit-unassigned] [Bug 137496] Add a microtask abstraction

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=137496





--- Comment #7 from Yoav Weiss <yoav at yoav.ws>  2014-10-10 00:27:08 PST ---
(In reply to comment #4)
> (From update of attachment 239430 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=239430&action=review
> 
> > Source/WebCore/dom/MicroTask.h:28
> > +class MicroTask : public RefCounted<MicroTask> {
> 
> Why do these need to be reference counted? Can we use std::unique_ptr instead and use single ownership?

You're right, they don't need to be. I removed it.

> 
> > Source/WebCore/dom/MicroTaskQueue.h:31
> > +class MicroTaskQueue : public RefCounted<MicroTaskQueue> {
> 
> I don’t think we should use a reference-counted object just to hold a Vector. We should simply have the vector of tasks in Document itself. This level of indirection doesn’t help us. No need to have a class for this.

Removed.

> 
> > Source/WebCore/dom/MicroTaskQueue.h:42
> > +        for (auto task : m_tasks)
> > +            task->run();
> > +        m_tasks.clear();
> 
> What prevents this from being reentered, running the same task more than once?

I assumed the microtask queue would be accessed only from the main thread. I've added ASSERTs to make sure this assumption stands.

(In reply to comment #5)
> (From update of attachment 239430 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=239430&action=review
> 
> > Source/WebCore/dom/ScriptRunner.cpp:121
> > +    m_document.runMicroTasks();
> 
> What guarantees we aren’t already executing a script here?

I've interpreted that code as the code running HTMLScriptElement's code one by one in the loop above my code addition. Therefore, I didn't think such protection is necessary.
It's highly probable that I was wrong about that though. If that's the case, I'd appreciate guidance at to how I can add the required protection here.

> 
> > Source/WebCore/html/parser/HTMLScriptRunner.cpp:236
> > +    m_document->runMicroTasks();
> 
> What guarantees we aren’t already inside a script here?

I've now protected against this. Thanks for catching that.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.