Bug 1580049 fix clearing newtab prefs used by extensions r=robwu a=lizzard
authorShane Caraveo <scaraveo@mozilla.com>
Wed, 11 Sep 2019 13:59:30 +0000
changeset 555236 ded64553b007ef8496356b278f22bef0720b78f2
parent 555235 eaf5ba87c7fcafc0ace60fc7a69f67452032c274
child 555237 e41b9f16b581b3acbe617da2b44c2a6bb6ab5396
push id2165
push userffxbld-merge
push dateMon, 14 Oct 2019 16:30:58 +0000
treeherdermozilla-release@0eae18af659f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrobwu, lizzard
bugs1580049
milestone70.0
Bug 1580049 fix clearing newtab prefs used by extensions r=robwu a=lizzard Differential Revision: https://phabricator.services.mozilla.com/D45279
browser/components/extensions/parent/ext-url-overrides.js
browser/components/extensions/test/browser/browser_ext_url_overrides_newtab.js
--- a/browser/components/extensions/parent/ext-url-overrides.js
+++ b/browser/components/extensions/parent/ext-url-overrides.js
@@ -95,25 +95,35 @@ 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) {
-      if (setting.action === "enable" || setting.item) {
-        let { item } = setting;
+      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 && item.id) || setting.id;
-        url = item && (item.value || item.initialValue);
+        extensionId = item.id || setting.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();
     if (
--- a/browser/components/extensions/test/browser/browser_ext_url_overrides_newtab.js
+++ b/browser/components/extensions/test/browser/browser_ext_url_overrides_newtab.js
@@ -787,8 +787,30 @@ add_task(async function test_overriding_
   // This will show a confirmation doorhanger, make sure we don't leave it open.
   let popupHidden = promisePopupHidden(panel);
   panel.hidePopup();
   await popupHidden;
 
   await extension.unload();
   await BrowserTestUtils.closeWindow(win);
 });
+
+// Test that prefs set by the newtab override code are
+// properly unset when all newtab extensions are gone.
+add_task(async function testNewTabPrefsReset() {
+  function isUndefinedPref(pref) {
+    try {
+      Services.prefs.getBoolPref(pref);
+      return false;
+    } catch (e) {
+      return true;
+    }
+  }
+
+  ok(
+    isUndefinedPref("browser.newtab.extensionControlled"),
+    "extensionControlled pref is not set"
+  );
+  ok(
+    isUndefinedPref("browser.newtab.privateAllowed"),
+    "privateAllowed pref is not set"
+  );
+});