Skip to content

Commit

Permalink
Expand breakage params (#2577)
Browse files Browse the repository at this point in the history
* Add parameters and content script

* Fixes

* lint

* Fix tests

* lint

* PR changes

* Update browsers/chrome/manifest.json

Co-authored-by: Sam Macbeth <[email protected]>

---------

Co-authored-by: Sam Macbeth <[email protected]>
  • Loading branch information
SlayterDev and sammacbeth authored Jul 5, 2024
1 parent f875594 commit a58a481
Show file tree
Hide file tree
Showing 14 changed files with 182 additions and 9 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ $(LAST_COPY): $(WATCHED_FILES) | $(MKDIR_TARGETS)
$(RSYNC) node_modules/@duckduckgo/privacy-dashboard/build/app/* $(BUILD_DIR)/dashboard
$(RSYNC) node_modules/@duckduckgo/autofill/dist/autofill.css $(BUILD_DIR)/public/css/autofill.css
$(RSYNC) node_modules/@duckduckgo/autofill/dist/autofill-host-styles_$(BROWSER_TYPE).css $(BUILD_DIR)/public/css/autofill-host-styles.css
$(RSYNC) node_modules/@duckduckgo/autofill/dist/*.js shared/js/content-scripts/content-scope-messaging.js $(BUILD_DIR)/public/js/content-scripts
$(RSYNC) node_modules/@duckduckgo/autofill/dist/*.js shared/js/content-scripts/*.js $(BUILD_DIR)/public/js/content-scripts
$(RSYNC) node_modules/@duckduckgo/tracker-surrogates/surrogates/* $(BUILD_DIR)/web_accessible_resources
touch $@

Expand Down
14 changes: 14 additions & 0 deletions browsers/chrome-mv2/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,20 @@
"all_frames": true,
"run_at": "document_start",
"match_about_blank": true
},
{
"matches": [
"<all_urls>"
],
"exclude_matches": [
"*://localhost/*",
"*://*.localhost/*"
],
"match_about_blank": true,
"run_at": "document_start",
"js": [
"public/js/content-scripts/breakage-stats.js"
]
}
],
"permissions": [
Expand Down
14 changes: 14 additions & 0 deletions browsers/chrome/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,20 @@
"run_at": "document_start",
"match_origin_as_fallback": true,
"match_about_blank": true
},
{
"matches": [
"<all_urls>"
],
"exclude_matches": [
"*://localhost/*",
"*://*.localhost/*"
],
"match_about_blank": true,
"run_at": "document_start",
"js": [
"public/js/content-scripts/breakage-stats.js"
]
}
],
"permissions": [
Expand Down
14 changes: 14 additions & 0 deletions browsers/firefox/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,20 @@
"all_frames": true,
"run_at": "document_start",
"match_about_blank": true
},
{
"matches": [
"<all_urls>"
],
"exclude_matches": [
"*://localhost/*",
"*://*.localhost/*"
],
"match_about_blank": true,
"run_at": "document_start",
"js": [
"public/js/content-scripts/breakage-stats.js"
]
}
],
"options_ui": {
Expand Down
29 changes: 27 additions & 2 deletions shared/js/background/broken-site-report.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,11 @@ export async function clearAllBrokenSiteReportTimes () {
* @prop {string} arg.remoteConfigVersion - config version
* @prop {string | undefined} arg.category - optional category
* @prop {string | undefined} arg.description - optional description
* @prop {Object | undefined} arg.pageParams - on page parameters
*/
export async function breakageReportForTab ({
tab, tds, remoteConfigEtag, remoteConfigVersion,
category, description
category, description, pageParams
}) {
if (!tab.url) {
return
Expand All @@ -179,6 +180,22 @@ export async function breakageReportForTab ({
}
}

// collect page parameters
if (pageParams.docReferrer && pageParams.docReferrer !== '') {
try {
const referrerUrl = new URL(pageParams.docReferrer)
if (referrerUrl.hostname === 'duckduckgo.com') {
tab.openerContext = 'serp'
} else {
tab.openerContext = 'navigation'
}
} catch {
console.error('Unable to construct referrer URL from:' + pageParams.docReferrer)
}
} else if (!pageParams.opener) {
tab.openerContext = 'external'
}

const urlParametersRemoved = tab.urlParametersRemoved ? 'true' : 'false'
const ctlYouTube = tab.ctlYouTube ? 'true' : 'false'
const ctlFacebookPlaceholderShown = tab.ctlFacebookPlaceholderShown ? 'true' : 'false'
Expand All @@ -190,6 +207,10 @@ export async function breakageReportForTab ({
const errorDescriptions = JSON.stringify(tab.errorDescriptions)
const httpErrorCodes = tab.httpErrorCodes.join(',')
const lastSentDay = await computeLastSentDay(tab.url)
const userRefreshCount = tab.userRefreshCount
const openerContext = tab.openerContext ? tab.openerContext : undefined
const jsPerformance = pageParams.jsPerformance ? pageParams.jsPerformance : undefined
const locale = tab.locale

const brokenSiteParams = new URLSearchParams({
siteUrl,
Expand All @@ -202,7 +223,10 @@ export async function breakageReportForTab ({
ctlFacebookPlaceholderShown,
ctlFacebookLogin,
performanceWarning,
protectionsState: tab.site.isFeatureEnabled('contentBlocking')
protectionsState: tab.site.isFeatureEnabled('contentBlocking'),
userRefreshCount,
jsPerformance,
locale
})

for (const [key, value] of Object.entries(requestCategories)) {
Expand All @@ -216,6 +240,7 @@ export async function breakageReportForTab ({
if (description) brokenSiteParams.set('description', description)
if (errorDescriptions) brokenSiteParams.set('errorDescriptions', errorDescriptions)
if (httpErrorCodes) brokenSiteParams.set('httpErrorCodes', httpErrorCodes)
if (openerContext) brokenSiteParams.set('openerContext', openerContext)

return fire(brokenSiteParams.toString())
}
9 changes: 9 additions & 0 deletions shared/js/background/classes/tab-state.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { getFromSessionStorage, setToSessionStorage, removeFromSessionStorage } from '../wrapper'
import { getUserLocale } from '../i18n'
import { Tracker } from './tracker'
import { AdClick } from './ad-click-attribution-policy'

Expand Down Expand Up @@ -58,6 +59,14 @@ export class TabState {
this.httpErrorCodes = []
/** @type {boolean} */
this.performanceWarning = false // True when the runtime.onPerformanceWarning event fired for the tab.
/** @type {number} */
this.userRefreshCount = 0
/** @type {string | null} */
this.openerContext = null
/** @type {number[]} */
this.jsPerformance = []
/** @type {string} */
this.locale = getUserLocale()
// Whilst restoring, prevent the tab data being stored
if (!restoring) {
Storage.backup(this)
Expand Down
33 changes: 33 additions & 0 deletions shared/js/background/classes/tab.js
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,38 @@ class Tab {
this._tabState.setValue('performanceWarning', value)
}

get userRefreshCount () {
return this._tabState.userRefreshCount
}

set userRefreshCount (value) {
this._tabState.setValue('userRefreshCount', value)
}

get openerContext () {
return this._tabState.openerContext
}

set openerContext (value) {
this._tabState.setValue('openerContext', value)
}

get jsPerformance () {
return this._tabState.jsPerformance
}

set jsPerformance (value) {
this._tabState.setValue('jsPerformance', value)
}

get locale () {
return this._tabState.locale
}

set locale (value) {
this._tabState.setValue('locale', value)
}

/**
* If given a valid adClick redirect, set the adClick to the tab.
* @param {string} requestURL
Expand Down Expand Up @@ -286,6 +318,7 @@ class Tab {

this.url = url
this.site = new Site(url, this._tabState)
this.userRefreshCount = 0
}

// Store all trackers for a given tab even if we don't block them.
Expand Down
9 changes: 9 additions & 0 deletions shared/js/background/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,15 @@ browser.runtime.onMessage.addListener((req, sender) => {
return Promise.resolve(messageHandlers[req.messageType](req.options, sender, req))
}

// Count refreshes per page
if (req.pageReloaded && (sender.tab !== undefined)) {
const tab = tabManager.get({ tabId: sender.tab.id })
if (tab) {
tab.userRefreshCount += 1
}
return
}

// TODO clean up legacy onboarding messaging
if (browserName === 'chrome') {
if (req === 'healthCheckRequest' || req === 'rescheduleCounterMessagingRequest') {
Expand Down
5 changes: 4 additions & 1 deletion shared/js/background/message-handlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,13 @@ export async function submitBrokenSiteReport (breakageReport) {
console.error('cannot access current tab with ID ' + currentTab.id)
return
}

const pageParams = await browser.tabs.sendMessage(currentTab.id, { getBreakagePageParams: true }) || {}

const tds = settings.getSetting('tds-etag')
const remoteConfigEtag = settings.getSetting('config-etag')
const remoteConfigVersion = tdsStorage.config.version
return breakageReportForTab({ tab, tds, remoteConfigEtag, remoteConfigVersion, category, description })
return breakageReportForTab({ tab, tds, remoteConfigEtag, remoteConfigVersion, category, description, pageParams })
}

/**
Expand Down
2 changes: 1 addition & 1 deletion shared/js/background/tab-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const {
// These tab properties are preserved when a new tab Object replaces an existing
// one for the same tab ID.
const persistentTabProperties = [
'ampUrl', 'cleanAmpUrl', 'dnrRuleIdsByDisabledClickToLoadRuleAction'
'ampUrl', 'cleanAmpUrl', 'dnrRuleIdsByDisabledClickToLoadRuleAction', 'userRefreshCount'
]

class TabManager {
Expand Down
39 changes: 39 additions & 0 deletions shared/js/content-scripts/breakage-stats.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
let pageReloaded = false
let jsPerformance = []

function notifyPageReloaded () {
(async () => {
await chrome.runtime.sendMessage({ pageReloaded: true })
})()
}

new PerformanceObserver((entryList) => {
for (const entry of entryList.getEntriesByName('first-contentful-paint')) {
jsPerformance = [entry.startTime]
}
}).observe({ type: 'paint', buffered: true })

document.addEventListener('DOMContentLoaded', function (event) {
pageReloaded = (
(window.performance.navigation && window.performance.navigation.type === 1) ||
window.performance
.getEntriesByType('navigation')
.map((nav) => nav.type)
.includes('reload')
)
if (pageReloaded) {
notifyPageReloaded()
}
})

chrome.runtime.onMessage.addListener(
function (req, sender, sendResponse) {
if (!req.getBreakagePageParams) return

sendResponse({
jsPerformance,
docReferrer: document.referrer,
opener: !!window.opener
})
}
)
3 changes: 2 additions & 1 deletion shared/js/content-scripts/content-scope-messaging.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ const allowedMessages = [
'setYoutubePreviewsEnabled',
'unblockClickToLoadContent',
'updateYouTubeCTLAddedFlag',
'updateFacebookCTLBreakageFlags'
'updateFacebookCTLBreakageFlags',
'pageReloaded'
]

function getSecret () {
Expand Down
6 changes: 5 additions & 1 deletion unit-test/background/classes/tab.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,11 @@ describe('Tab', () => {
debugFlags: [],
errorDescriptions: [],
httpErrorCodes: [],
performanceWarning: false
performanceWarning: false,
userRefreshCount: 0,
openerContext: null,
jsPerformance: [],
locale: 'en'
}
expect(tabClone.site.enabledFeatures.length).toBe(14)
expect(JSON.stringify(tabClone, null, 4)).toEqual(JSON.stringify(tabSnapshot, null, 4))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,20 @@ async function submitAndValidateReport (report) {
addActionRequests(report.adAttributionRequests, 'ad-attribution')
addActionRequests(report.noActionRequests, 'none')

const mockedPageParams = {
userRefreshCount: 2,
jsPerformance: [123.45],
docReferrer: 'http://example.com'
}

await breakageReportForTab({
tab,
tds: report.blocklistVersion,
remoteConfigEtag: report.remoteConfigEtag,
remoteConfigVersion: report.remoteConfigVersion,
category: report.category,
description: report.providedDescription
description: report.providedDescription,
pageParams: mockedPageParams
})
expect(loadPixelSpy.calls.count()).withContext('Expect only one pixel').toEqual(1)

Expand Down Expand Up @@ -153,7 +160,8 @@ describe('Broken Site Reporting tests / protections state', () => {
remoteConfigEtag: 'abd142',
remoteConfigVersion: '1234',
category: 'content',
description: 'test'
description: 'test',
pageParams: {}
})
const requestURLString = loadPixelSpy.calls.argsFor(0)[0]
return new URL(requestURLString).searchParams
Expand Down

0 comments on commit a58a481

Please sign in to comment.