From 0b5a2dd93dcbe483c12897062b389b0832c905df Mon Sep 17 00:00:00 2001 From: Robert Strong Date: Thu, 9 Sep 2010 12:22:27 -0700 Subject: [PATCH] Bug 546593 - If both a partial and complete update have a verification failure this._update is null, the UI breaks, and the user is not notified of the update failure. r=dtownsend, a=approval2.0 dtownsend --- toolkit/mozapps/update/content/updates.js | 2 +- toolkit/mozapps/update/nsUpdateService.js | 58 ++++++++--------- .../mozapps/update/test/chrome/Makefile.in | 8 +++ ...061_check_verifyFailPartial_noComplete.xul | 55 ++++++++++++++++ ...062_check_verifyFailComplete_noPartial.xul | 55 ++++++++++++++++ ...t_0063_check_verifyFailPartialComplete.xul | 55 ++++++++++++++++ ...heck_verifyFailPartial_successComplete.xul | 55 ++++++++++++++++ ...71_notify_verifyFailPartial_noComplete.xul | 55 ++++++++++++++++ ...72_notify_verifyFailComplete_noPartial.xul | 55 ++++++++++++++++ ..._0073_notify_verifyFailPartialComplete.xul | 59 +++++++++++++++++ ...tify_verifyFailPartial_successComplete.xul | 59 +++++++++++++++++ ...1_error_patchApplyFailure_partial_only.xul | 4 +- ...ror_patchApplyFailure_partial_complete.xul | 7 ++- ..._error_patchApplyFailure_verify_failed.xul | 7 ++- toolkit/mozapps/update/test/chrome/update.sjs | 16 +++-- toolkit/mozapps/update/test/chrome/utils.js | 63 +++++++++---------- toolkit/mozapps/update/test/shared.js | 1 + 17 files changed, 537 insertions(+), 77 deletions(-) create mode 100644 toolkit/mozapps/update/test/chrome/test_0061_check_verifyFailPartial_noComplete.xul create mode 100644 toolkit/mozapps/update/test/chrome/test_0062_check_verifyFailComplete_noPartial.xul create mode 100644 toolkit/mozapps/update/test/chrome/test_0063_check_verifyFailPartialComplete.xul create mode 100644 toolkit/mozapps/update/test/chrome/test_0064_check_verifyFailPartial_successComplete.xul create mode 100644 toolkit/mozapps/update/test/chrome/test_0071_notify_verifyFailPartial_noComplete.xul create mode 100644 toolkit/mozapps/update/test/chrome/test_0072_notify_verifyFailComplete_noPartial.xul create mode 100644 toolkit/mozapps/update/test/chrome/test_0073_notify_verifyFailPartialComplete.xul create mode 100644 toolkit/mozapps/update/test/chrome/test_0074_notify_verifyFailPartial_successComplete.xul diff --git a/toolkit/mozapps/update/content/updates.js b/toolkit/mozapps/update/content/updates.js index b1ae1c7860f..5f1b92746d0 100644 --- a/toolkit/mozapps/update/content/updates.js +++ b/toolkit/mozapps/update/content/updates.js @@ -1540,7 +1540,7 @@ var gDownloadingPage = { switch (status) { case CoR.NS_ERROR_UNEXPECTED: if (u.selectedPatch.state == STATE_DOWNLOAD_FAILED && - u.isCompleteUpdate) { + (u.isCompleteUpdate || u.patchCount != 2)) { // Verification error of complete patch, informational text is held in // the update object. this.removeDownloadListener(); diff --git a/toolkit/mozapps/update/nsUpdateService.js b/toolkit/mozapps/update/nsUpdateService.js index b2168b18a01..455e4122bff 100644 --- a/toolkit/mozapps/update/nsUpdateService.js +++ b/toolkit/mozapps/update/nsUpdateService.js @@ -2608,7 +2608,7 @@ Downloader.prototype = { var message = getStatusTextFromCode(vfCode, vfCode); this._update.statusText = message; - if (this._update.isCompleteUpdate) + if (this._update.isCompleteUpdate || this._update.patchCount != 2) deleteActiveUpdate = true; // Destroy the updates directory, since we're done with it. @@ -2654,13 +2654,11 @@ Downloader.prototype = { this._request = null; if (state == STATE_DOWNLOAD_FAILED) { - if (!this._update.isCompleteUpdate) { - var allFailed = true; - - // If we were downloading a patch and the patch verification phase - // failed, log this and then commence downloading the complete update. + var allFailed = true; + // Check if there is a complete update patch that can be downloaded. + if (!this._update.isCompleteUpdate && this._update.patchCount == 2) { LOG("Downloader:onStopRequest - verification of patch failed, " + - "downloading complete update"); + "downloading complete update patch"); this._update.isCompleteUpdate = true; var status = this.downloadUpdate(this._update); @@ -2669,38 +2667,32 @@ Downloader.prototype = { } else { allFailed = false; } - // This will reset the |.state| property on this._update if a new - // download initiates. } - // if we still fail after trying a complete download, give up completely if (allFailed) { - // In all other failure cases, i.e. we're S.O.L. - no more failing over - // ... + LOG("Downloader:onStopRequest - all update patch downloads failed"); + // If the update UI is not open (e.g. the user closed the window while + // downloading) and if at any point this was a foreground download + // notify the user about the error. If the update was a background + // update there is no notification since the user won't be expecting it. + if (!Services.wm.getMostRecentWindow(UPDATE_WINDOW_NAME)) { + try { + this._update.QueryInterface(Ci.nsIWritablePropertyBag); + var fgdl = this._update.getProperty("foregroundDownload"); + } + catch (e) { + } - // If this was ever a foreground download, and now there is no UI active - // (e.g. because the user closed the download window) and there was an - // error, we must notify now. Otherwise we can keep the failure to - // ourselves since the user won't be expecting it. - try { - this._update.QueryInterface(Ci.nsIWritablePropertyBag); - var fgdl = this._update.getProperty("foregroundDownload"); - } - catch (e) { - } - - if (fgdl == "true") { - var prompter = Cc["@mozilla.org/updates/update-prompt;1"]. - createInstance(Ci.nsIUpdatePrompt); - this._update.QueryInterface(Ci.nsIWritablePropertyBag); - this._update.setProperty("downloadFailed", "true"); - prompter.showUpdateError(this._update); + if (fgdl == "true") { + var prompter = Cc["@mozilla.org/updates/update-prompt;1"]. + createInstance(Ci.nsIUpdatePrompt); + prompter.showUpdateError(this._update); + } } + // Prevent leaking the update object (bug 454964). + this._update = null; } - - // Prevent leaking the update object (bug 454964) - this._update = null; - // the complete download succeeded or total failure was handled, so exit + // A complete download has been initiated or the failure was handled. return; } diff --git a/toolkit/mozapps/update/test/chrome/Makefile.in b/toolkit/mozapps/update/test/chrome/Makefile.in index 9951d8832b4..3fdf3f62e65 100644 --- a/toolkit/mozapps/update/test/chrome/Makefile.in +++ b/toolkit/mozapps/update/test/chrome/Makefile.in @@ -76,6 +76,14 @@ _CHROME_FILES = \ test_0052_check_no_updates.xul \ test_0053_check_billboard_license_noAttr.xul \ test_0054_check_billboard_license_404.xul \ + test_0061_check_verifyFailPartial_noComplete.xul \ + test_0062_check_verifyFailComplete_noPartial.xul \ + test_0063_check_verifyFailPartialComplete.xul \ + test_0064_check_verifyFailPartial_successComplete.xul \ + test_0071_notify_verifyFailPartial_noComplete.xul \ + test_0072_notify_verifyFailComplete_noPartial.xul \ + test_0073_notify_verifyFailPartialComplete.xul \ + test_0074_notify_verifyFailPartial_successComplete.xul \ test_0081_error_patchApplyFailure_partial_only.xul \ test_0082_error_patchApplyFailure_complete_only.xul \ test_0083_error_patchApplyFailure_partial_complete.xul \ diff --git a/toolkit/mozapps/update/test/chrome/test_0061_check_verifyFailPartial_noComplete.xul b/toolkit/mozapps/update/test/chrome/test_0061_check_verifyFailPartial_noComplete.xul new file mode 100644 index 00000000000..8b9315a8a69 --- /dev/null +++ b/toolkit/mozapps/update/test/chrome/test_0061_check_verifyFailPartial_noComplete.xul @@ -0,0 +1,55 @@ + + + + + + + + + + +

+ + 

+
+
diff --git a/toolkit/mozapps/update/test/chrome/test_0062_check_verifyFailComplete_noPartial.xul b/toolkit/mozapps/update/test/chrome/test_0062_check_verifyFailComplete_noPartial.xul
new file mode 100644
index 00000000000..91f21fe0149
--- /dev/null
+++ b/toolkit/mozapps/update/test/chrome/test_0062_check_verifyFailComplete_noPartial.xul
@@ -0,0 +1,55 @@
+
+
+
+
+
+
+
+
+
+
+  

+  
+  

+
+
diff --git a/toolkit/mozapps/update/test/chrome/test_0063_check_verifyFailPartialComplete.xul b/toolkit/mozapps/update/test/chrome/test_0063_check_verifyFailPartialComplete.xul
new file mode 100644
index 00000000000..fca5489ccc7
--- /dev/null
+++ b/toolkit/mozapps/update/test/chrome/test_0063_check_verifyFailPartialComplete.xul
@@ -0,0 +1,55 @@
+
+
+
+
+
+
+
+
+
+
+  

+  
+  

+
+
diff --git a/toolkit/mozapps/update/test/chrome/test_0064_check_verifyFailPartial_successComplete.xul b/toolkit/mozapps/update/test/chrome/test_0064_check_verifyFailPartial_successComplete.xul
new file mode 100644
index 00000000000..759caaa24af
--- /dev/null
+++ b/toolkit/mozapps/update/test/chrome/test_0064_check_verifyFailPartial_successComplete.xul
@@ -0,0 +1,55 @@
+
+
+
+
+
+
+
+
+
+
+  

+  
+  

+
+
diff --git a/toolkit/mozapps/update/test/chrome/test_0071_notify_verifyFailPartial_noComplete.xul b/toolkit/mozapps/update/test/chrome/test_0071_notify_verifyFailPartial_noComplete.xul
new file mode 100644
index 00000000000..2e15d9cc690
--- /dev/null
+++ b/toolkit/mozapps/update/test/chrome/test_0071_notify_verifyFailPartial_noComplete.xul
@@ -0,0 +1,55 @@
+
+
+
+
+
+
+
+
+
+
+  

+  
+  

+
+
diff --git a/toolkit/mozapps/update/test/chrome/test_0072_notify_verifyFailComplete_noPartial.xul b/toolkit/mozapps/update/test/chrome/test_0072_notify_verifyFailComplete_noPartial.xul
new file mode 100644
index 00000000000..4a74004bad1
--- /dev/null
+++ b/toolkit/mozapps/update/test/chrome/test_0072_notify_verifyFailComplete_noPartial.xul
@@ -0,0 +1,55 @@
+
+
+
+
+
+
+
+
+
+
+  

+  
+  

+
+
diff --git a/toolkit/mozapps/update/test/chrome/test_0073_notify_verifyFailPartialComplete.xul b/toolkit/mozapps/update/test/chrome/test_0073_notify_verifyFailPartialComplete.xul
new file mode 100644
index 00000000000..b7dde596d01
--- /dev/null
+++ b/toolkit/mozapps/update/test/chrome/test_0073_notify_verifyFailPartialComplete.xul
@@ -0,0 +1,59 @@
+
+
+
+
+
+
+
+
+
+
+  

+  
+  

+
+
diff --git a/toolkit/mozapps/update/test/chrome/test_0074_notify_verifyFailPartial_successComplete.xul b/toolkit/mozapps/update/test/chrome/test_0074_notify_verifyFailPartial_successComplete.xul
new file mode 100644
index 00000000000..2f49d4fa358
--- /dev/null
+++ b/toolkit/mozapps/update/test/chrome/test_0074_notify_verifyFailPartial_successComplete.xul
@@ -0,0 +1,59 @@
+
+
+
+
+
+
+
+
+
+
+  

+  
+  

+
+
diff --git a/toolkit/mozapps/update/test/chrome/test_0081_error_patchApplyFailure_partial_only.xul b/toolkit/mozapps/update/test/chrome/test_0081_error_patchApplyFailure_partial_only.xul
index 9b5beabee61..fa3716bcfe0 100644
--- a/toolkit/mozapps/update/test/chrome/test_0081_error_patchApplyFailure_partial_only.xul
+++ b/toolkit/mozapps/update/test/chrome/test_0081_error_patchApplyFailure_partial_only.xul
@@ -33,7 +33,9 @@ function runTest() {
                                     STATE_PENDING);
   let updates = getLocalUpdateString(patches, null, null, null,
                                      Services.appinfo.version,
-                                     Services.appinfo.platformVersion);
+                                     Services.appinfo.platformVersion, null,
+                                     null, null, null, null, null, null,
+                                     "false");
 
   writeUpdatesToXMLFile(getLocalUpdatesXMLString(updates), true);
 
diff --git a/toolkit/mozapps/update/test/chrome/test_0083_error_patchApplyFailure_partial_complete.xul b/toolkit/mozapps/update/test/chrome/test_0083_error_patchApplyFailure_partial_complete.xul
index 87303077702..0bd1000e9fc 100644
--- a/toolkit/mozapps/update/test/chrome/test_0083_error_patchApplyFailure_partial_complete.xul
+++ b/toolkit/mozapps/update/test/chrome/test_0083_error_patchApplyFailure_partial_complete.xul
@@ -39,10 +39,13 @@ function runTest() {
   let slowDownloadURL = URL_UPDATE + "?slowDownloadMar=1";
   let patches = getLocalPatchString("partial", null, null, null, null, null,
                                     STATE_PENDING) +
-                getLocalPatchString("complete", slowDownloadURL);
+                getLocalPatchString("complete", slowDownloadURL, null, null,
+                                    null, "false");
   let updates = getLocalUpdateString(patches, null, null, null,
                                      Services.appinfo.version,
-                                     Services.appinfo.platformVersion);
+                                     Services.appinfo.platformVersion, null,
+                                     null, null, null, null, null, null,
+                                     "false");
 
   writeUpdatesToXMLFile(getLocalUpdatesXMLString(updates), true);
 
diff --git a/toolkit/mozapps/update/test/chrome/test_0084_error_patchApplyFailure_verify_failed.xul b/toolkit/mozapps/update/test/chrome/test_0084_error_patchApplyFailure_verify_failed.xul
index d0d70fb5722..3e73a12ec51 100644
--- a/toolkit/mozapps/update/test/chrome/test_0084_error_patchApplyFailure_verify_failed.xul
+++ b/toolkit/mozapps/update/test/chrome/test_0084_error_patchApplyFailure_verify_failed.xul
@@ -40,10 +40,13 @@ function runTest() {
   let patches = getLocalPatchString("partial", null, null, null, null, null,
                                     STATE_PENDING) +
                 getLocalPatchString("complete", slowDownloadURL, "MD5",
-                                    "1234cd43a1c77e30191c53a329a3f99d");
+                                    "1234cd43a1c77e30191c53a329a3f99d", null,
+                                    "false");
   let updates = getLocalUpdateString(patches, null, null, null,
                                      Services.appinfo.version,
-                                     Services.appinfo.platformVersion);
+                                     Services.appinfo.platformVersion, null,
+                                     null, null, null, null, null, null,
+                                     "false");
 
   writeUpdatesToXMLFile(getLocalUpdatesXMLString(updates), true);
 
diff --git a/toolkit/mozapps/update/test/chrome/update.sjs b/toolkit/mozapps/update/test/chrome/update.sjs
index 07a8e2a3c3d..7a3a7e94983 100644
--- a/toolkit/mozapps/update/test/chrome/update.sjs
+++ b/toolkit/mozapps/update/test/chrome/update.sjs
@@ -74,12 +74,20 @@ function handleRequest(aRequest, aResponse) {
     return;
   }
 
-  var patches = getRemotePatchString("complete", SERVICE_URL, "SHA512",
-                                     SHA512_HASH, "775");
+  var hash;
+  var patches = "";
+  if (!params.partialPatchOnly) {
+    hash = SHA512_HASH + (params.invalidCompleteHash ? "e" : "");
+    patches += getRemotePatchString("complete", SERVICE_URL, "SHA512",
+                                    hash, "775");
+  }
 
-  if (!params.completePatchOnly)
+  if (!params.completePatchOnly) {
+    hash = SHA512_HASH + (params.invalidPartialHash ? "e" : "");
     patches += getRemotePatchString("partial", SERVICE_URL, "SHA512",
-                                    SHA512_HASH, "775");
+                                    hash, "775");
+  }
+
   var type = params.type ? params.type : "major";
   var name = params.name ? params.name : "App Update Test";
   var appVersion = params.appVersion ? params.appVersion : "99.9";
