Diagnosing uncaught exceptions when they happen. This helps determining when the task's event chain was broken, as opposed to judging only based on the last step's name.

Also, this takes us one step closer to being able to send crash stacks, and getting rid of explicitly setting step names.

BUG=164227
TEST=No testing needed.

Review URL: https://chromiumcodereview.appspot.com/14715009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@198554 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 81 insertions, 13 deletions

diff --git a/chrome/browser/resources/google_now/background.js b/chrome/browser/resources/google_now/background.js
index f22a54a..1f0c918 100644
--- a/chrome/browser/resources/google_now/background.js
+++ b/chrome/browser/resources/google_now/background.js
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-'use strict';
+// 'use strict'; TODO(vadimt): Uncomment once crbug.com/237617 is fixed.
 
 /**
  * @fileoverview The event page for Google Now for Chrome implementation.
@@ -108,6 +108,19 @@ function areTasksConflicting(newTaskName, scheduledTaskName) {
 
 var tasks = buildTaskManager(areTasksConflicting);
 
+// Add error processing to API calls.
+tasks.instrumentApiFunction(chrome.location.onLocationUpdate, 'addListener', 0);
+tasks.instrumentApiFunction(chrome.notifications, 'create', 2);
+tasks.instrumentApiFunction(chrome.notifications, 'update', 2);
+tasks.instrumentApiFunction(
+    chrome.notifications.onButtonClicked, 'addListener', 0);
+tasks.instrumentApiFunction(chrome.notifications.onClicked, 'addListener', 0);
+tasks.instrumentApiFunction(chrome.notifications.onClosed, 'addListener', 0);
+tasks.instrumentApiFunction(chrome.runtime.onInstalled, 'addListener', 0);
+tasks.instrumentApiFunction(chrome.runtime.onStartup, 'addListener', 0);
+tasks.instrumentApiFunction(chrome.tabs, 'create', 1);
+tasks.instrumentApiFunction(storage, 'get', 1);
+
 /**
  * Diagnostic event identifier.
  * @enum {number}
@@ -314,7 +327,7 @@ function requestNotificationCards(position, callback) {
   var request = new XMLHttpRequest();
 
   request.responseType = 'text';
-  request.onloadend = function(event) {
+  request.onloadend = tasks.wrapCallback(function(event) {
     console.log('requestNotificationCards-onloadend ' + request.status);
     if (request.status == HTTP_OK) {
       recordEvent(DiagnosticEvent.REQUEST_FOR_CARDS_SUCCESS);
@@ -322,7 +335,7 @@ function requestNotificationCards(position, callback) {
     } else {
       callback();
     }
-  };
+  });
 
   request.open(
       'POST',
@@ -400,13 +413,13 @@ function requestCardDismissal(
                           '&dismissalAge=' + (Date.now() - dismissalTimeMs);
   var request = new XMLHttpRequest();
   request.responseType = 'text';
-  request.onloadend = function(event) {
+  request.onloadend = tasks.wrapCallback(function(event) {
     console.log('requestDismissingCard-onloadend ' + request.status);
     if (request.status == HTTP_OK)
       recordEvent(DiagnosticEvent.DISMISS_REQUEST_SUCCESS);
 
     callbackBoolean(request.status == HTTP_OK);
-  };
+  });
 
   request.open(
       'POST',
diff --git a/chrome/browser/resources/google_now/utility.js b/chrome/browser/resources/google_now/utility.js
index 16efc52..d0153bc 100644
--- a/chrome/browser/resources/google_now/utility.js
+++ b/chrome/browser/resources/google_now/utility.js
@@ -2,7 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-'use strict';
+// 'use strict'; TODO(vadimt): Uncomment once crbug.com/237617 is fixed.
+
+// TODO(vadimt): Remove alerts.
 
 /**
  * @fileoverview Utility objects and functions for Google Now extension.
@@ -15,12 +17,8 @@
  *     false.
  */
 function verify(condition, message) {
-  // TODO(vadimt): Remove alert.
-  if (!condition) {
-    var errorText = 'ASSERT: ' + message;
-    alert(errorText);
-    throw new Error(errorText);
-  }
+  if (!condition)
+    throw new Error('ASSERT: ' + message);
 }
 
 /**
@@ -138,6 +136,60 @@ function buildTaskManager(areConflicting) {
     stepName = step;
   }
 
+  // Limiting 1 alert per background page load.
+  var alertShown = false;
+
+  /**
+   * Adds error processing to an API callback.
+   * @param {Function} callback Callback to instrument.
+   * @return {Function} Instrumented callback.
+   */
+  function wrapCallback(callback) {
+    return function() {
+      // This is the wrapper for the callback.
+      try {
+        return callback.apply(null, arguments);
+      } catch (error) {
+        var message = 'Uncaught exception:\n' + error.stack;
+        console.error(message);
+        if (!alertShown) {
+          alertShown = true;
+          alert(message);
+        }
+      }
+    };
+  }
+
+  /**
+   * 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.
+   * @param {number} callbackParameter Index of the callback parameter to this
+   *     API function.
+   */
+  function instrumentApiFunction(namespace, functionName, callbackParameter) {
+    var originalFunction = namespace[functionName];
+
+    if (!originalFunction)
+      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);
+      return originalFunction.apply(namespace, arguments);
+    };
+  }
+
+  instrumentApiFunction(chrome.alarms.onAlarm, 'addListener', 0);
+  instrumentApiFunction(chrome.runtime.onSuspend, 'addListener', 0);
+
   chrome.alarms.onAlarm.addListener(function(alarm) {
     if (alarm.name == CANNOT_UNLOAD_ALARM_NAME) {
       // Error if the event page wasn't unloaded after a reasonable timeout
@@ -163,6 +215,9 @@ function buildTaskManager(areConflicting) {
 
   return {
     add: add,
-    debugSetStepName: debugSetStepName
+    // TODO(vadimt): Replace with instrumenting callbacks.
+    debugSetStepName: debugSetStepName,
+    instrumentApiFunction: instrumentApiFunction,
+    wrapCallback: wrapCallback
   };
 }