Bug 1448221: Part 3 - Remove startup staging directory scan. r=aswan
authorKris Maglione <maglione.k@gmail.com>
Mon, 26 Mar 2018 16:09:52 -0700
changeset 410119 22cd4c5fcb397d80fc7f693737dc1c827ce61d74
parent 410118 6f8118a829c47450e90ef32958287db20cb0a3b0
child 410120 7e5efe49f034c364260d5f1e0964060693c50d7e
push id33718
push userbtara@mozilla.com
push dateTue, 27 Mar 2018 09:16:52 +0000
treeherdermozilla-central@97cdd8febc40 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan
bugs1448221
milestone61.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 1448221: Part 3 - Remove startup staging directory scan. r=aswan MozReview-Commit-ID: JHA1umCQS2D
testing/mozbase/mozprofile/mozprofile/addons.py
testing/mozbase/mozprofile/tests/test_addons.py
toolkit/mozapps/extensions/internal/XPIInstall.jsm
toolkit/mozapps/extensions/internal/XPIProvider.jsm
toolkit/mozapps/extensions/test/addons/test_delay_update_ignore_v2/install.rdf
--- a/testing/mozbase/mozprofile/mozprofile/addons.py
+++ b/testing/mozbase/mozprofile/mozprofile/addons.py
@@ -87,17 +87,17 @@ class AddonManager(object):
                 pass
 
         # Remove all downloaded add-ons
         for addon in self.downloaded_addons:
             mozfile.remove(addon)
 
         # restore backups
         if self.backup_dir and os.path.isdir(self.backup_dir):
-            extensions_path = os.path.join(self.profile, 'extensions', 'staged')
+            extensions_path = os.path.join(self.profile, 'extensions')
 
             for backup in os.listdir(self.backup_dir):
                 backup_path = os.path.join(self.backup_dir, backup)
                 shutil.move(backup_path, extensions_path)
 
             if not os.listdir(self.backup_dir):
                 mozfile.remove(self.backup_dir)
 
@@ -135,24 +135,20 @@ class AddonManager(object):
         return new_path
 
     def get_addon_path(self, addon_id):
         """Returns the path to the installed add-on
 
         :param addon_id: id of the add-on to retrieve the path from
         """
         # By default we should expect add-ons being located under the
-        # extensions folder. Only if the application hasn't been run and
-        # installed the add-ons yet, it will be located under 'staged'.
-        # Also add-ons could have been unpacked by the application.
+        # extensions folder.
         extensions_path = os.path.join(self.profile, 'extensions')
         paths = [os.path.join(extensions_path, addon_id),
-                 os.path.join(extensions_path, addon_id + '.xpi'),
-                 os.path.join(extensions_path, 'staged', addon_id),
-                 os.path.join(extensions_path, 'staged', addon_id + '.xpi')]
+                 os.path.join(extensions_path, addon_id + '.xpi')]
         for path in paths:
             if os.path.exists(path):
                 return path
 
         raise IOError('Add-on not found: %s' % addon_id)
 
     @classmethod
     def is_addon(self, addon_path):
@@ -401,17 +397,17 @@ class AddonManager(object):
             # note: we might want to let Firefox do it in case of addon details
             orig_path = None
             if os.path.isfile(addon) and (unpack or addon_details['unpack']):
                 orig_path = addon
                 addon = tempfile.mkdtemp()
                 mozfile.extract(orig_path, addon)
 
             # copy the addon to the profile
-            extensions_path = os.path.join(self.profile, 'extensions', 'staged')
+            extensions_path = os.path.join(self.profile, 'extensions')
             addon_path = os.path.join(extensions_path, addon_id)
 
             if os.path.isfile(addon):
                 addon_path += '.xpi'
 
                 # move existing xpi file to backup location to restore later
                 if os.path.exists(addon_path):
                     self.backup_dir = self.backup_dir or tempfile.mkdtemp()
