Issue #1000 - Remove isCustomTab flag (#4656)

2019-08-20 13:00:49 -04:00 · 2019-08-20 13:00:49 -04:00 · 95ef312b1e
parent 1afc0eacd8
commit 95ef312b1e
7 changed files with 257 additions and 126 deletions
--- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt
+++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt
@ -11,32 +11,33 @@ import android.os.Build
 import android.os.Bundle
 import android.util.AttributeSet
 import android.view.View
+import androidx.annotation.CallSuper
 import androidx.annotation.IdRes
+import androidx.annotation.VisibleForTesting
+import androidx.annotation.VisibleForTesting.PROTECTED
 import androidx.appcompat.app.AppCompatActivity
 import androidx.appcompat.widget.Toolbar
 import androidx.lifecycle.lifecycleScope
-import androidx.navigation.NavController
+import androidx.navigation.NavDestination
 import androidx.navigation.fragment.NavHostFragment
 import androidx.navigation.ui.AppBarConfiguration
 import androidx.navigation.ui.NavigationUI
 import com.google.android.material.snackbar.Snackbar
-import io.sentry.Sentry
-import io.sentry.event.Breadcrumb
-import io.sentry.event.BreadcrumbBuilder
 import kotlinx.coroutines.launch
 import mozilla.components.browser.search.SearchEngine
 import mozilla.components.browser.session.Session
 import mozilla.components.browser.session.SessionManager
-import mozilla.components.browser.session.intent.EXTRA_SESSION_ID
 import mozilla.components.concept.engine.EngineView
 import mozilla.components.lib.crash.Crash
 import mozilla.components.support.base.feature.BackHandler
 import mozilla.components.support.ktx.kotlin.isUrl
 import mozilla.components.support.ktx.kotlin.toNormalizedUrl
 import mozilla.components.support.utils.SafeIntent
+import mozilla.components.support.utils.toSafeIntent
 import org.mozilla.fenix.components.FenixSnackbar
 import org.mozilla.fenix.components.isSentryEnabled
 import org.mozilla.fenix.components.metrics.Event
+import org.mozilla.fenix.components.metrics.SentryBreadcrumbsRecorder
 import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.getRootView
 import org.mozilla.fenix.ext.nav
@ -45,93 +46,40 @@ import org.mozilla.fenix.utils.Settings

