Mark create/update/clear callbacks of notification API as optional

extensions/renderer/resources/last_error.js

Index: extensions/renderer/resources/last_error.js
diff --git a/extensions/renderer/resources/last_error.js b/extensions/renderer/resources/last_error.js
index 8d53371827acd01eeaa12840bd71497b5bb90120..7c93721880030ef914083ffe9a7053f7a839cc43 100644
--- a/extensions/renderer/resources/last_error.js
+++ b/extensions/renderer/resources/last_error.js
@@ -113,12 +113,28 @@ function run(name, message, stack, callback, args) {
   try {
     $Function.apply(callback, undefined, args);
   } finally {
+    if (!hasAccessed(targetChrome))

[not at google - send to devlin, 2015/01/20 17:41:35]

This doesn't look right, it'll always print an error no matter whether the
function succeeded or failed.

[robwu, 2015/01/20 22:34:01]

This is correct. Function run sets chrome.runtime.lastError, which should be
printed to the console if the user did not check its value.

[not at google - send to devlin, 2015/01/21 22:21:19]

What am I missing here? If I do:

chrome.someNamespace.someFunction.get(function() {
  // code
});

and that implementation uses lastError.run (most don't), then if it *succeeds*
with an unchecked error then i'll still print "Unchecked runtime.lastError...".

[robwu, 2015/01/21 23:21:09]

callback is a user-supplied callback. That "// code" ought to read the
chrome.runtime.lastError property. If the code does not, then we have to assume
that the developer does not know about the error and print the error to the
console.

[not at google - send to devlin, 2015/01/21 23:24:32]

but if there isn't an error, it'll still print a non-existent error.

[robwu, 2015/01/21 23:28:18]

A few lines earlier, chrome.runtime.lastError is set (set(name, message, ...)).
That's the whole point of calling this function, not? So, if lastError is not
checked, there is certainly a real error that needs to be reported.

+      reportUncheckedLastError(name, message, stack);
     clear(targetChrome);
   }
 }
 
+/**
+ * Call this method when runtime.lastError was set but not checked.
+ * This notifies the developer of the error via the (error) console.
+ *
+ * @param {string=} name - name of API.
+ * @param {string} message - error description.
+ * @param {string=} stack - Stack trace of the call up to the error.
+ */
+function reportUncheckedLastError(name, message, stack) {

[not at google - send to devlin, 2015/01/20 17:41:35]

Given that this method is under the "lastError" namespace, "reportUnchecked"
would be more accurate. Also, I think it should assert that there actually is a
last error at this point.

[robwu, 2015/01/20 22:34:01]

If we couple this method with the chrome object, then we'd better use the
following instead, to make sure that the reported error is consistent to what's
assigned to lastError:

function reportIfUnchecked(name, targetChrome, stack) {
  var error = hasAccessed(targetChrome) && chrome.runtime.lastError;
  if (!error)
    return;
  console.error("Unchecked runtime.lastError while running " +
      (name || "unknown") + ": " + error.message + (stack ? "\n" + stack : ""));
}

[robwu, 2015/01/21 23:21:09]

Done.

+  console.error("Unchecked runtime.lastError while running " +
+      (name || "unknown") + ": " + message + (stack ? "\n" + stack : ""));
+}
+
 exports.clear = clear;
 exports.hasAccessed = hasAccessed;
 exports.hasError = hasError;
 exports.set = set;
 exports.run = run;
+exports.reportUncheckedLastError = reportUncheckedLastError;