Skip to content

Commit 0e642df

Browse files
joshliebejoshliebe
authored
Update Loading Bar Experiment caching logic (#5041)
Task/Issue URL: https://app.asana.com/0/1200204095367872/1208349807308363/f ### Description - Updates the caching logic to load to memory on init. - Also adds a kill switch to disable the URI loaded pixel. ### Steps to test this PR Remove this from `LoadingBarExperimentVariantInitializer`: ``` loadingBarExperimentFeature.self().isEnabled() && loadingBarExperimentFeature.allocateVariants().isEnabled() ``` - [x] Install the app - [x] Verify that the experimental parameters are sent - [x] Kill the app - [x] Open the app again - [x] Verify that the correct experimental parameters are sent _Kill switch_ - [x] Load a page - [x] Verify that `m_uri_loaded` pixel is sent - [x] Load the config linked in the task - [x] Load a page - [x] Verify that `m_uri_loaded` pixel is not sent Privacy tests passed: https://github.com/duckduckgo/Android/actions/runs/10946311836 ✅
1 parent d06911c commit 0e642df

File tree

6 files changed

+165
-38
lines changed

6 files changed

+165
-38
lines changed

‎app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -940,11 +940,13 @@ class BrowserWebViewClientTest {
940940
val mockWebView = getImmediatelyInvokedMockWebView()
941941

942942
whenever(loadingBarExperimentManager.isExperimentEnabled()).thenReturn(true)
943+
whenever(loadingBarExperimentManager.shouldSendUriLoadedPixel).thenReturn(true)
943944
whenever(loadingBarExperimentManager.variant).thenReturn(true)
944945
whenever(mockWebView.settings).thenReturn(mock())
945946
whenever(mockWebView.safeCopyBackForwardList()).thenReturn(TestBackForwardList())
946947
whenever(mockWebView.progress).thenReturn(100)
947948

949+
testee.onPageStarted(mockWebView, EXAMPLE_URL, null)
948950
testee.onPageFinished(mockWebView, EXAMPLE_URL)
949951

950952
verify(pixel).fire(
@@ -959,10 +961,12 @@ class BrowserWebViewClientTest {
959961
val mockWebView = getImmediatelyInvokedMockWebView()
960962

961963
whenever(loadingBarExperimentManager.isExperimentEnabled()).thenReturn(false)
964+
whenever(loadingBarExperimentManager.shouldSendUriLoadedPixel).thenReturn(true)
962965
whenever(mockWebView.settings).thenReturn(mock())
963966
whenever(mockWebView.safeCopyBackForwardList()).thenReturn(TestBackForwardList())
964967
whenever(mockWebView.progress).thenReturn(100)
965968

969+
testee.onPageStarted(mockWebView, EXAMPLE_URL, null)
966970
testee.onPageFinished(mockWebView, EXAMPLE_URL)
967971

968972
verify(pixel).fire(AppPixelName.URI_LOADED)
@@ -974,11 +978,31 @@ class BrowserWebViewClientTest {
974978
val mockWebView = getImmediatelyInvokedMockWebView()
975979

976980
whenever(loadingBarExperimentManager.isExperimentEnabled()).thenReturn(true)
981+
whenever(loadingBarExperimentManager.shouldSendUriLoadedPixel).thenReturn(true)
977982
whenever(loadingBarExperimentManager.variant).thenReturn(true)
978983
whenever(mockWebView.settings).thenReturn(mock())
979984
whenever(mockWebView.safeCopyBackForwardList()).thenReturn(TestBackForwardList())
980985
whenever(mockWebView.progress).thenReturn(50)
981986

987+
testee.onPageStarted(mockWebView, EXAMPLE_URL, null)
988+
testee.onPageFinished(mockWebView, EXAMPLE_URL)
989+
990+
verify(pixel, never()).fire(anyString(), any(), any(), any())
991+
}
992+
993+
@UiThreadTest
994+
@Test
995+
fun whenShouldNotSendUriLoadedPixelThenNoPixelFired() {
996+
val mockWebView = getImmediatelyInvokedMockWebView()
997+
998+
whenever(loadingBarExperimentManager.isExperimentEnabled()).thenReturn(true)
999+
whenever(loadingBarExperimentManager.shouldSendUriLoadedPixel).thenReturn(false)
1000+
whenever(loadingBarExperimentManager.variant).thenReturn(true)
1001+
whenever(mockWebView.settings).thenReturn(mock())
1002+
whenever(mockWebView.safeCopyBackForwardList()).thenReturn(TestBackForwardList())
1003+
whenever(mockWebView.progress).thenReturn(100)
1004+
1005+
testee.onPageStarted(mockWebView, EXAMPLE_URL, null)
9821006
testee.onPageFinished(mockWebView, EXAMPLE_URL)
9831007

9841008
verify(pixel, never()).fire(anyString(), any(), any(), any())

‎app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -403,18 +403,20 @@ class BrowserWebViewClient @Inject constructor(
403403
navigationHistory.saveToHistory(url, navigationList.currentItem?.title)
404404
}
405405
}
406+
if (loadingBarExperimentManager.shouldSendUriLoadedPixel) {
407+
if (loadingBarExperimentManager.isExperimentEnabled()) {
408+
pixel.fire(
409+
AppPixelName.URI_LOADED.pixelName,
410+
mapOf(LOADING_BAR_EXPERIMENT to loadingBarExperimentManager.variant.toBinaryString()),
411+
)
412+
} else {
413+
pixel.fire(AppPixelName.URI_LOADED)
414+
}
415+
}
406416
start = null
407417
}
408418
}
409419
}
410-
if (loadingBarExperimentManager.isExperimentEnabled()) {
411-
pixel.fire(
412-
AppPixelName.URI_LOADED.pixelName,
413-
mapOf(LOADING_BAR_EXPERIMENT to loadingBarExperimentManager.variant.toBinaryString()),
414-
)
415-
} else {
416-
pixel.fire(AppPixelName.URI_LOADED)
417-
}
418420
}
419421
}
420422

