Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Lazily restore on crash/process kill (and always keep EngineSessionState in BrowserState) #8255

Closed
pocmo opened this issue Aug 27, 2020 · 3 comments · Fixed by #8310
Closed

Lazily restore on crash/process kill (and always keep EngineSessionState in BrowserState) #8255

pocmo opened this issue Aug 27, 2020 · 3 comments · Fixed by #8310
Assignees
@pocmo
Labels
<engine-gecko> Component: browser-engine-gecko

Comments

@pocmo
Copy link
Contributor

pocmo commented Aug 27, 2020 •
edited by data-sync-user
Loading

For: mozilla-mobile/fenix#12436

Rough plan:

  • Whenever GeckoSession passes us a state, do not keep it inside GeckoSession, but pass it along to end up in BrowserState
  • This will make EngineSession.saveState() obsolete since the latest state will always be available in BrowserState
  • Never clear EngineSessionState, only clear/close EngineSession
  • On process kill we clear/close the EngineSession. Once needed it will lazily be recreated and the last state will be restored
  • On crash we do not want to lazily restore (to avoid a crash loop) and let the app decide. Instead we could keep the "last state before crash" separately and let the app trigger the restore. As long as this is the case we may not render or restore the crashed EngineSession

┆Issue is synchronized with this Jira Task
@pocmo pocmo added the <engine-gecko> Component: browser-engine-gecko label Aug 27, 2020
@pocmo pocmo self-assigned this Aug 27, 2020
@pocmo
Copy link
Contributor Author

pocmo commented Aug 27, 2020 •
edited
Loading

  • connect to device adb shell
  • $ run-as org.mozilla.fenix.debug
  • Get :tab process id: $ ps -A | grep org.mozilla.fenix.debug:tab
  • Kill process $ kill -9 <PROCESS ID>

pocmo added a commit to pocmo/android-components that referenced this issue Aug 31, 2020
@pocmo
Issue #mozilla-mobile#8255: Add crash reporter with dummy crash servi…
a853000 
…ce to sample browser.
pocmo added a commit to pocmo/android-components that referenced this issue Aug 31, 2020
@pocmo
Issue mozilla-mobile#8255: Lazily restore engine sessions after conte…
9205872 
…nt process kill or crash.

* Instead of keeping the EngineSessionState inside EngineSession, we now always attach it to EngineState and also do not
  clear it anymore.
* If the content process gets killed we now just suspend affected EngineSession instances. They will automatically and
  lazily get restored from the last EngineSessionState once needed.
* On a content process crash we now mark the EngineState as crashed and suspend the EngineSession. We will not restore
  the EngineSession until explicitly restored by the application.
pocmo added a commit to pocmo/android-components that referenced this issue Aug 31, 2020
@pocmo
Issue mozilla-mobile#8255: Lazily restore engine sessions after conte…
3704f34 
…nt process kill or crash.

* Instead of keeping the EngineSessionState inside EngineSession, we now always attach it to EngineState and also do not
  clear it anymore.
* If the content process gets killed we now just suspend affected EngineSession instances. They will automatically and
  lazily get restored from the last EngineSessionState once needed.
* On a content process crash we now mark the EngineState as crashed and suspend the EngineSession. We will not restore
  the EngineSession until explicitly restored by the application.
@pocmo
Copy link
Contributor Author

pocmo commented Sep 1, 2020

Running into a white screen issue: https://bugzilla.mozilla.org/show_bug.cgi?id=1662400

@pocmo pocmo mentioned this issue Sep 1, 2020
Closed
@pocmo
Copy link
Contributor Author

pocmo commented Sep 1, 2020

Emily is seeing the same white zombie page issue in a different scenario in mozilla-mobile/fenix#14459

pocmo added a commit to pocmo/android-components that referenced this issue Sep 1, 2020
@pocmo
Issue #mozilla-mobile#8255: Add crash reporter with dummy crash servi…
1ab984d 
…ce to sample browser.
pocmo added a commit to pocmo/android-components that referenced this issue Sep 1, 2020
@pocmo
Issue mozilla-mobile#8255: Lazily restore engine sessions after conte…
8473cd6 
…nt process kill or crash.

* Instead of keeping the EngineSessionState inside EngineSession, we now always attach it to EngineState and also do not
  clear it anymore.
* If the content process gets killed we now just suspend affected EngineSession instances. They will automatically and
  lazily get restored from the last EngineSessionState once needed.
* On a content process crash we now mark the EngineState as crashed and suspend the EngineSession. We will not restore
  the EngineSession until explicitly restored by the application.
pocmo added a commit to pocmo/android-components that referenced this issue Sep 1, 2020
@pocmo
Issue mozilla-mobile#8255: Lazily restore engine sessions after conte…
945ac6e 
…nt process kill or crash.

* Instead of keeping the EngineSessionState inside EngineSession, we now always attach it to EngineState and also do not
  clear it anymore.
* If the content process gets killed we now just suspend affected EngineSession instances. They will automatically and
  lazily get restored from the last EngineSessionState once needed.
* On a content process crash we now mark the EngineState as crashed and suspend the EngineSession. We will not restore
  the EngineSession until explicitly restored by the application.
@pocmo pocmo linked a pull request Sep 1, 2020 that will close this issue
Lazily restore engine sessions after content process kill or crash. #8310
Merged
pocmo added a commit to pocmo/android-components that referenced this issue Sep 2, 2020
@pocmo
Issue mozilla-mobile#8255: Lazily restore engine sessions after conte…
001bb37 
…nt process kill or crash.

