Bug 1599708 fix setting newtab when setting is updated r=robwu
authorShane Caraveo <scaraveo@mozilla.com>
Wed, 27 Nov 2019 21:51:51 +0000
changeset 504118 3dc2c0a968db4d2b7f9892d98de5819cf9a869d2
parent 504117 051f105b52492dfc6c7cddcaabc5ef8cc0afe05d
child 504119 461d07650b52d3c61d8784b3f2ceb70e8c0dba3d
push id36856
push usercsabou@mozilla.com
push dateThu, 28 Nov 2019 04:36:32 +0000
treeherdermozilla-central@21d90776b745 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrobwu
bugs1599708
milestone72.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 1599708 fix setting newtab when setting is updated r=robwu Differential Revision: https://phabricator.services.mozilla.com/D55022
browser/components/extensions/parent/ext-url-overrides.js
browser/components/extensions/test/xpcshell/test_ext_url_overrides_newtab.js
--- a/browser/components/extensions/parent/ext-url-overrides.js
+++ b/browser/components/extensions/parent/ext-url-overrides.js
@@ -95,34 +95,26 @@ function setNewTabURL(extensionId, url) 
 }
 
 // eslint-disable-next-line mozilla/balanced-listeners
 ExtensionParent.apiManager.on(
   "extension-setting-changed",
   async (eventName, setting) => {
     let extensionId, url;
     if (setting.type === STORE_TYPE && setting.key === NEW_TAB_SETTING_NAME) {
-      let { item = {} } = setting;
-      if (setting.action === "enable") {
-        // If setting.item exists, it is the new value.  If it doesn't exist, and an
-        // extension is being enabled, we use the id.
-        extensionId = item.id || setting.id;
+      // If the actual setting has changed in some way, we will have
+      // setting.item which is what the setting has been changed to.  If
+      // we have an item, we always want to update the newTabUrl values.
+      let { item } = setting;
+      if (item) {
+        // If we're resetting, id will be undefined.
+        extensionId = item.id;
         url = item.value || item.initialValue;
-      } else if (setting.item) {
-        // We're disabling or removing a setting.
-        if (item.id && item.value) {
-          // Another extension is gaining control.
-          extensionId = item.id;
-          url = item.value;
-        } else {
-          // we're resetting to the original value, do not set extensionId.
-          url = item.initialValue;
-        }
+        setNewTabURL(extensionId, url);
       }
-      setNewTabURL(extensionId, url);
     }
   }
 );
 
 this.urlOverrides = class extends ExtensionAPI {
   static async onDisable(id) {
     newTabPopup.clearConfirmation(id);
     await ExtensionSettingsStore.initialize();
--- a/browser/components/extensions/test/xpcshell/test_ext_url_overrides_newtab.js
+++ b/browser/components/extensions/test/xpcshell/test_ext_url_overrides_newtab.js
@@ -53,16 +53,19 @@ add_task(async function test_multiple_ex
   const NEWTAB_URI_3 = "webext-newtab-2.html";
   const EXT_2_ID = "ext2@tests.mozilla.org";
   const EXT_3_ID = "ext3@tests.mozilla.org";
 
   const CONTROLLED_BY_THIS = "controlled_by_this_extension";
   const CONTROLLED_BY_OTHER = "controlled_by_other_extensions";
   const NOT_CONTROLLABLE = "not_controllable";
 
+  const NEW_TAB_PRIVATE_ALLOWED = "browser.newtab.privateAllowed";
+  const NEW_TAB_EXTENSION_CONTROLLED = "browser.newtab.extensionControlled";
+
   function background() {
     browser.test.onMessage.addListener(async msg => {
       switch (msg) {
         case "checkNewTabPage":
           let newTabPage = await browser.browserSettings.newTabPageOverride.get(
             {}
           );
           browser.test.sendMessage("newTabPage", newTabPage);
@@ -105,16 +108,41 @@ add_task(async function test_multiple_ex
     );
     equal(
       newTabPage.levelOfControl,
       expectedLevelOfControl,
       `newTabPageOverride setting returns the expected levelOfControl: ${expectedLevelOfControl}.`
     );
   }
 
+  function verifyNewTabSettings(ext, expectedLevelOfControl) {
+    if (expectedLevelOfControl !== NOT_CONTROLLABLE) {
+      // Verify the preferences are set as expected.
+      let policy = WebExtensionPolicy.getByID(ext.id);
+      equal(
+        policy && policy.privateBrowsingAllowed,
+        Services.prefs.getBoolPref(NEW_TAB_PRIVATE_ALLOWED),
+        "private browsing flag set correctly"
+      );
+      ok(
+        Services.prefs.getBoolPref(NEW_TAB_EXTENSION_CONTROLLED),
+        `extension controlled flag set correctly`
+      );
+    } else {
+      ok(
+        !Services.prefs.prefHasUserValue(NEW_TAB_PRIVATE_ALLOWED),
+        "controlled flag reset"
+      );
+      ok(
+        !Services.prefs.prefHasUserValue(NEW_TAB_EXTENSION_CONTROLLED),
+        "controlled flag reset"
+      );
+    }
+  }
+
   let extObj = {
     manifest: {
       chrome_url_overrides: {},
       permissions: ["browserSettings"],
     },
     useAddonManager: "temporary",
     background,
   };
@@ -122,16 +150,17 @@ add_task(async function test_multiple_ex
   let ext1 = ExtensionTestUtils.loadExtension(extObj);
 
   extObj.manifest.chrome_url_overrides = { newtab: NEWTAB_URI_2 };
   extObj.manifest.applications = { gecko: { id: EXT_2_ID } };
   let ext2 = ExtensionTestUtils.loadExtension(extObj);
 
   extObj.manifest.chrome_url_overrides = { newtab: NEWTAB_URI_3 };
   extObj.manifest.applications.gecko.id = EXT_3_ID;
+  extObj.incognitoOverride = "spanning";
   let ext3 = ExtensionTestUtils.loadExtension(extObj);
 
   equal(
     aboutNewTabService.newTabURL,
     DEFAULT_NEW_TAB_URL,
     "newTabURL is set to the default."
   );
 
@@ -144,106 +173,125 @@ add_task(async function test_multiple_ex
     "newTabURL is still set to the default."
   );
 
   await checkNewTabPageOverride(
     ext1,
     aboutNewTabService.newTabURL,
     NOT_CONTROLLABLE
   );
+  verifyNewTabSettings(ext1, NOT_CONTROLLABLE);
 
   await ext2.startup();
   ok(
     aboutNewTabService.newTabURL.endsWith(NEWTAB_URI_2),
     "newTabURL is overridden by the second extension."
   );
   await checkNewTabPageOverride(ext1, NEWTAB_URI_2, CONTROLLED_BY_OTHER);
+  verifyNewTabSettings(ext2, CONTROLLED_BY_THIS);
 
   // Verify that calling set and clear do nothing.
   ext2.sendMessage("trySet");
   await ext2.awaitMessage("newTabPageSet");
   await checkNewTabPageOverride(ext1, NEWTAB_URI_2, CONTROLLED_BY_OTHER);
+  verifyNewTabSettings(ext2, CONTROLLED_BY_THIS);
 
   ext2.sendMessage("tryClear");
   await ext2.awaitMessage("newTabPageCleared");
   await checkNewTabPageOverride(ext1, NEWTAB_URI_2, CONTROLLED_BY_OTHER);
+  verifyNewTabSettings(ext2, CONTROLLED_BY_THIS);
 
   // Disable the second extension.
   let addon = await AddonManager.getAddonByID(EXT_2_ID);
   let disabledPromise = awaitEvent("shutdown");
   await addon.disable();
   await disabledPromise;
   equal(
     aboutNewTabService.newTabURL,
     DEFAULT_NEW_TAB_URL,
     "newTabURL url is reset to the default after second extension is disabled."
   );
   await checkNewTabPageOverride(
     ext1,
     aboutNewTabService.newTabURL,
     NOT_CONTROLLABLE
   );
+  verifyNewTabSettings(ext1, NOT_CONTROLLABLE);
 
   // Re-enable the second extension.
   let enabledPromise = awaitEvent("ready");
   await addon.enable();
   await enabledPromise;
   ok(
     aboutNewTabService.newTabURL.endsWith(NEWTAB_URI_2),
     "newTabURL is overridden by the second extension."
   );
   await checkNewTabPageOverride(ext2, NEWTAB_URI_2, CONTROLLED_BY_THIS);
+  verifyNewTabSettings(ext2, CONTROLLED_BY_THIS);
 
   await ext1.unload();
   ok(
     aboutNewTabService.newTabURL.endsWith(NEWTAB_URI_2),
     "newTabURL is still overridden by the second extension."
   );
   await checkNewTabPageOverride(ext2, NEWTAB_URI_2, CONTROLLED_BY_THIS);
+  verifyNewTabSettings(ext2, CONTROLLED_BY_THIS);
 
   await ext3.startup();
   ok(
     aboutNewTabService.newTabURL.endsWith(NEWTAB_URI_3),
     "newTabURL is overridden by the third extension."
   );
   await checkNewTabPageOverride(ext2, NEWTAB_URI_3, CONTROLLED_BY_OTHER);
+  verifyNewTabSettings(ext3, CONTROLLED_BY_THIS);
 
   // Disable the second extension.
   disabledPromise = awaitEvent("shutdown");
   await addon.disable();
   await disabledPromise;
   ok(
     aboutNewTabService.newTabURL.endsWith(NEWTAB_URI_3),
     "newTabURL is still overridden by the third extension."
   );
   await checkNewTabPageOverride(ext3, NEWTAB_URI_3, CONTROLLED_BY_THIS);
+  verifyNewTabSettings(ext3, CONTROLLED_BY_THIS);
 
   // Re-enable the second extension.
   enabledPromise = awaitEvent("ready");
   await addon.enable();
   await enabledPromise;
   ok(
     aboutNewTabService.newTabURL.endsWith(NEWTAB_URI_3),
     "newTabURL is still overridden by the third extension."
   );
   await checkNewTabPageOverride(ext3, NEWTAB_URI_3, CONTROLLED_BY_THIS);
+  verifyNewTabSettings(ext3, CONTROLLED_BY_THIS);
 
   await ext3.unload();
   ok(
     aboutNewTabService.newTabURL.endsWith(NEWTAB_URI_2),
     "newTabURL reverts to being overridden by the second extension."
   );
   await checkNewTabPageOverride(ext2, NEWTAB_URI_2, CONTROLLED_BY_THIS);
+  verifyNewTabSettings(ext2, CONTROLLED_BY_THIS);
 
   await ext2.unload();
   equal(
     aboutNewTabService.newTabURL,
     DEFAULT_NEW_TAB_URL,
     "newTabURL url is reset to the default."
   );
+  ok(
+    !Services.prefs.prefHasUserValue(NEW_TAB_PRIVATE_ALLOWED),
+    "controlled flag reset"
+  );
+  ok(
+    !Services.prefs.prefHasUserValue(NEW_TAB_EXTENSION_CONTROLLED),
+    "controlled flag reset"
+  );
 
   await promiseShutdownManager();
 });
 
 // Tests that we handle the upgrade/downgrade process correctly
 // when an extension is installed temporarily on top of a permanently
 // installed one.
 add_task(async function test_temporary_installation() {