Bug 1227444 - "Restore Previous Session" opens duplicate window
Looks like a regression from the CPOW window work I did. Damn it. šŸ˜”

Okay, whatā€™d I do wrong?

Aliceā€™s STR:

Steps to reproduce
1. Start with newly created profile
--- Now, there is single window with 2 tabs( https://www.mozilla.org/en-US/firefox/nightly/firstrun/ and about:home)
2. Close browser
3. Start browser with the profile
4. Perform "Restore Previous Session"

Actual results:
"Restore Previous Session" opens duplicate window.
Now, there is 2 windows with 2 tabs( https://www.mozilla.org/en-US/firefox/nightly/firstrun/ and about:home)

Expected results:
"Restore Previous Session" should open only previous window.

I was able to reproduce this on olā€™ slowy mc-laptop.

So hereā€™s what I think is happening.

  1. Browsers havenā€™t flushed yet
  2. We ask for quit, which causes a sync flush of all windows
  3. Finish prompting, and confirm that weā€™re going down
  4. Window closes, which starts async flush
  5. Process starts to exit

...

Okay, I see whatā€™s really going on.

Quit application is requested, and we do our TabState flush and save state with our two tabs for the window.
We then close the windows, and start an async window flush, which will put the window in both the _closedWindows set, and in the _windows set, since we donā€™t remove it from _windows until the flush is complete.
We do a final write to the disk in that state, with the same window in _windows and _closedWindow.
By the time the TabStateFlusher flushWindow Promise resolves, weā€™ve already done our final write to the disk, and the party is over.

I think we can safely remove the window from this._windows in the onClose method, and not within the Promise resolution.

Patch seems to work! Posted for review.

Bah, backed out because I actually broke some stuff. Luckily we had a test catch it. I need to make sure that we calculate whether or not the window is the ā€œlast to closeā€ before the window actually closes.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e02c0cd0bf74

Going to try to hack the tests from bug 1226333 into here so that we can get some coverage and this doesnā€™t happen again.

Those tests

182 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_async_window_flushing.js | We should have added the window to the closed windows array - Got 1, expected 2

Try push with tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e865770694b2