--- a/testing/mozbase/mozprofile/tests/test_addons.py
+++ b/testing/mozbase/mozprofile/tests/test_addons.py
@@ -2,17 +2,16 @@
 
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this file,
 # You can obtain one at http://mozilla.org/MPL/2.0/.
 
 from __future__ import absolute_import
 
 import os
-import shutil
 import tempfile
 import unittest
 import zipfile
 
 import mozunit
 
 from manifestparser import ManifestParser
 import mozfile
@@ -162,17 +161,17 @@ class TestAddonsManager(unittest.TestCas
         # Generate installer stubs and install them
         for ext in ['test-addon-1@mozilla.org', 'test-addon-2@mozilla.org']:
             temp_addon = generate_addon(ext, path=self.tmpdir)
             addons_to_install.append(self.am.addon_details(temp_addon)['id'])
             self.am.install_from_path(temp_addon)
 
         # Generate a list of addons installed in the profile
         addons_installed = [str(x[:-len('.xpi')]) for x in os.listdir(os.path.join(
-                            self.profile.profile, 'extensions', 'staged'))]
+                            self.profile.profile, 'extensions'))]
         self.assertEqual(addons_to_install.sort(), addons_installed.sort())
 
     def test_install_from_path_folder(self):
         # Generate installer stubs for all possible types of addons
         addons = []
         addons.append(generate_addon('test-addon-1@mozilla.org',
                                      path=self.tmpdir))
         addons.append(generate_addon('test-addon-2@mozilla.org',
@@ -239,17 +238,17 @@ class TestAddonsManager(unittest.TestCas
         self.profile.addon_manager.install_from_path(addon)
 
         self.profile.reset()
 
         self.profile.addon_manager.install_from_path(addon)
         self.assertEqual(self.profile.addon_manager.installed_addons, [addon])
 
     def test_install_from_path_backup(self):
-        staged_path = os.path.join(self.profile_path, 'extensions', 'staged')
+        staged_path = os.path.join(self.profile_path, 'extensions')
 
         # Generate installer stubs for all possible types of addons
         addon_xpi = generate_addon('test-addon-1@mozilla.org',
                                    path=self.tmpdir)
         addon_folder = generate_addon('test-addon-1@mozilla.org',
                                       path=self.tmpdir,
                                       xpi=False)
         addon_name = generate_addon('test-addon-1@mozilla.org',
@@ -327,17 +326,17 @@ class TestAddonsManager(unittest.TestCas
         addons = m.get()
 
         # Obtain details of addons to install from the manifest
         addons_to_install = [self.am.addon_details(x['path']).get('id') for x in addons]
 
         self.am.install_from_manifest(temp_manifest)
         # Generate a list of addons installed in the profile
         addons_installed = [str(x[:-len('.xpi')]) for x in os.listdir(os.path.join(
-                            self.profile.profile, 'extensions', 'staged'))]
+                            self.profile.profile, 'extensions'))]
         self.assertEqual(addons_installed.sort(), addons_to_install.sort())
 
         # Cleanup the temporary addon and manifest directories
         mozfile.rmtree(os.path.dirname(temp_manifest))
 
     def test_addon_details(self):
         # Generate installer stubs for a valid and invalid add-on manifest
         valid_addon = generate_addon('test-addon-1@mozilla.org',
@@ -367,27 +366,27 @@ class TestAddonsManager(unittest.TestCas
 
     @unittest.skip("Bug 900154")
     def test_clean_addons(self):
         addon_one = generate_addon('test-addon-1@mozilla.org')
         addon_two = generate_addon('test-addon-2@mozilla.org')
 
         self.am.install_addons(addon_one)
         installed_addons = [str(x[:-len('.xpi')]) for x in os.listdir(os.path.join(
-                            self.profile.profile, 'extensions', 'staged'))]
+                            self.profile.profile, 'extensions'))]
 
         # Create a new profile based on an existing profile
         # Install an extra addon in the new profile
         # Cleanup addons
         duplicate_profile = mozprofile.profile.Profile(profile=self.profile.profile,
                                                        addons=addon_two)
         duplicate_profile.addon_manager.clean()
 
         addons_after_cleanup = [str(x[:-len('.xpi')]) for x in os.listdir(os.path.join(
-                                duplicate_profile.profile, 'extensions', 'staged'))]
+                                duplicate_profile.profile, 'extensions'))]
         # New addons installed should be removed by clean_addons()
         self.assertEqual(installed_addons, addons_after_cleanup)
 
     def test_noclean(self):
         """test `restore=True/False` functionality"""
 
         server = mozhttpd.MozHttpd(docroot=os.path.join(here, 'addons'))
         server.start()
