Bug 1287176 client code - Status file not written for staging errors. r=mhowell
authorRobert Strong <robert.bugzilla@gmail.com>
Fri, 12 Aug 2016 22:52:24 -0700
changeset 309272 10819baa58fc1f13edf81e9fb4e4ee19a1917034
parent 309271 1d41a61a39b31df7bb9adc3269ced2fec688ad2c
child 309273 49a53da6979c0139592e018b3e0fc25030ee2180
push id80550
push userrstrong@mozilla.com
push dateSat, 13 Aug 2016 05:53:46 +0000
treeherdermozilla-inbound@49a53da6979c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmhowell
bugs1287176
milestone51.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 1287176 client code - Status file not written for staging errors. r=mhowell
toolkit/mozapps/update/nsUpdateService.js
toolkit/mozapps/update/updater/updater.cpp
--- a/toolkit/mozapps/update/nsUpdateService.js
+++ b/toolkit/mozapps/update/nsUpdateService.js
@@ -1081,29 +1081,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;
@@ -1121,17 +1124,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);
@@ -2133,46 +2136,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);
@@ -3114,17 +3124,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;
         }
       }
@@ -3191,17 +3207,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() {
@@ -3209,41 +3225,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));