Bug 1287176 client code - Status file not written for staging errors. r=mhowell, a=rkothari
authorRobert Strong <robert.bugzilla@gmail.com>
Mon, 22 Aug 2016 12:48:16 -0700
changeset 332825 ddd7e7dd100e8f631696a0060f7ac5bcb2345d0b
parent 332824 7b44f5b3b0bb503b79166ac861b1f3f9c1f8abb9
child 332826 89e811fde1ac900bdd55b071d3d397dc80afb31d
push id9930
push userrstrong@mozilla.com
push dateMon, 22 Aug 2016 19:49:11 +0000
treeherdermozilla-aurora@07189ee91625 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmhowell, rkothari
bugs1287176
milestone50.0a2
Bug 1287176 client code - Status file not written for staging errors. r=mhowell, a=rkothari
toolkit/mozapps/update/nsUpdateService.js
toolkit/mozapps/update/updater/updater.cpp
--- a/toolkit/mozapps/update/nsUpdateService.js
+++ b/toolkit/mozapps/update/nsUpdateService.js
@@ -1082,29 +1082,32 @@ function cleanUpMozUpdaterDirs() {
       }
     }
   } catch (e) {
     LOG("cleanUpMozUpdaterDirs - Exception: " + e);
   }
 }
 
 /**
- * Removes the contents of the Updates Directory
+ * Removes the contents of the updates patch directory and rotates the update
+ * logs when present. If the update.log exists in the patch directory this will
+ * move the last-update.log if it exists to backup-update.log in the parent
+ * directory of the patch directory and then move the update.log in the patch
+ * directory to last-update.log in the parent directory of the patch directory.
  *
- * @param aBackgroundUpdate Whether the update has been performed in the
- *        background.  If this is true, we move the update log file to the
- *        updated directory, so that it survives replacing the directories
- *        later on.
+ * @param aRemovePatchFiles (optional, defaults to true)
+ *        When true the update's patch directory contents are removed.
  */
