Bug 1486931 - Fix CFR admin page CFR and add preview method r=ursula
☠☠ backed out by 21eff49dce24 ☠ ☠
authork88hudson <k88hudson@gmail.com>
Tue, 28 Aug 2018 21:48:29 +0000
changeset 433653 8a25fc40764a60d9dd5a72b710955bf45220f94d
parent 433652 ae13aec18c6c2252c01f44565909014a36a5a544
child 433654 644e800d1d8aa4bf2a4cc560132fc290539c5bea
push id34520
push userebalazs@mozilla.com
push dateWed, 29 Aug 2018 09:42:57 +0000
treeherdermozilla-central@56cfcee29fbc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersursula
bugs1486931
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1486931 - Fix CFR admin page CFR and add preview method r=ursula MozReview-Commit-ID: 1RApm3ApcOe Differential Revision: https://phabricator.services.mozilla.com/D4492
browser/components/newtab/lib/ASRouter.jsm
browser/components/newtab/lib/CFRPageActions.jsm
browser/components/newtab/test/unit/asrouter/ASRouter.test.js
--- a/browser/components/newtab/lib/ASRouter.jsm
+++ b/browser/components/newtab/lib/ASRouter.jsm
@@ -490,17 +490,21 @@ class _ASRouter {
     // For bundled messages, look for the rest of the bundle or else send CLEAR_ALL
     } else if (message.bundled) {
       const bundledMessages = await this._getBundledMessages(message, target, trigger, force);
       const action = bundledMessages ? {type: "SET_BUNDLED_MESSAGES", data: bundledMessages} : {type: "CLEAR_ALL"};
       target.sendAsyncMessage(OUTGOING_MESSAGE_NAME, action);
 
     // CFR doorhanger
     } else if (message.template === "cfr_doorhanger") {
-      CFRPageActions.addRecommendation(target, trigger.param, message, this.dispatch, force);
+      if (force) {
+        CFRPageActions.forceRecommendation(target, message, this.dispatch);
+      } else {
+        CFRPageActions.addRecommendation(target, trigger.param, message, this.dispatch);
+      }
 
     // New tab single messages
     } else {
       target.sendAsyncMessage(OUTGOING_MESSAGE_NAME, {type: "SET_MESSAGE", data: message});
     }
   }
 
   async addImpression(message) {
@@ -616,17 +620,17 @@ class _ASRouter {
     }
     await this._sendMessageToTarget(message, target, trigger);
   }
 
   async setMessageById(id, target, force = true, action = {}) {
     await this.setState({lastMessageId: id});
     const newMessage = this.getMessageById(id);
 
-    await this._sendMessageToTarget(newMessage, target, force, action.data);
+    await this._sendMessageToTarget(newMessage, target, action.data, force);
   }
 
   async blockMessageById(idOrIds) {
     const idsToBlock = Array.isArray(idOrIds) ? idOrIds : [idOrIds];
 
     await this.setState(state => {
       const messageBlockList = [...state.messageBlockList, ...idsToBlock];
       // When a message is blocked, its impressions should be cleared as well
--- a/browser/components/newtab/lib/CFRPageActions.jsm
+++ b/browser/components/newtab/lib/CFRPageActions.jsm
@@ -216,27 +216,45 @@ const CFRPageActions = {
       }
     } else {
       // There's no recommendation specified for this browser, so hide the page action
       pageAction.hide();
     }
   },
 
   /**
+   * Force a recommendation to be shown. Should only happen via the Admin page.
+   * @param browser             The browser for the recommendation
+   * @param recommendation      The recommendation to show
+   * @param dispatchToASRouter  A function to dispatch resulting actions to
+   * @return                    Did adding the recommendation succeed?
+   */
+  async forceRecommendation(browser, recommendation, dispatchToASRouter) {
+    // If we are forcing via the Admin page, the browser comes in a different format
+    const win = browser.browser.ownerGlobal;
+    const {id, content} = recommendation;
+    RecommendationMap.set(browser.browser, {id, content});
+    if (!PageActionMap.has(win)) {
+      PageActionMap.set(win, new PageAction(win, dispatchToASRouter));
+    }
+    await PageActionMap.get(win).show(recommendation.content.notification_text, true);
+    return true;
+  },
+
+  /**
    * Add a recommendation specific to the given browser and host.
    * @param browser             The browser for the recommendation
    * @param host                The host for the recommendation
    * @param recommendation      The recommendation to show
    * @param dispatchToASRouter  A function to dispatch resulting actions to
-   * @param force               Force the recommendation to appear if the host doesn't match
    * @return                    Did adding the recommendation succeed?
    */
-  async addRecommendation(browser, host, recommendation, dispatchToASRouter, force = false) {
+  async addRecommendation(browser, host, recommendation, dispatchToASRouter) {
     const win = browser.ownerGlobal;
-    if (browser !== win.gBrowser.selectedBrowser || !(force || isHostMatch(browser, host))) {
+    if (browser !== win.gBrowser.selectedBrowser || !isHostMatch(browser, host)) {
       return false;
     }
     const {id, content} = recommendation;
     RecommendationMap.set(browser, {id, host, content});
     if (!PageActionMap.has(win)) {
       PageActionMap.set(win, new PageAction(win, dispatchToASRouter));
     }
     await PageActionMap.get(win).show(recommendation.content.notification_text, true);
--- a/browser/components/newtab/test/unit/asrouter/ASRouter.test.js
+++ b/browser/components/newtab/test/unit/asrouter/ASRouter.test.js
@@ -636,24 +636,33 @@ describe("ASRouter", () => {
     it("should broadcast a SET_MESSAGE message to all clients with a particular id", async () => {
       const [testMessage] = Router.state.messages;
       const msg = fakeAsyncMessage({type: "OVERRIDE_MESSAGE", data: {id: testMessage.id}});
       await Router.onMessage(msg);
 
       assert.calledWith(msg.target.sendAsyncMessage, PARENT_TO_CHILD_MESSAGE_NAME, {type: "SET_MESSAGE", data: testMessage});
     });
 
-    it("should call CFRPageActions.addRecommendation if the template is cfr_action", async () => {
-      sandbox.stub(CFRPageActions, "addRecommendation");
+    it("should call CFRPageActions.forceRecommendation if the template is cfr_action and force is true", async () => {
+      sandbox.stub(CFRPageActions, "forceRecommendation");
       const testMessage = {id: "foo", template: "cfr_doorhanger"};
       await Router.setState({messages: [testMessage]});
       const msg = fakeAsyncMessage({type: "OVERRIDE_MESSAGE", data: {id: testMessage.id}});
       await Router.onMessage(msg);
 
       assert.notCalled(msg.target.sendAsyncMessage);
+      assert.calledOnce(CFRPageActions.forceRecommendation);
+    });
+
+    it("should call CFRPageActions.addRecommendation if the template is cfr_action and force is false", async () => {
+      sandbox.stub(CFRPageActions, "addRecommendation");
+      const testMessage = {id: "foo", template: "cfr_doorhanger"};
+      await Router.setState({messages: [testMessage]});
+      await Router._sendMessageToTarget(testMessage, {}, {}, false);
+
       assert.calledOnce(CFRPageActions.addRecommendation);
     });
 
     it("should broadcast CLEAR_ALL if provided id did not resolve to a message", async () => {
       const msg = fakeAsyncMessage({type: "OVERRIDE_MESSAGE", data: {id: -1}});
       await Router.onMessage(msg);
 
       assert.calledWith(msg.target.sendAsyncMessage, PARENT_TO_CHILD_MESSAGE_NAME, {type: "CLEAR_ALL"});