Server Request Tests and Mock Promise Enhancements

chrome/browser/resources/google_now/common_test_util.js

Index: chrome/browser/resources/google_now/common_test_util.js
diff --git a/chrome/browser/resources/google_now/common_test_util.js b/chrome/browser/resources/google_now/common_test_util.js
index 66d65f49d9efa9d3f0f77aa6db76165f119cf9c1..af4c453c95b41c616893a8bf56c34cf93908daf7 100644
--- a/chrome/browser/resources/google_now/common_test_util.js
+++ b/chrome/browser/resources/google_now/common_test_util.js
@@ -81,33 +81,65 @@ function getMockHandlerContainer(eventIdentifier) {
  */
 var Promise = function() {
   function PromisePrototypeObject(asyncTask) {
-    var result;
-    var resolved = false;
-    asyncTask(
-        function(asyncResult) {
-          result = asyncResult;
-          resolved = true;
-        },
-        function(asyncFailureResult) {
-          result = asyncFailureResult;
-          resolved = false;
-        });
+    function isThenable(value) {
+      return (typeof value === 'object') && isCallable(value.then);
+    }
 
-    function then(callback) {
-      if (resolved) {
-        callback.call(null, result);
+    function isCallable(value) {
+      return typeof value === 'function';
+    }
+
+    function callResolveRejectFunc(func) {
+      var funcResult;
+      var funcResolved = false;
+      func(
+          function(resolveResult) {
+            funcResult = resolveResult;
+            funcResolved = true;
+          },
+          function(rejectResult) {
+            funcResult = rejectResult;
+            funcResolved = false;
+          });
+      return { result: funcResult, resolved: funcResolved };
+    }
+
+    function then(onResolve, onReject) {
+      var resolutionHandler =
+          isCallable(onResolve) ? onResolve : function() { return result; };
+      var rejectionHandler =
+          isCallable(onReject) ? onReject : function() { return result; };
+      var handlerResult =
+          resolved ? resolutionHandler(result) : rejectionHandler(result);
+      var promiseResolved = resolved;
+      if (isThenable(handlerResult)) {
+        var resolveReject = callResolveRejectFunc(handlerResult.then);
+        handlerResult = resolveReject.result;
+        promiseResolved = resolveReject.resolved;
+      }
+
+      if (promiseResolved) {
+        return Promise.resolve(handlerResult);
+      } else {
+        return Promise.reject(handlerResult);
       }
-      return this;
     }
 
     // Promises use the function name "catch" to call back error handlers.
     // We can't use "catch" since function or variable names cannot use the word
     // "catch".
-    function catchFunc(callback) {
-      if (!resolved) {
-        callback.call(null, result);
-      }
-      return this;
+    function catchFunc(onRejected) {
+      return this.then(undefined, onRejected);
+    }
+
+    var resolveReject = callResolveRejectFunc(asyncTask);
+    var result = resolveReject.result;
+    var resolved = resolveReject.resolved;
+
+    if (isThenable(result)) {

[rgustafson, 2014/03/19 00:24:31]

I don't quite understand why this is here (and above). I'm guessing that you've
run these tests and they work, but this is a pretty hard read.

[robliao, 2014/03/19 00:43:39]

This code is indeed crazy. This implementation is adapted from the ECMAScript 6
spec
(http://wiki.ecmascript.org/lib/exe/fetch.php?id=harmony%3Aspecification_draft...)
with me adding synchronicity to it.

The craziness stems from Section 25.4.1.8 - UpdatePromiseFromPotentialThenable,
which is used during reaction and resolution.

Page 490 (PDF Page 508, Section 25.4) discusses Promises in all its detail.
On 2014/03/19 00:24:31, rgustafson wrote:

[rgustafson, 2014/03/19 23:02:25]

I'll take your word on this since that's just taking a small part of a more
complex larger spec. Maybe consider linking to that in comment for the whole
promise implementation, so some future person having to edit this has some idea
what's happening?

[robliao, 2014/03/19 23:34:39]

Links will change, so I've added a comment about this on the top.
On 2014/03/19 23:02:25, rgustafson wrote:

+      var thenResolveReject = callResolveRejectFunc(result.then);
+      result = thenResolveReject.result;
+      resolved = thenResolveReject.resolved;
     }
 
     return {then: then, catch: catchFunc, isPromise: true};
@@ -137,12 +169,19 @@ var Promise = function() {
     return promise;
   }
 
+  function reject(value) {
+    var promise = new PromisePrototypeObject(function(resolve, reject) {
+      reject(value);
+    });
+    return promise;
+  }
+
   PromisePrototypeObject.all = all;
   PromisePrototypeObject.resolve = resolve;

[skare_, 2014/03/19 23:20:23]

this is in the scope of a prior cl, so maybe I/we asked earlier :)

can these be placed inside the class definition above and then returned
alongside then/catch/isPromise?

[robliao, 2014/03/19 23:34:39]

What do you mean?

Are you asking for direct declarations like this?
PromisePrototypeObject.prototype.all = function(arrayOfPromises) {...}
On 2014/03/19 23:20:23, Travis Skare wrote:

[skare_, 2014/03/20 00:27:27]

moving all the callable functions/'api' to one place: 
return {then: then, ..., all: all}
and moving functions if necessary, though I don't think that's needed since they
should be in the closure.

but I'm not sure it's a net readability gain, so it was pre-lgtm'ed


On 2014/03/19 23:34:39, robliao wrote:

[robliao, 2014/03/20 00:31:55]

Ah. That wouldn't work. We actually need a constructor for this.
new Promise(...) is used by the code, which is why you see this sort of
initialization.

then, catch, and isPromise require instances. all, resolve, and reject are
"statics" and do not exist off of the global Promise object.

On 2014/03/20 00:27:27, Travis Skare wrote:

[robliao, 2014/03/20 00:32:49]

s/do not exist/exist/
On 2014/03/20 00:31:55, robliao wrote:

+  PromisePrototypeObject.reject = reject;
   return PromisePrototypeObject;
 }();
 
-
 /**
  * Sets up the test to expect a Chrome Local Storage call.
  * @param {Object} fixture Mock JS Test Object.