From a75d2bca75bc20f7392d7887dfc2d132a9e23860 Mon Sep 17 00:00:00 2001 From: Adam Hillier Date: Tue, 31 Jul 2018 13:31:23 -0700 Subject: [PATCH] Fix Bug 1471351: Add openURL trigger to ASRouter (#4255) * Fix Bug 1471351 (1/2): Allow ASRouter triggers to have params * Fix Bug 1471351 (2/2): Add openURL trigger to ASRouter --- content-src/asrouter/asrouter-content.jsx | 2 +- .../schemas/provider-response.schema.json | 22 ++- karma.mc.config.js | 3 +- lib/ASRouter.jsm | 47 +++++-- lib/ASRouterTargeting.jsm | 14 +- lib/ASRouterTriggerListeners.jsm | 118 ++++++++++++++++ lib/OnboardingMessageProvider.jsm | 8 +- test/browser/browser.ini | 1 + .../browser_asrouter_trigger_listeners.js | 58 ++++++++ test/unit/asrouter/ASRouter.test.js | 66 +++++++-- .../asrouter/ASRouterTriggerListeners.test.js | 133 ++++++++++++++++++ test/unit/unit-entry.js | 4 +- 12 files changed, 442 insertions(+), 34 deletions(-) create mode 100644 lib/ASRouterTriggerListeners.jsm create mode 100644 test/browser/browser_asrouter_trigger_listeners.js create mode 100644 test/unit/asrouter/ASRouterTriggerListeners.test.js diff --git a/content-src/asrouter/asrouter-content.jsx b/content-src/asrouter/asrouter-content.jsx index 0633b8a9d..653df9588 100644 --- a/content-src/asrouter/asrouter-content.jsx +++ b/content-src/asrouter/asrouter-content.jsx @@ -191,7 +191,7 @@ export class ASRouterUISurface extends React.PureComponent { // If we are loading about:welcome we want to trigger the onboarding messages if (this.props.document.location.href === "about:welcome") { - ASRouterUtils.sendMessage({type: "TRIGGER", data: {trigger: "firstRun"}}); + ASRouterUtils.sendMessage({type: "TRIGGER", data: {trigger: {id: "firstRun"}}}); } else { ASRouterUtils.sendMessage({type: "CONNECT_UI_REQUEST", data: {endpoint}}); } diff --git a/content-src/asrouter/schemas/provider-response.schema.json b/content-src/asrouter/schemas/provider-response.schema.json index 159fa1274..bbd9e5808 100644 --- a/content-src/asrouter/schemas/provider-response.schema.json +++ b/content-src/asrouter/schemas/provider-response.schema.json @@ -36,11 +36,27 @@ }, "targeting": { "type": "string", - "description": "a JEXL expression representing targeting information" + "description": "A JEXL expression representing targeting information" }, "trigger": { - "type": "string", - "description": "A string representing what the trigger to show this message is." + "type": "object", + "description": "An action to trigger potentially showing the message", + "properties": { + "id": { + "type": "string", + "description": "A string identifying the trigger action", + "enum": ["firstRun", "openURL"] + }, + "params": { + "type": "array", + "description": "An optional array of string parameters for the trigger action", + "items": { + "type": "string", + "description": "A parameter for the trigger action" + } + } + }, + "required": ["id"] }, "frequency": { "type": "object", diff --git a/karma.mc.config.js b/karma.mc.config.js index 6f443e611..1d63e4839 100644 --- a/karma.mc.config.js +++ b/karma.mc.config.js @@ -140,7 +140,8 @@ module.exports = function(config) { exclude: [ path.resolve("test"), path.resolve("vendor"), - path.resolve("lib/ASRouterTargeting.jsm") + path.resolve("lib/ASRouterTargeting.jsm"), + path.resolve("lib/ASRouterTriggerListeners.jsm") ] } ] diff --git a/lib/ASRouter.jsm b/lib/ASRouter.jsm index fcdb45378..81cc4d182 100644 --- a/lib/ASRouter.jsm +++ b/lib/ASRouter.jsm @@ -12,6 +12,8 @@ const {OnboardingMessageProvider} = ChromeUtils.import("resource://activity-stre ChromeUtils.defineModuleGetter(this, "ASRouterTargeting", "resource://activity-stream/lib/ASRouterTargeting.jsm"); +ChromeUtils.defineModuleGetter(this, "ASRouterTriggerListeners", + "resource://activity-stream/lib/ASRouterTriggerListeners.jsm"); const INCOMING_MESSAGE_NAME = "ASRouter:child-to-parent"; const OUTGOING_MESSAGE_NAME = "ASRouter:parent-to-child"; @@ -143,6 +145,7 @@ class _ASRouter { messages: [], ...initialState }; + this._triggerHandler = this._triggerHandler.bind(this); this.onMessage = this.onMessage.bind(this); } @@ -223,6 +226,21 @@ class _ASRouter { newState.messages = [...newState.messages, ...messages]; } } + + // Some messages have triggers that require us to initalise trigger listeners + const unseenListeners = new Set(ASRouterTriggerListeners.keys()); + for (const {trigger} of newState.messages) { + if (trigger && ASRouterTriggerListeners.has(trigger.id)) { + ASRouterTriggerListeners.get(trigger.id).init(this._triggerHandler, trigger.params); + unseenListeners.delete(trigger.id); + } + } + // We don't need these listeners, but they may have previously been + // initialised, so uninitialise them + for (const triggerID of unseenListeners) { + ASRouterTriggerListeners.get(triggerID).uninit(); + } + await this.setState(newState); await this.cleanupImpressions(); } @@ -261,6 +279,10 @@ class _ASRouter { Services.prefs.removeObserver(provider.endpointPref, this); } }); + // Uninitialise all trigger listeners + for (const listener of ASRouterTriggerListeners.values()) { + listener.uninit(); + } this._resetInitialization(); } @@ -281,9 +303,8 @@ class _ASRouter { this.messageChannel.sendAsyncMessage(OUTGOING_MESSAGE_NAME, {type: "ADMIN_SET_STATE", data: state}); } - async _findMessage(messages, target, data = {}) { + async _findMessage(messages, target, trigger) { let message; - const {trigger} = data; const {impressions} = this.state; if (trigger) { // Find a message that matches the targeting context as well as the trigger context @@ -300,7 +321,7 @@ class _ASRouter { return bundle.sort((a, b) => a.order - b.order); } - async _getBundledMessages(originalMessage, target, data, force = false) { + async _getBundledMessages(originalMessage, target, trigger, force = false) { let result = [{content: originalMessage.content, id: originalMessage.id, order: originalMessage.order || 0}]; // First, find all messages of same template. These are potential matching targeting candidates @@ -319,7 +340,7 @@ class _ASRouter { } else { while (bundledMessagesOfSameTemplate.length) { // Find a message that matches the targeting context - or break if there are no matching messages - const message = await this._findMessage(bundledMessagesOfSameTemplate, target, data); + const message = await this._findMessage(bundledMessagesOfSameTemplate, target, trigger); if (!message) { /* istanbul ignore next */ // Code coverage in mochitests break; @@ -348,11 +369,11 @@ class _ASRouter { return state.messages.filter(item => !state.blockList.includes(item.id)); } - async _sendMessageToTarget(message, target, data, force = false) { + async _sendMessageToTarget(message, target, trigger, force = false) { let bundledMessages; // If this message needs to be bundled with other messages of the same template, find them and bundle them together if (message && message.bundled) { - bundledMessages = await this._getBundledMessages(message, target, data, force); + bundledMessages = await this._getBundledMessages(message, target, trigger, force); } if (message && !message.bundled) { // If we only need to send 1 message, send the message @@ -434,8 +455,7 @@ class _ASRouter { }); } - async sendNextMessage(target, action = {}) { - let {data} = action; + async sendNextMessage(target, trigger) { const msgs = this._getUnblockedMessages(); let message = null; const previewMsgs = this.state.messages.filter(item => item.provider === "preview"); @@ -443,11 +463,11 @@ class _ASRouter { if (previewMsgs.length) { [message] = previewMsgs; } else { - message = await this._findMessage(msgs, target, data); + message = await this._findMessage(msgs, target, trigger); } await this.setState({lastMessageId: message ? message.id : null}); - await this._sendMessageToTarget(message, target, data); + await this._sendMessageToTarget(message, target, trigger); } async setMessageById(id, target, force = true, action = {}) { @@ -522,6 +542,11 @@ class _ASRouter { }, {...DEFAULT_WHITELIST_HOSTS}); } + // To be passed to ASRouterTriggerListeners + async _triggerHandler(target, trigger) { + await this.onMessage({target, data: {type: "TRIGGER", trigger}}); + } + async _addPreviewEndpoint(url) { const providers = [...this.state.providers]; if (this._validPreviewEndpoint(url) && !providers.find(p => p.url === url)) { @@ -543,7 +568,7 @@ class _ASRouter { } // Check if any updates are needed first await this.loadMessagesFromAllProviders(); - await this.sendNextMessage(target, action); + await this.sendNextMessage(target, (action.data && action.data.trigger) || {}); break; case ra.OPEN_PRIVATE_BROWSER_WINDOW: // Forcefully open about:privatebrowsing diff --git a/lib/ASRouterTargeting.jsm b/lib/ASRouterTargeting.jsm index 5f4ae0bbd..78e6cb17f 100644 --- a/lib/ASRouterTargeting.jsm +++ b/lib/ASRouterTargeting.jsm @@ -145,6 +145,15 @@ this.ASRouterTargeting = { return FilterExpressions.eval(filterExpression, context); }, + isTriggerMatch(trigger = {}, candidateMessageTrigger = {}) { + if (trigger.id !== candidateMessageTrigger.id) { + return false; + } else if (!candidateMessageTrigger.params) { + return true; + } + return candidateMessageTrigger.params.includes(trigger.param); + }, + isBelowFrequencyCap(message, impressionsForMessage) { if (!message.frequency || !impressionsForMessage || !impressionsForMessage.length) { return true; @@ -202,6 +211,9 @@ this.ASRouterTargeting = { }, async findMatchingMessageWithTrigger({messages, impressions = {}, target, trigger, context}) { + if (!trigger) { + return null; + } const arrayOfItems = [...messages]; let match; let candidate; @@ -209,8 +221,8 @@ this.ASRouterTargeting = { candidate = removeRandomItemFromArray(arrayOfItems); if ( candidate && + this.isTriggerMatch(trigger, candidate.trigger) && this.isBelowFrequencyCap(candidate, impressions[candidate.id]) && - candidate.trigger === trigger && (!candidate.targeting || await this.isMatch(candidate.targeting, target, context)) ) { match = candidate; diff --git a/lib/ASRouterTriggerListeners.jsm b/lib/ASRouterTriggerListeners.jsm new file mode 100644 index 000000000..7241ec87e --- /dev/null +++ b/lib/ASRouterTriggerListeners.jsm @@ -0,0 +1,118 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +"use strict"; + +ChromeUtils.import("resource://gre/modules/Services.jsm"); + +ChromeUtils.defineModuleGetter(this, "PrivateBrowsingUtils", + "resource://gre/modules/PrivateBrowsingUtils.jsm"); + +/** + * A Map from trigger IDs to singleton trigger listeners. Each listener must + * have idempotent `init` and `uninit` methods. + */ +this.ASRouterTriggerListeners = new Map([ + + /** + * Attach listeners to every browser window to detect location changes, and + * notify the trigger handler whenever we navigate to a URL with a hostname + * we're looking for. + */ + ["openURL", { + _initialized: false, + _triggerHandler: null, + _hosts: null, + + /* + * If the listener is already initialised, `init` will replace the trigger + * handler and add any new hosts to `this._hosts`. + */ + init(triggerHandler, hosts = []) { + if (!this._initialized) { + this.onLocationChange = this.onLocationChange.bind(this); + + // Listen for new windows being opened + Services.ww.registerNotification(this); + + // Add listeners to all existing browser windows + const winEnum = Services.wm.getEnumerator("navigator:browser"); + while (winEnum.hasMoreElements()) { + let win = winEnum.getNext(); + if (win.closed || PrivateBrowsingUtils.isWindowPrivate(win)) { + continue; + } + win.gBrowser.addTabsProgressListener(this); + } + + this._initialized = true; + } + this._triggerHandler = triggerHandler; + if (this._hosts) { + hosts.forEach(h => this._hosts.add(h)); + } else { + this._hosts = new Set(hosts); // Clone the hosts to avoid unexpected behaviour + } + }, + + uninit() { + if (this._initialized) { + Services.ww.unregisterNotification(this); + + const winEnum = Services.wm.getEnumerator("navigator:browser"); + while (winEnum.hasMoreElements()) { + let win = winEnum.getNext(); + if (win.closed || PrivateBrowsingUtils.isWindowPrivate(win)) { + continue; + } + + win.gBrowser.removeTabsProgressListener(this); + } + + this._initialized = false; + this._triggerHandler = null; + this._hosts = null; + } + }, + + onLocationChange(aBrowser, aWebProgress, aRequest, aLocationURI, aFlags) { + const location = aLocationURI ? aLocationURI.spec : ""; + if (location && aWebProgress.isTopLevel) { + try { + const host = (new URL(location)).hostname; + if (this._hosts.has(host)) { + this._triggerHandler(aBrowser.messageManager, {id: "openURL", param: host}); + } + } catch (e) {} // Couldn't parse location URL + } + }, + + observe(win, topic, data) { + let onLoad; + + switch (topic) { + case "domwindowopened": + if (!(win instanceof Ci.nsIDOMWindow) || win.closed || PrivateBrowsingUtils.isWindowPrivate(win)) { + break; + } + onLoad = () => { + // Ignore non-browser windows. + if (win.document.documentElement.getAttribute("windowtype") === "navigator:browser") { + win.gBrowser.addTabsProgressListener(this); + } + }; + win.addEventListener("load", onLoad, {once: true}); + break; + + case "domwindowclosed": + if ((win instanceof Ci.nsIDOMWindow) && + win.document.documentElement.getAttribute("windowtype") === "navigator:browser") { + win.gBrowser.removeTabsProgressListener(this); + } + break; + } + } + }] +]); + +const EXPORTED_SYMBOLS = ["ASRouterTriggerListeners"]; diff --git a/lib/OnboardingMessageProvider.jsm b/lib/OnboardingMessageProvider.jsm index d220694ba..c597885c0 100644 --- a/lib/OnboardingMessageProvider.jsm +++ b/lib/OnboardingMessageProvider.jsm @@ -16,7 +16,7 @@ const ONBOARDING_MESSAGES = [ button_label: "Try It Now", button_action: "OPEN_PRIVATE_BROWSER_WINDOW" }, - trigger: "firstRun" + trigger: {id: "firstRun"} }, { id: "ONBOARDING_2", @@ -31,7 +31,7 @@ const ONBOARDING_MESSAGES = [ button_action: "OPEN_URL", button_action_params: "https://screenshots.firefox.com/#tour" }, - trigger: "firstRun" + trigger: {id: "firstRun"} }, { id: "ONBOARDING_3", @@ -47,7 +47,7 @@ const ONBOARDING_MESSAGES = [ button_action_params: "addons" }, targeting: "isInExperimentCohort == 1", - trigger: "firstRun" + trigger: {id: "firstRun"} }, { id: "ONBOARDING_4", @@ -63,7 +63,7 @@ const ONBOARDING_MESSAGES = [ button_action_params: "https://addons.mozilla.org/en-US/firefox/addon/ghostery/" }, targeting: "isInExperimentCohort == 2", - trigger: "firstRun" + trigger: {id: "firstRun"} } ]; diff --git a/test/browser/browser.ini b/test/browser/browser.ini index 6c109a579..65b00f835 100644 --- a/test/browser/browser.ini +++ b/test/browser/browser.ini @@ -9,6 +9,7 @@ prefs = [browser_as_load_location.js] [browser_as_render.js] [browser_asrouter_targeting.js] +[browser_asrouter_trigger_listeners.js] [browser_enabled_newtabpage.js] [browser_highlights_section.js] [browser_getScreenshots.js] diff --git a/test/browser/browser_asrouter_trigger_listeners.js b/test/browser/browser_asrouter_trigger_listeners.js new file mode 100644 index 000000000..0a627fb8b --- /dev/null +++ b/test/browser/browser_asrouter_trigger_listeners.js @@ -0,0 +1,58 @@ +ChromeUtils.defineModuleGetter(this, "ASRouterTriggerListeners", + "resource://activity-stream/lib/ASRouterTriggerListeners.jsm"); +ChromeUtils.import("resource://gre/modules/Services.jsm"); +ChromeUtils.defineModuleGetter(this, "TestUtils", + "resource://testing-common/TestUtils.jsm"); +ChromeUtils.defineModuleGetter(this, "PrivateBrowsingUtils", + "resource://gre/modules/PrivateBrowsingUtils.jsm"); + +async function openURLInWindow(window, url) { + const {selectedBrowser} = window.gBrowser; + await BrowserTestUtils.loadURI(selectedBrowser, url); + await BrowserTestUtils.browserLoaded(selectedBrowser); +} + +add_task(async function check_openURL_listener() { + const TEST_URL = "https://example.com/browser/browser/components/newtab/test/browser/blue_page.html"; + + let urlVisitCount = 0; + const triggerHandler = () => urlVisitCount++; + + const normalWindow = await BrowserTestUtils.openNewBrowserWindow(); + const privateWindow = await BrowserTestUtils.openNewBrowserWindow({private: true}); + + // Initialise listener + const openURLListener = ASRouterTriggerListeners.get("openURL"); + openURLListener.init(triggerHandler, ["example.com"]); + + await openURLInWindow(normalWindow, TEST_URL); + is(urlVisitCount, 1, "should receive page visits from existing windows"); + + await openURLInWindow(normalWindow, "http://www.example.com/abc"); + is(urlVisitCount, 1, "should not receive page visits for different domains"); + + await openURLInWindow(privateWindow, TEST_URL); + is(urlVisitCount, 1, "should not receive page visits from existing private windows"); + + const secondNormalWindow = await BrowserTestUtils.openNewBrowserWindow(); + await openURLInWindow(secondNormalWindow, TEST_URL); + is(urlVisitCount, 2, "should receive page visits from newly opened windows"); + + const secondPrivateWindow = await BrowserTestUtils.openNewBrowserWindow({private: true}); + await openURLInWindow(secondPrivateWindow, TEST_URL); + is(urlVisitCount, 2, "should not receive page visits from newly opened private windows"); + + // Uninitialise listener + openURLListener.uninit(); + + await openURLInWindow(normalWindow, TEST_URL); + is(urlVisitCount, 2, "should now not receive page visits from existing windows"); + + const thirdNormalWindow = await BrowserTestUtils.openNewBrowserWindow(); + await openURLInWindow(thirdNormalWindow, TEST_URL); + is(urlVisitCount, 2, "should now not receive page visits from newly opened windows"); + + // Cleanup + const windows = [normalWindow, privateWindow, secondNormalWindow, secondPrivateWindow, thirdNormalWindow]; + await Promise.all(windows.map(win => BrowserTestUtils.closeWindow(win))); +}); diff --git a/test/unit/asrouter/ASRouter.test.js b/test/unit/asrouter/ASRouter.test.js index d10a424c6..2333f38b1 100644 --- a/test/unit/asrouter/ASRouter.test.js +++ b/test/unit/asrouter/ASRouter.test.js @@ -8,6 +8,7 @@ import { FakeRemotePageManager, PARENT_TO_CHILD_MESSAGE_NAME } from "./constants"; +import {ASRouterTriggerListeners} from "lib/ASRouterTriggerListeners.jsm"; const FAKE_PROVIDERS = [FAKE_LOCAL_PROVIDER, FAKE_REMOTE_PROVIDER]; const ALL_MESSAGE_IDS = [...FAKE_LOCAL_MESSAGES, ...FAKE_REMOTE_MESSAGES].map(message => message.id); @@ -207,6 +208,25 @@ describe("ASRouter", () => { // These are the local messages that should not have been deleted assertRouterContainsMessages(FAKE_LOCAL_MESSAGES); }); + it("should parse the triggers in the messages and register the trigger listeners", async () => { + sandbox.spy(ASRouterTriggerListeners.get("openURL"), "init"); + + /* eslint-disable object-curly-newline */ /* eslint-disable object-property-newline */ + await createRouterAndInit([ + {id: "foo", type: "local", messages: [ + {id: "foo", template: "simple_template", trigger: {id: "firstRun"}, content: {title: "Foo", body: "Foo123"}}, + {id: "bar1", template: "simple_template", trigger: {id: "openURL", params: ["www.mozilla.org", "www.mozilla.com"]}, content: {title: "Bar1", body: "Bar123"}}, + {id: "bar2", template: "simple_template", trigger: {id: "openURL", params: ["www.example.com"]}, content: {title: "Bar2", body: "Bar123"}} + ]} + ]); + /* eslint-enable object-curly-newline */ /* eslint-enable object-property-newline */ + + assert.calledTwice(ASRouterTriggerListeners.get("openURL").init); + assert.calledWithExactly(ASRouterTriggerListeners.get("openURL").init, + Router._triggerHandler, ["www.mozilla.org", "www.mozilla.com"]); + assert.calledWithExactly(ASRouterTriggerListeners.get("openURL").init, + Router._triggerHandler, ["www.example.com"]); + }); }); describe("blocking", () => { @@ -240,6 +260,17 @@ describe("ASRouter", () => { assert.calledWith(channel.removeMessageListener, CHILD_TO_PARENT_MESSAGE_NAME, listenerAdded); }); + it("should unregister the trigger listeners", () => { + for (const listener of ASRouterTriggerListeners.values()) { + sandbox.spy(listener, "uninit"); + } + + Router.uninit(); + + for (const listener of ASRouterTriggerListeners.values()) { + assert.calledOnce(listener.uninit); + } + }); }); describe("#onMessage: CONNECT_UI_REQUEST", () => { @@ -410,7 +441,7 @@ describe("ASRouter", () => { await Router.onMessage(msg); assert.calledOnce(Router.sendNextMessage); - assert.calledWithExactly(Router.sendNextMessage, sinon.match.instanceOf(FakeRemotePageManager), {type: "CONNECT_UI_REQUEST"}); + assert.calledWithExactly(Router.sendNextMessage, sinon.match.instanceOf(FakeRemotePageManager), {}); }); it("should call sendNextMessage on GET_NEXT_MESSAGE", async () => { sandbox.stub(Router, "sendNextMessage").resolves(); @@ -419,7 +450,7 @@ describe("ASRouter", () => { await Router.onMessage(msg); assert.calledOnce(Router.sendNextMessage); - assert.calledWithExactly(Router.sendNextMessage, sinon.match.instanceOf(FakeRemotePageManager), {type: "GET_NEXT_MESSAGE"}); + assert.calledWithExactly(Router.sendNextMessage, sinon.match.instanceOf(FakeRemotePageManager), {}); }); it("should return the preview message if that's available", async () => { const expectedObj = {provider: "preview"}; @@ -480,30 +511,30 @@ describe("ASRouter", () => { describe("#onMessage: TRIGGER", () => { it("should pass the trigger to ASRouterTargeting on TRIGGER message", async () => { sandbox.stub(Router, "_findMessage").resolves(); - const msg = fakeAsyncMessage({type: "TRIGGER", data: {trigger: "firstRun"}}); + const msg = fakeAsyncMessage({type: "TRIGGER", data: {trigger: {id: "firstRun"}}}); await Router.onMessage(msg); assert.calledOnce(Router._findMessage); - assert.deepEqual(Router._findMessage.firstCall.args[2], {trigger: "firstRun"}); + assert.deepEqual(Router._findMessage.firstCall.args[2], {id: "firstRun"}); }); it("consider the trigger when picking a message", async () => { let messages = [ - {id: "foo1", template: "simple_template", bundled: 1, trigger: "foo", content: {title: "Foo1", body: "Foo123-1"}} + {id: "foo1", template: "simple_template", bundled: 1, trigger: {id: "foo"}, content: {title: "Foo1", body: "Foo123-1"}} ]; - const {target, data} = fakeAsyncMessage({type: "TRIGGER", data: {trigger: "foo"}}); - let message = await Router._findMessage(messages, target, data.data); + const {target, data} = fakeAsyncMessage({type: "TRIGGER", data: {trigger: {id: "foo"}}}); + let message = await Router._findMessage(messages, target, data.data.trigger); assert.equal(message, messages[0]); }); it("should pick a message with the right targeting and trigger", async () => { let messages = [ - {id: "foo1", template: "simple_template", bundled: 2, trigger: "foo", content: {title: "Foo1", body: "Foo123-1"}}, - {id: "foo2", template: "simple_template", bundled: 2, trigger: "bar", content: {title: "Foo2", body: "Foo123-2"}}, - {id: "foo3", template: "simple_template", bundled: 2, trigger: "foo", content: {title: "Foo3", body: "Foo123-3"}} + {id: "foo1", template: "simple_template", bundled: 2, trigger: {id: "foo"}, content: {title: "Foo1", body: "Foo123-1"}}, + {id: "foo2", template: "simple_template", bundled: 2, trigger: {id: "bar"}, content: {title: "Foo2", body: "Foo123-2"}}, + {id: "foo3", template: "simple_template", bundled: 2, trigger: {id: "foo"}, content: {title: "Foo3", body: "Foo123-3"}} ]; await Router.setState({messages}); - const {target, data} = fakeAsyncMessage({type: "TRIGGER", data: {trigger: "foo"}}); - let {bundle} = await Router._getBundledMessages(messages[0], target, data.data); + const {target, data} = fakeAsyncMessage({type: "TRIGGER", data: {trigger: {id: "foo"}}}); + let {bundle} = await Router._getBundledMessages(messages[0], target, data.data.trigger); assert.equal(bundle.length, 2); // it should have picked foo1 and foo3 only assert.isTrue(bundle.every(elem => elem.id === "foo1" || elem.id === "foo3")); @@ -569,6 +600,17 @@ describe("ASRouter", () => { }); }); + describe("_triggerHandler", () => { + it("should call #onMessage with the correct trigger", () => { + sinon.spy(Router, "onMessage"); + const target = {}; + const trigger = {id: "FAKE_TRIGGER", param: "some fake param"}; + Router._triggerHandler(target, trigger); + assert.calledOnce(Router.onMessage); + assert.calledWithExactly(Router.onMessage, {target, data: {type: "TRIGGER", trigger}}); + }); + }); + describe("valid preview endpoint", () => { it("should report an error if url protocol is not https", () => { sandbox.stub(Cu, "reportError"); diff --git a/test/unit/asrouter/ASRouterTriggerListeners.test.js b/test/unit/asrouter/ASRouterTriggerListeners.test.js new file mode 100644 index 000000000..75eb723a2 --- /dev/null +++ b/test/unit/asrouter/ASRouterTriggerListeners.test.js @@ -0,0 +1,133 @@ +import {ASRouterTriggerListeners} from "lib/ASRouterTriggerListeners.jsm"; + +describe("ASRouterTriggerListeners", () => { + let sandbox; + let registerWindowNotificationStub; + let unregisterWindoNotificationStub; + let windowEnumeratorStub; + let existingWindow; + const triggerHandler = () => {}; + const openURLListener = ASRouterTriggerListeners.get("openURL"); + const hosts = ["www.mozilla.com", "www.mozilla.org"]; + + function resetEnumeratorStub(windows) { + windowEnumeratorStub + .withArgs("navigator:browser") + .returns({ + _count: -1, + hasMoreElements() { this._count++; return this._count < windows.length; }, + getNext() { return windows[this._count]; } + }); + } + + beforeEach(async () => { + sandbox = sinon.sandbox.create(); + registerWindowNotificationStub = sandbox.stub(global.Services.ww, "registerNotification"); + unregisterWindoNotificationStub = sandbox.stub(global.Services.ww, "unregisterNotification"); + existingWindow = {gBrowser: {addTabsProgressListener: sandbox.stub(), removeTabsProgressListener: sandbox.stub()}}; + windowEnumeratorStub = sandbox.stub(global.Services.wm, "getEnumerator"); + resetEnumeratorStub([existingWindow]); + sandbox.spy(openURLListener, "init"); + sandbox.spy(openURLListener, "uninit"); + }); + afterEach(() => { + sandbox.restore(); + }); + + describe("openURL listener", () => { + it("should exist and initially be uninitialised", () => { + assert.ok(openURLListener); + assert.notOk(openURLListener._initialized); + }); + + describe("#init", () => { + beforeEach(() => { + openURLListener.init(triggerHandler, hosts); + }); + afterEach(() => { + openURLListener.uninit(); + }); + + it("should set ._initialized to true and save the triggerHandler and hosts", () => { + assert.ok(openURLListener._initialized); + assert.deepEqual(openURLListener._hosts, new Set(hosts)); + assert.equal(openURLListener._triggerHandler, triggerHandler); + }); + + it("should register an open-window notification", () => { + assert.calledOnce(registerWindowNotificationStub); + assert.calledWith(registerWindowNotificationStub, openURLListener); + }); + + it("should add tab progress listeners to all existing browser windows", () => { + assert.calledOnce(existingWindow.gBrowser.addTabsProgressListener); + assert.calledWithExactly(existingWindow.gBrowser.addTabsProgressListener, openURLListener); + }); + + it("if already initialised, should only update the trigger handler and add the new hosts", () => { + const newHosts = ["www.example.com"]; + const newTriggerHandler = () => {}; + resetEnumeratorStub([existingWindow]); + registerWindowNotificationStub.reset(); + existingWindow.gBrowser.addTabsProgressListener.reset(); + + openURLListener.init(newTriggerHandler, newHosts); + assert.ok(openURLListener._initialized); + assert.deepEqual(openURLListener._hosts, new Set([...hosts, ...newHosts])); + assert.equal(openURLListener._triggerHandler, newTriggerHandler); + assert.notCalled(registerWindowNotificationStub); + assert.notCalled(existingWindow.gBrowser.addTabsProgressListener); + }); + }); + + describe("#uninit", () => { + beforeEach(() => { + openURLListener.init(triggerHandler, hosts); + // Ensure that the window enumerator will return the existing window for uninit as well + resetEnumeratorStub([existingWindow]); + openURLListener.uninit(); + }); + + it("should set ._initialized to false and clear the triggerHandler and hosts", () => { + assert.notOk(openURLListener._initialized); + assert.equal(openURLListener._hosts, null); + assert.equal(openURLListener._triggerHandler, null); + }); + + it("should remove an open-window notification", () => { + assert.calledOnce(unregisterWindoNotificationStub); + assert.calledWith(unregisterWindoNotificationStub, openURLListener); + }); + + it("should remove tab progress listeners from all existing browser windows", () => { + assert.calledOnce(existingWindow.gBrowser.removeTabsProgressListener); + assert.calledWithExactly(existingWindow.gBrowser.removeTabsProgressListener, openURLListener); + }); + + it("should do nothing if already uninitialised", () => { + unregisterWindoNotificationStub.reset(); + existingWindow.gBrowser.removeTabsProgressListener.reset(); + resetEnumeratorStub([existingWindow]); + + openURLListener.uninit(); + assert.notOk(openURLListener._initialized); + assert.notCalled(unregisterWindoNotificationStub); + assert.notCalled(existingWindow.gBrowser.removeTabsProgressListener); + }); + }); + + describe("#onLocationChange", () => { + it("should call the ._triggerHandler with the right arguments", () => { + const newTriggerHandler = sinon.stub(); + openURLListener.init(newTriggerHandler, hosts); + + const browser = {messageManager: {}}; + const webProgress = {isTopLevel: true}; + const location = "https://www.mozilla.org/something"; + openURLListener.onLocationChange(browser, webProgress, undefined, {spec: location}); + assert.calledOnce(newTriggerHandler); + assert.calledWithExactly(newTriggerHandler, browser.messageManager, {id: "openURL", param: "www.mozilla.org"}); + }); + }); + }); +}); diff --git a/test/unit/unit-entry.js b/test/unit/unit-entry.js index 92b470e24..93a325959 100644 --- a/test/unit/unit-entry.js +++ b/test/unit/unit-entry.js @@ -102,6 +102,7 @@ const TEST_GLOBAL = { }, PluralForm: {get() {}}, Preferences: FakePrefs, + PrivateBrowsingUtils: {isWindowPrivate: () => false}, DownloadsViewUI: {DownloadElementShell}, Services: { locale: { @@ -178,7 +179,8 @@ const TEST_GLOBAL = { createNullPrincipal() {}, getSystemPrincipal() {} }, - wm: {getMostRecentWindow: () => window}, + wm: {getMostRecentWindow: () => window, getEnumerator: () => ({hasMoreElements: () => false})}, + ww: {registerNotification() {}, unregisterNotification() {}}, appinfo: {appBuildID: "20180710100040"} }, XPCOMUtils: {