Bug 1096971 - Don't propagate an error when the package downloaded was the same we had. r=fabrice, a=bajaj
authorAntonio M. Amaya <amac@tid.es>
Mon, 17 Nov 2014 02:01:00 +0100
changeset 221522 3148f1ae8a27f0c1eaa231655793fddf1404c8ed
parent 221521 7b161e9fe25e6836df06fc3fe5a8c493267ba50d
child 221523 d92a441f3e88c341a0af8b9a14ffda4d1b39606d
push id272
push userryanvm@gmail.com
push dateTue, 16 Dec 2014 22:17:26 +0000
treeherdermozilla-b2g34_v2_1@e0431e466f13 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfabrice, bajaj
bugs1096971
milestone34.0
Bug 1096971 - Don't propagate an error when the package downloaded was the same we had. r=fabrice, a=bajaj
dom/apps/Webapps.jsm
dom/apps/tests/test_packaged_app_update.html
--- a/dom/apps/Webapps.jsm
+++ b/dom/apps/Webapps.jsm
@@ -3041,17 +3041,17 @@ this.DOMApplicationRegistry = {
 
     if (oldPackage) {
       debug("package's etag or hash unchanged; sending 'applied' event");
       // The package's Etag or hash has not changed.
       // We send an "applied" event right away so code awaiting that event
       // can proceed to access the app. We also throw an error to alert
       // the caller that the package wasn't downloaded.
       this._sendAppliedEvent(aOldApp);
-      throw new Error("PACKAGE_UNCHANGED");
+      throw "PACKAGE_UNCHANGED";
     }
 
     let newManifest = yield this._openAndReadPackage(zipFile, aOldApp, aNewApp,
             isLocalFileInstall, aIsUpdate, aManifest, requestChannel, hash);
 
     return [aOldApp.id, newManifest];
 
   }),
@@ -3700,16 +3700,23 @@ this.DOMApplicationRegistry = {
     // We avoid notifying the error to the DOM side if the app download
     // was cancelled via cancelDownload, which already sends its own
     // notification.
     if (aOldApp.isCanceling) {
       delete aOldApp.isCanceling;
       return;
     }
 
+    // If the error that got us here was that the package hasn't changed,
+    // since we already sent a success and an applied, let's not confuse
+    // the clients...
+    if (aError == "PACKAGE_UNCHANGED") {
+      return;
+    }
+
     let download = AppDownloadManager.get(aNewApp.manifestURL);
     aOldApp.downloading = false;
 
     // If there were not enough storage to download the package we
     // won't have a record of the download details, so we just set the
     // installState to 'pending' at first download and to 'installed' when
     // updating.
     aOldApp.installState = download ? download.previousState
--- a/dom/apps/tests/test_packaged_app_update.html
+++ b/dom/apps/tests/test_packaged_app_update.html
@@ -67,32 +67,40 @@ function checkLastAppState(aMiniManifest
     size: 0,
     readyToApplyDownload: aExpectedReady
   };
 
   PackagedTestHelper.checkAppState(PackagedTestHelper.gApp, aExpectedVersion,
                                    expected, true, false, aCb);
 }
 
-function updateApp(aExpectedReady, aPreviousVersion, aNextVersion) {
+function updateApp(aExpectedReady, aPreviousVersion, aNextVersion, aFailOnError) {
   var lApp = PackagedTestHelper.gApp;
 
   var ondownloadappliedhandler =
     checkLastAppState.bind(PackagedTestHelper, miniManifestURL, false, false,
-                           aNextVersion, PackagedTestHelper.next);
+                           aNextVersion,
+                           setTimeout.bind(undefined, PackagedTestHelper.next, 500));
 
   var ondownloadsuccesshandler =
     checkLastAppState.bind(undefined, miniManifestURL,
                            aExpectedReady, false, aPreviousVersion,
                            function() {
       navigator.mozApps.mgmt.applyDownload(lApp);
   });
 
+  var ondownloaderrorhandler = aFailOnError ?
+    function() {
+      ok(false, "We should not get an error but got " +
+         lApp.downloadError.name);
+      PackagedTestHelper.finish();
+    } : null;
+
   checkForUpdate(true, ondownloadsuccesshandler, ondownloadappliedhandler,
-                 null, true);
+                 ondownloaderrorhandler, true);
 
 }
 
 var initialPermissionState = {
   "geolocation": "prompt",
   "audio-capture": "prompt",
   "video-capture": "prompt",
   "test-permission-read": "prompt",
@@ -233,17 +241,17 @@ var steps = [
     info("== TEST == Check for Update after getting a new package");
     checkForUpdate(false);
   },
   function() {
     PackagedTestHelper.setAppVersion(4, PackagedTestHelper.next, true);
   },
   function() {
     info("== TEST == Update packaged app - same package");
-    updateApp(false, 3, 3);
+    updateApp(false, 3, 3, true);
   },
   function() {
     info("== TEST == Check for Update after getting the same package");
     checkForUpdate(false);
   },
   function() {
     PackagedTestHelper.setAppVersion(1, PackagedTestHelper.next);
   },