Comment 297 for bug 195698

Kai thanks for looking at this. To answer:

- What the PromptAggregator class is for:
Each instance of that class is bound to each prompt request. The first instance (and therefor the first prompt request with it) is saved in the mPasswordPromptAggregator member pointer. Any other instance (so, any other prompt request) checks that member is non-null, if so, it spins the event loop and waits for the first dialog to finish (WaitAndGrab method). After it is finished (the first prompt's chrome window has been unregistered) the class gets (grabs) return values (e.g. entered credentials) from the first dialog param block and immediately leaves without invoking the same dialog again.

- The test with few tabs and another window:
I tired that (on win machine). It doesn't pop-up another dialog after the first one is closed. But w/o this patch when I quit from the other window, all dialogs are closed and Firefox quits w/o waiting for dialogs to be closed by user, immediately. With this patch it waits for the prompt. We could override this by handling also some of the "quit" observer events to wake up the aggregates.

- mUserAndPasswordPromptAggregator never used:
Leftover, we can have one member for each type of a prompt or use one member to block/stack all types of dialogs.

- Wake up after unlock:
We could potentially unlock other aggregates by an event posted to the main thread from "domwindowclosed" observer event. True is we don't guarantee in any way to loop the spin. It is just highly probable it will loop.

- Multi-threading:
It seems nsPromptService is not designed as to be thread safe at all. As I've seen the code it is always used through a proxy where invoked from other threads. So I would not bother with any locks.

- GetString returns a copy:
Good catch, I wasn't aware of that. Have to release it.

I'll create a patch with fixes you propose, and with larger context.