Bug 1284687 - WebExtension page causes Firefox shutdown to hang
This is a minor tragedy, and I feel pretty responsible.
In bug 1177310 , I wrote patches that got rid of CPOW usage on application shutdown. Instead of causing flushes using CPOWs, I use the AsyncShutdown JSM to spin the event loop until each windows browsers had flushed their message queues.
As a perceived shutdown performance optimization, I added a line that closed the window after the flush messages go down to each browser.
This... this isn't perfect, and this WebExtension is exposing that.
The problem is that the onClose handler - which fires in response to the domwindowclosed notification - will likely fire before all of the updates have come up from each browser. onClose, when we're shutting down, will call cleanUpWindow, which resolves all of the in-flight flush Promises for each of the browsers still in the window and then removing all of the message listeners.
This is problematic for two reasons:
1) It's very likely that we haven't heard all of the browser updates by the time that onClose fires, so we're probably missing out on the last ~2 seconds of session state from the unflushed browsers
2) If something (like the WebExtension in this bug) removes a browser from a window once SessionStore has kicked off its async flush, but _before_ domwindowclosed has fired, then when cleanUpWindow is called, it'll resolve all browsers except the one that had been removed, and the async window flush will never finish because we'll never have resolved the one that was removed.
To me, the simplest solution is to not cause onClose to happen until the window flushes have completed. Instead of calling win.close after each flush message is sent, I set its visibility to false.
In bug 1177310 , I wrote patches that got rid of CPOW usage on application shutdown. Instead of causing flushes using CPOWs, I use the AsyncShutdown JSM to spin the event loop until each windows browsers had flushed their message queues.
As a perceived shutdown performance optimization, I added a line that closed the window after the flush messages go down to each browser.
This... this isn't perfect, and this WebExtension is exposing that.
The problem is that the onClose handler - which fires in response to the domwindowclosed notification - will likely fire before all of the updates have come up from each browser. onClose, when we're shutting down, will call cleanUpWindow, which resolves all of the in-flight flush Promises for each of the browsers still in the window and then removing all of the message listeners.
This is problematic for two reasons:
1) It's very likely that we haven't heard all of the browser updates by the time that onClose fires, so we're probably missing out on the last ~2 seconds of session state from the unflushed browsers
2) If something (like the WebExtension in this bug) removes a browser from a window once SessionStore has kicked off its async flush, but _before_ domwindowclosed has fired, then when cleanUpWindow is called, it'll resolve all browsers except the one that had been removed, and the async window flush will never finish because we'll never have resolved the one that was removed.
To me, the simplest solution is to not cause onClose to happen until the window flushes have completed. Instead of calling win.close after each flush message is sent, I set its visibility to false.
This solves the WebExtension case because when the WebExtension removes the tab, we'll hear the final update come up from it, which will resolve the async window flush Promise (whereas before we never had a chance to hear that final update message because cleanUpWindow removed the message listeners).
TODO:
File that follow-up bug about the potential for windows to be closed between quit-application-granted and the flush.
bug 1286077