If the window that is prompting is closed we don't currently get any callback from the prompt. That leaves our threads in a nasty state. Attached patch fixes this by watching dom window destruction. Also bulletproofs by allowing the shutdown timer to force a response. This patch needs bug 595253 applied.

This needs fixed before we ship IMO.

Carrying sicking's r+ forward, needs a slightly new patch based on changes in bug 595253.

Comment on attachment 477329 [details] [diff] [review] Patch, v1.1 Gavin, can you look over the js changes?

Gavin, review ping!

I landed this by accident here: http://hg.mozilla.org/mozilla-central/rev/1e456c2f2c8f And then removed the unreviewed JS changes here: http://hg.mozilla.org/mozilla-central/rev/23a389589086 Sorry about that.

Comment on attachment 477329 [details] [diff] [review] Patch, v1.1 >diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js >+ else if (topic == this._quotaCancel) { >+ responseTopic = this._quotaResponse; >+ } This doesn't look like it serves a purpose, since you don't notify for quotaCancel. Seems like this might be less confusing if you handled quotaCancel separately rather than trying to share the logic with the prompting in observe(). Not closing over the timeout ID means you couldn't cancel it, but it should be able to deal with running after the notification's already been closed.

(In reply to comment #6) > This doesn't look like it serves a purpose, since you don't notify for > quotaCancel. Yes, I do. timeoutNotification() does the call to the observer, with the responseTopic that I have set.

Comment on attachment 477329 [details] [diff] [review] Patch, v1.1 Gavin, can you look over this again? I think it works as it should. Maybe I just need more comments?

Comment on attachment 477329 [details] [diff] [review] Patch, v1.1 Reviewed this in bug 595253.

Let's call this fixed then.