From d09de4764673f3541a9fa95c1ff7fd2135c7425e Mon Sep 17 00:00:00 2001 From: groovecoder Date: Mon, 3 Apr 2017 14:38:39 -0500 Subject: [PATCH 1/3] for #306: Telemetry for container assignment events --- docs/metrics.md | 39 +++++++++++++++++++++++++++++++++ package.json | 2 +- webextension/background.js | 15 +++++++++++++ webextension/js/confirm-page.js | 4 ++++ webextension/manifest.json | 2 +- 5 files changed, 60 insertions(+), 2 deletions(-) diff --git a/docs/metrics.md b/docs/metrics.md index 4a13647..648af7f 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -189,6 +189,45 @@ of a `testpilottest` telemetry ping for each scenario. } ``` +* The user chooses "Always Open in this Container" context menu option. (Note: We send two separate event names: one for assigning a site to a container, one for removing a site from a container.) + +```js + { + "uuid": , + "userContextId": , + "event": "[added|removed]-container-assignment" + } +``` + +* Firefox prompts the user to reload a site into a container after the user picked "Always Open in this Container". + +```js + { + "uuid": , + "userContextId": , + "event": "prompt-reload-page-in-container" + } +``` + +* The user clicks "Take me there" to reload a site into a container after the user picked "Always Open in this Container". + +```js + { + "uuid": , + "event": "click-to-reload-page-in-container" + } +``` + +* Firefox automatically reloads a site into a container after the user picked "Always Open in this Container". + +```js + { + "uuid": , + "userContextId": , + "event": "auto-reload-page-in-container" + } +``` + ### A Redshift schema for the payload: ```lua diff --git a/package.json b/package.json index e34dd94..106488e 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "testpilot-containers", "title": "Containers Experiment", "description": "Containers works by isolating cookie jars using separate origin-attributes defined visually by colored ‘Container Tabs’. This add-on is a modified version of the containers feature for Firefox Test Pilot.", - "version": "2.1.0", + "version": "2.1.1", "author": "Andrea Marchesini, Luke Crouch and Jonathan Kingston", "bugs": { "url": "https://github.com/mozilla/testpilot-containers/issues" diff --git a/webextension/background.js b/webextension/background.js index c6dc283..bb70dac 100644 --- a/webextension/background.js +++ b/webextension/background.js @@ -115,6 +115,11 @@ const assignManager = { message: `Successfully ${actionName} site to always open in this container`, iconUrl: browser.extension.getURL("/img/onboarding-1.png") }); + browser.runtime.sendMessage({ + method: "sendTelemetryPayload", + event: `${actionName}-container-assignment`, + userContextId: userContextId, + }); this.calculateContextMenu(tab); }).catch((e) => { throw e; @@ -227,7 +232,17 @@ const assignManager = { // If the user has explicitly checked "Never Ask Again" on the warning page we will send them straight there if (neverAsk) { browser.tabs.create({url, cookieStoreId: `firefox-container-${userContextId}`, index}); + browser.runtime.sendMessage({ + method: "sendTelemetryPayload", + event: "auto-reload-page-in-container", + userContextId: userContextId, + }); } else { + browser.runtime.sendMessage({ + method: "sendTelemetryPayload", + event: "prompt-to-reload-page-in-container", + userContextId: userContextId, + }); const confirmUrl = `${loadPage}?url=${url}`; browser.tabs.create({url: confirmUrl, cookieStoreId: `firefox-container-${userContextId}`, index}).then(() => { // We don't want to sync this URL ever nor clutter the users history diff --git a/webextension/js/confirm-page.js b/webextension/js/confirm-page.js index 19a64f9..feefba1 100644 --- a/webextension/js/confirm-page.js +++ b/webextension/js/confirm-page.js @@ -17,6 +17,10 @@ document.getElementById("redirect-form").addEventListener("submit", (e) => { // Can't really do much here user will have to click it again }); } + browser.runtime.sendMessage({ + method: "sendTelemetryPayload", + event: "click-to-reload-page-in-container", + }); redirect(); }); diff --git a/webextension/manifest.json b/webextension/manifest.json index 675b179..a3f4c61 100644 --- a/webextension/manifest.json +++ b/webextension/manifest.json @@ -1,7 +1,7 @@ { "manifest_version": 2, "name": "Containers Experiment", - "version": "2.1.0", + "version": "2.1.1", "description": "Containers works by isolating cookie jars using separate origin-attributes defined visually by colored ‘Container Tabs’. This add-on is a modified version of the containers feature for Firefox Test Pilot.", "icons": { From 5a996c1dea0ec16824982f2a26a90eae44f810ce Mon Sep 17 00:00:00 2001 From: Jonathan Kingston Date: Mon, 3 Apr 2017 22:59:32 +0100 Subject: [PATCH 2/3] Moving code into messageHandler to share assignManager and tabPageCounter code. Fixes bind functions not working correctly. --- webextension/background.js | 106 +++++++++++++++++++++---------------- 1 file changed, 60 insertions(+), 46 deletions(-) diff --git a/webextension/background.js b/webextension/background.js index bb70dac..21d07f6 100644 --- a/webextension/background.js +++ b/webextension/background.js @@ -59,24 +59,6 @@ const assignManager = { }, init() { - browser.tabs.onActivated.addListener((info) => { - browser.tabs.get(info.tabId).then((tab) => { - this.calculateContextMenu(tab); - }).catch((e) => { - throw e; - }); - }); - - browser.windows.onFocusChanged.addListener((windowId) => { - browser.tabs.query({active: true, windowId}).then((tabs) => { - if (tabs && tabs[0]) { - this.calculateContextMenu(tabs[0]); - } - }).catch((e) => { - throw e; - }); - }); - browser.runtime.onMessage.addListener((neverAskMessage) => { const pageUrl = neverAskMessage.pageUrl; if (neverAskMessage.neverAsk === true) { @@ -127,6 +109,7 @@ const assignManager = { } }); + // Before a request is handled by the browser we decide if we should route through a different container browser.webRequest.onBeforeRequest.addListener((options) => { if (options.frameId !== 0 || options.tabId === -1) { return {}; @@ -155,17 +138,6 @@ const assignManager = { throw e; }); },{urls: [""], types: ["main_frame"]}, ["blocking"]); - - browser.webRequest.onCompleted.addListener((options) => { - if (options.frameId !== 0 || options.tabId === -1) { - return {}; - } - browser.tabs.get(options.tabId).then((tab) => { - this.calculateContextMenu(tab); - }).catch((e) => { - throw e; - }); - },{urls: [""], types: ["main_frame"]}); }, @@ -256,6 +228,7 @@ const assignManager = { const messageHandler = { init() { + // Handles messages from index.js const port = browser.runtime.connect(); port.onMessage.addListener(m => { switch (m.type) { @@ -269,6 +242,55 @@ const messageHandler = { throw new Error(`Unhandled message type: ${m.message}`); } }); + + browser.tabs.onCreated.addListener((tab, other) => { + // This works at capturing the tabs as they are created + // However we need onFocusChanged and onActivated to capture the initial tab + if (tab.id === -1) { + return {}; + } + tabPageCounter.initTabCounter(tab); + }); + + browser.tabs.onRemoved.addListener((tabId, other) => { + if (tabId === -1) { + return {}; + } + tabPageCounter.sendTabCountAndDelete(tabId); + }); + + browser.tabs.onActivated.addListener((info) => { + browser.tabs.get(info.tabId).then((tab) => { + tabPageCounter.initTabCounter(tab); + assignManager.calculateContextMenu(tab); + }).catch((e) => { + throw e; + }); + }); + + browser.windows.onFocusChanged.addListener((windowId) => { + browser.tabs.query({active: true, windowId}).then((tabs) => { + if (tabs && tabs[0]) { + tabPageCounter.initTabCounter(tabs[0]); + assignManager.calculateContextMenu(tabs[0]); + } + }).catch((e) => { + throw e; + }); + }); + + browser.webRequest.onCompleted.addListener((details) => { + if (details.frameId !== 0 || details.tabId === -1) { + return {}; + } + + browser.tabs.get(details.tabId).then((tab) => { + tabPageCounter.incrementTabCount(tab); + assignManager.calculateContextMenu(tab); + }).catch((e) => { + throw e; + }); + }, {urls: [""], types: ["main_frame"]}); } }; @@ -312,41 +334,33 @@ const themeManager = { const tabPageCounter = { counter: {}, - init() { - browser.tabs.onCreated.addListener(this.initTabCounter.bind(this)); - browser.tabs.onRemoved.addListener(this.sendTabCountAndDelete.bind(this)); - browser.webRequest.onCompleted.addListener(this.incrementTabCount.bind(this), {urls: [""], types: ["main_frame"]}); - }, - initTabCounter(tab) { + if (tab.id in this.counter) { + return; + } this.counter[tab.id] = { "cookieStoreId": tab.cookieStoreId, "pageRequests": 0 }; }, - sendTabCountAndDelete(tab) { + sendTabCountAndDelete(tabId) { browser.runtime.sendMessage({ method: "sendTelemetryPayload", event: "page-requests-completed-per-tab", - userContextId: this.counter[tab].cookieStoreId, - pageRequestCount: this.counter[tab].pageRequests + userContextId: this.counter[tabId].cookieStoreId, + pageRequestCount: this.counter[tabId].pageRequests }); - delete this.counter[tab.id]; + delete this.counter[tabId]; }, - incrementTabCount(details) { - browser.tabs.get(details.tabId).then(tab => { - this.counter[tab.id].pageRequests++; - }).catch(e => { - throw e; - }); + incrementTabCount(tab) { + this.counter[tab.id].pageRequests++; } }; assignManager.init(); themeManager.init(); -tabPageCounter.init(); // Lets do this last as theme manager did a check before connecting before messageHandler.init(); From 8a0f9a99cd80d66ac2482149cbf1f3d484ce4d3f Mon Sep 17 00:00:00 2001 From: groovecoder Date: Tue, 4 Apr 2017 07:53:24 -0500 Subject: [PATCH 3/3] eslint fixes --- webextension/background.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/webextension/background.js b/webextension/background.js index 21d07f6..57e7c6a 100644 --- a/webextension/background.js +++ b/webextension/background.js @@ -243,7 +243,7 @@ const messageHandler = { } }); - browser.tabs.onCreated.addListener((tab, other) => { + browser.tabs.onCreated.addListener((tab) => { // This works at capturing the tabs as they are created // However we need onFocusChanged and onActivated to capture the initial tab if (tab.id === -1) { @@ -252,7 +252,7 @@ const messageHandler = { tabPageCounter.initTabCounter(tab); }); - browser.tabs.onRemoved.addListener((tabId, other) => { + browser.tabs.onRemoved.addListener((tabId) => { if (tabId === -1) { return {}; }