Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Session restore can lead to lost windows if there is a hung or slow Window on shutdown #9806

Closed
bbondy opened this issue Jun 30, 2017 · 1 comment

Comments

@bbondy
Copy link
Member

bbondy commented Jun 30, 2017

Test plan

#9912 (comment)


Pasting the issue that I described on Slack here:

First something things by design:

  • Session-store-1 is saved every 5 minutes.
  • If there is a crash on minute 4, data is restored as it was 4 minutes ago. If this is a first time run, then no save is done it just restores to the same way they started it the first time.
  • How the shutdown process works is the browser process sends a message to all windows to report their state to the browser process. Browser process upon getting all state writes it all out into session-store-1

[3:48]
the problem comes into play if your computer is very very slow, or if you have a hung window.

[3:49]
In which case the hung window will be cut out of the session store.

[3:49]
https://github.com/brave/browser-laptop/blob/master/app/index.js#L188 <- read comment there, it is the issue

[3:49]
instead of using that window's last known good state

[3:49]
we just save without that window

[3:50]
I think we can store each window's last state and include at least that in the case of a hung window.

[3:50]
and that would be a quick fix we could include in 0.18.x (or a 0.17.x hotfix if you really want)

[3:52]
if you have a hung window in that case, whether you had a 5-minute auto save or a Quit operation, then it would lead to the cut out window issue.

[3:53]
if all windows are hung for some reason, then all windows are lost.

[3:53]
note that the save in that line of code was meant to protect against a hung window, so that we wouldn't lose app state. It does that job well, app state is not lost. But it could be better by keeping last known reported window state.

bbondy [4:00 PM]
https://github.com/brave/browser-laptop/blob/master/app/index.js#L180 <- we reset the number of windows to 0 on each initiate save there

@bbondy bbondy added this to the 0.18.x (Developer Channel) milestone Jun 30, 2017
@bsclifton
Copy link
Member

bsclifton commented Jul 1, 2017

Related / possible duplicate of #5512

@bbondy bbondy changed the title Losing tabs when crashing Windows and tabs list can be lost if there is a hung window or slow shutdown Jul 4, 2017
bbondy added a commit that referenced this issue Jul 8, 2017
@bbondy bbondy changed the title Windows and tabs list can be lost if there is a hung window or slow shutdown Session restore can lead to lost windows if there is a hung or slow Window on shutdown Jul 8, 2017
bbondy added a commit that referenced this issue Jul 12, 2017
bbondy added a commit that referenced this issue Jul 12, 2017
bbondy added a commit that referenced this issue Jul 13, 2017
bbondy added a commit that referenced this issue Jul 13, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants