Task Management - Handle Promises "Then" or "Catch" callback case.

chrome/browser/resources/google_now/utility.js

Index: chrome/browser/resources/google_now/utility.js
diff --git a/chrome/browser/resources/google_now/utility.js b/chrome/browser/resources/google_now/utility.js
index f2236209fc498ffa15c0b2cc10c89d254e5a8c84..5a0042a9a7b3eda55253b6ad49d6a744b96e615a 100644
--- a/chrome/browser/resources/google_now/utility.js
+++ b/chrome/browser/resources/google_now/utility.js
@@ -219,6 +219,17 @@ var instrumented = {};
 var WrapperPlugin;
 
 /**
+ * Promise Metadata. Holds fields that are used in identifying a promise during
+ * task processing.
+ *
+ * @typedef {{
+ *     id: number,
+ *     isCatch: boolean
+ * }}
+ */
+var PromiseMetadata;
+
+/**
  * Wrapper for callbacks. Used to add error handling and other services to
  * callbacks for HTML and Chrome functions and events.
  */
@@ -233,13 +244,14 @@ var wrapper = (function() {
    * An instance of WrapperPlugin can have state that can be shared by its
    * constructor, prologue() and epilogue(). Also WrapperPlugins can change
    * state of other objects, for example, to do refcounting.
-   * @type {?function(): WrapperPlugin}
+   * @type {?function(PromiseMetadata): WrapperPlugin}
    */
   var wrapperPluginFactory = null;
 
   /**
    * Registers a wrapper plugin factory.
-   * @param {function(): WrapperPlugin} factory Wrapper plugin factory.
+   * @param {function(PromiseMetadata): WrapperPlugin} factory
+   *     Wrapper plugin factory.
    */
   function registerWrapperPluginFactory(factory) {
     if (wrapperPluginFactory) {
@@ -267,6 +279,18 @@ var wrapper = (function() {
   var pendingCallbacks = {};
 
   /**
+   * A map of promise IDs to an array of "then" callback IDs.
+   * @type {Object.<number, array.<number>>}
+   */
+  var pendingThenCallbacks = {};
+
+  /**
+   * A map of promise IDs to an array of "catch" callback IDs.
+   * @type {Object.<number, array.<number>>}
+   */
+  var pendingCatchCallbacks = {};
+
+  /**
    * Unique ID of the next callback.
    * @type {number}
    */
@@ -296,19 +320,29 @@ var wrapper = (function() {
    * @param {Function} callback Callback to instrument.
    * @param {boolean=} opt_isEventListener True if the callback is a listener to
    *     a Chrome API event.
+   * @param {PromiseMetadata=} opt_promiseMetadata Set if wrapped from a
+   *     promise.
    * @return {Function} Instrumented callback.
    */
-  function wrapCallback(callback, opt_isEventListener) {
+  function wrapCallback(callback, opt_isEventListener, opt_promiseMetadata) {
     var callbackId = nextCallbackId++;
 
     if (!opt_isEventListener) {
       checkInWrappedCallback();
       pendingCallbacks[callbackId] = new Error().stack + ' @' + Date.now();
+      if (opt_promiseMetadata) {
+        var callbackIdMap = opt_promiseMetadata.isCatch ?
+            pendingCatchCallbacks :
+            pendingThenCallbacks;
+        callbackIdMap[opt_promiseMetadata.id] =
+            callbackIdMap[opt_promiseMetadata.id] || [];
+        callbackIdMap[opt_promiseMetadata.id].push(callbackId);
+      }
     }
-
     // wrapperPluginFactory may be null before task manager is built, and in
     // tests.
-    var wrapperPluginInstance = wrapperPluginFactory && wrapperPluginFactory();
+    var wrapperPluginInstance =
+        wrapperPluginFactory && wrapperPluginFactory(opt_promiseMetadata);
 
     return function() {
       // This is the wrapper for the callback.
@@ -316,8 +350,19 @@ var wrapper = (function() {
         verify(!isInWrappedCallback, 'Re-entering instrumented callback');
         isInWrappedCallback = true;
 
-        if (!opt_isEventListener)
+        if (!opt_isEventListener) {
           delete pendingCallbacks[callbackId];
+          if (opt_promiseMetadata) {
+            var callbackIdsToRemove = opt_promiseMetadata.isCatch ?
+                pendingThenCallbacks[opt_promiseMetadata.id] :
+                pendingCatchCallbacks[opt_promiseMetadata.id];
+            if (callbackIdsToRemove !== undefined) {
+              callbackIdsToRemove.forEach(function(idToRemove) {
+                delete pendingCallbacks[idToRemove];
+              });
+            }
+          }
+        }
 
         if (wrapperPluginInstance)
           wrapperPluginInstance.prologue();
@@ -437,13 +482,31 @@ wrapper.instrumentChromeApiFunction('identity.removeCachedAuthToken', 1);
 wrapper.instrumentChromeApiFunction('webstorePrivate.getBrowserLogin', 0);
 
 /**
+ * Unique ID of the next promise.
+ * @type {number}
+ */
+var nextPromiseId = 0;
+
+/**
  * Add task tracking support to Promise.then.
  * @override
  */
 Promise.prototype.then = function() {
   var originalThen = Promise.prototype.then;
   return function(callback) {
-    return originalThen.call(this, wrapper.wrapCallback(callback, false));
+    var promiseId = this.__promiseId;
+    if (promiseId === undefined) {
+      promiseId = nextPromiseId;
+      nextPromiseId++;
+    }
+    // "then" may return a new promise, getting rid of the ID!
+    // Set it before and after the call to keep the value.
+    this.__promiseId = promiseId;

[rgustafson, 2014/02/24 21:55:12]

Still kinda confused in general by setting the same id to the promise calling
then and the one resulting from then. Really we're tracking promise chains and
not individual promises, right? Chained thens would all end up with the same id?
Want to make sure I know what's going on here.

In this case, can't multiple thens succeed and then one fail and catch is called
which contradicts what is said farther down in the file about whenever then is
called, catch isn't?

[robliao, 2014/02/24 22:31:13]

You're right in the general case (and we'll need to handle this later once we
get promise chaining fully working).

In our specific case, our thens do not return promises.

This means that either all thens or all catches will execute.

Once our thens accept promises, then it is possible to have a varying number of
thens and catches in a single chain get called.

On 2014/02/24 21:55:12, rgustafson wrote:

[robliao, 2014/02/24 23:11:56]

Added a comment about this limitation.
On 2014/02/24 22:31:13, robliao wrote:

+    var promise = originalThen.call(
+        this, wrapper.wrapCallback(callback, false,
+            {id: this.__promiseId, isCatch: false}));
+    promise.__promiseId = promiseId;
+    return promise;
   }
 }();
 
@@ -454,11 +517,34 @@ Promise.prototype.then = function() {
 Promise.prototype.catch = function() {
   var originalCatch = Promise.prototype.catch;
   return function(callback) {
-    return originalCatch.call(this, wrapper.wrapCallback(callback, false));
+    var promiseId = this.__promiseId;
+    if (promiseId === undefined) {
+      promiseId = nextPromiseId;
+      nextPromiseId++;
+    }
+    // "catch" may return a new promise, getting rid of the ID!
+    // Set it before and after the call to keep the value.
+    this.__promiseId = promiseId;
+    var promise = originalCatch.call(
+        this, wrapper.wrapCallback(callback, false,
+            {id: this.__promiseId, isCatch: true}));
+    promise.__promiseId = promiseId;
+    return promise;
   }
 }();
 
 /**
+ * Promise Pending Callback Data. Counts outstanding "then" and "catch"
+ * callbacks;
+ *
+ * @typedef {{
+ *     thenCount: number,
+ *     catchCount: number
+ * }}
+ */
+var PromisePendingCallbackData;
+
+/**
  * Builds the object to manage tasks (mutually exclusive chains of events).
  * @param {function(string, string): boolean} areConflicting Function that
  *     checks if a new task can't be added to a task queue that contains an
@@ -480,6 +566,13 @@ function buildTaskManager(areConflicting) {
   var taskPendingCallbackCount = 0;
 
   /**
+   * Map of Promise ID to PromisePendingCallbackData to count the number of
+   * outstanding "then" and "catch" callbacks.
+   * @type {object.<number, PromisePendingCallbackData>}
+   */
+  var taskPromisePendingCallbackData = {};
+
+  /**
    * True if currently executed code is a part of a task.
    * @type {boolean}
    */
@@ -573,11 +666,28 @@ function buildTaskManager(areConflicting) {
   /**
    * Wrapper plugin for tasks.
    * @constructor
+   * @param {PromiseMetadata=} opt_promiseMetadata Set if wrapped from a
+   *     promise.
    */
-  function TasksWrapperPlugin() {
+  function TasksWrapperPlugin(opt_promiseMetadata) {
     this.isTaskCallback = isInTask;
-    if (this.isTaskCallback)
+    if (this.isTaskCallback) {
       ++taskPendingCallbackCount;
+      if (opt_promiseMetadata !== undefined) {
+        this.promiseId = opt_promiseMetadata.id;
+        if (!taskPromisePendingCallbackData[this.promiseId]) {
+          taskPromisePendingCallbackData[this.promiseId] =
+              {thenCount: 0, catchCount: 0};
+        }
+        if (opt_promiseMetadata.isCatch) {
+          taskPromisePendingCallbackData[this.promiseId].catchCount++;
+          this.isCatch = true;
+        } else {
+          taskPromisePendingCallbackData[this.promiseId].thenCount++;
+          this.isCatch = false;
+        }
+      }
+    }
   }
 
   TasksWrapperPlugin.prototype = {
@@ -598,15 +708,30 @@ function buildTaskManager(areConflicting) {
       if (this.isTaskCallback) {
         verify(isInTask, 'TasksWrapperPlugin.epilogue: not in task at exit');
         isInTask = false;
+        if (this.promiseId !== undefined) {
+          // Finishing up a promise callback. If a "then" calls back, then
+          // all "catch" callbacks will not call back and vice versa.
+          // Subtract the callbacks that will not call back.
+          if (this.isCatch) {
+            taskPendingCallbackCount -=
+                taskPromisePendingCallbackData[this.promiseId].thenCount;
+            taskPromisePendingCallbackData[this.promiseId].thenCount = 0;
+          } else {
+            taskPendingCallbackCount -=
+                taskPromisePendingCallbackData[this.promiseId].catchCount;
+            taskPromisePendingCallbackData[this.promiseId].catchCount = 0;
+          }
+        }
         if (--taskPendingCallbackCount == 0)
           finish();
       }
     }
   };
 
-  wrapper.registerWrapperPluginFactory(function() {
-    return new TasksWrapperPlugin();
-  });
+  wrapper.registerWrapperPluginFactory(
+      function(opt_promiseMetadata) {
+        return new TasksWrapperPlugin(opt_promiseMetadata);
+      });
 
   return {
     add: add