Remove chrome.* Function Instrumentation Inlining and Use a Separate Instrumented Object

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 0ab7c4b7c384b80ee3d9cea4387aef77c0df4cba..19e16f7400db73839d88d3df7da131cd8c743a94 100644
--- a/chrome/browser/resources/google_now/utility.js
+++ b/chrome/browser/resources/google_now/utility.js
@@ -55,6 +55,9 @@ function buildServerRequest(handlerName, contentType) {
   return request;
 }
 
+// Partial mirror of chrome.* for all instrumented functions.
+var instrumented = {};
+
 /**
  * Builds the object to manage tasks (mutually exclusive chains of events).
  * @param {function(string, string): boolean} areConflicting Function that
@@ -296,37 +299,79 @@ function buildTaskManager(areConflicting) {
   }
 
   /**
-   * Instruments an API function to add error processing to its user
-   * code-provided callback.
-   * @param {Object} namespace Namespace of the API function.
-   * @param {string} functionName Name of the API function.
+   * Adds an instrumented function to instrumented.* that forwards
+   * to the equivalent chrome.* function.
+   * @param {object} instrumentedContainer Instrumented container object
+   *     for the function in the 'chrome.' portion.
+   * @param {array} functionIdentifierParts Path to the chrome.* function.
+   * @param {string} functionName Name of the chrome API function.
    * @param {number} callbackParameter Index of the callback parameter to this
    *     API function.
    */
