Bug 834515 - Updating to a packaged app with a new name via introducing a locale override allows changing the app name r=fabrice

dom/apps/src/AppsUtils.jsm

@@ -235,9 +235,6 @@ this.AppsUtils = {
       }
     }
 
-    // Ensure that non-updatable fields contains the current app value
-    AppsUtils.normalizeManifest(aManifest, app);
-
     return true;
   },
 
@@ -257,33 +254,24 @@ this.AppsUtils = {
    * Method to apply modifications to webapp manifests file saved internally.
    * For now, only ensure app can't rename itself.
    */
-  normalizeManifest: function normalizeManifest(aManifest, aApp) {
-    // As normalizeManifest isn't only called on update but also
-    // during app install, we need to bail out on install.
-    if (aApp.installState != "installed" &&
-        aApp.installState != "updating") {
-      return;
-    }
-
-    let previousManifest = aApp.manifest;
-
+  ensureSameAppName: function ensureSameAppName(aOldManifest, aNewManifest, aApp) {
     // Ensure that app name can't be updated
-    aManifest.name = aApp.name;
+    aNewManifest.name = aApp.name;
 
     // Nor through localized names
-    if ('locales' in aManifest) {
-      let defaultName = new ManifestHelper(aManifest, aApp.origin).name;
-      for (let locale in aManifest.locales) {
-        let entry = aManifest.locales[locale];
+    if ('locales' in aNewManifest) {
+      let defaultName = new ManifestHelper(aOldManifest, aApp.origin).name;
+      for (let locale in aNewManifest.locales) {
+        let entry = aNewManifest.locales[locale];
         if (!entry.name) {
           continue;
         }
         // In case previous manifest didn't had a name,
         // we use the default app name
         let localizedName = defaultName;
-        if (previousManifest && 'locales' in previousManifest &&
-            locale in previousManifest.locales) {
-          localizedName = previousManifest.locales[locale].name;
+        if (aOldManifest && 'locales' in aOldManifest &&
+            locale in aOldManifest.locales) {
+          localizedName = aOldManifest.locales[locale].name;
         }
         entry.name = localizedName;
       }

dom/apps/src/Webapps.jsm

@@ -1482,20 +1482,8 @@ this.DOMApplicationRegistry = {
       return;
     }
 
-    // Try to download a new manifest.
-    let xhr = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"]
-                .createInstance(Ci.nsIXMLHttpRequest);
-    xhr.open("GET", aData.manifestURL, true);
-    xhr.channel.loadFlags |= Ci.nsIRequest.INHIBIT_CACHING;
-    xhr.responseType = "json";
-    if (app.etag) {
-      debug("adding manifest etag:" + app.etag);
-      xhr.setRequestHeader("If-None-Match", app.etag);
-    }
-    xhr.channel.notificationCallbacks =
-      this.createLoadContext(app.installerAppId, app.installerIsBrowser);
-
-    xhr.addEventListener("load", (function() {
+    // On xhr load request event
+    function onload(xhr, oldManifest) {
       debug("Got http status=" + xhr.status + " for " + aData.manifestURL);
       let oldHash = app.manifestHash;
       let isPackage = app.origin.startsWith("app://");
@@ -1514,6 +1502,8 @@ this.DOMApplicationRegistry = {
           sendError("INSTALL_FROM_DENIED");
           return;
         } else {
+          AppsUtils.ensureSameAppName(oldManifest, manifest, app);
+
           let hash = this.computeManifestHash(manifest);
           debug("Manifest hash = " + hash);
           if (isPackage) {
@@ -1543,12 +1533,10 @@ this.DOMApplicationRegistry = {
               this._saveApps();
             }
           } else {
-            this._readManifests([{ id: id }], (function(aResult) {
-              // Update only the appcache if the manifest has not changed
-              // based on the hash value.
-              updateHostedApp.call(this, aResult[0].manifest,
-                                   oldHash == hash ? null : manifest);
-            }).bind(this));
+            // Update only the appcache if the manifest has not changed
+            // based on the hash value.
+            updateHostedApp.call(this, oldManifest,
+                                 oldHash == hash ? null : manifest);
           }
         }
       } else if (xhr.status == 304) {
@@ -1567,21 +1555,40 @@ this.DOMApplicationRegistry = {
         } else {
           // For hosted apps, even if the manifest has not changed, we check
           // for offline cache updates.
-          this._readManifests([{ id: id }], (function(aResult) {
-            updateHostedApp.call(this, aResult[0].manifest, null);
-          }).bind(this));
+          updateHostedApp.call(this, oldManifest, null);
         }
       } else {
         sendError("MANIFEST_URL_ERROR");
       }
-    }).bind(this), false);
+    }
 
-    xhr.addEventListener("error", (function() {
-      sendError("NETWORK_ERROR");
-    }).bind(this), false);
+    // Try to download a new manifest.
+    function doRequest(oldManifest) {
+      let xhr = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"]
+                  .createInstance(Ci.nsIXMLHttpRequest);
+      xhr.open("GET", aData.manifestURL, true);
+      xhr.channel.loadFlags |= Ci.nsIRequest.INHIBIT_CACHING;
+      xhr.responseType = "json";
+      if (app.etag) {
+        debug("adding manifest etag:" + app.etag);
+        xhr.setRequestHeader("If-None-Match", app.etag);
+      }
+      xhr.channel.notificationCallbacks =
+        this.createLoadContext(app.installerAppId, app.installerIsBrowser);
 
-    debug("Checking manifest at " + aData.manifestURL);
-    xhr.send(null);
+      xhr.addEventListener("load", onload.bind(this, xhr, oldManifest), false);
+      xhr.addEventListener("error", (function() {
+        sendError("NETWORK_ERROR");
+      }).bind(this), false);
+
+      debug("Checking manifest at " + aData.manifestURL);
+      xhr.send(null);
+    }
+
+    // Read the current app manifest file
+    this._readManifests([{ id: id }], (function(aResult) {
+      doRequest.call(this, aResult[0].manifest);
+    }).bind(this));
   },
 
   // Creates a nsILoadContext object with a given appId and isBrowser flag.
@@ -2305,13 +2312,14 @@ this.DOMApplicationRegistry = {
                 let manifest = JSON.parse(converter.ConvertToUnicode(NetUtil.readInputStreamToString(istream,
                                                                      istream.available()) || ""));
 
-                // Call checkManifest before compareManifests, as checkManifest
-                // will normalize some attributes that has already been normalized
-                // for aManifest during checkForUpdate.
                 if (!AppsUtils.checkManifest(manifest, app)) {
                   throw "INVALID_MANIFEST";
                 }
 
+                // Call ensureSameAppName before compareManifests, as `manifest`,
+                // has been normalized to avoid app rename.
+                AppsUtils.ensureSameAppName(aManifest._manifest, manifest, app);
+
                 if (!AppsUtils.compareManifests(manifest,
                                                 aManifest._manifest)) {
                   throw "MANIFEST_MISMATCH";