497482 - [Mac] browser.sessionstore.resume_session_once not being set when user chooses "Save and Quit" on reboot / restart

Status: VERIFIED FIXED

(Firefox :: Session Restore, defect)

Description

[u279076]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5.7; en-US; rv; 1.9.1pre) Gecko/20090610 Shiretoko/3.5pre

In accordance with the test case (see URL), I cannot restore a session from restarting the OS.

STR:
   1. Start Firefox with a new profile
   2. Open the Preferences dialog
   3. On the Main page, under Startup, make sure "When Firefox starts" is set to "Show my home page"
   4. Close the Preferences dialog
   5. Have three tabs open (google.com, cnn.com, digg.com)
   6. Restart your Operating System
   7. Select "Save and Quit" if prompted to do so.
   8. Start Firefox when your operating system restarts

RESULT:
I see the default start-up page.

EXPECTED:
My session should be restored.


I'm also seeing the following error in Error Console:

Error: Warning: unrecognized command line flag -foreground
Source File: Shiretoko.app/Contents/MacOS/components/nsBrowserContentHandler.js (Line 708)

NOTE: I see this on Mac OSX 10.5 as well (not just the one reported).  Windows and Linux both restore the session after restarting the OS.

Comment 1

[Dietrich Ayala (:dietrich)]

I'd like to get more confirmation, but asking to block on this for investigation due to the dataloss.

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

Yes, definitely need to confirm.

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

Where is the -foreground command line flag coming from?

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

<span id="potential-red-herring">

The only relevant looking reference to -foreground I found was:

http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsCommandLineServiceMac.cpp#173

and the last time that was touched was:

Bug 486733 - optimize/modernize Mac OS X toolkit/xre code

which was around April.

Josh, Markus: any chance that's related?

</span>

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

