Bug 1539470 - Prevent tabs opened in the background from affecting th…

lib/ASRouterTriggerListeners.jsm

@@ -30,6 +30,20 @@ async function checkStartupFinished(win) {
   }
 }
 
+/**
+ * Check if the current browser location matches the location received by the
+ * navigation listener.
+ */
+function isFocusedTab(aBrowser, aLocationURI) {
+  try {
+    if (aBrowser.ownerGlobal.gBrowser.currentURI.spec === aLocationURI.spec) {
+      return true;
+    }
+  } catch (e) {} // nsIURI.host can throw for non-nsStandardURL nsIURIs
+
+  return false;
+}
+
 function isPrivateWindow(win) {
   return !(win instanceof Ci.nsIDOMWindow) || win.closed || PrivateBrowsingUtils.isWindowPrivate(win);
 }
@@ -179,6 +193,12 @@ this.ASRouterTriggerListeners = new Map([
     },
 
     onLocationChange(aBrowser, aWebProgress, aRequest, aLocationURI, aFlags) {
+      // If a new tab was opened in the background we don't want it to increment
+      // the visits count. That will happen anyway because of the `onTabSwitch`
+      // listener that fires when the tab is selected.
+      if (!isFocusedTab(aBrowser, aLocationURI)) {
+        return;
+      }
       // Some websites trigger redirect events after they finish loading even
       // though the location remains the same. This results in onLocationChange
       // events to be fired twice.

test/browser/browser_asrouter_cfr.js

@@ -377,9 +377,17 @@ add_task(async function test_matchPattern() {
   let count = 0;
   const triggerHandler = () => ++count;
   const frequentVisitsTrigger = ASRouterTriggerListeners.get("frequentVisits");
-  frequentVisitsTrigger.init(triggerHandler, [], ["*://*.example.com/"]);
+  await frequentVisitsTrigger.init(triggerHandler, [], ["*://*.example.com/"]);
 
   const browser = gBrowser.selectedBrowser;
+  // Add a background tab that matches the pattern but shouldn't trigger the handler
+  const backgroundTab = BrowserTestUtils.addTab(gBrowser, "http://example.com");
+  gBrowser.removeTab(backgroundTab);
+
+  // No visits should be registered
+  Assert.ok(!frequentVisitsTrigger._visits.get("www.example.com"));
+  Assert.ok(!frequentVisitsTrigger._visits.get("example.com"));
+
   await BrowserTestUtils.loadURI(browser, "http://example.com/");
   await BrowserTestUtils.browserLoaded(browser, false, "http://example.com/");
 
@@ -400,6 +408,11 @@ add_task(async function test_matchPattern() {
 
   await BrowserTestUtils.waitForCondition(() => frequentVisitsTrigger._visits.get("www.example.com").length === 1, "www.example.com is a different host that also matches the pattern.");
   await BrowserTestUtils.waitForCondition(() => frequentVisitsTrigger._visits.get("example.com").length === 1, "www.example.com is a different host that also matches the pattern.");
+
+  Assert.equal(frequentVisitsTrigger._visits.get("www.example.com").length, 1);
+  Assert.equal(frequentVisitsTrigger._visits.get("example.com").length, 1);
+
+  frequentVisitsTrigger.uninit();
 });
 
 add_task(async function test_providerNames() {

test/unit/asrouter/ASRouterTriggerListeners.test.js

@@ -214,9 +214,9 @@ describe("ASRouterTriggerListeners", () => {
           const newTriggerHandler = sinon.stub();
           await trigger.init(newTriggerHandler, hosts);
 
-          const browser = {};
           const webProgress = {isTopLevel: true};
           const aLocationURI = {host: "subdomain.mozilla.org", spec: "subdomain.mozilla.org"};
+          const browser = {ownerGlobal: {gBrowser: {currentURI: aLocationURI}}};
           const aRequest = {
             QueryInterface: sandbox.stub().returns({
               originalURI: {spec: "www.mozilla.org", host: "www.mozilla.org"},
@@ -231,9 +231,9 @@ describe("ASRouterTriggerListeners", () => {
         const newTriggerHandler = sinon.stub();
         await openURLListener.init(newTriggerHandler, hosts);
 
-        const browser = {};
         const webProgress = {isTopLevel: true};
         const aLocationURI = {host: "subdomain.mozilla.org", spec: "subdomain.mozilla.org"};
+        const browser = {ownerGlobal: {gBrowser: {currentURI: aLocationURI}}};
         const aRequest = {
           QueryInterface: sandbox.stub().returns({
             originalURI: {spec: "www.mozilla.org", host: "www.mozilla.org"},
@@ -247,9 +247,9 @@ describe("ASRouterTriggerListeners", () => {
           const newTriggerHandler = sinon.stub();
           await trigger.init(newTriggerHandler, hosts);
 
-          const browser = {};
           const webProgress = {isTopLevel: true};
           const aLocationURI = {host: "subdomain.mozilla.org", spec: "subdomain.mozilla.org"};
+          const browser = {ownerGlobal: {gBrowser: {currentURI: aLocationURI}}};
           const aRequest = {
             QueryInterface: sandbox.stub().returns({
               originalURI: {spec: "www.mozilla.org", host: "www.mozilla.org"},
@@ -291,6 +291,21 @@ describe("ASRouterTriggerListeners", () => {
         assert.calledOnce(aRequest.QueryInterface);
         assert.notCalled(newTriggerHandler);
       });
+      it("should not call triggerHandler if the tab opened is not focused", async () => {
+        const newTriggerHandler = sinon.stub();
+        await frequentVisitsListener.init(newTriggerHandler, hosts);
+
+        const webProgress = {isTopLevel: true};
+        const aLocationURI = {host: "subdomain.mozilla.org", spec: "subdomain.mozilla.org"};
+        const browser = {ownerGlobal: {gBrowser: {currentURI: {spec: "different location"}}}};
+        const aRequest = {
+          QueryInterface: sandbox.stub().returns({
+            originalURI: {spec: "www.mozilla.org", host: "www.mozilla.org"},
+          }),
+        };
+        frequentVisitsListener.onLocationChange(browser, webProgress, aRequest, aLocationURI);
+        assert.notCalled(newTriggerHandler);
+      });
     });
 
     describe("delayed startup finished", () => {