fix(popup): fix race conditions and memory leaks

Closes #2815
ionic-team · Apr 13, 2015 · e86b331 · e86b331
1 parent 008df7b
commit e86b331
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 60 deletions.
diff --git a/js/angular/service/popup.js b/js/angular/service/popup.js
@@ -119,6 +119,7 @@ function($ionicTemplateLoader, $ionicBackdrop, $q, $timeout, $rootScope, $ionicB
     stackPushDelay: 75
   };
   var popupStack = [];
+
   var $ionicPopup = {
     /**
      * @ngdoc method
@@ -288,8 +289,7 @@ function($ionicTemplateLoader, $ionicBackdrop, $q, $timeout, $rootScope, $ionicB
       $ionicTemplateLoader.load(options.templateUrl) :
       $q.when(options.template || options.content || '');
 
-    return $q.all([popupPromise, contentPromise])
-    .then(function(results) {
+    return $q.all([popupPromise, contentPromise]).then(function(results) {
       var self = results[0];
       var content = results[1];
       var responseDeferred = $q.defer();
@@ -322,7 +322,7 @@ function($ionicTemplateLoader, $ionicBackdrop, $q, $timeout, $rootScope, $ionicB
       });
 
       self.show = function() {
-        if (self.isShown) return;
+        if (self.isShown || self.removed) return;
 
         self.isShown = true;
         ionic.requestAnimationFrame(function() {
@@ -334,6 +334,7 @@ function($ionicTemplateLoader, $ionicBackdrop, $q, $timeout, $rootScope, $ionicB
           focusInput(self.element);
         });
       };
+
       self.hide = function(callback) {
         callback = callback || noop;
         if (!self.isShown) return callback();
@@ -343,6 +344,7 @@ function($ionicTemplateLoader, $ionicBackdrop, $q, $timeout, $rootScope, $ionicB
         self.element.addClass('popup-hidden');
         $timeout(callback, 250);
       };
+
       self.remove = function() {
         if (self.removed) return;
 
@@ -359,74 +361,79 @@ function($ionicTemplateLoader, $ionicBackdrop, $q, $timeout, $rootScope, $ionicB
   }
 
   function onHardwareBackButton(e) {
-    popupStack[0] && popupStack[0].responseDeferred.resolve();
+    var last = popupStack[popupStack.length - 1];
+    last && last.responseDeferred.resolve();
   }
 
   function showPopup(options) {
+    var resultDeferred;
     var popupPromise = $ionicPopup._createPopup(options);
-    var previousPopup = popupStack[0];
 
-    if (previousPopup) {
-      previousPopup.hide();
+    if (popupStack.length > 0) {
+      popupStack[popupStack.length - 1].hide();
+      resultDeferred = $timeout(doShowPopup, config.stackPushDelay);
+    } else {
+      //Add popup-open & backdrop if this is first popup
+      $ionicBody.addClass('popup-open');
+      $ionicBackdrop.retain();
+      //only show the backdrop on the first popup
+      $ionicPopup._backButtonActionDone = $ionicPlatform.registerBackButtonAction(
+        onHardwareBackButton,
+        PLATFORM_BACK_BUTTON_PRIORITY_POPUP
+      );
+      resultDeferred = doShowPopup();
     }
 
-    var resultPromise = $timeout(noop, previousPopup ? config.stackPushDelay : 0)
-    .then(function() { return popupPromise; })
-    .then(function(popup) {
-      if (!previousPopup) {
-        //Add popup-open & backdrop if this is first popup
-        $ionicBody.addClass('popup-open');
-        $ionicBackdrop.retain();
-        //only show the backdrop on the first popup
-        $ionicPopup._backButtonActionDone = $ionicPlatform.registerBackButtonAction(
-          onHardwareBackButton,
-          PLATFORM_BACK_BUTTON_PRIORITY_POPUP
-        );
-      }
-      popupStack.unshift(popup);
-      popup.show();
-
-      //DEPRECATED: notify the promise with an object with a close method
-      popup.responseDeferred.notify({
-        close: resultPromise.close
+    resultDeferred.close = function popupClose(result) {
+      popupPromise.then(function(popup) {
+        if (!popup.removed) popup.responseDeferred.resolve(result);
       });
+    };
 
-      return popup.responseDeferred.promise.then(function(result) {
-        var index = popupStack.indexOf(popup);
-        if (index !== -1) {
-          popupStack.splice(index, 1);
-        }
-        popup.remove();
-
-        var previousPopup = popupStack[0];
-        if (previousPopup) {
-          previousPopup.show();
-        } else {
-          //Remove popup-open & backdrop if this is last popup
-          $timeout(function() {
-            // wait to remove this due to a 300ms delay native
-            // click which would trigging whatever was underneath this
-            $ionicBody.removeClass('popup-open');
-          }, 400);
-          $timeout(function() {
-            $ionicBackdrop.release();
-          }, config.stackPushDelay || 0);
-          ($ionicPopup._backButtonActionDone || noop)();
-        }
-        return result;
-      });
-    });
+    return resultDeferred;
+
+    function doShowPopup() {
+      return popupPromise.then(function(popup) {
+        popupStack.push(popup);
+        popup.show();
+
+        //DEPRECATED: notify the promise with an object with a close method
+        popup.responseDeferred.notify({
+          close: resultDeferred.close
+        });
+
+        return popup.responseDeferred.promise.then(function(result) {
+          var index = popupStack.indexOf(popup);
+          if (index !== -1) {
+            popupStack.splice(index, 1);
+          }
+          popup.remove();
+
+          if (popupStack.length > 0) {
+            popupStack[popupStack.length - 1].show();
+          } else {
+            //Remove popup-open & backdrop if this is last popup
+            $timeout(function() {
+              // wait to remove this due to a 300ms delay native
+              // click which would trigging whatever was underneath this
+              if (!popupStack.length) {
+                $ionicBody.removeClass('popup-open');
+              }
+            }, 400, false);
+            $timeout(function() {
+              if (!popupStack.length) $ionicBackdrop.release();
+            }, config.stackPushDelay || 0, false);
+
+            ($ionicPopup._backButtonActionDone || noop)();
+          }
+
+          return result;
+        });
 
-    function close(result) {
-      popupPromise.then(function(popup) {
-        if (!popup.removed) {
-          popup.responseDeferred.resolve(result);
-        }
       });
+
     }
-    resultPromise.close = close;
 
-    return resultPromise;
   }
 
   function focusInput(element) {

diff --git a/test/unit/angular/service/popup.unit.js b/test/unit/angular/service/popup.unit.js
@@ -277,8 +277,8 @@ describe('$ionicPopup service', function() {
       expect($ionicBackdrop.release).toHaveBeenCalled();
       expect(document.body.classList.contains('popup-open')).toBe(false);
     }));
-    it('template should only overwrite prompt input if it includes html', inject(function($timeout) {
-      spyOn($ionicPopup, '_createPopup');
+    it('template should only overwrite prompt input if it includes html', inject(function($timeout, $q) {
+      spyOn($ionicPopup, '_createPopup').andCallThrough();
       $ionicPopup.prompt({template: "Tacos!"});
       params = $ionicPopup._createPopup.mostRecentCall.args;
       expect(params[0].template.indexOf('<span>Tacos!</span>')).toEqual(0);