Bug 906114 - Wait local installation before sending messages to content. r=myk
authorMarco Castelluccio <mar.castelluccio@studenti.unina.it>
Fri, 11 Jul 2014 13:25:46 +0200
changeset 215497 c43489001722a791d24c2774475bdcb9582f8d2f
parent 215496 d59df02050b3d9e147cb0f82fb9d0bd443c805a6
child 215498 059b364302aff7371bf82aa4f534ad32e6b915e8
push id515
push userraliiev@mozilla.com
push dateMon, 06 Oct 2014 12:51:51 +0000
treeherdermozilla-release@267c7a481bef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmyk
bugs906114
milestone33.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 906114 - Wait local installation before sending messages to content. r=myk
browser/modules/WebappManager.jsm
dom/apps/src/Webapps.jsm
webapprt/WebappManager.jsm
--- a/browser/modules/WebappManager.jsm
+++ b/browser/modules/WebappManager.jsm
@@ -129,55 +129,49 @@ this.WebappManager = {
                              "webapps-notification-icon");
 
         let progressMeter = chromeDoc.createElement("progressmeter");
         progressMeter.setAttribute("mode", "undetermined");
         popupProgressContent.appendChild(progressMeter);
 
         let manifestURL = aData.app.manifestURL;
 
-        let cleanup = () => {
+        let nativeApp = new NativeApp(aData.app, jsonManifest,
+                                      aData.app.categories);
+
+        this.installations[manifestURL] = Promise.defer();
+        this.installations[manifestURL].promise.then(() => {
+          notifyInstallSuccess(aData.app, nativeApp, bundle);
+        }, (error) => {
+          Cu.reportError("Error installing webapp: " + error);
+        }).then(() => {
           popupProgressContent.removeChild(progressMeter);
           delete this.installations[manifestURL];
           if (Object.getOwnPropertyNames(this.installations).length == 0) {
             notification.remove();
           }
-        };
-
-        this.installations[manifestURL] = Promise.defer();
-        this.installations[manifestURL].promise.then(null, (error) => {
-          Cu.reportError("Error installing webapp: " + error);
-          cleanup();
         });
 
-        let nativeApp = new NativeApp(aData.app, jsonManifest,
-                                      aData.app.categories);
         let localDir;
         try {
           localDir = nativeApp.createProfile();
         } catch (ex) {
-          Cu.reportError("Error installing webapp: " + ex);
           DOMApplicationRegistry.denyInstall(aData);
-          cleanup();
           return;
         }
 
         DOMApplicationRegistry.confirmInstall(aData, localDir,
-          (aApp, aManifest, aZipPath) => Task.spawn((function*() {
+          Task.async(function*(aApp, aManifest, aZipPath) {
             try {
               yield nativeApp.install(aApp, aManifest, aZipPath);
-              yield this.installations[manifestURL].promise;
-              notifyInstallSuccess(aApp, nativeApp, bundle);
             } catch (ex) {
               Cu.reportError("Error installing webapp: " + ex);
-              // TODO: Notify user that the installation has failed
-            } finally {
-              cleanup();
+              throw ex;
             }
-          }).bind(this))
+          })
         );
       }
     };
 
     let requestingURI = chromeWin.makeURI(aData.from);
     let manifest = new ManifestHelper(jsonManifest, aData.app.origin);
 
     let host;
