From 4ecd3d03beab4ae2e44f0bcd10778c7034d08574 Mon Sep 17 00:00:00 2001 From: ilyazarembsky Date: Wed, 17 Jul 2019 14:58:10 -0400 Subject: [PATCH 01/14] Update broken ping reload threshhold to 60 seconds --- src/classes/PolicySmartBlock.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/classes/PolicySmartBlock.js b/src/classes/PolicySmartBlock.js index 831dbbd1e..c370f64e4 100644 --- a/src/classes/PolicySmartBlock.js +++ b/src/classes/PolicySmartBlock.js @@ -251,7 +251,7 @@ class PolicySmartBlock { checkReloadThreshold(tabId) { if (!this.shouldCheck(tabId)) { return false; } - const THRESHHOLD = 30000; // 30 seconds + const THRESHHOLD = 60000; // 60 seconds return ( tabInfo.getTabInfoPersist(tabId, 'numOfReloads') > 1 && From 1ed578e1f4912b1530d93ea26d30d11ca716eb5a Mon Sep 17 00:00:00 2001 From: ilyazarembsky Date: Mon, 22 Jul 2019 11:23:59 -0400 Subject: [PATCH 02/14] Add startPossibleBrokenPageTimer method to Metrics, implement the delayed off switch for the watcher, and set it to trigger on site whitelist --- src/background.js | 1 + src/classes/Metrics.js | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/src/background.js b/src/background.js index 423a41ba0..855617945 100644 --- a/src/background.js +++ b/src/background.js @@ -1101,6 +1101,7 @@ function initializeDispatcher() { button.update(); utils.flushChromeMemoryCache(); cliqz.modules.core.action('refreshAppState'); + metrics.startPossibleBrokenPageTimer(); }); dispatcher.on('conf.save.enable_human_web', (enableHumanWeb) => { if (!IS_CLIQZ) { diff --git a/src/classes/Metrics.js b/src/classes/Metrics.js index 0e13db90f..947b31ed0 100644 --- a/src/classes/Metrics.js +++ b/src/classes/Metrics.js @@ -30,6 +30,7 @@ const FIRST_REWARD_METRICS = ['rewards_first_accept', 'rewards_first_reject', 'r const { METRICS_SUB_DOMAIN, EXTENSION_VERSION, BROWSER_INFO } = globals; const IS_EDGE = (BROWSER_INFO.name === 'edge'); const MAX_DELAYED_PINGS = 100; +const BROKEN_PAGE_WATCH_THRESHOLD = 60000; // 60 seconds /** * Class for handling telemetry pings. * @memberOf BackgroundClasses @@ -39,6 +40,9 @@ class Metrics { this.utm_source = ''; this.utm_campaign = ''; this.ping_set = new Set(); + this._pageMightBeBroken = false; + this._pageMightBeBrokenOffSwitch = null; + this._brokenPageOnNewTabListener = null; } /** @@ -107,6 +111,21 @@ class Metrics { }); } + startPossibleBrokenPageTimer() { + this._pageMightBeBroken = true; + + // Stand down after a minute + if (this._pageMightBeBrokenOffSwitch) { + clearTimeout(this._pageMightBeBrokenOffSwitch); + } + this._pageMightBeBrokenOffSwitch = setTimeout(() => { + this._pageMightBeBroken = false; + }, + BROKEN_PAGE_WATCH_THRESHOLD + ); + + } + /** * Prepare data and send telemetry pings. * @param {string} type type of the telemetry ping From 99305fc2c442b566cc1411126ba4360a9e54363f Mon Sep 17 00:00:00 2001 From: ilyazarembsky Date: Mon, 22 Jul 2019 13:11:53 -0400 Subject: [PATCH 03/14] Implement metrics broken page trigger handler that uses existing event listeners instead of relying on new ones --- src/background.js | 2 +- src/classes/EventHandlers.js | 1 + src/classes/Globals.js | 4 +++ src/classes/Metrics.js | 48 +++++++++++++++++++++++++----------- 4 files changed, 40 insertions(+), 15 deletions(-) diff --git a/src/background.js b/src/background.js index 855617945..ad4e99cca 100644 --- a/src/background.js +++ b/src/background.js @@ -1101,7 +1101,7 @@ function initializeDispatcher() { button.update(); utils.flushChromeMemoryCache(); cliqz.modules.core.action('refreshAppState'); - metrics.startPossibleBrokenPageTimer(); + metrics.handleBrokenPageTrigger(globals.BROKEN_PAGE_WHITELISTED); }); dispatcher.on('conf.save.enable_human_web', (enableHumanWeb) => { if (!IS_CLIQZ) { diff --git a/src/classes/EventHandlers.js b/src/classes/EventHandlers.js index 9173db3f5..f96f917d4 100644 --- a/src/classes/EventHandlers.js +++ b/src/classes/EventHandlers.js @@ -110,6 +110,7 @@ class EventHandlers { if (frameId === 0) { // update reload info before creating/clearing tab info if (transitionType === 'reload' && !transitionQualifiers.includes('forward_back')) { + metrics.handleBrokenPageTrigger(globals.BROKEN_PAGE_REFRESH); tabInfo.setTabInfo(tabId, 'numOfReloads', tabInfo.getTabInfo(tabId, 'numOfReloads') + 1); } else if (transitionType !== 'auto_subframe' && transitionType !== 'manual_subframe') { tabInfo.setTabInfo(tabId, 'reloaded', false); diff --git a/src/classes/Globals.js b/src/classes/Globals.js index 023d7069c..5df20fb1a 100644 --- a/src/classes/Globals.js +++ b/src/classes/Globals.js @@ -70,6 +70,10 @@ class Globals { this.BLACKLISTED = 1; this.WHITELISTED = 2; + // Broken page metrics named constants + this.BROKEN_PAGE_REFRESH = 1; + this.BROKEN_PAGE_WHITELISTED = 2; + // data stores this.REDIRECT_MAP = new Map(); this.BLOCKED_REDIRECT_DATA = {}; diff --git a/src/classes/Metrics.js b/src/classes/Metrics.js index 947b31ed0..b3f733b75 100644 --- a/src/classes/Metrics.js +++ b/src/classes/Metrics.js @@ -40,9 +40,11 @@ class Metrics { this.utm_source = ''; this.utm_campaign = ''; this.ping_set = new Set(); - this._pageMightBeBroken = false; - this._pageMightBeBrokenOffSwitch = null; - this._brokenPageOnNewTabListener = null; + this._brokenPageWatcher = { + flag: false, + triggerId: null, + triggerTime: null, + }; } /** @@ -111,19 +113,37 @@ class Metrics { }); } - startPossibleBrokenPageTimer() { - this._pageMightBeBroken = true; - - // Stand down after a minute - if (this._pageMightBeBrokenOffSwitch) { - clearTimeout(this._pageMightBeBrokenOffSwitch); + handleBrokenPageTrigger(triggerId) { + switch (triggerId) { + case globals.BROKEN_PAGE_REFRESH: + if (this._brokenPageWatcher.flag) { + this.ping('broken-page'); + this._clearBrokenPageWatcher(); + } else { + this._setBrokenPageWatcher(triggerId); + } + break; + default: + break; } - this._pageMightBeBrokenOffSwitch = setTimeout(() => { - this._pageMightBeBroken = false; - }, - BROKEN_PAGE_WATCH_THRESHOLD - ); + } + _clearBrokenPageWatcher() { + this._brokenPageWatcher = Object.assign({}, { + flag: false, + triggerId: null, + triggerTime: null, + watcherOffSwitch: null + }); + } + + _setBrokenPageWatcher(triggerId) { + this._brokenPageWatcher = Object.assign({}, { + flag: true, + triggerId, + triggerTime: Date.now(), + }); + setTimeout(this._clearBrokenPageWatcher, BROKEN_PAGE_WATCH_THRESHOLD); } /** From 52e9a7a0911772e4757dce281f1199771b6d16a4 Mon Sep 17 00:00:00 2001 From: ilyazarembsky Date: Mon, 22 Jul 2019 13:46:43 -0400 Subject: [PATCH 04/14] Add setup_pth and setup_block parameters to broken_page ping --- src/classes/Metrics.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/classes/Metrics.js b/src/classes/Metrics.js index b3f733b75..8998d4152 100644 --- a/src/classes/Metrics.js +++ b/src/classes/Metrics.js @@ -42,8 +42,8 @@ class Metrics { this.ping_set = new Set(); this._brokenPageWatcher = { flag: false, - triggerId: null, - triggerTime: null, + triggerId: '', + triggerTime: '', }; } @@ -131,9 +131,8 @@ class Metrics { _clearBrokenPageWatcher() { this._brokenPageWatcher = Object.assign({}, { flag: false, - triggerId: null, - triggerTime: null, - watcherOffSwitch: null + triggerId: '', + triggerTime: '', }); } @@ -361,6 +360,12 @@ class Metrics { metrics_url += // Reward ID `&rid=${encodeURIComponent(this._getRewardId().toString())}`; + } else if (type === 'broken_page' && this._brokenPageWatcher.flag) { + metrics_url += + // What triggered the broken page ping? + `&setup_path=${encodeURIComponent(this._brokenPageWatcher.triggerId.toString())}` + + // How much time passed between the trigger and the page refresh / open in new tab? + `&setup_block=${encodeURIComponent((Date.now() - this._brokenPageWatcher.triggerTime).toString())}`; } return metrics_url; From 311b49839f6d9baff2887fa8df72a80b78a3a1fa Mon Sep 17 00:00:00 2001 From: ilyazarembsky Date: Mon, 22 Jul 2019 14:52:53 -0400 Subject: [PATCH 05/14] Clear and reset broken page watcher setTimeout handler if a new trigger comes in before it fires --- src/classes/Metrics.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/classes/Metrics.js b/src/classes/Metrics.js index 8998d4152..07248d413 100644 --- a/src/classes/Metrics.js +++ b/src/classes/Metrics.js @@ -44,6 +44,7 @@ class Metrics { flag: false, triggerId: '', triggerTime: '', + timeoutId: null, }; } @@ -133,16 +134,21 @@ class Metrics { flag: false, triggerId: '', triggerTime: '', + timeoutId: null, }); } _setBrokenPageWatcher(triggerId) { + const { timeoutId } = this._brokenPageWatcher; + + if (timeoutId) { clearTimeout(timeoutId); } + this._brokenPageWatcher = Object.assign({}, { flag: true, triggerId, triggerTime: Date.now(), + timeoutId: setTimeout(this._clearBrokenPageWatcher, BROKEN_PAGE_WATCH_THRESHOLD), }); - setTimeout(this._clearBrokenPageWatcher, BROKEN_PAGE_WATCH_THRESHOLD); } /** From dd1d21505e3f1033570ae0d72f5b5f366979c2f3 Mon Sep 17 00:00:00 2001 From: ilyazarembsky Date: Mon, 22 Jul 2019 15:19:14 -0400 Subject: [PATCH 06/14] Add pause Ghostery trigger for Metrics broken page watcher --- src/background.js | 3 ++- src/classes/Globals.js | 3 ++- src/classes/Metrics.js | 35 ++++++++++++++++++++++++++--------- 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/background.js b/src/background.js index ad4e99cca..46bcadea5 100644 --- a/src/background.js +++ b/src/background.js @@ -1101,7 +1101,7 @@ function initializeDispatcher() { button.update(); utils.flushChromeMemoryCache(); cliqz.modules.core.action('refreshAppState'); - metrics.handleBrokenPageTrigger(globals.BROKEN_PAGE_WHITELISTED); + metrics.handleBrokenPageTrigger(globals.BROKEN_PAGE_WHITELIST); }); dispatcher.on('conf.save.enable_human_web', (enableHumanWeb) => { if (!IS_CLIQZ) { @@ -1157,6 +1157,7 @@ function initializeDispatcher() { dispatcher.on('globals.save.paused_blocking', () => { // update content script state when blocking is paused/unpaused + if (globals.SESSION.paused_blocking) { metrics.handleBrokenPageTrigger(globals.BROKEN_PAGE_PAUSE); } cliqz.modules.core.action('refreshAppState'); }); } diff --git a/src/classes/Globals.js b/src/classes/Globals.js index 5df20fb1a..2863ba9b8 100644 --- a/src/classes/Globals.js +++ b/src/classes/Globals.js @@ -72,7 +72,8 @@ class Globals { // Broken page metrics named constants this.BROKEN_PAGE_REFRESH = 1; - this.BROKEN_PAGE_WHITELISTED = 2; + this.BROKEN_PAGE_WHITELIST = 2; + this.BROKEN_PAGE_PAUSE = 3; // data stores this.REDIRECT_MAP = new Map(); diff --git a/src/classes/Metrics.js b/src/classes/Metrics.js index 07248d413..f4f5b7f35 100644 --- a/src/classes/Metrics.js +++ b/src/classes/Metrics.js @@ -41,7 +41,7 @@ class Metrics { this.utm_campaign = ''; this.ping_set = new Set(); this._brokenPageWatcher = { - flag: false, + on: false, triggerId: '', triggerTime: '', timeoutId: null, @@ -119,9 +119,21 @@ class Metrics { case globals.BROKEN_PAGE_REFRESH: if (this._brokenPageWatcher.flag) { this.ping('broken-page'); - this._clearBrokenPageWatcher(); + this._unplugBrokenPageWatcher(); } else { - this._setBrokenPageWatcher(triggerId); + this._rebootBrokenPageWatcher(triggerId); + } + break; + case globals.BROKEN_PAGE_WHITELIST: + if (this._brokenPageWatcher.flag) { + this.ping('broken-page'); + this._unplugBrokenPageWatcher(); + } + break; + case globals.BROKEN_PAGE_PAUSE: + if (this._brokenPageWatcher.flag) { + this.ping('broken-page'); + this._unplugBrokenPageWatcher(); } break; default: @@ -129,8 +141,10 @@ class Metrics { } } - _clearBrokenPageWatcher() { - this._brokenPageWatcher = Object.assign({}, { + _unplugBrokenPageWatcher() { + this._clearBrokenPageWatcherTimeout(); + + this._brokenPageWatcher = Object.assign({},{ flag: false, triggerId: '', triggerTime: '', @@ -138,10 +152,8 @@ class Metrics { }); } - _setBrokenPageWatcher(triggerId) { - const { timeoutId } = this._brokenPageWatcher; - - if (timeoutId) { clearTimeout(timeoutId); } + _rebootBrokenPageWatcher(triggerId) { + this._clearBrokenPageWatcherTimeout(); this._brokenPageWatcher = Object.assign({}, { flag: true, @@ -151,6 +163,11 @@ class Metrics { }); } + _clearBrokenPageWatcherTimeout() { + const { timeoutId } = this._brokenPageWatcher; + if (timeoutId) { clearTimeout(timeoutId); } + } + /** * Prepare data and send telemetry pings. * @param {string} type type of the telemetry ping From 49281a8618f9c9785b3c69c19deea2414e5a7e2f Mon Sep 17 00:00:00 2001 From: ilyazarembsky Date: Mon, 22 Jul 2019 15:54:42 -0400 Subject: [PATCH 07/14] Add local tracker unblock trigger for Metrics broken page watcher. --- src/background.js | 8 +++++++- src/classes/Globals.js | 1 + src/classes/Metrics.js | 35 +++++++++-------------------------- 3 files changed, 17 insertions(+), 27 deletions(-) diff --git a/src/background.js b/src/background.js index 46bcadea5..0c9799489 100644 --- a/src/background.js +++ b/src/background.js @@ -1096,11 +1096,16 @@ function initializeDispatcher() { // can't simply compare num_selected and size(db.apps) since apps get removed sometimes db.allSelected = (!!num_selected && every(db.apps, (app, app_id) => appIds.hasOwnProperty(app_id))); }); + dispatcher.on('conf.save.site_specific_unblocks', () => { + // if user has unblock a tracker on this page, suspect a broken page + metrics.handleBrokenPageTrigger(globals.BROKEN_PAGE_LOCAL_TRACKER_UNBLOCK); + }) dispatcher.on('conf.save.site_whitelist', () => { // TODO debounce with below button.update(); utils.flushChromeMemoryCache(); cliqz.modules.core.action('refreshAppState'); + // if user has whitelisted site, suspect broken page metrics.handleBrokenPageTrigger(globals.BROKEN_PAGE_WHITELIST); }); dispatcher.on('conf.save.enable_human_web', (enableHumanWeb) => { @@ -1156,8 +1161,9 @@ function initializeDispatcher() { }, 200)); dispatcher.on('globals.save.paused_blocking', () => { - // update content script state when blocking is paused/unpaused + // if user has paused Ghostery, suspect broken page if (globals.SESSION.paused_blocking) { metrics.handleBrokenPageTrigger(globals.BROKEN_PAGE_PAUSE); } + // update content script state when blocking is paused/unpaused cliqz.modules.core.action('refreshAppState'); }); } diff --git a/src/classes/Globals.js b/src/classes/Globals.js index 2863ba9b8..1ef3a8f50 100644 --- a/src/classes/Globals.js +++ b/src/classes/Globals.js @@ -74,6 +74,7 @@ class Globals { this.BROKEN_PAGE_REFRESH = 1; this.BROKEN_PAGE_WHITELIST = 2; this.BROKEN_PAGE_PAUSE = 3; + this.BROKEN_PAGE_LOCAL_TRACKER_UNBLOCK = 4; // data stores this.REDIRECT_MAP = new Map(); diff --git a/src/classes/Metrics.js b/src/classes/Metrics.js index f4f5b7f35..5b2120536 100644 --- a/src/classes/Metrics.js +++ b/src/classes/Metrics.js @@ -115,48 +115,31 @@ class Metrics { } handleBrokenPageTrigger(triggerId) { - switch (triggerId) { - case globals.BROKEN_PAGE_REFRESH: - if (this._brokenPageWatcher.flag) { - this.ping('broken-page'); - this._unplugBrokenPageWatcher(); - } else { - this._rebootBrokenPageWatcher(triggerId); - } - break; - case globals.BROKEN_PAGE_WHITELIST: - if (this._brokenPageWatcher.flag) { - this.ping('broken-page'); - this._unplugBrokenPageWatcher(); - } - break; - case globals.BROKEN_PAGE_PAUSE: - if (this._brokenPageWatcher.flag) { - this.ping('broken-page'); - this._unplugBrokenPageWatcher(); - } - break; - default: - break; + if (this._brokenPageWatcher.on && triggerId === globals.BROKEN_PAGE_REFRESH) { + this.ping('broken-page'); + this._unplugBrokenPageWatcher(); + return; } + + this._resetBrokenPageWatcher(triggerId); } _unplugBrokenPageWatcher() { this._clearBrokenPageWatcherTimeout(); this._brokenPageWatcher = Object.assign({},{ - flag: false, + on: false, triggerId: '', triggerTime: '', timeoutId: null, }); } - _rebootBrokenPageWatcher(triggerId) { + _resetBrokenPageWatcher(triggerId) { this._clearBrokenPageWatcherTimeout(); this._brokenPageWatcher = Object.assign({}, { - flag: true, + on: true, triggerId, triggerTime: Date.now(), timeoutId: setTimeout(this._clearBrokenPageWatcher, BROKEN_PAGE_WATCH_THRESHOLD), From 5e5d9968b7adba4971b950cd7808da0691db3f81 Mon Sep 17 00:00:00 2001 From: ilyazarembsky Date: Tue, 23 Jul 2019 09:59:26 -0400 Subject: [PATCH 08/14] Handle new tab creation for broken page watcher. Add documentation comments to broken page Metrics methods --- src/classes/EventHandlers.js | 7 ++++- src/classes/Globals.js | 1 + src/classes/Metrics.js | 59 +++++++++++++++++++++++++++--------- 3 files changed, 52 insertions(+), 15 deletions(-) diff --git a/src/classes/EventHandlers.js b/src/classes/EventHandlers.js index f96f917d4..1b8fccc4f 100644 --- a/src/classes/EventHandlers.js +++ b/src/classes/EventHandlers.js @@ -25,6 +25,7 @@ import conf from './Conf'; import foundBugs from './FoundBugs'; import globals from './Globals'; import latency from './Latency'; +import metrics from './Metrics'; import panelData from './PanelData'; import Policy, { BLOCK_REASON_SS_UNBLOCKED, BLOCK_REASON_C2P_ALLOWED_THROUGH } from './Policy'; import PolicySmartBlock from './PolicySmartBlock'; @@ -546,7 +547,11 @@ class EventHandlers { * * @param {Object} tab Details of the tab that was created */ - onTabCreated() {} + onTabCreated(tab) { + const { url } = tab; + + metrics.handleBrokenPageTrigger(globals.BROKEN_PAGE_NEW_TAB, url); + } /** * Handler for tabs.onActivated event. diff --git a/src/classes/Globals.js b/src/classes/Globals.js index 1ef3a8f50..16498e895 100644 --- a/src/classes/Globals.js +++ b/src/classes/Globals.js @@ -75,6 +75,7 @@ class Globals { this.BROKEN_PAGE_WHITELIST = 2; this.BROKEN_PAGE_PAUSE = 3; this.BROKEN_PAGE_LOCAL_TRACKER_UNBLOCK = 4; + this.BROKEN_PAGE_NEW_TAB = 86; // data stores this.REDIRECT_MAP = new Map(); diff --git a/src/classes/Metrics.js b/src/classes/Metrics.js index 5b2120536..5f01d7f43 100644 --- a/src/classes/Metrics.js +++ b/src/classes/Metrics.js @@ -14,7 +14,7 @@ import globals from './Globals'; import conf from './Conf'; import { log, prefsSet, prefsGet } from '../utils/common'; -import { processUrlQuery } from '../utils/utils'; +import { getActiveTab, processUrlQuery } from '../utils/utils'; import rewards from './Rewards'; // CONSTANTS @@ -45,6 +45,7 @@ class Metrics { triggerId: '', triggerTime: '', timeoutId: null, + url: '', }; } @@ -114,16 +115,56 @@ class Metrics { }); } - handleBrokenPageTrigger(triggerId) { + /** + * Responds to individual user actions and sequences of user actions that may indicate a broken page, + * sending broken_page pings as needed + * For example, sends a broken_page ping when the user whitelists a site, + * then refreshes the page less than a minute later + * @param {int} triggerId 'what specifically triggered this broken_page ping?' identifier sent along to the metrics server + * @param {string} newTabUrl for checking whether user has opened the same url in a new tab, which confirms a suspicion raised by certain triggers + */ + handleBrokenPageTrigger(triggerId, newTabUrl = null) { if (this._brokenPageWatcher.on && triggerId === globals.BROKEN_PAGE_REFRESH) { this.ping('broken-page'); this._unplugBrokenPageWatcher(); return; } + if (this._brokenPageWatcher.on && triggerId === globals.BROKEN_PAGE_NEW_TAB && this._brokenPageWatcher.url === newTabUrl) { + this.ping('broken-page'); + this._unplugBrokenPageWatcher(); + return; + } + + if (triggerId === globals.BROKEN_PAGE_NEW_TAB) { return; } + this._resetBrokenPageWatcher(triggerId); } + /** + * handleBrokenPageTrigger helper + * starts the temporary watch for a second suspicious user action in response to a first + * @param {int} triggerId 'what specifically triggered this broken_page ping?' identifier sent along to the metrics server + * @private + */ + _resetBrokenPageWatcher(triggerId) { + this._clearBrokenPageWatcherTimeout(); + getActiveTab((tab) => { + const tabUrl = tab && tab.url ? tab.url : ''; + + this._brokenPageWatcher = Object.assign({}, { + on: true, + triggerId, + triggerTime: Date.now(), + timeoutId: setTimeout(this._clearBrokenPageWatcher, BROKEN_PAGE_WATCH_THRESHOLD), + url: tabUrl, + }); + }); + } + /** + * handleBrokenPageTrigger helper + * @private + */ _unplugBrokenPageWatcher() { this._clearBrokenPageWatcherTimeout(); @@ -132,17 +173,7 @@ class Metrics { triggerId: '', triggerTime: '', timeoutId: null, - }); - } - - _resetBrokenPageWatcher(triggerId) { - this._clearBrokenPageWatcherTimeout(); - - this._brokenPageWatcher = Object.assign({}, { - on: true, - triggerId, - triggerTime: Date.now(), - timeoutId: setTimeout(this._clearBrokenPageWatcher, BROKEN_PAGE_WATCH_THRESHOLD), + url: '', }); } @@ -366,7 +397,7 @@ class Metrics { metrics_url += // Reward ID `&rid=${encodeURIComponent(this._getRewardId().toString())}`; - } else if (type === 'broken_page' && this._brokenPageWatcher.flag) { + } else if (type === 'broken_page' && this._brokenPageWatcher.on) { metrics_url += // What triggered the broken page ping? `&setup_path=${encodeURIComponent(this._brokenPageWatcher.triggerId.toString())}` + From f2c49ab4eb273c02f8fa8ca8c403634fbe1cc12f Mon Sep 17 00:00:00 2001 From: ilyazarembsky Date: Tue, 23 Jul 2019 10:31:57 -0400 Subject: [PATCH 09/14] Remove broken page metrics ping call from TabInfo#create --- src/classes/TabInfo.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/classes/TabInfo.js b/src/classes/TabInfo.js index c7a430383..e8818d733 100644 --- a/src/classes/TabInfo.js +++ b/src/classes/TabInfo.js @@ -69,11 +69,7 @@ class TabInfo { }, insecureRedirects: [], }; - - if (info.reloaded) { - metrics.ping('broken_page'); - } - + this._tabInfo[tab_id] = info; this._updateUrl(tab_id, tab_url); } From 0664aa54c7043216ef3670307f8e8a51c4c6513d Mon Sep 17 00:00:00 2001 From: ilyazarembsky Date: Tue, 23 Jul 2019 10:54:25 -0400 Subject: [PATCH 10/14] Revert smart unblock threshold to 30 seconds. Add comments explaining that the difference for thresholds for metrics and smart block is intentional. --- src/classes/Metrics.js | 9 +++++++-- src/classes/PolicySmartBlock.js | 6 ++++-- src/classes/TabInfo.js | 2 +- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/classes/Metrics.js b/src/classes/Metrics.js index 5f01d7f43..b90686147 100644 --- a/src/classes/Metrics.js +++ b/src/classes/Metrics.js @@ -30,7 +30,12 @@ const FIRST_REWARD_METRICS = ['rewards_first_accept', 'rewards_first_reject', 'r const { METRICS_SUB_DOMAIN, EXTENSION_VERSION, BROWSER_INFO } = globals; const IS_EDGE = (BROWSER_INFO.name === 'edge'); const MAX_DELAYED_PINGS = 100; -const BROKEN_PAGE_WATCH_THRESHOLD = 60000; // 60 seconds + +// Note that this threshold is intentionally different from the 30 second threshold in PolicySmartBlock, +// which is used to set the reloaded property on tab objects and to activate smart unblock behavior +// see GH-1797 for more details +const BROKEN_PAGE_METRICS_THRESHOLD = 60000; // 60 seconds + /** * Class for handling telemetry pings. * @memberOf BackgroundClasses @@ -156,7 +161,7 @@ class Metrics { on: true, triggerId, triggerTime: Date.now(), - timeoutId: setTimeout(this._clearBrokenPageWatcher, BROKEN_PAGE_WATCH_THRESHOLD), + timeoutId: setTimeout(this._clearBrokenPageWatcher, BROKEN_PAGE_METRICS_THRESHOLD), url: tabUrl, }); }); diff --git a/src/classes/PolicySmartBlock.js b/src/classes/PolicySmartBlock.js index c370f64e4..df09f1e79 100644 --- a/src/classes/PolicySmartBlock.js +++ b/src/classes/PolicySmartBlock.js @@ -251,11 +251,13 @@ class PolicySmartBlock { checkReloadThreshold(tabId) { if (!this.shouldCheck(tabId)) { return false; } - const THRESHHOLD = 60000; // 60 seconds + // Note that this threshold is different from the broken page ping threshold in Metrics, which is 60 seconds + // see GH-1797 for more details + const SMART_BLOCK_BEHAVIOR_THRESHOLD = 30000; // 30 seconds return ( tabInfo.getTabInfoPersist(tabId, 'numOfReloads') > 1 && - ((Date.now() - tabInfo.getTabInfoPersist(tabId, 'firstLoadTimestamp')) < THRESHHOLD) || false + ((Date.now() - tabInfo.getTabInfoPersist(tabId, 'firstLoadTimestamp')) < SMART_BLOCK_BEHAVIOR_THRESHOLD) || false ); } diff --git a/src/classes/TabInfo.js b/src/classes/TabInfo.js index e8818d733..7a28b025b 100644 --- a/src/classes/TabInfo.js +++ b/src/classes/TabInfo.js @@ -69,7 +69,7 @@ class TabInfo { }, insecureRedirects: [], }; - + this._tabInfo[tab_id] = info; this._updateUrl(tab_id, tab_url); } From 28da0dffd1b83f4592a35dd415755e0021b8fc6c Mon Sep 17 00:00:00 2001 From: ilyazarembsky Date: Tue, 23 Jul 2019 11:21:05 -0400 Subject: [PATCH 11/14] Trigger metrics broken page watcher on individual tracker site specific or global unblock as well as on site-specific allow --- app/panel/reducers/blocking.js | 1 + app/panel/utils/blocking.js | 5 ++++- src/background.js | 4 ---- src/classes/Globals.js | 2 +- src/classes/PanelData.js | 5 +++++ 5 files changed, 11 insertions(+), 6 deletions(-) diff --git a/app/panel/reducers/blocking.js b/app/panel/reducers/blocking.js index c0fb0e59a..2ec315ec4 100644 --- a/app/panel/reducers/blocking.js +++ b/app/panel/reducers/blocking.js @@ -144,6 +144,7 @@ const _updateTrackerTrustRestrict = (state, action) => { sendMessage('setPanelData', { site_specific_unblocks: updated_site_specific_unblocks, site_specific_blocks: updated_site_specific_blocks, + brokenPageMetricsTrackerTrustOrUnblock: msg.trust || (!msg.trust && !msg.restrict), }); return { diff --git a/app/panel/utils/blocking.js b/app/panel/utils/blocking.js index fb3683ad6..c797e87df 100644 --- a/app/panel/utils/blocking.js +++ b/app/panel/utils/blocking.js @@ -214,7 +214,10 @@ export function updateTrackerBlocked(state, action) { }); // persist to background - sendMessage('setPanelData', { selected_app_ids: updated_app_ids }); + sendMessage('setPanelData', { + selected_app_ids: updated_app_ids, + brokenPageMetricsTrackerTrustOrUnblock: !blocked + }); return { categories: updated_categories, diff --git a/src/background.js b/src/background.js index 0c9799489..e820f63c9 100644 --- a/src/background.js +++ b/src/background.js @@ -1096,10 +1096,6 @@ function initializeDispatcher() { // can't simply compare num_selected and size(db.apps) since apps get removed sometimes db.allSelected = (!!num_selected && every(db.apps, (app, app_id) => appIds.hasOwnProperty(app_id))); }); - dispatcher.on('conf.save.site_specific_unblocks', () => { - // if user has unblock a tracker on this page, suspect a broken page - metrics.handleBrokenPageTrigger(globals.BROKEN_PAGE_LOCAL_TRACKER_UNBLOCK); - }) dispatcher.on('conf.save.site_whitelist', () => { // TODO debounce with below button.update(); diff --git a/src/classes/Globals.js b/src/classes/Globals.js index 16498e895..bfdc99d51 100644 --- a/src/classes/Globals.js +++ b/src/classes/Globals.js @@ -74,7 +74,7 @@ class Globals { this.BROKEN_PAGE_REFRESH = 1; this.BROKEN_PAGE_WHITELIST = 2; this.BROKEN_PAGE_PAUSE = 3; - this.BROKEN_PAGE_LOCAL_TRACKER_UNBLOCK = 4; + this.BROKEN_PAGE_TRACKER_TRUST_OR_UNBLOCK = 4; this.BROKEN_PAGE_NEW_TAB = 86; // data stores diff --git a/src/classes/PanelData.js b/src/classes/PanelData.js index b84c929cb..eb020a4df 100644 --- a/src/classes/PanelData.js +++ b/src/classes/PanelData.js @@ -21,6 +21,7 @@ import conf from './Conf'; import foundBugs from './FoundBugs'; import bugDb from './BugDb'; import globals from './Globals'; +import metrics from './Metrics'; import Policy from './Policy'; import tabInfo from './TabInfo'; import rewards from './Rewards'; @@ -643,6 +644,10 @@ class PanelData { tabInfo.setTabInfo(this._activeTab.id, 'needsReload', data.needsReload); } + if (data.brokenPageMetricsTrackerTrustOrUnblock) { + metrics.handleBrokenPageTrigger(globals.BROKEN_PAGE_TRACKER_TRUST_OR_UNBLOCK); + } + if (syncSetDataChanged) { // Push conf changes to the server account.saveUserSettings().catch(err => log('PanelData saveUserSettings', err)); From e7cd1bc81c6e9f5514b6ec0afac718f57010f87a Mon Sep 17 00:00:00 2001 From: ilyazarembsky Date: Tue, 23 Jul 2019 11:37:27 -0400 Subject: [PATCH 12/14] Pacify linter --- src/classes/Metrics.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/classes/Metrics.js b/src/classes/Metrics.js index b90686147..b8f709d84 100644 --- a/src/classes/Metrics.js +++ b/src/classes/Metrics.js @@ -145,6 +145,7 @@ class Metrics { this._resetBrokenPageWatcher(triggerId); } + /** * handleBrokenPageTrigger helper * starts the temporary watch for a second suspicious user action in response to a first @@ -166,6 +167,7 @@ class Metrics { }); }); } + /** * handleBrokenPageTrigger helper * @private @@ -173,7 +175,7 @@ class Metrics { _unplugBrokenPageWatcher() { this._clearBrokenPageWatcherTimeout(); - this._brokenPageWatcher = Object.assign({},{ + this._brokenPageWatcher = Object.assign({}, { on: false, triggerId: '', triggerTime: '', From e0076c1191cd988540e8323a6ef9a3494fa69a89 Mon Sep 17 00:00:00 2001 From: ilyazarembsky Date: Tue, 23 Jul 2019 13:01:15 -0400 Subject: [PATCH 13/14] Only set broken page watcher on whitelist add, not both add and remove --- app/panel/reducers/summary.js | 1 + src/background.js | 2 -- src/classes/PanelData.js | 4 ++++ 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/panel/reducers/summary.js b/app/panel/reducers/summary.js index e94a644dc..04ca49dad 100644 --- a/app/panel/reducers/summary.js +++ b/app/panel/reducers/summary.js @@ -136,6 +136,7 @@ const _updateSitePolicy = (state, action) => { sendMessage('setPanelData', { site_whitelist: updated_whitelist, site_blacklist: updated_blacklist, + brokenPageMetricsWhitelistSite: updated_site_policy === 2, }); return { diff --git a/src/background.js b/src/background.js index e820f63c9..1833fc65b 100644 --- a/src/background.js +++ b/src/background.js @@ -1101,8 +1101,6 @@ function initializeDispatcher() { button.update(); utils.flushChromeMemoryCache(); cliqz.modules.core.action('refreshAppState'); - // if user has whitelisted site, suspect broken page - metrics.handleBrokenPageTrigger(globals.BROKEN_PAGE_WHITELIST); }); dispatcher.on('conf.save.enable_human_web', (enableHumanWeb) => { if (!IS_CLIQZ) { diff --git a/src/classes/PanelData.js b/src/classes/PanelData.js index eb020a4df..b558d846f 100644 --- a/src/classes/PanelData.js +++ b/src/classes/PanelData.js @@ -648,6 +648,10 @@ class PanelData { metrics.handleBrokenPageTrigger(globals.BROKEN_PAGE_TRACKER_TRUST_OR_UNBLOCK); } + if (data.brokenPageMetricsWhitelistSite) { + metrics.handleBrokenPageTrigger(globals.BROKEN_PAGE_WHITELIST); + } + if (syncSetDataChanged) { // Push conf changes to the server account.saveUserSettings().catch(err => log('PanelData saveUserSettings', err)); From b9cf1ee4c5b9f02aff6c3babd9614c5597fc963a Mon Sep 17 00:00:00 2001 From: ilyazarembsky Date: Tue, 23 Jul 2019 15:10:31 -0400 Subject: [PATCH 14/14] Change this.BROKEN_APGE_NEW_TAB value to a less mysterious 5. RIP Agent Smart --- src/classes/Globals.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/classes/Globals.js b/src/classes/Globals.js index bfdc99d51..8c58edcf4 100644 --- a/src/classes/Globals.js +++ b/src/classes/Globals.js @@ -75,7 +75,7 @@ class Globals { this.BROKEN_PAGE_WHITELIST = 2; this.BROKEN_PAGE_PAUSE = 3; this.BROKEN_PAGE_TRACKER_TRUST_OR_UNBLOCK = 4; - this.BROKEN_PAGE_NEW_TAB = 86; + this.BROKEN_PAGE_NEW_TAB = 5; // data stores this.REDIRECT_MAP = new Map();