-  function instrumentApiFunction(namespace, functionName, callbackParameter) {
-    var originalFunction = namespace[functionName];
-
-    if (!originalFunction)
-      debugAlert('Cannot instrument ' + functionName);
-
-    namespace[functionName] = function() {
+  function addInstrumentedFunction(
+      instrumentedContainer,
+      functionIdentifierParts,
+      functionName,
+      callbackParameter) {
+    instrumentedContainer[functionName] = function() {

[vadimt, 2013/08/08 18:19:25]

Alternatively, you could simply return the new function from the method and add
it to the container outside. This way, you'd not need to pass
instrumentedContainer parameter.

But up to you.

[robliao, 2013/08/08 21:35:35]

Sounds better.
On 2013/08/08 18:19:25, vadimt wrote:

       // This is the wrapper for the API function. Pass the wrapped callback to
       // the original function.
       var callback = arguments[callbackParameter];
       if (typeof callback != 'function') {
-        debugAlert('Argument ' + callbackParameter + ' of ' + functionName +
-              ' is not a function');
+        debugAlert('Argument ' + callbackParameter + ' of ' +
+                   functionIdentifierParts.join('.') + '.' + functionName +
+                   ' is not a function');
       }
       arguments[callbackParameter] = wrapCallback(
           callback, functionName == 'addListener');
-      return originalFunction.apply(namespace, arguments);
+
+      var chromeContainer = chrome;
+      functionIdentifierParts.map(function(fragment) {
+        chromeContainer = chromeContainer[fragment];
+      });
+      return chromeContainer[functionName].
+          apply(chromeContainer, arguments);
     };
   }
 
-  instrumentApiFunction(chrome.alarms, 'get', 1);
-  instrumentApiFunction(chrome.alarms.onAlarm, 'addListener', 0);
-  instrumentApiFunction(chrome.identity, 'getAuthToken', 1);
-  instrumentApiFunction(chrome.runtime.onSuspend, 'addListener', 0);
+  /**
+   * Instruments an API function to add error processing to its user
+   * code-provided callback.
+   * @param {string} functionIdentifier Full identifier of the function without
+   *     the 'chrome.' portion.
+   * @param {number} callbackParameter Index of the callback parameter to this
+   *     API function.
+   */
+  function instrumentChromeApiFunction(functionIdentifier, callbackParameter) {
+    var functionIdentifierParts = functionIdentifier.split('.');
+    var functionName = functionIdentifierParts.pop();
+    var chromeContainer = chrome;
+    var instrumentedContainer = instrumented;
+    functionIdentifierParts.map(function(fragment) {
+      chromeContainer = chromeContainer[fragment];
+      if (!(fragment in instrumentedContainer))
+        instrumentedContainer[fragment] = {};
+
+      instrumentedContainer = instrumentedContainer[fragment];

[vadimt, 2013/08/08 18:19:25]

Or, if you prefer:
instrumentedContainer = instrumentedContainer[fragment] =
instrumentedContainer[fragment] || {};

[robliao, 2013/08/08 21:35:35]

Keeping current for explicitness.
On 2013/08/08 18:19:25, vadimt wrote:

+    });
+
+    var targetFunction = chromeContainer[functionName];
+
+    if (!targetFunction)
+      debugAlert('Cannot instrument ' + functionName);
+
+    addInstrumentedFunction(
+        instrumentedContainer,
+        functionIdentifierParts,
+        functionName,
+        callbackParameter);
+  }
+
+  instrumentChromeApiFunction('alarms.get', 1);
+  instrumentChromeApiFunction('alarms.onAlarm.addListener', 0);
+  instrumentChromeApiFunction('identity.getAuthToken', 1);
+  instrumentChromeApiFunction('identity.removeCachedAuthToken', 1);
+  instrumentChromeApiFunction('runtime.onSuspend.addListener', 0);
 
   chrome.runtime.onSuspend.addListener(function() {
     var stringifiedPendingCallbacks = JSON.stringify(pendingCallbacks);
@@ -340,13 +385,11 @@ function buildTaskManager(areConflicting) {
   return {
     add: add,
     debugSetStepName: function() {},  // TODO(vadimt): remove
-    instrumentApiFunction: instrumentApiFunction,
+    instrumentChromeApiFunction: instrumentChromeApiFunction,
     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.
@@ -380,7 +423,7 @@ function buildAttemptManager(
    *     true if the attempt manager has started, false otherwise.
    */
   function isRunning(callback) {
-    chrome.alarms.get(alarmName, function(alarmInfo) {
+    instrumented.alarms.get(alarmName, function(alarmInfo) {
       callback(!!alarmInfo);
     });
   }
@@ -401,7 +444,7 @@ function buildAttemptManager(
 
     var items = {};
     items[currentDelayStorageKey] = newRetryDelaySeconds;
-    storage.set(items);
+    chrome.storage.local.set(items);
   }
 
   /**
@@ -413,7 +456,7 @@ function buildAttemptManager(
   function start(opt_firstDelaySeconds) {
     if (opt_firstDelaySeconds) {
       createAlarm(opt_firstDelaySeconds);
-      storage.remove(currentDelayStorageKey);
+      chrome.storage.local.remove(currentDelayStorageKey);
     } else {
       scheduleNextAttempt();
     }
@@ -424,7 +467,7 @@ function buildAttemptManager(
    */
   function stop() {
     chrome.alarms.clear(alarmName);
-    storage.remove(currentDelayStorageKey);
+    chrome.storage.local.remove(currentDelayStorageKey);
   }
 
   /**
@@ -433,14 +476,14 @@ function buildAttemptManager(
    *     the planning is done.
    */
   function planForNext(callback) {
-    storage.get(currentDelayStorageKey, function(items) {
+    instrumented.storage.local.get(currentDelayStorageKey, function(items) {
       console.log('planForNext-get-storage ' + JSON.stringify(items));
       scheduleNextAttempt(items[currentDelayStorageKey]);
       callback();
     });
   }
 
-  chrome.alarms.onAlarm.addListener(function(alarm) {
+  instrumented.alarms.onAlarm.addListener(function(alarm) {
     if (alarm.name == alarmName)
       isRunning(function(running) {
         if (running)
@@ -474,7 +517,7 @@ function buildAuthenticationManager() {
    *     If the user is signed in, the string contains the token.
    */
   function isSignedIn(callback) {
-    chrome.identity.getAuthToken({interactive: false}, function(token) {
+    instrumented.identity.getAuthToken({interactive: false}, function(token) {
       token = chrome.runtime.lastError ? undefined : token;
       callback(token);
       checkAndNotifyListeners(!!token);
@@ -487,7 +530,7 @@ function buildAuthenticationManager() {
    * @param {function} onSuccess Called on completion.
    */
   function removeToken(token, onSuccess) {
-    chrome.identity.removeCachedAuthToken({token: token}, function() {
+    instrumented.identity.removeCachedAuthToken({token: token}, function() {
       // Removing the token from the cache will change the sign in state.
       // Repoll now to check the state and notify listeners.
       // This also lets Chrome now about a possible problem with the token.
@@ -522,7 +565,7 @@ function buildAuthenticationManager() {
     lastReturnedSignedInState = currentSignedInState;
   }
 
-  chrome.alarms.onAlarm.addListener(function(alarm) {
+  instrumented.alarms.onAlarm.addListener(function(alarm) {
     if (alarm.name == alarmName)
       isSignedIn(function() {});
   });