@@ -408,17 +407,17 @@ class TestAddonsManager(unittest.TestCas
             # install it with a restore=True AddonManager
             am = mozprofile.addons.AddonManager(profile, restore=True)
 
             for addon in addons:
                 am.install_from_path(addon)
 
             # now its there
             self.assertEqual(os.listdir(profile), ['extensions'])
-            staging_folder = os.path.join(profile, 'extensions', 'staged')
+            staging_folder = os.path.join(profile, 'extensions')
             self.assertTrue(os.path.exists(staging_folder))
             self.assertEqual(len(os.listdir(staging_folder)), 2)
 
             # del addons; now its gone though the directory tree exists
             downloaded_addons = am.downloaded_addons
             del am
 
             self.assertEqual(os.listdir(profile), ['extensions'])
@@ -437,23 +436,19 @@ class TestAddonsManager(unittest.TestCas
         addons.append(generate_addon('test-addon-1@mozilla.org',
                                      path=self.tmpdir))
         addons.append(generate_addon('test-addon-2@mozilla.org',
                                      path=self.tmpdir))
 
         self.am.install_from_path(self.tmpdir)
 
         extensions_path = os.path.join(self.profile_path, 'extensions')
-        staging_path = os.path.join(extensions_path, 'staged')
-
-        # Fake a run by virtually installing one of the staged add-ons
-        shutil.move(os.path.join(staging_path, 'test-addon-1@mozilla.org.xpi'),
-                    extensions_path)
+        staging_path = os.path.join(extensions_path)
 
         for addon in self.am._addons:
             self.am.remove_addon(addon)
 
         self.assertEqual(os.listdir(staging_path), [])
-        self.assertEqual(os.listdir(extensions_path), ['staged'])
+        self.assertEqual(os.listdir(extensions_path), [])
 
 
 if __name__ == '__main__':
     mozunit.main()
--- a/toolkit/mozapps/extensions/internal/XPIInstall.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIInstall.jsm
@@ -2,17 +2,16 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 var EXPORTED_SYMBOLS = [
   "DownloadAddonInstall",
   "LocalAddonInstall",
-  "StagedAddonInstall",
   "UpdateChecker",
   "loadManifestFromFile",
   "verifyBundleSignedState",
 ];
 
 /* globals DownloadAddonInstall, LocalAddonInstall */
 
 ChromeUtils.import("resource://gre/modules/Services.jsm");
@@ -198,42 +197,16 @@ function setFilePermissions(aFile, aPerm
   try {
     aFile.permissions = aPermissions;
   } catch (e) {
     logger.warn("Failed to set permissions " + aPermissions.toString(8) + " on " +
          aFile.path, e);
   }
 }
 
-/**
- * Write a given string to a file
- *
- * @param  file
- *         The nsIFile instance to write into
- * @param  string
- *         The string to write
- */
-function writeStringToFile(file, string) {
-  let stream = Cc["@mozilla.org/network/file-output-stream;1"].
-               createInstance(Ci.nsIFileOutputStream);
-  let converter = Cc["@mozilla.org/intl/converter-output-stream;1"].
-                  createInstance(Ci.nsIConverterOutputStream);
-
-  try {
-    stream.init(file, FileUtils.MODE_WRONLY | FileUtils.MODE_CREATE |
-                            FileUtils.MODE_TRUNCATE, FileUtils.PERMS_FILE,
-                           0);
-    converter.init(stream, "UTF-8");
-    converter.writeString(string);
-  } finally {
-    converter.close();
-    stream.close();
-  }
-}
-
 function EM_R(aProperty) {
   return gRDF.GetResource(PREFIX_NS_EM + aProperty);
 }
 
 function getManifestFileForDir(aDir) {
   let file = getFile(FILE_RDF_MANIFEST, aDir);
   if (file.exists() && file.isFile())
     return file;
@@ -1426,18 +1399,17 @@ class AddonInstall {
       AddonManagerPrivate.callInstallListeners("onDownloadCancelled",
                                                this.listeners, this.wrapper);
       this.removeTemporaryFile();
       break;
     case AddonManager.STATE_INSTALLED:
       logger.debug("Cancelling install of " + this.addon.id);
       let xpi = getFile(`${this.addon.id}.xpi`, this.installLocation.getStagingDir());
       flushJarCache(xpi);
-      this.installLocation.cleanStagingDir([this.addon.id, this.addon.id + ".xpi",
-                                            this.addon.id + ".json"]);
+      this.installLocation.cleanStagingDir([this.addon.id, this.addon.id + ".xpi"]);
       this.state = AddonManager.STATE_CANCELLED;
       XPIProvider.removeActiveInstall(this);
 
       if (this.existingAddon) {
         delete this.existingAddon.pendingUpgrade;
         this.existingAddon.pendingUpgrade = null;
       }
 
@@ -1850,19 +1822,16 @@ class AddonInstall {
       return this.installLocation.releaseStagingDir();
     });
   }
 
   /**
    * Stages an upgrade for next application restart.
    */
   async stageInstall(restartRequired, stagedAddon, isUpgrade) {
-    let stagedJSON = stagedAddon.clone();
-    stagedJSON.leafName = this.addon.id + ".json";
-
     // First stage the file regardless of whether restarting is necessary
     if (this.addon.unpack) {
       logger.debug("Addon " + this.addon.id + " will be installed as " +
                    "an unpacked directory");
       stagedAddon.leafName = this.addon.id;
       await OS.File.makeDir(stagedAddon.path);
       await ZipUtils.extractFilesAsync(this.file, stagedAddon);
     } else {
@@ -1872,34 +1841,32 @@ class AddonInstall {
       await OS.File.copy(this.file.path, stagedAddon.path);
     }
 
     if (restartRequired) {
       // Point the add-on to its extracted files as the xpi may get deleted
       this.addon._sourceBundle = stagedAddon;
 
       // Cache the AddonInternal as it may have updated compatibility info
-      writeStringToFile(stagedJSON, JSON.stringify(this.addon));
+      XPIStates.getLocation(this.installLocation.name).stageAddon(this.addon.id,
+                                                                  this.addon.toJSON());
 
-      logger.debug("Staged install of " + this.addon.id + " from " + this.sourceURI.spec + " ready; waiting for restart.");
+      logger.debug(`Staged install of ${this.addon.id} from ${this.sourceURI.spec} ready; waiting for restart.`);
       if (isUpgrade) {
         delete this.existingAddon.pendingUpgrade;
         this.existingAddon.pendingUpgrade = this.addon;
       }
     }
   }
 
   /**
    * Removes any previously staged upgrade.
    */
   async unstageInstall(stagedAddon) {
-    let stagedJSON = getFile(`${this.addon.id}.json`, stagedAddon);
-    if (stagedJSON.exists()) {
-      stagedJSON.remove(true);
-    }
+    XPIStates.getLocation(this.installLocation.name).unstageAddon(this.addon.id);
 
     await removeAsync(getFile(this.addon.id, stagedAddon));
 
     await removeAsync(getFile(`${this.addon.id}.xpi`, stagedAddon));
   }
 
   /**
     * Postone a pending update, until restart or until the add-on resumes.
@@ -2460,44 +2427,16 @@ var DownloadAddonInstall = class extends
       return this;
     }
 
     return this.badCertHandler.getInterface(iid);
   }
 };
 
 /**
- * This class exists just for the specific case of staged add-ons that
- * fail to install at startup.  When that happens, the add-on remains
- * staged but we want to keep track of it like other installs so that we
- * can clean it up if the same add-on is installed again (see the comment
- * about "pending installs for the same add-on" in AddonInstall.startInstall)
- */
-var StagedAddonInstall = class extends AddonInstall {
-  constructor(installLocation, dir, manifest) {
-    super(installLocation, dir);
-
-    this.name = manifest.name;
-    this.type = manifest.type;
-    this.version = manifest.version;
-    this.icons = manifest.icons;
-    this.releaseNotesURI = manifest.releaseNotesURI ?
-                           Services.io.newURI(manifest.releaseNotesURI) :
-                           null;
-    this.sourceURI = manifest.sourceURI ?
-                     Services.io.newURI(manifest.sourceURI) :
-                     null;
-    this.file = null;
-    this.addon = manifest;
-
-    this.state = AddonManager.STATE_INSTALLED;
-  }
-};
-
-/**
  * Creates a new AddonInstall for an update.
  *
  * @param  aCallback
  *         The callback to pass the new AddonInstall to
  * @param  aAddon
  *         The add-on being updated
  * @param  aUpdate
  *         The metadata about the new version from the update manifest
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -29,17 +29,16 @@ XPCOMUtils.defineLazyModuleGetters(this,
   UpdateUtils: "resource://gre/modules/UpdateUtils.jsm",
   JSONFile: "resource://gre/modules/JSONFile.jsm",
   LegacyExtensionsUtils: "resource://gre/modules/LegacyExtensionsUtils.jsm",
   setTimeout: "resource://gre/modules/Timer.jsm",
   clearTimeout: "resource://gre/modules/Timer.jsm",
 
   DownloadAddonInstall: "resource://gre/modules/addons/XPIInstall.jsm",
   LocalAddonInstall: "resource://gre/modules/addons/XPIInstall.jsm",
-  StagedAddonInstall: "resource://gre/modules/addons/XPIInstall.jsm",
   UpdateChecker: "resource://gre/modules/addons/XPIInstall.jsm",
   loadManifestFromFile: "resource://gre/modules/addons/XPIInstall.jsm",
   verifyBundleSignedState: "resource://gre/modules/addons/XPIInstall.jsm",
 });
 
 const {nsIBlocklistService} = Ci;
 
 XPCOMUtils.defineLazyServiceGetters(this, {
@@ -1329,31 +1328,35 @@ class XPIState {
  *        The persisted JSON state data to restore.
  */
 class XPIStateLocation extends Map {
   constructor(name, path, saved = {}) {
     super();
 
     this.name = name;
     this.path = path || saved.path || null;
+    this.staged = saved.staged || {};
     this.dir = this.path && new nsIFile(this.path);
 
     for (let [id, data] of Object.entries(saved.addons || {})) {
       let xpiState = this._addState(id, data);
       // Make a note that this state was restored from saved data.
       xpiState.wasRestored = true;
     }
   }
 
   /**
    * Returns a JSON-compatible representation of this location's state
    * data, to be saved to addonStartup.json.
    */
   toJSON() {
-    let json = { addons: {} };
+    let json = {
+      addons: {},
+      staged: this.staged,
+    };
 
     if (this.path) {
       json.path = this.path;
     }
 
     if (STARTUP_MTIME_SCOPES.includes(this.name)) {
       json.checkStartupModifications = true;
     }
@@ -1361,16 +1364,23 @@ class XPIStateLocation extends Map {
     for (let [id, addon] of this.entries()) {
       if (addon.type != "experiment") {
         json.addons[id] = addon;
       }
     }
     return json;
   }
 
+  get hasStaged() {
+    for (let key in this.staged) {
+      return true;
+    }
+    return false;
+  }
+
   _addState(addonId, saved) {
     let xpiState = new XPIState(this, addonId, saved);
     this.set(addonId, xpiState);
     return xpiState;
   }
 
   /**
    * Adds state data for the given DB add-on to the DB.
@@ -1398,16 +1408,52 @@ class XPIStateLocation extends Map {
    */
   addFile(addonId, file) {
     let xpiState = this._addState(addonId, {enabled: false, file: file.clone()});
     xpiState.getModTime(xpiState.file, addonId);
     return xpiState;
   }
 
   /**
+   * Adds metadata for a staged install which should be performed after
+   * the next restart.
+   *
+   * @param {string} addonId
+   *        The ID of the staged install. The leaf name of the XPI
+   *        within the location's staging directory must correspond to
+   *        this ID.
+   * @param {object} metadata
+   *        The JSON metadata of the parsed install, to be used during
+   *        the next startup.
+   */
+  stageAddon(addonId, metadata) {
+    this.staged[addonId] = metadata;
+    XPIStates.save();
+  }
+
+  /**
+   * Removes staged install metadata for the given add-on ID.
+   *
+   * @param {string} addonId
+   *        The ID of the staged install.
+   */
+  unstageAddon(addonId) {
+    if (addonId in this.staged) {
+      delete this.staged[addonId];
+      XPIStates.save();
+    }
+  }
+
+  * getStagedAddons() {
+    for (let [id, metadata] of Object.entries(this.staged)) {
+      yield [id, metadata];
+    }
+  }
+
+  /**
    * Migrates saved state data for the given add-on from the values
    * stored in xpiState and bootstrappedAddons preferences, and adds it to
    * the DB.
    *
    * @param {string} id
    *        The ID of the add-on to migrate.
    * @param {object} state
    *        The add-on's data from the xpiState preference.
@@ -1722,17 +1768,17 @@ var XPIStates = {
     }
 
     this._jsonFile.saveSoon();
   },
 
   toJSON() {
     let data = {};
     for (let [key, loc] of this.db.entries()) {
-      if (key != TemporaryInstallLocation.name && loc.size) {
+      if (key != TemporaryInstallLocation.name && (loc.size || loc.hasStaged)) {
         data[key] = loc;
       }
     }
     return data;
   },
 
   /**
    * Remove the XPIState for an add-on and save the new state.
@@ -2675,212 +2721,105 @@ var XPIProvider = {
     let changed = false;
     for (let location of this.installLocations) {
       aManifests[location.name] = {};
       // We can't install or uninstall anything in locked locations
       if (location.locked) {
         continue;
       }
 
-      let stagingDir = location.getStagingDir();
-
-      try {
-        if (!stagingDir || !stagingDir.exists() || !stagingDir.isDirectory())
-          continue;
-      } catch (e) {
-        logger.warn("Failed to find staging directory", e);
-        continue;
-      }
-
-      let seenFiles = [];
-      // Use a snapshot of the directory contents to avoid possible issues with
-      // iterating over a directory while removing files from it (the YAFFS2
-      // embedded filesystem has this issue, see bug 772238), and to remove
-      // normal files before their resource forks on OSX (see bug 733436).
-      let stagingDirEntries = getDirectoryEntries(stagingDir, true);
-      for (let stageDirEntry of stagingDirEntries) {
-        let id = stageDirEntry.leafName;
-
-        let isDir;
-        try {
-          isDir = stageDirEntry.isDirectory();
-        } catch (e) {
-          if (e.result != Cr.NS_ERROR_FILE_TARGET_DOES_NOT_EXIST)
-            throw e;
-          // If the file has already gone away then don't worry about it, this
-          // can happen on OSX where the resource fork is automatically moved
-          // with the data fork for the file. See bug 733436.
-          continue;
-        }
-
-        if (!isDir) {
-          if (id.substring(id.length - 4).toLowerCase() == ".xpi") {
-            id = id.substring(0, id.length - 4);
-          } else {
-            if (id.substring(id.length - 5).toLowerCase() != ".json") {
-              logger.warn("Ignoring file: " + stageDirEntry.path);
-              seenFiles.push(stageDirEntry.leafName);
-            }
-            continue;
-          }
-        }
+      let state = XPIStates.getLocation(location.name);
+
+      let cleanNames = [];
+      for (let [id, metadata] of state.getStagedAddons()) {
+        state.unstageAddon(id);
+
+        let source = getFile(`${id}.xpi`, location.getStagingDir());
 
         // Check that the directory's name is a valid ID.
-        if (!gIDTest.test(id)) {
-          logger.warn("Ignoring directory whose name is not a valid add-on ID: " +
-               stageDirEntry.path);
-          seenFiles.push(stageDirEntry.leafName);
+        if (!gIDTest.test(id) || !source.exists() || !source.isFile()) {
+          logger.warn("Ignoring invalid staging directory entry: ${id}", {id});
+          cleanNames.push(source.leafName);
           continue;
         }
 
         changed = true;
-
-        if (isDir) {
-          // Check if the directory contains an install manifest.
-          let manifest = getManifestFileForDir(stageDirEntry);
-
-          // If the install manifest doesn't exist uninstall this add-on in this
-          // install location.
-          if (!manifest) {
-            logger.debug("Processing uninstall of " + id + " in " + location.name);
-
-            try {
-              let addonFile = location.getLocationForID(id);
-              let addonToUninstall = syncLoadManifestFromFile(addonFile, location);
-              if (addonToUninstall.bootstrap) {
-                this.callBootstrapMethod(addonToUninstall, addonToUninstall._sourceBundle,
-                                         "uninstall", BOOTSTRAP_REASONS.ADDON_UNINSTALL);
-              }
-            } catch (e) {
-              logger.warn("Failed to call uninstall for " + id, e);
-            }
-
-            try {
-              location.uninstallAddon(id);
-              XPIStates.removeAddon(location.name, id);
-              seenFiles.push(stageDirEntry.leafName);
-            } catch (e) {
-              logger.error("Failed to uninstall add-on " + id + " in " + location.name, e);
-            }
-            // The file check later will spot the removal and cleanup the database
-            continue;
-          }
-        }
-
         aManifests[location.name][id] = null;
-        let existingAddonID = id;
-
-        let jsonfile = getFile(`${id}.json`, stagingDir);
-        // Assume this was a foreign install if there is no cached metadata file
-        let foreignInstall = !jsonfile.exists();
+
         let addon;
-
         try {
-          addon = syncLoadManifestFromFile(stageDirEntry, location);
+          addon = syncLoadManifestFromFile(source, location);
         } catch (e) {
-          logger.error("Unable to read add-on manifest from " + stageDirEntry.path, e);
-          // This add-on can't be installed so just remove it now
-          seenFiles.push(stageDirEntry.leafName);
-          seenFiles.push(jsonfile.leafName);
+          logger.error(`Unable to read add-on manifest from ${source.path}`, e);
+          cleanNames.push(source.leafName);
           continue;
         }
 
         if (mustSign(addon.type) &&
             addon.signedState <= AddonManager.SIGNEDSTATE_MISSING) {
-          logger.warn("Refusing to install staged add-on " + id + " with signed state " + addon.signedState);
-          seenFiles.push(stageDirEntry.leafName);
-          seenFiles.push(jsonfile.leafName);
+          logger.warn(`Refusing to install staged add-on ${id} with signed state ${addon.signedState}`);
+          cleanNames.push(source.leafName);
           continue;
         }
 
-        // Check for a cached metadata for this add-on, it may contain updated
-        // compatibility information
-        if (!foreignInstall) {
-          logger.debug("Found updated metadata for " + id + " in " + location.name);
-          let fis = Cc["@mozilla.org/network/file-input-stream;1"].
-                       createInstance(Ci.nsIFileInputStream);
-          try {
-            fis.init(jsonfile, -1, 0, 0);
-
-            let bytes = NetUtil.readInputStream(fis, jsonfile.fileSize);
-            let metadata = JSON.parse(gTextDecoder.decode(bytes));
-            addon.importMetadata(metadata);
-
-            // Pass this through to addMetadata so it knows this add-on was
-            // likely installed through the UI
-            aManifests[location.name][id] = addon;
-          } catch (e) {
-            // If some data can't be recovered from the cached metadata then it
-            // is unlikely to be a problem big enough to justify throwing away
-            // the install, just log an error and continue
-            logger.error("Unable to read metadata from " + jsonfile.path, e);
-          } finally {
-            fis.close();
-          }
-        }
-        seenFiles.push(jsonfile.leafName);
-
-        existingAddonID = addon.existingAddonID || id;
+        addon.importMetadata(metadata);
+        aManifests[location.name][id] = addon;
 
         var oldBootstrap = null;
-        logger.debug("Processing install of " + id + " in " + location.name);
-        let existingAddon = XPIStates.findAddon(existingAddonID);
+        logger.debug(`Processing install of ${id} in ${location.name}`);
+        let existingAddon = XPIStates.findAddon(id);
         if (existingAddon && existingAddon.bootstrapped) {
           try {
             var file = existingAddon.file;
             if (file.exists()) {
               oldBootstrap = existingAddon;
 
               // We'll be replacing a currently active bootstrapped add-on so
               // call its uninstall method
               let newVersion = addon.version;
               let oldVersion = existingAddon;
               let uninstallReason = newVersionReason(oldVersion, newVersion);
 
               this.callBootstrapMethod(existingAddon,
                                        file, "uninstall", uninstallReason,
                                        { newVersion });
-              this.unloadBootstrapScope(existingAddonID);
+              this.unloadBootstrapScope(id);
               flushChromeCaches();
             }
           } catch (e) {
+            Cu.reportError(e);
           }
         }
 
         try {
           addon._sourceBundle = location.installAddon({
-            id,
-            source: stageDirEntry,
-            existingAddonID
+            id, source, existingAddonID: id,
           });
           XPIStates.addAddon(addon);
         } catch (e) {
           logger.error("Failed to install staged add-on " + id + " in " + location.name,
                 e);
-          // Re-create the staged install
-          new StagedAddonInstall(location, stageDirEntry, addon);
-          // Make sure not to delete the cached manifest json file
-          seenFiles.pop();
 
           delete aManifests[location.name][id];
 
           if (oldBootstrap) {
             // Re-install the old add-on
             this.callBootstrapMethod(oldBootstrap, existingAddon, "install",
                                      BOOTSTRAP_REASONS.ADDON_INSTALL);
           }
-          continue;
         }
       }
 
       try {
-        location.cleanStagingDir(seenFiles);
+        if (cleanNames.length) {
+          location.cleanStagingDir(cleanNames);
+        }
       } catch (e) {
         // Non-critical, just saves some perf on startup if we clean this up.
-        logger.debug("Error cleaning staging dir " + stagingDir.path, e);
+        logger.debug("Error cleaning staging dir", e);
       }
     }
     return changed;
   },
 
   /**
    * Installs any add-ons located in the extensions directory of the
    * application's distribution specific directory into the profile unless a
--- a/toolkit/mozapps/extensions/test/addons/test_delay_update_ignore_v2/install.rdf
+++ b/toolkit/mozapps/extensions/test/addons/test_delay_update_ignore_v2/install.rdf
@@ -21,8 +21,9 @@
       <Description>
         <em:id>xpcshell@tests.mozilla.org</em:id>
         <em:minVersion>1</em:minVersion>
         <em:maxVersion>1</em:maxVersion>
       </Description>
     </em:targetApplication>
 
   </Description>
+</RDF>