Bug 747552 - During webapp install, icon retrieval may return non-image files. r=myk

browser/modules/webappsUI.jsm

@@ -114,9 +114,15 @@ this.webappsUI = {
 
           DOMApplicationRegistry.confirmInstall(aData, false, localDir, null,
             function (aManifest) {
-              if (WebappsInstaller.install(aData, aManifest)) {
+              WebappsInstaller.install(aData, aManifest).then(
-                installationSuccessNotification(aData, app, chromeWin);
+                function() {
-              }
+                  installationSuccessNotification(aData, app, chromeWin);
+                },
+                function(error) {
+                  Cu.reportError("Error installing webapp: " + error);
+                  // TODO: Notify user that the installation has failed
+                }
+              );
             }
           );
         } else {

toolkit/webapps/WebappsIconHelpers.js

@@ -2,6 +2,8 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+const DEFAULT_ICON_URL = "chrome://global/skin/icons/webapps-64.png";
+
 /**
  * This function receives a list of icon sizes
  * and URLs and returns the url string for the biggest icon.
@@ -17,95 +19,83 @@
  */
 function getBiggestIconURL(aIcons) {
   if (!aIcons) {
-    return "chrome://global/skin/icons/webapps-64.png";
+    return DEFAULT_ICON_URL;
   }
 
   let iconSizes = Object.keys(aIcons);
   if (iconSizes.length == 0) {
-    return "chrome://global/skin/icons/webapps-64.png";
+    return DEFAULT_ICON_URL;
   }
   iconSizes.sort(function(a, b) a - b);
   return aIcons[iconSizes.pop()];
 }
 
-/**
+// Download an icon using either a temp file or a pipe.
- * This function retrieves the icon for an app as specified
+function downloadIcon(aIconURI) {
- * in the iconURI on the shell object.
+  let deferred = Promise.defer();
- * Upon completion it will call aShell.processIcon()
- *
- * @param aShell The shell that specifies the properties
- *               of the native app. Three properties from this
- *               shell will be used in this function:
- *                 - iconURI
- *                 - useTmpForIcon
- *                 - processIcon()
- */
-function getIconForApp(aShell, callback) {
-  let iconURI = aShell.iconURI;
-  let mimeService = Cc["@mozilla.org/mime;1"]
-                      .getService(Ci.nsIMIMEService);
 
+  let mimeService = Cc["@mozilla.org/mime;1"].getService(Ci.nsIMIMEService);
   let mimeType;
   try {
-    let tIndex = iconURI.path.indexOf(";");
+    let tIndex = aIconURI.path.indexOf(";");
-    if("data" == iconURI.scheme && tIndex != -1) {
+    if("data" == aIconURI.scheme && tIndex != -1) {
-      mimeType = iconURI.path.substring(0, tIndex);
+      mimeType = aIconURI.path.substring(0, tIndex);
     } else {
-      mimeType = mimeService.getTypeFromURI(iconURI);
+      mimeType = mimeService.getTypeFromURI(aIconURI);
-    }
+     }
   } catch(e) {
-    throw("getIconFromURI - Failed to determine MIME type");
+    deferred.reject("Failed to determine icon MIME type: " + e);
+    return deferred.promise;
+  }
+
+  function onIconDownloaded(aStatusCode, aIcon) {
+    if (Components.isSuccessCode(aStatusCode)) {
+      deferred.resolve([ mimeType, aIcon ]);
+    } else {
+      deferred.reject("Failure downloading icon: " + aStatusCode);
+    }
   }
 
   try {
-    let listener;
+#ifdef XP_MACOSX
-    if(aShell.useTmpForIcon) {
+    let downloadObserver = {
-      let downloadObserver = {
+      onDownloadComplete: function(downloader, request, cx, aStatus, file) {
-        onDownloadComplete: function(downloader, request, cx, aStatus, file) {
+        onIconDownloaded(aStatus, file);
-          // pass downloader just to keep reference around
+      }
-          onIconDownloaded(aShell, mimeType, aStatus, file, callback, downloader);
+    };
-        }
-      };
 
-      let tmpIcon = Services.dirsvc.get("TmpD", Ci.nsIFile);
+    let tmpIcon = Services.dirsvc.get("TmpD", Ci.nsIFile);
-      tmpIcon.append("tmpicon." + mimeService.getPrimaryExtension(mimeType, ""));
+    tmpIcon.append("tmpicon." + mimeService.getPrimaryExtension(mimeType, ""));
-      tmpIcon.createUnique(Ci.nsIFile.NORMAL_FILE_TYPE, 0666);
+    tmpIcon.createUnique(Ci.nsIFile.NORMAL_FILE_TYPE, parseInt("666", 8));
 
-      listener = Cc["@mozilla.org/network/downloader;1"]
+    let listener = Cc["@mozilla.org/network/downloader;1"]
-                   .createInstance(Ci.nsIDownloader);
+                     .createInstance(Ci.nsIDownloader);
-      listener.init(downloadObserver, tmpIcon);
+    listener.init(downloadObserver, tmpIcon);
-    } else {
+#else
-      let pipe = Cc["@mozilla.org/pipe;1"]
+    let pipe = Cc["@mozilla.org/pipe;1"]
-                   .createInstance(Ci.nsIPipe);
+                 .createInstance(Ci.nsIPipe);
-      pipe.init(true, true, 0, 0xffffffff, null);
+    pipe.init(true, true, 0, 0xffffffff, null);
 
-      listener = Cc["@mozilla.org/network/simple-stream-listener;1"]
+    let listener = Cc["@mozilla.org/network/simple-stream-listener;1"]
-                   .createInstance(Ci.nsISimpleStreamListener);
+                     .createInstance(Ci.nsISimpleStreamListener);
-      listener.init(pipe.outputStream, {
+    listener.init(pipe.outputStream, {
-          onStartRequest: function() {},
+        onStartRequest: function() {},
-          onStopRequest: function(aRequest, aContext, aStatusCode) {
+        onStopRequest: function(aRequest, aContext, aStatusCode) {
-            pipe.outputStream.close();
+          pipe.outputStream.close();
-            onIconDownloaded(aShell, mimeType, aStatusCode, pipe.inputStream, callback);
+          onIconDownloaded(aStatusCode, pipe.inputStream);
-         }
+       }
-      });
+    });
-    }
+#endif
 
-    let channel = NetUtil.newChannel(iconURI);
+    let channel = NetUtil.newChannel(aIconURI);
-    let CertUtils = { };
+    let { BadCertHandler } = Cu.import("resource://gre/modules/CertUtils.jsm", {});
-    Cu.import("resource://gre/modules/CertUtils.jsm", CertUtils);
     // Pass true to avoid optional redirect-cert-checking behavior.
-    channel.notificationCallbacks = new CertUtils.BadCertHandler(true);
+    channel.notificationCallbacks = new BadCertHandler(true);
 
     channel.asyncOpen(listener, null);
   } catch(e) {
-    throw("getIconFromURI - Failure getting icon (" + e + ")");
+    deferred.reject("Failure initiating download of icon: " + e);
   }
-}
 
-function onIconDownloaded(aShell, aMimeType, aStatusCode, aIcon, aCallback) {
+  return deferred.promise;
-  if (Components.isSuccessCode(aStatusCode)) {
-    aShell.processIcon(aMimeType, aIcon, aCallback);
-  } else {
-    aCallback.call(aShell);
-  }
 }

toolkit/webapps/WebappsInstaller.jsm

@@ -16,6 +16,7 @@ Cu.import("resource://gre/modules/osfile.jsm");
 Cu.import("resource://gre/modules/WebappOSUtils.jsm");
 Cu.import("resource://gre/modules/AppsUtils.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
+Cu.import("resource://gre/modules/Promise.jsm");
 
 this.WebappsInstaller = {
   shell: null,
@@ -60,35 +61,27 @@ this.WebappsInstaller = {
    *
    * @param aData the data provided to the install function
    * @param aManifest the manifest data provided by the web app
-   *
-   * @returns true on success, false if an error was thrown
    */
   install: function(aData, aManifest) {
     try {
       if (Services.prefs.getBoolPref("browser.mozApps.installer.dry_run")) {
-        return true;
+        return Promise.resolve();
       }
     } catch (ex) {}
 
     this.shell.init(aData, aManifest);
 
-    try {
+    return this.shell.install().then(() => {
-      this.shell.install();
+      let data = {
-    } catch (ex) {
+        "installDir": this.shell.installDir.path,
-      Cu.reportError("Error installing app: " + ex);
+        "app": {
-      return false;
+          "manifest": aManifest,
-    }
+          "origin": aData.app.origin
+        }
+      };
 
-    let data = {
+      Services.obs.notifyObservers(null, "webapp-installed", JSON.stringify(data));
-      "installDir": this.shell.installDir.path,
+    });
-      "app": {
-        "manifest": aManifest,
-        "origin": aData.app.origin
-      }
-    };
-    Services.obs.notifyObservers(null, "webapp-installed", JSON.stringify(data));
-
-    return true;
   }
 }
 
@@ -192,7 +185,30 @@ NativeApp.prototype = {
     };
 
     this.runtimeFolder = Services.dirsvc.get("GreD", Ci.nsIFile);
-  }
+  },
+
+  /**
+   * This function retrieves the icon for an app.
+   * If the retrieving fails, it uses the default chrome icon.
+   */
+  getIcon: function() {
+    try {
+      let [ mimeType, icon ] = yield downloadIcon(this.iconURI);
+      yield this.processIcon(mimeType, icon);
+    }
+    catch(e) {
+      Cu.reportError("Failure retrieving icon: " + e);
+
+      let iconURI = Services.io.newURI(DEFAULT_ICON_URL, null, null);
+
+      let [ mimeType, icon ] = yield downloadIcon(iconURI);
+      yield this.processIcon(mimeType, icon);
+
+      // Set the iconURI property so that the user notification will have the
+      // correct icon.
+      this.iconURI = iconURI;
+    }
+  },
 };
 
 #ifdef XP_WIN
