Skip to content

Commit 5f528da

Browse files
lagheeanikiki
andauthored
Add 4 new params to broken site pixel metadata (#4873)
Task/Issue URL: https://app.asana.com/0/1201870266890790/1207382663027468/f ### Description We want to add `locale` (lang-country, e.g., `en-US`), `userRefreshCount`, `openerContext` (serp, navigation, external) and `jsPerformance` (only first contentful paint so far) to breakage reports to match other platforms. ### Steps to test this PR - [x] Override privacy configuration url to use https://jsonblob.com/api/1263877779169861632 - [x] Check that you have content-scope-scripts v. 6.6.0+ - [x] Override [features.js](https://github.com/duckduckgo/content-scope-scripts/blob/607d1752d7a329199469ed5b9c5ebf945e6d49ff/src/features.js#L45) file in content-scope-scripts to add `'breakageReporting'`, rebuild and update [the C-S-S build.gradle](https://github.com/duckduckgo/Android/blob/a91d3ca35e2675b72c20732a18d30d2b6569bd89/content-scope-scripts/content-scope-scripts-impl/build.gradle#L75) to use your local path - [x] Launch and set this build as the default browser app - [x] Perform a search, then click on a result - [x] Refresh the page a few times (remembering the count 😁) - [x] Send a broken site report through the 3-dot menu - [x] Verify from logcat and/or checking that pixel `ebpf.android*` is sent correctly with all four new params (and that refresh count corresponds and openerContext = "serp" - [x] Click on a link in your current tab to go to a new page - [x] Refresh a couple more times - [x] Send a broken site report through the duckfoot dashboard - [x] Verify the pixel metadata params again in logcat and/or directly (openerContext should be "navigation") - [x] Close the app and click on a url link from email (custom tab), notes or MM (directly to browser), or any other 3rd-party app - [x] Repeat sending and verifying pixel params + [x] openerContext should be "external" in all these cases: - opening in a custom tab - opening the full browser from the custom tab - opening in the full browser directly from another app --------- Co-authored-by: Ana Capatina <[email protected]>
1 parent 2075d1e commit 5f528da

File tree

49 files changed

+1481
-46
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+1481
-46
lines changed

app/build.gradle

+2
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,8 @@ dependencies {
348348
implementation project(':survey-api')
349349
implementation project(':survey-impl')
350350

351+
implementation project(':breakage-reporting-impl')
352+
351353
// Deprecated. TODO: Stop using this artifact.
352354
implementation "androidx.legacy:legacy-support-v4:_"
353355
debugImplementation Square.leakCanary.android

app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt

+26-7
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ import com.duckduckgo.autofill.api.email.EmailManager
164164
import com.duckduckgo.autofill.api.passwordgeneration.AutomaticSavedLoginsMonitor
165165
import com.duckduckgo.autofill.impl.AutofillFireproofDialogSuppressor
166166
import com.duckduckgo.browser.api.UserBrowserProperties
167+
import com.duckduckgo.browser.api.brokensite.BrokenSiteContext
167168
import com.duckduckgo.common.test.CoroutineTestRule
168169
import com.duckduckgo.common.test.InstantSchedulersRule
169170
import com.duckduckgo.common.utils.DispatcherProvider
@@ -388,6 +389,9 @@ class BrowserTabViewModelTest {
388389
@Mock
389390
private lateinit var mockUserAllowListRepository: UserAllowListRepository
390391

392+
@Mock
393+
private lateinit var mockBrokenSiteContext: BrokenSiteContext
394+
391395
@Mock
392396
private lateinit var mockFileChooserCallback: ValueCallback<Array<Uri>>
393397

@@ -533,6 +537,7 @@ class BrowserTabViewModelTest {
533537
mockBypassedSSLCertificatesRepository,
534538
coroutineRule.testScope,
535539
coroutineRule.testDispatcherProvider,
540+
DuckDuckGoUrlDetectorImpl(),
536541
)
537542

538543
accessibilitySettingsDataStore = AccessibilitySettingsSharedPreferences(
@@ -617,7 +622,7 @@ class BrowserTabViewModelTest {
617622
newTabPixels = { mockNewTabPixels },
618623
)
619624

620-
testee.loadData("abc", null, false)
625+
testee.loadData("abc", null, false, false)
621626
testee.command.observeForever(mockCommandObserver)
622627
}
623628

@@ -2036,21 +2041,21 @@ class BrowserTabViewModelTest {
20362041

20372042
@Test
20382043
fun whenUrlNullThenSetBrowserNotShowing() = runTest {
2039-
testee.loadData("id", null, false)
2044+
testee.loadData("id", null, false, false)
20402045
testee.determineShowBrowser()
20412046
assertEquals(false, testee.browserViewState.value?.browserShowing)
20422047
}
20432048

20442049
@Test
20452050
fun whenUrlBlankThenSetBrowserNotShowing() = runTest {
2046-
testee.loadData("id", " ", false)
2051+
testee.loadData("id", " ", false, false)
20472052
testee.determineShowBrowser()
20482053
assertEquals(false, testee.browserViewState.value?.browserShowing)
20492054
}
20502055

20512056
@Test
20522057
fun whenUrlPresentThenSetBrowserShowing() = runTest {
2053-
testee.loadData("id", "https://example.com", false)
2058+
testee.loadData("id", "https://example.com", false, false)
20542059
testee.determineShowBrowser()
20552060
assertEquals(true, testee.browserViewState.value?.browserShowing)
20562061
}
@@ -4944,6 +4949,19 @@ class BrowserTabViewModelTest {
49444949
verify(mockPrivacyProtectionsPopupManager).onPageRefreshTriggeredByUser()
49454950
}
49464951

4952+
@Test
4953+
fun whenOnlyChangeInUrlIsHttpsUpgradeNakedDomainRedirectOrTrailingSlashThenConsiderSameForExternalLaunch() = runTest {
4954+
val urlA = "https://example.com"
4955+
val urlB = "http://www.example.com"
4956+
val urlC = "https://www.example.com/"
4957+
val urlD = "http://www.example.com/path/"
4958+
4959+
assertTrue(testee.urlUnchangedForExternalLaunchPurposes(urlA, urlB))
4960+
assertTrue(testee.urlUnchangedForExternalLaunchPurposes(urlB, urlC))
4961+
assertTrue(testee.urlUnchangedForExternalLaunchPurposes(urlA, urlC))
4962+
assertFalse(testee.urlUnchangedForExternalLaunchPurposes(urlC, urlD))
4963+
}
4964+
49474965
@Test
49484966
fun whenPrivacyProtectionsAreToggledThenCorrectPixelsAreSent() = runTest {
49494967
val params = mapOf("test_key" to "test_value")
@@ -5561,7 +5579,7 @@ class BrowserTabViewModelTest {
55615579

55625580
private fun givenOneActiveTabSelected() {
55635581
selectedTabLiveData.value = TabEntity("TAB_ID", "https://example.com", "", skipHome = false, viewed = true, position = 0)
5564-
testee.loadData("TAB_ID", "https://example.com", false)
5582+
testee.loadData("TAB_ID", "https://example.com", false, false)
55655583
}
55665584

55675585
private fun givenFireproofWebsiteDomain(vararg fireproofWebsitesDomain: String) {
@@ -5577,10 +5595,11 @@ class BrowserTabViewModelTest {
55775595
whenever(site.url).thenReturn(domain)
55785596
whenever(site.nextUrl).thenReturn(domain)
55795597
whenever(site.uri).thenReturn(Uri.parse(domain))
5598+
whenever(site.realBrokenSiteContext).thenReturn(mockBrokenSiteContext)
55805599
val siteLiveData = MutableLiveData<Site>()
55815600
siteLiveData.value = site
55825601
whenever(mockTabRepository.retrieveSiteData("TAB_ID")).thenReturn(siteLiveData)
5583-
testee.loadData("TAB_ID", domain, false)
5602+
testee.loadData("TAB_ID", domain, false, false)
55845603

55855604
return site
55865605
}
@@ -5608,7 +5627,7 @@ class BrowserTabViewModelTest {
56085627
}
56095628

56105629
private fun loadTabWithId(tabId: String) {
5611-
testee.loadData(tabId, initialUrl = null, skipHome = false)
5630+
testee.loadData(tabId, initialUrl = null, skipHome = false, isExternal = false)
56125631
}
56135632

56145633
private fun loadUrl(

app/src/androidTest/java/com/duckduckgo/app/tabs/model/TabDataRepositoryTest.kt

+3
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import androidx.lifecycle.MutableLiveData
2323
import androidx.room.Room
2424
import androidx.test.platform.app.InstrumentationRegistry
2525
import com.duckduckgo.app.blockingObserve
26+
import com.duckduckgo.app.browser.DuckDuckGoUrlDetector
2627
import com.duckduckgo.app.browser.certificates.BypassedSSLCertificatesRepository
2728
import com.duckduckgo.app.browser.favicon.FaviconManager
2829
import com.duckduckgo.app.browser.tabpreview.WebViewPreviewPersister
@@ -411,6 +412,7 @@ class TabDataRepositoryTest {
411412
webViewPreviewPersister: WebViewPreviewPersister = mock(),
412413
faviconManager: FaviconManager = mock(),
413414
tabSwitcherDataStore: TabSwitcherDataStore = mock(),
415+
duckDuckGoUrlDetector: DuckDuckGoUrlDetector = mock(),
414416
): TabDataRepository {
415417
return TabDataRepository(
416418
dao,
@@ -421,6 +423,7 @@ class TabDataRepositoryTest {
421423
bypassedSSLCertificatesRepository,
422424
coroutinesTestRule.testScope,
423425
coroutinesTestRule.testDispatcherProvider,
426+
duckDuckGoUrlDetector,
424427
),
425428
webViewPreviewPersister,
426429
faviconManager,

app/src/main/java/com/duckduckgo/app/brokensite/BrokenSiteActivity.kt

+13
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import com.duckduckgo.appbuildconfig.api.AppBuildConfig
3535
import com.duckduckgo.browser.api.WebViewVersionProvider
3636
import com.duckduckgo.browser.api.brokensite.BrokenSiteData
3737
import com.duckduckgo.browser.api.brokensite.BrokenSiteData.ReportFlow
38+
import com.duckduckgo.browser.api.brokensite.BrokenSiteOpenerContext
3839
import com.duckduckgo.common.ui.DuckDuckGoActivity
3940
import com.duckduckgo.common.ui.view.dialog.DaxAlertDialog
4041
import com.duckduckgo.common.ui.view.dialog.RadioListAlertDialogBuilder
@@ -89,6 +90,9 @@ class BrokenSiteActivity : DuckDuckGoActivity() {
8990
val httpErrorCodes = intent.getStringExtra(HTTP_ERROR_CODES).orEmpty()
9091
val isDesktopMode = intent.getBooleanExtra(IS_DESKTOP_MODE, false)
9192
val reportFlow = intent.getSerializableExtra<ReportFlow>(REPORT_FLOW)
93+
val userRefreshCount = intent.getIntExtra(USER_REFRESH_COUNT, 0)
94+
val openerContext = intent.getSerializableExtra<BrokenSiteOpenerContext>(OPENER_CONTEXT)
95+
val jsPerformance = intent.getDoubleArrayExtra(JS_PERFORMANCE)
9296
viewModel.setInitialBrokenSite(
9397
url = url,
9498
blockedTrackers = blockedTrackers,
@@ -102,6 +106,9 @@ class BrokenSiteActivity : DuckDuckGoActivity() {
102106
httpErrorCodes = httpErrorCodes,
103107
isDesktopMode = isDesktopMode,
104108
reportFlow = reportFlow,
109+
userRefreshCount = userRefreshCount,
110+
openerContext = openerContext,
111+
jsPerformance = jsPerformance,
105112
)
106113
}
107114

@@ -248,6 +255,9 @@ class BrokenSiteActivity : DuckDuckGoActivity() {
248255
private const val HTTP_ERROR_CODES = "HTTP_ERROR_CODES"
249256
private const val IS_DESKTOP_MODE = "IS_DESKTOP_MODE"
250257
private const val REPORT_FLOW = "REPORT_FLOW"
258+
private const val USER_REFRESH_COUNT = "USER_REFRESH_COUNT"
259+
private const val OPENER_CONTEXT = "OPENER_CONTEXT"
260+
private const val JS_PERFORMANCE = "JS_PERFORMANCE"
251261

252262
fun intent(
253263
context: Context,
@@ -266,6 +276,9 @@ class BrokenSiteActivity : DuckDuckGoActivity() {
266276
intent.putExtra(HTTP_ERROR_CODES, data.httpErrorCodes)
267277
intent.putExtra(IS_DESKTOP_MODE, data.isDesktopMode)
268278
intent.putExtra(REPORT_FLOW, data.reportFlow)
279+
intent.putExtra(USER_REFRESH_COUNT, data.userRefreshCount)
280+
intent.putExtra(OPENER_CONTEXT, data.openerContext)
281+
intent.putExtra(JS_PERFORMANCE, data.jsPerformance)
269282
return intent
270283
}
271284
}

app/src/main/java/com/duckduckgo/app/brokensite/BrokenSiteViewModel.kt

+13
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.COUNT
3838
import com.duckduckgo.browser.api.brokensite.BrokenSiteData.ReportFlow
3939
import com.duckduckgo.browser.api.brokensite.BrokenSiteData.ReportFlow.DASHBOARD
4040
import com.duckduckgo.browser.api.brokensite.BrokenSiteData.ReportFlow.MENU
41+
import com.duckduckgo.browser.api.brokensite.BrokenSiteOpenerContext
4142
import com.duckduckgo.common.utils.SingleLiveEvent
4243
import com.duckduckgo.common.utils.extractDomain
4344
import com.duckduckgo.di.scopes.ActivityScope
@@ -112,6 +113,9 @@ class BrokenSiteViewModel @Inject constructor(
112113
private var httpErrorCodes: String = ""
113114
private var isDesktopMode: Boolean = false
114115
private var reportFlow: ReportFlow? = null
116+
private var userRefreshCount: Int = 0
117+
private var openerContext: BrokenSiteOpenerContext? = null
118+
private var jsPerformance: DoubleArray? = null
115119

116120
var shuffledCategories = mutableListOf<BrokenSiteCategory>()
117121

@@ -133,6 +137,9 @@ class BrokenSiteViewModel @Inject constructor(
133137
httpErrorCodes: String,
134138
isDesktopMode: Boolean,
135139
reportFlow: ReportFlow?,
140+
userRefreshCount: Int,
141+
openerContext: BrokenSiteOpenerContext?,
142+
jsPerformance: DoubleArray?,
136143
) {
137144
this.url = url
138145
this.blockedTrackers = blockedTrackers
@@ -146,6 +153,9 @@ class BrokenSiteViewModel @Inject constructor(
146153
this.httpErrorCodes = httpErrorCodes
147154
this.isDesktopMode = isDesktopMode
148155
this.reportFlow = reportFlow
156+
this.userRefreshCount = userRefreshCount
157+
this.openerContext = openerContext
158+
this.jsPerformance = jsPerformance
149159

150160
loadProtectionsState()
151161
}
@@ -268,6 +278,9 @@ class BrokenSiteViewModel @Inject constructor(
268278
httpErrorCodes = httpErrorCodes,
269279
loginSite = loginSite,
270280
reportFlow = reportFlow?.mapToBrokenSiteModelReportFlow(),
281+
userRefreshCount = userRefreshCount,
282+
openerContext = openerContext,
283+
jsPerformance = jsPerformance,
271284
)
272285
}
273286

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
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.app.brokensite
18+
19+
import androidx.core.net.toUri
20+
import com.duckduckgo.app.browser.DuckDuckGoUrlDetector
21+
import com.duckduckgo.browser.api.brokensite.BrokenSiteContext
22+
import com.duckduckgo.browser.api.brokensite.BrokenSiteOpenerContext
23+
import com.duckduckgo.common.utils.isHttp
24+
import com.duckduckgo.common.utils.isHttps
25+
import com.duckduckgo.di.scopes.AppScope
26+
import com.squareup.anvil.annotations.ContributesBinding
27+
import javax.inject.Inject
28+
import org.json.JSONArray
29+
import timber.log.Timber
30+
31+
@ContributesBinding(AppScope::class)
32+
class RealBrokenSiteContext @Inject constructor(
33+
private val duckDuckGoUrlDetector: DuckDuckGoUrlDetector,
34+
) : BrokenSiteContext {
35+
36+
override var userRefreshCount: Int = 0
37+
38+
override var openerContext: BrokenSiteOpenerContext? = null
39+
40+
override var jsPerformance: DoubleArray? = null
41+
42+
override fun onUserTriggeredRefresh() {
43+
userRefreshCount++
44+
}
45+
46+
override fun inferOpenerContext(
47+
referrer: String?,
48+
isExternalLaunch: Boolean,
49+
) {
50+
if (isExternalLaunch) {
51+
openerContext = BrokenSiteOpenerContext.EXTERNAL
52+
} else if (referrer != null) {
53+
openerContext = when {
54+
duckDuckGoUrlDetector.isDuckDuckGoUrl(referrer) -> BrokenSiteOpenerContext.SERP
55+
referrer.toUri().isHttp || referrer.toUri().isHttps -> BrokenSiteOpenerContext.NAVIGATION
56+
else -> null
57+
}
58+
Timber.d(
59+
"openerContext inferred -> ${openerContext?.context}",
60+
)
61+
} else {
62+
Timber.d("openerContext not inferred because referrer is null")
63+
}
64+
}
65+
66+
override fun recordJsPerformance(performanceMetrics: JSONArray) {
67+
val recordedJsValues = DoubleArray(performanceMetrics.length())
68+
for (i in 0 until performanceMetrics.length()) {
69+
recordedJsValues[i] = performanceMetrics.getDouble(i)
70+
}
71+
jsPerformance = recordedJsValues
72+
Timber.d("jsPerformance recorded as $performanceMetrics")
73+
}
74+
}

app/src/main/java/com/duckduckgo/app/brokensite/api/BrokenSiteSender.kt

+10
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import com.duckduckgo.brokensite.api.BrokenSiteLastSentReport
3333
import com.duckduckgo.common.utils.DispatcherProvider
3434
import com.duckduckgo.common.utils.absoluteString
3535
import com.duckduckgo.common.utils.domain
36+
import com.duckduckgo.common.utils.extensions.toSanitizedLanguageTag
3637
import com.duckduckgo.di.scopes.AppScope
3738
import com.duckduckgo.experiments.api.VariantManager
3839
import com.duckduckgo.feature.toggles.api.FeatureToggle
@@ -86,6 +87,7 @@ class BrokenSiteSubmitter @Inject constructor(
8687
!contentBlocking.isAnException(brokenSite.siteUrl)
8788

8889
val vpnOn = runCatching { networkProtectionState.isRunning() }.getOrNull()
90+
val locale = appBuildConfig.deviceLocale.toSanitizedLanguageTag()
8991

9092
val params = mutableMapOf(
9193
CATEGORY_KEY to brokenSite.category.orEmpty(),
@@ -111,6 +113,10 @@ class BrokenSiteSubmitter @Inject constructor(
111113
HTTP_ERROR_CODES_KEY to brokenSite.httpErrorCodes,
112114
PROTECTIONS_STATE to protectionsState.toString(),
113115
VPN_ON to vpnOn.toString(),
116+
LOCALE to locale,
117+
USER_REFRESH_COUNT to brokenSite.userRefreshCount.toString(),
118+
OPENER_CONTEXT to brokenSite.openerContext?.context.orEmpty(),
119+
JS_PERFORMANCE to brokenSite.jsPerformance?.joinToString(",").orEmpty(),
114120
)
115121

116122
brokenSite.reportFlow?.let { reportFlow ->
@@ -178,6 +184,10 @@ class BrokenSiteSubmitter @Inject constructor(
178184
private const val LOGIN_SITE = "loginSite"
179185
private const val REPORT_FLOW = "reportFlow"
180186
private const val VPN_ON = "vpnOn"
187+
private const val LOCALE = "locale"
188+
private const val USER_REFRESH_COUNT = "userRefreshCount"
189+
private const val OPENER_CONTEXT = "openerContext"
190+
private const val JS_PERFORMANCE = "jsPerformance"
181191
}
182192
}
183193

app/src/main/java/com/duckduckgo/app/brokensite/model/BrokenSite.kt

+4
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package com.duckduckgo.app.brokensite.model
1818

1919
import androidx.annotation.StringRes
2020
import com.duckduckgo.app.browser.R
21+
import com.duckduckgo.browser.api.brokensite.BrokenSiteOpenerContext
2122

2223
data class BrokenSite(
2324
val category: String?,
@@ -36,6 +37,9 @@ data class BrokenSite(
3637
val httpErrorCodes: String,
3738
val loginSite: String?,
3839
val reportFlow: ReportFlow?,
40+
val userRefreshCount: Int,
41+
val openerContext: BrokenSiteOpenerContext?,
42+
val jsPerformance: DoubleArray?,
3943
)
4044

4145
sealed class BrokenSiteCategory(

0 commit comments

Comments
 (0)