Bug 1594810 - Remove DevTools support for IndexedDB persistent storage r=jdescottes
☠☠ backed out by 6777cec97d18 ☠ ☠
authorMichael Ratcliffe <mratcliffe@mozilla.com>
Fri, 15 Nov 2019 15:49:17 +0000
changeset 502188 31a1b40b20084e96947cf3b660a5b5ef9c5612d0
parent 502187 4658f41dadeee748ec1fe0c9ad6a0ab4c4690921
child 502189 741f4a9a2f642a96907b6a56a9dfc2936f7dea75
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdescottes
bugs1594810
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 1594810 - Remove DevTools support for IndexedDB persistent storage r=jdescottes Differential Revision: https://phabricator.services.mozilla.com/D52564
devtools/client/storage/test/browser_storage_indexeddb_duplicate_names.js
devtools/client/storage/test/storage-indexeddb-duplicate-names.html
devtools/client/storage/test/storage-overflow-indexeddb.html
devtools/server/actors/storage.js
devtools/server/tests/browser/browser_storage_browser_toolbox_indexeddb.js
devtools/server/tests/unit/test_extension_storage_actor.js
--- a/devtools/client/storage/test/browser_storage_indexeddb_duplicate_names.js
+++ b/devtools/client/storage/test/browser_storage_indexeddb_duplicate_names.js
@@ -19,18 +19,16 @@ add_task(async function() {
   await openTabAndSetupStorage(TESTPAGE);
 
   await checkState([
     [
       ["indexedDB", "http://test1.example.org"],
       [
         "idb1 (default)",
         "idb1 (temporary)",
-        "idb1 (persistent)",
         "idb2 (default)",
         "idb2 (temporary)",
-        "idb2 (persistent)",
       ],
     ],
   ]);
 
   await finishTests();
 });
--- a/devtools/client/storage/test/storage-indexeddb-duplicate-names.html
+++ b/devtools/client/storage/test/storage-indexeddb-duplicate-names.html
@@ -6,20 +6,18 @@
 
   <script type="application/javascript">
     "use strict";
 
     /* exported createIndexedDBs */
     function createIndexedDBs() {
       createIndexedDB("idb1", "temporary");
       createIndexedDB("idb1", "default");
-      createIndexedDB("idb1", "persistent");
       createIndexedDB("idb2", "temporary");
       createIndexedDB("idb2", "default");
-      createIndexedDB("idb2", "persistent");
     }
 
     function createIndexedDB(name, storage) {
       const open = indexedDB.open(name, {storage: storage});
 
       open.onsuccess = function () {
         const db = open.result;
         db.close();
@@ -31,20 +29,18 @@
         dump(`removing database ${dbName} (${storage}) from ${document.location}\n`);
         indexedDB.deleteDatabase(dbName, { storage: storage }).onsuccess = resolve;
       });
     }
 
     window.clear = async function () {
       await deleteDB("idb1", "temporary");
       await deleteDB("idb1", "default");
-      await deleteDB("idb1", "persistent");
       await deleteDB("idb2", "temporary");
       await deleteDB("idb2", "default");
-      await deleteDB("idb2", "persistent");
 
       dump(`removed indexedDB data from ${document.location}\n`);
     };
   </script>
 </head>
 <body onload="createIndexedDBs()">
   <h1>storage-indexeddb-duplicate-names.html</h1>
 </body>
--- a/devtools/client/storage/test/storage-overflow-indexeddb.html
+++ b/devtools/client/storage/test/storage-overflow-indexeddb.html
@@ -36,16 +36,15 @@ function deleteDB(dbName, storage) {
   return new Promise(resolve => {
     indexedDB.deleteDatabase(dbName, { storage: storage }).onsuccess = resolve;
   });
 }
 
 window.clear = async function() {
   await deleteDB("database", "temporary");
   await deleteDB("database", "default");
-  await deleteDB("database", "persistent");
 
   dump(`removed indexedDB data from ${document.location}\n`);
 };
 
 </script>
 </body>
 </html>
--- a/devtools/server/actors/storage.js
+++ b/devtools/server/actors/storage.js
@@ -21,18 +21,16 @@ loader.lazyGetter(this, "ExtensionStorag
     .ExtensionStorageIDB;
 });
 loader.lazyGetter(
   this,
   "WebExtensionPolicy",
   () => Cu.getGlobalForObject(ExtensionProcessScript).WebExtensionPolicy
 );
 
