Prevent ANR when assigning AppTP cohort (#3014)

Task/Issue URL: https://app.asana.com/0/488551667048375/1204265639917299/f ### Description Ensure calls to `CohortStore` are not done in the main thread. The approach taken is has been: * marking method as `WorkerThread` * in internal builds `checkMainThread` and throw * in production build we don't do the `checkMainThread` ### Steps to test this PR _Check cohort is assigned_ - [x] fresh install from this branch the internal build - [x] open the app - [x] enable AppTP - [x] filter logcat with `atp_cohort` - [x] navigate to AppTP exclusion list screen - [x] verify pixels are fired with `atp_cohort` parameter assigned - [x] repeat with play build Smoke test AppTP as well is useful
duckduckgo · Mar 29, 2023 · 81e7b0d · 81e7b0d
1 parent 0579935
commit 81e7b0d
Show file tree

Hide file tree

Showing 4 changed files with 78 additions and 6 deletions.
diff --git a/...protection/vpn-impl/src/main/java/com/duckduckgo/mobile/android/vpn/cohort/CohortStore.kt b/...protection/vpn-impl/src/main/java/com/duckduckgo/mobile/android/vpn/cohort/CohortStore.kt
@@ -17,7 +17,12 @@
 package com.duckduckgo.mobile.android.vpn.cohort
 
 import android.content.SharedPreferences
+import androidx.annotation.WorkerThread
 import androidx.core.content.edit
+import com.duckduckgo.app.global.DispatcherProvider
+import com.duckduckgo.app.utils.checkMainThread
+import com.duckduckgo.appbuildconfig.api.AppBuildConfig
+import com.duckduckgo.appbuildconfig.api.isInternalBuild
 import com.duckduckgo.di.scopes.AppScope
 import com.duckduckgo.di.scopes.VpnScope
 import com.duckduckgo.mobile.android.vpn.AppTpVpnFeature
@@ -29,18 +34,21 @@ import com.squareup.anvil.annotations.ContributesBinding
 import com.squareup.anvil.annotations.ContributesMultibinding
 import javax.inject.Inject
 import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.launch
 import org.threeten.bp.LocalDate
 import org.threeten.bp.format.DateTimeFormatter
 
 interface CohortStore {
     /**
      * @return the stored cohort local date or [null] if never set
      */
+    @WorkerThread
     fun getCohortStoredLocalDate(): LocalDate?
 
     /**
      * Stores the cohort [LocalDate] passed as parameter
      */
+    @WorkerThread
     fun setCohortLocalDate(localDate: LocalDate)
 }
 
@@ -55,6 +63,8 @@ interface CohortStore {
 class RealCohortStore @Inject constructor(
     private val sharedPreferencesProvider: VpnSharedPreferencesProvider,
     private val vpnFeaturesRegistry: VpnFeaturesRegistry,
+    private val dispatcherProvider: DispatcherProvider,
+    private val appBuildConfig: AppBuildConfig,
 ) : CohortStore, VpnServiceCallbacks {
 
     private val formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd")
@@ -63,21 +73,31 @@ class RealCohortStore @Inject constructor(
         get() = sharedPreferencesProvider.getSharedPreferences(FILENAME, multiprocess = true, migrate = true)
 
     override fun getCohortStoredLocalDate(): LocalDate? {
+        if (appBuildConfig.isInternalBuild()) {
+            checkMainThread()
+        }
+
         return preferences.getString(KEY_COHORT_LOCAL_DATE, null)?.let {
             LocalDate.parse(it)
         }
     }
 
     override fun setCohortLocalDate(localDate: LocalDate) {
+        if (appBuildConfig.isInternalBuild()) {
+            checkMainThread()
+        }
+
         preferences.edit { putString(KEY_COHORT_LOCAL_DATE, formatter.format(localDate)) }
     }
 
     override fun onVpnStarted(coroutineScope: CoroutineScope) {
-        if (vpnFeaturesRegistry.isFeatureRegistered(AppTpVpnFeature.APPTP_VPN)) {
-            // skip if already stored
-            getCohortStoredLocalDate()?.let { return }
+        coroutineScope.launch(dispatcherProvider.io()) {
+            if (vpnFeaturesRegistry.isFeatureRegistered(AppTpVpnFeature.APPTP_VPN)) {
+                // skip if already stored
+                getCohortStoredLocalDate()?.let { return@launch }
 
-            setCohortLocalDate(LocalDate.now())
+                setCohortLocalDate(LocalDate.now())
+            }
         }
     }
 

diff --git a/...impl/src/test/java/com/duckduckgo/mobile/android/vpn/cohort/CohortPixelInterceptorTest.kt b/...impl/src/test/java/com/duckduckgo/mobile/android/vpn/cohort/CohortPixelInterceptorTest.kt
@@ -16,12 +16,16 @@
 
 package com.duckduckgo.mobile.android.vpn.cohort
 
+import com.duckduckgo.app.CoroutineTestRule
 import com.duckduckgo.app.global.api.FakeChain
 import com.duckduckgo.app.global.api.InMemorySharedPreferences
+import com.duckduckgo.appbuildconfig.api.AppBuildConfig
+import com.duckduckgo.appbuildconfig.api.BuildFlavor
 import com.duckduckgo.mobile.android.vpn.VpnFeaturesRegistry
 import com.duckduckgo.mobile.android.vpn.prefs.VpnSharedPreferencesProvider
 import org.junit.Assert
 import org.junit.Before
+import org.junit.Rule
 import org.junit.Test
 import org.mockito.Mock
 import org.mockito.MockitoAnnotations
@@ -31,8 +35,16 @@ import org.mockito.kotlin.whenever
 import org.threeten.bp.LocalDate
 
 class CohortPixelInterceptorTest {
+    @get:Rule
+    @Suppress("unused")
+    val coroutineRule = CoroutineTestRule()
+
     @Mock
     private lateinit var vpnFeaturesRegistry: VpnFeaturesRegistry
+
+    @Mock
+    private lateinit var appBuildConfig: AppBuildConfig
+
     private lateinit var cohortPixelInterceptor: CohortPixelInterceptor
     private lateinit var cohortStore: CohortStore
     private lateinit var cohortCalculator: CohortCalculator
@@ -47,7 +59,9 @@ class CohortPixelInterceptorTest {
             sharedPreferencesProvider.getSharedPreferences(eq("com.duckduckgo.mobile.atp.cohort.prefs"), eq(true), eq(true)),
         ).thenReturn(prefs)
 
-        cohortStore = RealCohortStore(sharedPreferencesProvider, vpnFeaturesRegistry)
+        whenever(appBuildConfig.flavor).thenReturn(BuildFlavor.PLAY)
+
+        cohortStore = RealCohortStore(sharedPreferencesProvider, vpnFeaturesRegistry, coroutineRule.testDispatcherProvider, appBuildConfig)
         cohortCalculator = RealCohortCalculator()
         cohortPixelInterceptor = CohortPixelInterceptor(cohortCalculator, cohortStore)
     }

diff --git a/...on/vpn-impl/src/test/java/com/duckduckgo/mobile/android/vpn/cohort/RealCohortStoreTest.kt b/...on/vpn-impl/src/test/java/com/duckduckgo/mobile/android/vpn/cohort/RealCohortStoreTest.kt
@@ -16,14 +16,18 @@
 
 package com.duckduckgo.mobile.android.vpn.cohort
 
+import com.duckduckgo.app.CoroutineTestRule
 import com.duckduckgo.app.global.api.InMemorySharedPreferences
+import com.duckduckgo.appbuildconfig.api.AppBuildConfig
+import com.duckduckgo.appbuildconfig.api.BuildFlavor
 import com.duckduckgo.mobile.android.vpn.AppTpVpnFeature
 import com.duckduckgo.mobile.android.vpn.VpnFeaturesRegistry
 import com.duckduckgo.mobile.android.vpn.prefs.VpnSharedPreferencesProvider
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.test.TestScope
 import org.junit.Assert.*
 import org.junit.Before
+import org.junit.Rule
 import org.junit.Test
 import org.mockito.Mock
 import org.mockito.MockitoAnnotations
@@ -34,8 +38,16 @@ import org.threeten.bp.LocalDate
 
 @ExperimentalCoroutinesApi
 class RealCohortStoreTest {
+    @get:Rule
+    @Suppress("unused")
+    val coroutineRule = CoroutineTestRule()
+
     @Mock
     private lateinit var vpnFeaturesRegistry: VpnFeaturesRegistry
+
+    @Mock
+    private lateinit var appBuildConfig: AppBuildConfig
+
     private val sharedPreferencesProvider = mock<VpnSharedPreferencesProvider>()
 
     private lateinit var cohortStore: CohortStore
@@ -47,8 +59,9 @@ class RealCohortStoreTest {
         whenever(
             sharedPreferencesProvider.getSharedPreferences(eq("com.duckduckgo.mobile.atp.cohort.prefs"), eq(true), eq(true)),
         ).thenReturn(prefs)
+        whenever(appBuildConfig.flavor).thenReturn(BuildFlavor.PLAY)
 
-        cohortStore = RealCohortStore(sharedPreferencesProvider, vpnFeaturesRegistry)
+        cohortStore = RealCohortStore(sharedPreferencesProvider, vpnFeaturesRegistry, coroutineRule.testDispatcherProvider, appBuildConfig)
     }
 
     @Test

diff --git a/common/src/main/java/com/duckduckgo/app/utils/CheckMainThread.kt b/common/src/main/java/com/duckduckgo/app/utils/CheckMainThread.kt
@@ -0,0 +1,25 @@
+/*
+ * Copyright (c) 2023 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.utils
+
+import android.os.Looper
+
+fun checkMainThread() {
+    check(Looper.myLooper() != Looper.getMainLooper()) {
+        "Not expected to be called on the main thread but was "
+    }
+}