873292 - Add NS_DispatchToMainThreadWhenSafeToRunScript

Status: NEW

(Core :: XPCOM, defect)

Description

[(no longer active)]

The idea is to run the runnable argument only when it's safe to run scripts, to prevent idioms such as <http://hg.mozilla.org/mozilla-central/annotate/55e790d0ff02/content/media/webaudio/ScriptProcessorNode.cpp#l294> being necessary.

One naive way to implement this would be to wrap the runnable argument in another runnable which does that check.  Perhaps that's not the best way though.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

One way would be to have an nsScriptRunnable subclass of nsRunnable that overrides Run() to check if it's safe to run scripts, and if is calls a new method RunScript() that subclasses override.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Uh...  how can we possibly end up in a situation where a thread runnable is running when it's _not_ safe to run script?  That should never happen, since thread runnables can include things like setTimeout firing, which can obviously run script and does no such checks.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

during the nested event loop of an alert(), according to Ehsan?

Although since alerts are tab-modal I thought we'd still be running script in general.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Yeah, during an alert IsSafeToRunScript should be true...

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

That's what I thought, but Ehsan assured me it wasn't!

Comment 6

[Benjamin Smedberg]

I am very confused. If we ever end up in a situation where it's not safe to run script from a nested event loop, we've probably lost in general.

Comment 7

[(no longer active)]

Hmm, I have some code in ScriptProcessorNode which dispatches a runnable to the main thread in order to be able to dispatch a DOM event there.  While testing that code, I was using alert() to print out some of the data attached to the event, and it was then when I hit this assertion: <http://mxr.mozilla.org/mozilla-central/source/content/events/src/nsEventDispatcher.cpp#494>.  Unfortunately I don't have a stack right now, and I tried to quickly reproduce it but I couldn't (and I'm not sure what my test case looked like back then exactly.)