-const CHROME_ENABLED_PREF = "devtools.chrome.enabled";
-const REMOTE_ENABLED_PREF = "devtools.debugger.remote-enabled";
 const EXTENSION_STORAGE_ENABLED_PREF =
   "devtools.storage.extensionStorage.enabled";
 
 const DEFAULT_VALUE = "value";
 
 loader.lazyRequireGetter(
   this,
   "naturalSortCaseInsensitive",
@@ -43,18 +41,16 @@ loader.lazyRequireGetter(
 // "Lax", "Strict" and "Unset" are special values of the sameSite property
 // that should not be translated.
 const COOKIE_SAMESITE = {
   LAX: "Lax",
   STRICT: "Strict",
   UNSET: "Unset",
 };
 
-const SAFE_HOSTS_PREFIXES_REGEX = /^(about\+|https?\+|file\+|moz-extension\+)/;
-
 // GUID to be used as a separator in compound keys. This must match the same
 // constant in devtools/client/storage/ui.js,
 // devtools/client/storage/test/head.js and
 // devtools/server/tests/browser/head.js
 const SEPARATOR_GUID = "{9d414cc5-8319-0a04-0586-c0a6ae01670a}";
 
 loader.lazyImporter(this, "OS", "resource://gre/modules/osfile.jsm");
 loader.lazyImporter(this, "Sqlite", "resource://gre/modules/Sqlite.jsm");
@@ -150,33 +146,27 @@ StorageActors.defaults = function(typeNa
     typeName: typeName,
 
     get conn() {
       return this.storageActor.conn;
     },
 
     /**
      * Returns a list of currently known hosts for the target window. This list
-     * contains unique hosts from the window + all inner windows. If
-     * this._internalHosts is defined then these will also be added to the list.
+     * contains unique hosts from the window + all inner windows.
      */
     get hosts() {
       const hosts = new Set();
       for (const { location } of this.storageActor.windows) {
         const host = this.getHostName(location);
 
         if (host) {
           hosts.add(host);
         }
       }
-      if (this._internalHosts) {
-        for (const host of this._internalHosts) {
-          hosts.add(host);
-        }
-      }
       return hosts;
     },
 
     /**
      * Returns all the windows present on the page. Includes main window + inner
      * iframe windows.
      */
     get windows() {
@@ -2130,17 +2120,17 @@ ObjectStoreMetadata.prototype = {
 /**
  * Meta data object for a particular indexed db in a host.
  *
  * @param {string} origin
  *        The host associated with this indexed db.
  * @param {IDBDatabase} db
  *        The particular indexed db.
  * @param {String} storage
- *        Storage type, either "temporary", "default" or "persistent".
+ *        Storage type, either "temporary" or "default".
  */
 function DatabaseMetadata(origin, db, storage) {
   this._origin = origin;
   this._name = db.name;
   this._version = db.version;
   this._objectStores = [];
   this.storage = storage;
 
@@ -2204,31 +2194,16 @@ StorageActors.createActor(
       this.storageActor.off("window-destroyed", this.onWindowDestroyed);
 
       protocol.Actor.prototype.destroy.call(this);
 
       this.storageActor = null;
     },
 
     /**
-     * Returns a list of currently known hosts for the target window. This list
-     * contains unique hosts from the window, all inner windows and all permanent
-     * indexedDB hosts defined inside the browser.
-     */
-    async getHosts() {
-      // Add internal hosts to this._internalHosts, which will be picked up by
-      // the this.hosts getter. Because this.hosts is a property on the default
-      // storage actor and inherited by all storage actors we have to do it this
-      // way.
-      this._internalHosts = await this.getInternalHosts();
-
-      return this.hosts;
-    },
-
-    /**
      * Remove an indexedDB database from given host with a given name.
      */
     async removeDatabase(host, name) {
       const win = this.storageActor.getWindowFromHost(host);
       if (!win) {
         return { error: `Window for host ${host} not found` };
       }
 
@@ -2339,17 +2314,17 @@ StorageActors.createActor(
      * Purpose of this method is same as populateStoresForHosts but this is async.
      * This exact same operation cannot be performed in populateStoresForHosts
      * method, as that method is called in initialize method of the actor, which
      * cannot be asynchronous.
      */
     async preListStores() {
       this.hostVsStores = new Map();
 
-      for (const host of await this.getHosts()) {
+      for (const host of await this.hosts) {
         await this.populateStoresForHost(host);
       }
     },
 
     async populateStoresForHost(host) {
       const storeMap = new Map();
 
       const win = this.storageActor.getWindowFromHost(host);
@@ -2452,37 +2427,32 @@ StorageActors.createActor(
         this.getNameFromDatabaseFile = indexedDBHelpers.getNameFromDatabaseFile;
         this.getObjectStoreData = indexedDBHelpers.getObjectStoreData;
         this.getSanitizedHost = indexedDBHelpers.getSanitizedHost;
         this.getValuesForHost = indexedDBHelpers.getValuesForHost;
         this.openWithPrincipal = indexedDBHelpers.openWithPrincipal;
         this.removeDB = indexedDBHelpers.removeDB;
         this.removeDBRecord = indexedDBHelpers.removeDBRecord;
         this.splitNameAndStorage = indexedDBHelpers.splitNameAndStorage;
-        this.getInternalHosts = indexedDBHelpers.getInternalHosts;
         return;
       }
 
       const mm = this.conn.parentMessageManager;
 
       // eslint-disable-next-line no-restricted-properties
       this.conn.setupInParent({
         module: "devtools/server/actors/storage",
         setupParent: "setupParentProcessForIndexedDB",
       });
 
       this.getDBMetaData = callParentProcessAsync.bind(null, "getDBMetaData");
       this.splitNameAndStorage = callParentProcessAsync.bind(
         null,
         "splitNameAndStorage"
       );
-      this.getInternalHosts = callParentProcessAsync.bind(
-        null,
-        "getInternalHosts"
-      );
       this.getDBNamesForHost = callParentProcessAsync.bind(
         null,
         "getDBNamesForHost"
       );
       this.getValuesForHost = callParentProcessAsync.bind(
         null,
         "getValuesForHost"
       );
@@ -2605,44 +2575,16 @@ var indexedDBHelpers = {
     const storage = name.substr(lastOpenBracketIndex + 1, delta);
 
     name = name.substr(0, lastOpenBracketIndex - 1);
 
     return { storage, name };
   },
 
   /**
-   * Get all "internal" hosts. Internal hosts are database namespaces used by
-   * the browser.
-   */
-  async getInternalHosts() {
-    // Return an empty array if the browser toolbox is not enabled.
-    if (
-      !Services.prefs.getBoolPref(CHROME_ENABLED_PREF) ||
-      !Services.prefs.getBoolPref(REMOTE_ENABLED_PREF)
-    ) {
-      return this.backToChild("getInternalHosts", []);
-    }
-
-    const profileDir = OS.Constants.Path.profileDir;
-    const storagePath = OS.Path.join(profileDir, "storage", "permanent");
-    const iterator = new OS.File.DirectoryIterator(storagePath);
-    const hosts = [];
-
-    await iterator.forEach(entry => {
-      if (entry.isDir && !SAFE_HOSTS_PREFIXES_REGEX.test(entry.name)) {
-        hosts.push(entry.name);
-      }
-    });
-    iterator.close();
-
-    return this.backToChild("getInternalHosts", hosts);
-  },
-
-  /**
    * Opens an indexed db connection for the given `principal` and
    * database `name`.
    */
   openWithPrincipal: function(principal, name, storage) {
     return indexedDBForStorage.openForPrincipal(principal, name, {
       storage: storage,
     });
   },
@@ -2759,39 +2701,36 @@ var indexedDBHelpers = {
     const profileDir = OS.Constants.Path.profileDir;
     const files = [];
     const names = [];
     const storagePath = OS.Path.join(profileDir, "storage");
 
     // We expect sqlite DB paths to look something like this:
     // - PathToProfileDir/storage/default/http+++www.example.com/
     //   idb/1556056096MeysDaabta.sqlite
-    // - PathToProfileDir/storage/permanent/http+++www.example.com/
-    //   idb/1556056096MeysDaabta.sqlite
     // - PathToProfileDir/storage/temporary/http+++www.example.com/
     //   idb/1556056096MeysDaabta.sqlite
     // The subdirectory inside the storage folder is determined by the storage
     // type:
     // - default:   { storage: "default" } or not specified.
-    // - permanent: { storage: "persistent" }.
     // - temporary: { storage: "temporary" }.
     const sqliteFiles = await this.findSqlitePathsForHost(
       storagePath,
       sanitizedHost
     );
 
     for (const file of sqliteFiles) {
       const splitPath = OS.Path.split(file).components;
       const idbIndex = splitPath.indexOf("idb");
       const storage = splitPath[idbIndex - 2];
       const relative = file.substr(profileDir.length + 1);
 
       files.push({
         file: relative,
-        storage: storage === "permanent" ? "persistent" : storage,
+        storage,
       });
     }
 
     if (files.length > 0) {
       for (const { file, storage } of files) {
         const name = await this.getNameFromDatabaseFile(file);
         if (name) {
           names.push({
@@ -2836,25 +2775,28 @@ var indexedDBHelpers = {
       if (await OS.File.exists(idbPath)) {
         idbPaths.push(idbPath);
       }
     }
     return idbPaths;
   },
 
   /**
-   * Find all the storage types, such as "default", "permanent", or "temporary".
-   * These names have changed over time, so it seems simpler to look through all types
-   * that currently exist in the profile.
+   * Find all the storage types, such as "default" or "temporary".
+   * These names have changed over time, so it seems simpler to look through all
+   * types that currently exist in the profile.
    */
   async findStorageTypePaths(storagePath) {
     const iterator = new OS.File.DirectoryIterator(storagePath);
     const typePaths = [];
     await iterator.forEach(entry => {
-      if (entry.isDir) {
+      if (
+        entry.isDir &&
+        (entry.path.endsWith("/default") || entry.path.endsWith("/temporary"))
+      ) {
         typePaths.push(entry.path);
       }
     });
     iterator.close();
     return typePaths;
   },
 
   /**
@@ -2969,17 +2911,17 @@ var indexedDBHelpers = {
    *
    * @param {string} host
    *        The given host.
    * @param {nsIPrincipal} principal
    *        The principal of the given document.
    * @param {string} dbName
    *        The name of the indexed db from the above host.
    * @param {String} storage
-   *        Storage type, either "temporary", "default" or "persistent".
+   *        Storage type, either "temporary" or "default".
    * @param {Object} requestOptions
    *        An object in the following format:
    *        {
    *          objectStore: The name of the object store from the above db,
    *          id:          Id of the requested entry from the above object
    *                       store. null if all entries from the above object
    *                       store are requested,
    *          index:       Name of the IDBIndex to be iterated on while fetching
@@ -3093,19 +3035,16 @@ var indexedDBHelpers = {
   handleChildRequest(msg) {
     const args = msg.data.args;
 
     switch (msg.json.method) {
       case "getDBMetaData": {
         const [host, principal, name, storage] = args;
         return indexedDBHelpers.getDBMetaData(host, principal, name, storage);
       }
-      case "getInternalHosts": {
-        return indexedDBHelpers.getInternalHosts();
-      }
       case "splitNameAndStorage": {
         const [name] = args;
         return indexedDBHelpers.splitNameAndStorage(name);
       }
       case "getDBNamesForHost": {
         const [host, principal] = args;
         return indexedDBHelpers.getDBNamesForHost(host, principal);
       }
--- a/devtools/server/tests/browser/browser_storage_browser_toolbox_indexeddb.js
+++ b/devtools/server/tests/browser/browser_storage_browser_toolbox_indexeddb.js
@@ -40,14 +40,14 @@ async function createIndexedDB() {
 
   await undefined;
 }
 
 async function testInternalDBs(front) {
   const data = await front.listStores();
   const hosts = data.indexedDB.hosts;
 
-  ok(hosts.chrome, `indexedDB hosts contains "chrome"`);
-
-  const path = `["MyDatabase (persistent)","MyObjectStore"]`;
-  const foundDB = hosts.chrome.includes(path);
-  ok(foundDB, `Host "chrome" includes ${path}`);
+  // According to bug 1594810 "persistent" storage is deprecated and we plan
+  // to remove support completely in dom/indexedDB. To make this possible
+  // we need to skip "persistent" storage entries to prevent the toolbox
+  // from breaking when support is removed.
+  ok(!hosts.chrome, `indexedDB hosts doesn't contain "chrome"`);
 }
--- a/devtools/server/tests/unit/test_extension_storage_actor.js
+++ b/devtools/server/tests/unit/test_extension_storage_actor.js
@@ -1,19 +1,15 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 /* globals browser */
 
 "use strict";
 
-const { FileUtils } = ChromeUtils.import(
-  "resource://gre/modules/FileUtils.jsm"
-);
-
 const { ExtensionTestUtils } = ChromeUtils.import(
   "resource://testing-common/ExtensionXPCShellUtils.jsm"
 );
 
 // Ignore rejection related to the storage.onChanged listener being removed while the extension context is being closed.
 const { PromiseTestUtils } = ChromeUtils.import(
   "resource://testing-common/PromiseTestUtils.jsm"
 );
@@ -199,40 +195,18 @@ const ext_no_bg = {
  */
 async function shutdown(extension, target) {
   if (target) {
     await target.destroy();
   }
   await extension.unload();
 }
 
-/**
- * Mocks the missing 'storage/permanent' directory needed by the "indexedDB"
- * storage actor's 'preListStores' method (called when 'listStores' is called). This
- * directory exists in a full browser i.e. mochitest.
- */
-function createMissingIndexedDBDirs() {
-  const dir = Services.dirsvc.get("ProfD", Ci.nsIFile).clone();
-  dir.append("storage");
-  if (!dir.exists()) {
-    dir.create(dir.DIRECTORY_TYPE, FileUtils.PERMS_DIRECTORY);
-  }
-  dir.append("permanent");
-  if (!dir.exists()) {
-    dir.create(dir.DIRECTORY_TYPE, FileUtils.PERMS_DIRECTORY);
-  }
-  Assert.ok(
-    dir.exists(),
-    "Should have a 'storage/permanent' dir in the profile dir"
-  );
-}
-
 add_task(async function setup() {
   await promiseStartupManager();
-  createMissingIndexedDBDirs();
 });
 
 add_task(async function test_extension_store_exists() {
   const extension = await startupExtension(getExtensionConfig());
 
   const { target, extensionStorage } = await openAddonStoragePanel(
     extension.id
   );