820411 - Eliminate NS_ProcessNextEvent outside main message pumps

Status: NEW

(Core :: DOM: Core & HTML, defect, P5)

Description

[Gian-Carlo Pascutto [:gcp]]

Related to bug 715376.

There's quite a bit of code calling NS_ProcessNextEvent in a loop, outside the main event loop/pump:
http://mxr.mozilla.org/mozilla-central/ident?i=NS_ProcessNextEvent&filter=

This breaks various things (see, for example, bug 715376).

Comment 1

[(no longer active)]

Can you please provide more details on what is broken by this and how you plan to fix it?  Is it just a general bug about nested event queues?

Comment 2

[Gian-Carlo Pascutto [:gcp]]

My understanding is that there's a bunch of code that does some operation, and then waits for something to happen (the result of that operation) by spinning the event loop itself and checking for status changes. This will fail when the event loop being spun isn't the one containing that result, which becomes possible as of bug 715376.

Comment 3

[Nathan Froyd [:froydnj]]

(In reply to Gian-Carlo Pascutto (:gcp) from comment #2)
> My understanding is that there's a bunch of code that does some operation,
> and then waits for something to happen (the result of that operation) by
> spinning the event loop itself and checking for status changes. This will
> fail when the event loop being spun isn't the one containing that result,
> which becomes possible as of bug 715376.

More correctly, the code for bug 715736 contains a hack to work around other places spinning the event loop.  That hack could go away if we eliminated nested event loops elsewhere or if we made the code in bug 715376 more deeply integrated with the event dispatching system.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

In the main thread the only valid use for nested event loop spinning is window.showModalDialog.
But without e10s we can't really fix sync XHR nor alert() etc.

The mxr query in comment 0 includes also other than main thread. Spinning event loop in those ones
may not be a problem at all.

Comment 5

[(no longer active)]

Also note that Javascript can spin the event loop as well.  I know at least one component which does this (sync).

Comment 6

[Karl Tomlinson (:karlt)]

(In reply to Olli Pettay [:smaug] from comment #4)
> In the main thread the only valid use for nested event loop spinning is
> window.showModalDialog.

We can go further to say that use case is only valid if the window is really modal wrt to all windows using the same stack.  (Even in this case, a nested event loop is not necessary.)  On some platforms the window is only modal wrt one another window, which leads to the same problems as comment 2 and already happens without bug 715376.

(In reply to Olli Pettay [:smaug] from comment #4)
> But without e10s we can't really fix sync XHR nor alert() etc.

In the meantime, perhaps it would be worthwhile making alerts in background tabs post messages and return immediately, and dismissing alerts in foreground tabs when focus is received by another window using the same stack/thread.

Or perhaps all alerts should be displayed in the focused window (regardless of the tab to which they originate), so they can be dismissed by the user in the "right" (necessary due to application/JS design) order.

Don't know any other solutions beyond e10s for sync XHR.
Bug 384412 has been abandoned.  At least sync XHR times out eventually, I assume.
Unfortunately they'll time out in the "wrong" order, so stack growth is unbounded with multiple XHR requests.  The half-sane solution is to block the rest of the browser, but that doesn't win friends.

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

It just occurred to me that bug 715376 (which initiated this discussion) may mitigate some of these problems (with a little additional re-architecting) as a model dialog only needs to block events from it's document's event queue. We could have "modal" runnables return NS_ERROR_MODAL which will cause the event loop to stop processing events from it's document's queue until another "special" event is posted to the main loop to resume those.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

modal dialog would need to block also events coming from necko.
var x = new XMLHttpRequest();
x.open("GET", "http://www.foobar.com", true /* async*/);
x.onprogress = function progress() { /* do something */}
x.send();
alert("hello world"); // progress() shouldn't be called while alert() is shown.

Comment 9

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven't been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.