From 3b63afbf48ac837d02a3b890bc4343c1e547426e Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Wed, 10 Jun 2020 17:45:21 +0200 Subject: [PATCH 01/13] Register when app is opened with a intent with params and Fire pixels when app restarts due to data clearing onAppForeground --- .../duckduckgo/app/browser/BrowserActivity.kt | 11 +- .../com/duckduckgo/app/di/PrivacyModule.kt | 5 +- .../app/fire/AutomaticDataClearer.kt | 5 +- .../app/fire/UnsentForgetAllPixelStore.kt | 102 ++++++++++++++++++ .../app/global/DuckDuckGoApplication.kt | 10 ++ .../duckduckgo/app/statistics/pixels/Pixel.kt | 2 + .../app/systemsearch/SystemSearchActivity.kt | 9 ++ 7 files changed, 136 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index 4850b08715cd..d42ba900e345 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -34,6 +34,7 @@ import com.duckduckgo.app.browser.rating.ui.GiveFeedbackDialogFragment import com.duckduckgo.app.browser.rating.ui.RateAppDialogFragment import com.duckduckgo.app.feedback.ui.common.FeedbackActivity import com.duckduckgo.app.fire.DataClearer +import com.duckduckgo.app.fire.UnsentPixelDataClearerAppRestartedWithIntentStoreSharedPreferences import com.duckduckgo.app.global.ApplicationClearDataState import com.duckduckgo.app.global.DuckDuckGoActivity import com.duckduckgo.app.global.intentText @@ -64,6 +65,9 @@ class BrowserActivity : DuckDuckGoActivity(), CoroutineScope by MainScope() { @Inject lateinit var playStoreUtils: PlayStoreUtils + @Inject + lateinit var unsentPixelDataClearerRestart: UnsentPixelDataClearerAppRestartedWithIntentStoreSharedPreferences + private var currentTab: BrowserTabFragment? = null private val viewModel: BrowserViewModel by bindViewModel() @@ -81,11 +85,9 @@ class BrowserActivity : DuckDuckGoActivity(), CoroutineScope by MainScope() { @SuppressLint("MissingSuperCall") override fun onCreate(savedInstanceState: Bundle?) { super.daggerInject() - - renderer = BrowserStateRenderer() - Timber.i("onCreate called. freshAppLaunch: ${dataClearer.isFreshAppLaunch}, savedInstanceState: $savedInstanceState") - + unsentPixelDataClearerRestart.registerIntent(intent) + renderer = BrowserStateRenderer() val newInstanceState = if (dataClearer.isFreshAppLaunch) null else savedInstanceState instanceStateBundles = CombinedInstanceState(originalInstanceState = savedInstanceState, newInstanceState = newInstanceState) @@ -110,6 +112,7 @@ class BrowserActivity : DuckDuckGoActivity(), CoroutineScope by MainScope() { override fun onNewIntent(intent: Intent?) { super.onNewIntent(intent) Timber.i("onNewIntent: $intent") + unsentPixelDataClearerRestart.registerIntent(intent) if (dataClearer.dataClearerState.value == ApplicationClearDataState.FINISHED) { Timber.i("Automatic data clearer has finished, so processing intent now") diff --git a/app/src/main/java/com/duckduckgo/app/di/PrivacyModule.kt b/app/src/main/java/com/duckduckgo/app/di/PrivacyModule.kt index 195a56d4dcdb..f1839e3b76d1 100644 --- a/app/src/main/java/com/duckduckgo/app/di/PrivacyModule.kt +++ b/app/src/main/java/com/duckduckgo/app/di/PrivacyModule.kt @@ -73,9 +73,10 @@ class PrivacyModule { workManager: WorkManager, settingsDataStore: SettingsDataStore, clearDataAction: ClearDataAction, - dataClearerTimeKeeper: BackgroundTimeKeeper + dataClearerTimeKeeper: BackgroundTimeKeeper, + unsentPixelDataClearerRestart: UnsentPixelDataClearerAppRestartedWithIntentStoreSharedPreferences ): DataClearer { - return AutomaticDataClearer(workManager, settingsDataStore, clearDataAction, dataClearerTimeKeeper) + return AutomaticDataClearer(workManager, settingsDataStore, clearDataAction, dataClearerTimeKeeper, unsentPixelDataClearerRestart) } @Provides diff --git a/app/src/main/java/com/duckduckgo/app/fire/AutomaticDataClearer.kt b/app/src/main/java/com/duckduckgo/app/fire/AutomaticDataClearer.kt index b934692bdbc5..b956904b26fe 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/AutomaticDataClearer.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/AutomaticDataClearer.kt @@ -44,7 +44,8 @@ class AutomaticDataClearer( private val workManager: WorkManager, private val settingsDataStore: SettingsDataStore, private val clearDataAction: ClearDataAction, - private val dataClearerTimeKeeper: BackgroundTimeKeeper + private val dataClearerTimeKeeper: BackgroundTimeKeeper, + private val unsentPixelDataClearerRestart: UnsentPixelDataClearerAppRestartedWithIntentStoreSharedPreferences ) : DataClearer, LifecycleObserver, CoroutineScope { private val clearJob: Job = Job() @@ -159,7 +160,7 @@ class AutomaticDataClearer( Timber.i("All data now cleared, will restart process? $processNeedsRestarted") if (processNeedsRestarted) { clearDataAction.setAppUsedSinceLastClearFlag(false) - + unsentPixelDataClearerRestart.incrementCount() // need a moment to draw background color (reduces flickering UX) Handler().postDelayed(100) { Timber.i("Will now restart process") diff --git a/app/src/main/java/com/duckduckgo/app/fire/UnsentForgetAllPixelStore.kt b/app/src/main/java/com/duckduckgo/app/fire/UnsentForgetAllPixelStore.kt index d3ccc88c9ce2..122f7a076345 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/UnsentForgetAllPixelStore.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/UnsentForgetAllPixelStore.kt @@ -17,10 +17,21 @@ package com.duckduckgo.app.fire import android.content.Context +import android.content.Intent import android.content.SharedPreferences +import androidx.annotation.UiThread import androidx.annotation.VisibleForTesting import androidx.core.content.edit +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleObserver +import androidx.lifecycle.OnLifecycleEvent +import com.duckduckgo.app.global.intentText +import com.duckduckgo.app.statistics.pixels.Pixel +import com.duckduckgo.app.systemsearch.SystemSearchActivity +import kotlinx.coroutines.launch +import timber.log.Timber import javax.inject.Inject +import javax.inject.Singleton interface UnsentForgetAllPixelStore { val pendingPixelCountClearData: Int @@ -69,3 +80,94 @@ class UnsentForgetAllPixelStoreSharedPreferences @Inject constructor(private val const val KEY_TIMESTAMP_LAST_CLEARED = "KEY_TIMESTAMP_LAST_CLEARED" } } + + +/** + * Stores information about unsent clear data Pixels. + * + * When writing values here to SharedPreferences, it is crucial to use `commit = true`. As otherwise the change can be lost in the process restart. + */ +@Singleton +class UnsentPixelDataClearerAppRestartedWithIntentStoreSharedPreferences @Inject constructor( + private val context: Context, + private val pixel: Pixel +): LifecycleObserver { + private var externalIntent: Boolean = false + + private val pendingAppForegroundRestart: Int + get() = preferences.getInt(KEY_UNSENT_CLEAR_APP_RESTARTED_PIXELS, 0) + + private val pendingAppForegroundRestartWithIntent: Int + get() = preferences.getInt(KEY_UNSENT_CLEAR_APP_RESTARTED_WITH_INTENT_PIXELS, 0) + + @UiThread + @OnLifecycleEvent(Lifecycle.Event.ON_STOP) + fun onAppForegrounded() { + Timber.i("Registered App on_stop") + externalIntent = false + } + + fun registerIntent(intent: Intent?) { + if (widgetActivity(intent)) { + Timber.i("Registered Intent with extras") + externalIntent = true + } else { + if (!intent?.intentText.isNullOrEmpty()) { + externalIntent = true + Timber.i("Registered Intent with extras") + } + } + } + + fun incrementCount() { + if (externalIntent) { + Timber.i("Registered restart with intent") + incrementCount(pendingAppForegroundRestart, KEY_UNSENT_CLEAR_APP_RESTARTED_WITH_INTENT_PIXELS) + } else { + Timber.i("Registered restart w/ intent") + incrementCount(pendingAppForegroundRestartWithIntent, KEY_UNSENT_CLEAR_APP_RESTARTED_PIXELS) + } + } + + fun firePendingPixels() { + firePendingPixels(pendingAppForegroundRestart, Pixel.PixelName.FORGET_ALL_AUTO_RESTART) + firePendingPixels(pendingAppForegroundRestartWithIntent, Pixel.PixelName.FORGET_ALL_AUTO_RESTART_WITH_INTENT) + resetCount() + } + + private fun incrementCount(counter: Int, sharedPrefKey: String) { + val updated = counter + 1 + preferences.edit(commit = true) { + putInt(sharedPrefKey, updated) + } + } + + private fun firePendingPixels(counter: Int, pixelName: Pixel.PixelName) { + if (counter > 0) { + for (i in 1..counter) { + Timber.i("Fired pixel: ${pixelName.pixelName}/$counter") + pixel.fire(pixelName) + } + } + } + + private fun resetCount() { + preferences.edit(commit = true) { + putInt(KEY_UNSENT_CLEAR_APP_RESTARTED_PIXELS, 0) + putInt(KEY_UNSENT_CLEAR_APP_RESTARTED_WITH_INTENT_PIXELS, 0) + } + Timber.i("counter reset") + } + + private fun widgetActivity(intent: Intent?): Boolean = intent?.component?.className?.contains(SystemSearchActivity::class.java.canonicalName.orEmpty()) == true + + private val preferences: SharedPreferences + get() = context.getSharedPreferences(FILENAME, Context.MODE_PRIVATE) + + companion object { + @VisibleForTesting + const val FILENAME = "com.duckduckgo.app.fire.unsentpixels.settings" + const val KEY_UNSENT_CLEAR_APP_RESTARTED_PIXELS = "KEY_UNSENT_CLEAR_APP_RESTARTED_PIXELS" + const val KEY_UNSENT_CLEAR_APP_RESTARTED_WITH_INTENT_PIXELS = "KEY_UNSENT_CLEAR_APP_RESTARTED_WITH_INTENT_PIXELS" + } +} \ No newline at end of file diff --git a/app/src/main/java/com/duckduckgo/app/global/DuckDuckGoApplication.kt b/app/src/main/java/com/duckduckgo/app/global/DuckDuckGoApplication.kt index 8059dfe94fac..b458cafefc7b 100644 --- a/app/src/main/java/com/duckduckgo/app/global/DuckDuckGoApplication.kt +++ b/app/src/main/java/com/duckduckgo/app/global/DuckDuckGoApplication.kt @@ -30,6 +30,7 @@ import com.duckduckgo.app.di.DaggerAppComponent import com.duckduckgo.app.fire.DataClearer import com.duckduckgo.app.fire.FireActivity import com.duckduckgo.app.fire.UnsentForgetAllPixelStore +import com.duckduckgo.app.fire.UnsentPixelDataClearerAppRestartedWithIntentStoreSharedPreferences import com.duckduckgo.app.global.Theming.initializeTheme import com.duckduckgo.app.global.initialization.AppDataLoader import com.duckduckgo.app.global.install.AppInstallStore @@ -110,6 +111,9 @@ open class DuckDuckGoApplication : HasAndroidInjector, Application(), LifecycleO @Inject lateinit var unsentForgetAllPixelStore: UnsentForgetAllPixelStore + @Inject + lateinit var unsentPixelDataClearerRestart: UnsentPixelDataClearerAppRestartedWithIntentStoreSharedPreferences + @Inject lateinit var offlinePixelScheduler: OfflinePixelScheduler @@ -170,6 +174,7 @@ open class DuckDuckGoApplication : HasAndroidInjector, Application(), LifecycleO it.addObserver(appDaysUsedRecorder) it.addObserver(defaultBrowserObserver) it.addObserver(appEnjoymentLifecycleObserver) + it.addObserver(unsentPixelDataClearerRestart) } if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N_MR1) { @@ -254,6 +259,11 @@ open class DuckDuckGoApplication : HasAndroidInjector, Application(), LifecycleO } unsentForgetAllPixelStore.resetCount() } + submitUnsentFireAppRestartedWithIntentPixels() + } + + private fun submitUnsentFireAppRestartedWithIntentPixels() { + unsentPixelDataClearerRestart.firePendingPixels() } /** diff --git a/app/src/main/java/com/duckduckgo/app/statistics/pixels/Pixel.kt b/app/src/main/java/com/duckduckgo/app/statistics/pixels/Pixel.kt index df914befcfbb..9e585fef9ca8 100644 --- a/app/src/main/java/com/duckduckgo/app/statistics/pixels/Pixel.kt +++ b/app/src/main/java/com/duckduckgo/app/statistics/pixels/Pixel.kt @@ -36,6 +36,8 @@ interface Pixel { FORGET_ALL_PRESSED_BROWSING("mf_bp"), FORGET_ALL_PRESSED_TABSWITCHING("mf_tp"), FORGET_ALL_EXECUTED("mf"), + FORGET_ALL_AUTO_RESTART("mf_r"), + FORGET_ALL_AUTO_RESTART_WITH_INTENT("mf_ri"), APPLICATION_CRASH("m_d_ac"), APPLICATION_CRASH_GLOBAL("m_d_ac_g"), diff --git a/app/src/main/java/com/duckduckgo/app/systemsearch/SystemSearchActivity.kt b/app/src/main/java/com/duckduckgo/app/systemsearch/SystemSearchActivity.kt index 1909795d94e8..fde699730a27 100644 --- a/app/src/main/java/com/duckduckgo/app/systemsearch/SystemSearchActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/systemsearch/SystemSearchActivity.kt @@ -34,6 +34,7 @@ import com.duckduckgo.app.browser.BrowserActivity import com.duckduckgo.app.browser.R import com.duckduckgo.app.browser.autocomplete.BrowserAutoCompleteSuggestionsAdapter import com.duckduckgo.app.browser.omnibar.OmnibarScrolling +import com.duckduckgo.app.fire.UnsentPixelDataClearerAppRestartedWithIntentStoreSharedPreferences import com.duckduckgo.app.global.DuckDuckGoActivity import com.duckduckgo.app.global.view.TextChangedWatcher import com.duckduckgo.app.global.view.hideKeyboard @@ -43,6 +44,7 @@ import com.duckduckgo.app.systemsearch.SystemSearchViewModel.Command.* import com.duckduckgo.app.systemsearch.SystemSearchViewModel.SystemSearchResultsViewState import kotlinx.android.synthetic.main.activity_system_search.* import kotlinx.android.synthetic.main.include_system_search_onboarding.* +import timber.log.Timber import javax.inject.Inject class SystemSearchActivity : DuckDuckGoActivity() { @@ -53,6 +55,9 @@ class SystemSearchActivity : DuckDuckGoActivity() { @Inject lateinit var omnibarScrolling: OmnibarScrolling + @Inject + lateinit var unsentPixelDataClearerRestart: UnsentPixelDataClearerAppRestartedWithIntentStoreSharedPreferences + private val viewModel: SystemSearchViewModel by bindViewModel() private lateinit var autocompleteSuggestionsAdapter: BrowserAutoCompleteSuggestionsAdapter private lateinit var deviceAppSuggestionsAdapter: DeviceAppSuggestionsAdapter @@ -66,6 +71,8 @@ class SystemSearchActivity : DuckDuckGoActivity() { override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) + Timber.i("onCreate called") + unsentPixelDataClearerRestart.registerIntent(intent) setContentView(R.layout.activity_system_search) configureObservers() configureOnboarding() @@ -82,6 +89,8 @@ class SystemSearchActivity : DuckDuckGoActivity() { override fun onNewIntent(newIntent: Intent?) { super.onNewIntent(newIntent) + Timber.i("onNewIntent") + unsentPixelDataClearerRestart.registerIntent(newIntent) viewModel.resetViewState() newIntent?.let { sendLaunchPixels(it) } } From c103abd8848dbd0d1a36620990b25c53bb1ccd01 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Fri, 12 Jun 2020 10:11:30 +0200 Subject: [PATCH 02/13] move DataClearerForegroundAppRestartPixel into its own class --- .../duckduckgo/app/browser/BrowserActivity.kt | 4 +- .../com/duckduckgo/app/di/PrivacyModule.kt | 2 +- .../app/fire/AutomaticDataClearer.kt | 2 +- .../DataClearerForegroundAppRestartPixel.kt | 117 ++++++++++++++++++ .../app/fire/UnsentForgetAllPixelStore.kt | 91 -------------- .../app/global/DuckDuckGoApplication.kt | 4 +- .../app/systemsearch/SystemSearchActivity.kt | 4 +- 7 files changed, 125 insertions(+), 99 deletions(-) create mode 100644 app/src/main/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixel.kt diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index d42ba900e345..d3a7b6313942 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -34,7 +34,7 @@ import com.duckduckgo.app.browser.rating.ui.GiveFeedbackDialogFragment import com.duckduckgo.app.browser.rating.ui.RateAppDialogFragment import com.duckduckgo.app.feedback.ui.common.FeedbackActivity import com.duckduckgo.app.fire.DataClearer -import com.duckduckgo.app.fire.UnsentPixelDataClearerAppRestartedWithIntentStoreSharedPreferences +import com.duckduckgo.app.fire.DataClearerForegroundAppRestartPixel import com.duckduckgo.app.global.ApplicationClearDataState import com.duckduckgo.app.global.DuckDuckGoActivity import com.duckduckgo.app.global.intentText @@ -66,7 +66,7 @@ class BrowserActivity : DuckDuckGoActivity(), CoroutineScope by MainScope() { lateinit var playStoreUtils: PlayStoreUtils @Inject - lateinit var unsentPixelDataClearerRestart: UnsentPixelDataClearerAppRestartedWithIntentStoreSharedPreferences + lateinit var unsentPixelDataClearerRestart: DataClearerForegroundAppRestartPixel private var currentTab: BrowserTabFragment? = null diff --git a/app/src/main/java/com/duckduckgo/app/di/PrivacyModule.kt b/app/src/main/java/com/duckduckgo/app/di/PrivacyModule.kt index f1839e3b76d1..d96ea95aa815 100644 --- a/app/src/main/java/com/duckduckgo/app/di/PrivacyModule.kt +++ b/app/src/main/java/com/duckduckgo/app/di/PrivacyModule.kt @@ -74,7 +74,7 @@ class PrivacyModule { settingsDataStore: SettingsDataStore, clearDataAction: ClearDataAction, dataClearerTimeKeeper: BackgroundTimeKeeper, - unsentPixelDataClearerRestart: UnsentPixelDataClearerAppRestartedWithIntentStoreSharedPreferences + unsentPixelDataClearerRestart: DataClearerForegroundAppRestartPixel ): DataClearer { return AutomaticDataClearer(workManager, settingsDataStore, clearDataAction, dataClearerTimeKeeper, unsentPixelDataClearerRestart) } diff --git a/app/src/main/java/com/duckduckgo/app/fire/AutomaticDataClearer.kt b/app/src/main/java/com/duckduckgo/app/fire/AutomaticDataClearer.kt index b956904b26fe..d2aeb9731c14 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/AutomaticDataClearer.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/AutomaticDataClearer.kt @@ -45,7 +45,7 @@ class AutomaticDataClearer( private val settingsDataStore: SettingsDataStore, private val clearDataAction: ClearDataAction, private val dataClearerTimeKeeper: BackgroundTimeKeeper, - private val unsentPixelDataClearerRestart: UnsentPixelDataClearerAppRestartedWithIntentStoreSharedPreferences + private val unsentPixelDataClearerRestart: DataClearerForegroundAppRestartPixel ) : DataClearer, LifecycleObserver, CoroutineScope { private val clearJob: Job = Job() diff --git a/app/src/main/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixel.kt b/app/src/main/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixel.kt new file mode 100644 index 000000000000..6c7528fe600c --- /dev/null +++ b/app/src/main/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixel.kt @@ -0,0 +1,117 @@ +/* + * Copyright (c) 2020 DuckDuckGo + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.duckduckgo.app.fire + +import android.content.Context +import android.content.Intent +import android.content.SharedPreferences +import androidx.annotation.UiThread +import androidx.annotation.VisibleForTesting +import androidx.core.content.edit +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleObserver +import androidx.lifecycle.OnLifecycleEvent +import com.duckduckgo.app.global.intentText +import com.duckduckgo.app.statistics.pixels.Pixel +import com.duckduckgo.app.systemsearch.SystemSearchActivity +import timber.log.Timber +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Stores information about unsent automatic data clearer restart Pixels, detecting if user started the app from an external Intent. + * Contains logic to sent unsent pixels. + * + * When writing values here to SharedPreferences, it is crucial to use `commit = true`. As otherwise the change can be lost in the process restart. + */ +@Singleton +class DataClearerForegroundAppRestartPixel @Inject constructor( + private val context: Context, + private val pixel: Pixel +) : LifecycleObserver { + private var detectedUserIntent: Boolean = false + + private val pendingAppForegroundRestart: Int + get() = preferences.getInt(KEY_UNSENT_CLEAR_APP_RESTARTED_PIXELS, 0) + + private val pendingAppForegroundRestartWithIntent: Int + get() = preferences.getInt(KEY_UNSENT_CLEAR_APP_RESTARTED_WITH_INTENT_PIXELS, 0) + + @UiThread + @OnLifecycleEvent(Lifecycle.Event.ON_STOP) + fun onAppForegrounded() { + Timber.i("Registered App on_stop") + detectedUserIntent = false + } + + fun registerIntent(intent: Intent?) { + detectedUserIntent = widgetActivity(intent) || !intent?.intentText.isNullOrEmpty() + } + + fun incrementCount() { + if (detectedUserIntent) { + Timber.i("Registered restart with intent") + incrementCount(pendingAppForegroundRestart, KEY_UNSENT_CLEAR_APP_RESTARTED_WITH_INTENT_PIXELS) + } else { + Timber.i("Registered restart w/ intent") + incrementCount(pendingAppForegroundRestartWithIntent, KEY_UNSENT_CLEAR_APP_RESTARTED_PIXELS) + } + } + + fun firePendingPixels() { + firePendingPixels(pendingAppForegroundRestart, Pixel.PixelName.FORGET_ALL_AUTO_RESTART) + firePendingPixels(pendingAppForegroundRestartWithIntent, Pixel.PixelName.FORGET_ALL_AUTO_RESTART_WITH_INTENT) + resetCount() + } + + private fun incrementCount(counter: Int, sharedPrefKey: String) { + val updated = counter + 1 + preferences.edit(commit = true) { + putInt(sharedPrefKey, updated) + } + } + + private fun firePendingPixels(counter: Int, pixelName: Pixel.PixelName) { + if (counter > 0) { + for (i in 1..counter) { + Timber.i("Fired pixel: ${pixelName.pixelName}/$counter") + pixel.fire(pixelName) + } + } + } + + private fun resetCount() { + preferences.edit(commit = true) { + putInt(KEY_UNSENT_CLEAR_APP_RESTARTED_PIXELS, 0) + putInt(KEY_UNSENT_CLEAR_APP_RESTARTED_WITH_INTENT_PIXELS, 0) + } + Timber.i("counter reset") + } + + private fun widgetActivity(intent: Intent?): Boolean = + intent?.component?.className?.contains(SystemSearchActivity::class.java.canonicalName.orEmpty()) == true + + private val preferences: SharedPreferences + get() = context.getSharedPreferences(FILENAME, Context.MODE_PRIVATE) + + companion object { + @VisibleForTesting + const val FILENAME = "com.duckduckgo.app.fire.unsentpixels.settings" + const val KEY_UNSENT_CLEAR_APP_RESTARTED_PIXELS = "KEY_UNSENT_CLEAR_APP_RESTARTED_PIXELS" + const val KEY_UNSENT_CLEAR_APP_RESTARTED_WITH_INTENT_PIXELS = "KEY_UNSENT_CLEAR_APP_RESTARTED_WITH_INTENT_PIXELS" + } +} diff --git a/app/src/main/java/com/duckduckgo/app/fire/UnsentForgetAllPixelStore.kt b/app/src/main/java/com/duckduckgo/app/fire/UnsentForgetAllPixelStore.kt index 122f7a076345..92a5ad1c9347 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/UnsentForgetAllPixelStore.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/UnsentForgetAllPixelStore.kt @@ -80,94 +80,3 @@ class UnsentForgetAllPixelStoreSharedPreferences @Inject constructor(private val const val KEY_TIMESTAMP_LAST_CLEARED = "KEY_TIMESTAMP_LAST_CLEARED" } } - - -/** - * Stores information about unsent clear data Pixels. - * - * When writing values here to SharedPreferences, it is crucial to use `commit = true`. As otherwise the change can be lost in the process restart. - */ -@Singleton -class UnsentPixelDataClearerAppRestartedWithIntentStoreSharedPreferences @Inject constructor( - private val context: Context, - private val pixel: Pixel -): LifecycleObserver { - private var externalIntent: Boolean = false - - private val pendingAppForegroundRestart: Int - get() = preferences.getInt(KEY_UNSENT_CLEAR_APP_RESTARTED_PIXELS, 0) - - private val pendingAppForegroundRestartWithIntent: Int - get() = preferences.getInt(KEY_UNSENT_CLEAR_APP_RESTARTED_WITH_INTENT_PIXELS, 0) - - @UiThread - @OnLifecycleEvent(Lifecycle.Event.ON_STOP) - fun onAppForegrounded() { - Timber.i("Registered App on_stop") - externalIntent = false - } - - fun registerIntent(intent: Intent?) { - if (widgetActivity(intent)) { - Timber.i("Registered Intent with extras") - externalIntent = true - } else { - if (!intent?.intentText.isNullOrEmpty()) { - externalIntent = true - Timber.i("Registered Intent with extras") - } - } - } - - fun incrementCount() { - if (externalIntent) { - Timber.i("Registered restart with intent") - incrementCount(pendingAppForegroundRestart, KEY_UNSENT_CLEAR_APP_RESTARTED_WITH_INTENT_PIXELS) - } else { - Timber.i("Registered restart w/ intent") - incrementCount(pendingAppForegroundRestartWithIntent, KEY_UNSENT_CLEAR_APP_RESTARTED_PIXELS) - } - } - - fun firePendingPixels() { - firePendingPixels(pendingAppForegroundRestart, Pixel.PixelName.FORGET_ALL_AUTO_RESTART) - firePendingPixels(pendingAppForegroundRestartWithIntent, Pixel.PixelName.FORGET_ALL_AUTO_RESTART_WITH_INTENT) - resetCount() - } - - private fun incrementCount(counter: Int, sharedPrefKey: String) { - val updated = counter + 1 - preferences.edit(commit = true) { - putInt(sharedPrefKey, updated) - } - } - - private fun firePendingPixels(counter: Int, pixelName: Pixel.PixelName) { - if (counter > 0) { - for (i in 1..counter) { - Timber.i("Fired pixel: ${pixelName.pixelName}/$counter") - pixel.fire(pixelName) - } - } - } - - private fun resetCount() { - preferences.edit(commit = true) { - putInt(KEY_UNSENT_CLEAR_APP_RESTARTED_PIXELS, 0) - putInt(KEY_UNSENT_CLEAR_APP_RESTARTED_WITH_INTENT_PIXELS, 0) - } - Timber.i("counter reset") - } - - private fun widgetActivity(intent: Intent?): Boolean = intent?.component?.className?.contains(SystemSearchActivity::class.java.canonicalName.orEmpty()) == true - - private val preferences: SharedPreferences - get() = context.getSharedPreferences(FILENAME, Context.MODE_PRIVATE) - - companion object { - @VisibleForTesting - const val FILENAME = "com.duckduckgo.app.fire.unsentpixels.settings" - const val KEY_UNSENT_CLEAR_APP_RESTARTED_PIXELS = "KEY_UNSENT_CLEAR_APP_RESTARTED_PIXELS" - const val KEY_UNSENT_CLEAR_APP_RESTARTED_WITH_INTENT_PIXELS = "KEY_UNSENT_CLEAR_APP_RESTARTED_WITH_INTENT_PIXELS" - } -} \ No newline at end of file diff --git a/app/src/main/java/com/duckduckgo/app/global/DuckDuckGoApplication.kt b/app/src/main/java/com/duckduckgo/app/global/DuckDuckGoApplication.kt index b458cafefc7b..670c061158a1 100644 --- a/app/src/main/java/com/duckduckgo/app/global/DuckDuckGoApplication.kt +++ b/app/src/main/java/com/duckduckgo/app/global/DuckDuckGoApplication.kt @@ -30,7 +30,7 @@ import com.duckduckgo.app.di.DaggerAppComponent import com.duckduckgo.app.fire.DataClearer import com.duckduckgo.app.fire.FireActivity import com.duckduckgo.app.fire.UnsentForgetAllPixelStore -import com.duckduckgo.app.fire.UnsentPixelDataClearerAppRestartedWithIntentStoreSharedPreferences +import com.duckduckgo.app.fire.DataClearerForegroundAppRestartPixel import com.duckduckgo.app.global.Theming.initializeTheme import com.duckduckgo.app.global.initialization.AppDataLoader import com.duckduckgo.app.global.install.AppInstallStore @@ -112,7 +112,7 @@ open class DuckDuckGoApplication : HasAndroidInjector, Application(), LifecycleO lateinit var unsentForgetAllPixelStore: UnsentForgetAllPixelStore @Inject - lateinit var unsentPixelDataClearerRestart: UnsentPixelDataClearerAppRestartedWithIntentStoreSharedPreferences + lateinit var unsentPixelDataClearerRestart: DataClearerForegroundAppRestartPixel @Inject lateinit var offlinePixelScheduler: OfflinePixelScheduler diff --git a/app/src/main/java/com/duckduckgo/app/systemsearch/SystemSearchActivity.kt b/app/src/main/java/com/duckduckgo/app/systemsearch/SystemSearchActivity.kt index fde699730a27..15aac7c67897 100644 --- a/app/src/main/java/com/duckduckgo/app/systemsearch/SystemSearchActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/systemsearch/SystemSearchActivity.kt @@ -34,7 +34,7 @@ import com.duckduckgo.app.browser.BrowserActivity import com.duckduckgo.app.browser.R import com.duckduckgo.app.browser.autocomplete.BrowserAutoCompleteSuggestionsAdapter import com.duckduckgo.app.browser.omnibar.OmnibarScrolling -import com.duckduckgo.app.fire.UnsentPixelDataClearerAppRestartedWithIntentStoreSharedPreferences +import com.duckduckgo.app.fire.DataClearerForegroundAppRestartPixel import com.duckduckgo.app.global.DuckDuckGoActivity import com.duckduckgo.app.global.view.TextChangedWatcher import com.duckduckgo.app.global.view.hideKeyboard @@ -56,7 +56,7 @@ class SystemSearchActivity : DuckDuckGoActivity() { lateinit var omnibarScrolling: OmnibarScrolling @Inject - lateinit var unsentPixelDataClearerRestart: UnsentPixelDataClearerAppRestartedWithIntentStoreSharedPreferences + lateinit var unsentPixelDataClearerRestart: DataClearerForegroundAppRestartPixel private val viewModel: SystemSearchViewModel by bindViewModel() private lateinit var autocompleteSuggestionsAdapter: BrowserAutoCompleteSuggestionsAdapter From cc00e63c5ddcec695ed40054304673012ce156cc Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Fri, 12 Jun 2020 10:35:39 +0200 Subject: [PATCH 03/13] Added test cases for DataClearerForegroundAppRestartPixel --- .../app/fire/AutomaticDataClearerTest.kt | 3 +- ...ataClearerForegroundAppRestartPixelTest.kt | 86 +++++++++++++++++++ .../app/fire/AutomaticDataClearer.kt | 4 +- 3 files changed, 90 insertions(+), 3 deletions(-) create mode 100644 app/src/androidTest/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixelTest.kt diff --git a/app/src/androidTest/java/com/duckduckgo/app/fire/AutomaticDataClearerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/fire/AutomaticDataClearerTest.kt index 5054d26a75e6..0699facaef1f 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/fire/AutomaticDataClearerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/fire/AutomaticDataClearerTest.kt @@ -39,12 +39,13 @@ class AutomaticDataClearerTest { private val mockClearAction: ClearDataAction = mock() private val mockTimeKeeper: BackgroundTimeKeeper = mock() private val mockWorkManager: WorkManager = mock() + private val dataClearerForegroundAppRestartPixel: DataClearerForegroundAppRestartPixel = mock() @UiThreadTest @Before fun setup() { whenever(mockSettingsDataStore.hasBackgroundTimestampRecorded()).thenReturn(true) - testee = AutomaticDataClearer(mockWorkManager, mockSettingsDataStore, mockClearAction, mockTimeKeeper) + testee = AutomaticDataClearer(mockWorkManager, mockSettingsDataStore, mockClearAction, mockTimeKeeper, dataClearerForegroundAppRestartPixel) } private suspend fun simulateLifecycle(isFreshAppLaunch: Boolean) { diff --git a/app/src/androidTest/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixelTest.kt new file mode 100644 index 000000000000..55bc249e9493 --- /dev/null +++ b/app/src/androidTest/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixelTest.kt @@ -0,0 +1,86 @@ +/* + * Copyright (c) 2020 DuckDuckGo + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.duckduckgo.app.fire + +import android.content.Intent +import android.net.Uri +import androidx.test.platform.app.InstrumentationRegistry +import com.duckduckgo.app.browser.BrowserActivity +import com.duckduckgo.app.statistics.pixels.Pixel +import com.duckduckgo.app.systemsearch.SystemSearchActivity +import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.verify +import org.junit.Test + + +class DataClearerForegroundAppRestartPixelTest { + + private val context = InstrumentationRegistry.getInstrumentation().targetContext + private val pixel = mock() + private val testee = DataClearerForegroundAppRestartPixel(context, pixel) + + @Test + fun whenAppRestartsAfterOpenSearchWidgetThenPixelWithIntentIsSent() { + val intent = SystemSearchActivity.fromWidget(context) + testee.registerIntent(intent) + testee.incrementCount() + + testee.firePendingPixels() + + verify(pixel).fire(Pixel.PixelName.FORGET_ALL_AUTO_RESTART_WITH_INTENT) + } + + @Test + fun whenAppRestartsAfterOpenExternalLinkThenPixelWithIntentIsSent() { + val i = givenIntentWithData("https://example.com") + testee.registerIntent(i) + testee.incrementCount() + + testee.firePendingPixels() + + verify(pixel).fire(Pixel.PixelName.FORGET_ALL_AUTO_RESTART_WITH_INTENT) + } + + @Test + fun whenAppRestartsAfterOpenAnEmptyIntentThenPixelIsSent() { + val intent = givenEmptyIntent() + testee.registerIntent(intent) + testee.incrementCount() + + testee.firePendingPixels() + + verify(pixel).fire(Pixel.PixelName.FORGET_ALL_AUTO_RESTART) + } + + @Test + fun whenAllUnsentPixelsAreFiredThenResetCounter() { + val intent = givenEmptyIntent() + testee.registerIntent(intent) + testee.incrementCount() + + testee.firePendingPixels() + testee.firePendingPixels() + + verify(pixel).fire(Pixel.PixelName.FORGET_ALL_AUTO_RESTART) + } + + private fun givenEmptyIntent(): Intent = Intent(context, BrowserActivity::class.java) + + private fun givenIntentWithData(url: String) = Intent(Intent.ACTION_VIEW).apply { + data = Uri.parse(url) + } +} diff --git a/app/src/main/java/com/duckduckgo/app/fire/AutomaticDataClearer.kt b/app/src/main/java/com/duckduckgo/app/fire/AutomaticDataClearer.kt index d2aeb9731c14..1c8cc6596603 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/AutomaticDataClearer.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/AutomaticDataClearer.kt @@ -45,7 +45,7 @@ class AutomaticDataClearer( private val settingsDataStore: SettingsDataStore, private val clearDataAction: ClearDataAction, private val dataClearerTimeKeeper: BackgroundTimeKeeper, - private val unsentPixelDataClearerRestart: DataClearerForegroundAppRestartPixel + private val dataClearerForegroundAppRestartPixel: DataClearerForegroundAppRestartPixel ) : DataClearer, LifecycleObserver, CoroutineScope { private val clearJob: Job = Job() @@ -160,7 +160,7 @@ class AutomaticDataClearer( Timber.i("All data now cleared, will restart process? $processNeedsRestarted") if (processNeedsRestarted) { clearDataAction.setAppUsedSinceLastClearFlag(false) - unsentPixelDataClearerRestart.incrementCount() + dataClearerForegroundAppRestartPixel.incrementCount() // need a moment to draw background color (reduces flickering UX) Handler().postDelayed(100) { Timber.i("Will now restart process") From fc6b454c2d8afd516e58d940ad425aa4e634886b Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Fri, 12 Jun 2020 10:37:57 +0200 Subject: [PATCH 04/13] rename based on Class name --- .../main/java/com/duckduckgo/app/browser/BrowserActivity.kt | 6 +++--- app/src/main/java/com/duckduckgo/app/di/PrivacyModule.kt | 4 ++-- .../java/com/duckduckgo/app/global/DuckDuckGoApplication.kt | 6 +++--- .../com/duckduckgo/app/systemsearch/SystemSearchActivity.kt | 6 +++--- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index d3a7b6313942..bd2a483dfca9 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -66,7 +66,7 @@ class BrowserActivity : DuckDuckGoActivity(), CoroutineScope by MainScope() { lateinit var playStoreUtils: PlayStoreUtils @Inject - lateinit var unsentPixelDataClearerRestart: DataClearerForegroundAppRestartPixel + lateinit var dataClearerForegroundAppRestartPixel: DataClearerForegroundAppRestartPixel private var currentTab: BrowserTabFragment? = null @@ -86,7 +86,7 @@ class BrowserActivity : DuckDuckGoActivity(), CoroutineScope by MainScope() { override fun onCreate(savedInstanceState: Bundle?) { super.daggerInject() Timber.i("onCreate called. freshAppLaunch: ${dataClearer.isFreshAppLaunch}, savedInstanceState: $savedInstanceState") - unsentPixelDataClearerRestart.registerIntent(intent) + dataClearerForegroundAppRestartPixel.registerIntent(intent) renderer = BrowserStateRenderer() val newInstanceState = if (dataClearer.isFreshAppLaunch) null else savedInstanceState instanceStateBundles = CombinedInstanceState(originalInstanceState = savedInstanceState, newInstanceState = newInstanceState) @@ -112,7 +112,7 @@ class BrowserActivity : DuckDuckGoActivity(), CoroutineScope by MainScope() { override fun onNewIntent(intent: Intent?) { super.onNewIntent(intent) Timber.i("onNewIntent: $intent") - unsentPixelDataClearerRestart.registerIntent(intent) + dataClearerForegroundAppRestartPixel.registerIntent(intent) if (dataClearer.dataClearerState.value == ApplicationClearDataState.FINISHED) { Timber.i("Automatic data clearer has finished, so processing intent now") diff --git a/app/src/main/java/com/duckduckgo/app/di/PrivacyModule.kt b/app/src/main/java/com/duckduckgo/app/di/PrivacyModule.kt index d96ea95aa815..8a13c396fda9 100644 --- a/app/src/main/java/com/duckduckgo/app/di/PrivacyModule.kt +++ b/app/src/main/java/com/duckduckgo/app/di/PrivacyModule.kt @@ -74,9 +74,9 @@ class PrivacyModule { settingsDataStore: SettingsDataStore, clearDataAction: ClearDataAction, dataClearerTimeKeeper: BackgroundTimeKeeper, - unsentPixelDataClearerRestart: DataClearerForegroundAppRestartPixel + dataClearerForegroundAppRestartPixel: DataClearerForegroundAppRestartPixel ): DataClearer { - return AutomaticDataClearer(workManager, settingsDataStore, clearDataAction, dataClearerTimeKeeper, unsentPixelDataClearerRestart) + return AutomaticDataClearer(workManager, settingsDataStore, clearDataAction, dataClearerTimeKeeper, dataClearerForegroundAppRestartPixel) } @Provides diff --git a/app/src/main/java/com/duckduckgo/app/global/DuckDuckGoApplication.kt b/app/src/main/java/com/duckduckgo/app/global/DuckDuckGoApplication.kt index 670c061158a1..4c0b0ddcfd30 100644 --- a/app/src/main/java/com/duckduckgo/app/global/DuckDuckGoApplication.kt +++ b/app/src/main/java/com/duckduckgo/app/global/DuckDuckGoApplication.kt @@ -112,7 +112,7 @@ open class DuckDuckGoApplication : HasAndroidInjector, Application(), LifecycleO lateinit var unsentForgetAllPixelStore: UnsentForgetAllPixelStore @Inject - lateinit var unsentPixelDataClearerRestart: DataClearerForegroundAppRestartPixel + lateinit var dataClearerForegroundAppRestartPixel: DataClearerForegroundAppRestartPixel @Inject lateinit var offlinePixelScheduler: OfflinePixelScheduler @@ -174,7 +174,7 @@ open class DuckDuckGoApplication : HasAndroidInjector, Application(), LifecycleO it.addObserver(appDaysUsedRecorder) it.addObserver(defaultBrowserObserver) it.addObserver(appEnjoymentLifecycleObserver) - it.addObserver(unsentPixelDataClearerRestart) + it.addObserver(dataClearerForegroundAppRestartPixel) } if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N_MR1) { @@ -263,7 +263,7 @@ open class DuckDuckGoApplication : HasAndroidInjector, Application(), LifecycleO } private fun submitUnsentFireAppRestartedWithIntentPixels() { - unsentPixelDataClearerRestart.firePendingPixels() + dataClearerForegroundAppRestartPixel.firePendingPixels() } /** diff --git a/app/src/main/java/com/duckduckgo/app/systemsearch/SystemSearchActivity.kt b/app/src/main/java/com/duckduckgo/app/systemsearch/SystemSearchActivity.kt index 15aac7c67897..6fd0faa88e79 100644 --- a/app/src/main/java/com/duckduckgo/app/systemsearch/SystemSearchActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/systemsearch/SystemSearchActivity.kt @@ -56,7 +56,7 @@ class SystemSearchActivity : DuckDuckGoActivity() { lateinit var omnibarScrolling: OmnibarScrolling @Inject - lateinit var unsentPixelDataClearerRestart: DataClearerForegroundAppRestartPixel + lateinit var dataClearerForegroundAppRestartPixel: DataClearerForegroundAppRestartPixel private val viewModel: SystemSearchViewModel by bindViewModel() private lateinit var autocompleteSuggestionsAdapter: BrowserAutoCompleteSuggestionsAdapter @@ -72,7 +72,7 @@ class SystemSearchActivity : DuckDuckGoActivity() { override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) Timber.i("onCreate called") - unsentPixelDataClearerRestart.registerIntent(intent) + dataClearerForegroundAppRestartPixel.registerIntent(intent) setContentView(R.layout.activity_system_search) configureObservers() configureOnboarding() @@ -90,7 +90,7 @@ class SystemSearchActivity : DuckDuckGoActivity() { override fun onNewIntent(newIntent: Intent?) { super.onNewIntent(newIntent) Timber.i("onNewIntent") - unsentPixelDataClearerRestart.registerIntent(newIntent) + dataClearerForegroundAppRestartPixel.registerIntent(newIntent) viewModel.resetViewState() newIntent?.let { sendLaunchPixels(it) } } From 24f147906576e4349b53ad1db5d301bab6c7d394 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Fri, 12 Jun 2020 10:43:30 +0200 Subject: [PATCH 05/13] add test case to ensure sending the app to background after being opened from an external link restarts state. --- .../fire/DataClearerForegroundAppRestartPixelTest.kt | 12 ++++++++++++ .../app/fire/DataClearerForegroundAppRestartPixel.kt | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixelTest.kt index 55bc249e9493..6e81afe067d6 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixelTest.kt @@ -78,6 +78,18 @@ class DataClearerForegroundAppRestartPixelTest { verify(pixel).fire(Pixel.PixelName.FORGET_ALL_AUTO_RESTART) } + @Test + fun whenAppRestartedAfterGoingBackFromBackgroundThenPixelIsSent() { + val intent = SystemSearchActivity.fromWidget(context) + testee.registerIntent(intent) + testee.onAppBackgrounded() + testee.incrementCount() + + testee.firePendingPixels() + + verify(pixel).fire(Pixel.PixelName.FORGET_ALL_AUTO_RESTART) + } + private fun givenEmptyIntent(): Intent = Intent(context, BrowserActivity::class.java) private fun givenIntentWithData(url: String) = Intent(Intent.ACTION_VIEW).apply { diff --git a/app/src/main/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixel.kt b/app/src/main/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixel.kt index 6c7528fe600c..814471a39d4b 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixel.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixel.kt @@ -53,7 +53,7 @@ class DataClearerForegroundAppRestartPixel @Inject constructor( @UiThread @OnLifecycleEvent(Lifecycle.Event.ON_STOP) - fun onAppForegrounded() { + fun onAppBackgrounded() { Timber.i("Registered App on_stop") detectedUserIntent = false } From 329a99a0d5399d7bf656250c8f1d6359fb39250e Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Fri, 12 Jun 2020 10:57:26 +0200 Subject: [PATCH 06/13] clenaup imports --- .../fire/DataClearerForegroundAppRestartPixelTest.kt | 1 - .../duckduckgo/app/fire/UnsentForgetAllPixelStore.kt | 11 ----------- 2 files changed, 12 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixelTest.kt index 6e81afe067d6..8462a6b7daad 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixelTest.kt @@ -26,7 +26,6 @@ import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.verify import org.junit.Test - class DataClearerForegroundAppRestartPixelTest { private val context = InstrumentationRegistry.getInstrumentation().targetContext diff --git a/app/src/main/java/com/duckduckgo/app/fire/UnsentForgetAllPixelStore.kt b/app/src/main/java/com/duckduckgo/app/fire/UnsentForgetAllPixelStore.kt index 92a5ad1c9347..d3ccc88c9ce2 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/UnsentForgetAllPixelStore.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/UnsentForgetAllPixelStore.kt @@ -17,21 +17,10 @@ package com.duckduckgo.app.fire import android.content.Context -import android.content.Intent import android.content.SharedPreferences -import androidx.annotation.UiThread import androidx.annotation.VisibleForTesting import androidx.core.content.edit -import androidx.lifecycle.Lifecycle -import androidx.lifecycle.LifecycleObserver -import androidx.lifecycle.OnLifecycleEvent -import com.duckduckgo.app.global.intentText -import com.duckduckgo.app.statistics.pixels.Pixel -import com.duckduckgo.app.systemsearch.SystemSearchActivity -import kotlinx.coroutines.launch -import timber.log.Timber import javax.inject.Inject -import javax.inject.Singleton interface UnsentForgetAllPixelStore { val pendingPixelCountClearData: Int From 5fef39901c3850b101eddfd171852daaae09ebf8 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Fri, 12 Jun 2020 14:21:50 +0200 Subject: [PATCH 07/13] fix failing test cases: use concrete class instead of mock --- .../java/com/duckduckgo/app/fire/AutomaticDataClearerTest.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/fire/AutomaticDataClearerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/fire/AutomaticDataClearerTest.kt index 0699facaef1f..d2612b03203a 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/fire/AutomaticDataClearerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/fire/AutomaticDataClearerTest.kt @@ -19,11 +19,13 @@ package com.duckduckgo.app.fire import androidx.test.annotation.UiThreadTest +import androidx.test.platform.app.InstrumentationRegistry import androidx.work.WorkManager import com.duckduckgo.app.global.view.ClearDataAction import com.duckduckgo.app.settings.clear.ClearWhatOption import com.duckduckgo.app.settings.clear.ClearWhenOption import com.duckduckgo.app.settings.db.SettingsDataStore +import com.duckduckgo.app.statistics.pixels.Pixel import com.nhaarman.mockitokotlin2.* import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.runBlocking @@ -39,7 +41,8 @@ class AutomaticDataClearerTest { private val mockClearAction: ClearDataAction = mock() private val mockTimeKeeper: BackgroundTimeKeeper = mock() private val mockWorkManager: WorkManager = mock() - private val dataClearerForegroundAppRestartPixel: DataClearerForegroundAppRestartPixel = mock() + private val pixel: Pixel = mock() + private val dataClearerForegroundAppRestartPixel = DataClearerForegroundAppRestartPixel(InstrumentationRegistry.getInstrumentation().targetContext, pixel) @UiThreadTest @Before From 47ae684d2249420aa35302fc281618601f69b414 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Fri, 12 Jun 2020 14:38:51 +0200 Subject: [PATCH 08/13] remove timber logs --- .../com/duckduckgo/app/systemsearch/SystemSearchActivity.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/systemsearch/SystemSearchActivity.kt b/app/src/main/java/com/duckduckgo/app/systemsearch/SystemSearchActivity.kt index 6fd0faa88e79..bd5bdd3618d6 100644 --- a/app/src/main/java/com/duckduckgo/app/systemsearch/SystemSearchActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/systemsearch/SystemSearchActivity.kt @@ -71,7 +71,6 @@ class SystemSearchActivity : DuckDuckGoActivity() { override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) - Timber.i("onCreate called") dataClearerForegroundAppRestartPixel.registerIntent(intent) setContentView(R.layout.activity_system_search) configureObservers() @@ -89,7 +88,6 @@ class SystemSearchActivity : DuckDuckGoActivity() { override fun onNewIntent(newIntent: Intent?) { super.onNewIntent(newIntent) - Timber.i("onNewIntent") dataClearerForegroundAppRestartPixel.registerIntent(newIntent) viewModel.resetViewState() newIntent?.let { sendLaunchPixels(it) } From c7a309f4ad8c2d7a17e7371e3a5e5f9a305197ee Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Fri, 12 Jun 2020 15:08:19 +0200 Subject: [PATCH 09/13] Using lazy instead of get sharedpref multiple times --- .../app/fire/DataClearerForegroundAppRestartPixel.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixel.kt b/app/src/main/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixel.kt index 814471a39d4b..99c83f311287 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixel.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixel.kt @@ -105,8 +105,9 @@ class DataClearerForegroundAppRestartPixel @Inject constructor( private fun widgetActivity(intent: Intent?): Boolean = intent?.component?.className?.contains(SystemSearchActivity::class.java.canonicalName.orEmpty()) == true - private val preferences: SharedPreferences - get() = context.getSharedPreferences(FILENAME, Context.MODE_PRIVATE) + private val preferences: SharedPreferences by lazy { + context.getSharedPreferences(FILENAME, Context.MODE_PRIVATE) + } companion object { @VisibleForTesting From 65455a8cceea7495adff02d571cb019b14b252b9 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Fri, 12 Jun 2020 15:16:59 +0200 Subject: [PATCH 10/13] import clenaup --- .../java/com/duckduckgo/app/systemsearch/SystemSearchActivity.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/main/java/com/duckduckgo/app/systemsearch/SystemSearchActivity.kt b/app/src/main/java/com/duckduckgo/app/systemsearch/SystemSearchActivity.kt index bd5bdd3618d6..252d130df396 100644 --- a/app/src/main/java/com/duckduckgo/app/systemsearch/SystemSearchActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/systemsearch/SystemSearchActivity.kt @@ -44,7 +44,6 @@ import com.duckduckgo.app.systemsearch.SystemSearchViewModel.Command.* import com.duckduckgo.app.systemsearch.SystemSearchViewModel.SystemSearchResultsViewState import kotlinx.android.synthetic.main.activity_system_search.* import kotlinx.android.synthetic.main.include_system_search_onboarding.* -import timber.log.Timber import javax.inject.Inject class SystemSearchActivity : DuckDuckGoActivity() { From 9191d9803f14271763c0d21914376efd2c37af5b Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 25 Jun 2020 15:03:42 +0200 Subject: [PATCH 11/13] use m pixels instead of mf --- .../main/java/com/duckduckgo/app/statistics/pixels/Pixel.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/statistics/pixels/Pixel.kt b/app/src/main/java/com/duckduckgo/app/statistics/pixels/Pixel.kt index 0e7e624ebe28..57bfd2d8fb67 100644 --- a/app/src/main/java/com/duckduckgo/app/statistics/pixels/Pixel.kt +++ b/app/src/main/java/com/duckduckgo/app/statistics/pixels/Pixel.kt @@ -36,8 +36,8 @@ interface Pixel { FORGET_ALL_PRESSED_BROWSING("mf_bp"), FORGET_ALL_PRESSED_TABSWITCHING("mf_tp"), FORGET_ALL_EXECUTED("mf"), - FORGET_ALL_AUTO_RESTART("mf_r"), - FORGET_ALL_AUTO_RESTART_WITH_INTENT("mf_ri"), + FORGET_ALL_AUTO_RESTART("m_f_r"), + FORGET_ALL_AUTO_RESTART_WITH_INTENT("m_f_ri"), APPLICATION_CRASH("m_d_ac"), APPLICATION_CRASH_GLOBAL("m_d_ac_g"), From 9bc638ce018c4302ba2e5621cdfb1acbf7eafc34 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 25 Jun 2020 15:05:02 +0200 Subject: [PATCH 12/13] fix typo --- .../app/fire/DataClearerForegroundAppRestartPixel.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixel.kt b/app/src/main/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixel.kt index 99c83f311287..235bf6a3801b 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixel.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/DataClearerForegroundAppRestartPixel.kt @@ -34,7 +34,7 @@ import javax.inject.Singleton /** * Stores information about unsent automatic data clearer restart Pixels, detecting if user started the app from an external Intent. - * Contains logic to sent unsent pixels. + * Contains logic to send unsent pixels. * * When writing values here to SharedPreferences, it is crucial to use `commit = true`. As otherwise the change can be lost in the process restart. */ @@ -67,7 +67,7 @@ class DataClearerForegroundAppRestartPixel @Inject constructor( Timber.i("Registered restart with intent") incrementCount(pendingAppForegroundRestart, KEY_UNSENT_CLEAR_APP_RESTARTED_WITH_INTENT_PIXELS) } else { - Timber.i("Registered restart w/ intent") + Timber.i("Registered restart without intent") incrementCount(pendingAppForegroundRestartWithIntent, KEY_UNSENT_CLEAR_APP_RESTARTED_PIXELS) } } From dd9a07ccf5297e4168ae0a8c0253aa8175988693 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 25 Jun 2020 15:13:37 +0200 Subject: [PATCH 13/13] remove method with single method call. This reduces method redirections and improves readability. --- .../java/com/duckduckgo/app/global/DuckDuckGoApplication.kt | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/global/DuckDuckGoApplication.kt b/app/src/main/java/com/duckduckgo/app/global/DuckDuckGoApplication.kt index 4c0b0ddcfd30..729622256a10 100644 --- a/app/src/main/java/com/duckduckgo/app/global/DuckDuckGoApplication.kt +++ b/app/src/main/java/com/duckduckgo/app/global/DuckDuckGoApplication.kt @@ -259,10 +259,7 @@ open class DuckDuckGoApplication : HasAndroidInjector, Application(), LifecycleO } unsentForgetAllPixelStore.resetCount() } - submitUnsentFireAppRestartedWithIntentPixels() - } - private fun submitUnsentFireAppRestartedWithIntentPixels() { dataClearerForegroundAppRestartPixel.firePendingPixels() }