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

If window / tab restore fails, persisted state will be deleted #12984

Closed
petemill opened this issue Feb 1, 2018 · 2 comments
Closed

If window / tab restore fails, persisted state will be deleted #12984

petemill opened this issue Feb 1, 2018 · 2 comments

Comments

@petemill
Copy link
Member

petemill commented Feb 1, 2018

Description

This is a theory as we had reports in the upgrade from 0.19 -> 0.20 that 'open' windows and tabs were not persisted. Once the upgrade is complete, users experience working window / tab perisistence again - that is once on 0.20, any tabs and windows opened will still be opened upon restarting the same version. There are many state differences between 0.19 and 0.20 causing many 'upgrade' operations to run:

Could it be caused by the following?
On startup we read perWindowState in to memory, delete perWindowState from to-be-persisted state, and then use the in-memory perWindowState to create windows. If something goes wrong creating windows, but Brave somehow exits and cleanly writes the in-memory state to file again, then the windows and tabs will be gone

Steps to Reproduce

I do not have a STR for causing window / frame creation to fail. It could be that something else fails which causes Brave to exit before windows / frames have been created but cleanly writes session-store to file (without the perWindowState which was deleted from memory)

@BrendanEich
Copy link
Member

From my slack items:

@petemill what you describe makes sense. unless perWindowState is put in a set-aside that startup looks for to catch unclean shutdown...

these bugs all amount to "mutate state destructively lose data"

the fix is "save prev state or keep it safe in situ; put new state in separate location; atomic-swap name from old to new"

filesystem supports last step for the file

for anything in-heap in the running app, we need our own logic that looks for the set-aside

@petemill petemill added this to the Triage Backlog milestone Feb 2, 2018
@bsclifton
Copy link
Member

Related: #12896, #11576

@bsclifton bsclifton added the bug label Mar 2, 2018
@bsclifton bsclifton removed this from the Triage Backlog milestone Oct 19, 2018
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