Bug 1389840: Part 2 - Store last optional permissions state in the startup cache. r?aswan draft
authorKris Maglione <maglione.k@gmail.com>
Sat, 12 Aug 2017 14:42:44 -0700
changeset 647801 6bcfdc51bdd5b8695c2718b960d1fda9c133398f
parent 647800 741cffe9c1d8d734ff8e9d27dc3de7369b222213
child 647808 534d9bea16eec16ef9aaf9b70f94a5b2e3f24e88
child 647814 f6ac5827fc4745be237436316d6c4c9945e607a4
push id74542
push usermaglione.k@gmail.com
push dateWed, 16 Aug 2017 23:10:03 +0000
reviewersaswan
bugs1389840
milestone57.0a1
Bug 1389840: Part 2 - Store last optional permissions state in the startup cache. r?aswan MozReview-Commit-ID: 95krDpu1JZr
toolkit/components/extensions/Extension.jsm
toolkit/components/extensions/ExtensionParent.jsm
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -478,116 +478,129 @@ this.ExtensionData = class {
     // a *.domain.com to specific-host.domain.com that's actually a
     // drop in permissions but the simple test below will cause a prompt.
     return {
       origins: newPermissions.origins.filter(perm => !oldPermissions.origins.includes(perm)),
       permissions: newPermissions.permissions.filter(perm => !oldPermissions.permissions.includes(perm)),
     };
   }
 
