[Mac] media/track/audio-track.html is flakey
Created attachment 241010 [details] Patch
Anders, is there an existing message queue class that should be used instead? I feel like we have too many. MessageQueue? Document::postTask?
(In reply to comment #2) > Anders, is there an existing message queue class that should be used > instead? I feel like we have too many. MessageQueue? MessageQueue isn't really for Events, but more for cross-thread messaging. > Document::postTask? I don't think we should re-use Document::postTask() for this. It does not guarantee ordering across ScriptExecutionContexts. Ideally, Sam's work to add official task queues <http://www.w3.org/TR/html5/webappapis.html#task-queue> would add a specific Event task queue, which would replace GenericEventQueue, and would be tied to a Browsing Context. I can add a FIXME to this effect.
(In reply to comment #3) > Ideally, Sam's work to add official task queues > <http://www.w3.org/TR/html5/webappapis.html#task-queue> would add a specific > Event task queue, which would replace GenericEventQueue, and would be tied > to a Browsing Context. In fact, GenericEventQueue could become the basis for the Event Task Queue.
> I don't think we should re-use Document::postTask() for this. It does not guarantee ordering across ScriptExecutionContexts. This actually sounds like a good thing - documents are isolated, and adding a shared event queue would be a side channel for XSS. It obviously wouldn't be the biggest one, so it's not a show stopper in any way, however it surprises me if that's a desirable behavior.
(In reply to comment #5) > > I don't think we should re-use Document::postTask() for this. It does not guarantee ordering across ScriptExecutionContexts. > > This actually sounds like a good thing - documents are isolated, and adding > a shared event queue would be a side channel for XSS. It obviously wouldn't > be the biggest one, so it's not a show stopper in any way, however it > surprises me if that's a desirable behavior. Hm, maybe.
(In reply to comment #6) > (In reply to comment #5) > > > I don't think we should re-use Document::postTask() for this. It does not guarantee ordering across ScriptExecutionContexts. > > > > This actually sounds like a good thing - documents are isolated, and adding > > a shared event queue would be a side channel for XSS. It obviously wouldn't > > be the biggest one, so it's not a show stopper in any way, however it > > surprises me if that's a desirable behavior. > > Hm, maybe. Actually, the spec specifies a minimum of one event loop (and Event task queue) per User Agent, and _allows_ for one event loop per Browsing Context, but does not require it. So it may be a good idea to segregate event loops on a Browsing Context (i.e. Document) basis, it's not necessary according to the spec. <http://www.w3.org/TR/html5/webappapis.html#event-loops>
(In reply to comment #7) > (In reply to comment #6) > > (In reply to comment #5) > > > > I don't think we should re-use Document::postTask() for this. It does not guarantee ordering across ScriptExecutionContexts. > > > > > > This actually sounds like a good thing - documents are isolated, and adding > > > a shared event queue would be a side channel for XSS. It obviously wouldn't > > > be the biggest one, so it's not a show stopper in any way, however it > > > surprises me if that's a desirable behavior. > > > > Hm, maybe. > > Actually, the spec specifies a minimum of one event loop (and Event task > queue) per User Agent, and _allows_ for one event loop per Browsing Context, > but does not require it. > > So it may be a good idea to segregate event loops on a Browsing Context > (i.e. Document) basis, it's not necessary according to the spec. > > <http://www.w3.org/TR/html5/webappapis.html#event-loops> And, under a close reading of the spec, at a minimum Events must be dispatched in enqueued order across same-origin Browsing Contexts.
Created attachment 241308 [details] Patch
Comment on attachment 241308 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=241308&action=review > Source/WebCore/dom/GenericEventQueue.cpp:33 > +#include <wtf/text/CString.h> Is this include needed? > Source/WebCore/dom/GenericEventQueue.cpp:66 > + static NeverDestroyed<Timer> timer(GenericEventQueue::sharedTimerFired); Please add an ASSERT(isMainThread()) here. The last thing we need is for this to get used from a secondary thread. > Source/WebCore/dom/GenericEventQueue.cpp:88 > + static NeverDestroyed<Deque<WeakPtr<GenericEventQueue>>> queues; Please add an ASSERT(isMainThread()) here, too.
Comment on attachment 241308 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=241308&action=review >> Source/WebCore/dom/GenericEventQueue.cpp:33 >> +#include <wtf/text/CString.h> > > Is this include needed? No, it's not. I had logging in this file previously, but removed it before uploading. I'll remove this include as well. >> Source/WebCore/dom/GenericEventQueue.cpp:66 >> + static NeverDestroyed<Timer> timer(GenericEventQueue::sharedTimerFired); > > Please add an ASSERT(isMainThread()) here. The last thing we need is for this to get used from a secondary thread. Ok. >> Source/WebCore/dom/GenericEventQueue.cpp:88 >> + static NeverDestroyed<Deque<WeakPtr<GenericEventQueue>>> queues; > > Please add an ASSERT(isMainThread()) here, too. Ok.
Committed r176024: <http://trac.webkit.org/changeset/176024>