Last Comment Bug 686065 - Don't clear nsSessionStartup::sessionType after the session startup phase finished
: Don't clear nsSessionStartup::sessionType after the session startup phase fin...
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Session Restore (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 10
Assigned To: :Paolo Amadini [Away]
:
Mentors:
Depends on: 748516 588506
Blocks: 698276 DownloadsPanel
  Show dependency treegraph
 
Reported: 2011-09-09 17:22 PDT by :Paolo Amadini [Away]
Modified: 2012-04-24 18:03 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
The patch (2.39 KB, patch)
2011-09-09 17:22 PDT, :Paolo Amadini [Away]
paul: review+
Details | Diff | Review

Description :Paolo Amadini [Away] 2011-09-09 17:22:04 PDT
Created attachment 559631 [details] [diff] [review]
The patch

For the new Downloads panel, having sessionType available after the session
startup phase finished can simplify some of the session restore code.
Comment 1 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-09-14 12:03:58 PDT
Comment on attachment 559631 [details] [diff] [review]
The patch

Review of attachment 559631 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. Thanks!
Comment 2 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-10-25 15:13:57 PDT
Landed on fx-team: https://hg.mozilla.org/integration/fx-team/rev/40e4b0f6174d
Comment 3 :Margaret Leibovic 2011-10-27 11:36:16 PDT
https://hg.mozilla.org/mozilla-central/rev/40e4b0f6174d
Comment 4 neil@parkwaycc.co.uk 2011-11-01 10:46:07 PDT
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 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-11-17 14:13:47 PST
(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 neil@parkwaycc.co.uk 2011-11-26 15:08:30 PST
(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 ;-)

Note You need to log in before you can comment on or make changes to this bug.