Skip to content

Commit

Permalink
Issue mozilla-mobile#16435: Finish ExternalAppBrowserActivity and rem…
Browse files Browse the repository at this point in the history
…ove task if linked tab doesn't exist anymore.

This doesn't fix the underlying issue of mozilla-mobile#16435, but a symptom we have seen quite often in different
situations: When the ExternalAppBrowserActivity is not linked to a tab anymore then it falls back
to displaying a (partially broken) browser UI. With multiple browser activities at the same time
sooner or later we crash with a "display already acquired" error because both activities try to
render the same tab.
  • Loading branch information
pocmo authored and csadilek committed Nov 30, 2020
1 parent 0bd3654 commit 776b64f
Showing 1 changed file with 29 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import androidx.navigation.NavDestination
import androidx.navigation.NavDirections
import kotlinx.android.synthetic.main.activity_home.*
import mozilla.components.browser.session.runWithSession
import mozilla.components.browser.state.selector.findCustomTab
import mozilla.components.browser.state.state.SessionState
import mozilla.components.concept.engine.manifest.WebAppManifestParser
import mozilla.components.feature.intent.ext.getSessionId
import mozilla.components.feature.pwa.ext.getWebAppManifest
Expand All @@ -24,7 +26,20 @@ import java.security.InvalidParameterException
* Activity that holds the [ExternalAppBrowserFragment] that is launched within an external app,
* such as custom tabs and progressive web apps.
*/
@Suppress("TooManyFunctions")
open class ExternalAppBrowserActivity : HomeActivity() {
override fun onResume() {
super.onResume()

if (!hasExternalTab()) {
// An ExternalAppBrowserActivity is always bound to a specific tab. If this tab doesn't
// exist anymore on resume then this activity has nothing to display anymore. Let's just
// finish it AND remove this task to avoid it hanging around in the recent apps screen.
// Without this the parent HomeActivity class may decide to show the browser UI and we
// end up with multiple browsers (causing "display already acquired" crashes).
finishAndRemoveTask()
}
}

final override fun getBreadcrumbMessage(destination: NavDestination): String {
val fragmentName = resources.getResourceEntryName(destination.id)
Expand Down Expand Up @@ -86,8 +101,7 @@ open class ExternalAppBrowserActivity : HomeActivity() {
// When this activity finishes, the process is staying around and the session still
// exists then remove it now to free all its resources. Once this activity is finished
// then there's no way to get back to it other than relaunching it.
val sessionId = getIntentSessionId(SafeIntent(intent))
components.core.sessionManager.runWithSession(sessionId) { session ->
components.core.sessionManager.runWithSession(getExternalTabId()) { session ->
// If the custom tag config has been removed we are opening this in normal browsing
if (session.customTabConfig != null) {
remove(session)
Expand All @@ -96,4 +110,17 @@ open class ExternalAppBrowserActivity : HomeActivity() {
}
}
}

private fun hasExternalTab(): Boolean {
return getExternalTab() != null
}

private fun getExternalTab(): SessionState? {
val id = getExternalTabId() ?: return null
return components.core.store.state.findCustomTab(id)
}

private fun getExternalTabId(): String? {
return getIntentSessionId(SafeIntent(intent))
}
}

0 comments on commit 776b64f

Please sign in to comment.