Protect all uses of JSON.parse against exceptions.

remoting/webapp/remoting.js

Index: remoting/webapp/remoting.js
diff --git a/remoting/webapp/remoting.js b/remoting/webapp/remoting.js
index e7836c85b7ccaee6ec1fd411231e9c04da3d94a7..a51407e951ac8343a950141aaa1def9188e089da 100644
--- a/remoting/webapp/remoting.js
+++ b/remoting/webapp/remoting.js
@@ -127,9 +127,13 @@ remoting.logExtensionInfoAsync_ = function() {
   xhr.onload = function(e) {
     var manifest =
         /** @type {{name: string, version: string, default_locale: string}} */
-        JSON.parse(xhr.responseText);
-    var name = chrome.i18n.getMessage('PRODUCT_NAME');
-    console.log(name + ' version: ' + manifest.version);
+        jsonParseSafe(xhr.responseText);

[Wez, 2012/04/28 00:00:42]

I'd suggest leaving this one JSON.parse(), so that the web-app will fail to work
if we manage to break it in such a fundamental way.

[Jamie, 2012/04/28 00:15:41]

TBH, I'm pretty sure it won't even load if the manifest is invalid. I changed it
just so that there is only one place where we call JSON.parse. If it does manage
to load, then we just lose the version logging, which is not the end of the
world.

+    if (manifest) {
+      var name = chrome.i18n.getMessage('PRODUCT_NAME');
+      console.log(name + ' version: ' + manifest.version);
+    } else {
+      console.error('Failed to get product version. Corrupt manifest?');
+    }
   }
   xhr.send(null);
 };
@@ -276,3 +280,15 @@ function getUrlParameters_() {
   }
   return result;
 }
+
+/**
+ * @param {string} jsonString A JSON-encoded string.
+ * @return {*} The decoded object, or undefined if the string cannot be parsed.
+ */
+function jsonParseSafe(jsonString) {

[Wez, 2012/04/28 00:00:42]

nit: parseJsonOrNull?

[Jamie, 2012/04/28 00:15:41]

That was what I called it originally, but it doesn't return null (see below) and
jsonParseOrUndefined just didn't sound right.

[Wez, 2012/04/28 00:32:03]

In that case safeParseJson(), then? :)

+  try {
+    return JSON.parse(jsonString);
+  } catch (err) {
+    return undefined;

[Wez, 2012/04/28 00:00:42]

Is the aim in returning undefined that if the caller uses the value without
testing then we'll see errors about using undefined values?

[Jamie, 2012/04/28 00:15:41]

No. JSON.parse("null") legitimately returns null and I wanted to make sure we
can distinguish that case if we ever want to. AFAICT, it can never return
undefined (JSON.stringify(undefined) = undefined, not "undefined"). Trying to
access a property or using the 'in' operator on either null or undefined is
illegal, so there's no difference in that respect.

+  }
+}