Simplifying using tasks

chrome/browser/resources/google_now/utility.js

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 'use strict';
 
 // TODO(vadimt): Remove alerts.
 
 /**
  * @fileoverview Utility objects and functions for Google Now extension.
@@ skipping 42 matching lines @@
  */
 function buildTaskManager(areConflicting) {
   /**
    * Queue of scheduled tasks. The first element, if present, corresponds to the
    * currently running task.
    * @type {Array.<Object.<string, function(function())>>}
    */
   var queue = [];
 
   /**
-   * Name of the current step of the currently running task if present,
-   * otherwise, null. For diagnostics only.
-   * It's set when the task is started and before each asynchronous operation.
+   * Count of unfinished callbacks of the current task.
+   * @type {number}
    */
-  var stepName = null;
+  var taskPendingCallbackCount = 0;
+
+  /**
+   * Not yet called required callbacks. Includes both task and non-task
+   * callbacks. This is a map from unique callback id to the stack at the moment
+   * when the callback was wrapped. This stack identifies the callback.
+   * Used only for diagnostics.
+   * @type {Object.<number, string>}
+   */
+  var pendingCallbacks = {};
+
+  /**
+   * True if currently executed code is a part of a task.
+   * @type {boolean}
+   */
+  var isInTask = false;
 
   /**
    * Starts the first queued task.
    */
   function startFirst() {
     verify(queue.length >= 1, 'startFirst: queue is empty');
+    verify(!isInTask, 'startFirst: already in task');
+    isInTask = true;
 
     // Start the oldest queued task, but don't remove it from the queue.
     verify(
-        stepName == null,
-        'tasks.startFirst: stepName is not null: ' + stepName +
-        ', queue = ' + JSON.stringify(queue));
+        taskPendingCallbackCount == 0,
+        'tasks.startFirst: still have pending task callbacks: ' +
+        taskPendingCallbackCount +
+        ', queue = ' + JSON.stringify(queue) +
+        ', pendingCallbacks = ' + JSON.stringify(pendingCallbacks));
     var entry = queue[0];
-    stepName = entry.name + '-initial';
     console.log('Starting task ' + entry.name);
-    entry.task(finish);
+
+    entry.task(function() {}); // TODO(vadimt): Don't pass parameter.
+
+    verify(isInTask, 'startFirst: not in task at exit');
+    isInTask = false;
+    if (taskPendingCallbackCount == 0)

[skare_, 2013/07/22 23:09:54]

does this need to be kept separate from pendingCallbacks.length? The latter
should be fast. As a side effect that would make pendingCallbacks non-debug info
of course.

[vadimt, 2013/07/22 23:59:59]

pendingCallbacks also includes non-task callbacks. We may want to call finish()
even when it's not empty, given that all task callbacks are complete.

+      finish();
   }
 
   /**
    * Checks if a new task can be added to the task queue.
    * @param {string} taskName Name of the new task.
    * @return {boolean} Whether the new task can be added.
    */
   function canQueue(taskName) {
     for (var i = 0; i < queue.length; ++i) {
       if (areConflicting(taskName, queue[i].name)) {
@@ skipping 24 matching lines @@
     if (queue.length == 1) {
       startFirst();
     }
   }
 
   /**
    * Completes the current task and starts the next queued task if available.
    */
   function finish() {
     verify(queue.length >= 1,
-           'tasks.finish: The task queue is empty; step = ' + stepName);
+           'tasks.finish: The task queue is empty');
     console.log('Finishing task ' + queue[0].name);
     queue.shift();
-    stepName = null;
 
     if (queue.length >= 1)
       startFirst();
   }
 
-  /**
-   * Associates a name with the current step of the task. Used for diagnostics
-   * only. A task is a chain of asynchronous events; debugSetStepName should be
-   * called before starting any asynchronous operation.
-   * @param {string} step Name of new step.
-   */
-  function debugSetStepName(step) {
-    stepName = step;
-  }
-
   // Limiting 1 error report per background page load.
   var errorReported = false;
 
   /**
    * Sends an error report to the server.
    * @param {Error} error Error to report.
    */
   function sendErrorReport(error) {
     var filteredStack = error.stack.replace(/.*\n/, '\n');
     var file;
@@ skipping 21 matching lines @@
         '&line=' + encodeURIComponent(line) +
         '&trace=' + encodeURIComponent(filteredStack);
     var request = buildServerRequest('jserror');
     request.onloadend = function(event) {
       console.log('sendErrorReport status: ' + request.status);
     };
     request.send(requestParameters);
   }
 
   /**
+   * Unique ID of the next callback.
+   * @type {number}
+   */
+  var nextCallbackId = 0;
+
+  /**
    * Adds error processing to an API callback.
    * @param {Function} callback Callback to instrument.
+   * @param {boolean=} opt_dontRequire True if the callback is not required to
+   *     be invoked.
    * @return {Function} Instrumented callback.
    */
-  function wrapCallback(callback) {
+  function wrapCallback(callback, opt_dontRequire) {
+    verify(!(opt_dontRequire && isInTask), 'Unrequired callback in a task.');
+    var callbackId = nextCallbackId++;
+    var isTaskCallback = isInTask;
+    if (isInTask)
+      ++taskPendingCallbackCount;
+    if (!opt_dontRequire)
+      pendingCallbacks[callbackId] = new Error().stack;
+
     return function() {
       // This is the wrapper for the callback.
       try {
-        return callback.apply(null, arguments);
+        if (isTaskCallback) {
+          verify(!isInTask, 'wrapCallback: already in task');
+          isInTask = true;
+        }
+        if (!opt_dontRequire)
+          delete pendingCallbacks[callbackId];
+
+        // Call the original callback.
+        callback.apply(null, arguments);
+
+        if (isTaskCallback) {
+          verify(isInTask, 'wrapCallback: not in task at exit');
+          isInTask = false;
+          if (--taskPendingCallbackCount == 0)
+            finish();
+        }
       } catch (error) {
         var message = 'Uncaught exception:\n' + error.stack;
         console.error(message);
         if (!errorReported) {
           errorReported = true;
           chrome.metricsPrivate.getIsCrashReportingEnabled(function(isEnabled) {
             if (isEnabled)
               sendErrorReport(error);
           });
           alert(message);
@@ skipping 17 matching lines @@
       alert('Cannot instrument ' + functionName);
 
     namespace[functionName] = function() {
       // This is the wrapper for the API function. Pass the wrapped callback to
       // the original function.
       var callback = arguments[callbackParameter];
       if (typeof callback != 'function') {
         alert('Argument ' + callbackParameter + ' of ' + functionName +
               ' is not a function');
       }
-      arguments[callbackParameter] = wrapCallback(callback);
+      arguments[callbackParameter] = wrapCallback(
+          callback, functionName == 'addListener');
       return originalFunction.apply(namespace, arguments);
     };
   }
 
   instrumentApiFunction(chrome.alarms.onAlarm, 'addListener', 0);
   instrumentApiFunction(chrome.runtime.onSuspend, 'addListener', 0);
 
   chrome.runtime.onSuspend.addListener(function() {
+    var stringifiedPendingCallbacks = JSON.stringify(pendingCallbacks);
     verify(
-        queue.length == 0,
-        'Incomplete task when unloading event page, queue = ' +
-        JSON.stringify(queue) + ', step = ' + stepName);
-    verify(
-        stepName == null,
-        'Step name not null when unloading event page, queue = ' +
-        JSON.stringify(queue) + ', step = ' + stepName);
+        queue.length == 0 && stringifiedPendingCallbacks == '{}',
+        'Incomplete task or pending callbacks when unloading event page,' +
+        ' queue = ' + JSON.stringify(queue) +
+        ', pendingCallbacks = ' + stringifiedPendingCallbacks);
   });
 
   return {
     add: add,
-    // TODO(vadimt): Replace with instrumenting callbacks.
-    debugSetStepName: debugSetStepName,
+    debugSetStepName: function() {}, // TODO(vadimt): remove

[skare_, 2013/07/22 23:09:54]

nit: 2 spaces between code and comments.
Could this be removed now? or makes more sense with a change that just removes
the function?

[vadimt, 2013/07/22 23:59:59]

Removing this now would break code or would require removing the call from all
places, which would create lost of conflicts with pending CLs.
I'll choose a quiet moment and remove this method later.

     instrumentApiFunction: instrumentApiFunction,
     wrapCallback: wrapCallback
   };
 }
 
 var storage = chrome.storage.local;
 
 /**
  * Builds an object to manage retrying activities with exponential backoff.
  * @param {string} name Name of this attempt manager.
@@ skipping 81 matching lines @@
     if (alarm.name == alarmName)
       attempt();
   });
 
   return {
     start: start,
     planForNext: planForNext,
     stop: stop
   };
 }