* Instead of keeping the EngineSessionState inside EngineSession, we now always attach it to EngineState and also do not
  clear it anymore.
* If the content process gets killed we now just suspend affected EngineSession instances. They will automatically and
  lazily get restored from the last EngineSessionState once needed.
* On a content process crash we now mark the EngineState as crashed and suspend the EngineSession. We will not restore
  the EngineSession until explicitly restored by the application.
pocmo added a commit to pocmo/android-components that referenced this issue Sep 2, 2020
@pocmo
Issue mozilla-mobile#8255: GeckoEngine: Drop speculative EngineSessio…
b189eb3 
…n if it crashed or was killed.
pocmo added a commit to pocmo/android-components that referenced this issue Sep 2, 2020
@pocmo
Issue mozilla-mobile#8255: GeckoEngine: Drop speculative EngineSessio…
aa7e213 
…n if it crashed or was killed.
pocmo added a commit to pocmo/android-components that referenced this issue Sep 2, 2020
@pocmo
Issue #mozilla-mobile#8255: Add crash reporter with dummy crash servi…
cef0bfc 
…ce to sample browser.
pocmo added a commit to pocmo/android-components that referenced this issue Sep 2, 2020
@pocmo
Issue mozilla-mobile#8255: Lazily restore engine sessions after conte…
35fe396 
…nt process kill or crash.

* Instead of keeping the EngineSessionState inside EngineSession, we now always attach it to EngineState and also do not
  clear it anymore.
* If the content process gets killed we now just suspend affected EngineSession instances. They will automatically and
  lazily get restored from the last EngineSessionState once needed.
* On a content process crash we now mark the EngineState as crashed and suspend the EngineSession. We will not restore
  the EngineSession until explicitly restored by the application.
pocmo added a commit to pocmo/android-components that referenced this issue Sep 2, 2020
@pocmo
Issue mozilla-mobile#8255: Lazily restore engine sessions after conte…
ed63e3d 
…nt process kill or crash.

* Instead of keeping the EngineSessionState inside EngineSession, we now always attach it to EngineState and also do not
  clear it anymore.
* If the content process gets killed we now just suspend affected EngineSession instances. They will automatically and
  lazily get restored from the last EngineSessionState once needed.
* On a content process crash we now mark the EngineState as crashed and suspend the EngineSession. We will not restore
  the EngineSession until explicitly restored by the application.
csadilek added a commit to csadilek/android-components that referenced this issue Sep 2, 2020
@pocmo @csadilek
Issue mozilla-mobile#8255: GeckoEngine: Drop speculative EngineSessio…
68559df 
…n if it crashed or was killed.

Co-authored-by: Christian Sadilek <[email protected]>
pocmo added a commit to pocmo/android-components that referenced this issue Sep 2, 2020
@pocmo
Issue mozilla-mobile#8255: Lazily restore engine sessions after conte…
2ca4918 
…nt process kill or crash.

* Instead of keeping the EngineSessionState inside EngineSession, we now always attach it to EngineState and also do not
  clear it anymore.
* If the content process gets killed we now just suspend affected EngineSession instances. They will automatically and
  lazily get restored from the last EngineSessionState once needed.
* On a content process crash we now mark the EngineState as crashed and suspend the EngineSession. We will not restore
  the EngineSession until explicitly restored by the application.
bors bot pushed a commit that referenced this issue Sep 2, 2020
@MozLando @pocmo
Merge #8320
3f00d96 
8320: Issue #8255: GeckoEngine: Drop speculative EngineSession if it crashed or process was killed r=pocmo a=csadilek



Co-authored-by: Sebastian Kaspari <[email protected]>
pocmo added a commit to pocmo/android-components that referenced this issue Sep 7, 2020
@pocmo
Issue mozilla-mobile#8255: Lazily restore engine sessions after conte…
b3073b9 
…nt process kill or crash.

* Instead of keeping the EngineSessionState inside EngineSession, we now always attach it to EngineState and also do not
  clear it anymore.
* If the content process gets killed we now just suspend affected EngineSession instances. They will automatically and
  lazily get restored from the last EngineSessionState once needed.
* On a content process crash we now mark the EngineState as crashed and suspend the EngineSession. We will not restore
  the EngineSession until explicitly restored by the application.
pocmo added a commit to pocmo/android-components that referenced this issue Sep 7, 2020
@pocmo
Issue mozilla-mobile#8255: Lazily restore engine sessions after conte…
cbbc80e 
…nt process kill or crash.

* Instead of keeping the EngineSessionState inside EngineSession, we now always attach it to EngineState and also do not
  clear it anymore.
* If the content process gets killed we now just suspend affected EngineSession instances. They will automatically and
  lazily get restored from the last EngineSessionState once needed.
* On a content process crash we now mark the EngineState as crashed and suspend the EngineSession. We will not restore
  the EngineSession until explicitly restored by the application.
@bors bors bot closed this as completed in #8310 Sep 7, 2020
This was referenced Sep 7, 2020
Merged
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@pocmo pocmo

Labels
<engine-gecko> Component: browser-engine-gecko
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Lazily restore engine sessions after content process kill or crash. pocmo/android-components
1 participant
@pocmo