Bah; ignore comment 4, that change is mozilla-central only :(

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

(Dietrich: fwiw, I can confirm that it happens on OSX, not on w32)

Comment 7

[Nochum Sossonko [:Natch]]

-foregoud is an os command, and Firefox doesn't really support it. There's another bug that shows similar symptoms, can't find it atm, but it screws up startup and a bunch of other stuff.

Reporter: can you set javascript.options.showInConsole to true and copy the chrome errors that you get (if you do get any) in this bug?

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

ashughes: you finding this on trunk as well as branch? how about 3.0.x? any chance it's an event-notification-change in OSX?

Comment 9

[Nochum Sossonko [:Natch]]

Bug 462760 is what I was talking about, similar symptoms with a resolution/explanation. Specifically, see bug 462760 comment 3 for a detailed explanation of what's going on.

Comment 10

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: my sessionstore.js

Here's my sessionstore.js (FWIW). This was from a couple-day old branch nightly, but nothing changed in sessionstore, so that shouldn't matter.

Note: we are saving this file, so I don't think that's the problem, but we'd be saving that anyway. We might not be setting the browser.sessionstore.resume_session_once pref to true (which would happen here I think: http://mxr.mozilla.org/mozilla-central/source/browser/components/nsBrowserGlue.js#140)

From a quick glance the file looks ok. I can inspect more tomorrow - throw some debugging code into sessionstore and also check if it's a pref setting problem.

(In reply to comment #7)
> Reporter: can you set javascript.options.showInConsole to true and copy the
> chrome errors that you get (if you do get any) in this bug?

Nothing there.

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

re: comment 9 - I don't think that's at all related, actually. Josh has convinced me that the -foreground thing is more likely than not a red herring.

Paul: any way to check the value of browser.sessionstore.resume_session_once as we exit? I suppose I could look in prefs.js before I re-open Firefox, right?

Comment 12

[Josh Aas]

I can reproduce this with the latest trunk nightly.

Note that you can't reproduce this unless in the "Tabs" preference pane you have "Warn me when closing multiple tabs" checked, otherwise you won't get the save dialog at all.

Comment 13

[Josh Aas]

I can also reproduce this with Firefox 3.5 Preview, so both trunk and 1.9.1 have this problem.

Comment 14

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to comment #11)
> Paul: any way to check the value of browser.sessionstore.resume_session_once as
> we exit? I suppose I could look in prefs.js before I re-open Firefox, right?

Yea, you should be able to look in prefs.js.

Comment 15

[Josh Aas]

"browser.sessionstore.resume_session_once" is in fact false after the reboot when the browser fails to restore. I added some debug code to print that value when it is read at launch in the session restore code.

Comment 16

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #11)
> Paul: any way to check the value of browser.sessionstore.resume_session_once as
> we exit? I suppose I could look in prefs.js before I re-open Firefox, right?

I tried to look into this in two ways:

1. I opened an about:config tab, filtered to this pref, and watched it as I clicked "save and quit"

2. I checked the prefs.js file before re-opening the browser

In OSX, the pref didn't flip before the browser quit and there was no write to prefs.js

In Windows, the pref visibly flipped right before the browser quit, and indeed, prefs.js contained the write-out.

I think we have our culprit; but why isn't it writing on OSX?

Comment 17

[Nochum Sossonko [:Natch]]

I'd venture a guess that bug 477934 *might* have affected this. I'll stop spamming now...

Comment 18

[Mike Beltzner [:beltzner, not reading bugmail]]

Shawn/Biesi: can you look and see if this is related to bug 477934?

Comment 19

[Mike Beltzner [:beltzner, not reading bugmail]]

Are we writing any just-before-exit-when-rebooting preference changes to disk? Or are we just missing this specific one?

Comment 20

[Markus Stange [:mstange]]

Steven, does restarting the system terminate the app the same way as choosing Quit from the Dock? If so, is this the same bug as bug 491122?
(And if it is, then this is not a regression from 3.0)

Comment 21

[Mike Beltzner [:beltzner, not reading bugmail]]

FWIW, I couldn't reproduce this with 3.0.x

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I'll start investigating this problem in a couple of minutes. I'll test with and without the patch on bug 477934.

(In reply to comment #20)
> Steven, does restarting the system terminate the app the same way as choosing
> Quit from the Dock? If so, is this the same bug as bug 491122?

I believe that it is the same way.

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #22)
> > Steven, does restarting the system terminate the app the same way as choosing
> > Quit from the Dock? If so, is this the same bug as bug 491122?
> 
> I believe that it is the same way.

Ok, looks to be different. Using the dock to close Firefox this behavior cannot be seen.

Comment 24

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Bug 477934 isn't involved here. A build from 09051103 shows this behavior too. The relanding of the patch on bug 477934 happened on May 12th. I'll check older builds to find a possible regression range.

Comment 25

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Just a heads-up: The regression range is between beta 2 and beta 3. I'll come with more details soon. Having restarts in between each test makes it somewhat harder.

Comment 26

[Simon Bünzli]

(In reply to comment #25)
> Having restarts in between each test makes it somewhat harder.

Doesn't just logging out and in again have the same effect?

Comment 27

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Regression started between the builds 09021602 and 09021702

Changesets:
Pass: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f38dee2a8ccd
Fail: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9fe3e9c7e44f

Checkins: http://hg.mozilla.org/releases/mozilla-1.9.1/pushloghtml?fromchange=f38dee2a8ccd&tochange=9fe3e9c7e44f

I blame Marco's patch on bug 478218.

Comment 28

[Marco Bonardo [:mak]]

So, bug 478218 moved some Places shutdown work from quit-application to quit-application-granted. The save session dialog is fired at quit-application-requested, and the pref is set to true at quit-application based on result from dialog.

the only pertinent thing i see is that Places calls SavePrefFile on q-a-g, not sure that can cause an issue on mac

Comment 29

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #28)
> the only pertinent thing i see is that Places calls SavePrefFile on q-a-g, not
> sure that can cause an issue on mac

Perhaps _setPrefToSaveSession was depending on the places SavePrefFile call to actually work? Not sure why that would only happen on Mac...

Comment 30

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I will check with a local build around this changeset if it is the real cause.

Comment 31

[Marco Bonardo [:mak]]

from what i see _setPrefToSaveSession is still called

Comment 32

[Marco Bonardo [:mak]]

so, sounds like Gavin could be right, adding a this._prefs.savePrefFile(null); in _setPrefToSaveSession (after setting the pref) have make this work one time here... unfortunatly i have to leave, will be back on this in the afternoon (PDT). but if someone want to test that or make a patch, feel free to :)

Comment 33

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #23)
> Ok, looks to be different. Using the dock to close Firefox this behavior cannot
> be seen.

That's not what I'm seeing. I see this bug when quitting from the Dock icon.

Comment 34

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #23)

>>> Steven, does restarting the system terminate the app the same way
>>> as choosing Quit from the Dock? If so, is this the same bug as bug
>>> 491122?
>>
>> I believe that it is the same way.
>
> Ok, looks to be different. Using the dock to close Firefox this
> behavior cannot be seen.

Quitting from the Dock should produce exactly the same shutdown
behavior as restarting the system (or logging out) while FF is
running.  I've just confirmed that the following methods get called on
logout.  They don't get called when you Quit from the Firefox menu.

[MacApplicationDelegate applicationShouldTerminate:]
[AppShellDelegate applicationWillTerminate:]
[NSApplication terminate:]

Comment 35

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #20)

> Steven, does restarting the system terminate the app the same way as
> choosing Quit from the Dock?

Yes (as I said in comment #34).

> If so, is this the same bug as bug 491122?

No.  Both this bug (bug 497482) and bug 491122 are special cases of a
more general problem (one that's currently unfixable) -- the shutdown
sequence is different (and partially broken) when you quit via
[NSApplication terminate:].  But otherwise they're different, and
probably need to be fixed in different ways.  See bug 491122 comment
#8 and bug 491122 comment #10.

Comment 36

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

For what it's worth, the pref is being set by the pref service (thanks gdb). I'm still looking to see why the file isn't saved though. That should confirm what Marco said in comment #32. Also, being able to repro by quitting from the Dock makes this easier to work with, but also worse.

Comment 37

[:Gavin Sharp [email: [e-mail]]]

Attachment: workaround

This ensures that we flush the resume_session_once pref change to disk as soon as we set it, and works around this symptom of the more general bug for me. It also catches the "don't ask me again" pref since that's set earlier (when the prompt is dismissed).

It has the potential to cause a tshutdown regression in the cases where we prompt on a normal shutdown, but there's no real way to avoid that.

Comment 38

[Mike Connor [:mconnor]]

Comment on attachment 382773 [details] [diff] [review]
workaround

can you add a comment that this is a workaround, and link to whatever bug we file on figuring out why the prefs file isn't being written out normally?

Comment 39

[:Gavin Sharp [email: [e-mail]]]

I filed bug 497652 on the general issue.

Comment 40

[:Gavin Sharp [email: [e-mail]]]

Attachment: with comment

Comment 41

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 382795 [details] [diff] [review]
with comment

(blockers don't need approval, but what the hell!)

Comment 42

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/releases/mozilla-1.9.1/rev/03e1f059d307

Comment 43

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/mozilla-central/rev/6c95fd93471f

Comment 44

[u279076]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1) Gecko/20090612 Firefox/3.5

Works for me!  verified1.9.1