Bug 1488971 Move logic for disabling screenshots out of the extension r=kmag,_6a68
authorAndrew Swan <aswan@mozilla.com>
Fri, 07 Sep 2018 13:43:48 -0700
changeset 436004 3113a83b34557d35bdfb54f08c72c937a0980b38
parent 436003 309d6b502c71a21751333c699d47639545fc53cd
child 436005 84b407430f086cf1f40fcb24e541e5dbbdfcaea1
push id107771
push useraswan@mozilla.com
push dateWed, 12 Sep 2018 20:40:27 +0000
treeherdermozilla-inbound@3113a83b3455 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskmag, _6a68
bugs1488971
milestone64.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 1488971 Move logic for disabling screenshots out of the extension r=kmag,_6a68 Differential Revision: https://phabricator.services.mozilla.com/D5310
browser/base/content/test/performance/browser_preferences_usage.js
browser/components/nsBrowserGlue.js
browser/extensions/screenshots/bootstrap.js
toolkit/mozapps/extensions/internal/XPIDatabase.jsm
--- a/browser/base/content/test/performance/browser_preferences_usage.js
+++ b/browser/base/content/test/performance/browser_preferences_usage.js
@@ -94,16 +94,25 @@ add_task(async function startup() {
     },
     "network.loadinfo.skip_type_assertion": {
       // This is accessed in debug only.
     },
     "extensions.getAddons.cache.enabled": {
       min: 7,
       max: 55,
     },
+
+    // Disabling screenshots in the default test profile triggers some
+    // work in the chrome registry that reads this pref.  This can be removed
+    // when bootstrapped extensions are gone, or even when screenshots
+    // moves away from bootstrap (bug 1422437)
+    "chrome.override_package.global": {
+      min: 0,
+      max: 50,
+    },
   };
 
   let startupRecorder = Cc["@mozilla.org/test/startuprecorder;1"].getService().wrappedJSObject;
   await startupRecorder.done;
 
   ok(startupRecorder.data.prefStats, "startupRecorder has prefStats");
 
   checkPrefGetters(startupRecorder.data.prefStats, max, whitelist);
--- a/browser/components/nsBrowserGlue.js
+++ b/browser/components/nsBrowserGlue.js
@@ -1396,16 +1396,32 @@ BrowserGlue.prototype = {
     // them is collected on all channels.
     if (AppConstants.MOZ_DATA_REPORTING) {
       this.browserErrorReporter.uninit();
     }
 
     Normandy.uninit();
   },
 
+  // Set up a listener to enable/disable the screenshots extension
+  // based on its preference.
+  _monitorScreenshotsPref() {
+    const PREF = "extensions.screenshots.disabled";
+    const ID = "screenshots@mozilla.org";
+    Services.prefs.addObserver(PREF, async () => {
+      let addon = await AddonManager.getAddonByID(ID);
+      let disabled = Services.prefs.getBoolPref(PREF, false);
+      if (disabled) {
+        await addon.disable({allowSystemAddons: true});
+      } else {
+        await addon.enable({allowSystemAddons: true});
+      }
+    });
+  },
+
   // All initial windows have opened.
   _onWindowsRestored: function BG__onWindowsRestored() {
     if (this._windowsWereRestored) {
       return;
     }
     this._windowsWereRestored = true;
 
     // Browser errors are only collected on Nightly, but telemetry for
@@ -1452,16 +1468,18 @@ BrowserGlue.prototype = {
         idleService.removeIdleObserver(this._lateTasksIdleObserver,
                                        LATE_TASKS_IDLE_TIME_SEC);
         delete this._lateTasksIdleObserver;
         this._scheduleArbitrarilyLateIdleTasks();
       }
     };
     this._idleService.addIdleObserver(
       this._lateTasksIdleObserver, LATE_TASKS_IDLE_TIME_SEC);
