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

Do not restore immediately when GeckoSession.ContentDelegate.onKill() is getting called #7820

Closed
pocmo opened this issue Jul 22, 2020 · 3 comments
Assignees
Labels
<engine-gecko> Component: browser-engine-gecko 🌟 feature New functionality and improvements

Comments

@pocmo
Copy link
Contributor

pocmo commented Jul 22, 2020

When the content process gets killed by Android (to reclaim resources) then we immediately create a new GeckoSession and restore the last state we have:
https://github.com/mozilla-mobile/android-components/blob/master/components/browser/engine-gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt#L648-L670

Since this should behappening while the app is in the background and the system is low on memory, restoring immediately can cause issues and loops of "kill and restore".

For content process crashes we do something else. We also create a new GeckoSession but do not restore. Instead we mark the session as crashed and notify the app. The app then usually shows some "This tab crashed. Do you want to restore?" UI and then the user can decide to restore or close the tab.
https://github.com/mozilla-mobile/android-components/blob/master/components/browser/engine-gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt#L640-L646

For content process kills we should delay restoring (and I guess recreating the GeckoSession?) until the tab is the next time rendered (when the app gets resumed).

┆Issue is synchronized with this Jira Task

@pocmo pocmo added 🌟 feature New functionality and improvements <engine-gecko> Component: browser-engine-gecko labels Jul 22, 2020
@pocmo pocmo self-assigned this Jul 22, 2020
@pocmo
Copy link
Contributor Author

pocmo commented Jul 23, 2020

This would be a thousand times easier if EngineSession instances wouldn't exist in both, SessionManager and BrowserStore. I will take a little detour and see if we are ready to move them completely.

@pocmo
Copy link
Contributor Author

pocmo commented Jul 27, 2020

Yep, we will do the move first:
#7867

@data-sync-user data-sync-user changed the title Do not restore immediately when GeckoSession.ContentDelegate.onKill() is getting called FNX3-22838 ⁃ Do not restore immediately when GeckoSession.ContentDelegate.onKill() is getting called Aug 5, 2020
@st3fan st3fan changed the title FNX3-22838 ⁃ Do not restore immediately when GeckoSession.ContentDelegate.onKill() is getting called Do not restore immediately when GeckoSession.ContentDelegate.onKill() is getting called Aug 5, 2020
@pocmo
Copy link
Contributor Author

pocmo commented Sep 7, 2020

This was fixed in #8310 / #8255.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
<engine-gecko> Component: browser-engine-gecko 🌟 feature New functionality and improvements
Projects
None yet
Development

No branches or pull requests

1 participant