feat(http): run interceptors synchronously until first non-sync interceptor

Author: yjbanov

lib/core_dom/http.dart

@@ -457,7 +457,7 @@ class Http {
       if (v is Function) headers[k] = v();
     });
 
-    var serverRequest = (HttpResponseConfig config) {
+    serverRequest(HttpResponseConfig config) {
       // Strip content-type if data is undefined
       if (config.data == null) {
         new List.from(headers.keys)
@@ -509,11 +509,11 @@ class Http {
 
     var chain = [[serverRequest, null]];
 
-    var future = new async.Future.value(new HttpResponseConfig(
+    var initialInput = new HttpResponseConfig(
         url: url,
         params: params,
         headers: headers,
-        data: data));
+        data: data);
 
     _interceptors.constructChain(chain);
 
@@ -525,11 +525,18 @@ class Http {
       interceptors.constructChain(chain);
     }
 
-    chain.forEach((chainFns) {
-      future = future.then(chainFns[0], onError: chainFns[1]);
-    });
-
-    return future;
+    // Try to run interceptors synchronously until one of them returns a Future. This
+    // makes sure that in common cases the HTTP backend sends the HTTP request immediately
+    // saving dozens of millis of RPC latency.
+    var chainResult = chain.fold(initialInput, (prev, chainFns) => prev is async.Future
+        ? prev.then(chainFns[0], onError: chainFns[1])
+        : chainFns[0](prev));
+
+    // Depending on the implementation of HttpBackend (e.g. with a local cache) the entire
+    // chain could finish synchronously with a non-Future result.
+    return chainResult is async.Future
+        ? chainResult
+        : new async.Future.value(chainResult);
   }
 
   /**

test/core_dom/http_spec.dart

@@ -88,9 +88,8 @@ void main() {
         // we don't care about the data field.
         backend.expect('POST', '/url', 'null').respond('');
 
-        http(url: '/url', method: 'POST');
         expect(() {
-          flush();
+          http(url: '/url', method: 'POST');
         }).toThrow('with different data');
 
         // satisfy the expectation for our afterEach's assert.
@@ -952,6 +951,60 @@ void main() {
           flush();
         }));
       });
+
+      describe('request interceptors', () {
+        bool interceptorCalled;
+
+        beforeEach(() {
+          interceptorCalled = false;
+        });
+
+        describe('synchronous', () {
+          beforeEachModule((Module module) {
+            module.bind(HttpInterceptors, toValue: new HttpInterceptors()
+              // The first interceptor is sync, causing the second interceptor to be called synchronously
+              ..add(new HttpInterceptor(request: (cfg) => cfg))
+              ..add(new HttpInterceptor(request: (cfg) {
+                interceptorCalled = true;
+                return cfg;
+              })));
+          });
+
+          it('should call backend synchronously if request interceptor chain is '
+             'synchronous', async(() {
+            backend.expect('POST', '/url', '').respond('');
+            http(url: '/url', method: 'POST', data: '');
+            expect(interceptorCalled).toBe(true);
+            expect(backend.responses.isEmpty).toBe(false);  // request made immediately
+            flush();
+          }));
+        });
+
+        describe('asynchronous', () {
+          beforeEachModule((Module module) {
+            module.bind(HttpInterceptors, toValue: new HttpInterceptors()
+              // The first interceptor is async, causing the second interceptor to be
+              // called in a microtask
+              ..add(new HttpInterceptor(request: (cfg) => new Future.value(cfg)))
+              ..add(new HttpInterceptor(request: (cfg) {
+                interceptorCalled = true;
+                return cfg;
+              })));
+          });
+
+          it('should call backend asynchronously if request interceptor chain is '
+             'asynchronous', async(() {
+            backend.expect('POST', '/url', '').respond('');
+            http(url: '/url', method: 'POST', data: '');
+            expect(interceptorCalled).toBe(false);
+            expect(backend.expectations.isEmpty).toBe(false);
+            backend.verifyNoOutstandingRequest();
+
+            flush();
+            expect(interceptorCalled).toBe(true);
+          }));
+        });
+      });
     });
 
     describe('url rewriting', () {