Bug 1364768: Part 6 - Use startup cache for initial extension permission data. r?aswan draft
authorKris Maglione <maglione.k@gmail.com>
Sun, 14 May 2017 16:12:33 -0700
changeset 577575 129da39e7c39b031eaa16b605a558e49754ebe1d
parent 577574 96ee56aebb0892233e65429802b5709d1d8409b4
child 577576 e3af12f73427dd5e047883ace508094f63dca6b7
push id58718
push usermaglione.k@gmail.com
push dateSun, 14 May 2017 23:25:47 +0000
reviewersaswan
bugs1364768
milestone55.0a1
Bug 1364768: Part 6 - Use startup cache for initial extension permission data. r?aswan Reading the extension permissions DB at startup takes several hundred milliseconds, largely from the overhead of initializing OS.File. We can avoid that somewhat by using the stream APIs to read the files, and beginning the read very early. But the eager initialization gets complicated, and we still add extra IO to startup. After this change, the permissions JSON file still remains the primary source of truth, but the state as of the last session is cached in the volatile extension startup cache to decrease the overhead of reading it at startup. MozReview-Commit-ID: HGDt5kSsdzX
toolkit/components/extensions/Extension.jsm
toolkit/components/extensions/ExtensionPermissions.jsm
toolkit/components/extensions/ExtensionUtils.jsm
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -980,17 +980,20 @@ this.Extension = class extends Extension
     this.startupPromise = this._startup();
     return this.startupPromise;
   }
 
   async _startup() {
     this.started = false;
 
     try {
-      let [, perms] = await Promise.all([this.loadManifest(), ExtensionPermissions.get(this)]);
+      let [perms] = await Promise.all([
+        ExtensionPermissions.get(this),
+        this.loadManifest(),
+      ]);
 
       ExtensionManagement.startupExtension(this.uuid, this.addonData.resourceURI, this);
       this.started = true;
 
       if (!this.hasShutdown) {
         await this.initLocale();
       }
 
--- a/toolkit/components/extensions/ExtensionPermissions.jsm
+++ b/toolkit/components/extensions/ExtensionPermissions.jsm
@@ -1,58 +1,90 @@
 "use strict";
 
 const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
+XPCOMUtils.defineLazyModuleGetter(this, "ExtensionUtils",
+                                  "resource://gre/modules/ExtensionUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "FileUtils",
+                                  "resource://gre/modules/FileUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "JSONFile",
                                   "resource://gre/modules/JSONFile.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "OS",
                                   "resource://gre/modules/osfile.jsm");
 
+XPCOMUtils.defineLazyGetter(this, "StartupCache", () => ExtensionUtils.StartupCache);
+
 this.EXPORTED_SYMBOLS = ["ExtensionPermissions"];
 
 const FILE_NAME = "extension-preferences.json";
 
 let prefs;
 let _initPromise;
+
+async function _lazyInit() {
+  let file = FileUtils.getFile("ProfD", [FILE_NAME]);
+
+  prefs = new JSONFile({path: file.path});
+  prefs.data = {};
+
+  try {
+    let blob = await ExtensionUtils.promiseFileContents(file);
+    prefs.data = JSON.parse(new TextDecoder().decode(blob));
+  } catch (e) {
+    Cu.reportError(e);
+  }
+}
+
 function lazyInit() {
   if (!_initPromise) {
-    prefs = new JSONFile({path: OS.Path.join(OS.Constants.Path.profileDir, FILE_NAME)});
-
-    _initPromise = prefs.load();
+    _initPromise = _lazyInit();
   }
   return _initPromise;
 }
 
 function emptyPermissions() {
   return {permissions: [], origins: []};
 }
 
 this.ExtensionPermissions = {
-  async get(extension) {
+  async _saveSoon(extension) {
+    await lazyInit();
+
+    prefs.data[extension.id] = await this._getCached(extension);
+    return prefs.saveSoon();
+  },
+
+  async _get(extension) {
     await lazyInit();
 
-    let perms = emptyPermissions();
-    if (prefs.data[extension.id]) {
-      Object.assign(perms, prefs.data[extension.id]);
+    let perms = prefs.data[extension.id];
+    if (!perms) {
+      perms = emptyPermissions();
+      prefs.data[extension.id] = perms;
     }
+
     return perms;
   },
 
+  async _getCached(extension) {
+    return StartupCache.permissions.get(extension.id,
+                                        () => this._get(extension));
+  },
+
+  get(extension) {
+    return this._getCached(extension);
+  },
+
   // Add new permissions for the given extension.  `permissions` is
   // in the format that is passed to browser.permissions.request().
   async add(extension, perms) {
-    await lazyInit();
-
-    if (!prefs.data[extension.id]) {
-      prefs.data[extension.id] = emptyPermissions();
-    }
-    let {permissions, origins} = prefs.data[extension.id];
+    let {permissions, origins} = await this._getCached(extension);
 
     let added = emptyPermissions();
 
     for (let perm of perms.permissions) {
       if (!permissions.includes(perm)) {
         added.permissions.push(perm);
         permissions.push(perm);
       }
@@ -60,30 +92,25 @@ this.ExtensionPermissions = {
     for (let origin of perms.origins) {
       if (!origins.includes(origin)) {
         added.origins.push(origin);
         origins.push(origin);
       }
     }
 
     if (added.permissions.length > 0 || added.origins.length > 0) {
-      prefs.saveSoon();
+      this._saveSoon(extension);
       extension.emit("add-permissions", added);
     }
   },
 
   // Revoke permissions from the given extension.  `permissions` is
   // in the format that is passed to browser.permissions.remove().
   async remove(extension, perms) {
-    await lazyInit();
-
-    if (!prefs.data[extension.id]) {
-      return;
-    }
-    let {permissions, origins} = prefs.data[extension.id];
+    let {permissions, origins} = await this._getCached(extension);
 
     let removed = emptyPermissions();
 
     for (let perm of perms.permissions) {
       let i = permissions.indexOf(perm);
       if (i >= 0) {
         removed.permissions.push(perm);
         permissions.splice(i, 1);
@@ -93,25 +120,28 @@ this.ExtensionPermissions = {
       let i = origins.indexOf(origin);
       if (i >= 0) {
         removed.origins.push(origin);
         origins.splice(i, 1);
       }
     }
 
     if (removed.permissions.length > 0 || removed.origins.length > 0) {
-      prefs.saveSoon();
+      this.saveSoon(extension);
       extension.emit("remove-permissions", removed);
     }
   },
 
   async removeAll(extension) {
-    await lazyInit();
-    delete prefs.data[extension.id];
-    prefs.saveSoon();
+    let perms = await this._getCached(extension);
+
+    if (perms.permission.length || perms.origins.length) {
+      Object.assign(perms, emptyPermissions());
+      prefs.saveSoon();
+    }
   },
 
   // This is meant for tests only
   async _uninit() {
     if (!_initPromise) {
       return;
     }
 
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -78,17 +78,17 @@ function promiseFileContents(file) {
       }
     });
   });
 }
 
 let StartupCache = {
   DB_NAME: "ExtensionStartupCache",
 
-  STORE_NAMES: Object.freeze(["locales", "manifests", "schemas"]),
+  STORE_NAMES: Object.freeze(["locales", "manifests", "permissions", "schemas"]),
 
   get file() {
     return FileUtils.getFile("ProfLD", ["startupCache", "webext.sc.lz4"]);
   },
 
   get saver() {
     if (!this._saver) {
       this._saver = new DeferredSave(this.file.path,
@@ -127,16 +127,17 @@ let StartupCache = {
     }
     return this._dataPromise;
   },
 
   clearAddonData(id) {
     return Promise.all([
       this.locales.delete(id),
       this.manifests.delete(id),
+      this.permissions.delete(id),
     ]).catch(e => {
       // Ignore the error. It happens when we try to flush the add-on
       // data after the AddonManager has flushed the entire startup cache.
     });
   },
 
   observe(subject, topic, data) {
     if (topic === "startupcache-invalidate") {