1035557 - about:sessionrestore shows instead of about:welcomeback after a reset

Status: VERIFIED FIXED

(Firefox :: Session Restore, defect)

Description

[Matthew N. [:MattN]]

After a reset, "Well. this is embarrasing" is shown instead of "Welcome Back!".

Could somebody find a regression range so we know which versions to track the regression for?

Comment 1

[Alice0775 White]

Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/316466d0b962
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140117114854
Bad:
http://hg.mozilla.org/mozilla-central/rev/fad7172d4542
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140117115833
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=316466d0b962&tochange=fad7172d4542

Regression window(fx)
Good:
http://hg.mozilla.org/integration/fx-team/rev/af860898eb79
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140116153922
Bad:
http://hg.mozilla.org/integration/fx-team/rev/10256adb9f9e
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140117024217
Pushlog:
http://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=af860898eb79&tochange=10256adb9f9e

Suspect:
10256adb9f9e	Steven MacLeod — Bug 887780 - Stop writing state immediately after startup and switch to CrashMonitor for crash detection. r=Yoric, r=ttaubert X-Git-Commit-ID: bb02f51399059295041176769438b0f451f983bf

Comment 2

[Matthew N. [:MattN]]

Thanks Alice

Comment 3

[David Teller [:Yoric] - still alive but not very active]

Good catch, we assume that no CrashMonitor data == big crash, but in fact, this could also mean fresh profile.

Comment 4

[:Gijs (out for now; he/him)]

Hrmpf. *raises guilty hand*

If only I'd spent more time chasing bug 888624... :-\

Comment 5

[David Teller [:Yoric] - still alive but not very active]

Attachment: Empty crash file != crash

This version of the patch should be upliftable. We might wish for something a little more robust for FF33 as bug 883609 introduces more data that may be used to know with certainly that there is no sessionstore.js.

Comment 6

[David Teller [:Yoric] - still alive but not very active]

Try: https://tbpl.mozilla.org/?tree=Try&rev=cdff919f5020

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/77728b5c5d7d

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/77728b5c5d7d

Comment 9

[Benjamin Kerensa [:bkerensa]]

You don't need tracking on this any longer do you?

Comment 10

[David Teller [:Yoric] - still alive but not very active]

Benjamin: How do I determine that? I have never been involved in tracking flags so far.

Comment 11

[Camelia Badau [:cbadau], Desktop Test Engineering]

Reproduced the bug on Firefox 31 RC.
Verified the bug on Windows 7 64bit, Ubuntu 13.10 32bit and Mac OSX 10.7.5 using latest Nightly 33.0a1 (buildID: 20140717030339) and the following mentions should be done: 
- the page which is displayed after resetting Firefox is "Thank you for using Firefox Nightly" (which is the page displayed when opening Firefox with a new profile)
- after resetting Firefox, the actual profile is replaced with a new profile (e.g: default-14056...)
- after resetting Firefox, a new folder named "Old Firefox Data" is created on Desktop

Any thoughts?

Comment 12

[David Teller [:Yoric] - still alive but not very active]

Well, my thought is that we clearly need to uplift this.

Comment 13

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8452270 [details] [diff] [review]
Empty crash file != crash

