Open Bug 677424 Opened 8 years ago Updated 4 years ago

If a tab is closed between quit-application-requested and quit-application-granted then it is still restored when the application restarts

Categories

(Firefox :: Session Restore, defect, minor)

defect
Not set
minor

Tracking

()

REOPENED
Firefox 8

People

(Reporter: mossop, Unassigned)

References

Details

Attachments

(1 file, 1 obsolete file)

In bug 476430 I have an in-content UI that wants to close itself and restart the application. It sends quit-application-requested first because it doesn't want to close itself if the request is denied, then it closes itself, then it restarts the application which sends quit-application-granted.

Session store gathers its last set of data in quit-application-granted and so it ignores the fact that a tab goes away immediately after that
Attached patch Patch v0.1 (obsolete) — Splinter Review
The alternative to this felt to focused on the case presented here so this is a bit more general purpose (I was going to delete the tab from window.tabs and adjust window.selected in onTabRemove).

I think this will work, but no test yet (well, Dave can say if it works). If I write a test that notifies quit-application-requested, is that going to mess things up? Actually I'm not sure we can test this properly since we really need to be quitting :/
Assignee: nobody → paul
Status: NEW → ASSIGNED
If any code breaks after receiving multiple quit-application-requested notifications then that code is itself broken
(In reply to Dave Townsend (:Mossop) from comment #2)
> If any code breaks after receiving multiple quit-application-requested
> notifications then that code is itself broken

I wasn't worried so much about that, just wasn't sure if it would trigger anything (eg. the quit dialog) that wasn't something we could pref off.
Oh, good point. Yes it is bound to do that in some cases, but you can handle that in the test.

To really handle the shutdown/restart properly we probably need mozmill tests for session restore, maybe some even already exist
Attached patch patch rev 1Splinter Review
The previous patch doesn't work because the tab is still in tabbrowsers list of tabs at the time the TabClose event is dispatched. This works by removing the tabs data from the cached window state when it is removed.
Assignee: paul → dtownsend
Attachment #551649 - Attachment is obsolete: true
Attachment #552242 - Flags: review?(paul)
Comment on attachment 552242 [details] [diff] [review]
patch rev 1

This is going to make the e10s work a little bit more interesting with the currentURI check, but that problem is for future-Paul/Jez to figure out. This will need tweaking when app tabs happen like I want them too (but that's just a comment for me to remember).
Attachment #552242 - Flags: review?(paul) → review+
Landed: http://hg.mozilla.org/mozilla-central/rev/d980e7a3aa49
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Firefox 8
Backed out due to test failures (timeout in browser_500328.js on OSX debug builds)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee: dtownsend → nobody
You need to log in before you can comment on or make changes to this bug.