From 73f6968811b8b660e14e46924b9b40704bd9ddab Mon Sep 17 00:00:00 2001 From: Julien Wajsberg Date: Tue, 27 Nov 2012 12:59:39 +0100 Subject: [PATCH] Bug 815203 - [system] handle the app install restart when the homescreen starts it r=etienne r=arcturus We used to display the notification as soon as we got the applicationinstall event. Now we display the notification in the notification center only when the first progress event is fired on the app object. When an app install is restarted, we'll get only progress events, then it fits correctly in our new way of displaying the notification. We take also great care to remove the handlers on the app object only if they're our own handlers. We also have to add the handlers on pending apps during init to listen for app install restarts. --- apps/system/js/app_install_manager.js | 148 ++++-- .../test/unit/app_install_manager_test.js | 456 +++++++++++++----- 2 files changed, 430 insertions(+), 174 deletions(-) diff --git a/apps/system/js/app_install_manager.js b/apps/system/js/app_install_manager.js index 06f4cb966411..364c3d9aa583 100644 --- a/apps/system/js/app_install_manager.js +++ b/apps/system/js/app_install_manager.js @@ -20,7 +20,7 @@ var AppInstallManager = { this.notifContainer = document.getElementById('install-manager-notification-container'); - this.installNotifications = {}; + this.appInfos = {}; window.addEventListener('mozChromeEvent', (function ai_handleChromeEvent(e) { @@ -43,6 +43,34 @@ var AppInstallManager = { this.handleConfirmDownloadCancel.bind(this); this.downloadCancelDialog.querySelector('.cancel').onclick = this.handleCancelDownloadCancel.bind(this); + + // bind these handlers so that we can have only one instance and check + // them later on + ['handleDownloadSuccess', + 'handleDownloadError', + 'handleProgress', + 'handleApplicationReady' + ].forEach(function(name) { + this[name] = this[name].bind(this); + }, this); + + window.addEventListener('applicationready', + this.handleApplicationReady); + }, + + handleApplicationReady: function ai_handleApplicationReady(e) { + window.removeEventListener('applicationready', + this.handleApplicationReady); + + var apps = e.detail.applications; + + Object.keys(apps) + .map(function(key) { return apps[key]; }) + .forEach(this.prepareForDownload, this); + }, + + handleApplicationInstall: function ai_handleApplicationInstallEvent(e) { + this.prepareForDownload(e.detail.application); }, handleAppInstallPrompt: function ai_handleInstallPrompt(detail) { @@ -100,25 +128,26 @@ var AppInstallManager = { this.dialog.classList.remove('visible'); }, - handleApplicationInstall: function ai_handleApplicationInstall(e) { - var app = e.detail.application, - manifestURL = app.manifestURL; + prepareForDownload: function ai_prepareForDownload(app) { + var manifestURL = app.manifestURL; if (app.installState === 'installed') { // nothing more to do here, everything is already done return; } - StatusBar.incSystemDownloads(); - this.addNotification(app); - app.ondownloadsuccess = this.handleDownloadSuccess.bind(this); - app.ondownloaderror = this.handleDownloadError.bind(this); - app.onprogress = this.handleProgress.bind(this); + this.appInfos[manifestURL] = {}; + + // these methods are already bound to |this| + app.ondownloadsuccess = this.handleDownloadSuccess; + app.ondownloaderror = this.handleDownloadError; + app.onprogress = this.handleProgress; }, handleDownloadSuccess: function ai_handleDownloadSuccess(evt) { var app = evt.application; - this.finishDownload(app); + this.onDownloadStop(app); + this.onDownloadFinish(app); }, handleDownloadError: function ai_handleDownloadError(evt) { @@ -126,7 +155,6 @@ var AppInstallManager = { var _ = navigator.mozL10n.get; var manifest = app.manifest || app.updateManifest; var name = manifest.name; - StatusBar.decSystemDownloads(); var error = app.downloadError; @@ -143,23 +171,52 @@ var AppInstallManager = { SystemBanner.show(msg); } - this.finishDownload(app); + this.onDownloadStop(app); + }, + + onDownloadStart: function ai_onDownloadStart(app) { + if (! this.hasNotification(app)) { + StatusBar.incSystemDownloads(); + this.addNotification(app); + } + }, + + onDownloadStop: function ai_onDownloadStop(app) { + if (this.hasNotification(app)) { + StatusBar.decSystemDownloads(); + this.removeNotification(app); + } }, - finishDownload: function ai_cleanUp(app) { - StatusBar.decSystemDownloads(); - this.removeNotification(app); - app.ondownloadsuccess = null; - app.ondownloaderror = null; - app.onprogress = null; + hasNotification: function ai_hasNotification(app) { + var appInfo = this.appInfos[app.manifestURL]; + return appInfo && !!appInfo.installNotification; + }, + + onDownloadFinish: function ai_onDownloadFinish(app) { + delete this.appInfos[app.manifestURL]; + + // check if these are our handlers before removing them + if (app.ondownloadsuccess === this.handleDownloadSuccess) { + app.ondownloadsuccess = null; + } + + if (app.ondownloaderror === this.handleDownloadError) { + app.ondownloaderror = null; + } + + if (app.onprogress === this.handleProgress) { + app.onprogress = null; + } }, addNotification: function ai_addNotification(app) { // should be unique (this is used already in applications.js) var manifestURL = app.manifestURL, - manifest = app.manifest || app.updateManifest; + manifest = app.manifest || app.updateManifest, + appInfo = this.appInfos[manifestURL]; - if (this.installNotifications[manifestURL]) { + if (appInfo.installNotification) { return; } @@ -182,36 +239,33 @@ var AppInstallManager = { newNode.querySelector('.message').textContent = message; - var progressNode = newNode.querySelector('progress'); - - var size = manifest.size; + var size = manifest.size, + progressNode = newNode.querySelector('progress'); if (size) { progressNode.max = size; - progressNode.value = 0; - message = _('downloadingAppProgress', - { - progress: this.humanizeSize(0), - max: this.humanizeSize(size) - }); - } else { - message = navigator.mozL10n.get( - 'downloadingAppProgressNoMax', { progress: 0 }); + appInfo.hasMax = true; } - progressNode.textContent = message; - this.installNotifications[manifestURL] = newNode; + appInfo.installNotification = newNode; NotificationScreen.incExternalNotifications(); }, + getNotificationProgressNode: function ai_getNotificationProgressNode(app) { + var appInfo = this.appInfos[app.manifestURL]; + var progressNode = appInfo + && appInfo.installNotification + && appInfo.installNotification.querySelector('progress'); + return progressNode || null; + }, + handleProgress: function ai_handleProgress(evt) { - var app = evt.application; - var notifNode = this.installNotifications[app.manifestURL]; + var app = evt.application, + appInfo = this.appInfos[app.manifestURL]; + + this.onDownloadStart(app); - if (!notifNode) { - return; - } - var progressNode = notifNode.querySelector('progress'); + var progressNode = this.getNotificationProgressNode(app); var message; var _ = navigator.mozL10n.get; @@ -220,31 +274,31 @@ var AppInstallManager = { // handle the null and undefined cases as well message = _('downloadingAppProgressIndeterminate'); progressNode.value = undefined; // switch to indeterminate state - } else if (progressNode.position === -1) { - // already in indeterminate state, means we have no max - message = _('downloadingAppProgressNoMax', - { progress: app.progress }); - } else { + } else if (appInfo.hasMax) { message = _('downloadingAppProgress', { progress: this.humanizeSize(app.progress), max: this.humanizeSize(progressNode.max) }); progressNode.value = app.progress; + } else { + message = _('downloadingAppProgressNoMax', + { progress: this.humanizeSize(app.progress) }); } progressNode.textContent = message; }, removeNotification: function ai_removeNotification(app) { var manifestURL = app.manifestURL, - node = this.installNotifications[manifestURL]; + appInfo = this.appInfos[manifestURL], + node = appInfo.installNotification; if (!node) { return; } node.parentNode.removeChild(node); - delete this.installNotifications[manifestURL]; + delete appInfo.installNotification; NotificationScreen.decExternalNotifications(); }, diff --git a/apps/system/test/unit/app_install_manager_test.js b/apps/system/test/unit/app_install_manager_test.js index 0fc0c70ea3f3..146d3629508b 100644 --- a/apps/system/test/unit/app_install_manager_test.js +++ b/apps/system/test/unit/app_install_manager_test.js @@ -378,26 +378,21 @@ suite('system/AppInstallManager >', function() { }); suite('duringInstall >', function() { - var mockApp, e; - - setup(function() { - e = new CustomEvent('applicationinstall', { detail: {} }); - }); - - teardown(function() { - MockSystemBanner.mTeardown(); - }); + var mockApp, mockAppName; function dispatchEvent() { - e.detail.application = mockApp; - window.dispatchEvent(e); + var e = new CustomEvent('applicationinstall', { + detail: { application: mockApp } + }); + window.dispatchEvent(e); } suite('hosted app without cache >', function() { setup(function() { + mockAppName = 'Fake hosted app'; mockApp = new MockApp({ manifest: { - name: 'Fake hosted app', + name: mockAppName, developer: { name: 'Fake dev', url: 'http://fakesoftware.com' @@ -419,11 +414,65 @@ suite('system/AppInstallManager >', function() { }); + function beforeFirstProgressSuite() { + suite('before first progress >', function() { + test('should not show the icon', function() { + var method = 'incSystemDownloads'; + assert.isUndefined(MockStatusBar.wasMethodCalled[method]); + }); + + test('should not add a notification', function() { + assert.equal(fakeNotif.childElementCount, 0); + }); + + suite('on downloadsuccess >', function() { + setup(function() { + // reseting these mocks as we want to test only one call + MockNotificationScreen.mTeardown(); + MockStatusBar.mTeardown(); + + mockApp.mTriggerDownloadSuccess(); + }); + + test('should not remove a notification', function() { + var method = 'decExternalNotifications'; + assert.isUndefined(MockNotificationScreen.wasMethodCalled[method]); + }); + + test('should not remove the download icon', function() { + var method = 'decSystemDownloads'; + assert.isUndefined(MockStatusBar.wasMethodCalled[method]); + }); + }); + + suite('on downloaderror >', function() { + setup(function() { + // reseting these mocks as we want to test only one call + MockNotificationScreen.mTeardown(); + MockStatusBar.mTeardown(); + + mockApp.mTriggerDownloadError(); + }); + + test('should not remove a notification', function() { + var method = 'decExternalNotifications'; + assert.isUndefined(MockNotificationScreen.wasMethodCalled[method]); + }); + + test('should not remove the download icon', function() { + var method = 'decSystemDownloads'; + assert.isUndefined(MockStatusBar.wasMethodCalled[method]); + }); + }); + }); + } + suite('hosted app with cache >', function() { setup(function() { + mockAppName = 'Fake hosted app with cache'; mockApp = new MockApp({ manifest: { - name: 'Fake hosted app with cache', + name: mockAppName, developer: { name: 'Fake dev', url: 'http://fakesoftware.com' @@ -436,66 +485,134 @@ suite('system/AppInstallManager >', function() { dispatchEvent(); }); - test('should show the icon', function() { - assert.ok(MockStatusBar.wasMethodCalled['incSystemDownloads']); - }); + function downloadEventsSuite(afterError) { + var suiteName = 'on first progress'; + if (afterError) { + suiteName += ' after error'; + } + suiteName += ' >'; - test('should remove the icon if we get downloadsuccess', function() { - mockApp.mTriggerDownloadSuccess(); - assert.ok(MockStatusBar.wasMethodCalled['decSystemDownloads']); - }); + suite(suiteName, function() { + setup(function() { + // reseting these mocks as we want to test only the following + // calls + MockNotificationScreen.mTeardown(); + MockStatusBar.mTeardown(); - test('should remove the icon and display error if we get downloaderror', - function() { - mockApp.mTriggerDownloadError(); - assert.ok(MockStatusBar.wasMethodCalled['decSystemDownloads']); - assert.equal(MockSystemBanner.mMessage, - 'download-stopped2{"appName":"Fake hosted app with cache"}'); - assert.isFalse(MockModalDialog.alert.mWasCalled); - }); + mockApp.mTriggerDownloadProgress(NaN); + }); - test('should add a notification', function() { - assert.equal(fakeNotif.childElementCount, 1); - }); + test('should add a notification', function() { + var method = 'incExternalNotifications'; + assert.equal(fakeNotif.childElementCount, 1); + assert.ok(MockNotificationScreen.wasMethodCalled[method]); + }); - test('notification should have a message', function() { - assert.equal(fakeNotif.querySelector('.message').textContent, - 'downloadingAppMessage{"appName":"Fake hosted app with cache"}'); - assert.equal(fakeNotif.querySelector('progress').textContent, - 'downloadingAppProgressNoMax{"progress":0}'); - }); + test('notification should have a message', function() { + assert.equal(fakeNotif.querySelector('.message').textContent, + 'downloadingAppMessage{"appName":"Fake hosted app with cache"}'); + assert.equal(fakeNotif.querySelector('progress').textContent, + 'downloadingAppProgressIndeterminate'); + }); - test('notification progress should be indeterminate', function() { - assert.equal(fakeNotif.querySelector('progress').position, -1); - }); + test('notification progress should be indeterminate', function() { + assert.equal(fakeNotif.querySelector('progress').position, -1); + }); - test('should remove the notif if we get downloadsuccess', function() { - mockApp.mTriggerDownloadSuccess(); - assert.equal(fakeNotif.childElementCount, 0); - }); + test('downloadsuccess > should remove the notif', function() { + var method = 'decExternalNotifications'; + mockApp.mTriggerDownloadSuccess(); + assert.equal(fakeNotif.childElementCount, 0); + assert.ok(MockNotificationScreen.wasMethodCalled[method]); + }); - test('should remove the notif if we get downloaderror', function() { - mockApp.mTriggerDownloadError(); - assert.equal(fakeNotif.childElementCount, 0); - }); + test('on downloadsuccess > should remove only its progress handler', + function() { - test('should keep the progress indeterminate on progress', function() { - mockApp.mTriggerDownloadProgress(NaN); + var onprogressCalled = false; + mockApp.onprogress = function() { + onprogressCalled = true; + }; + mockApp.mTriggerDownloadSuccess(); + mockApp.mTriggerDownloadProgress(10); + assert.isTrue(onprogressCalled); + }); - var progressNode = fakeNotif.querySelector('progress'); - assert.equal(progressNode.position, -1); - assert.equal(progressNode.textContent, - 'downloadingAppProgressIndeterminate'); - }); + suite('on indeterminate progress >', function() { + setup(function() { + mockApp.mTriggerDownloadProgress(NaN); + }); + + test('should keep the progress indeterminate', function() { + var progressNode = fakeNotif.querySelector('progress'); + assert.equal(progressNode.position, -1); + assert.equal(progressNode.textContent, + 'downloadingAppProgressIndeterminate'); + }); + }); + + suite('on quantified progress >', function() { + setup(function() { + mockApp.mTriggerDownloadProgress(10); + }); + + test('should have a quantified progress', function() { + var progressNode = fakeNotif.querySelector('progress'); + assert.equal(progressNode.position, -1); + assert.equal(progressNode.textContent, + 'downloadingAppProgressNoMax{"progress":"10.00 bytes"}'); + }); + }); + + 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); + }); + } + }); + } + + beforeFirstProgressSuite(); + downloadEventsSuite(/*afterError*/ false); }); suite('packaged app >', function() { setup(function() { + mockAppName = 'Fake packaged app'; mockApp = new MockApp({ manifest: null, updateManifest: { - name: 'Fake packaged app', + name: mockAppName, size: 5245678, developer: { name: 'Fake dev', @@ -508,96 +625,157 @@ suite('system/AppInstallManager >', function() { dispatchEvent(); }); - test('should show the icon', function() { - assert.ok(MockStatusBar.wasMethodCalled['incSystemDownloads']); - }); - test('should remove the icon if we get downloadsuccess', function() { - mockApp.mTriggerDownloadSuccess(); - assert.ok(MockStatusBar.wasMethodCalled['decSystemDownloads']); - }); + function downloadEventsSuite(afterError) { + var suiteName = 'on first progress'; + if (afterError) { + suiteName += ' after error'; + } + suiteName += ' >'; - test('should remove the icon and display error if we get downloaderror', - function() { - mockApp.mTriggerDownloadError(); - assert.ok(MockStatusBar.wasMethodCalled['decSystemDownloads']); - assert.equal(MockSystemBanner.mMessage, - 'download-stopped2{"appName":"Fake packaged app"}'); - assert.isFalse(MockModalDialog.alert.mWasCalled); - }); + suite(suiteName, function() { + var newprogress = 5; - test('should display an alert if we get INSUFFICIENT STORAGE error', - function() { - mockApp.mTriggerDownloadError('INSUFFICIENT_STORAGE'); - assert.ok(MockStatusBar.wasMethodCalled['decSystemDownloads']); - assert.isNull(MockSystemBanner.mMessage); - assert.isTrue(MockModalDialog.alert.mWasCalled); - var args = MockModalDialog.alert.mArgs; - assert.equal(args[0], 'not-enough-space'); - assert.equal(args[1], 'not-enough-space-message'); - assert.deepEqual(args[2], { title: 'ok' }); - }); + setup(function() { + // resetting this mock because we want to test only the + // following call + MockNotificationScreen.mTeardown(); - test('should add a notification', function() { - var method = 'incExternalNotifications'; - assert.equal(fakeNotif.childElementCount, 1); - assert.ok(MockNotificationScreen.wasMethodCalled[method]); - }); + mockApp.mTriggerDownloadProgress(newprogress); + }); - test('notification should have a message', function() { - assert.equal(fakeNotif.querySelector('.message').textContent, - 'downloadingAppMessage{"appName":"Fake packaged app"}'); - }); + test('should add a notification', function() { + var method = 'incExternalNotifications'; + assert.equal(fakeNotif.childElementCount, 1); + assert.ok(MockNotificationScreen.wasMethodCalled[method]); + }); - test('notification progress should have a max and a value', function() { - assert.equal(fakeNotif.querySelector('progress').max, - mockApp.updateManifest.size); - assert.equal(fakeNotif.querySelector('progress').value, 0); - }); + test('notification should have a message', function() { + var expectedText = 'downloadingAppMessage{"appName":"' + + mockAppName + '"}'; + assert.equal(fakeNotif.querySelector('.message').textContent, + expectedText); + }); - test('notification progress should not be indeterminate', function() { - assert.notEqual(fakeNotif.querySelector('progress').position, -1); - }); + test('notification progress should have a max and a value', + function() { + assert.equal(fakeNotif.querySelector('progress').max, + mockApp.updateManifest.size); + assert.equal(fakeNotif.querySelector('progress').value, + newprogress); + }); - test('should remove the notif if we get downloadsuccess', function() { - var method = 'decExternalNotifications'; - mockApp.mTriggerDownloadSuccess(); - assert.equal(fakeNotif.childElementCount, 0); - assert.ok(MockNotificationScreen.wasMethodCalled[method]); - }); + test('notification progress should not be indeterminate', + function() { + assert.notEqual(fakeNotif.querySelector('progress').position, -1); + }); - test('should remove the notif if we get downloaderror', function() { - mockApp.mTriggerDownloadError(); - assert.equal(fakeNotif.childElementCount, 0); - }); + test('on downloadsuccess > should remove the notif', function() { + var method = 'decExternalNotifications'; + mockApp.mTriggerDownloadSuccess(); + assert.equal(fakeNotif.childElementCount, 0); + assert.ok(MockNotificationScreen.wasMethodCalled[method]); + }); - test('should update the progress notification on progress', function() { - var newprogress = 10, - size = mockApp.updateManifest.size, - ratio = newprogress / size; - mockApp.mTriggerDownloadProgress(newprogress); + test('on indeterminate progress > ' + + 'should update the progress text content', + function() { + mockApp.mTriggerDownloadProgress(NaN); + + var progressNode = fakeNotif.querySelector('progress'); + assert.equal(progressNode.textContent, + 'downloadingAppProgressIndeterminate'); + }); + + suite('on progress >', function() { + var size, ratio; + var newprogress = 10; + + setup(function() { + size = mockApp.updateManifest.size; + ratio = newprogress / size; + mockApp.mTriggerDownloadProgress(newprogress); + }); + + test('should update the progress notification', function() { + var progressNode = fakeNotif.querySelector('progress'); + assert.equal(progressNode.position, ratio); + assert.equal(progressNode.textContent, + 'downloadingAppProgress{"progress":"10.00 bytes",' + + '"max":"5.00 MB"}'); + }); + }); - var progressNode = fakeNotif.querySelector('progress'); - assert.equal(progressNode.position, ratio); - assert.equal(progressNode.textContent, - 'downloadingAppProgress{"progress":"10.00 bytes","max":"5.00 MB"}'); - }); + 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); + }); + } + }); + } - test('should update the progress text content if we do not have the ' + - 'actual progress', function() { - mockApp.mTriggerDownloadProgress(NaN); + beforeFirstProgressSuite(); + downloadEventsSuite(/*afterError*/ false); + + suite('on INSUFFICIENT_STORAGE downloaderror >', function() { + test('should display an alert', function() { + mockApp.mTriggerDownloadError('INSUFFICIENT_STORAGE'); + assert.isNull(MockSystemBanner.mMessage); + assert.isTrue(MockModalDialog.alert.mWasCalled); + var args = MockModalDialog.alert.mArgs; + assert.equal(args[0], 'not-enough-space'); + assert.equal(args[1], 'not-enough-space-message'); + assert.deepEqual(args[2], { title: 'ok' }); + }); - var progressNode = fakeNotif.querySelector('progress'); - assert.equal(progressNode.textContent, - 'downloadingAppProgressIndeterminate'); + beforeFirstProgressSuite(); + downloadEventsSuite(/*afterError*/ true); }); + + }); suite('cancelling a download >', function() { setup(function() { - mockApp = new MockApp(); + mockApp = new MockApp({ installState: 'pending' }); MockApplications.mRegisterMockApp(mockApp); - AppInstallManager.addNotification(mockApp); + dispatchEvent(); + mockApp.mTriggerDownloadProgress(10); }); test('tapping the notification should display the dialog', function() { @@ -654,6 +832,30 @@ suite('system/AppInstallManager >', function() { }); }); + + suite('restarting after reboot >', function() { + setup(function() { + var mockApp = new MockApp({ + updateManifest: null, + installState: 'pending' + }); + + var e = new CustomEvent('applicationready', { + detail: { applications: {} } + }); + e.detail.applications[mockApp.manifestURL] = mockApp; + window.dispatchEvent(e); + + mockApp.mTriggerDownloadProgress(50); + }); + + test('should add a notification', function() { + var method = 'incExternalNotifications'; + assert.equal(fakeNotif.childElementCount, 1); + assert.ok(MockNotificationScreen.wasMethodCalled[method]); + }); + }); + suite('humanizeSize >', function() { test('should handle bytes size', function() { assert.equal('42.00 bytes', AppInstallManager.humanizeSize(42));