From 8895ff9f62f8be5e63dbbd3f1df96179c7ae65c1 Mon Sep 17 00:00:00 2001 From: Andreea Pavel Date: Wed, 2 Jun 2021 00:40:43 +0300 Subject: [PATCH] Backed out changeset 7315ee22da8e (bug 1713323) for failing bc at browser_interactions_view_time.js on a CLOSED TREE --- browser/components/places/Interactions.jsm | 128 ++++------ .../components/places/InteractionsChild.jsm | 16 -- .../browser_interactions_view_time.js | 227 ++---------------- 3 files changed, 58 insertions(+), 313 deletions(-) diff --git a/browser/components/places/Interactions.jsm b/browser/components/places/Interactions.jsm index 2eeb56838d2d2..ec666b9abea94 100644 --- a/browser/components/places/Interactions.jsm +++ b/browser/components/places/Interactions.jsm @@ -12,7 +12,6 @@ const { XPCOMUtils } = ChromeUtils.import( XPCOMUtils.defineLazyModuleGetters(this, { BrowserWindowTracker: "resource:///modules/BrowserWindowTracker.jsm", - PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.jsm", Services: "resource://gre/modules/Services.jsm", }); @@ -54,21 +53,21 @@ class _Interactions { #interactions = new WeakMap(); /** - * Tracks the currently active window so that we can avoid recording - * interactions in non-active windows. + * This is a reference to the interaction for the foreground browser. If the + * application is not focussed, or a non-interaction page is visible, then + * this will be undefined. * - * @type {DOMWindow} + * @type {InteractionInfo} */ - #activeWindow = undefined; + #currentInteraction = undefined; /** - * This stores the page view start time of the current page view. - * For any single page view, this may be moved multiple times as the - * associated interaction is updated for the current total page view time. + * This stores the time that the current interaction was activated or last + * updated. It is used to calculate the total page view time. * * @type {number} */ - _pageViewStartTime = Cu.now(); + _lastUpdateTime = Cu.now(); /** * Initializes, sets up actors and observers. @@ -105,8 +104,6 @@ class _Interactions { }, }); - this.#activeWindow = Services.wm.getMostRecentBrowserWindow(); - for (let win of BrowserWindowTracker.orderedWindows) { if (!win.closed) { this.#registerWindow(win); @@ -136,10 +133,9 @@ class _Interactions { }; this.#interactions.set(browser, interaction); - // Only reset the time if this is being loaded in the active tab of the - // active window. - if (docInfo.isActive && browser.ownerGlobal == this.#activeWindow) { - this._pageViewStartTime = Cu.now(); + if (docInfo.isActive) { + this.#currentInteraction = interaction; + this._lastUpdateTime = Cu.now(); } } @@ -152,70 +148,30 @@ class _Interactions { * The browser object associated with the interaction. */ registerEndOfInteraction(browser) { - this.logConsole.debug("End of interaction"); - - this.#updateInteraction(browser); - this.#interactions.delete(browser); - } - - /** - * Updates the current interaction. - * - * @param {Browser} [browser] - * The browser object that has triggered the update, if known. This is - * used to check if the browser is in the active window, and as an - * optimization to avoid obtaining the browser object. - */ - #updateInteraction(browser = undefined) { - if ( - !this.#activeWindow || - (browser && browser.ownerGlobal != this.#activeWindow) - ) { - this.logConsole.debug("No active window"); - return; - } - - if (!browser) { - browser = this.#activeWindow.gBrowser.selectedTab.linkedBrowser; - } - let interaction = this.#interactions.get(browser); if (!interaction) { - this.logConsole.debug("No interaction to update"); return; } - interaction.totalViewTime += Cu.now() - this._pageViewStartTime; - this._pageViewStartTime = Cu.now(); + this.#updateInteractionViewTime(); + this.logConsole.debug("End of interaction for", interaction); + this._updateDatabase(interaction); + + this.#interactions.delete(browser); + this.#currentInteraction = undefined; } /** - * Handles a window becoming active. - * - * @param {DOMWindow} win + * Updates the interaction view time of the current interaction. */ - #onActivateWindow(win) { - this.logConsole.debug("Activate window"); - - if (PrivateBrowsingUtils.isWindowPrivate(win)) { + #updateInteractionViewTime() { + if (!this.#currentInteraction) { return; } - - this.#activeWindow = win; - this._pageViewStartTime = Cu.now(); - } - - /** - * Handles a window going inactive. - * - * @param {DOMWindow} win - */ - #onDeactivateWindow(win) { - this.logConsole.debug("Deactivate window"); - - this.#updateInteraction(); - this.#activeWindow = undefined; + let now = Cu.now(); + this.#currentInteraction.totalViewTime += now - this._lastUpdateTime; + this._lastUpdateTime = now; } /** @@ -225,12 +181,25 @@ class _Interactions { * * @param {Browser} previousBrowser * The instance of the browser that the user switched away from. + * @param {Browser} browser + * The instance of the browser that the user switched to. */ - #onTabSelect(previousBrowser) { + #onTabSelect(previousBrowser, browser) { this.logConsole.debug("Tab switch notified"); - this.#updateInteraction(previousBrowser); - this._pageViewStartTime = Cu.now(); + let interaction = this.#interactions.get(previousBrowser); + if (interaction) { + this.#updateInteractionViewTime(); + // The interaction might continue later, but for now we update the + // database to ensure that the interaction does not go away (e.g. on + // tab close). + this._updateDatabase(interaction); + } + + // Note: this could be undefined in the case of switching to a browser with + // something that we don't track, e.g. about:blank. + this.#currentInteraction = this.#interactions.get(browser); + this._lastUpdateTime = Cu.now(); } /** @@ -241,13 +210,10 @@ class _Interactions { handleEvent(event) { switch (event.type) { case "TabSelect": - this.#onTabSelect(event.detail.previousTab.linkedBrowser); - break; - case "activate": - this.#onActivateWindow(event.target); - break; - case "deactivate": - this.#onDeactivateWindow(event.target); + this.#onTabSelect( + event.detail.previousTab.linkedBrowser, + event.target.linkedBrowser + ); break; case "unload": this.#unregisterWindow(event.target); @@ -277,13 +243,7 @@ class _Interactions { * The window to register in. */ #registerWindow(win) { - if (PrivateBrowsingUtils.isWindowPrivate(win)) { - return; - } - win.addEventListener("TabSelect", this, true); - win.addEventListener("deactivate", this, true); - win.addEventListener("activate", this, true); } /** @@ -294,8 +254,6 @@ class _Interactions { */ #unregisterWindow(win) { win.removeEventListener("TabSelect", this, true); - win.removeEventListener("deactivate", this, true); - win.removeEventListener("activate", this, true); } /** diff --git a/browser/components/places/InteractionsChild.jsm b/browser/components/places/InteractionsChild.jsm index deb2974f6f627..2e764666f371b 100644 --- a/browser/components/places/InteractionsChild.jsm +++ b/browser/components/places/InteractionsChild.jsm @@ -6,28 +6,12 @@ var EXPORTED_SYMBOLS = ["InteractionsChild"]; -ChromeUtils.defineModuleGetter( - this, - "PrivateBrowsingUtils", - "resource://gre/modules/PrivateBrowsingUtils.jsm" -); - /** * Listens for interactions in the child process and passes information to the * parent. */ class InteractionsChild extends JSWindowActorChild { - actorCreated() { - this.isContentWindowPrivate = PrivateBrowsingUtils.isContentWindowPrivate( - this.contentWindow - ); - } - async handleEvent(event) { - if (this.isContentWindowPrivate) { - // No recording in private browsing mode. - return; - } switch (event.type) { case "DOMContentLoaded": { if ( diff --git a/browser/components/places/tests/browser/interactions/browser_interactions_view_time.js b/browser/components/places/tests/browser/interactions/browser_interactions_view_time.js index 851bc3c31106d..27bc2198f6392 100644 --- a/browser/components/places/tests/browser/interactions/browser_interactions_view_time.js +++ b/browser/components/places/tests/browser/interactions/browser_interactions_view_time.js @@ -8,8 +8,6 @@ const TEST_URL = "https://example.com/"; const TEST_URL2 = "https://example.com/browser"; -const TEST_URL3 = "https://example.com/browser/browser"; -const TEST_URL4 = "https://example.com/browser/browser/components"; add_task(async function setup() { sinon.spy(Interactions, "_updateDatabase"); @@ -19,13 +17,12 @@ add_task(async function setup() { }); }); -async function assertDatabaseValues(expected) { +async function assertDatabaseValues(args, expected) { await BrowserTestUtils.waitForCondition( () => Interactions._updateDatabase.callCount == expected.length, "Should have saved to the database" ); - let args = Interactions._updateDatabase.args; for (let i = 0; i < expected.length; i++) { let actual = args[i][0]; Assert.equal( @@ -51,24 +48,24 @@ async function assertDatabaseValues(expected) { add_task(async function test_interactions_simple_load_and_navigate_away() { await BrowserTestUtils.withNewTab(TEST_URL, async browser => { - Interactions._pageViewStartTime = Cu.now() - 10000; + Interactions._lastUpdateTime = Cu.now() - 10000; BrowserTestUtils.loadURI(browser, TEST_URL2); await BrowserTestUtils.browserLoaded(browser, false, TEST_URL2); - await assertDatabaseValues([ + await assertDatabaseValues(Interactions._updateDatabase.args, [ { url: TEST_URL, totalViewTime: 10000, }, ]); - Interactions._pageViewStartTime = Cu.now() - 20000; + Interactions._lastUpdateTime = Cu.now() - 20000; BrowserTestUtils.loadURI(browser, "about:blank"); await BrowserTestUtils.browserLoaded(browser, false, "about:blank"); - await assertDatabaseValues([ + await assertDatabaseValues(Interactions._updateDatabase.args, [ { url: TEST_URL, totalViewTime: 10000, @@ -84,10 +81,10 @@ add_task(async function test_interactions_simple_load_and_navigate_away() { add_task(async function test_interactions_close_tab() { sinon.reset(); await BrowserTestUtils.withNewTab(TEST_URL, async browser => { - Interactions._pageViewStartTime = Cu.now() - 20000; + Interactions._lastUpdateTime = Cu.now() - 20000; }); - await assertDatabaseValues([ + await assertDatabaseValues(Interactions._updateDatabase.args, [ { url: TEST_URL, totalViewTime: 20000, @@ -105,7 +102,7 @@ add_task(async function test_interactions_background_tab() { let tab2 = BrowserTestUtils.addTab(gBrowser, TEST_URL2); await BrowserTestUtils.browserLoaded(tab2.linkedBrowser, false, TEST_URL2); - Interactions._pageViewStartTime = Cu.now() - 10000; + Interactions._lastUpdateTime = Cu.now() - 10000; BrowserTestUtils.removeTab(tab2); @@ -122,7 +119,7 @@ add_task(async function test_interactions_background_tab() { BrowserTestUtils.removeTab(tab1); // Only the interaction in the visible tab should have been recorded. - await assertDatabaseValues([ + await assertDatabaseValues(Interactions._updateDatabase.args, [ { url: TEST_URL, totalViewTime: 10000, @@ -141,11 +138,11 @@ add_task(async function test_interactions_switch_tabs() { await BrowserTestUtils.browserLoaded(tab2.linkedBrowser, false, TEST_URL2); info("Switch to second tab"); - Interactions._pageViewStartTime = Cu.now() - 10000; + Interactions._lastUpdateTime = Cu.now() - 10000; gBrowser.selectedTab = tab2; // Only the interaction of the first tab should be recorded so far. - await assertDatabaseValues([ + await assertDatabaseValues(Interactions._updateDatabase.args, [ { url: TEST_URL, totalViewTime: 10000, @@ -154,11 +151,11 @@ add_task(async function test_interactions_switch_tabs() { let tab1ViewTime = Interactions._updateDatabase.args[0][0].totalViewTime; info("Switch back to first tab"); - Interactions._pageViewStartTime = Cu.now() - 20000; + Interactions._lastUpdateTime = Cu.now() - 20000; gBrowser.selectedTab = tab1; // The interaction of the second tab should now be recorded. - await assertDatabaseValues([ + await assertDatabaseValues(Interactions._updateDatabase.args, [ { url: TEST_URL, exactTotalViewTime: tab1ViewTime, @@ -172,11 +169,11 @@ add_task(async function test_interactions_switch_tabs() { info("Switch to second tab again"); // We reset the stub here to make the database change clearer. sinon.reset(); - Interactions._pageViewStartTime = Cu.now() - 30000; + Interactions._lastUpdateTime = Cu.now() - 30000; gBrowser.selectedTab = tab2; // The interaction of the second tab should now be recorded. - await assertDatabaseValues([ + await assertDatabaseValues(Interactions._updateDatabase.args, [ { url: TEST_URL, totalViewTime: tab1ViewTime + 30000, @@ -186,197 +183,3 @@ add_task(async function test_interactions_switch_tabs() { BrowserTestUtils.removeTab(tab1); BrowserTestUtils.removeTab(tab2); }); - -add_task(async function test_interactions_switch_windows() { - sinon.reset(); - - // Open a tab in the first window. - let tabInOriginalWindow = await BrowserTestUtils.openNewForegroundTab({ - gBrowser, - url: TEST_URL, - }); - - // and then load the second window. - Interactions._pageViewStartTime = Cu.now() - 10000; - - let otherWin = await BrowserTestUtils.openNewBrowserWindow(); - - BrowserTestUtils.loadURI(otherWin.gBrowser.selectedBrowser, TEST_URL2); - await BrowserTestUtils.browserLoaded( - otherWin.gBrowser.selectedBrowser, - false, - TEST_URL2 - ); - - await assertDatabaseValues([ - { - url: TEST_URL, - totalViewTime: 10000, - }, - ]); - let originalWindowViewTime = - Interactions._updateDatabase.args[0][0].totalViewTime; - - info("Switch back to original window"); - Interactions._pageViewStartTime = Cu.now() - 20000; - window.focus(); - - await assertDatabaseValues([ - { - url: TEST_URL, - exactTotalViewTime: originalWindowViewTime, - }, - { - url: TEST_URL2, - totalViewTime: 20000, - }, - ]); - let newWindowViewTime = Interactions._updateDatabase.args[1][0].totalViewTime; - - info("Switch back to new window"); - Interactions._pageViewStartTime = Cu.now() - 30000; - otherWin.focus(); - - await assertDatabaseValues([ - { - url: TEST_URL, - // Note: this should be `exactTotalViewTime:originalWindowViewTime`, but - // apply updates to the same object. - totalViewTime: originalWindowViewTime + 30000, - }, - { - url: TEST_URL2, - exactTotalViewTime: newWindowViewTime, - }, - { - url: TEST_URL, - totalViewTime: originalWindowViewTime + 30000, - }, - ]); - - BrowserTestUtils.removeTab(tabInOriginalWindow); - await BrowserTestUtils.closeWindow(otherWin); -}); - -add_task(async function test_interactions_loading_in_unfocused_windows() { - sinon.reset(); - - let otherWin = await BrowserTestUtils.openNewBrowserWindow(); - - BrowserTestUtils.loadURI(otherWin.gBrowser.selectedBrowser, TEST_URL); - await BrowserTestUtils.browserLoaded( - otherWin.gBrowser.selectedBrowser, - false, - TEST_URL - ); - - Interactions._pageViewStartTime = Cu.now() - 10000; - - BrowserTestUtils.loadURI(otherWin.gBrowser.selectedBrowser, TEST_URL2); - await BrowserTestUtils.browserLoaded( - otherWin.gBrowser.selectedBrowser, - false, - TEST_URL2 - ); - - // Only the interaction of the first tab should be recorded so far. - await assertDatabaseValues([ - { - url: TEST_URL, - totalViewTime: 10000, - }, - ]); - let newWindowViewTime = Interactions._updateDatabase.args[0][0].totalViewTime; - - // Open a tab in the background window, and then navigate somewhere else, - // this should not record an intereaction. - let tabInOriginalWindow = await BrowserTestUtils.openNewForegroundTab({ - gBrowser, - url: TEST_URL3, - }); - - Interactions._pageViewStartTime = Cu.now() - 20000; - - BrowserTestUtils.loadURI(tabInOriginalWindow.linkedBrowser, TEST_URL4); - await BrowserTestUtils.browserLoaded( - tabInOriginalWindow.linkedBrowser, - false, - TEST_URL4 - ); - - // Only the interaction of the first tab should be recorded so far. - await assertDatabaseValues([ - { - url: TEST_URL, - exactTotalViewTime: newWindowViewTime, - }, - ]); - - BrowserTestUtils.removeTab(tabInOriginalWindow); - await BrowserTestUtils.closeWindow(otherWin); -}); - -add_task(async function test_interactions_private_browsing() { - sinon.reset(); - - // Open a tab in the first window. - let tabInOriginalWindow = await BrowserTestUtils.openNewForegroundTab({ - gBrowser, - url: TEST_URL, - }); - - // and then load the second window. - Interactions._pageViewStartTime = Cu.now() - 10000; - - let privateWin = await BrowserTestUtils.openNewBrowserWindow({ - private: true, - }); - - BrowserTestUtils.loadURI(privateWin.gBrowser.selectedBrowser, TEST_URL2); - await BrowserTestUtils.browserLoaded( - privateWin.gBrowser.selectedBrowser, - false, - TEST_URL2 - ); - - await assertDatabaseValues([ - { - url: TEST_URL, - totalViewTime: 10000, - }, - ]); - let originalWindowViewTime = - Interactions._updateDatabase.args[0][0].totalViewTime; - - info("Switch back to original window"); - Interactions._pageViewStartTime = Cu.now() - 20000; - window.focus(); - - // The private window site should not be recorded. - await assertDatabaseValues([ - { - url: TEST_URL, - exactTotalViewTime: originalWindowViewTime, - }, - ]); - - info("Switch back to new window"); - Interactions._pageViewStartTime = Cu.now() - 30000; - privateWin.focus(); - - await assertDatabaseValues([ - { - url: TEST_URL, - // Note: this should be `exactTotalViewTime:originalWindowViewTime`, but - // apply updates to the same object. - totalViewTime: originalWindowViewTime + 30000, - }, - { - url: TEST_URL, - totalViewTime: originalWindowViewTime + 30000, - }, - ]); - - BrowserTestUtils.removeTab(tabInOriginalWindow); - await BrowserTestUtils.closeWindow(privateWin); -});