+++ This bug was initially created as a clone of Bug #1451985 +++ Cloning this off to track the performance around closing the page. STR: 1. Open a shared dropbox link like this: https://www.dropbox.com/s/21kyjktup1omju9/ghost-cat.jpg?dl=0 2. Close the tab. 3. Observe that it takes many seconds for the tab to close.

Looking at a profile here, it seems maybe it's a long beforeunload handler being run.

I know we made some changes in the past to make tab closing visually fast but run the beforeunload handlers in the background. Mike, is that not working as intended here?

(In reply to Andrew Overholt [:overholt] from comment #2) > I know we made some changes in the past to make tab closing visually fast > but run the beforeunload handlers in the background. Mike, is that not > working as intended here? What we do is skip sending a message to the content to run the beforeunload event handlers if we know no beforeunload event handler was set. If, however, content _has_ set a beforeunload event handler, we send it a message, and we wait for a maximum of 1s for it to complete running that function before bailing out: https://searchfox.org/mozilla-central/rev/2466b82b729765fb0a3ab62f812c1a96a7362478/toolkit/content/widgets/remote-browser.xml#321 I'm not certain why it'd take many seconds - it's supposed to take a maximum of 1s. tbh, we might want to consider ratcheting that timeout down even further. bkelly, are you still seeing it take many seconds?

I see it taking perhaps 4 seconds on this over-the-forest-under-the-lake network connection.

There is sync XHR running inside beforeunload listener.

Oh yikes - it looks like we've got code in here that intentionally disables the timeout if the content process has responded: https://searchfox.org/mozilla-central/rev/2466b82b729765fb0a3ab62f812c1a96a7362478/toolkit/content/widgets/remote-browser.xml#335 We should probably re-evaluate that.

Sync XHR is probably one of the most common time consuming operations happening during beforeunload. We should probably hide the tab rather soon if we detect sync XHR, not kill the tab, but hide it.

(In reply to Olli Pettay [:smaug] from comment #8) > Sync XHR is probably one of the most common time consuming operations > happening during beforeunload. > We should probably hide the tab rather soon if we detect sync XHR, not kill > the tab, but hide it. I think we've considered it in the past. As I recall, the concern was "what do we do if the sync XHR finishes, and then beforeunload returns a signal saying to not permit the unload"? Presumably, in that case, the tab has to re-appear, and I think we never found a great way of doing that which wasn't jarring for the user. But it's definitely worth revisiting.

Tentatively setting fxperf:p2 . I think this would be good to fix but also quite hairy (having the tab re-appear if a prompt is required, and more generally how to detect we need to reshow the tab - e.g. what if a permissions request fires?). Other notes: I also think this is kind of a dupe of bug 1103323, and I can't reproduce with the link from comment #0 (404).

Another thing we might reconsider is skipping the timeout if content has responded. Maybe we should just have a 1sec (or 500ms?) cap, no matter what. This, of course, risks data loss on pages that are trying to save things with onbeforeunload...

Would anyone have time to test other browsers? Some simple test case using sync XHR during beforeunload and a slow response from network.