Closed
Bug 686065
Opened 13 years ago
Closed 13 years ago
Don't clear nsSessionStartup::sessionType after the session startup phase finished
Categories
(Firefox :: Session Restore, defect)
Firefox
Session Restore
Tracking
()
RESOLVED
FIXED
Firefox 10
People
(Reporter: Paolo, Assigned: Paolo)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
Attachments
(1 file)
2.39 KB,
patch
|
zpao
:
review+
|
Details | Diff | Splinter Review |
For the new Downloads panel, having sessionType available after the session startup phase finished can simplify some of the session restore code.
Attachment #559631 -
Flags: review?(paul)
Comment 1•13 years ago
|
||
Comment on attachment 559631 [details] [diff] [review] The patch Review of attachment 559631 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. Thanks!
Attachment #559631 -
Flags: review?(paul) → review+
Comment 2•13 years ago
|
||
Landed on fx-team: https://hg.mozilla.org/integration/fx-team/rev/40e4b0f6174d
Whiteboard: [fixed-in-fx-team]
Comment 3•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/40e4b0f6174d
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 10
Comment 4•13 years ago
|
||
Comment on attachment 559631 [details] [diff] [review] The patch >@@ -172,16 +172,19 @@ SessionStartup.prototype = { [init: function sss_init() {] >+ if (this._sessionType != Ci.nsISessionStartup.NO_SESSION) >+ Services.obs.addObserver(this, "browser:purge-session-history", true); So, we have init() adding the browser:purge-session-history observer. >@@ -192,29 +195,35 @@ SessionStartup.prototype = { [case "final-ui-startup":] > Services.obs.removeObserver(this, "final-ui-startup"); > Services.obs.removeObserver(this, "quit-application"); > this.init(); And we have final-ui-startup calling init() adding the observer, but also removing the quit-application observer. > case "quit-application": > // no reason for initializing at this point (cf. bug 409115) > Services.obs.removeObserver(this, "final-ui-startup"); > Services.obs.removeObserver(this, "quit-application"); >+ if (this._sessionType != Ci.nsISessionStartup.NO_SESSION) >+ Services.obs.removeObserver(this, "browser:purge-session-history"); So, if the quit-application observer gets hit, this means that the final-ui-startup hasn't run, and hasn't called init, and hasn't added the browser:purge-session-history observer, so there's nothing to remove. In fact you'll find that the sessionType can only ever be NO_SESSION at this point.
Comment 5•13 years ago
|
||
(In reply to neil@parkwaycc.co.uk from comment #4) > Comment on attachment 559631 [details] [diff] [review] [diff] [details] [review] True story. So the correct way to do this would be to just put back in what was taken out in bug 588506. Neil, want to file a new bug/write the patch or are you just following up for the SM patch?
Comment 6•13 years ago
|
||
(In reply to Paul O'Shannessy from comment #5) > So the correct way to do this would be to just put back in what > was taken out in bug 588506. > > Neil, want to file a new bug/write the patch or are you just following up > for the SM patch? I was just trying to understand Misak's port. I'm not actually sure what it is you're trying to say, so perhaps it would be better if I wait for the code ;-)
You need to log in
before you can comment on or make changes to this bug.
Description
•