diff --git a/toolkit/mozapps/update/test/chrome/utils.js b/toolkit/mozapps/update/test/chrome/utils.js
index 69a3ed1b5f4..664920d5e54 100644
--- a/toolkit/mozapps/update/test/chrome/utils.js
+++ b/toolkit/mozapps/update/test/chrome/utils.js
@@ -155,7 +155,7 @@ const TEST_ADDONS = [ "appdisabled_1", "appdisabled_2",
                       "updateversion_1", "updateversion_2",
                       "userdisabled_1", "userdisabled_2" ];
 
-const DEBUG_DUMP = false;
+const DEBUG = false;
 
 const TEST_TIMEOUT = 30000; // 30 seconds
 var gTimeoutTimer;
@@ -177,7 +177,7 @@ var gDisableNoUpdateAddon = false;
 #include ../shared.js
 
 function debugDump(msg) {
-  if (DEBUG_DUMP) {
+  if (DEBUG) {
     dump("*** " + msg + "\n");
   }
 }
@@ -710,8 +710,9 @@ function setupPrefs() {
   gAppUpdateChannel = gDefaultPrefBranch.getCharPref(PREF_APP_UPDATE_CHANNEL);
   setUpdateChannel();
 
-  // Uncomment for debugging
-//  Services.prefs.setBoolPref(PREF_APP_UPDATE_LOG, true)
+  if (DEBUG) {
+    Services.prefs.setBoolPref(PREF_APP_UPDATE_LOG, true)
+  }
 
   if (Services.prefs.prefHasUserValue(PREF_APP_UPDATE_URL_OVERRIDE)) {
     gAppUpdateURL = Services.prefs.setIntPref(PREF_APP_UPDATE_URL_OVERRIDE);
@@ -731,6 +732,7 @@ function setupPrefs() {
   debugDump("extensions.update.url: " + extUpdateUrl);
 
   Services.prefs.setIntPref(PREF_APP_UPDATE_IDLETIME, 0);
+  Services.prefs.setIntPref(PREF_APP_UPDATE_PROMPTWAITTIME, 0);
 }
 
 /**
@@ -770,6 +772,10 @@ function resetPrefs() {
     Services.prefs.clearUserPref(PREF_APP_UPDATE_IDLETIME);
   }
 
+  if (Services.prefs.prefHasUserValue(PREF_APP_UPDATE_PROMPTWAITTIME)) {
+    Services.prefs.clearUserPref(PREF_APP_UPDATE_PROMPTWAITTIME);
+  }
+
   if (Services.prefs.prefHasUserValue(PREF_APP_UPDATE_URL_DETAILS)) {
     Services.prefs.clearUserPref(PREF_APP_UPDATE_URL_DETAILS);
   }
@@ -1142,46 +1148,38 @@ var certErrorsPrefObserver = {
  * nsIObserver for receiving window open and close notifications.
  */
 var gWindowObserver = {
-  loaded: false,
-
   observe: function WO_observe(aSubject, aTopic, aData) {
     let win = aSubject.QueryInterface(AUS_Ci.nsIDOMEventTarget);
 
     if (aTopic == "domwindowclosed") {
-      if (win.location == URI_UPDATE_PROMPT_DIALOG) {
-        // Allow tests the ability to provide their own function (it must be
-        // named finishTest) for finishing the test.
-        try {
-          finishTest();
-        }
-        catch (e) {
-          finishTestDefault();
-        }
+      if (win.location != URI_UPDATE_PROMPT_DIALOG) {
+        debugDump("gWindowObserver:observe - domwindowclosed event for " +
+                  "window not being tested - location: " + win.location +
+                  "... returning early");
+        return;
+      }
+      // Allow tests the ability to provide their own function (it must be
+      // named finishTest) for finishing the test.
+      try {
+        finishTest();
+      }
+      catch (e) {
+        finishTestDefault();
       }
-      return;
-    }
-
-    // Defensive measure to prevent adding multiple listeners.
-    if (this.loaded) {
-      // This should never happen but if it does this will provide a clue for
-      // diagnosing the cause.
-      ok(false, "Unexpected gWindowObserver:observe - called with aTopic = " +
-         aTopic + "... returning early");
       return;
     }
 
     win.addEventListener("load", function onLoad() {
-      // Defensive measure to prevent windows we shouldn't see from breaking
-      // a test.
+      win.removeEventListener("load", onLoad, false);
+      // Ignore windows other than the update UI window.
       if (win.location != URI_UPDATE_PROMPT_DIALOG) {
-        // This should never happen.
-        ok(false, "Unexpected load event - win.location got: " + location +
-           ", expected: " + URI_UPDATE_PROMPT_DIALOG + "... returning early");
+        debugDump("gWindowObserver:observe:onLoad - load event for window " +
+                  "not being tested - location: " + win.location +
+                  "... returning early");
         return;
       }
 
-      // Defensive measure to prevent an unexpected wizard page from breaking
-      // a test.
+      // The first wizard page should always be the dummy page.
       let pageid = win.document.documentElement.currentPage.pageid;
       if (pageid != PAGEID_DUMMY) {
         // This should never happen but if it does this will provide a clue
@@ -1191,7 +1189,6 @@ var gWindowObserver = {
         return;
       }
 
-      win.removeEventListener("load", onLoad, false);
       gTimeoutTimer = AUS_Cc["@mozilla.org/timer;1"].
                       createInstance(AUS_Ci.nsITimer);
       gTimeoutTimer.initWithCallback(finishTestTimeout, TEST_TIMEOUT,
@@ -1201,7 +1198,5 @@ var gWindowObserver = {
       gDocElem = gWin.document.documentElement;
       gDocElem.addEventListener("pageshow", onPageShowDefault, false);
     }, false);
-
-    this.loaded = true;
   }
 };
diff --git a/toolkit/mozapps/update/test/shared.js b/toolkit/mozapps/update/test/shared.js
index 1ac1fd3a854..81e26c59a0a 100644
--- a/toolkit/mozapps/update/test/shared.js
+++ b/toolkit/mozapps/update/test/shared.js
@@ -54,6 +54,7 @@ const PREF_APP_UPDATE_ENABLED             = "app.update.enabled";
 const PREF_APP_UPDATE_IDLETIME            = "app.update.idletime";
 const PREF_APP_UPDATE_LOG                 = "app.update.log";
 const PREF_APP_UPDATE_NEVER_BRANCH        = "app.update.never.";
+const PREF_APP_UPDATE_PROMPTWAITTIME      = "app.update.promptWaitTime";
 const PREF_APP_UPDATE_SHOW_INSTALLED_UI   = "app.update.showInstalledUI";
 const PREF_APP_UPDATE_URL                 = "app.update.url";
 const PREF_APP_UPDATE_URL_DETAILS         = "app.update.url.details";