-  parseManifest() {
-    return Promise.all([
+  async parseManifest() {
+    let [manifest] = await Promise.all([
       this.readJSON("manifest.json"),
       Management.lazyInit(),
-    ]).then(([manifest]) => {
-      this.manifest = manifest;
-      this.rawManifest = manifest;
+    ]);
+
+    this.manifest = manifest;
+    this.rawManifest = manifest;
+
+    if (manifest && manifest.default_locale) {
+      await this.initLocale();
+    }
+
+    let context = {
+      url: this.baseURI && this.baseURI.spec,
+
+      principal: this.principal,
 
-      if (manifest && manifest.default_locale) {
-        return this.initLocale();
+      logError: error => {
+        this.manifestWarning(error);
+      },
+
+      preprocessors: {},
+    };
+
+    if (this.manifest.theme) {
+      let invalidProps = validateThemeManifest(Object.getOwnPropertyNames(this.manifest));
+
+      if (invalidProps.length) {
+        let message = `Themes defined in the manifest may only contain static resources. ` +
+          `If you would like to use additional properties, please use the "theme" permission instead. ` +
+          `(the invalid properties found are: ${invalidProps})`;
+        this.manifestError(message);
       }
-    }).then(() => {
-      let context = {
-        url: this.baseURI && this.baseURI.spec,
-
-        principal: this.principal,
+    }
 
-        logError: error => {
-          this.manifestWarning(error);
-        },
+    if (this.localeData) {
+      context.preprocessors.localize = (value, context) => this.localize(value);
+    }
 
-        preprocessors: {},
-      };
+    let normalized = Schemas.normalize(this.manifest, "manifest.WebExtensionManifest", context);
+    if (normalized.error) {
+      this.manifestError(normalized.error);
+      return null;
+    }
+
+    manifest = normalized.value;
 
-      if (this.manifest.theme) {
-        let invalidProps = validateThemeManifest(Object.getOwnPropertyNames(this.manifest));
+    let id;
+    try {
+      if (manifest.applications.gecko.id) {
+        id = manifest.applications.gecko.id;
+      }
+    } catch (e) {
+      // Errors are handled by the type checks above.
+    }
 
-        if (invalidProps.length) {
-          let message = `Themes defined in the manifest may only contain static resources. ` +
-            `If you would like to use additional properties, please use the "theme" permission instead. ` +
-            `(the invalid properties found are: ${invalidProps})`;
-          this.manifestError(message);
+    if (!this.id) {
+      this.id = id;
+    }
+
+    let apiNames = new Set();
+    let dependencies = new Set();
+    let originPermissions = new Set();
+    let permissions = new Set();
+
+    for (let perm of manifest.permissions) {
+      if (perm === "geckoProfiler") {
+        const acceptedExtensions = Services.prefs.getStringPref("extensions.geckoProfiler.acceptedExtensionIds", "");
+        if (!acceptedExtensions.split(",").includes(id)) {
+          this.manifestError("Only whitelisted extensions are allowed to access the geckoProfiler.");
+          continue;
         }
       }
 
-      if (this.localeData) {
-        context.preprocessors.localize = (value, context) => this.localize(value);
-      }
-
-      let normalized = Schemas.normalize(this.manifest, "manifest.WebExtensionManifest", context);
-      if (normalized.error) {
-        this.manifestError(normalized.error);
-        return null;
-      }
+      let type = classifyPermission(perm);
+      if (type.origin) {
+        let matcher = new MatchPattern(perm, {ignorePath: true});
 
-      let manifest = normalized.value;
-
-      let id;
-      try {
-        if (manifest.applications.gecko.id) {
-          id = manifest.applications.gecko.id;
-        }
-      } catch (e) {
-        // Errors are handled by the type checks above.
+        perm = matcher.pattern;
+        originPermissions.add(perm);
+      } else if (type.api) {
+        apiNames.add(type.api);
       }
 
-      let apiNames = new Set();
-      let dependencies = new Set();
-      let hostPermissions = new Set();
-      let permissions = new Set();
+      permissions.add(perm);
+    }
 
-      for (let perm of manifest.permissions) {
-        if (perm === "geckoProfiler") {
-          const acceptedExtensions = Services.prefs.getStringPref("extensions.geckoProfiler.acceptedExtensionIds", "");
-          if (!acceptedExtensions.split(",").includes(id)) {
-            this.manifestError("Only whitelisted extensions are allowed to access the geckoProfiler.");
-            continue;
-          }
-        }
+    if (this.id) {
+      // An extension always gets permission to its own url.
+      let matcher = new MatchPattern(this.getURL(), {ignorePath: true});
+      originPermissions.add(matcher.pattern);
 
-        let type = classifyPermission(perm);
-        if (type.origin) {
-          let matcher = new MatchPattern(perm, {ignorePath: true});
-
-          perm = matcher.pattern;
-          hostPermissions.add(perm);
-        } else if (type.api) {
-          apiNames.add(type.api);
-        }
-
+      // Apply optional permissions
+      let perms = await ExtensionPermissions.get(this);
+      for (let perm of perms.permissions) {
         permissions.add(perm);
       }
-
-      // An extension always gets permission to its own url.
-      if (this.id) {
-        let matcher = new MatchPattern(this.getURL(), {ignorePath: true});
-        hostPermissions.add(matcher.pattern);
+      for (let origin of perms.origins) {
+        originPermissions.add(origin);
       }
+    }
 
-      for (let api of apiNames) {
-        dependencies.add(`${api}@experiments.addons.mozilla.org`);
-      }
+    for (let api of apiNames) {
+      dependencies.add(`${api}@experiments.addons.mozilla.org`);
+    }
 
-      // Normalize all patterns to contain a single leading /
-      let webAccessibleResources = (manifest.web_accessible_resources || [])
-          .map(path => path.replace(/^\/*/, "/"));
+    // Normalize all patterns to contain a single leading /
+    let webAccessibleResources = (manifest.web_accessible_resources || [])
+        .map(path => path.replace(/^\/*/, "/"));
 
-      return {apiNames, dependencies, hostPermissions, id, manifest, permissions,
-              webAccessibleResources};
-    });
+    return {apiNames, dependencies, originPermissions, id, manifest, permissions,
+            webAccessibleResources};
   }
 
   // Reads the extension's |manifest.json| file, and stores its
   // parsed contents in |this.manifest|.
   async loadManifest() {
     let [manifestData] = await Promise.all([
       this.parseManifest(),
       Management.lazyInit(),
@@ -603,17 +616,17 @@ this.ExtensionData = class {
     }
 
     this.manifest = manifestData.manifest;
     this.apiNames = manifestData.apiNames;
     this.dependencies = manifestData.dependencies;
     this.permissions = manifestData.permissions;
 
     this.webAccessibleResources = manifestData.webAccessibleResources.map(res => new MatchGlob(res));
-    this.whiteListedHosts = new MatchPatternSet(manifestData.hostPermissions);
+    this.whiteListedHosts = new MatchPatternSet(manifestData.originPermissions);
 
     return this.manifest;
   }
 
   localizeMessage(...args) {
     return this.localeData.localizeMessage(...args);
   }
 
@@ -844,38 +857,42 @@ this.Extension = class extends Extension
     this.on("add-permissions", (ignoreEvent, permissions) => {
       for (let perm of permissions.permissions) {
         this.permissions.add(perm);
       }
 
       if (permissions.origins.length > 0) {
         let patterns = this.whiteListedHosts.patterns.map(host => host.pattern);
 
-        this.whiteListedHosts = new MatchPatternSet([...patterns, ...permissions.origins],
+        this.whiteListedHosts = new MatchPatternSet(new Set([...patterns, ...permissions.origins]),
                                                     {ignorePath: true});
       }
 
       this.policy.permissions = Array.from(this.permissions);
       this.policy.allowedOrigins = this.whiteListedHosts;
+
+      this.cachePermissions();
     });
 
     this.on("remove-permissions", (ignoreEvent, permissions) => {
       for (let perm of permissions.permissions) {
         this.permissions.delete(perm);
       }
 
       let origins = permissions.origins.map(
         origin => new MatchPattern(origin, {ignorePath: true}).pattern);
 
       this.whiteListedHosts = new MatchPatternSet(
         this.whiteListedHosts.patterns
             .filter(host => !origins.includes(host.pattern)));
 
       this.policy.permissions = Array.from(this.permissions);
       this.policy.allowedOrigins = this.whiteListedHosts;
+
+      this.cachePermissions();
     });
     /* eslint-enable mozilla/balanced-listeners */
   }
 
   static getBootstrapScope(id, file) {
     return new BootstrapScope();
   }
 
@@ -956,19 +973,30 @@ this.Extension = class extends Extension
   readLocaleFile(locale) {
     return StartupCache.locales.get([this.id, this.version, locale],
                                     () => super.readLocaleFile(locale))
       .then(result => {
         this.localeData.messages.set(locale, result);
       });
   }
 
+  get manifestCacheKey() {
+    return [this.id, this.version, Services.locale.getAppLocaleAsLangTag()];
+  }
+
   parseManifest() {
-    return StartupCache.manifests.get([this.id, this.version, Services.locale.getAppLocaleAsLangTag()],
-                                      () => super.parseManifest());
+    return StartupCache.manifests.get(this.manifestCacheKey, () => super.parseManifest());
+  }
+
+  async cachePermissions() {
+    let manifestData = await this.parseManifest();
+
+    manifestData.originPermissions = this.whiteListedHosts.patterns.map(pat => pat.pattern);
+    manifestData.permissions = this.permissions;
+    return StartupCache.manifests.set(this.manifestCacheKey, manifestData);
   }
 
   async loadManifest() {
     let manifest = await super.loadManifest();
 
     if (this.errors.length) {
       return Promise.reject({errors: this.errors});
     }
@@ -1165,46 +1193,32 @@ this.Extension = class extends Extension
       // so during upgrades and add-on restarts, startup() gets called
       // before the last shutdown has completed, and this fails when
       // there's another active add-on with the same ID.
       this.policy.active = true;
     }
 
     TelemetryStopwatch.start("WEBEXT_EXTENSION_STARTUP_MS", this);
     try {
-      let [perms] = await Promise.all([
-        ExtensionPermissions.get(this),
-        this.loadManifest(),
-      ]);
+      await this.loadManifest();
 
       if (!this.hasShutdown) {
         await this.initLocale();
       }
 
       if (this.errors.length) {
         return Promise.reject({errors: this.errors});
       }
 
       if (this.hasShutdown) {
         return;
       }
 
       GlobalManager.init(this);
 
-      // Apply optional permissions
-      for (let perm of perms.permissions) {
-        this.permissions.add(perm);
-      }
-      if (perms.origins.length > 0) {
-        let patterns = this.whiteListedHosts.patterns.map(host => host.pattern);
-
-        this.whiteListedHosts = new MatchPatternSet([...patterns, ...perms.origins],
-                                                    {ignorePath: true});
-      }
-
       this.policy.active = false;
       this.policy = processScript.initExtension(this);
 
       this.updatePermissions(this.startupReason);
 
       // The "startup" Management event sent on the extension instance itself
       // is emitted just before the Management "startup" event,
       // and it is used to run code that needs to be executed before
--- a/toolkit/components/extensions/ExtensionParent.jsm
+++ b/toolkit/components/extensions/ExtensionParent.jsm
@@ -1526,16 +1526,22 @@ class CacheStore {
       result = await createFunc(path);
       store.set(key, result);
       StartupCache.save();
     }
 
     return result;
   }
 
+  async set(path, value) {
+    let [store, key] = await this.getStore(path);
+
+    store.set(key, value);
+  }
+
   async getAll() {
     let [store] = await this.getStore();
 
     return new Map(store);
   }
 
   async delete(path) {
     let [store, key] = await this.getStore(path);