From 42e0c6d672dfd64af12bd185a96aee6c664e27b8 Mon Sep 17 00:00:00 2001 From: Arturo Mejia Date: Wed, 2 Jun 2021 21:14:18 -0400 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/19789 Start on Home after some amount of time --- .../java/org/mozilla/fenix/HomeActivity.kt | 33 ++++++++++- .../mozilla/fenix/browser/BrowserFragment.kt | 13 ++++- .../fenix/settings/TabsSettingsFragment.kt | 13 +++++ .../java/org/mozilla/fenix/utils/Settings.kt | 51 +++++++++++++++++ app/src/main/res/values/preference_keys.xml | 5 +- app/src/main/res/values/strings.xml | 9 +++ app/src/main/res/xml/tabs_preferences.xml | 22 ++++++++ .../org/mozilla/fenix/HomeActivityTest.kt | 27 +++++++++ .../fenix/browser/BrowserFragmentTest.kt | 13 +++++ .../org/mozilla/fenix/utils/SettingsTest.kt | 55 +++++++++++++++++++ 10 files changed, 237 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index 3d45483ab..f730ece39 100644 --- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -6,6 +6,7 @@ package org.mozilla.fenix import android.content.Context import android.content.Intent +import android.content.Intent.ACTION_MAIN import android.content.res.Configuration import android.os.Build import android.os.Bundle @@ -116,6 +117,7 @@ import org.mozilla.fenix.tabstray.TabsTrayFragmentDirections import org.mozilla.fenix.theme.DefaultThemeManager import org.mozilla.fenix.theme.ThemeManager import org.mozilla.fenix.utils.BrowsersCache +import org.mozilla.fenix.utils.Settings import java.lang.ref.WeakReference /** @@ -217,8 +219,9 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { it.start() } - if (isActivityColdStarted(intent, savedInstanceState) && - !externalSourceIntentProcessors.any { it.process(intent, navHost.navController, this.intent) }) { + if (!shouldStartOnHome() && + shouldNavigateBrowserFragmentOnCouldStart(savedInstanceState) + ) { navigateToBrowserOnColdStart() } @@ -980,6 +983,32 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { startingIntent.flags and Intent.FLAG_ACTIVITY_LAUNCHED_FROM_HISTORY == 0 } + /** + * Indicates if the user should be redirected to the [BrowserFragment] or to the [HomeFragment], + * links from an external apps should always opened in the [BrowserFragment]. + */ + fun shouldStartOnHome(intent: Intent? = this.intent): Boolean { + return components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + // We only want to open on home when users tap the app, + // we want to ignore other cases when the app gets open by users clicking on links. + getSettings().shouldStartOnHome() && intent?.action == ACTION_MAIN + } + } + + @VisibleForTesting + internal fun getSettings(): Settings = settings() + + private fun shouldNavigateBrowserFragmentOnCouldStart(savedInstanceState: Bundle?): Boolean { + return isActivityColdStarted(intent, savedInstanceState) && + !externalSourceIntentProcessors.any { + it.process( + intent, + navHost.navController, + this.intent + ) + } + } + companion object { const val OPEN_TO_BROWSER = "open_to_browser" const val OPEN_TO_BROWSER_AND_LOAD = "open_to_browser_and_load" diff --git a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt index ac70ff92c..1beabd19e 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt @@ -8,6 +8,7 @@ import android.content.Context import android.os.StrictMode import android.view.View import android.view.ViewGroup +import androidx.annotation.VisibleForTesting import androidx.appcompat.content.res.AppCompatResources import androidx.lifecycle.Observer import androidx.navigation.fragment.findNavController @@ -183,7 +184,7 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler { override fun onStop() { super.onStop() - + updateLastBrowseActivity() pwaOnboardingObserver?.stop() } @@ -292,4 +293,14 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler { ) + ContextMenuCandidate.createOpenInExternalAppCandidate(requireContext(), contextMenuCandidateAppLinksUseCases) } + + /** + * Updates the last time the user was active on the [BrowserFragment]. + * This is useful to determine if the user has to start on the [HomeFragment] + * or it should go directly to the [BrowserFragment]. + */ + @VisibleForTesting + internal fun updateLastBrowseActivity() { + requireContext().settings().lastBrowseActivity = System.currentTimeMillis() + } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/TabsSettingsFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/TabsSettingsFragment.kt index 23124d9a1..982b67e22 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/TabsSettingsFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/TabsSettingsFragment.kt @@ -23,6 +23,9 @@ class TabsSettingsFragment : PreferenceFragmentCompat() { private lateinit var radioOneDay: RadioButtonPreference private lateinit var radioOneWeek: RadioButtonPreference private lateinit var radioOneMonth: RadioButtonPreference + private lateinit var startOnHomeRadioFourHours: RadioButtonPreference + private lateinit var startOnHomeRadioAlways: RadioButtonPreference + private lateinit var startOnHomeRadioNever: RadioButtonPreference override fun onCreatePreferences(savedInstanceState: Bundle?, rootKey: String?) { setPreferencesFromResource(R.xml.tabs_preferences, rootKey) @@ -53,6 +56,10 @@ class TabsSettingsFragment : PreferenceFragmentCompat() { radioOneWeek = requirePreference(R.string.pref_key_close_tabs_after_one_week) radioOneMonth = requirePreference(R.string.pref_key_close_tabs_after_one_month) + startOnHomeRadioFourHours = requirePreference(R.string.pref_key_start_on_home_after_four_hours) + startOnHomeRadioAlways = requirePreference(R.string.pref_key_start_on_home_always) + startOnHomeRadioNever = requirePreference(R.string.pref_key_start_on_home_never) + setupRadioGroups() } @@ -68,5 +75,11 @@ class TabsSettingsFragment : PreferenceFragmentCompat() { radioOneMonth, radioOneWeek ) + + addToRadioGroup( + startOnHomeRadioFourHours, + startOnHomeRadioAlways, + startOnHomeRadioNever + ) } } diff --git a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt index e451a73f7..3c847380d 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt @@ -66,6 +66,7 @@ class Settings(private val appContext: Context) : PreferencesHolder { private const val CFR_COUNT_CONDITION_FOCUS_NOT_INSTALLED = 3 private const val APP_LAUNCHES_TO_SHOW_DEFAULT_BROWSER_CARD = 3 + const val FOUR_HOURS_MS = 60 * 60 * 4 * 1000L const val ONE_DAY_MS = 60 * 60 * 24 * 1000L const val THREE_DAYS_MS = 3 * ONE_DAY_MS const val ONE_WEEK_MS = 60 * 60 * 24 * 7 * 1000L @@ -351,6 +352,56 @@ class Settings(private val appContext: Context) : PreferencesHolder { default = false ) + /** + * Indicates the last time when the user was interacting with the [BrowserFragment], + * This is useful to determine if the user has to start on the [HomeFragment] + * or it should go directly to the [BrowserFragment]. + */ + var lastBrowseActivity by longPreference( + appContext.getPreferenceKey(R.string.pref_key_last_browse_activity_time), + default = timeNowInMillis() + ) + + /** + * Indicates if the user has selected the option to start on the home screen after + * four hours of inactivity. + */ + var startOnHomeAfterFourHours by booleanPreference( + appContext.getPreferenceKey(R.string.pref_key_start_on_home_after_four_hours), + default = true + ) + + /** + * Indicates if the user has selected the option to always start on the home screen. + */ + var startOnHomeAlways by booleanPreference( + appContext.getPreferenceKey(R.string.pref_key_start_on_home_always), + default = false + ) + + /** + * Indicates if the user has selected the option to never start on the home screen. + */ + var startOnHomeNever by booleanPreference( + appContext.getPreferenceKey(R.string.pref_key_start_on_home_never), + default = false + ) + + /** + * Indicates if the user should start on the home screen, based on the user's preferences. + */ + fun shouldStartOnHome(): Boolean { + return when { + startOnHomeAfterFourHours -> timeNowInMillis() - lastBrowseActivity >= FOUR_HOURS_MS + startOnHomeAlways -> true + startOnHomeNever -> false + else -> false + } + } + + @VisibleForTesting + internal fun timeNowInMillis(): Long = System.currentTimeMillis() + fun getTabTimeout(): Long = when { closeTabsAfterOneDay -> ONE_DAY_MS closeTabsAfterOneWeek -> ONE_WEEK_MS diff --git a/app/src/main/res/values/preference_keys.xml b/app/src/main/res/values/preference_keys.xml index 9292fd28a..091bac845 100644 --- a/app/src/main/res/values/preference_keys.xml +++ b/app/src/main/res/values/preference_keys.xml @@ -65,6 +65,7 @@ pref_key_install_pwa_visits pref_key_times_app_opened pref_key_last_review_prompt_shown_time + pref_key_last_browse_activity_time pref_key_last_cfr_shown_time @@ -263,7 +264,9 @@ pref_key_close_tabs_after_one_week pref_key_close_tabs_after_one_month pref_key_allow_third_party_cert_roots - + pref_key_start_on_home_after_four_hours + pref_key_start_on_home_always + pref_key_start_on_home_never pref_key_camera_permissions_needed pref_key_return_to_browser diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 0062939e5..850d43372 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -570,6 +570,15 @@ After one month + + + Start on home + + After four hours + + Always + + Never Close manually diff --git a/app/src/main/res/xml/tabs_preferences.xml b/app/src/main/res/xml/tabs_preferences.xml index d26f828cc..f0cab2e0b 100644 --- a/app/src/main/res/xml/tabs_preferences.xml +++ b/app/src/main/res/xml/tabs_preferences.xml @@ -45,4 +45,26 @@ android:key="@string/pref_key_close_tabs_after_one_month" android:title="@string/close_tabs_after_one_month" /> + + + + + + + + + diff --git a/app/src/test/java/org/mozilla/fenix/HomeActivityTest.kt b/app/src/test/java/org/mozilla/fenix/HomeActivityTest.kt index 9c086a9f5..644566db6 100644 --- a/app/src/test/java/org/mozilla/fenix/HomeActivityTest.kt +++ b/app/src/test/java/org/mozilla/fenix/HomeActivityTest.kt @@ -139,6 +139,33 @@ class HomeActivityTest { assertFalse(activity.isActivityColdStarted(startingIntent, Bundle())) } + @Test + fun `GIVEN the user has been away for a long time WHEN the user opens the app THEN do start on home`() { + val settings: Settings = mockk() + val startingIntent = Intent().apply { + action = Intent.ACTION_MAIN + } + every { activity.applicationContext } returns testContext + + every { settings.shouldStartOnHome() } returns true + every { activity.getSettings() } returns settings + + assertTrue(activity.shouldStartOnHome(startingIntent)) + } + + @Test + fun `GIVEN the user has been away for a long time WHEN opening a link THEN do not start on home`() { + val settings: Settings = mockk() + val startingIntent = Intent().apply { + action = Intent.ACTION_VIEW + } + every { settings.shouldStartOnHome() } returns true + every { activity.getSettings() } returns settings + every { activity.applicationContext } returns testContext + + assertFalse(activity.shouldStartOnHome(startingIntent)) + } + @Test fun `WHEN onCreate is called THEN the duration is measured`() { assertFalse(PerfStartup.homeActivityOnCreate.testHasValue()) // sanity check. diff --git a/app/src/test/java/org/mozilla/fenix/browser/BrowserFragmentTest.kt b/app/src/test/java/org/mozilla/fenix/browser/BrowserFragmentTest.kt index cd32d5ee0..f274d97f2 100644 --- a/app/src/test/java/org/mozilla/fenix/browser/BrowserFragmentTest.kt +++ b/app/src/test/java/org/mozilla/fenix/browser/BrowserFragmentTest.kt @@ -44,6 +44,7 @@ import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.settings import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.onboarding.FenixOnboarding +import org.mozilla.fenix.utils.Settings import java.lang.Exception @ExperimentalCoroutinesApi @@ -369,4 +370,16 @@ class BrowserFragmentTest { override fun getLifecycle(): Lifecycle = lifecycleRegistry } + + @Test + fun `WHEN updating the last browse activity THEN update the associated preference`() { + val settings: Settings = mockk(relaxed = true) + + every { browserFragment.context } returns context + every { context.settings() } returns settings + + browserFragment.updateLastBrowseActivity() + + verify(exactly = 1) { settings.lastBrowseActivity = any() } + } } diff --git a/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt b/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt index 1fb6803f9..ede3d7e0f 100644 --- a/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt +++ b/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt @@ -4,6 +4,8 @@ package org.mozilla.fenix.utils +import io.mockk.every +import io.mockk.spyk import mozilla.components.feature.sitepermissions.SitePermissionsRules import mozilla.components.feature.sitepermissions.SitePermissionsRules.Action.ALLOWED import mozilla.components.feature.sitepermissions.SitePermissionsRules.Action.ASK_TO_ALLOW @@ -19,6 +21,7 @@ import org.junit.runner.RunWith import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.settings.PhoneFeature import org.mozilla.fenix.settings.deletebrowsingdata.DeleteBrowsingDataOnQuitType +import java.util.Calendar @RunWith(FenixRobolectricTestRunner::class) class SettingsTest { @@ -690,4 +693,56 @@ class SettingsTest { // Then assertEquals(2, settings.creditCardsSavedCount) } + + @Test + fun `GIVEN startOnHomeAlways is selected WHEN calling shouldStartOnHome THEN return true`() { + settings.startOnHomeAlways = true + settings.startOnHomeNever = false + settings.startOnHomeAfterFourHours = false + + assertTrue(settings.shouldStartOnHome()) + } + + @Test + fun `GIVEN startOnHomeNever is selected WHEN calling shouldStartOnHome THEN return be false`() { + settings.startOnHomeNever = true + settings.startOnHomeAlways = false + settings.startOnHomeAfterFourHours = false + + assertFalse(settings.shouldStartOnHome()) + } + + @Test + fun `GIVEN startOnHomeAfterFourHours is selected after four hours of inactivity WHEN calling shouldStartOnHome THEN return true`() { + val localSetting = spyk(settings) + val now = Calendar.getInstance() + + localSetting.startOnHomeAfterFourHours = true + localSetting.startOnHomeNever = false + localSetting.startOnHomeAlways = false + + now.timeInMillis = System.currentTimeMillis() + localSetting.lastBrowseActivity = now.timeInMillis + now.add(Calendar.HOUR, 4) + + every { localSetting.timeNowInMillis() } returns now.timeInMillis + + assertTrue(localSetting.shouldStartOnHome()) + } + + @Test + fun `GIVEN startOnHomeAfterFourHours is selected and with recent activity WHEN calling shouldStartOnHome THEN return false`() { + val localSetting = spyk(settings) + val now = System.currentTimeMillis() + + localSetting.startOnHomeAfterFourHours = true + localSetting.startOnHomeNever = false + localSetting.startOnHomeAlways = false + + localSetting.lastBrowseActivity = now + + every { localSetting.timeNowInMillis() } returns now + + assertFalse(localSetting.shouldStartOnHome()) + } }