Gate console.log output in Google Now

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 6674edf7754803d8a231179b383d8360daaed637..f1f7884e311476f4d2d1531d78eef47e326ff2e5 100644
--- a/chrome/browser/resources/google_now/utility.js
+++ b/chrome/browser/resources/google_now/utility.js
@@ -29,13 +29,21 @@
  */
 var NOTIFICATION_CARDS_URL = 'https://www.googleapis.com/chromenow/v1';
 
-var DEBUG_MODE = localStorage['debug_mode'];
+/**
+ * Returns true if debug mode is enabled.
+ * localStorage returns items as strings, which means if we store a boolean,
+ * it returns a string. Use this function to compare against true.
+ * @return {boolean} Debug mode is enabled.

[skare_, 2014/01/28 21:25:52]

tiny nit: Debug mode is enabled -> Whether debug mode... or True if debug
mode...

[robliao, 2014/01/28 21:56:02]

Done.

+ */
+function isInDebugMode() {
+  return !!localStorage.debug_mode && (localStorage.debug_mode === 'true');

[vadimt, 2014/01/28 21:07:12]

Why do we need the first check (!!localStorage.debug_mode)?
Also: every use of localStorage causes a blocking roundtrip to the browser. It's
better to use it only once, not from every log call.

[robliao, 2014/01/28 21:56:02]

I guess we don't need to check it based off of this structure.

The goal of this function is to have a one-stop-shop for getting the debug mode.
It turns out, localStorage is cached in the renderer, so lookups happen in the
same process and appear to be quick!

Check out
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/d...

and
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/d...
On 2014/01/28 21:07:12, vadimt wrote:

+}
 
 /**
  * Initializes for debug or release modes of operation.
  */
 function initializeDebug() {
-  if (DEBUG_MODE) {
+  if (isInDebugMode()) {
     NOTIFICATION_CARDS_URL =
         localStorage['server_url'] || NOTIFICATION_CARDS_URL;
   }
@@ -44,6 +52,18 @@ function initializeDebug() {
 initializeDebug();
 
 /**
+ * Conditionally allow console.log output based off of the debug mode.
+ */
+console.log = function() {
+  var originalConsoleLog = console.log;

[skare_, 2014/01/28 21:25:52]

any chance this and the other replacement will be run in the same file? should
behave ok, just checking

[robliao, 2014/01/28 21:56:02]

There aren't any includes in JS world, just Chrome loading all of the JS files
together. As long as utilty.js is specified once, we should be okay here.
On 2014/01/28 21:25:52, Travis Skare wrote:

+  return function() {
+    if (isInDebugMode()) {
+      originalConsoleLog.apply(console, arguments);
+    }
+  };
+}();
+
+/**
  * Location Card Storage.
  */
 if (localStorage['locationCardsShown'] === undefined)
@@ -171,7 +191,7 @@ function reportError(error) {
     chrome.metricsPrivate.getIsCrashReportingEnabled(function(isEnabled) {
       if (isEnabled)
         sendErrorReport(error);
-      if (DEBUG_MODE)
+      if (isInDebugMode())
         alert(message);
     });
   }