‎experiments/experiments-api/src/main/java/com/duckduckgo/experiments/api/loadingbarexperiment/LoadingBarExperimentManager.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,5 @@ interface LoadingBarExperimentManager {
2020
fun isExperimentEnabled(): Boolean
2121
suspend fun update()
2222
val variant: Boolean
23+
val shouldSendUriLoadedPixel: Boolean
2324
}

‎experiments/experiments-impl/src/main/java/com/duckduckgo/experiments/impl/loadingbarexperiment/DuckDuckGoLoadingBarExperimentManager.kt‎

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,42 +16,63 @@
1616

1717
package com.duckduckgo.experiments.impl.loadingbarexperiment
1818

19+
import com.duckduckgo.app.di.AppCoroutineScope
20+
import com.duckduckgo.app.di.IsMainProcess
21+
import com.duckduckgo.common.utils.DispatcherProvider
1922
import com.duckduckgo.di.scopes.AppScope
2023
import com.duckduckgo.experiments.api.loadingbarexperiment.LoadingBarExperimentManager
2124
import com.squareup.anvil.annotations.ContributesBinding
2225
import dagger.SingleInstanceIn
2326
import javax.inject.Inject
27+
import kotlinx.coroutines.CoroutineScope
28+
import kotlinx.coroutines.launch
2429
import timber.log.Timber
2530

