Bug 1533359 - Targeting support for any subdomain (mozilla#4869)

dmose · Apr 3, 2019 · ff07488 · ff07488
1 parent e76ce12
commit ff07488
Show file tree

Hide file tree

Showing 7 changed files with 154 additions and 25 deletions.
diff --git a/content-src/asrouter/schemas/message-format.md b/content-src/asrouter/schemas/message-format.md
@@ -10,6 +10,8 @@ Field name | Type     | Required | Description | Example / Note
 `campaign` | `string` | No | Campaign id that the message belongs to | `RustWebAssembly`
 `targeting` | `string` `JEXL` | No | A [JEXL expression](http://normandy.readthedocs.io/en/latest/user/filter_expressions.html#jexl-basics) with all targeting information needed in order to decide if the message is shown | Not yet implemented, [Examples](#targeting-attributes)
 `trigger` | `string` | No | An event or condition upon which the message will be immediately shown. This can be combined with `targeting`. Messages that define a trigger will not be shown during non-trigger-based passive message rotation.
+`trigger.params` | `[string]` | No | A set of hostnames passed down as parameters to the trigger condition. Used to restrict the number of domains where the trigger/message is valid. | [See example below](#trigger-params)
+`trigger.patterns` | `[string]` | No | A set of patterns that match multiple hostnames passed down as parameters to the trigger condition. Used to restrict the number of domains where the trigger/message is valid. | [See example below](#trigger-patterns)
 `frequency` | `object` | No | A definition for frequency cap information for the message
 `frequency.lifetime` | `integer` | No | The maximum number of lifetime impressions for the message.
 `frequency.custom` | `array` | No | An array of frequency cap definition objects including `period`, a time period in milliseconds, and `cap`, a max number of impressions for that period.
@@ -81,5 +83,19 @@ If a tag that is not on the allowed is used, the text content will be extracted
 
 Grouping multiple allowed elements is not possible, only the first level will be used: `<u><b>text</b></u>` will be interpreted as `<u>text</u>`.
 
+### Trigger params
+A set of hostnames that need to exactly match the location of the selected tab in order for the trigger to execute.
+```
+["github.com", "wwww.github.com"]
+```
+More examples in the [CFRMessageProvider](https://github.com/mozilla/activity-stream/blob/e76ce12fbaaac1182aa492b84fc038f78c3acc33/lib/CFRMessageProvider.jsm#L40-L47).
+
+### Trigger patterns
+A set of patterns that can match multiple hostnames. When the location of the selected tab matches one of the patterns it can execute a trigger.
+```
+["*://*.github.com"] // can match `github.com` but also match `https://gist.github.com/`
+```
+More [MatchPattern examples](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Match_patterns#Examples).
+
 ### Targeting attributes
 (This section has moved to [targeting-attributes.md](../docs/targeting-attributes.md)).
diff --git a/lib/ASRouter.jsm b/lib/ASRouter.jsm
@@ -463,7 +463,7 @@ class _ASRouter {
       const unseenListeners = new Set(ASRouterTriggerListeners.keys());
       for (const {trigger} of newState.messages) {
         if (trigger && ASRouterTriggerListeners.has(trigger.id)) {
-          await ASRouterTriggerListeners.get(trigger.id).init(this._triggerHandler, trigger.params);
+          await ASRouterTriggerListeners.get(trigger.id).init(this._triggerHandler, trigger.params, trigger.patterns);
           unseenListeners.delete(trigger.id);
         }
       }

diff --git a/lib/ASRouterTargeting.jsm b/lib/ASRouterTargeting.jsm
@@ -337,6 +337,8 @@ this.ASRouterTargeting = {
       return false;
     } else if (!candidateMessageTrigger.params) {
       return true;
+    } else if (candidateMessageTrigger.patterns) {
+      return true;
     }
     return candidateMessageTrigger.params.includes(trigger.param);
   },

diff --git a/lib/ASRouterTriggerListeners.jsm b/lib/ASRouterTriggerListeners.jsm
@@ -9,6 +9,7 @@ ChromeUtils.defineModuleGetter(this, "PrivateBrowsingUtils",
   "resource://gre/modules/PrivateBrowsingUtils.jsm");
 
 const FEW_MINUTES = 15 * 60 * 1000; // 15 mins
+const MATCH_PATTERN_OPTIONS = {ignorePath: true};
 
 /**
  * Wait for browser startup to finish to avoid accessing uninitialized
@@ -40,12 +41,23 @@ function isPrivateWindow(win) {
  *
  * @returns {string} - the host that matched the whitelist
  */
-function checkHost(aLocationURI, hosts, aRequest) {
+function checkURLMatch(aLocationURI, {hosts, matchPatternSet}, aRequest) {
   // Check current location against whitelisted hosts
   if (hosts.has(aLocationURI.host)) {
     return aLocationURI.host;
   }
 
+  if (matchPatternSet) {
+    if (matchPatternSet.matches(aLocationURI.spec)) {
+      return aLocationURI.host;
+    }
+  }
+
+  // Nothing else to check, return early
+  if (!aRequest) {
+    return false;
+  }
+
   // The original URL at the start of the request
   const originalLocation = aRequest.QueryInterface(Ci.nsIChannel).originalURI;
   // We have been redirected
@@ -65,27 +77,37 @@ this.ASRouterTriggerListeners = new Map([
     _initialized: false,
     _triggerHandler: null,
     _hosts: null,
+    _matchPatternSet: null,
     _visits: null,
 
-    async init(triggerHandler, hosts) {
-      if (this._initialized) {
-        return;
-      }
-      this.onTabSwitch = this.onTabSwitch.bind(this);
+    async init(triggerHandler, hosts = [], patterns) {
+      if (!this._initialized) {
+        this.onTabSwitch = this.onTabSwitch.bind(this);
 
-      // Add listeners to all existing browser windows
-      for (let win of Services.wm.getEnumerator("navigator:browser")) {
-        if (isPrivateWindow(win)) {
-          continue;
+        // Add listeners to all existing browser windows
+        for (let win of Services.wm.getEnumerator("navigator:browser")) {
+          if (isPrivateWindow(win)) {
+            continue;
+          }
+          await checkStartupFinished(win);
+          win.addEventListener("TabSelect", this.onTabSwitch);
+          win.gBrowser.addTabsProgressListener(this);
         }
-        await checkStartupFinished(win);
-        win.addEventListener("TabSelect", this.onTabSwitch);
-        win.gBrowser.addTabsProgressListener(this);
-      }
 
-      this._initialized = true;
+        this._initialized = true;
+        this._visits = new Map();
+      }
       this._triggerHandler = triggerHandler;
-      this._visits = new Map();
+      if (patterns) {
+        if (this._matchPatternSet) {
+          this._matchPatternSet = new MatchPatternSet(new Set([
+            ...this._matchPatternSet.patterns,
+            ...patterns,
+          ]), MATCH_PATTERN_OPTIONS);
+        } else {
+          this._matchPatternSet = new MatchPatternSet(patterns, MATCH_PATTERN_OPTIONS);
+        }
+      }
       if (this._hosts) {
         hosts.forEach(h => this._hosts.add(h));
       } else {
@@ -124,9 +146,11 @@ this.ASRouterTriggerListeners = new Map([
       try {
         // nsIURI.host can throw for non-nsStandardURL nsIURIs.
         host = gBrowser.currentURI.host;
-      } catch (e) {} // Couldn't parse location URL
+      } catch (e) {
+        return; // Couldn't parse location URL
+      }
 
-      if (host && this._hosts.has(host)) {
+      if (checkURLMatch(gBrowser.currentURI, {hosts: this._hosts, matchPatternSet: this._matchPatternSet})) {
         this.triggerHandler(gBrowser.selectedBrowser, host);
       }
     },
@@ -163,7 +187,7 @@ this.ASRouterTriggerListeners = new Map([
       // events to be fired twice.
       const isSameDocument = !!(aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT);
       if (host && aWebProgress.isTopLevel && !isSameDocument) {
-        host = checkHost(aLocationURI, this._hosts, aRequest);
+        host = checkURLMatch(aLocationURI, {hosts: this._hosts, matchPatternSet: this._matchPatternSet}, aRequest);
         if (host) {
           this.triggerHandler(aBrowser, host);
         }
@@ -214,6 +238,7 @@ this.ASRouterTriggerListeners = new Map([
         this._initialized = false;
         this._triggerHandler = null;
         this._hosts = null;
+        this._matchPatternSet = null;
         this._visits = null;
       }
     },
@@ -233,7 +258,7 @@ this.ASRouterTriggerListeners = new Map([
      * If the listener is already initialised, `init` will replace the trigger
      * handler and add any new hosts to `this._hosts`.
      */
-    async init(triggerHandler, hosts = []) {
+    async init(triggerHandler, hosts = [], patterns) {
       if (!this._initialized) {
         this.onLocationChange = this.onLocationChange.bind(this);
 
@@ -281,15 +306,15 @@ this.ASRouterTriggerListeners = new Map([
       let host;
       try {
         host = aLocationURI ? aLocationURI.host : "";
-      } catch (e) { // about: pages will throw errors
+      } catch (e) { // about: pages throw errors
         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.
       const isSameDocument = !!(aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT);
       if (host && aWebProgress.isTopLevel && !isSameDocument) {
-        host = checkHost(aLocationURI, this._hosts, aRequest);
+        host = checkURLMatch(aLocationURI, {hosts: this._hosts}, aRequest);
         if (host) {
           this._triggerHandler(aBrowser, {id: "openURL", param: host});
         }

diff --git a/test/browser/browser_asrouter_cfr.js b/test/browser/browser_asrouter_cfr.js
@@ -372,3 +372,30 @@ add_task(async function test_onLocationChange_cb() {
 
   Assert.equal(count, 2, "We moved to a new document");
 });
+
+add_task(async function test_matchPattern() {
+  let count = 0;
+  const triggerHandler = () => ++count;
+  ASRouterTriggerListeners.get("frequentVisits").init(triggerHandler, [], ["*://*.example.com/"]);
+
+  const browser = gBrowser.selectedBrowser;
+  await BrowserTestUtils.loadURI(browser, "http://example.com/");
+  await BrowserTestUtils.browserLoaded(browser, false, "http://example.com/");
+
+  Assert.equal(count, 1, "Registered pattern matched the current location");
+
+  await BrowserTestUtils.loadURI(browser, "about:config");
+  await BrowserTestUtils.browserLoaded(browser, false, "about:config");
+
+  Assert.equal(count, 1, "Navigated to a new page but not a match");
+
+  await BrowserTestUtils.loadURI(browser, "http://example.com/");
+  await BrowserTestUtils.browserLoaded(browser, false, "http://example.com/");
+
+  Assert.equal(count, 1, "Navigated to a location that matches the pattern but within 15 mins");
+
+  await BrowserTestUtils.loadURI(browser, "http://www.example.com/");
+  await BrowserTestUtils.browserLoaded(browser, false, "http://www.example.com/");
+
+  Assert.equal(count, 2, "www.example.com is a different host that also matches the pattern.");
+});
diff --git a/test/unit/asrouter/ASRouter.test.js b/test/unit/asrouter/ASRouter.test.js
@@ -341,9 +341,9 @@ describe("ASRouter", () => {
 
       assert.calledTwice(ASRouterTriggerListeners.get("openURL").init);
       assert.calledWithExactly(ASRouterTriggerListeners.get("openURL").init,
-        Router._triggerHandler, ["www.mozilla.org", "www.mozilla.com"]);
+        Router._triggerHandler, ["www.mozilla.org", "www.mozilla.com"], undefined);
       assert.calledWithExactly(ASRouterTriggerListeners.get("openURL").init,
-        Router._triggerHandler, ["www.example.com"]);
+        Router._triggerHandler, ["www.example.com"], undefined);
     });
     it("should gracefully handle RemoteSettings blowing up", async () => {
       sandbox.stub(MessageLoaderUtils, "_getRemoteSettingsMessages").rejects("fake error");

diff --git a/test/unit/asrouter/ASRouterTriggerListeners.test.js b/test/unit/asrouter/ASRouterTriggerListeners.test.js
@@ -1,4 +1,5 @@
 import {ASRouterTriggerListeners} from "lib/ASRouterTriggerListeners.jsm";
+import {GlobalOverrider} from "test/unit/utils";
 
 describe("ASRouterTriggerListeners", () => {
   let sandbox;
@@ -83,6 +84,30 @@ describe("ASRouterTriggerListeners", () => {
 
       assert.notCalled(stub);
     });
+    describe("MatchPattern", () => {
+      let globals;
+      beforeEach(() => {
+        globals = new GlobalOverrider();
+        globals.set("MatchPatternSet", sandbox.stub().callsFake(patterns => ({patterns})));
+      });
+      afterEach(() => {
+        globals.restore();
+        frequentVisitsListener.uninit();
+      });
+      it("should create a matchPatternSet", async () => {
+        await frequentVisitsListener.init(_triggerHandler, hosts, ["pattern"]);
+
+        assert.calledOnce(window.MatchPatternSet);
+        assert.calledWithExactly(window.MatchPatternSet, ["pattern"], {ignorePath: true});
+      });
+      it("should allow to add multiple patterns and dedupe", async () => {
+        await frequentVisitsListener.init(_triggerHandler, hosts, ["pattern"]);
+        await frequentVisitsListener.init(_triggerHandler, hosts, ["foo"]);
+
+        assert.calledTwice(window.MatchPatternSet);
+        assert.calledWithExactly(window.MatchPatternSet, new Set(["pattern", "foo"]), {ignorePath: true});
+      });
+    });
   });
 
   describe("openURL listener", () => {
@@ -170,6 +195,7 @@ describe("ASRouterTriggerListeners", () => {
     describe("#onLocationChange", () => {
       afterEach(() => {
         openURLListener.uninit();
+        frequentVisitsListener.uninit();
       });
 
       it("should call the ._triggerHandler with the right arguments", async () => {
@@ -216,6 +242,39 @@ describe("ASRouterTriggerListeners", () => {
         openURLListener.onLocationChange(browser, webProgress, aRequest, aLocationURI);
         assert.calledWithExactly(newTriggerHandler, browser, {id: "openURL", param: "www.mozilla.org"});
       });
+      it("should call triggerHandler for a redirect (openURL + frequentVisits)", async () => {
+        for (let trigger of [openURLListener, frequentVisitsListener]) {
+          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 aRequest = {
+            QueryInterface: sandbox.stub().returns({
+              originalURI: {spec: "www.mozilla.org", host: "www.mozilla.org"},
+            }),
+          };
+          trigger.onLocationChange(browser, webProgress, aRequest, aLocationURI);
+          assert.calledOnce(aRequest.QueryInterface);
+          assert.calledOnce(newTriggerHandler);
+        }
+      });
+      it("should call triggerHandler with the right arguments (redirect)", async () => {
+        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 aRequest = {
+          QueryInterface: sandbox.stub().returns({
+            originalURI: {spec: "www.mozilla.org", host: "www.mozilla.org"},
+          }),
+        };
+        openURLListener.onLocationChange(browser, webProgress, aRequest, aLocationURI);
+        assert.calledWithExactly(newTriggerHandler, browser, {id: "openURL", param: "www.mozilla.org"});
+      });
       it("should fail for subdomains (not redirect)", async () => {
         const newTriggerHandler = sinon.stub();
         await openURLListener.init(newTriggerHandler, hosts);