@SuppressWarnings("TooManyFunctions", "LargeClass")
 open class HomeActivity : AppCompatActivity(), ShareFragment.TabsSharedCallback {
-    open val isCustomTab = false
-    private var sessionObserver: SessionManager.Observer? = null

    lateinit var themeManager: ThemeManager
+    lateinit var browsingModeManager: BrowsingModeManager
+
+    private var sessionObserver: SessionManager.Observer? = null

    private val navHost by lazy {
        supportFragmentManager.findFragmentById(R.id.container) as NavHostFragment
    }

-    lateinit var browsingModeManager: BrowsingModeManager
-
-    private val onDestinationChangedListener =
-        NavController.OnDestinationChangedListener { _, dest, _ ->
-            val fragmentName = resources.getResourceEntryName(dest.id)
-            Sentry.getContext().recordBreadcrumb(
-                BreadcrumbBuilder()
-                    .setCategory("DestinationChanged")
-                    .setMessage("Changing to fragment $fragmentName, isCustomTab: $isCustomTab")
-                    .setLevel(Breadcrumb.Level.INFO)
-                    .build()
-            )
-        }
-
-    override fun onCreate(savedInstanceState: Bundle?) {
+    final override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)

        components.publicSuffixList.prefetch()
-        browsingModeManager = createBrowsingModeManager()
-        themeManager = createThemeManager(browsingModeManager.mode)
+        setupThemeAndBrowsingMode()

        setContentView(R.layout.activity_home)

        setupToolbarAndNavigation()

-        if (Settings.getInstance(this).isTelemetryEnabled && isSentryEnabled()) {
-            navHost.navController.addOnDestinationChangedListener(onDestinationChangedListener)
-        }
-
-        intent
-            ?.let { SafeIntent(it) }
-            ?.let {
-                when {
-                    isCustomTab -> Event.OpenedApp.Source.CUSTOM_TAB
-                    it.isLauncherIntent -> Event.OpenedApp.Source.APP_ICON
-                    it.action == Intent.ACTION_VIEW -> Event.OpenedApp.Source.LINK
-                    else -> null
-                }
+        if (Settings.getInstance(this).isTelemetryEnabled) {
+            if (isSentryEnabled()) {
+                lifecycle.addObserver(SentryBreadcrumbsRecorder(navHost.navController, ::getSentryBreadcrumbMessage))
            }
-            ?.also { components.analytics.metrics.track(Event.OpenedApp(it)) }

-        handleOpenedFromExternalSourceIfNecessary(intent)
-    }
-
-    private fun setupToolbarAndNavigation() {
-        // Add ids to this that we don't want to have a toolbar back button
-        val appBarConfiguration = AppBarConfiguration.Builder().build()
-        val navigationToolbar = findViewById<Toolbar>(R.id.navigationToolbar)
-        setSupportActionBar(navigationToolbar)
-        NavigationUI.setupWithNavController(
-            navigationToolbar,
-            navHost.navController,
-            appBarConfiguration
-        )
-        navigationToolbar.setNavigationOnClickListener {
-            onBackPressed()
+            intent
+                ?.toSafeIntent()
+                ?.let(::getIntentSource)
+                ?.also { components.analytics.metrics.track(Event.OpenedApp(it)) }
        }
        supportActionBar?.hide()
    }

-    override fun onDestroy() {
-        sessionObserver?.let { components.core.sessionManager.unregister(it) }
-        navHost.navController.removeOnDestinationChangedListener(onDestinationChangedListener)
-        super.onDestroy()
-    }
-
-    override fun onNewIntent(intent: Intent?) {
-        super.onNewIntent(intent)
-
-        intent?.let {
-            if (Crash.isCrashIntent(it)) {
-                openToCrashReporter(it)
-            } else {
-                handleOpenedFromExternalSourceIfNecessary(it)
-            }
-        }
-    }
-
+    @CallSuper
    override fun onResume() {
        super.onResume()
        lifecycleScope.launch {
@ -147,21 +95,29 @@ open class HomeActivity : AppCompatActivity(), ShareFragment.TabsSharedCallback
        }
    }

-    override fun onCreateView(
+    /**
+     * Handles intents received when the activity is open.
+     */
+    final override fun onNewIntent(intent: Intent?) {
+        super.onNewIntent(intent)
+        handleCrashIfNecessary(intent)
+        handleOpenedFromExternalSourceIfNecessary(intent)
+    }
+
+    /**
+     * Overrides view inflation to inject a custom [EngineView] from [components].
+     */
+    final override fun onCreateView(
        parent: View?,
        name: String,
        context: Context,
        attrs: AttributeSet
-    ): View? =
-        when (name) {
-            EngineView::class.java.name -> components.core.engine.createView(
-                context,
-                attrs
-            ).asView()
-            else -> super.onCreateView(parent, name, context, attrs)
-        }
+    ): View? = when (name) {
+        EngineView::class.java.name -> components.core.engine.createView(context, attrs).asView()
+        else -> super.onCreateView(parent, name, context, attrs)
+    }

-    override fun onBackPressed() {
+    final override fun onBackPressed() {
        supportFragmentManager.primaryNavigationFragment?.childFragmentManager?.fragments?.forEach {
            if (it is BackHandler && it.onBackPressed()) {
                return
@ -170,6 +126,46 @@ open class HomeActivity : AppCompatActivity(), ShareFragment.TabsSharedCallback
        super.onBackPressed()
    }

+    protected open fun getSentryBreadcrumbMessage(destination: NavDestination): String {
+        val fragmentName = resources.getResourceEntryName(destination.id)
+        return "Changing to fragment $fragmentName, isCustomTab: false"
+    }
+
+    @VisibleForTesting(otherwise = PROTECTED)
+    internal open fun getIntentSource(intent: SafeIntent): Event.OpenedApp.Source? {
+        return when {
+            intent.isLauncherIntent -> Event.OpenedApp.Source.APP_ICON
+            intent.action == Intent.ACTION_VIEW -> Event.OpenedApp.Source.LINK
+            else -> null
+        }
+    }
+
+    private fun setupThemeAndBrowsingMode() {
+        browsingModeManager = createBrowsingModeManager()
+        themeManager = createThemeManager()
+        themeManager.setActivityTheme(this)
+        themeManager.applyStatusBarTheme(this)
+    }
+
+    private fun setupToolbarAndNavigation() {
+        // Add ids to this that we don't want to have a toolbar back button
+        val appBarConfiguration = AppBarConfiguration.Builder().build()
+        val navigationToolbar = findViewById<Toolbar>(R.id.navigationToolbar)
+        setSupportActionBar(navigationToolbar)
+        NavigationUI.setupWithNavController(navigationToolbar, navHost.navController, appBarConfiguration)
+        navigationToolbar.setNavigationOnClickListener {
+            onBackPressed()
+        }
+
+        handleOpenedFromExternalSourceIfNecessary(intent)
+    }
+
+    private fun handleCrashIfNecessary(intent: Intent?) {
+        if (intent != null && Crash.isCrashIntent(intent)) {
+            openToCrashReporter(intent)
+        }
+    }
+
    private fun openToCrashReporter(intent: Intent) {
        val directions = NavGraphDirections.actionGlobalCrashReporter(intent)
        navHost.navController.navigate(directions)
@ -196,13 +192,8 @@ open class HomeActivity : AppCompatActivity(), ShareFragment.TabsSharedCallback
        if (intent?.extras?.getBoolean(OPEN_TO_BROWSER) != true) return

        this.intent.putExtra(OPEN_TO_BROWSER, false)
-        var customTabSessionId: String? = null

-        if (isCustomTab) {
-            customTabSessionId = SafeIntent(intent).getStringExtra(EXTRA_SESSION_ID)
-        }
-
-        openToBrowser(BrowserDirection.FromGlobal, customTabSessionId)
+        openToBrowser(BrowserDirection.FromGlobal, getIntentSessionId(intent.toSafeIntent()))
    }

    @SuppressWarnings("ComplexMethod")
@ -248,6 +239,8 @@ open class HomeActivity : AppCompatActivity(), ShareFragment.TabsSharedCallback
        navHost.navController.navigate(directions)
    }

+    protected open fun getIntentSessionId(intent: SafeIntent): String? = null
+
    @Suppress("LongParameterList")
    fun openToBrowserAndLoad(
        searchTermOrURL: String,
@ -312,18 +305,6 @@ open class HomeActivity : AppCompatActivity(), ShareFragment.TabsSharedCallback
        }
    }

-    private val singleSessionObserver = object : Session.Observer {
-        var urlLoading: String? = null
-
-        override fun onLoadingStateChanged(session: Session, loading: Boolean) {
-            if (loading) {
-                urlLoading = session.url
-            } else if (urlLoading != null && !session.private) {
-                components.analytics.metrics.track(Event.UriOpened)
-            }
-        }
-    }
-
    fun updateThemeForSession(session: Session) {
        val sessionMode = BrowsingMode.fromBoolean(session.private)
        if (sessionMode != browsingModeManager.mode) {
@ -331,31 +312,29 @@ open class HomeActivity : AppCompatActivity(), ShareFragment.TabsSharedCallback
        }
    }

-    private fun createBrowsingModeManager(): BrowsingModeManager {
-        return if (isCustomTab) {
-            CustomTabBrowsingModeManager()
-        } else {
-            DefaultBrowsingModeManager(Settings.getInstance(this)) { mode ->
-                themeManager.currentTheme = mode
-            }
+    protected open fun createBrowsingModeManager(): BrowsingModeManager {
+        return DefaultBrowsingModeManager(Settings.getInstance(this)) { mode ->
+            themeManager.currentTheme = mode
        }
    }

-    private fun createThemeManager(currentTheme: BrowsingMode): ThemeManager {
-        val themeManager = if (isCustomTab) {
-            CustomTabThemeManager()
-        } else {
-            DefaultThemeManager(currentTheme) {
-                themeManager.setActivityTheme(this)
-                recreate()
-            }
-        }
-        themeManager.setActivityTheme(this)
-        themeManager.applyStatusBarTheme(this)
-        return themeManager
+    protected open fun createThemeManager(): ThemeManager {
+        return DefaultThemeManager(browsingModeManager.mode, this)
    }

+    @Suppress("ComplexMethod")
    private fun subscribeToSessions(): SessionManager.Observer {
+        val singleSessionObserver = object : Session.Observer {
+            var urlLoading: String? = null
+
+            override fun onLoadingStateChanged(session: Session, loading: Boolean) {
+                if (loading) {
+                    urlLoading = session.url
+                } else if (urlLoading != null && !session.private) {
+                    components.analytics.metrics.track(Event.UriOpened)
+                }
+            }
+        }

        return object : SessionManager.Observer {
            override fun onAllSessionsRemoved() {
@ -381,7 +360,7 @@ open class HomeActivity : AppCompatActivity(), ShareFragment.TabsSharedCallback
    }

    override fun onTabsShared(tabsSize: Int) {
-        this@HomeActivity.getRootView()?.let {
+        getRootView()?.let {
            FenixSnackbar.make(it, Snackbar.LENGTH_SHORT).setText(
                getString(
                    if (tabsSize == 1) R.string.sync_sent_tab_snackbar else
--- a/app/src/main/java/org/mozilla/fenix/ThemeManager.kt
+++ b/app/src/main/java/org/mozilla/fenix/ThemeManager.kt
@ -101,13 +101,15 @@ abstract class ThemeManager {

 class DefaultThemeManager(
    currentTheme: BrowsingMode,
-    private val onThemeChanged: (BrowsingMode) -> Unit
+    private val activity: Activity
 ) : ThemeManager() {
    override var currentTheme: BrowsingMode = currentTheme
        set(value) {
            if (currentTheme != value) {
                field = value
-                onThemeChanged(value)
+
+                setActivityTheme(activity)
+                activity.recreate()
            }
        }
 }
--- a/app/src/main/java/org/mozilla/fenix/components/metrics/SentryBreadcrumbsRecorder.kt
+++ b/app/src/main/java/org/mozilla/fenix/components/metrics/SentryBreadcrumbsRecorder.kt
@ -0,0 +1,57 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.fenix.components.metrics
+
+import android.os.Bundle
+import androidx.lifecycle.Lifecycle
+import androidx.lifecycle.LifecycleObserver
+import androidx.lifecycle.OnLifecycleEvent
+import androidx.navigation.NavController
+import androidx.navigation.NavDestination
+import io.sentry.Sentry
+import io.sentry.event.Breadcrumb
+import io.sentry.event.BreadcrumbBuilder
+import org.mozilla.fenix.components.isSentryEnabled
+
+/**
+ * Records breadcrumbs in Sentry when the fragment changes.
+ *
+ * Should be registered as a [LifecycleObserver] on an activity if telemetry is enabled.
+ * It will automatically be removed when the lifecycle owner is destroyed.
+ */
+class SentryBreadcrumbsRecorder(
+    private val navController: NavController,
+    private val getBreadcrumbMessage: (NavDestination) -> String
+) : NavController.OnDestinationChangedListener, LifecycleObserver {
+
+    @OnLifecycleEvent(Lifecycle.Event.ON_CREATE)
+    fun onCreate() {
+        if (isSentryEnabled()) {
+            navController.addOnDestinationChangedListener(this)
+        }
+    }
+
+    @OnLifecycleEvent(Lifecycle.Event.ON_DESTROY)
+    fun onDestroy() {
+        navController.removeOnDestinationChangedListener(this)
+    }
+
+    /**
+     * When the destination changes, record the new destination as a breadcrumb.
+     */
+    override fun onDestinationChanged(
+        controller: NavController,
+        destination: NavDestination,
+        arguments: Bundle?
+    ) {
+        Sentry.getContext().recordBreadcrumb(
+            BreadcrumbBuilder()
+                .setCategory("DestinationChanged")
+                .setMessage(getBreadcrumbMessage(destination))
+                .setLevel(Breadcrumb.Level.INFO)
+                .build()
+        )
+    }
+}
--- a/app/src/main/java/org/mozilla/fenix/customtabs/AuthCustomTabActivity.kt
+++ b/app/src/main/java/org/mozilla/fenix/customtabs/AuthCustomTabActivity.kt
@ -1,8 +1,8 @@
-package org.mozilla.fenix.customtabs
-
 /* This Source Code Form is subject to the terms of the Mozilla Public
-   License, v. 2.0. If a copy of the MPL was not distributed with this
-   file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.fenix.customtabs

 import mozilla.components.concept.sync.AccountObserver
 import mozilla.components.concept.sync.OAuthAccount
--- a/app/src/main/java/org/mozilla/fenix/customtabs/CustomTabActivity.kt
+++ b/app/src/main/java/org/mozilla/fenix/customtabs/CustomTabActivity.kt
@ -4,8 +4,25 @@

 package org.mozilla.fenix.customtabs

+import androidx.navigation.NavDestination
+import mozilla.components.browser.session.intent.getSessionId
+import mozilla.components.support.utils.SafeIntent
+import org.mozilla.fenix.CustomTabBrowsingModeManager
+import org.mozilla.fenix.CustomTabThemeManager
 import org.mozilla.fenix.HomeActivity
+import org.mozilla.fenix.components.metrics.Event

 open class CustomTabActivity : HomeActivity() {
-    override val isCustomTab = true
+    final override fun getSentryBreadcrumbMessage(destination: NavDestination): String {
+        val fragmentName = resources.getResourceEntryName(destination.id)
+        return "Changing to fragment $fragmentName, isCustomTab: true"
+    }
+
+    final override fun getIntentSource(intent: SafeIntent) = Event.OpenedApp.Source.CUSTOM_TAB
+
+    final override fun getIntentSessionId(intent: SafeIntent) = intent.getSessionId()
+
+    final override fun createBrowsingModeManager() = CustomTabBrowsingModeManager()
+
+    final override fun createThemeManager() = CustomTabThemeManager()
 }
--- a/app/src/test/java/org/mozilla/fenix/HomeActivityTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/HomeActivityTest.kt
@ -0,0 +1,38 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.fenix
+
+import android.content.Intent
+import kotlinx.coroutines.ObsoleteCoroutinesApi
+import mozilla.components.support.utils.toSafeIntent
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mozilla.fenix.components.metrics.Event
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.annotation.Config
+
+@ObsoleteCoroutinesApi
+@RunWith(RobolectricTestRunner::class)
+@Config(application = TestApplication::class)
+class HomeActivityTest {
+
+    @Test
+    fun getIntentSource() {
+        val activity = HomeActivity()
+
+        val launcherIntent = Intent(Intent.ACTION_MAIN).apply {
+            addCategory(Intent.CATEGORY_LAUNCHER)
+        }.toSafeIntent()
+        assertEquals(Event.OpenedApp.Source.APP_ICON, activity.getIntentSource(launcherIntent))
+
+        val viewIntent = Intent(Intent.ACTION_VIEW).toSafeIntent()
+        assertEquals(Event.OpenedApp.Source.LINK, activity.getIntentSource(viewIntent))
+
+        val otherIntent = Intent().toSafeIntent()
+        assertNull(activity.getIntentSource(otherIntent))
+    }
+}
--- a/app/src/test/java/org/mozilla/fenix/customtabs/CustomTabActivityTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/customtabs/CustomTabActivityTest.kt
@ -0,0 +1,38 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.fenix.customtabs
+
+import android.content.Intent
+import kotlinx.coroutines.ObsoleteCoroutinesApi
+import mozilla.components.support.utils.toSafeIntent
+import org.junit.Assert.assertEquals
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mozilla.fenix.TestApplication
+import org.mozilla.fenix.components.metrics.Event
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.annotation.Config
+
+@ObsoleteCoroutinesApi
+@RunWith(RobolectricTestRunner::class)
+@Config(application = TestApplication::class)
+class CustomTabActivityTest {
+
+    @Test
+    fun getIntentSource() {
+        val activity = CustomTabActivity()
+
+        val launcherIntent = Intent(Intent.ACTION_MAIN).apply {
+            addCategory(Intent.CATEGORY_LAUNCHER)
+        }.toSafeIntent()
+        assertEquals(Event.OpenedApp.Source.CUSTOM_TAB, activity.getIntentSource(launcherIntent))
+
+        val viewIntent = Intent(Intent.ACTION_VIEW).toSafeIntent()
+        assertEquals(Event.OpenedApp.Source.CUSTOM_TAB, activity.getIntentSource(viewIntent))
+
+        val otherIntent = Intent().toSafeIntent()
+        assertEquals(Event.OpenedApp.Source.CUSTOM_TAB, activity.getIntentSource(otherIntent))
+    }
+}