From 3be06953d6a38f768997feb8872813630f7eab55 Mon Sep 17 00:00:00 2001 From: Severin Rudie Date: Thu, 3 Oct 2019 10:23:13 -0700 Subject: [PATCH] For #4780 switch off private mode (#5614) * Do not launch in Private Mode When the app launches do not launch in Private Mode in order to prevent usage leaks to other users of the device. * Issue #4780: add comments to use private mode * For #4780: write tests for clear private mode on create app * For #4780: clear private mode when privacy notification is removed --- .../org/mozilla/fenix/FenixApplication.kt | 13 +++++ .../session/SessionNotificationService.kt | 6 +++ .../java/org/mozilla/fenix/utils/Settings.kt | 8 +++ .../org/mozilla/fenix/FenixApplicationTest.kt | 49 +++++++++++++++++++ 4 files changed, 76 insertions(+) create mode 100644 app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt index 9e5826def..42c68accc 100644 --- a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt +++ b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt @@ -36,6 +36,7 @@ import org.mozilla.fenix.components.Components import org.mozilla.fenix.ext.settings import org.mozilla.fenix.session.NotificationSessionObserver import org.mozilla.fenix.session.VisibilityLifecycleCallback +import org.mozilla.fenix.utils.Settings import java.io.File @SuppressLint("Registered") @@ -115,6 +116,10 @@ open class FenixApplication : Application() { if ((System.currentTimeMillis() - settings().lastPlacesStorageMaintenance) > ONE_DAY_MILLIS) { runStorageMaintenance() } + + // This needs to be called before the theme is set. No BrowsingModeManager is available + // at this point, which is why this is set directly + maybeClearPrivateMode() } private fun runStorageMaintenance() { @@ -126,6 +131,14 @@ open class FenixApplication : Application() { settings().lastPlacesStorageMaintenance = System.currentTimeMillis() } + /** + * Clears private mode. This is done in order to avoid leaking the fact that + * private mode was in use during the previous session. + */ + fun maybeClearPrivateMode(settings: Settings = settings()) { + if (!settings.alwaysOpenInPrivateMode) settings.usePrivateMode = false + } + private fun registerRxExceptionHandling() { RxJavaPlugins.setErrorHandler { throw it.cause ?: it diff --git a/app/src/main/java/org/mozilla/fenix/session/SessionNotificationService.kt b/app/src/main/java/org/mozilla/fenix/session/SessionNotificationService.kt index 38de2ae2a..60ebf6db4 100644 --- a/app/src/main/java/org/mozilla/fenix/session/SessionNotificationService.kt +++ b/app/src/main/java/org/mozilla/fenix/session/SessionNotificationService.kt @@ -22,6 +22,7 @@ import mozilla.components.support.utils.ThreadUtils import org.mozilla.fenix.R import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.ext.application import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.metrics import org.mozilla.fenix.ext.sessionsOfType @@ -62,6 +63,11 @@ class SessionNotificationService : Service() { override fun onTaskRemoved(rootIntent: Intent) { components.core.sessionManager.removeAndCloseAllPrivateSessions() + // This is currently safe because we remove this service while destroying activities. If + // this service is ever removed while HomeActivity is still active, this could cause + // theming issues. See usePrivateMode kdoc + baseContext.application.maybeClearPrivateMode() + stopForeground(true) stopSelf() } 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 cf899a6c8..bb4327d6c 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt @@ -22,6 +22,7 @@ import org.mozilla.fenix.BuildConfig import org.mozilla.fenix.Config import org.mozilla.fenix.FeatureFlags import org.mozilla.fenix.R +import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.components.metrics.MozillaProductDetector import org.mozilla.fenix.ext.getPreferenceKey import org.mozilla.fenix.settings.PhoneFeature @@ -82,6 +83,13 @@ class Settings private constructor( default = "" ) + /** + * Warning: when possible, prefer to set this via [BrowsingModeManager]. + * + * [BrowsingModeManager] defines a callback to be hit whenever this setting changes. For + * example, setting this value directly at the wrong time could cause the private theme to + * not be applied. + */ var usePrivateMode by booleanPreference( appContext.getPreferenceKey(R.string.pref_key_private_mode), default = false diff --git a/app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt b/app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt new file mode 100644 index 000000000..a9eb5f686 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt @@ -0,0 +1,49 @@ +package org.mozilla.fenix + +import io.mockk.MockKAnnotations +import io.mockk.every +import io.mockk.impl.annotations.MockK +import io.mockk.just +import io.mockk.runs +import io.mockk.verify +import kotlinx.coroutines.ObsoleteCoroutinesApi +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.fenix.utils.Settings +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +@ObsoleteCoroutinesApi +@RunWith(RobolectricTestRunner::class) +@Config(application = TestApplication::class) +class FenixApplicationTest { + + lateinit var application: FenixApplication + @MockK lateinit var settings: Settings + + @Before + fun before() { + MockKAnnotations.init(this) + every { settings setProperty "usePrivateMode" value any() } just runs + application = TestApplication() + } + + @Test + fun `GIVEN alwaysOpenInPrivateMode is active WHEN maybeClearPrivateMode is called THEN private mode should not be changed`() { + every { settings.alwaysOpenInPrivateMode } returns true + + application.maybeClearPrivateMode(settings) + + verify(exactly = 0) { settings.usePrivateMode = any() } + } + + @Test + fun `GIVEN alwaysOpenInPrivateMode is inactive WHEN maybeClearPrivateMode is called THEN private mode should be disabled`() { + every { settings.alwaysOpenInPrivateMode } returns false + + application.maybeClearPrivateMode(settings) + + verify(exactly = 1) { settings.usePrivateMode = false } + } +}