--- a/dom/apps/src/Webapps.jsm
+++ b/dom/apps/src/Webapps.jsm
@@ -2547,36 +2547,42 @@ this.DOMApplicationRegistry = {
 
     // We notify about the successful installation via mgmt.oninstall and the
     // corresponding DOMRequest.onsuccess event as soon as the app is properly
     // saved in the registry.
     yield this._saveApps();
 
     this.broadcastMessage("Webapps:AddApp", { id: id, app: appObject });
 
+    if (!aData.isPackage) {
+      this.updateAppHandlers(null, app.manifest, app);
+      if (aInstallSuccessCallback) {
+        try {
+          yield aInstallSuccessCallback(app, app.manifest);
+        } catch (e) {
+          // Ignore exceptions during the local installation of
+          // an app. If it fails, the app will anyway be considered
+          // as not installed because isLaunchable will return false.
+        }
+      }
+    }
+
     // The presence of a requestID means that we have a page to update.
     if (aData.isPackage && aData.apkInstall && !aData.requestID) {
       // Skip directly to onInstallSuccessAck, since there isn't
       // a WebappsRegistry to receive Webapps:Install:Return:OK and respond
       // Webapps:Install:Return:Ack when an app is being auto-installed.
       this.onInstallSuccessAck(app.manifestURL);
     } else {
       // Broadcast Webapps:Install:Return:OK so the WebappsRegistry can notify
       // the installing page about the successful install, after which it'll
       // respond Webapps:Install:Return:Ack, which calls onInstallSuccessAck.
       this.broadcastMessage("Webapps:Install:Return:OK", aData);
     }
 
-    if (!aData.isPackage) {
-      this.updateAppHandlers(null, app.manifest, app);
-      if (aInstallSuccessCallback) {
-        aInstallSuccessCallback(app, app.manifest);
-      }
-    }
-
     Services.obs.notifyObservers(null, "webapps-installed",
       JSON.stringify({ manifestURL: app.manifestURL }));
 
     if (aData.forceSuccessAck) {
       // If it's a local install, there's no content process so just
       // ack the install.
       this.onInstallSuccessAck(app.manifestURL, dontNeedNetwork);
     }
@@ -2637,33 +2643,39 @@ this.DOMApplicationRegistry = {
         origin: aNewApp.origin,
         manifestURL: aNewApp.manifestURL
       }, true);
     }
 
     this.updateDataStore(this.webapps[aId].localId, aNewApp.origin,
                          aNewApp.manifestURL, aManifest);
 
+    if (aInstallSuccessCallback) {
+      try {
+        yield aInstallSuccessCallback(aNewApp, aManifest, zipFile.path);
+      } catch (e) {
+        // Ignore exceptions during the local installation of
+        // an app. If it fails, the app will anyway be considered
+        // as not installed because isLaunchable will return false.
+      }
+    }
+
     this.broadcastMessage("Webapps:UpdateState", {
       app: app,
       manifest: aManifest,
       manifestURL: aNewApp.manifestURL
     });
 
     // Check if we have asm.js code to preload for this application.
     yield ScriptPreloader.preload(aNewApp, aManifest);
 
     this.broadcastMessage("Webapps:FireEvent", {
       eventType: ["downloadsuccess", "downloadapplied"],
       manifestURL: aNewApp.manifestURL
     });
-
-    if (aInstallSuccessCallback) {
-      aInstallSuccessCallback(aNewApp, aManifest, zipFile.path);
-    }
   }),
 
   _nextLocalId: function() {
     let id = Services.prefs.getIntPref("dom.mozApps.maxLocalId") + 1;
 
     while (this.getManifestURLByLocalId(id)) {
       id++;
     }
--- a/webapprt/WebappManager.jsm
+++ b/webapprt/WebappManager.jsm
@@ -74,19 +74,19 @@ this.WebappManager = {
       try {
         localDir = nativeApp.createProfile();
       } catch (ex) {
         DOMApplicationRegistry.denyInstall(aData);
         return;
       }
 
       DOMApplicationRegistry.confirmInstall(data, localDir,
-        function (aApp, aManifest, aZipPath) {
-          nativeApp.install(aApp, aManifest, aZipPath);
-        }
+        Task.async(function*(aApp, aManifest, aZipPath) {
+          yield nativeApp.install(aApp, aManifest, aZipPath);
+        })
       );
     } else {
       DOMApplicationRegistry.denyInstall(data);
     }
   },
 
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver,
                                          Ci.nsISupportsWeakReference])