887394 - [Session Restore] Don't collect state right after startup when restoring the initial session

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

http://mxr.mozilla.org/mozilla-central/source/browser/components/sessionstore/src/SessionStore.jsm#712

Right after restoring the initial state for the first window loaded after startup, we call .saveState() only to set the session state from STOPPED to RUNNING to count startup crashes correctly.

It would be great to just change the state without writing the whole file but we cannot do that. What we can do is at least not collect the browser state at a point where probably nothing has even finished restoring, yet.

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Attachment: don't collect state right after startup when restoring the initial session

Comment 2

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

Comment on attachment 767900 [details] [diff] [review]
don't collect state right after startup when restoring the initial session

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

::: browser/components/sessionstore/src/SessionStore.jsm
@@ +714,5 @@
>  
>            // _loadState changed from "stopped" to "running"
>            // force a save operation so that crashes happening during startup are correctly counted
> +          this._initialState.session.state = STATE_RUNNING_STR;
> +          this._saveStateObject(this._initialState);

Do we actually need the call to saveStateObject?

@@ +715,5 @@
>            // _loadState changed from "stopped" to "running"
>            // force a save operation so that crashes happening during startup are correctly counted
> +          this._initialState.session.state = STATE_RUNNING_STR;
> +          this._saveStateObject(this._initialState);
> +          delete this._initialState;

Could we take the opportunity to remove that |delete| and replace it with an assignment? Calling |delete| destroys inline caching.

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Attachment: don't collect state right after startup when restoring the initial session

Comment 4

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

Comment on attachment 768277 [details] [diff] [review]
don't collect state right after startup when restoring the initial session

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

Looks good.
It might also prove useful for the day saveState becomes async.

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

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

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/086465524970