Bug 1356826: Part 3 - Get rid of recursive last modified scan. r?rhelmer draft
authorKris Maglione <maglione.k@gmail.com>
Sat, 15 Apr 2017 19:12:20 -0700
changeset 566756 03d456aa5398206243202a48f9efbb4293866c78
parent 566755 1dd5c7cd6e6dc71f2c458963420cf422650800c8
child 566757 c83a5527a7f39db943bfe7406cf595d6a3f450d4
child 566801 38082a4b9ad23b668164f9036f04e7670e13aabc
push id55320
push usermaglione.k@gmail.com
push dateSun, 23 Apr 2017 05:18:40 +0000
reviewersrhelmer
bugs1356826
milestone55.0a1
Bug 1356826: Part 3 - Get rid of recursive last modified scan. r?rhelmer This scan no longer serves a useful purpose in production, now that mandatory extension signing requires manifest changes after update. MozReview-Commit-ID: 817qRuyzL5P
toolkit/mozapps/extensions/internal/XPIProvider.jsm
toolkit/mozapps/extensions/internal/XPIProviderUtils.js
toolkit/mozapps/extensions/test/addons/test_bug594058/directory/file1
toolkit/mozapps/extensions/test/addons/test_bug594058/install.rdf
toolkit/mozapps/extensions/test/xpcshell/test_XPIStates.js
toolkit/mozapps/extensions/test/xpcshell/test_bootstrap.js
toolkit/mozapps/extensions/test/xpcshell/test_bug594058.js
toolkit/mozapps/extensions/test/xpcshell/test_signed_inject.js
toolkit/mozapps/extensions/test/xpcshell/xpcshell-shared.ini
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -2045,56 +2045,16 @@ function recursiveRemove(aFile) {
     aFile.remove(true);
   } catch (e) {
     logger.error("Failed to remove empty directory " + aFile.path, e);
     throw e;
   }
 }
 
 /**
- * Returns the timestamp and leaf file name of the most recently modified
- * entry in a directory,
- * or simply the file's own timestamp if it is not a directory.
- * Also returns the total number of items (directories and files) visited in the scan
- *
- * @param  aFile
- *         A non-null nsIFile object
- * @return [File Name, Epoch time, items visited], as described above.
- */
-function recursiveLastModifiedTime(aFile) {
-  try {
-    let modTime = aFile.lastModifiedTime;
-    let fileName = aFile.leafName;
-    if (aFile.isFile())
-      return [fileName, modTime, 1];
-
-    if (aFile.isDirectory()) {
-      let entries = aFile.directoryEntries.QueryInterface(Ci.nsIDirectoryEnumerator);
-      let entry;
-      let totalItems = 1;
-      while ((entry = entries.nextFile)) {
-        let [subName, subTime, items] = recursiveLastModifiedTime(entry);
-        totalItems += items;
-        if (subTime > modTime) {
-          modTime = subTime;
-          fileName = subName;
-        }
-      }
-      entries.close();
-      return [fileName, modTime, totalItems];
-    }
-  } catch (e) {
-    logger.warn("Problem getting last modified time for " + aFile.path, e);
-  }
-
-  // If the file is something else, just ignore it.
-  return ["", 0, 0];
-}
-
-/**
  * Gets a snapshot of directory entries.
  *
  * @param  aDir
  *         Directory to look at
  * @param  aSortEntries
  *         True to sort entries by filename
  * @return An array of nsIFile, or an empty array if aDir is not a readable directory
  */