-function cleanUpUpdatesDir(aBackgroundUpdate) {
-  // Bail out if we don't have appropriate permissions
+function cleanUpUpdatesDir(aRemovePatchFiles = true) {
   let updateDir;
   try {
     updateDir = getUpdatesDir();
   } catch (e) {
+    LOG("cleanUpUpdatesDir - unable to get the updates patch directory. " +
+        "Exception: " + e);
     return;
   }
 
   // Preserve the last update log file for debugging purposes.
   let file = updateDir.clone();
   file.append(FILE_UPDATE_LOG);
   if (file.exists()) {
     let dir = updateDir.parent;
@@ -1122,17 +1125,17 @@ function cleanUpUpdatesDir(aBackgroundUp
     try {
       file.moveTo(dir, FILE_LAST_UPDATE_LOG);
     } catch (e) {
       LOG("cleanUpUpdatesDir - failed to rename file " + file.path +
           " to " + FILE_LAST_UPDATE_LOG);
     }
   }
 
-  if (!aBackgroundUpdate) {
+  if (aRemovePatchFiles) {
     let e = updateDir.directoryEntries;
     while (e.hasMoreElements()) {
       let f = e.getNext().QueryInterface(Ci.nsIFile);
       if (AppConstants.platform == "gonk") {
         if (f.leafName == FILE_UPDATE_LINK) {
           let linkedFile = getFileFromUpdateLink(updateDir);
           if (linkedFile && linkedFile.exists()) {
             linkedFile.remove(false);
@@ -2134,46 +2137,53 @@ UpdateService.prototype = {
       if (status != STATE_SUCCEEDED) {
         LOG("UpdateService:_postUpdateProcessing - previous patch failed " +
             "and no patch available");
         cleanupActiveUpdate();
         return;
       }
       update = new Update(null);
     }
-    update.state = status;
+
+    let parts = status.split(":");
+    update.state = parts[0];
+    if (update.state == STATE_FAILED && parts[1]) {
+      update.errorCode = parseInt(parts[1]);
+    }
+
+
+    if (status != STATE_SUCCEEDED) {
+      // Since the update didn't succeed save a copy of the active update's
+      // current state to the updates.xml so it is possible to track failures.
+      um.saveUpdates();
+      // Rotate the update logs so the update log isn't removed. By passing
+      // false the patch directory won't be removed.
+      cleanUpUpdatesDir(false);
+    }
 
     if (status == STATE_SUCCEEDED) {
       update.statusText = gUpdateBundle.GetStringFromName("installSuccess");
 
       // Update the patch's metadata.
       um.activeUpdate = update;
       Services.prefs.setBoolPref(PREF_APP_UPDATE_POSTUPDATE, true);
 
       // Done with this update. Clean it up.
       cleanupActiveUpdate();
     } else if (status == STATE_PENDING_ELEVATE) {
       let prompter = Cc["@mozilla.org/updates/update-prompt;1"].
                      createInstance(Ci.nsIUpdatePrompt);
       prompter.showUpdateElevationRequired();
       return;
     } else {
-      // If we hit an error, then the error code will be included in the status
-      // string following a colon and a space. If we had an I/O error, then we
-      // assume that the patch is not invalid, and we re-stage the patch so that
-      // it can be attempted again the next time we restart. This will leave a
-      // space at the beginning of the error code when there is a failure which
-      // will be removed by using parseInt below. This prevents panic which has
-      // occurred numerous times previously (see bug 569642 comment #9 for one
-      // example) when testing releases due to forgetting to include the space.
-      var ary = status.split(":");
-      update.state = ary[0];
-      if (update.state == STATE_FAILED && ary[1]) {
-        if (handleUpdateFailure(update, ary[1])) {
-          cleanUpUpdatesDir(true);
+      // If there was an I/O error it is assumed that the patch is not invalid
+      // and it is set to pending so an attempt to apply it again will happen
+      // when the application is restarted.
+      if (update.state == STATE_FAILED && update.errorCode) {
+        if (handleUpdateFailure(update, update.errorCode)) {
           return;
         }
       }
 
       // Something went wrong with the patch application process.
       handleFallbackToCompleteUpdate(update, false);
       let prompter = Cc["@mozilla.org/updates/update-prompt;1"].
                      createInstance(Ci.nsIUpdatePrompt);
@@ -3115,17 +3125,23 @@ UpdateManager.prototype = {
    *          The nsIUpdate object to add.
    */
   _addUpdate: function UM__addUpdate(update) {
     if (!update)
       return;
     this._ensureUpdates();
     if (this._updates) {
       for (var i = 0; i < this._updates.length; ++i) {
-        if (this._updates[i] &&
+        // Keep all update entries with a state of STATE_FAILED and replace the
+        // first update entry that has the same application version and build ID
+        // if it exists. This allows the update history to only have one success
+        // entry for an update and entries for all failed updates.
+        if (update.state != STATE_FAILED &&
+            this._updates[i] &&
+            this._updates[i].state != STATE_FAILED &&
             this._updates[i].appVersion == update.appVersion &&
             this._updates[i].buildID == update.buildID) {
           // Replace the existing entry with the new value, updating
           // all metadata.
           this._updates[i] = update;
           return;
         }
       }
@@ -3192,17 +3208,17 @@ UpdateManager.prototype = {
         if (state == STATE_NONE || state == STATE_DOWNLOADING ||
             state == STATE_APPLIED || state == STATE_APPLIED_SERVICE ||
             state == STATE_PENDING || state == STATE_PENDING_SERVICE ||
             state == STATE_PENDING_ELEVATE) {
           updates.splice(i, 1);
         }
       }
 
-      this._writeUpdatesToXMLFile(updates.slice(0, 10),
+      this._writeUpdatesToXMLFile(updates.slice(0, 20),
                                   getUpdateFile([FILE_UPDATES_XML]));
     }
   },
 
   /**
    * See nsIUpdateService.idl
    */
   refreshUpdateStatus: function UM_refreshUpdateStatus() {
@@ -3210,41 +3226,39 @@ UpdateManager.prototype = {
     if (!update) {
       return;
     }
     var updateSucceeded = true;
     var status = readStatusFile(getUpdatesDir());
     pingStateAndStatusCodes(update, false, status);
     var parts = status.split(":");
     update.state = parts[0];
-
-    // Move the update log to the last update log so it isn't removed when
-    // falling back to a complete update.
-    cleanUpUpdatesDir(true);
+    if (update.state == STATE_FAILED && parts[1]) {
+      update.errorCode = parseInt(parts[1]);
+    }
+    let um = Cc["@mozilla.org/updates/update-manager;1"].
+             getService(Ci.nsIUpdateManager);
+    // Save a copy of the active update's current state to the updates.xml so
+    // it is possible to track failures.
+    um.saveUpdates();
+
+    // Rotate the update logs so the update log isn't removed if a complete
+    // update is downloaded. By passing false the patch directory won't be
+    // removed.
+    cleanUpUpdatesDir(false);
 
     if (update.state == STATE_FAILED && parts[1]) {
       updateSucceeded = false;
       if (!handleUpdateFailure(update, parts[1])) {
         handleFallbackToCompleteUpdate(update, true);
       }
     }
     if (update.state == STATE_APPLIED && shouldUseService()) {
       writeStatusFile(getUpdatesDir(), update.state = STATE_APPLIED_SERVICE);
     }
-    var um = Cc["@mozilla.org/updates/update-manager;1"].
-             getService(Ci.nsIUpdateManager);
-    um.saveUpdates();
-
-    if (update.state != STATE_PENDING &&
-        update.state != STATE_PENDING_SERVICE &&
-        update.state != STATE_PENDING_ELEVATE) {
-      // If the update has not fallen back to a non staged update destroy the
-      // updates directory since a new update will be downloaded.
-      cleanUpUpdatesDir(updateSucceeded);
-    }
 
     // Send an observer notification which the update wizard uses in
     // order to update its UI.
     LOG("UpdateManager:refreshUpdateStatus - Notifying observers that " +
         "the update was staged. state: " + update.state + ", status: " + status);
     Services.obs.notifyObservers(null, "update-staged", update.state);
 
     if (AppConstants.platform == "gonk") {
--- a/toolkit/mozapps/update/updater/updater.cpp
+++ b/toolkit/mozapps/update/updater/updater.cpp
@@ -2529,16 +2529,18 @@ UpdateThreadFunc(void *param)
     // to non-staged updates in case of a failure.  We do this by
     // setting the status to pending, exiting the updater, and
     // launching the callback application.  The callback application's
     // startup path will see the pending status, and will start the
     // updater application again in order to apply the update without
     // staging.
     if (sReplaceRequest) {
       WriteStatusFile(sUsingService ? "pending-service" : "pending");
+    } else {
+      WriteStatusFile(rv);
     }
 #ifdef TEST_UPDATER
     // Some tests need to use --test-process-updates again.
     putenv(const_cast<char*>("MOZ_TEST_PROCESS_UPDATES="));
 #endif
   } else {
     if (rv) {
       LOG(("failed: %d", rv));