Bug 1426214 - Clean up XPIProvider.tryGetMTime's handling of null arguments;r?kmag draft
authorDavid Teller <dteller@mozilla.com>
Tue, 19 Dec 2017 22:29:25 +0100
changeset 713305 36f16e57f128b5aa7a01ff3a85312460f12a8a61
parent 706650 de1f7a92e8726bdd365d4bbc5e65eaa369fbc20a
child 744318 2a87ed8dfed655f4e2638b947bc6dd91fd2f7cd5
push id93623
push userdteller@mozilla.com
push dateWed, 20 Dec 2017 06:28:05 +0000
reviewerskmag
bugs1426214, 1426145
milestone59.0a1
Bug 1426214 - Clean up XPIProvider.tryGetMTime's handling of null arguments;r?kmag Method `XPIProvider.tryGetMTime` wraps its body within a try/catch to handle XPCOM errors, which also catches TypeError. This will cause general mochitest failures once bug 1426145 lands and starts treating TypeError as a programmer error. Modifying the callers to ensure that we never call it with a `null` argument. MozReview-Commit-ID: D22E0RN0z0z
toolkit/mozapps/extensions/internal/XPIProvider.jsm
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -369,17 +369,18 @@ function getFile(path, base = null) {
 
 /**
  * Returns the modification time of the given file, or 0 if the file
  * does not exist, or cannot be accessed.
  *
  * @param {nsIFile} file
  *        The file to retrieve the modification time for.
  * @returns {double}
- *        The file's modification time, in seconds since the Unix epoch.
+ *        The file's modification time, in seconds since the Unix epoch,
+ *        or 0 if `file.lastModifiedTime` cannot be accessed.
  */
 function tryGetMtime(file) {
   try {
     return file.lastModifiedTime;
   } catch (e) {
     return 0;
   }
 }
@@ -1271,18 +1272,20 @@ class XPIState {
    * @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) {
     // 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}
-    let mtime = (tryGetMtime(getManifestFileForDir(aFile)) ||
-                 tryGetMtime(aFile));
+    let manifestFile = getManifestFileForDir(aFile);
+    let mtime = (manifestFile && tryGetMtime(manifestFile)) ||
+                (aFile && tryGetMtime(aFile)) ||
+                0;
     if (!mtime) {
       logger.warn("Can't get modified time of ${file}", {file: aFile.path});
     }
 
     this.changed = mtime != this.lastModifiedTime;
     this.lastModifiedTime = mtime;
     return this.changed;
   }