+
+    this._monitorScreenshotsPref();
   },
 
   /**
    * Use this function as an entry point to schedule tasks that
    * need to run only once after startup, and can be scheduled
    * by using an idle callback.
    *
    * The functions scheduled here will fire from idle callbacks
--- a/browser/extensions/screenshots/bootstrap.js
+++ b/browser/extensions/screenshots/bootstrap.js
@@ -20,35 +20,16 @@ ChromeUtils.defineModuleGetter(this, "Se
 
 let addonResourceURI;
 let appStartupDone;
 let appStartupPromise = new Promise((resolve, reject) => {
   appStartupDone = resolve;
 });
 
 const prefs = Services.prefs;
-const prefObserver = {
-  register() {
-    prefs.addObserver(PREF_BRANCH, this, false); // eslint-disable-line mozilla/no-useless-parameters
-  },
-
-  unregister() {
-    prefs.removeObserver(PREF_BRANCH, this, false); // eslint-disable-line mozilla/no-useless-parameters
-  },
-
-  observe(aSubject, aTopic, aData) {
-    // aSubject is the nsIPrefBranch we're observing (after appropriate QI)
-    // aData is the name of the pref that's been changed (relative to aSubject)
-    if (aData === USER_DISABLE_PREF) {
-      // eslint-disable-next-line promise/catch-or-return
-      appStartupPromise = appStartupPromise.then(handleStartup);
-    }
-  }
-};
-
 
 const appStartupObserver = {
   register() {
     Services.obs.addObserver(this, "sessionstore-windows-restored", false); // eslint-disable-line mozilla/no-useless-parameters
   },
 
   unregister() {
     Services.obs.removeObserver(this, "sessionstore-windows-restored", false); // eslint-disable-line mozilla/no-useless-parameters
@@ -114,31 +95,40 @@ const LibraryButton = {
   },
 };
 
 const APP_STARTUP = 1;
 const APP_SHUTDOWN = 2;
 let addonData, startupReason;
 
 function startup(data, reason) { // eslint-disable-line no-unused-vars
+  addonResourceURI = data.resourceURI;
+
+  if (Services.prefs.getBoolPref(USER_DISABLE_PREF, false)) {
+    AddonManager.getActiveAddons().then(result => {
+      let addon = result.addons.find(a => a.id == ADDON_ID);
+      if (addon) {
+        addon.disable({allowSystemAddons: true});
+      }
+    });
+    return;
+  }
+
   addonData = data;
   startupReason = reason;
   if (reason === APP_STARTUP) {
     appStartupObserver.register();
   } else {
     appStartupDone();
   }
-  prefObserver.register();
-  addonResourceURI = data.resourceURI;
   // eslint-disable-next-line promise/catch-or-return
   appStartupPromise = appStartupPromise.then(handleStartup);
 }
 
 function shutdown(data, reason) { // eslint-disable-line no-unused-vars
-  prefObserver.unregister();
   const webExtension = LegacyExtensionsUtils.getEmbeddedExtensionFor({
     id: ADDON_ID,
     resourceURI: addonResourceURI
   });
   // Immediately exit if Firefox is exiting, #3323
   if (reason === APP_SHUTDOWN) {
     stop(webExtension, reason);
     return;
@@ -150,30 +140,24 @@ function shutdown(data, reason) { // esl
 function install(data, reason) {} // eslint-disable-line no-unused-vars
 
 function uninstall(data, reason) {} // eslint-disable-line no-unused-vars
 
 function getBoolPref(pref) {
   return prefs.getPrefType(pref) && prefs.getBoolPref(pref);
 }
 
-function shouldDisable() {
-  return getBoolPref(USER_DISABLE_PREF);
-}
-
 function handleStartup() {
   const webExtension = LegacyExtensionsUtils.getEmbeddedExtensionFor({
     id: ADDON_ID,
     resourceURI: addonResourceURI
   });
 
-  if (!shouldDisable() && !webExtension.started) {
+  if (!webExtension.started) {
     start(webExtension);
-  } else if (shouldDisable()) {
-    stop(webExtension, ADDON_DISABLE);
   }
 }
 
 function start(webExtension) {
   return webExtension.startup(startupReason, addonData).then((api) => {
     api.browser.runtime.onMessage.addListener(handleMessage);
     LibraryButton.init(webExtension);
   }).catch((err) => {
--- a/toolkit/mozapps/extensions/internal/XPIDatabase.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIDatabase.jsm
@@ -574,25 +574,25 @@ class AddonInternal {
         this.userDisabled = userDisabled;
       }
       if (softDisabled !== undefined) {
         this.softDisabled = softDisabled;
       }
     }
   }
 
-  async setUserDisabled(val) {
+  async setUserDisabled(val, allowSystemAddons = false) {
     if (val == (this.userDisabled || this.softDisabled)) {
       return;
     }
 
     if (this.inDatabase) {
       // System add-ons should not be user disabled, as there is no UI to
       // re-enable them.
-      if (this.location.isSystem) {
+      if (this.location.isSystem && !allowSystemAddons) {
         throw new Error(`Cannot disable system add-on ${this.id}`);
       }
       await XPIDatabase.updateAddonDisabledState(this, val);
     } else {
       this.userDisabled = val;
       // When enabling remove the softDisabled flag
       if (!val)
         this.softDisabled = false;
@@ -975,22 +975,24 @@ AddonWrapper = class {
     return addonFor(this).updateBlocklistState({applySoftBlock});
   }
 
   get userDisabled() {
     let addon = addonFor(this);
     return addon.softDisabled || addon.userDisabled;
   }
 
-  enable() {
-    return addonFor(this).setUserDisabled(false);
+  enable(options = {}) {
+    const {allowSystemAddons = false} = options;
+    return addonFor(this).setUserDisabled(false, allowSystemAddons);
   }
 
-  disable() {
-    return addonFor(this).setUserDisabled(true);
+  disable(options = {}) {
+    const {allowSystemAddons = false} = options;
+    return addonFor(this).setUserDisabled(true, allowSystemAddons);
   }
 
   set softDisabled(val) {
     let addon = addonFor(this);
     if (val == addon.softDisabled)
       return val;
 
     if (addon.inDatabase) {