@@ -243,17 +259,18 @@ WinNativeApp.prototype = {
    *
    */
   install: function() {
-    try {
+    return Task.spawn(function() {
-      this._copyPrebuiltFiles();
+      try {
-      this._createShortcutFiles();
+        this._copyPrebuiltFiles();
-      this._createConfigFiles();
+        this._createShortcutFiles();
-      this._writeSystemKeys();
+        this._createConfigFiles();
-    } catch (ex) {
+        this._writeSystemKeys();
-      this._removeInstallation(false);
+        yield this.getIcon();
-      throw(ex);
+      } catch (ex) {
-    }
+        this._removeInstallation(false);
-
+        throw(ex);
-    getIconForApp(this, function() {});
+      }
+    }.bind(this));
   },
 
   /**
@@ -526,14 +543,6 @@ WinNativeApp.prototype = {
     shortcut.remove(false);
   },
 
-  /**
-   * This variable specifies if the icon retrieval process should
-   * use a temporary file in the system or a binary stream. This
-   * is accessed by a common function in WebappsIconHelpers.js and
-   * is different for each platform.
-   */
-  useTmpForIcon: false,
-
   /**
    * Process the icon from the imageStream as retrieved from
    * the URL by getIconForApp(). This will save the icon to the
@@ -541,28 +550,31 @@ WinNativeApp.prototype = {
    *
    * @param aMimeType     ahe icon mimetype
    * @param aImageStream  the stream for the image data
-   * @param aCallback     a callback function to be called
-   *                      after the process finishes
    */
-  processIcon: function(aMimeType, aImageStream, aCallback) {
+  processIcon: function(aMimeType, aImageStream) {
-    let iconStream;
+    let deferred = Promise.defer();
-    try {
-      let imgTools = Cc["@mozilla.org/image/tools;1"]
-                       .createInstance(Ci.imgITools);
-      let imgContainer = { value: null };
 
-      imgTools.decodeImageData(aImageStream, aMimeType, imgContainer);
+    let imgTools = Cc["@mozilla.org/image/tools;1"]
-      iconStream = imgTools.encodeImage(imgContainer.value,
+                     .createInstance(Ci.imgITools);
-                                        "image/vnd.microsoft.icon",
+
-                                        "format=bmp;bpp=32");
+    let imgContainer = imgTools.decodeImage(aImageStream, aMimeType);
-    } catch (e) {
+    let iconStream = imgTools.encodeImage(imgContainer,
-      throw("processIcon - Failure converting icon (" + e + ")");
+                                          "image/vnd.microsoft.icon",
+                                          "format=bmp;bpp=32");
+
+    if (!this.iconFile.parent.exists()) {
+      this.iconFile.parent.create(Ci.nsIFile.DIRECTORY_TYPE, parseInt("0755", 8));
     }
-
-    if (!this.iconFile.parent.exists())
-      this.iconFile.parent.create(Ci.nsIFile.DIRECTORY_TYPE, 0755);
     let outputStream = FileUtils.openSafeFileOutputStream(this.iconFile);
-    NetUtil.asyncCopy(iconStream, outputStream);
+    NetUtil.asyncCopy(iconStream, outputStream, function(aResult) {
+      if (Components.isSuccessCode(aResult)) {
+        deferred.resolve();
+      } else {
+        deferred.reject("Failure copying icon: " + aResult);
+      }
+    });
+
+    return deferred.promise;
   }
 }
 
@@ -613,15 +625,17 @@ MacNativeApp.prototype = {
   },
 
   install: function() {
-    try {
+    return Task.spawn(function() {
-      this._copyPrebuiltFiles();
+      try {
-      this._createConfigFiles();
+        this._copyPrebuiltFiles();
-    } catch (ex) {
+        this._createConfigFiles();
-      this._removeInstallation(false);
+        yield this.getIcon();
-      throw(ex);
+        this._moveToApplicationsFolder();
-    }
+      } catch (ex) {
-
+        this._removeInstallation(false);
-    getIconForApp(this, this._moveToApplicationsFolder);
+        throw(ex);
+      }
+    }.bind(this));
   },
 
   _removeInstallation: function(keepProfile) {
@@ -722,19 +736,11 @@ MacNativeApp.prototype = {
                                                this.appNameAsFilename,
                                               ".app");
     if (!destinationName) {
-      return false;
+      throw("No available filename");
     }
     this.installDir.moveTo(appDir, destinationName);
   },
 
-  /**
-   * This variable specifies if the icon retrieval process should
-   * use a temporary file in the system or a binary stream. This
-   * is accessed by a common function in WebappsIconHelpers.js and
-   * is different for each platform.
-   */
-  useTmpForIcon: true,
-
   /**
    * Process the icon from the imageStream as retrieved from
    * the URL by getIconForApp(). This will bundle the icon to the
@@ -742,29 +748,33 @@ MacNativeApp.prototype = {
    *
    * @param aMimeType     the icon mimetype
    * @param aImageStream  the stream for the image data
-   * @param aCallback     a callback function to be called
-   *                      after the process finishes
    */
-  processIcon: function(aMimeType, aIcon, aCallback) {
+  processIcon: function(aMimeType, aIcon) {
-    try {
+    let deferred = Promise.defer();
-      let process = Cc["@mozilla.org/process/util;1"]
-                    .createInstance(Ci.nsIProcess);
-      let sipsFile = Cc["@mozilla.org/file/local;1"]
-                    .createInstance(Ci.nsILocalFile);
-      sipsFile.initWithPath("/usr/bin/sips");
 
-      process.init(sipsFile);
+    function conversionDone(aSubject, aTopic) {
-      process.run(true, ["-s",
+      if (aTopic == "process-finished") {
-                  "format", "icns",
+        deferred.resolve();
-                  aIcon.path,
+      } else {
-                  "--out", this.iconFile.path,
+        deferred.reject("Failure converting icon.");
-                  "-z", "128", "128"],
+      }
-                  9);
-    } catch(e) {
-      throw(e);
-    } finally {
-      aCallback.call(this);
     }
+
+    let process = Cc["@mozilla.org/process/util;1"].
+                  createInstance(Ci.nsIProcess);
+    let sipsFile = Cc["@mozilla.org/file/local;1"].
+                   createInstance(Ci.nsILocalFile);
+    sipsFile.initWithPath("/usr/bin/sips");
+
+    process.init(sipsFile);
+    process.runAsync(["-s",
+                "format", "icns",
+                aIcon.path,
+                "--out", this.iconFile.path,
+                "-z", "128", "128"],
+                9, conversionDone);
+
+    return deferred.promise;
   }
 
 }
@@ -822,15 +832,16 @@ LinuxNativeApp.prototype = {
   },
 
   install: function() {
-    try {
+    return Task.spawn(function() {
-      this._copyPrebuiltFiles();
+      try {
-      this._createConfigFiles();
+        this._copyPrebuiltFiles();
-    } catch (ex) {
+        this._createConfigFiles();
-      this._removeInstallation(false);
+        yield this.getIcon();
-      throw(ex);
+      } catch (ex) {
-    }
+        this._removeInstallation(false);
-
+        throw(ex);
-    getIconForApp(this, function() {});
+      }
+    }.bind(this));
   },
 
   _removeInstallation: function(keepProfile) {
@@ -949,38 +960,32 @@ LinuxNativeApp.prototype = {
     writer.writeFile();
   },
 
-  /**
-   * This variable specifies if the icon retrieval process should
-   * use a temporary file in the system or a binary stream. This
-   * is accessed by a common function in WebappsIconHelpers.js and
-   * is different for each platform.
-   */
-  useTmpForIcon: false,
-
   /**
    * Process the icon from the imageStream as retrieved from
    * the URL by getIconForApp().
    *
    * @param aMimeType     ahe icon mimetype
    * @param aImageStream  the stream for the image data
-   * @param aCallback     a callback function to be called
-   *                      after the process finishes
    */
-  processIcon: function(aMimeType, aImageStream, aCallback) {
+  processIcon: function(aMimeType, aImageStream) {
-    let iconStream;
+    let deferred = Promise.defer();
-    try {
-      let imgTools = Cc["@mozilla.org/image/tools;1"]
-                       .createInstance(Ci.imgITools);
-      let imgContainer = { value: null };
 
-      imgTools.decodeImageData(aImageStream, aMimeType, imgContainer);
+    let imgTools = Cc["@mozilla.org/image/tools;1"]
-      iconStream = imgTools.encodeImage(imgContainer.value, "image/png");
+                     .createInstance(Ci.imgITools);
-    } catch (e) {
+
-      throw("processIcon - Failure converting icon (" + e + ")");
+    let imgContainer = imgTools.decodeImage(aImageStream, aMimeType);
-    }
+    let iconStream = imgTools.encodeImage(imgContainer, "image/png");
 
     let outputStream = FileUtils.openSafeFileOutputStream(this.iconFile);
-    NetUtil.asyncCopy(iconStream, outputStream);
+    NetUtil.asyncCopy(iconStream, outputStream, function(aResult) {
+      if (Components.isSuccessCode(aResult)) {
+        deferred.resolve();
+      } else {
+        deferred.reject("Failure copying icon: " + aResult);
+      }
+    });
+
+    return deferred.promise;
   }
 }