Bug 1047645 - Use manual timeout to guard _closed event from not firi…

…ng r=alive
mozilla-b2g · Aug 15, 2014 · 5b7daa1 · 5b7daa1
1 parent a528344
commit 5b7daa1
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 4 deletions.
diff --git a/apps/system/js/app_transition_controller.js b/apps/system/js/app_transition_controller.js
@@ -306,7 +306,7 @@
 
   AppTransitionController.prototype.clearTransitionClasses =
     function atc_removeTransitionClasses() {
-      if (!this.app) {
+      if (!this.app || !this.app.element) {
         return;
       }
 

diff --git a/apps/system/js/app_window.js b/apps/system/js/app_window.js
@@ -420,10 +420,18 @@
 
     // If the app is the currently displayed app, switch to the homescreen
     if (this.isActive() && !this.isHomescreen) {
-      this.element.addEventListener('_closed', (function onClosed() {
-        window.removeEventListener('_closed', onClosed);
+
+      var fallbackTimeout;
+      var onClosed = function() {
+        clearTimeout(fallbackTimeout);
+        this.element.removeEventListener('_closed', onClosed);
         this.destroy();
-      }).bind(this));
+      }.bind(this);
+
+      this.element.addEventListener('_closed', onClosed);
+      fallbackTimeout = setTimeout(onClosed,
+        this.transitionController.CLOSING_TRANSITION_TIMEOUT);
+
       if (this.previousWindow) {
         this.previousWindow.getBottomMostWindow().open('in-from-left');
         this.close('out-to-right');

diff --git a/apps/system/test/unit/app_window_test.js b/apps/system/test/unit/app_window_test.js
@@ -1146,6 +1146,11 @@ suite('system/AppWindow', function() {
   });
 
   suite('Event handlers', function() {
+    var fakeTransitionController = {
+      requireOpen: function() {},
+      requireClose: function() {}
+    };
+
     test('ActivityDone event', function() {
       var app1 = new AppWindow(fakeAppConfig1);
       var app2 = new AppWindow(fakeAppConfig2);
@@ -1303,6 +1308,7 @@ suite('system/AppWindow', function() {
       var stubCloseSelf = this.sinon.stub(app1, 'close');
       stubIsActive.returns(true);
 
+      app1.transitionController = fakeTransitionController;
       app1.kill();
       assert.isTrue(stubOpenParent.calledWith('in-from-left'));
       assert.isTrue(stubCloseSelf.calledWith('out-to-right'));
@@ -1315,6 +1321,24 @@ suite('system/AppWindow', function() {
       assert.isTrue(stubDestroy.called);
     });
 
+    test('kill guards against missed transitions', function() {
+      var app = new AppWindow(fakeAppConfig1);
+
+      // Ensure that the closed event does not trigger the destroy method.
+      this.sinon.stub(app.element, 'addEventListener');
+
+      this.sinon.stub(app, 'isActive').returns(true);
+      var destroyStub = this.sinon.stub(app, 'destroy');
+
+      app.transitionController = fakeTransitionController;
+      app.kill();
+      assert.ok(destroyStub.notCalled);
+
+      var fallbackTimeout = 1000;
+      this.sinon.clock.tick(fallbackTimeout);
+      assert.ok(destroyStub.calledOnce);
+    });
+
     test('Load event', function() {
       var app1 = new AppWindow(fakeAppConfig1);
 

diff --git a/apps/system/test/unit/lockscreen_window_test.js b/apps/system/test/unit/lockscreen_window_test.js
@@ -77,6 +77,7 @@ suite('system/LockScreenWindow', function() {
       // Or the AppWindow would look for it.
       app.element = document.createElement('div');
       parentElement.appendChild(app.element);
+      app.transitionController = {};
       app.kill();
       assert.isTrue(stubDispatch.calledWithMatch(sinon.match(
           function(e) {
@@ -95,6 +96,9 @@ suite('system/LockScreenWindow', function() {
       // Or the AppWindow would look for it.
       app.element = document.createElement('div');
       parentElement.appendChild(app.element);
+      app.transitionController = {
+        requireClose: function() {}
+      };
       app.kill();
       app.close();
       assert.isTrue(stubDispatch.calledWithMatch(sinon.match(

diff --git a/apps/system/test/unit/secure_window_test.js b/apps/system/test/unit/secure_window_test.js
@@ -69,6 +69,7 @@ suite('system/SecureWindow', function() {
       // Or the AppWindow would look for it.
       app.element = document.createElement('div');
       parentElement.appendChild(app.element);
+      app.transitionController = {};
       app.kill();
       assert.isTrue(stubDispatch.calledWithMatch(sinon.match(
           function(e) {
@@ -89,6 +90,9 @@ suite('system/SecureWindow', function() {
       // Or the AppWindow would look for it.
       app.element = document.createElement('div');
       parentElement.appendChild(app.element);
+      app.transitionController = {
+        requireClose: function() {}
+      };
       app.kill();
       app.close();
       assert.isTrue(stubDispatch.calledWithMatch(sinon.match(