Closed Bug 497482 Opened 11 years ago Closed 11 years ago

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

Categories

(Firefox :: Session Restore, defect)

3.5 Branch
All
macOS
defect
Not set

Tracking

()

VERIFIED FIXED

People

(Reporter: ashughes, Assigned: Gavin)

References

(Depends on 1 open bug, )

Details

(Keywords: dataloss, regression, verified1.9.1)

Attachments

(2 files, 1 obsolete file)

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.
I'd like to get more confirmation, but asking to block on this for investigation due to the dataloss.
Flags: blocking-firefox3.5?
Keywords: dataloss
Yes, definitely need to confirm.
Flags: blocking-firefox3.5? → blocking-firefox3.5+
Keywords: qawanted
Where is the -foreground command line flag coming from?
<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>
Bah; ignore comment 4, that change is mozilla-central only :(
(Dietrich: fwiw, I can confirm that it happens on OSX, not on w32)
-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?
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?
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.
Attached file 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.
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?
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.
I can also reproduce this with Firefox 3.5 Preview, so both trunk and 1.9.1 have this problem.
(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.
"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.
(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?
Summary: Session not restored from OS reboot → [Mac] browser.sessionstore.resume_session_once not being set when user chooses "Save and Quit" on reboot / restart
I'd venture a guess that bug 477934 *might* have affected this. I'll stop spamming now...
Shawn/Biesi: can you look and see if this is related to bug 477934?
Are we writing any just-before-exit-when-rebooting preference changes to disk? Or are we just missing this specific one?
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)
FWIW, I couldn't reproduce this with 3.0.x
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.
(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.
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.
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.
(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?
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
(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...
I will check with a local build around this changeset if it is the real cause.
from what i see _setPrefToSaveSession is still called
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 :)
(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.
(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:]
(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.
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.
Attached patch workaround (obsolete) — Splinter Review
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.
Attachment #382773 - Flags: review+
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?
Depends on: 497652
I filed bug 497652 on the general issue.
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
Hardware: x86 → All
Attached patch with commentSplinter Review
Attachment #382773 - Attachment is obsolete: true
Attachment #382795 - Flags: approval1.9.1?
Comment on attachment 382795 [details] [diff] [review]
with comment

(blockers don't need approval, but what the hell!)
Attachment #382795 - Flags: approval1.9.1? → approval1.9.1+
https://hg.mozilla.org/mozilla-central/rev/6c95fd93471f
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.