add $browser.defer and $defer service and fix async xhr cache issue

- Closes #152 ($resource().query() sometimes calls callback before
  returning, and it shouldn't)
- add $browser.defer method
- add $defer service
- integrate $browser.defer with outstandingRequests counter in $browser
- fix all old tests that relied on buggy behavior

8 files changed, 205 insertions, 28 deletions

diff --git a/src/AngularPublic.js b/src/AngularPublic.js
index 98e0eed8..af572340 100644
--- a/src/AngularPublic.js
+++ b/src/AngularPublic.js
@@ -15,7 +15,8 @@ angularService('$browser', function($log){
         jqLite(window.document),
         jqLite(window.document.getElementsByTagName('head')[0]),
         XHR,
-        $log);
+        $log,
+        window.setTimeout);
     browserSingleton.startPoller(50, function(delay, fn){setTimeout(delay,fn);});
     browserSingleton.bind();
   }
diff --git a/src/Browser.js b/src/Browser.js
index 197cf1f4..94807a8c 100644
--- a/src/Browser.js
+++ b/src/Browser.js
@@ -8,7 +8,7 @@ var XHR = window.XMLHttpRequest || function () {
   throw new Error("This browser does not support XMLHttpRequest.");
 };
 
-function Browser(location, document, head, XHR, $log) {
+function Browser(location, document, head, XHR, $log, setTimeout) {
   var self = this;
   self.isMock = false;
 
@@ -19,6 +19,28 @@ function Browser(location, document, head, XHR, $log) {
   var outstandingRequestCount = 0;
   var outstandingRequestCallbacks = [];
 
+
+  /**
+   * Executes the `fn` function (supports currying) and decrements the `outstandingRequestCallbacks`
+   * counter. If the counter reaches 0, all the `outstandingRequestCallbacks` are executed.
+   */
+  function completeOutstandingRequest(fn) {
+    try {
+      fn.apply(null, slice.call(arguments, 1));
+    } finally {
+      outstandingRequestCount--;
+      if (outstandingRequestCount === 0) {
+        while(outstandingRequestCallbacks.length) {
+          try {
+            outstandingRequestCallbacks.pop()();
+          } catch (e) {
+            $log.error(e);
+          }
+        }
+      }
+    }
+  }
+
   /**
    * @workInProgress
    * @ngdoc method
@@ -58,19 +80,7 @@ function Browser(location, document, head, XHR, $log) {
       outstandingRequestCount ++;
       xhr.onreadystatechange = function() {
         if (xhr.readyState == 4) {
-          try {
-            callback(xhr.status || 200, xhr.responseText);
-          } finally {
-            outstandingRequestCount--;
-            if (outstandingRequestCount === 0) {
-              while(outstandingRequestCallbacks.length) {
-                try {
-                  outstandingRequestCallbacks.pop()();
-                } catch (e) {
-                }
-              }
-            }
-          }
+          completeOutstandingRequest(callback, xhr.status || 200, xhr.responseText);
         }
       };
       xhr.send(post || '');
@@ -250,6 +260,27 @@ function Browser(location, document, head, XHR, $log) {
     }
   };
 
+
+  /**
+   * @workInProgress
+   * @ngdoc
+   * @name angular.service.$browser#defer
+   * @methodOf angular.service.$browser
+   *
+   * @description
+   * Executes a fn asynchroniously via `setTimeout(fn, 0)`.
+   *
+   * Unlike when calling `setTimeout` directly, in test this function is mocked and instead of using
+   * `setTimeout` in tests, the fns are queued in an array, which can be programaticaly flushed via
+   * `$browser.defer.flush()`.
+   *
+   * @param {function()} fn A function, who's execution should be defered.
+   */
+  self.defer = function(fn) {
+    outstandingRequestCount++;
+    setTimeout(function() { completeOutstandingRequest(fn); }, 0);
+  };
+
   //////////////////////////////////////////////////////////////
   // Misc API
   //////////////////////////////////////////////////////////////
diff --git a/src/services.js b/src/services.js
index 91932b44..ef3de549 100644
--- a/src/services.js
+++ b/src/services.js
@@ -685,7 +685,7 @@ angularServiceInject('$route', function(location) {
  * @ngdoc service
  * @name angular.service.$xhr
  * @requires $browser
- * @requires $error
+ * @requires $xhr.error
  * @requires $log
  * 
  * @description
@@ -801,6 +801,36 @@ angularServiceInject('$xhr.bulk', function($xhr, $error, $log){
   return bulkXHR;
 }, ['$xhr', '$xhr.error', '$log']);
 
+
+/**
+ * @workInProgress
+ * @ngdoc service
+ * @name angular.service.$defer
+ * @requires $browser
+ * @requires $log
+ *
+ * @description
+ * Delegates to {@link angular.service.$browser.defer $browser.defer}, but wraps the `fn` function
+ * into a try/catch block and delegates any exceptions to
+ * {@link angular.services.$exceptionHandler $exceptionHandler} service.
+ *
+ * In tests you can use `$browser.defer.flush()` to flush the queue of deferred functions.
+ *
+ * @param {function()} fn A function, who's execution should be deferred.
+ */
+angularServiceInject('$defer', function($browser, $exceptionHandler) {
+  return function(fn) {
+    $browser.defer(function() {
+      try {
+        fn();
+      } catch(e) {
+        $exceptionHandler(e);
+      }
+    });
+  };
+}, ['$browser', '$exceptionHandler']);
+
+
 /**
  * @workInProgress
  * @ngdoc service
@@ -811,7 +841,7 @@ angularServiceInject('$xhr.bulk', function($xhr, $error, $log){
  * 
  * @example
  */
-angularServiceInject('$xhr.cache', function($xhr){
+angularServiceInject('$xhr.cache', function($xhr, $defer){
   var inflight = {}, self = this;
   function cache(method, url, post, callback, verifyCache){
     if (isFunction(post)) {
@@ -819,9 +849,9 @@ angularServiceInject('$xhr.cache', function($xhr){
       post = _null;
     }
     if (method == 'GET') {
-      var data;
-      if (data = cache.data[url]) {
-        callback(200, copy(data.value));
+      var data, dataCached;
+      if (dataCached = cache.data[url]) {
+        $defer(function() { callback(200, copy(dataCached.value)); });
         if (!verifyCache)
           return;
       }
@@ -853,7 +883,7 @@ angularServiceInject('$xhr.cache', function($xhr){
   cache.data = {};
   cache.delegate = $xhr;
   return cache;
-}, ['$xhr.bulk']);
+}, ['$xhr.bulk', '$defer']);
 
 
 /**
diff --git a/test/BrowserSpecs.js b/test/BrowserSpecs.js
index 5f28b610..eb43e3c5 100644
--- a/test/BrowserSpecs.js
+++ b/test/BrowserSpecs.js
@@ -1,8 +1,21 @@
 describe('browser', function(){
 
-  var browser, location, head, xhr;
+  var browser, location, head, xhr, setTimeoutQueue;
+
+  function fakeSetTimeout(fn) {
+    setTimeoutQueue.push(fn);
+  }
+
+  fakeSetTimeout.flush = function() {
+    foreach(setTimeoutQueue, function(fn) {
+      fn();
+    });
+  };
+
 
   beforeEach(function(){
+    setTimeoutQueue = [];
+
     location = {href:"http://server", hash:""};
     head = {
         scripts: [],
@@ -14,7 +27,7 @@ describe('browser', function(){
       this.open = noop;
       this.setRequestHeader = noop;
       this.send = noop;
-    });
+    }, undefined, fakeSetTimeout);
   });
 
   it('should contain cookie cruncher', function() {
@@ -59,6 +72,28 @@ describe('browser', function(){
   });
 
 
+  describe('defer', function() {
+    it('should execute fn asynchroniously via setTimeout', function() {
+      var counter = 0;
+      browser.defer(function() {counter++;});
+      expect(counter).toBe(0);
+
+      fakeSetTimeout.flush();
+      expect(counter).toBe(1);
+    });
+
+
+    it('should update outstandingRequests counter', function() {
+      var callback = jasmine.createSpy('callback');
+      browser.defer(callback);
+      expect(callback).not.wasCalled();
+
+      fakeSetTimeout.flush();
+      expect(callback).wasCalled();
+    });
+  });
+
+
   describe('cookies', function() {
 
     function deleteAllCookies() {
diff --git a/test/ResourceSpec.js b/test/ResourceSpec.js
index a026263b..6b987bd8 100644
--- a/test/ResourceSpec.js
+++ b/test/ResourceSpec.js
@@ -184,6 +184,8 @@ describe("resource", function() {
 
     $browser.xhr.expectGET('/Person/123').respond('[\n{\nname:\n"rob"\n}\n]');
     var person2 = Person.query({id:123});
+    $browser.defer.flush();
+
     expect(person2[0].name).toEqual('misko');
     var person2Cache = person2;
     $browser.xhr.flush();
diff --git a/test/angular-mocks.js b/test/angular-mocks.js
index 0f637bf7..fd53a189 100644
--- a/test/angular-mocks.js
+++ b/test/angular-mocks.js
@@ -113,6 +113,15 @@ function MockBrowser() {
 
   self.cookieHash = {};
   self.lastCookieHash = {};
+  self.deferredFns = [];
+
+  self.defer = function(fn) {
+    self.deferredFns.push(fn);
+  };
+
+  self.defer.flush = function() {
+    while (self.deferredFns.length) self.deferredFns.shift()();
+  };
 }
 MockBrowser.prototype = {
 
@@ -156,7 +165,6 @@ MockBrowser.prototype = {
       return this.cookieHash;
     }
   }
-
 };
 
 angular.service('$browser', function(){
diff --git a/test/servicesSpec.js b/test/servicesSpec.js
index ff90e0a1..5a9a4702 100644
--- a/test/servicesSpec.js
+++ b/test/servicesSpec.js
@@ -329,6 +329,47 @@ describe("service", function(){
     });
   });
 
+
+  describe('$defer', function() {
+    var $defer, $exceptionHandler;
+
+    beforeEach(function(){
+      scope = createScope({}, angularService, {
+        '$exceptionHandler': jasmine.createSpy('$exceptionHandler')
+      });
+
+      $browser = scope.$inject('$browser');
+      $defer = scope.$inject('$defer');
+      $exceptionHandler = scope.$inject('$exceptionHandler');
+    });
+
+
+    it('should delegate functions to $browser.defer', function() {
+      var counter = 0;
+      $defer(function() { counter++; });
+
+      expect(counter).toBe(0);
+
+      $browser.defer.flush();
+      expect(counter).toBe(1);
+
+      $browser.defer.flush(); //does nothing
+      expect(counter).toBe(1);
+
+      expect($exceptionHandler).not.toHaveBeenCalled();
+    });
+
+
+    it('should delegate exception to the $exceptionHandler service', function() {
+      $defer(function() { throw "Test Error"; });
+      expect($exceptionHandler).not.toHaveBeenCalled();
+
+      $browser.defer.flush();
+      expect($exceptionHandler).toHaveBeenCalledWith("Test Error");
+    });
+  });
+
+
   describe('$xhr', function(){
     var log;
     function callback(code, response) {
@@ -426,12 +467,15 @@ describe("service", function(){
         $browserXhr.expectGET('/url').respond('first');
         cache('GET', '/url', null, callback);
         $browserXhr.flush();
+
         $browserXhr.expectGET('/url').respond('ERROR');
         cache('GET', '/url', null, callback);
+        $browser.defer.flush();
         $browserXhr.flush();
         expect(log).toEqual('"first";"first";');
+
         cache('GET', '/url', null, callback, false);
-        $browserXhr.flush();
+        $browser.defer.flush();
         expect(log).toEqual('"first";"first";"first";');
       });
 
@@ -439,9 +483,12 @@ describe("service", function(){
         $browserXhr.expectGET('/url').respond('first');
         cache('GET', '/url', null, callback, true);
         $browserXhr.flush();
+
         $browserXhr.expectGET('/url').respond('ERROR');
         cache('GET', '/url', null, callback, true);
+        $browser.defer.flush();
         expect(log).toEqual('"first";"first";');
+
         $browserXhr.flush();
         expect(log).toEqual('"first";"first";"ERROR";');
       });
@@ -449,8 +496,11 @@ describe("service", function(){
       it('should serve requests from cache', function(){
         cache.data.url = {value:'123'};
         cache('GET', 'url', null, callback);
+        $browser.defer.flush();
         expect(log).toEqual('"123";');
+
         cache('GET', 'url', null, callback, false);
+        $browser.defer.flush();
         expect(log).toEqual('"123";"123";');
       });
 
@@ -478,6 +528,21 @@ describe("service", function(){
         cache('POST', 'abc', {});
         expect(cache.data.url).toBeUndefined();
       });
+
+      it('should call callback asynchronously for both cache hit and cache miss', function() {
+        $browserXhr.expectGET('/url').respond('+');
+        cache('GET', '/url', null, callback);
+        expect(log).toEqual(''); //callback hasn't executed
+
+        $browserXhr.flush();
+        expect(log).toEqual('"+";'); //callback has executed
+
+        cache('GET', '/url', null, callback);
+        expect(log).toEqual('"+";'); //callback hasn't executed
+
+        $browser.defer.flush();
+        expect(log).toEqual('"+";"+";'); //callback has executed
+      });
     });
 
   });
diff --git a/test/widgetsSpec.js b/test/widgetsSpec.js
index cb3b76a1..ceec2c90 100644
--- a/test/widgetsSpec.js
+++ b/test/widgetsSpec.js
@@ -530,6 +530,7 @@ describe("widget", function(){
       scope.url = 'myUrl';
       scope.$inject('$xhr.cache').data.myUrl = {value:'{{name}}'};
       scope.$init();
+      scope.$inject('$browser').defer.flush();
       expect(element.text()).toEqual('misko');
       dealoc(scope);
     });
@@ -542,6 +543,7 @@ describe("widget", function(){
       scope.url = 'myUrl';
       scope.$inject('$xhr.cache').data.myUrl = {value:'{{name}}'};
       scope.$init();
+      scope.$inject('$browser').defer.flush();
 
       expect(element.text()).toEqual('igor');
 
@@ -558,9 +560,11 @@ describe("widget", function(){
       scope.url = 'myUrl';
       scope.$inject('$xhr.cache').data.myUrl = {value:'{{c=c+1}}'};
       scope.$init();
-      // This should not be 4, but to fix this properly
-      // we need to have real events on the scopes.
-      expect(element.text()).toEqual('4');
+      scope.$inject('$browser').defer.flush();
+
+      // this one should really be just '1', but due to lack of real events things are not working
+      // properly. see discussion at: http://is.gd/ighKk
+      expect(element.text()).toEqual('2');
       dealoc(scope);
     });
 
@@ -573,6 +577,7 @@ describe("widget", function(){
       scope.url = 'myUrl';
       scope.$inject('$xhr.cache').data.myUrl = {value:'my partial'};
       scope.$init();
+      scope.$inject('$browser').defer.flush();
       expect(element.text()).toEqual('my partial');
       expect(scope.loaded).toBe(true);
       dealoc(scope);