Skip to content

Commit

Permalink
Fix stats leakage (#58)
Browse files Browse the repository at this point in the history
* Testing setup

* Remove dead code

* Page store: prevent Firefox bug breaking previous page chain

* Update packages/reporting/src/webrequest-pipeline/page-store.js

Co-authored-by: Philipp Claßen <philipp.classen@posteo.de>

---------

Co-authored-by: Philipp Claßen <philipp.classen@posteo.de>
  • Loading branch information
chrmod and philipp-classen authored Oct 10, 2023
1 parent c7449af commit 3b68b58
Show file tree
Hide file tree
Showing 9 changed files with 90 additions and 21 deletions.
14 changes: 13 additions & 1 deletion packages/reporting/example/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@ import {
} from '../src/index.js';
import rules from './rules.json';

(chrome.action || chrome.browserAction).onClicked.addListener(() => {
chrome.tabs.create({
active: true,
url: chrome.runtime.getURL('inspector/index.html'),
});
});

setLogLevel('debug');

const storage = {
Expand Down Expand Up @@ -71,14 +78,18 @@ const requestReporter = new RequestReporter(config.request, {
getBrowserInfo: () => ({ name: 'xx' }),
});

chrome.runtime.onMessage.addListener((request, sender) => {
chrome.runtime.onMessage.addListener((request, sender, sendResponse) => {
if (request.action === 'mousedown') {
requestReporter.recordClick(
request.event,
request.context,
request.href,
sender,
);
} else if (request.action === 'debug') {
sendResponse({
tabs: [...webRequestPipeline.pageStore.tabs._inMemoryMap.values()],
});
}
});

Expand All @@ -91,5 +102,6 @@ chrome.runtime.onMessage.addListener((request, sender) => {
await requestReporter.init();
})();

globalThis.webRequestPipeline = webRequestPipeline;
globalThis.urlReporter = urlReporter;
globalThis.requestReporter = requestReporter;
17 changes: 17 additions & 0 deletions packages/reporting/example/inspector/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>wtm/reporting inspector</title>
<script type="module" src="./index.js"></script>
</head>
<body>
<h1>WTM Reporting Inspector</h1>
<label for="filter">Filter:</label><input id="filter" type="text"/>
<section>
<h2>Page Store</h2>
<pre id="page-store"></pre>
</section>

</body>
</html>
27 changes: 27 additions & 0 deletions packages/reporting/example/inspector/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
const browser = globalThis.browser || globalThis.chrome;
const $container = document.querySelector('#page-store');
const $filter = document.querySelector('#filter');

$filter.value = localStorage.filter || '';
$filter.addEventListener('input', () => {
localStorage.filter = $filter.value;
});

async function render() {
let { tabs } = await browser.runtime.sendMessage({ action: 'debug' });

if (localStorage.filter) {
const matchById = tabs.find(
(tab) => tab.id === Number(localStorage.filter),
);

tabs = matchById
? [matchById]
: tabs.filter((tab) => tab.url.includes(localStorage.filter));
}
$container.innerHTML = JSON.stringify(tabs, null, 2);
}

await render();

setInterval(render, 500);
3 changes: 3 additions & 0 deletions packages/reporting/example/manifests/chromium.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
"storage",
"windows"
],
"action": {
"default_title": "WTM/Reporting Inspector"
},
"background": {
"service_worker": "index.bundle.js",
"type": "module"
Expand Down
4 changes: 4 additions & 0 deletions packages/reporting/example/manifests/firefox.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@
"ws://*/*",
"wss://*/*"
],
"browser_action": {
"default_title": "WTM/Reporting Inspector",
"default_area": "navbar"
},
"content_scripts": [
{
"matches": ["http://*/*", "https://*/*"],
Expand Down
10 changes: 10 additions & 0 deletions packages/reporting/example/tests/test1.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>wtm/reporting test1</title>
</head>
<body>
<a href="https://ghostery.com" id="a">go to ghostery.com (forces process switch on Firefox)</a>
</body>
</html>
2 changes: 0 additions & 2 deletions packages/reporting/src/request/page-telemetry.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,5 @@ export default function buildPageLoadObject(page) {
tsv: page.tsv,
tsv_id: page.tsvId !== undefined,
frames: {},
cmp: page.annotations.cmp,
hiddenElements: page.annotations.hiddenElements,
};
}
29 changes: 14 additions & 15 deletions packages/reporting/src/webrequest-pipeline/page-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@ export default class PageStore {
chrome.tabs.onActivated.addListener(this.onTabActivated);
chrome.webNavigation.onBeforeNavigate.addListener(this.onBeforeNavigate);
chrome.webNavigation.onCommitted.addListener(this.onNavigationCommitted);
chrome.webNavigation.onDOMContentLoaded.addListener(
this.onNavigationLoaded,
);
chrome.webNavigation.onCompleted.addListener(this.onNavigationComplete);
if (chrome.windows && chrome.windows.onFocusChanged) {
chrome.windows.onFocusChanged.addListener(this.onWindowFocusChanged);
Expand All @@ -62,9 +59,6 @@ export default class PageStore {
chrome.tabs.onActivated.removeListener(this.onTabActivated);
chrome.webNavigation.onBeforeNavigate.removeListener(this.onBeforeNavigate);
chrome.webNavigation.onCommitted.removeListener(this.onNavigationCommitted);
chrome.webNavigation.onDOMContentLoaded.removeListener(
this.onNavigationLoaded,
);
chrome.webNavigation.onCompleted.removeListener(this.onNavigationComplete);
if (chrome.windows && chrome.windows.onFocusChanged) {
chrome.windows.onFocusChanged.removeListener(this.onWindowFocusChanged);
Expand Down Expand Up @@ -147,9 +141,18 @@ export default class PageStore {
};

onBeforeNavigate = (details) => {
const { frameId, tabId, url } = details;
const { frameId, tabId, url, timeStamp } = details;
const tabContext = this.tabs.get(tabId);
if (frameId === 0) {
// ignore duplicated onBeforeNavigate https://bugzilla.mozilla.org/show_bug.cgi?id=1732564
if (
tabContext &&
tabContext.id === tabId &&
tabContext.url === url &&
tabContext.created + 200 > timeStamp
) {
return;
}
// We are starting a navigation to a new page - if the previous page is complete (i.e. fully
// loaded), stage it before we create the new page info.
if (tabContext && tabContext.state === PAGE_LOADING_STATE.COMPLETE) {
Expand All @@ -162,6 +165,7 @@ export default class PageStore {
active: false,
url,
incognito: tabContext ? tabContext.isPrivate : false,
created: timeStamp,
});
nextContext.previous = tabContext;
this.tabs.set(tabId, nextContext);
Expand All @@ -180,13 +184,6 @@ export default class PageStore {
}
};

onNavigationLoaded = ({ frameId, tabId }) => {
const tabContext = this.tabs.get(tabId);
if (frameId === 0 && tabContext) {
tabContext.updateState(PAGE_LOADING_STATE.LOADED);
}
};

onNavigationComplete = ({ frameId, tabId }) => {
const tabContext = this.tabs.get(tabId);
if (frameId === 0 && tabContext) {
Expand Down Expand Up @@ -244,7 +241,8 @@ export default class PageStore {
};
};

getPageForRequest({ tabId, frameId, originUrl, type, initiator }) {
getPageForRequest(context) {
const { tabId, frameId, originUrl, type, initiator } = context;
const tab = this.tabs.get(tabId);
if (!tab) {
return null;
Expand All @@ -260,6 +258,7 @@ export default class PageStore {

const couldBePreviousPage =
frameId === 0 && type !== 'main_frame' && tab.previous;

// for main frame requests: check if the origin url is from the previous page (Firefox)
if (
couldBePreviousPage &&
Expand Down
5 changes: 2 additions & 3 deletions packages/reporting/src/webrequest-pipeline/page.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,17 @@ export const PAGE_LOADING_STATE = {
CREATED: 'created',
NAVIGATING: 'navigating',
COMMITTED: 'committed',
LOADED: 'loaded',
COMPLETE: 'complete',
};

export default class Page {
constructor({ id, active, url, incognito }) {
constructor({ id, active, url, incognito, created }) {
this.id = id || 0;
this.url = url;
this.isRedirect = false;
this.isPrivate = incognito;
this.isPrivateServer = false;
this.created = Date.now();
this.created = created || Date.now();
this.destroyed = null;
this.lastRequestId = null;
this.frames = {
Expand Down

0 comments on commit 3b68b58

Please sign in to comment.