342885 - Session restore launches even when I haven't crashed

Status: VERIFIED INCOMPLETE

(Firefox :: Session Restore, defect)

Description

[Marcia Knous [:marcia]]

I have seen this intermittently using the Bon Echo builds. When I go from one Bon Echo Mac nightly to another, sometimes the session restore dialog comes up even when my last session had not crashed. Is this expected?

I usually don't exit the browser at the end of the day, but I put my machine to sleep - not sure if that matters.

Comment 1

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

Could this be a dupe of bug 336193? Maybe the "sleep" command sends SIGTERM?

Comment 2

[Asaf Romano (gone)]

I've just seen this in my windows branch build after a *normal* exit.

Comment 3

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

Can we get some STR here? It sounds annoying, but also pretty low frequency.

Comment 4

[Marcia Knous [:marcia]]

STR on my Mac that always sleeps and I rarely ever close the browser:

1. Shut down the previous day's Bon Echo build. 
2. Launch the new build that I have just installed, the "Session Restore" screen comes up, even though my last session did not crash.

I am using an existing profile with a good number of extensions, but no extensions that relate to restoring an extension.

Comment 5

[Ger Teunis]

I'm seeing this when starting firefox after a shutdown with firefox enabled.
(I don't have fast shutdown enabled in windows XP pro sp2).

Comment 6

[Aaron P]

Using 20060711 branch nightly with a (fairly) clean profile I've seen this at least twice. It happened for me in situations described in comments #4 and #5.

Comment 7

[Simon Bünzli]

(In reply to comment #5)
> I'm seeing this when starting firefox after a shutdown with firefox enabled.

That's bug 333907.

(In reply to comment #4)
> I am using an existing profile with a good number of extensions, but no
> extensions that relate to restoring an extension.

This might still be an extension conflict. Can you reproduce this on a clean profile?

BTW: Depending on how you shutdown Bon Echo, this could also be bug 333907 (the -quit command line switch also doesn't notify SessionStore of the shutdown).

Comment 8

[Nickolay_Ponomarev]

Bug 344852 could make this issue much less visible.

Comment 9

[Mike Connor [:mconnor]]

Without reliable steps to reproduce, there isn't much we can do here.  If you shut down the OS while the app is running, we don't always shut down gracefully (this helped me earlier when XP auto-rebooted overnight)

Comment 10

[Mike Connor [:mconnor]]

Please renominate if/when we have reliable steps to reproduce.

The Windows shutdown case is expected behaviour since Firefox does not allow the user to abort shutdown.

Comment 11

[u130342]

(In reply to comment #10)

> The Windows shutdown case is expected behaviour since Firefox does not allow
> the user to abort shutdown.
> 

I really hate the session restore on a normal shutdown. You should rethink of "expected behaviour" is really what users expect.
BTW: WM_ENDSESSION is sent on a normal windows shutdown (preceded by  WM_QUERYENDSESSION).
So.. if there is a WM_ENDSESSION with wParam=TRUE in wParam then clear session history.

For me it's a privacy issue!
Or let me disable that feature at all.

Comment 12

[Nickolay_Ponomarev]

You can disable restore-after-crash, which will also disable restore-after-shutdown: http://kb.mozillazine.org/Browser.sessionstore.resume_from_crash

As I said in bug 344852 comment 12, it should be possible to shut down session store correctly on WM_ENDSESSION, which would be easier than fixing bug 333907 properly.

Comment 13

[Mark Jansen]

I've disabled the feature, simply because it indeed occurs randomly, which makes Firefox start very slow.

Extensions:
Adblock Plus 0.5.11.4
DOM Inspector 1.8.1b2
Download statusbar 0.9.4.1
Flashgot 0.5.96.060823
IEView 1.3.0
Talkback 2.0b2

Comment 14

[Kai Hendry]

When I quit X, I do a CTRL+ALT+BACKSPACE combo. I am not sure what signal is sent to Firefox, though I do get an annoying unwanted "Your last Firefox session closed unexpectedly"... message when I log back in and start Firefox.

When I do a X killclient from my WM dwm however, it does not come up with this message. So I suggest someone who knows more about X figure out how Firefox can cope with CTRL+ALT+BACKSPACE type kill. Otherwise allow me to disable browser.sessionstore.resume_from_crash easily #343834. For some reason I can't see this on about:config

Comment 15

[Robert Chapin]

I can reproduce this bug every time on XP.  If Firefox is running when I shut down or restart windows, then I will get a session restore dialog the next time Firefox runs.

Comment 16

[Ger Teunis]

(In reply to comment #15)
> I can reproduce this bug every time on XP.

Confirming, this should be a Fx 3.0 blocker

Comment 17

[Simon Bünzli]

Attachment: minimal branch patch (Windows only)

So, people are falling over this again and again. As proposed in comment #12, we should really consider applying minimal bandaid to get rid of this annoyance until a proper patch for bug 333907 sees the light.

This patch achieves this through a new (temporary) global notification dispatched at WM_ENDSESSION to which SessionStore reacts as if Firefox had been shut down roughly through nsIAppStartup without calling |canQuitApplication| first.

Note that to minimize the patch's impact, the notification is currently Firefox only. Additionally, similar code paths would be required for other platforms (as e.g. KDE; cf. bug 354686).

Comment 18

[[e-mail]]

Comment on attachment 249002 [details] [diff] [review]
minimal branch patch (Windows only)

>+#ifdef MOZ_PHOENIX
Why bother, it's not as if anyone else is listening, is it?

>+    case WM_ENDSESSION:
Shouldn't you check the value of wParam?

>+    case "quit-application-roughly":
>       if (aData == "restart")
>         this._prefBranch.setBoolPref("sessionstore.resume_session_once", true);
>       this._loadState = STATE_QUITTING; // just to be sure
>       this._uninit();
>       break;
Will this will save state synchronously?

Comment 19

[Simon Bünzli]

Attachment: minimal branch patch (Windows only) [checked in on the trunk for baking]

(In reply to comment #18)
> >+#ifdef MOZ_PHOENIX
> Why bother, it's not as if anyone else is listening, is it?

Not really. Removed.

> >+    case WM_ENDSESSION:
> Shouldn't you check the value of wParam?

Sure - although practically it will never be FALSE (otherwise we would have properly reacted to WM_QUERYENDSESSION which we don't).

> Will this will save state synchronously?

Of course.

Comment 20

[[e-mail]]

(In reply to comment #19)
>Sure - although practically it will never be FALSE (otherwise we would have
>properly reacted to WM_QUERYENDSESSION which we don't).
By default, DefWindowProc returns TRUE for WM_QUERYENDSESSION, and you then receive a WM_ENDSESSION where wParam is FALSE if someone else returned FALSE.

Comment 21

[Simon Bünzli]

Comment on attachment 249012 [details] [diff] [review]
minimal branch patch (Windows only) [checked in on the trunk for baking]

It would probably make sense to have this patch baking on Trunk first (and back it out once bug 333907 has been fixed).

Comment 22

[Jay Patel [:jay]]

Not blocking the release for this just yet, please get this on the Trunk first and let's figure out the right thing to do on the branch.

Comment 23

[Simon Bünzli]

Comment on attachment 249012 [details] [diff] [review]
minimal branch patch (Windows only) [checked in on the trunk for baking]

Gavin: could you please check this patch into Trunk for baking? Thanks.

Comment 24

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

Yeah, sorry for the delay.

mozilla/widget/src/windows/nsWindow.cpp 	3.679
mozilla/browser/components/sessionstore/src/nsSessionStore.js 	1.55

Comment 25

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

Comment on attachment 249012 [details] [diff] [review]
minimal branch patch (Windows only) [checked in on the trunk for baking]

Actually, should this bug remain open for a better trunk fix?

Comment 26

[Simon Bünzli]

(In reply to comment #25)
> Actually, should this bug remain open for a better trunk fix?

Once bug 333907 has been fixed, the correct fix will consist in backing this patch out (it's really just a branch bandaid).

As for closing this bug: this patch only applies to Windows while apparently the same symptoms exist on other platforms as well (e.g. bug 336193 and bug 365749 under Linux). We might want to keep this bug open for further bandaids/steps to reproduce...

Comment 27

[Jay Patel [:jay]]

Comment on attachment 249012 [details] [diff] [review]
minimal branch patch (Windows only) [checked in on the trunk for baking]

Approved for the 1.8 branch, a=jay for drivers.

Thanks for the input everyone, let's get his landed asap.

Comment 28

[Simon Bünzli]

Gavin: Could you please check this in for me? Thanks.

Comment 29

[Simon Bünzli]

Guys: Any chance for a check-in?

Comment 30

[Mike Connor [:mconnor]]

Comment on attachment 249012 [details] [diff] [review]
minimal branch patch (Windows only) [checked in on the trunk for baking]

Needs more discussion, sorry.

Comment 31

[Mark Jansen]

Could this be fixed in a minor revision, like 2.0.0.2?

Comment 32

[Daniel Veditz [:dveditz]]

Comment on attachment 249012 [details] [diff] [review]
minimal branch patch (Windows only) [checked in on the trunk for baking]

Not approved for 1.8.1.2, beltzner and mconnor don't like this behavior. A minor polish fix might be appropriate for an update, but not a controversial one -- better to have consistent  behavior.

Comment 33

[Simon Bünzli]

Reopening. The checked-in patch will have to be backed out from Trunk anyways, once bug 333907 gets some traction.

Comment 34

[Simon Bünzli]

Moving this off my plate so that anybody can feel free to come up with a better short-term solution...

Dietrich or Gavin: Could you please back out the changes from comment #24? Thanks.

Comment 35

[Dietrich Ayala (:dietrich)]

backed out from trunk

Checking in browser/components/sessionstore/src/nsSessionStore.js;
/cvsroot/mozilla/browser/components/sessionstore/src/nsSessionStore.js,v  <--  nsSessionStore.js
new revision: 1.60; previous revision: 1.59
done
Checking in widget/src/windows/nsWindow.cpp;
/cvsroot/mozilla/widget/src/windows/nsWindow.cpp,v  <--  nsWindow.cpp
new revision: 3.682; previous revision: 3.681
done

Comment 36

[Mike Mazanek]

I'm using FF 2.0.0.1 for a couple months with no problems, no updates applied and no system changes.  Out of nowhere, every time I power on/restart (FF in Startup), no matter how I've shut down PROPERLY, I now get the Error Msg at start up that FF was closed unexpectedly, and I have to choose to either restore prior tabs, or start new (something to this effect anyway).  The real issue is that this msg appears and stops auto loading of FF requiring user action. Running XP with SP2 and all current updates, and have had DOM Inspector and Talkback Add-ons installed all the time.

Comment 37

[Mateusz]

I am using FF 2.0.0.2 and find this problem perfectly reproductable. Just install firefox preloader (v. 1.0 build 366) and restart XP. You will always get the "restore session" error. I've just tested it on several clean installs, with different patches and systems, even Windows x64 - always the same. Perhaps it will help to trace down the problem.

Comment 38

[Mike Mazanek]

Glad somebody else has confirmed my specific bug as it continues even with FF 2.0.0.2 for me as well

Comment 39

[Simon Bünzli]

(In reply to comment #37)
You're actually just experiencing bug 333907. Similar bugs exist for OS X and Linux as well. Since there's never be any further confirmation that this issue may also occur in a different situation, I tend to see this bug as WORKSFORME.

Comment 40

[Jeanne]

I'm new to bugzilla, but I wanted to report that I am having this problem with Vista, too. 

Comment 41

[Michael Follin]

If anyone's interested, this happens with Windows 98 SE too.  Really annoying since Firefox 2 - I always shut down several applications open, so always get the annoying "restore?" dialog.  If FF gets the windows shutdown message, it should not display the dialog next startup.  Simple.  ?

Comment 42

[Leon Stringer]

From a corporate roll-out point-of-view this is a significant problem. Users shut down their computers, start Firefox at next login and then get what is, in effect, an error message. Error messages become support calls which is a good reason not to roll out 2.x. Certainly we're sticking with 1.5 until this stops.

Comment 43

[Nickolay_Ponomarev]

Leon: that's a pity. I think you should mail dev-apps-firefox (and maybe try to reach fx2 drivers) with this comment. Perhaps the decision could be reviewed.

Until the issue is fixed, you could install firefox 2 and disable session store - that won't make things any worse than with 1.5.

Comment 44

[Ali Lakhia]

I have been annoyed by this bug too. So, would like to suggest a short-term solution: would it be possible to always assume the answer is yes to restoring a session? And if a startup session is causing people problem, then they would use the safe mode which would ask user if he/she wants to restore the session or not.

My 2 cents. Thanks for reading.

Comment 45

[Simon Bünzli]

(In reply to comment #44)
> would it be possible to always assume the answer is yes to restoring a session?

Probably won't happen for various reasons: (1) users should be informed about what's actually happening; (2) not everybody knows about safe-mode in the first place; (3) you might want to cancel the restoration depending on who looks over your shoulders.

Work-around, until this issue is fixed:

1. Enter about:config into the address bar.
2. Right-click into the appearing list and select New -> String.
3. Enter "browser.sessionstore.restore_prompt_uri" (without the quotes) as the pref's name.
4. Enter "javascript:window.close();" (without the quotes) as the pref's value.
5. Restart Firefox and be happy about never having to bother with that prompt again.

Comment 46

[Randy Santmann]

If I have Firefox open and I reboot, it sometimes asks me to restore the session.  I've seen this even after I recently reinstalled my PC.  I'm running Windows XP SP2.

Comment 47

[Simon Bünzli]

Marcia (or anybody on OS X): Have you ever observed this issue again (if possible on an extension-less profile)? If not, please close this bug as WORKSFORME.

Apart from the potential OS X issue, there seems to be no other problem here which isn't already covered by another bug. Should anybody see an unexpected Restore Session prompt with Firefox 3.0 (after bug 333907 has been fixed!), please file a new bug.

Comment 48

[Egor Pelevin]

I still see this on Win XP with clean profile

Comment 49

[Egor Pelevin]

(In reply to comment #48)
> I still see this on Win XP with clean profile
> 

It's due to bug 391940 (I use -no-remote for testing)

Comment 50

[Simon Bünzli]

As long as there are no Steps to Reproduce for this OS X issue, this bug is INCOMPLETE. Should you ever see an unexpected Restore Session prompt on OS X, please file a new bug, though, as this one mixes multiple issues.

And just to repeat: Windows and Linux are/were handled separately (see the status whiteboard at the top of this bug).