Skip to content

Commit 27b9b29

Browse files
CDRussellCDRussell
authored
Ensure EnqueuedPixelWorker does not do IO work on main thread (#7110)
Task/Issue URL: https://app.asana.com/1/137249556945/project/608920331025315/task/1211922784726284?focus=true ### Description Moves work which can trigger IO to an `io` dispatcher instead of `main` and `Default` dispatchers that can be used currently. ### Steps to test this PR Add logcat filter: `unsent clear data pixels|Pixel sent: ml|Pixel sent: mf` #### ml pixel - [x] Launch the app; verify you see `ml` pixel sent - [x] Switch to another app, then switch back to our app (it's not always enough just to background it and return). verify you see the `ml` pixel sent #### mf pixel - [x] Apply patch (which triggers the background worker to run immediately on app launch instead of waiting for it to run after a few hours); install and launch app - [x] Use the fire button a few times (_n_ times) to clear data - [x] Switch to another app, then switch back to ours; verify you see `Found n unsent clear data pixels` and _n_ instances of `Pixel sent: mf` ### Patch ``` Index: app/src/main/java/com/duckduckgo/app/pixels/EnqueuedPixelWorker.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/app/src/main/java/com/duckduckgo/app/pixels/EnqueuedPixelWorker.kt b/app/src/main/java/com/duckduckgo/app/pixels/EnqueuedPixelWorker.kt --- a/app/src/main/java/com/duckduckgo/app/pixels/EnqueuedPixelWorker.kt (revision Staged) +++ b/app/src/main/java/com/duckduckgo/app/pixels/EnqueuedPixelWorker.kt (date 1762956896767) @@ -86,6 +86,9 @@ return } + val oneTimeRequest = OneTimeWorkRequestBuilder<RealEnqueuedPixelWorker>().build() + workManager.beginUniqueWork("pixel_sender", ExistingWorkPolicy.REPLACE, oneTimeRequest).enqueue() + appCoroutineScope.launch(dispatchers.io()) { sendAppLaunchPixel() } ``` Co-authored-by: Craig Russell <[email protected]>
1 parent d3029de commit 27b9b29

File tree

2 files changed

+80
-19
lines changed

2 files changed

+80
-19
lines changed

‎app/src/main/java/com/duckduckgo/app/pixels/EnqueuedPixelWorker.kt‎

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import com.duckduckgo.app.statistics.pixels.Pixel.PixelParameter.WEBVIEW_FULL_VE
3232
import com.duckduckgo.app.statistics.pixels.Pixel.PixelParameter.WEBVIEW_VERSION
3333
import com.duckduckgo.appbuildconfig.api.AppBuildConfig
3434
import com.duckduckgo.browser.api.WebViewVersionProvider
35+
import com.duckduckgo.common.utils.DispatcherProvider
3536
import com.duckduckgo.customtabs.api.CustomTabDetector
3637
import com.duckduckgo.di.scopes.AppScope
3738
import com.duckduckgo.privacyprotectionspopup.api.PrivacyProtectionsPopupExperimentExternalPixels
@@ -40,6 +41,7 @@ import com.squareup.anvil.annotations.ContributesMultibinding
4041
import dagger.SingleInstanceIn
4142
import kotlinx.coroutines.CoroutineScope
4243
import kotlinx.coroutines.launch
44+
import kotlinx.coroutines.withContext
4345
import logcat.LogPriority.INFO
4446
import logcat.LogPriority.VERBOSE
4547
import logcat.logcat
@@ -63,6 +65,7 @@ class EnqueuedPixelWorker @Inject constructor(
6365
private val privacyProtectionsPopupExperimentExternalPixels: PrivacyProtectionsPopupExperimentExternalPixels,
6466
private val isVerifiedPlayStoreInstall: IsVerifiedPlayStoreInstall,
6567
private val appBuildConfig: AppBuildConfig,
68+
private val dispatchers: DispatcherProvider,
6669
@AppCoroutineScope private val appCoroutineScope: CoroutineScope,
6770
) : MainProcessLifecycleObserver {
6871

@@ -82,9 +85,17 @@ class EnqueuedPixelWorker @Inject constructor(
8285
launchedByFireAction = false
8386
return
8487
}
88+
89+
appCoroutineScope.launch(dispatchers.io()) {
90+
sendAppLaunchPixel()
91+
}
92+
}
93+
94+
private suspend fun sendAppLaunchPixel() {
8595
logcat(INFO) { "Sending app launch pixel" }
8696
val collectWebViewFullVersion =
8797
androidBrowserConfigFeature.self().isEnabled() && androidBrowserConfigFeature.collectFullWebViewVersion().isEnabled()
98+
8899
val paramsMap = mutableMapOf<String, String>().apply {
89100
put(WEBVIEW_VERSION, webViewVersionProvider.getMajorVersion())
90101
put(DEFAULT_BROWSER, defaultBrowserDetector.isDefaultBrowser().toString())
@@ -93,20 +104,22 @@ class EnqueuedPixelWorker @Inject constructor(
93104
put(WEBVIEW_FULL_VERSION, webViewVersionProvider.getFullVersion())
94105
}
95106
}.toMap()
96-
appCoroutineScope.launch {
97-
val popupExperimentParams = privacyProtectionsPopupExperimentExternalPixels.getPixelParams()
98-
val parameters = paramsMap + popupExperimentParams
107+
108+
val popupExperimentParams = privacyProtectionsPopupExperimentExternalPixels.getPixelParams()
109+
val parameters = paramsMap + popupExperimentParams
110+
111+
// app launch pixel
112+
pixel.get().fire(
113+
pixel = AppPixelName.APP_LAUNCH,
114+
parameters = parameters,
115+
)
116+
117+
// verified app launch pixel
118+
if (isVerifiedPlayStoreInstall() && !customTabDetector.isCustomTab()) {
99119
pixel.get().fire(
100-
pixel = AppPixelName.APP_LAUNCH,
120+
pixel = AppPixelName.APP_LAUNCH_VERIFIED_INSTALL,
101121
parameters = parameters,
102122
)
103-
104-
if (isVerifiedPlayStoreInstall() && !customTabDetector.isCustomTab()) {
105-
pixel.get().fire(
106-
pixel = AppPixelName.APP_LAUNCH_VERIFIED_INSTALL,
107-
parameters = parameters,
108-
)
109-
}
110123
}
111124
}
112125

@@ -123,14 +136,16 @@ class EnqueuedPixelWorker @Inject constructor(
123136
return false
124137
}
125138

126-
fun submitUnsentFirePixels() {
127-
val count = unsentForgetAllPixelStore.pendingPixelCountClearData
128-
logcat(INFO) { "Found $count unsent clear data pixels" }
129-
if (count > 0) {
130-
for (i in 1..count) {
131-
pixel.get().fire(AppPixelName.FORGET_ALL_EXECUTED)
139+
suspend fun submitUnsentFirePixels() {
140+
withContext(dispatchers.io()) {
141+
val count = unsentForgetAllPixelStore.pendingPixelCountClearData
142+
logcat(INFO) { "Found $count unsent clear data pixels" }
143+
if (count > 0) {
144+
for (i in 1..count) {
145+
pixel.get().fire(AppPixelName.FORGET_ALL_EXECUTED)
146+
}
147+
unsentForgetAllPixelStore.resetCount()
132148
}
133-
unsentForgetAllPixelStore.resetCount()
134149
}
135150
}
136151

‎app/src/test/java/com/duckduckgo/app/pixels/EnqueuedPixelWorkerTest.kt‎

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package com.duckduckgo.app.pixels
1818

19+
import android.annotation.SuppressLint
1920
import androidx.lifecycle.LifecycleOwner
2021
import androidx.work.ExistingPeriodicWorkPolicy
2122
import androidx.work.WorkManager
@@ -31,11 +32,13 @@ import com.duckduckgo.feature.toggles.api.FakeFeatureToggleFactory
3132
import com.duckduckgo.feature.toggles.api.Toggle.State
3233
import com.duckduckgo.privacyprotectionspopup.api.PrivacyProtectionsPopupExperimentExternalPixels
3334
import com.duckduckgo.verifiedinstallation.IsVerifiedPlayStoreInstall
35+
import kotlinx.coroutines.test.runTest
3436
import org.junit.Before
3537
import org.junit.Rule
3638
import org.junit.Test
3739
import org.mockito.kotlin.*
3840

41+
@SuppressLint("DenyListedApi")
3942
class EnqueuedPixelWorkerTest {
4043
@get:Rule
4144
var coroutineRule = CoroutineTestRule()
@@ -67,7 +70,8 @@ class EnqueuedPixelWorkerTest {
6770
privacyProtectionsPopupExperimentExternalPixels,
6871
isVerifiedPlayStoreInstall,
6972
appBuildConfig,
70-
coroutineRule.testScope,
73+
appCoroutineScope = coroutineRule.testScope,
74+
dispatchers = coroutineRule.testDispatcherProvider,
7175
)
7276
setupRemoteConfig(browserEnabled = false, collectFullWebViewVersionEnabled = false)
7377
}
@@ -285,6 +289,48 @@ class EnqueuedPixelWorkerTest {
285289
)
286290
}
287291

292+
@Test
293+
fun whenOnStartAndVerifiedAppLaunchThenSendVerifiedAppLaunchPixel() {
294+
whenever(isVerifiedPlayStoreInstall.invoke()).thenReturn(true)
295+
whenever(unsentForgetAllPixelStore.pendingPixelCountClearData).thenReturn(1)
296+
whenever(webViewVersionProvider.getMajorVersion()).thenReturn("91")
297+
whenever(defaultBrowserDetector.isDefaultBrowser()).thenReturn(false)
298+
299+
enqueuedPixelWorker.onCreate(lifecycleOwner)
300+
enqueuedPixelWorker.onStart(lifecycleOwner)
301+
302+
verify(pixel).fire(
303+
AppPixelName.APP_LAUNCH_VERIFIED_INSTALL,
304+
mapOf(
305+
Pixel.PixelParameter.WEBVIEW_VERSION to "91",
306+
Pixel.PixelParameter.DEFAULT_BROWSER to "false",
307+
Pixel.PixelParameter.IS_DUCKDUCKGO_PACKAGE to "false",
308+
),
309+
)
310+
}
311+
312+
@Test
313+
fun whenNoUnsentClearDataPixelsPendingThenPixelNotSent() = runTest {
314+
whenever(unsentForgetAllPixelStore.pendingPixelCountClearData).thenReturn(0)
315+
enqueuedPixelWorker.submitUnsentFirePixels()
316+
verify(pixel, never()).fire(AppPixelName.FORGET_ALL_EXECUTED)
317+
verify(unsentForgetAllPixelStore, never()).resetCount()
318+
}
319+
320+
@Test
321+
fun whenUnsentClearDataPixelsPendingThenPixelSent() = runTest {
322+
whenever(unsentForgetAllPixelStore.pendingPixelCountClearData).thenReturn(5)
323+
enqueuedPixelWorker.submitUnsentFirePixels()
324+
verify(pixel, times(5)).fire(AppPixelName.FORGET_ALL_EXECUTED)
325+
}
326+
327+
@Test
328+
fun whenClearDataPixelsSentThenStoreCleared() = runTest {
329+
whenever(unsentForgetAllPixelStore.pendingPixelCountClearData).thenReturn(5)
330+
enqueuedPixelWorker.submitUnsentFirePixels()
331+
verify(unsentForgetAllPixelStore).resetCount()
332+
}
333+
288334
private fun setupRemoteConfig(browserEnabled: Boolean, collectFullWebViewVersionEnabled: Boolean) {
289335
androidBrowserConfigFeature.self().setRawStoredState(State(enable = browserEnabled))
290336
androidBrowserConfigFeature.collectFullWebViewVersion().setRawStoredState(State(enable = collectFullWebViewVersionEnabled))

0 commit comments

Comments
 (0)