Bug 1615685 - Trigger Sync CFR when a non-FxA user updates a saved password from doorhanger r=andreio

Differential Revision: https://phabricator.services.mozilla.com/D65034

--HG--
extra : moz-landing-system : lando

browser/components/newtab/lib/ASRouterTriggerListeners.jsm

@@ -396,8 +396,8 @@ this.ASRouterTriggerListeners = new Map([
   ],
 
   /**
-   * Add an observer notification to notify the trigger handler whenever the user saves a new login
-   * via the login capture doorhanger.
+   * Add an observer notification to notify the trigger handler whenever the user
+   * saves or updates a login via the login capture doorhanger.
    */
   [
     "newSavedLogin",
@@ -412,6 +412,7 @@ this.ASRouterTriggerListeners = new Map([
       init(triggerHandler) {
         if (!this._initialized) {
           Services.obs.addObserver(this, "LoginStats:NewSavedPassword");
+          Services.obs.addObserver(this, "LoginStats:LoginUpdateSaved");
           this._initialized = true;
         }
         this._triggerHandler = triggerHandler;
@@ -420,6 +421,7 @@ this.ASRouterTriggerListeners = new Map([
       uninit() {
         if (this._initialized) {
           Services.obs.removeObserver(this, "LoginStats:NewSavedPassword");
+          Services.obs.removeObserver(this, "LoginStats:LoginUpdateSaved");
 
           this._initialized = false;
           this._triggerHandler = null;
@@ -433,7 +435,26 @@ this.ASRouterTriggerListeners = new Map([
           // to enable Sync during the sign up process is a bad UX.
           return;
         }
-        this._triggerHandler(aSubject, { id: "newSavedLogin" });
+
+        switch (aTopic) {
+          case "LoginStats:NewSavedPassword": {
+            this._triggerHandler(aSubject, {
+              id: "newSavedLogin",
+              context: { type: "save" },
+            });
+            break;
+          }
+          case "LoginStats:LoginUpdateSaved": {
+            this._triggerHandler(aSubject, {
+              id: "newSavedLogin",
+              context: { type: "update" },
+            });
+            break;
+          }
+          default: {
+            throw new Error(`Unexpected observer notification: ${aTopic}`);
+          }
+        }
       },
     },
   ],

browser/components/newtab/lib/CFRMessageProvider.jsm

@@ -547,9 +547,9 @@ const CFR_MESSAGES = [
     frequency: {
       lifetime: 3,
     },
-    targeting: "usesFirefoxSync == false",
+    targeting:
+      "(!type || type == 'save') && isFxAEnabled == true && usesFirefoxSync == false",
     template: "cfr_doorhanger",
-    last_modified: 1565907636313,
     content: {
       layout: "icon_and_message",
       text: {
@@ -616,6 +616,80 @@ const CFR_MESSAGES = [
       id: "newSavedLogin",
     },
   },
+  {
+    id: "UPDATE_LOGIN",
+    frequency: {
+      lifetime: 3,
+    },
+    targeting:
+      "type == 'update' && isFxAEnabled == true && usesFirefoxSync == false",
+    template: "cfr_doorhanger",
+    content: {
+      layout: "icon_and_message",
+      text: {
+        string_id: "cfr-doorhanger-sync-logins-body",
+      },
+      icon: "chrome://browser/content/aboutlogins/icons/intro-illustration.svg",
+      icon_class: "cfr-doorhanger-large-icon",
+      buttons: {
+        secondary: [
+          {
+            label: {
+              string_id: "cfr-doorhanger-extension-cancel-button",
+            },
+            action: {
+              type: "CANCEL",
+            },
+          },
+          {
+            label: {
+              string_id: "cfr-doorhanger-extension-never-show-recommendation",
+            },
+          },
+          {
+            label: {
+              string_id: "cfr-doorhanger-extension-manage-settings-button",
+            },
+            action: {
+              type: "OPEN_PREFERENCES_PAGE",
+              data: {
+                category: "general-cfrfeatures",
+              },
+            },
+          },
+        ],
+        primary: {
+          label: {
+            string_id: "cfr-doorhanger-sync-logins-ok-button",
+          },
+          action: {
+            type: "OPEN_PREFERENCES_PAGE",
+            data: {
+              category: "sync",
+              entrypoint: "cfr-update-login",
+            },
+          },
+        },
+      },
+      bucket_id: "CFR_UPDATE_LOGIN",
+      heading_text: {
+        string_id: "cfr-doorhanger-sync-logins-header",
+      },
+      info_icon: {
+        label: {
+          string_id: "cfr-doorhanger-extension-sumo-link",
+        },
+        sumo_path: "extensionrecommendations",
+      },
+      notification_text: {
+        string_id: "cfr-doorhanger-feature-notification",
+      },
+      category: "cfrFeatures",
+    },
+    trigger: {
+      id: "newSavedLogin",
+    },
+  },
   {
     id: "SOCIAL_TRACKING_PROTECTION",
     template: "cfr_doorhanger",

browser/components/newtab/test/browser/browser_asrouter_trigger_listeners.js

@@ -162,12 +162,18 @@ add_task(async function check_openURL_listener() {
   await Promise.all(windows.map(win => BrowserTestUtils.closeWindow(win)));
 });
 
-add_task(async function check_newSavedLogin_listener() {
+add_task(async function check_newSavedLogin_save_listener() {
   const TEST_URL =
     "https://example.com/browser/browser/components/newtab/test/browser/red_page.html";
 
-  let loginsSaved = 0;
-  const triggerHandler = () => loginsSaved++;
+  let triggerTypesHandled = {
+    save: 0,
+    update: 0,
+  };
+  const triggerHandler = (sub, { id, context }) => {
+    is(id, "newSavedLogin", "Check trigger id");
+    triggerTypesHandled[context.type]++;
+  };
   const newSavedLoginListener = ASRouterTriggerListeners.get("newSavedLogin");
 
   // Previously initialized by the Router
@@ -181,13 +187,15 @@ add_task(async function check_newSavedLogin_listener() {
     async function triggerNewSavedPassword(browser) {
       Services.obs.notifyObservers(browser, "LoginStats:NewSavedPassword");
       await BrowserTestUtils.waitForCondition(
-        () => loginsSaved !== 0,
+        () => triggerTypesHandled.save !== 0,
         "Wait for the observer notification to run"
       );
-      is(loginsSaved, 1, "should receive observer notification");
+      is(triggerTypesHandled.save, 1, "should receive observer notification");
     }
   );
 
+  is(triggerTypesHandled.update, 0, "shouldn't have handled other trigger");
+
   // Uninitialise listener
   newSavedLoginListener.uninit();
 
@@ -196,7 +204,62 @@ add_task(async function check_newSavedLogin_listener() {
     async function triggerNewSavedPasswordAfterUninit(browser) {
       Services.obs.notifyObservers(browser, "LoginStats:NewSavedPassword");
       await new Promise(resolve => executeSoon(resolve));
-      is(loginsSaved, 1, "shouldn't receive obs. notification after uninit");
+      is(
+        triggerTypesHandled.save,
+        1,
+        "shouldn't receive obs. notification after uninit"
+      );
+    }
+  );
+});
+
+add_task(async function check_newSavedLogin_update_listener() {
+  const TEST_URL =
+    "https://example.com/browser/browser/components/newtab/test/browser/red_page.html";
+
+  let triggerTypesHandled = {
+    save: 0,
+    update: 0,
+  };
+  const triggerHandler = (sub, { id, context }) => {
+    is(id, "newSavedLogin", "Check trigger id");
+    triggerTypesHandled[context.type]++;
+  };
+  const newSavedLoginListener = ASRouterTriggerListeners.get("newSavedLogin");
+
+  // Previously initialized by the Router
+  newSavedLoginListener.uninit();
+
+  // Initialise listener
+  await newSavedLoginListener.init(triggerHandler);
+
+  await BrowserTestUtils.withNewTab(
+    TEST_URL,
+    async function triggerLoginUpdateSaved(browser) {
+      Services.obs.notifyObservers(browser, "LoginStats:LoginUpdateSaved");
+      await BrowserTestUtils.waitForCondition(
+        () => triggerTypesHandled.update !== 0,
+        "Wait for the observer notification to run"
+      );
+      is(triggerTypesHandled.update, 1, "should receive observer notification");
+    }
+  );
+
+  is(triggerTypesHandled.save, 0, "shouldn't have handled other trigger");
+
+  // Uninitialise listener
+  newSavedLoginListener.uninit();
+
+  await BrowserTestUtils.withNewTab(
+    TEST_URL,
+    async function triggerLoginUpdateSavedAfterUninit(browser) {
+      Services.obs.notifyObservers(browser, "LoginStats:LoginUpdateSaved");
+      await new Promise(resolve => executeSoon(resolve));
+      is(
+        triggerTypesHandled.update,
+        1,
+        "shouldn't receive obs. notification after uninit"
+      );
     }
   );
 });

browser/components/newtab/test/unit/asrouter/CFRMessageProvider.test.js

@@ -12,7 +12,7 @@ const REGULAR_IDS = [
 
 describe("CFRMessageProvider", () => {
   it("should have a total of 10 messages", () => {
-    assert.lengthOf(messages, 10);
+    assert.lengthOf(messages, 11);
   });
   it("should have one message each for the three regular addons", () => {
     for (const id of REGULAR_IDS) {

toolkit/components/passwordmgr/LoginManagerPrompter.jsm

@@ -383,6 +383,10 @@ class LoginManagerPrompter {
         histogram.add(PROMPT_ADD_OR_UPDATE);
         if (histogramName == "PWMGR_PROMPT_REMEMBER_ACTION") {
           Services.obs.notifyObservers(browser, "LoginStats:NewSavedPassword");
+        } else if (histogramName == "PWMGR_PROMPT_UPDATE_ACTION") {
+          Services.obs.notifyObservers(browser, "LoginStats:LoginUpdateSaved");
+        } else {
+          throw new Error("Unknown histogram");
         }
         readDataFromUI();
         persistData();

toolkit/components/passwordmgr/test/browser/browser_doorhanger_remembering.js

@@ -508,7 +508,12 @@ add_task(async function test_changeUPLoginOnUPForm_change() {
     is(notif.message, "Would you like to update this login?", "Check message");
 
     await checkDoorhangerUsernamePassword("notifyu1", "pass2");
+    let promiseLoginUpdateSaved = TestUtils.topicObserved(
+      "LoginStats:LoginUpdateSaved",
+      (subject, data) => subject == gBrowser.selectedBrowser
+    );
     clickDoorhangerButton(notif, CHANGE_BUTTON);
+    await promiseLoginUpdateSaved;
 
     ok(!getCaptureDoorhanger("password-change"), "popup should be gone");
   });