1036341 - [Session Restore] Discriminate more precisely between new profile and catastrophic crash

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

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

Bug 1035557 fixes an issue in which we failed to discriminate between a new profile and a catastrophic crash. However, now that bug 883609 has landed, by tweaking slightly the loading code, we can find this information much more precisely.

We need to:
- tweak `SessionFile.read` to return one more field `freshProfile` that is `true` if none of the files we attempt to read even exist, or `false` if at least one exists;
- tweak `nsSesionStartup._onSessionFileRead` to use this information if `checkpoints` is `null`, by setting `this._previousSessionCrashed` to `false` if `freshProfile` is `true`, or by using `this._initialState` as we currently do if `freshProfile` is `false`.

Comment 1

[Steven MacLeod [:smacleod]]

Marco, please add this to the current iteration

Comment 2

[Marco Mucci [:MarcoM]]

Added to Iteration 34.1

Comment 3

[Marco Mucci [:MarcoM]]

Hi Steven, can you mark this bug as [qa+] or [qa-] for verification.

Comment 4

[Steven MacLeod [:smacleod]]

Attachment: Patch - Use the existence of any session files to determine if a profile crashed or is fresh

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8471079 [details] [diff] [review]
Patch - Use the existence of any session files to determine if a profile crashed or is fresh

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

::: browser/components/sessionstore/SessionFile.jsm
@@ +240,5 @@
>          parsed: null
>        };
>      }
>  
> +    result["noFilesFound"] = noFilesFound;

Maybe result.noFilesFound = ...? That's a little more common in the code base.

Comment 6

[Steven MacLeod [:smacleod]]

https://hg.mozilla.org/integration/fx-team/rev/8ce45b343da9

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/8ce45b343da9

Comment 8

[Cornel Ionce [:noni] [Hubs QA]]

Is there any proper way to manually test this fix?

Comment 9

[Steven MacLeod [:smacleod]]

(In reply to Cornel Ionce [QA] from comment #8)
> Is there any proper way to manually test this fix?

The way this was implemented it shouldn't actually result in a behaviour change, it just makes the code more explicit about why we do things. I don't think we need to verify this.

Sorry about the confusion!