2631
@ContributesBinding(AppScope::class)
2732
@SingleInstanceIn(AppScope::class)
2833
class DuckDuckGoLoadingBarExperimentManager @Inject constructor(
2934
private val loadingBarExperimentDataStore: LoadingBarExperimentDataStore,
3035
private val loadingBarExperimentFeature: LoadingBarExperimentFeature,
36+
private val uriLoadedPixelFeature: UriLoadedPixelFeature,
37+
@AppCoroutineScope appCoroutineScope: CoroutineScope,
38+
dispatcherProvider: DispatcherProvider,
39+
@IsMainProcess isMainProcess: Boolean,
3140
) : LoadingBarExperimentManager {
3241

33-
private var hasVariant: Boolean? = null
34-
private var enabled: Boolean? = null
42+
private var cachedShouldSendUriLoadedPixel: Boolean = false
43+
private var cachedVariant: Boolean = false
44+
private var hasVariant: Boolean = false
45+
private var enabled: Boolean = false
3546

36-
override fun isExperimentEnabled(): Boolean {
37-
Timber.d("Loading bar experiment: Retrieving experiment status")
38-
if (hasVariant == null) {
39-
hasVariant = loadingBarExperimentDataStore.hasVariant
40-
}
47+
override val variant: Boolean
48+
get() = cachedVariant
49+
50+
override val shouldSendUriLoadedPixel: Boolean
51+
get() = cachedShouldSendUriLoadedPixel
4152

42-
if (enabled == null) {
43-
enabled = loadingBarExperimentFeature.self().isEnabled()
53+
init {
54+
appCoroutineScope.launch(dispatcherProvider.io()) {
55+
if (isMainProcess) {
56+
Timber.d("Loading bar experiment: Experimental variables initialized")
57+
loadToMemory()
58+
}
4459
}
60+
}
4561

46-
return hasVariant == true && enabled == true
62+
override fun isExperimentEnabled(): Boolean {
63+
Timber.d("Loading bar experiment: Retrieving experiment status")
64+
return hasVariant && enabled
4765
}
4866

4967
override suspend fun update() {
5068
Timber.d("Loading bar experiment: Experimental variables updated")
69+
loadToMemory()
70+
}
71+
72+
private fun loadToMemory() {
73+
cachedVariant = loadingBarExperimentDataStore.variant
5174
hasVariant = loadingBarExperimentDataStore.hasVariant
5275
enabled = loadingBarExperimentFeature.self().isEnabled()
76+
cachedShouldSendUriLoadedPixel = uriLoadedPixelFeature.self().isEnabled()
5377
}
54-
55-
override val variant: Boolean
56-
get() = loadingBarExperimentDataStore.variant
5778
}

‎experiments/experiments-impl/src/main/java/com/duckduckgo/experiments/impl/loadingbarexperiment/UriLoadedPixelFeature.kt‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/*
2+
* Copyright (c) 2024 DuckDuckGo
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.duckduckgo.experiments.impl.loadingbarexperiment
18+
19+
import com.duckduckgo.anvil.annotations.ContributesRemoteFeature
20+
import com.duckduckgo.di.scopes.AppScope
21+
import com.duckduckgo.feature.toggles.api.Toggle
22+
23+
@ContributesRemoteFeature(
24+
scope = AppScope::class,
25+
featureName = "sendUriLoadedPixel",
26+
)
27+
interface UriLoadedPixelFeature {
28+
29+
@Toggle.DefaultValue(true)
30+
fun self(): Toggle
31+
}

‎experiments/experiments-impl/src/test/java/com/duckduckgo/experiments/impl/loadingbarexperiment/DuckDuckGoLoadingBarExperimentManagerTest.kt‎

Lines changed: 65 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,18 @@
1414
* limitations under the License.
1515
*/
1616

17+
import com.duckduckgo.common.test.CoroutineTestRule
1718
import com.duckduckgo.experiments.impl.loadingbarexperiment.DuckDuckGoLoadingBarExperimentManager
1819
import com.duckduckgo.experiments.impl.loadingbarexperiment.LoadingBarExperimentDataStore
1920
import com.duckduckgo.experiments.impl.loadingbarexperiment.LoadingBarExperimentFeature
21+
import com.duckduckgo.experiments.impl.loadingbarexperiment.UriLoadedPixelFeature
2022
import com.duckduckgo.feature.toggles.api.Toggle
2123
import junit.framework.TestCase.assertFalse
2224
import junit.framework.TestCase.assertTrue
25+
import kotlinx.coroutines.test.TestScope
2326
import kotlinx.coroutines.test.runTest
2427
import org.junit.Before
28+
import org.junit.Rule
2529
import org.junit.Test
2630
import org.mockito.kotlin.*
2731

@@ -31,21 +35,28 @@ class DuckDuckGoLoadingBarExperimentManagerTest {
3135

3236
private val mockLoadingBarExperimentDataStore: LoadingBarExperimentDataStore = mock()
3337
private val mockLoadingBarExperimentFeature: LoadingBarExperimentFeature = mock()
38+
private val mockUriLoadedPixelFeature: UriLoadedPixelFeature = mock()
3439
private val mockToggle: Toggle = mock()
40+
private val mockUriLoadedKillSwitch: Toggle = mock()
41+
42+
@get:Rule
43+
val coroutineTestRule: CoroutineTestRule = CoroutineTestRule()
3544

3645
@Before
3746
fun setup() {
38-
testee = DuckDuckGoLoadingBarExperimentManager(mockLoadingBarExperimentDataStore, mockLoadingBarExperimentFeature)
3947
whenever(mockLoadingBarExperimentFeature.self()).thenReturn(mockToggle)
48+
whenever(mockUriLoadedPixelFeature.self()).thenReturn(mockUriLoadedKillSwitch)
49+
whenever(mockUriLoadedKillSwitch.isEnabled()).thenReturn(true)
4050
}
4151

4252
@Test
4353
fun whenHasVariantAndIsEnabledThenIsExperimentEnabledReturnsTrue() {
4454
whenever(mockLoadingBarExperimentDataStore.hasVariant).thenReturn(true)
4555
whenever(mockToggle.isEnabled()).thenReturn(true)
4656

47-
assertTrue(testee.isExperimentEnabled())
57+
initialize()
4858

59+
assertTrue(testee.isExperimentEnabled())
4960
verify(mockLoadingBarExperimentDataStore).hasVariant
5061
verify(mockLoadingBarExperimentFeature.self()).isEnabled()
5162
}
@@ -55,79 +66,116 @@ class DuckDuckGoLoadingBarExperimentManagerTest {
5566
whenever(mockLoadingBarExperimentDataStore.hasVariant).thenReturn(false)
5667
whenever(mockToggle.isEnabled()).thenReturn(true)
5768

58-
assertFalse(testee.isExperimentEnabled())
69+
initialize()
5970

71+
assertFalse(testee.isExperimentEnabled())
6072
verify(mockLoadingBarExperimentDataStore).hasVariant
61-
verify(mockLoadingBarExperimentFeature.self()).isEnabled()
6273
}
6374

6475
@Test
6576
fun whenIsNotEnabledThenIsExperimentEnabledReturnsFalse() {
6677
whenever(mockLoadingBarExperimentDataStore.hasVariant).thenReturn(true)
6778
whenever(mockToggle.isEnabled()).thenReturn(false)
6879

69-
assertFalse(testee.isExperimentEnabled())
80+
initialize()
7081

82+
assertFalse(testee.isExperimentEnabled())
7183
verify(mockLoadingBarExperimentDataStore).hasVariant
72-
verify(mockLoadingBarExperimentFeature.self()).isEnabled()
7384
}
7485

7586
@Test
7687
fun whenHasNoVariantAndIsNotEnabledThenIsExperimentEnabledReturnsFalse() {
7788
whenever(mockLoadingBarExperimentDataStore.hasVariant).thenReturn(false)
7889
whenever(mockToggle.isEnabled()).thenReturn(false)
7990

80-
assertFalse(testee.isExperimentEnabled())
91+
initialize()
8192

93+
assertFalse(testee.isExperimentEnabled())
8294
verify(mockLoadingBarExperimentDataStore).hasVariant
83-
verify(mockLoadingBarExperimentFeature.self()).isEnabled()
8495
}
8596

8697
@Test
8798
fun whenUpdateCalledThenCachedVariablesAreUpdated() = runTest {
99+
var numInvocations = 0
100+
101+
initialize()
102+
103+
verifyVariablesUpdated(++numInvocations)
104+
88105
whenever(mockLoadingBarExperimentDataStore.hasVariant).thenReturn(false)
89106
whenever(mockToggle.isEnabled()).thenReturn(true)
90107

91108
testee.update()
92109

93110
assertFalse(testee.isExperimentEnabled())
94-
verify(mockLoadingBarExperimentDataStore, times(1)).hasVariant
95-
verify(mockToggle, times(1)).isEnabled()
111+
verifyVariablesUpdated(++numInvocations)
96112

97113
whenever(mockLoadingBarExperimentDataStore.hasVariant).thenReturn(false)
98114
whenever(mockToggle.isEnabled()).thenReturn(false)
99115

100116
testee.update()
101117

102118
assertFalse(testee.isExperimentEnabled())
103-
verify(mockLoadingBarExperimentDataStore, times(2)).hasVariant
104-
verify(mockToggle, times(2)).isEnabled()
119+
verifyVariablesUpdated(++numInvocations)
105120

106121
whenever(mockLoadingBarExperimentDataStore.hasVariant).thenReturn(true)
107122
whenever(mockToggle.isEnabled()).thenReturn(false)
108123

109124
testee.update()
110125

111126
assertFalse(testee.isExperimentEnabled())
112-
verify(mockLoadingBarExperimentDataStore, times(3)).hasVariant
113-
verify(mockToggle, times(3)).isEnabled()
127+
verifyVariablesUpdated(++numInvocations)
114128

115129
whenever(mockLoadingBarExperimentDataStore.hasVariant).thenReturn(true)
116130
whenever(mockToggle.isEnabled()).thenReturn(true)
117131

118132
testee.update()
119133

120134
assertTrue(testee.isExperimentEnabled())
121-
verify(mockLoadingBarExperimentDataStore, times(4)).hasVariant
122-
verify(mockToggle, times(4)).isEnabled()
135+
verifyVariablesUpdated(++numInvocations)
123136
}
124137

125138
@Test
126139
fun whenGetVariantThenVariantIsReturned() {
127140
whenever(mockLoadingBarExperimentDataStore.variant).thenReturn(true)
128141

129-
assertTrue(testee.variant)
142+
initialize()
130143

144+
assertTrue(testee.variant)
131145
verify(mockLoadingBarExperimentDataStore).variant
132146
}
147+
148+
@Test
149+
fun whenShouldSendUriLoadedPixelEnabledThenReturnTrue() {
150+
initialize()
151+
152+
assertTrue(testee.shouldSendUriLoadedPixel)
153+
}
154+
155+
@Test
156+
fun whenShouldSendUriLoadedPixelDisabledThenReturnFalse() {
157+
whenever(mockUriLoadedKillSwitch.isEnabled()).thenReturn(false)
158+
159+
initialize()
160+
161+
assertFalse(testee.shouldSendUriLoadedPixel)
162+
}
163+
164+
private fun initialize() {
165+
testee = DuckDuckGoLoadingBarExperimentManager(
166+
mockLoadingBarExperimentDataStore,
167+
mockLoadingBarExperimentFeature,
168+
mockUriLoadedPixelFeature,
169+
TestScope(),
170+
coroutineTestRule.testDispatcherProvider,
171+
isMainProcess = true,
172+
)
173+
}
174+
175+
private fun verifyVariablesUpdated(numInvocations: Int) {
176+
verify(mockLoadingBarExperimentDataStore, times(numInvocations)).hasVariant
177+
verify(mockLoadingBarExperimentDataStore, times(numInvocations)).variant
178+
verify(mockToggle, times(numInvocations)).isEnabled()
179+
verify(mockUriLoadedKillSwitch, times(numInvocations)).isEnabled()
180+
}
133181
}

0 commit comments

Comments
 (0)