Approval Request Comment
[Feature/regressing bug #]: bug 887780
[User impact if declined]: Firefox Reset causes the "Oops" welcome page to be displayed instead of the First Use
[Describe test coverage new/current, TBPL]: This has been on m-c for ~1 week, + QAed
[Risks and why]: None that I can think of.
[String/UUID change made/needed]: None.

Comment 14

[Sylvestre Ledru [:Sylvestre]]

David, you will learn more about tracking flags here:
https://wiki.mozilla.org/Release_Management/Tracking_rules

Too late for 31. Too bad, I would have take this :/

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/23ad26e7e8d3

Comment 16

[Camelia Badau [:cbadau], Desktop Test Engineering]

(In reply to Camelia Badau, QA [:cbadau] from comment #11)
> Reproduced the bug on Firefox 31 RC.
> Verified the bug on Windows 7 64bit, Ubuntu 13.10 32bit and Mac OSX 10.7.5
> using latest Nightly 33.0a1 (buildID: 20140717030339) and the following
> mentions should be done: 
> - the page which is displayed after resetting Firefox is "Thank you for
> using Firefox Nightly" (which is the page displayed when opening Firefox
> with a new profile)
> - after resetting Firefox, the actual profile is replaced with a new profile
> (e.g: default-14056...)
> - after resetting Firefox, a new folder named "Old Firefox Data" is created
> on Desktop


Is this the behaviour intended? Or those are bugs?

Comment 17

[David Teller [:Yoric] - still alive but not very active]

This looks correct.

Comment 18

[:Gijs (out for now; he/him)]

(In reply to Camelia Badau, QA [:cbadau] from comment #16)
> (In reply to Camelia Badau, QA [:cbadau] from comment #11)
> > Reproduced the bug on Firefox 31 RC.
> > Verified the bug on Windows 7 64bit, Ubuntu 13.10 32bit and Mac OSX 10.7.5
> > using latest Nightly 33.0a1 (buildID: 20140717030339) and the following
> > mentions should be done: 
> > - the page which is displayed after resetting Firefox is "Thank you for
> > using Firefox Nightly" (which is the page displayed when opening Firefox
> > with a new profile)

Err, unless I'm missing something, this doesn't look correct. If you:

- open 3 pages in tabs before resetting (e.g. www.mozilla.org, bugzilla, about:mozilla);
- then open about:support in another tab and click the reset button

you should see the "about:welcomeback" page which lists the 3 or 4 tabs, shouldn't you?

Comment 19

[David Teller [:Yoric] - still alive but not very active]

Oh. In that case, I am clearly the wrong interlocutor for this. Someone else who knows Firefox Reset better than me should answer Camelia.

Comment 20

[:Gijs (out for now; he/him)]

(In reply to :Gijs Kruitbosch (Gone July 26 - August 3) from comment #18)
> (In reply to Camelia Badau, QA [:cbadau] from comment #16)
> > (In reply to Camelia Badau, QA [:cbadau] from comment #11)
> > > Reproduced the bug on Firefox 31 RC.
> > > Verified the bug on Windows 7 64bit, Ubuntu 13.10 32bit and Mac OSX 10.7.5
> > > using latest Nightly 33.0a1 (buildID: 20140717030339) and the following
> > > mentions should be done: 
> > > - the page which is displayed after resetting Firefox is "Thank you for
> > > using Firefox Nightly" (which is the page displayed when opening Firefox
> > > with a new profile)
> 
> Err, unless I'm missing something, this doesn't look correct. If you:
> 
> - open 3 pages in tabs before resetting (e.g. www.mozilla.org, bugzilla,
> about:mozilla);
> - then open about:support in another tab and click the reset button
> 
> you should see the "about:welcomeback" page which lists the 3 or 4 tabs,
> shouldn't you?

Matt, can you clarify per comment #19?

Comment 21

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Looks like we're just waiting for an update from Matt on this one.

Comment 22

[Matthew N. [:MattN]]

(In reply to :Gijs Kruitbosch (Gone July 26 - August 3) from comment #20)
> (In reply to :Gijs Kruitbosch (Gone July 26 - August 3) from comment #18)
> > (In reply to Camelia Badau, QA [:cbadau] from comment #16)
> > > (In reply to Camelia Badau, QA [:cbadau] from comment #11)
> > > > Reproduced the bug on Firefox 31 RC.
> > > > Verified the bug on Windows 7 64bit, Ubuntu 13.10 32bit and Mac OSX 10.7.5
> > > > using latest Nightly 33.0a1 (buildID: 20140717030339) and the following
> > > > mentions should be done: 
> > > > - the page which is displayed after resetting Firefox is "Thank you for
> > > > using Firefox Nightly" (which is the page displayed when opening Firefox
> > > > with a new profile)
> > 
> > Err, unless I'm missing something, this doesn't look correct. If you:
> > 
> > - open 3 pages in tabs before resetting (e.g. www.mozilla.org, bugzilla,
> > about:mozilla);
> > - then open about:support in another tab and click the reset button
> > 
> > you should see the "about:welcomeback" page which lists the 3 or 4 tabs,
> > shouldn't you?
> 
> Matt, can you clarify per comment #19?

You should see about:welcomeback and the firstrun tab that Camelia described. Unfortunately the firstrun tab  will be the selected one since that firstrun code knows nothing about sessionrestore. This has been a known issue for a while as it interacts poorly with some other startup stuff but I'm not sure it's filed so it should be.

Comment 11 wasn't explicit about whether there was more than one tab. If about:welcomeback wasn't opened then that is definitely a problem which needs a bug.

Comment 23

[:Gijs (out for now; he/him)]

(In reply to Camelia Badau, QA [:cbadau] from comment #11)
> Reproduced the bug on Firefox 31 RC.
> Verified the bug on Windows 7 64bit, Ubuntu 13.10 32bit and Mac OSX 10.7.5
> using latest Nightly 33.0a1 (buildID: 20140717030339) and the following
> mentions should be done: 
> - the page which is displayed after resetting Firefox is "Thank you for
> using Firefox Nightly" (which is the page displayed when opening Firefox
> with a new profile)

Camelia, can you doublecheck this bit wrt comment #22 ? Thank you!

Comment 24

[Camelia Badau [:cbadau], Desktop Test Engineering]

I double checked this and here are the steps and the conclusions: 

1. Open 3 pages in tabs(e.g. www.mozilla.org, bugzilla, about:mozilla)
2. Open about:support in another tab and click the Reset button.
3. a) Select to import data from Firefox (on "Import Settings and Data" dialog). 
   b) Select "don't import anything" (on "Import Settings and Data" dialog). 

-> After step 3 a), the "Restore Session" page is opened (with "Well, this is embarrassing"). See screenshot "import.PNG"
-> After step 3 b), the "Nightly Start Page" and the firstrun tab are opened. See screenshot "don't import.PNG".

Comment 25

[Camelia Badau [:cbadau], Desktop Test Engineering]

Attachment: issues.zip

Comment 26

[David Teller [:Yoric] - still alive but not very active]

Ok, so it sounds like there is still a bug.

I am in a wrong position to handle this bug at the moment, as I'll be on PTO in two days. Steve, any chance you could take a look at this? I believe that the best way to fix this is to patch SessionFile.read to add a field to `result` that tells us if at least one of the files of `loadOrder` exists.

Comment 27

[Matthew N. [:MattN]]

(In reply to Camelia Badau, QA [:cbadau] from comment #24)
>    b) Select "don't import anything" (on "Import Settings and Data" dialog). 

Note that "don't import anything" isn't really a valid choice for this dialog after a reset. That page of the wizard isn't even supposed to be shown. That's bug 1032958.

It sounds 3A which is valid is still incorrect though.

Comment 28

[Tim Taubert [:ttaubert] (inactive)]

(In reply to David Rajchenbach Teller [:Yoric] from comment #26)
> I believe that
> the best way to fix this is to patch SessionFile.read to add a field to
> `result` that tells us if at least one of the files of `loadOrder` exists.

That is what bug 1036341 is about, right?

Comment 29

[Steven MacLeod [:smacleod]]

(In reply to Tim Taubert [:ttaubert] from comment #28)
> (In reply to David Rajchenbach Teller [:Yoric] from comment #26)
> > I believe that
> > the best way to fix this is to patch SessionFile.read to add a field to
> > `result` that tells us if at least one of the files of `loadOrder` exists.
> 
> That is what bug 1036341 is about, right?

For the most part yeah. I'll take care of Bug 1036341 and this one.

Comment 30

[Marco Mucci [:MarcoM]]

Added to Iteration 34.1

Comment 31

[Steven MacLeod [:smacleod]]

Attachment: Patch - Migrate crash checkpoints with the session so that we don't appear to crash during reset

Comment 32

[Steven MacLeod [:smacleod]]

https://hg.mozilla.org/integration/fx-team/rev/07e19701c947

Comment 33

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/07e19701c947

Comment 34

[Camelia Badau [:cbadau], Desktop Test Engineering]

Verified fixed on Windows 7 64bit, Ubuntu 13.10 32bit and Mac OSX 10.8.5 using latest Nightly 34.0a1 (buildID: 20140819030202). 

Changing the Target Milestone and the status flags to reflect the current situation.

Comment 35

[:Gijs (out for now; he/him)]

Can we try to uplift the second patch for 32/33 still, Steven?

Comment 36

[Steven MacLeod [:smacleod]]

Comment on attachment 8471081 [details] [diff] [review]
Patch - Migrate crash checkpoints with the session so that we don't appear to crash during reset

Approval Request Comment
[Feature/regressing bug #]: Regressed by Bug 887780
[User impact if declined]: Successful resets will show the "Well. this is embarrasing" crash page instead of the "Welcome back!" page.
[Describe test coverage new/current, TBPL]: No automated test coverage.
[Risks and why]: Should be low risk, just copying one more file from the old profile when migrating.
[String/UUID change made/needed]: None

Comment 37

[Steven MacLeod [:smacleod]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a93ff5caef5f

Comment 38

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8471081 [details] [diff] [review]
Patch - Migrate crash checkpoints with the session so that we don't appear to crash during reset

I'm going to take this for the final 32 beta as it looks to be very low risk and a user visible reward.

Comment 39

[Steven MacLeod [:smacleod]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8d583074b918

Comment 40

[Camelia Badau [:cbadau], Desktop Test Engineering]

Verified fixed on:
- Windows 7 64bit, Ubuntu 13.10 32bit and Mac OSX 10.9.4 using latest Aurora 33.0a2 (buildID: 20140821004002)
- Ubuntu 13.10 32bit and Mac OSX 10.9.4 using Firefox 32 Beta 9 (buildID: 20140822024446)

Comment 41

[juan becerra [:juanb]]

Verified on Win7 using Fx32b9(build1) with the steps in comment #24. I get a "Welcome Back!" page after reset.

Comment 42

[Lukas Blakk [:lsblakk] use ?needinfo]

This doesn't meet ESR landing criteria, without an ESR community request there's no need to take this.