fix(carousel): replaced $timeout with $interval when it was wrong

I had a to tweak a bit two tests since $interval never throws an exception after flushing, hope I did it OK.

Ammend: addresses @chrisirhc critics

Fixes #1308
Fixes #2454
Closes #2776

Author: Gahen

Diff for: src/carousel/carousel.js

@@ -7,11 +7,11 @@
 *
 */
 angular.module('ui.bootstrap.carousel', ['ui.bootstrap.transition'])
-.controller('CarouselController', ['$scope', '$timeout', '$transition', function ($scope, $timeout, $transition) {
+.controller('CarouselController', ['$scope', '$timeout', '$interval', '$transition', function ($scope, $timeout, $interval, $transition) {
   var self = this,
     slides = self.slides = $scope.slides = [],
     currentIndex = -1,
-    currentTimeout, isPlaying;
+    currentInterval, isPlaying;
   self.currentSlide = null;
 
   var destroyed = false;
@@ -106,22 +106,22 @@ angular.module('ui.bootstrap.carousel', ['ui.bootstrap.transition'])
   function restartTimer() {
     resetTimer();
     var interval = +$scope.interval;
-    if (!isNaN(interval) && interval>=0) {
-      currentTimeout = $timeout(timerFn, interval);
+    if (!isNaN(interval) && interval > 0) {
+      currentInterval = $interval(timerFn, interval);
     }
   }
 
   function resetTimer() {
-    if (currentTimeout) {
-      $timeout.cancel(currentTimeout);
-      currentTimeout = null;
+    if (currentInterval) {
+      $interval.cancel(currentInterval);
+      currentInterval = null;
     }
   }
 
   function timerFn() {
-    if (isPlaying) {
+    var interval = +$scope.interval;
+    if (isPlaying && !isNaN(interval) && interval > 0) {
       $scope.next();
-      restartTimer();
     } else {
       $scope.pause();
     }

Diff for: src/carousel/test/carousel.spec.js

@@ -14,12 +14,12 @@ describe('carousel', function() {
   }));
   beforeEach(module('template/carousel/carousel.html', 'template/carousel/slide.html'));
 
-  var $rootScope, $compile, $controller, $timeout;
-  beforeEach(inject(function(_$rootScope_, _$compile_, _$controller_, _$timeout_) {
+  var $rootScope, $compile, $controller, $interval;
+  beforeEach(inject(function(_$rootScope_, _$compile_, _$controller_, _$interval_) {
     $rootScope = _$rootScope_;
     $compile = _$compile_;
     $controller = _$controller_;
-    $timeout = _$timeout_;
+    $interval = _$interval_;
   }));
 
   describe('basics', function() {
@@ -148,16 +148,18 @@ describe('carousel', function() {
 
     it('shouldnt go forward if interval is NaN or negative', function() {
       testSlideActive(0);
+      var previousInterval = scope.interval;
       scope.$apply('interval = -1');
-      //no timeout to flush, interval watch doesn't make a new one when interval is invalid
+      $interval.flush(previousInterval);
       testSlideActive(0);
       scope.$apply('interval = 1000');
-      $timeout.flush();
+      $interval.flush(1000);
       testSlideActive(1);
       scope.$apply('interval = false');
+      $interval.flush(1000);
       testSlideActive(1);
       scope.$apply('interval = 1000');
-      $timeout.flush();
+      $interval.flush(1000);
       testSlideActive(2);
     });
 
@@ -182,34 +184,35 @@ describe('carousel', function() {
 
     it('should be playing by default and cycle through slides', function() {
       testSlideActive(0);
-      $timeout.flush();
+      $interval.flush(scope.interval);
       testSlideActive(1);
-      $timeout.flush();
+      $interval.flush(scope.interval);
       testSlideActive(2);
-      $timeout.flush();
+      $interval.flush(scope.interval);
       testSlideActive(0);
     });
 
     it('should pause and play on mouseover', function() {
       testSlideActive(0);
-      $timeout.flush();
+      $interval.flush(scope.interval);
       testSlideActive(1);
       elm.trigger('mouseenter');
-      expect($timeout.flush).toThrow();//pause should cancel current timeout
+      testSlideActive(1);
+      $interval.flush(scope.interval);
       testSlideActive(1);
       elm.trigger('mouseleave');
-      $timeout.flush();
+      $interval.flush(scope.interval);
       testSlideActive(2);
     });
 
     it('should not pause on mouseover if noPause', function() {
       scope.$apply('nopause = true');
       testSlideActive(0);
       elm.trigger('mouseenter');
-      $timeout.flush();
+      $interval.flush(scope.interval);
       testSlideActive(1);
       elm.trigger('mouseleave');
-      $timeout.flush();
+      $interval.flush(scope.interval);
       testSlideActive(2);
     });
 
@@ -219,7 +222,7 @@ describe('carousel', function() {
       scope.$apply('slides.splice(0,1)');
       expect(elm.find('div.item').length).toBe(2);
       testSlideActive(1);
-      $timeout.flush();
+      $interval.flush(scope.interval);
       testSlideActive(0);
       scope.$apply('slides.splice(1,1)');
       expect(elm.find('div.item').length).toBe(1);
@@ -253,14 +256,15 @@ describe('carousel', function() {
 
     it('issue 1414 - should not continue running timers after scope is destroyed', function() {
       testSlideActive(0);
-      $timeout.flush();
+      $interval.flush(scope.interval);
       testSlideActive(1);
-      $timeout.flush();
+      $interval.flush(scope.interval);
       testSlideActive(2);
-      $timeout.flush();
+      $interval.flush(scope.interval);
       testSlideActive(0);
+      spyOn($interval, 'cancel').andCallThrough();
       scope.$destroy();
-      expect($timeout.flush).toThrow('No deferred tasks to be flushed');
+      expect($interval.cancel).toHaveBeenCalled();
     });
 
   });
@@ -326,12 +330,12 @@ describe('carousel', function() {
         scope.interval = 2000;
         scope.$digest();
 
-        $timeout.flush();
+        $interval.flush(scope.interval);
         expect(scope.next.calls.length).toBe(1);
 
         scope.$destroy();
 
-        $timeout.flush(scope.interval);
+        $interval.flush(scope.interval);
         expect(scope.next.calls.length).toBe(1);
       });
     });