@@ -2179,39 +2139,26 @@ XPIState.prototype = {
    * Update the last modified time for an add-on on disk.
    * @param aFile: nsIFile path of the add-on.
    * @param aId: The add-on ID.
    * @return True if the time stamp has changed.
    */
   getModTime(aFile, aId) {
     let changed = false;
     let scanStarted = Cu.now();
-    // For an unknown or enabled add-on, we do a full recursive scan.
-    if (!("scanTime" in this) || this.enabled) {
-      logger.debug("getModTime: Recursive scan of " + aId);
-      let [modFile, modTime, items] = recursiveLastModifiedTime(aFile);
-      XPIProvider._mostRecentlyModifiedFile[aId] = modFile;
-      XPIProvider.setTelemetry(aId, "scan_items", items);
-      if (modTime != this.scanTime) {
-        this.scanTime = modTime;
-        changed = true;
-      }
-    }
-    // if the add-on is disabled, modified time is the install manifest time, if
-    // any. If no manifest exists, we assume this is a packed .xpi and use
-    // the time stamp of {path}
+    // Modified time is the install manifest time, if any. If no manifest
+    // exists, we assume this is a packed .xpi and use the time stamp of
+    // {path}
     try {
       // Get the install manifest update time, if any.
       let maniFile = getManifestFileForDir(aFile);
-      if (!(aId in XPIProvider._mostRecentlyModifiedFile)) {
-        XPIProvider._mostRecentlyModifiedFile[aId] = maniFile.leafName;
-      }
       let maniTime = maniFile.lastModifiedTime;
       if (maniTime != this.manifestTime) {
         this.manifestTime = maniTime;
+        this.scanTime = maniTime;
         changed = true;
       }
     } catch (e) {
       // No manifest
       delete this.manifestTime;
       try {
         let dtime = aFile.lastModifiedTime;
         if (dtime != this.scanTime) {
@@ -2525,19 +2472,16 @@ this.XPIProvider = {
   extensionsActive: false,
   // True if all of the add-ons found during startup were installed in the
   // application install location
   allAppGlobal: true,
   // A string listing the enabled add-ons for annotating crash reports
   enabledAddons: null,
   // Keep track of startup phases for telemetry
   runPhase: XPI_STARTING,
-  // Keep track of the newest file in each add-on, in case we want to
-  // report it to telemetry.
-  _mostRecentlyModifiedFile: {},
   // Per-addon telemetry information
   _telemetryDetails: {},
   // A Map from an add-on install to its ID
   _addonFileMap: new Map(),
   // Flag to know if ToolboxProcess.jsm has already been loaded by someone or not
   _toolboxProcessLoaded: false,
   // Have we started shutting down bootstrap add-ons?
   _closing: false,
--- a/toolkit/mozapps/extensions/internal/XPIProviderUtils.js
+++ b/toolkit/mozapps/extensions/internal/XPIProviderUtils.js
@@ -1949,23 +1949,19 @@ this.XPIDatabaseReconcile = {
             // Here the add-on was present in the database and on disk
             recordAddonTelemetry(oldAddon);
 
             // Check if the add-on has been changed outside the XPI provider
             if (oldAddon.updateDate != xpiState.mtime) {
               // Did time change in the wrong direction?
               if (xpiState.mtime < oldAddon.updateDate) {
                 XPIProvider.setTelemetry(oldAddon.id, "olderFile", {
-                  name: XPIProvider._mostRecentlyModifiedFile[id],
                   mtime: xpiState.mtime,
                   oldtime: oldAddon.updateDate
                 });
-              } else {
-                XPIProvider.setTelemetry(oldAddon.id, "modifiedFile",
-                                         XPIProvider._mostRecentlyModifiedFile[id]);
               }
             }
 
             // The add-on has changed if the modification time has changed, if
             // we have an updated manifest for it, or if the schema version has
             // changed.
             //
             // Also reload the metadata for add-ons in the application directory
deleted file mode 100644
deleted file mode 100644
--- a/toolkit/mozapps/extensions/test/addons/test_bug594058/install.rdf
+++ /dev/null
@@ -1,21 +0,0 @@
-<?xml version="1.0"?>
-
-<RDF xmlns="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
-     xmlns:em="http://www.mozilla.org/2004/em-rdf#">
-
-  <Description about="urn:mozilla:install-manifest">
-    <em:id>bug594058@tests.mozilla.org</em:id>
-    <em:version>1.0</em:version>
-    
-    <em:targetApplication>
-      <Description>
-        <em:id>xpcshell@tests.mozilla.org</em:id>
-        <em:minVersion>1</em:minVersion>
-        <em:maxVersion>2</em:maxVersion>
-      </Description>
-    </em:targetApplication>
-    <em:name>bug 594058</em:name>
-    <em:description>stat-based invalidation</em:description>
-    <em:unpack>true</em:unpack>
-  </Description>      
-</RDF>
--- a/toolkit/mozapps/extensions/test/xpcshell/test_XPIStates.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_XPIStates.js
@@ -143,30 +143,26 @@ add_task(function* detect_touches() {
   checkChange(XS, pdFile, true);
 
   // We notice changing install.rdf for an enabled unpacked add-on.
   let ueDir = profileDir.clone();
   ueDir.append("unpacked-enabled@tests.mozilla.org");
   let manifest = ueDir.clone();
   manifest.append("install.rdf");
   checkChange(XS, manifest, true);
-  // We also notice changing another file for enabled unpacked add-on.
-  let otherFile = ueDir.clone();
-  otherFile.append("extraFile.js");
-  checkChange(XS, otherFile, true);
 
   // We notice changing install.rdf for a *disabled* unpacked add-on.
   let udDir = profileDir.clone();
   udDir.append("unpacked-disabled@tests.mozilla.org");
   manifest = udDir.clone();
   manifest.append("install.rdf");
   checkChange(XS, manifest, true);
   // Finally, the case we actually care about...
   // We *don't* notice changing another file for disabled unpacked add-on.
-  otherFile = udDir.clone();
+  let otherFile = udDir.clone();
   otherFile.append("extraFile.js");
   checkChange(XS, otherFile, false);
 
   /*
    * When we enable an unpacked add-on that was modified while it was
    * disabled, we reflect the new timestamp in the add-on DB (otherwise, we'll
    * think it changed on next restart).
    */
--- a/toolkit/mozapps/extensions/test/xpcshell/test_bootstrap.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_bootstrap.js
@@ -1048,16 +1048,18 @@ function run_test_21() {
 }
 
 // Check that an upgrade from the filesystem is detected and applied correctly
 function run_test_22() {
   shutdownManager();
 
   let file = manuallyInstall(do_get_addon("test_bootstrap1_1"), profileDir,
                              ID1);
+  if (file.isDirectory())
+    file.append("install.rdf");
 
   // Make it look old so changes are detected
   setExtensionModifiedTime(file, file.lastModifiedTime - 5000);
 
   startupManager();
 
   AddonManager.getAddonByID(ID1, callback_soon(function(b1) {
     // Should have installed and started
deleted file mode 100644
--- a/toolkit/mozapps/extensions/test/xpcshell/test_bug594058.js
+++ /dev/null
@@ -1,88 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
- * http://creativecommons.org/publicdomain/zero/1.0/
- */
-
-// This tests is modifying a file in an unpacked extension
-// causes cache invalidation.
-
-// Disables security checking our updates which haven't been signed
-Services.prefs.setBoolPref("extensions.checkUpdateSecurity", false);
-// Allow the mismatch UI to show
-Services.prefs.setBoolPref("extensions.showMismatchUI", true);
-
-Components.utils.import("resource://testing-common/MockRegistrar.jsm");
-
-var Ci = Components.interfaces;
-const extDir = gProfD.clone();
-extDir.append("extensions");
-
-var gCachePurged = false;
-
-// Override the window watcher
-var WindowWatcher = {
-  openWindow(parent, url, name, features, args) {
-    do_check_false(gCachePurged);
-  },
-
-  QueryInterface(iid) {
-    if (iid.equals(Ci.nsIWindowWatcher)
-     || iid.equals(Ci.nsISupports))
-      return this;
-
-    throw Components.results.NS_ERROR_NO_INTERFACE;
-  }
-}
-
-MockRegistrar.register("@mozilla.org/embedcomp/window-watcher;1", WindowWatcher);
-
-/**
- * Start the test by installing extensions.
- */
-function run_test() {
-  do_test_pending();
-  gCachePurged = false;
-
-  let obs = AM_Cc["@mozilla.org/observer-service;1"].
-    getService(AM_Ci.nsIObserverService);
-  obs.addObserver({
-    observe(aSubject, aTopic, aData) {
-      gCachePurged = true;
-    }
-  }, "startupcache-invalidate");
-  createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "1", "1");
-
-  startupManager();
-  // nsAppRunner takes care of clearing this when a new app is installed
-  do_check_false(gCachePurged);
-
-  installAllFiles([do_get_addon("test_bug594058")], function() {
-    restartManager();
-    do_check_true(gCachePurged);
-    gCachePurged = false;
-
-    // Now, make it look like we've updated the file. First, start the EM
-    // so it records the bogus old time, then update the file and restart.
-    let extFile = extDir.clone();
-    let pastTime = extFile.lastModifiedTime - 5000;
-    extFile.append("bug594058@tests.mozilla.org");
-    setExtensionModifiedTime(extFile, pastTime);
-    let otherFile = extFile.clone();
-    otherFile.append("directory");
-    otherFile.lastModifiedTime = pastTime;
-    otherFile.append("file1");
-    otherFile.lastModifiedTime = pastTime;
-
-    restartManager();
-    gCachePurged = false;
-
-    otherFile.lastModifiedTime = pastTime + 5000;
-    restartManager();
-    do_check_true(gCachePurged);
-    gCachePurged = false;
-
-    restartManager();
-    do_check_false(gCachePurged);
-
-    do_test_finished();
-  });
-}
--- a/toolkit/mozapps/extensions/test/xpcshell/test_signed_inject.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_signed_inject.js
@@ -20,18 +20,23 @@ const ADDONS = {
 const ID = "test@tests.mozilla.org";
 
 const profileDir = gProfD.clone();
 profileDir.append("extensions");
 
 // Deletes a file from the test add-on in the profile
 function breakAddon(file) {
   if (TEST_UNPACKED) {
-    file.append("test.txt");
-    file.remove(true);
+    let f = file.clone();
+    f.append("test.txt");
+    f.remove(true);
+
+    f = file.clone();
+    f.append("install.rdf");
+    f.lastModifiedTime = Date.now();
   } else {
     var zipW = AM_Cc["@mozilla.org/zipwriter;1"].
                createInstance(AM_Ci.nsIZipWriter);
     zipW.open(file, FileUtils.MODE_RDWR | FileUtils.MODE_APPEND);
     zipW.removeEntry("test.txt", false);
     zipW.close();
   }
 }
--- a/toolkit/mozapps/extensions/test/xpcshell/xpcshell-shared.ini
+++ b/toolkit/mozapps/extensions/test/xpcshell/xpcshell-shared.ini
@@ -146,17 +146,16 @@ run-sequentially = Uses hardcoded ports 
 fail-if = os == "android"
 [test_bug564030.js]
 [test_bug566626.js]
 [test_bug567184.js]
 [test_bug569138.js]
 [test_bug570173.js]
 [test_bug576735.js]
 [test_bug587088.js]
-[test_bug594058.js]
 [test_bug595081.js]
 [test_bug595573.js]
 [test_bug596607.js]
 [test_bug616841.js]
 # Bug 676992: test consistently fails on Android
 fail-if = os == "android"
 [test_bug619730.js]
 tags = blocklist