Bug 816448 - [App Install] Handle errors during app download & instal…

…l r=etienne

Add different error messages for different errors. The messages are still
generic but all the keys are present in locale so this may change if necessary
without a change in the code.

Also, the actual error name is displayed in logcat at the INFO level, so that a
developer could know what really happens.

apps/system/js/app_install_manager.js

@@ -1,6 +1,16 @@
 'use strict';
 
 var AppInstallManager = {
+  mapDownloadErrorsToMessage: {
+    'NETWORK_ERROR': 'download-failed',
+    'DOWNLOAD_ERROR': 'download-failed',
+    'MISSING_MANIFEST': 'install-failed',
+    'INVALID_MANIFEST': 'install-failed',
+    'INSTALL_FROM_DENIED': 'install-failed',
+    'INVALID_SECURITY_LEVEL': 'install-failed',
+    'INVALID_PACKAGE': 'install-failed',
+    'APP_CACHE_DOWNLOAD_ERROR': 'download-failed'
+  },
 
   init: function ai_init() {
     this.dialog = document.getElementById('app-install-dialog');
@@ -156,9 +166,9 @@ var AppInstallManager = {
     var manifest = app.manifest || app.updateManifest;
     var name = manifest.name;
 
-    var error = app.downloadError;
+    var errorName = app.downloadError.name;
 
-    switch (error.name) {
+    switch (errorName) {
       case 'INSUFFICIENT_STORAGE':
         var title = _('not-enough-space'),
             buttonText = _('ok'),
@@ -167,7 +177,11 @@ var AppInstallManager = {
         ModalDialog.alert(title, message, {title: buttonText});
         break;
       default:
-        var msg = _('download-stopped2', { appName: name });
+        // showing the real error to a potential developer
+        console.info('downloadError event, error code is', errorName);
+
+        var key = this.mapDownloadErrorsToMessage[errorName] || 'generic-error';
+        var msg = _('app-install-' + key, { appName: name });
         SystemBanner.show(msg);
     }
 
@@ -254,9 +268,9 @@ var AppInstallManager = {
 
   getNotificationProgressNode: function ai_getNotificationProgressNode(app) {
     var appInfo = this.appInfos[app.manifestURL];
-    var progressNode = appInfo
-      && appInfo.installNotification
-      && appInfo.installNotification.querySelector('progress');
+    var progressNode = appInfo &&
+      appInfo.installNotification &&
+      appInfo.installNotification.querySelector('progress');
     return progressNode || null;
   },
 

apps/system/js/updatable.js

@@ -84,7 +84,9 @@ AppUpdatable.prototype.appliedCallBack = function() {
   this.cleanCallbacks();
 };
 
-AppUpdatable.prototype.errorCallBack = function() {
+AppUpdatable.prototype.errorCallBack = function(e) {
+  var errorName = e.application.downloadError.name;
+  console.info('downloadError event, error code is', errorName);
   UpdateManager.requestErrorBanner();
   UpdateManager.removeFromDownloadsQueue(this);
   this.cleanCallbacks();

apps/system/locales/system.en-US.properties

@@ -66,7 +66,9 @@ MB=MB
 GB=GB
 TB=TB
 PB=PB
-download-stopped2={{ appName }} download stopped
+app-install-generic-error={{ appName }} download stopped
+app-install-download-failed={{ appName }} download failed
+app-install-install-failed={{ appName }} install failed
 cancelling-will-not-refund=Cancelling will not refund a purchase. Refunds for paid content are provided by the original seller.
 apps-can-be-installed-later=Apps can be installed later from the original installation source.
 are-you-sure-you-want-to-cancel=Are you sure you want to cancel this install?

apps/system/test/unit/app_install_manager_test.js

@@ -1,3 +1,5 @@
+'use strict';
+
 requireApp('system/test/unit/mock_app.js');
 requireApp('system/test/unit/mock_chrome_event.js');
 requireApp('system/test/unit/mock_statusbar.js');
@@ -476,6 +478,78 @@ suite('system/AppInstallManager >', function() {
       });
     }
 
+    function downloadErrorSuite(downloadEventsSuite) {
+      suite('on downloadError >', function() {
+        setup(function() {
+          // reseting these mocks as we only want to test the
+          // following call
+          MockStatusBar.mTeardown();
+          MockSystemBanner.mTeardown();
+          MockModalDialog.mTeardown();
+        });
+
+        function downloadErrorTests(errorName) {
+          test('should remove the icon', function() {
+              var method = 'decSystemDownloads';
+              assert.ok(MockStatusBar.wasMethodCalled[method]);
+            });
+
+          test('should display an error', function() {
+            var expectedErrorMsg = knownErrors[errorName] +
+                                   '{"appName":"' + mockAppName + '"}';
+
+            assert.equal(MockSystemBanner.mMessage, expectedErrorMsg);
+          });
+
+          test('should not display the error dialog', function() {
+            assert.isFalse(MockModalDialog.alert.mWasCalled);
+          });
+
+          test('should remove the notif', function() {
+            assert.equal(fakeNotif.childElementCount, 0);
+          });
+
+        }
+
+        function specificDownloadErrorSuite(errorName) {
+          suite(errorName + ' >', function() {
+            setup(function() {
+              mockApp.mTriggerDownloadError(errorName);
+            });
+
+            downloadErrorTests(errorName);
+          });
+        }
+
+        var knownErrors = {
+          'FALLBACK_ERROR': 'app-install-generic-error',
+          'NETWORK_ERROR': 'app-install-download-failed',
+          'DOWNLOAD_ERROR': 'app-install-download-failed',
+          'MISSING_MANIFEST': 'app-install-install-failed',
+          'INVALID_MANIFEST': 'app-install-install-failed',
+          'INSTALL_FROM_DENIED': 'app-install-install-failed',
+          'INVALID_SECURITY_LEVEL': 'app-install-install-failed',
+          'INVALID_PACKAGE': 'app-install-install-failed',
+          'APP_CACHE_DOWNLOAD_ERROR': 'app-install-download-failed'
+        };
+
+        Object.keys(knownErrors).forEach(specificDownloadErrorSuite);
+
+        suite('GENERIC_ERROR >', function() {
+          setup(function() {
+            mockApp.mTriggerDownloadError('GENERIC_ERROR');
+          });
+
+          /* this makes the test execution too slow because of
+           * https://bugzilla.mozilla.org/show_bug.cgi?id=820883
+           * When this bug is fixed we might uncomment these lines. */
+          beforeFirstProgressSuite();
+          downloadEventsSuite(/*afterError*/ true);
+        });
+
+      });
+    }
+
     suite('hosted app with cache >', function() {
       setup(function() {
         mockAppName = 'Fake hosted app with cache';
@@ -589,39 +663,7 @@ suite('system/AppInstallManager >', function() {
           });
 
           if (!afterError) {
-
-            suite('on downloadError >', function() {
-              setup(function() {
-                // reseting these mocks as we only want to test the
-                // following call
-                MockStatusBar.mTeardown();
-                MockSystemBanner.mTeardown();
-                MockModalDialog.mTeardown();
-
-                mockApp.mTriggerDownloadError();
-              });
-
-              test('should remove the icon', function() {
-                  var method = 'decSystemDownloads';
-                  assert.ok(MockStatusBar.wasMethodCalled[method]);
-                });
-
-              test('should display an error', function() {
-                assert.equal(MockSystemBanner.mMessage,
-                  'download-stopped2{"appName":"' + mockAppName + '"}');
-              });
-
-              test('should not display the error dialog', function() {
-                assert.isFalse(MockModalDialog.alert.mWasCalled);
-              });
-
-              test('should remove the notif', function() {
-                assert.equal(fakeNotif.childElementCount, 0);
-              });
-
-              beforeFirstProgressSuite();
-              downloadEventsSuite(/*afterError*/ true);
-            });
+            downloadErrorSuite(downloadEventsSuite);
           }
         });
       }
@@ -755,44 +797,7 @@ suite('system/AppInstallManager >', function() {
           });
 
           if (!afterError) {
-            suite('on downloadError >', function() {
-              setup(function() {
-                // resetting these tests because we want to test the
-                // following call only
-                MockStatusBar.mTeardown();
-                MockSystemBanner.mTeardown();
-                MockModalDialog.mTeardown();
-
-                mockApp.mTriggerDownloadError();
-              });
-
-              test('should remove the icon and display error',
-                function() {
-                  var method = 'decSystemDownloads';
-                  assert.ok(MockStatusBar.wasMethodCalled[method]);
-                });
-
-              test('should remove the icon', function() {
-                  var method = 'decSystemDownloads';
-                  assert.ok(MockStatusBar.wasMethodCalled[method]);
-                });
-
-              test('should display an error', function() {
-                assert.equal(MockSystemBanner.mMessage,
-                  'download-stopped2{"appName":"' + mockAppName + '"}');
-              });
-
-              test('should not display the error dialog', function() {
-                assert.isFalse(MockModalDialog.alert.mWasCalled);
-              });
-
-              test('should remove the notif', function() {
-                assert.equal(fakeNotif.childElementCount, 0);
-              });
-
-              beforeFirstProgressSuite();
-              downloadEventsSuite(/*afterError*/ true);
-            });
+            downloadErrorSuite(downloadEventsSuite);
           }
         });
       }

apps/system/test/unit/mock_app.js

@@ -59,7 +59,7 @@ MockApp.prototype.mTriggerDownloadError = function(error) {
   this.downloadAvailable = true;
 
   this.downloadError = {
-    name: error || 'NETWORK_ERROR'
+    name: error || 'UNKNOWN_ERROR'
   };
 
   if (this.ondownloaderror) {