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:
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.
- Browsers havenāt flushed yet
- We ask for quit, which causes a sync flush of all windows
- Finish prompting, and confirm that weāre going down
- Window closes, which starts async flush
- 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.
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