[Extensions] Don't store logs in the error console

The error console shows errors that were reported across all the
different contexts an extension runs in (e.g., content scripts,
background pages, etc). It should ignore logs, though, so as to
avoid being overly noisy. Keep reporting warnings for now.

Add a unittest for the same.

Bug: 837401
Change-Id: I4a0d89f85f2cfd31f2cb3a29051b8598bd78fafc
Reviewed-on: https://chromium-review.googlesource.com/1043156Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556232}

chrome/browser/extensions/api/developer_private/extension_info_generator_unittest.cc

@@ -255,7 +255,7 @@ TEST_F(ExtensionInfoGeneratorUnitTest, BasicInfoTest) {
       base::UTF8ToUTF16("message"),
       StackTrace(1, StackFrame(1, 1, base::UTF8ToUTF16("source"),
                                base::UTF8ToUTF16("function"))),
-      kContextUrl, logging::LOG_VERBOSE, 1, 1));
+      kContextUrl, logging::LOG_WARNING, 1, 1));
 
   // It's not feasible to validate every field here, because that would be
   // a duplication of the logic in the method itself. Instead, test a handful
@@ -303,7 +303,7 @@ TEST_F(ExtensionInfoGeneratorUnitTest, BasicInfoTest) {
   ASSERT_EQ(1u, info->manifest_errors.size());
   const api::developer_private::RuntimeError& runtime_error_verbose =
       info->runtime_errors[1];
-  EXPECT_EQ(api::developer_private::ERROR_LEVEL_LOG,
+  EXPECT_EQ(api::developer_private::ERROR_LEVEL_WARN,
             runtime_error_verbose.severity);
   const api::developer_private::ManifestError& manifest_error =
       info->manifest_errors[0];

chrome/browser/extensions/error_console/error_console.cc

@@ -140,6 +140,9 @@ void ErrorConsole::ReportError(std::unique_ptr<ExtensionError> error) {
   if (!enabled_ || !crx_file::id_util::IdIsValid(error->extension_id()))
     return;
 
+  DCHECK_GE(error->level(), extension_misc::kMinimumSeverityToReportError)
+      << "Errors less than severity warning should not be reported.";
+
   int mask = GetMaskForExtension(error->extension_id());
   if (!(mask & (1 << error->type())))
     return;

chrome/browser/extensions/error_console/error_console_browsertest.cc

@@ -387,27 +387,30 @@ IN_PROC_BROWSER_TEST_F(ErrorConsoleBrowserTest,
   const ErrorList& errors =
       error_console()->GetErrorsForExtension(extension->id());
 
+  // The extension logs a message with console.log(), then another with
+  // console.warn(), and then triggers a TypeError.
+  // There should be exactly two errors (the warning and the TypeError). The
+  // error console ignores logs - this would tend to be too noisy, and doesn't
+  // jive with the big `ERRORS` button in the UI.
+  // See https://crbug.com/837401.
+  ASSERT_EQ(2u, errors.size());
+
   // The first error should be a console log.
   CheckRuntimeError(errors[0].get(), extension->id(),
                     script_url,  // The source should be the content script url.
                     false,       // Not from incognito.
-                    "Hello, World!",  // The error message is the log.
-                    logging::LOG_INFO,
+                    "warned message",  // The error message is the log.
+                    logging::LOG_WARNING,
                     GetTestURL(),  // Content scripts run in the web page.
                     2u);
 
   const StackTrace& stack_trace1 = GetStackTraceFromError(errors[0].get());
-  CheckStackFrame(stack_trace1[0],
-                  script_url,
-                  "logHelloWorld",  // function name
-                  6u,  // line number
+  CheckStackFrame(stack_trace1[0], script_url,
+                  "warnMessage",  // function name
+                  10u,            // line number
                   11u /* column number */);
 
-  CheckStackFrame(stack_trace1[1],
-                  script_url,
-                  kAnonymousFunction,
-                  9u,
-                  1u);
+  CheckStackFrame(stack_trace1[1], script_url, kAnonymousFunction, 14u, 1u);
 
   // The second error should be a runtime error.
   CheckRuntimeError(errors[1].get(), extension->id(), script_url,
@@ -418,11 +421,7 @@ IN_PROC_BROWSER_TEST_F(ErrorConsoleBrowserTest,
                     GetTestURL(), 1u);
 
   const StackTrace& stack_trace2 = GetStackTraceFromError(errors[1].get());
-  CheckStackFrame(stack_trace2[0],
-                  script_url,
-                  kAnonymousFunction,
-                  12u,
-                  1u);
+  CheckStackFrame(stack_trace2[0], script_url, kAnonymousFunction, 17u, 1u);
 }
 
 // Catch an error from a BrowserAction; this is more complex than a content

chrome/browser/extensions/error_console/error_console_unittest.cc

@@ -11,6 +11,7 @@
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/utf_string_conversions.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/crx_file/id_util.h"

chrome/test/data/extensions/error_console/content_script_log_and_runtime_error/content_script.js

@@ -2,11 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-function logHelloWorld() {
-  console.log("Hello, World!");
+function logMessage() {
+  console.log('logged message');
 }
 
-logHelloWorld();
+function warnMessage() {
+  console.warn('warned message');
+}
+
+logMessage();
+warnMessage();
 
 var bar = undefined;
 bar.foo = 'baz';

extensions/browser/extension_error_test_util.cc

@@ -39,7 +39,7 @@ std::unique_ptr<ExtensionError> CreateNewRuntimeError(
       new RuntimeError(extension_id, from_incognito, source,
                        base::UTF8ToUTF16(message), stack_trace,
                        GURL::EmptyGURL(),  // no context url
-                       logging::LOG_INFO,
+                       logging::LOG_ERROR,
                        0,    // Render frame id
                        0));  // Render process id
 }

extensions/common/constants.cc

@@ -121,4 +121,6 @@ const char kPolicyBlockedScripting[] =
 
 const int kContentVerificationDefaultBlockSize = 4096;
 
+const logging::LogSeverity kMinimumSeverityToReportError = logging::LOG_WARNING;
+
 }  // namespace extension_misc

extensions/common/constants.h

@@ -6,6 +6,7 @@
 #define EXTENSIONS_COMMON_CONSTANTS_H_
 
 #include "base/files/file_path.h"
+#include "base/logging.h"
 #include "ui/base/layout.h"
 
 namespace extensions {
@@ -240,6 +241,9 @@ extern const char kPolicyBlockedScripting[];
 // The default block size for hashing used in content verification.
 extern const int kContentVerificationDefaultBlockSize;
 
+// The minimum severity of a log or error in order to report it to the browser.
+extern const logging::LogSeverity kMinimumSeverityToReportError;
+
 }  // namespace extension_misc
 
 #endif  // EXTENSIONS_COMMON_CONSTANTS_H_

extensions/renderer/extensions_render_frame_observer.cc

@@ -10,6 +10,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "content/public/renderer/render_frame.h"
 #include "content/public/renderer/render_view.h"
+#include "extensions/common/constants.h"
 #include "extensions/common/extension_messages.h"
 #include "extensions/common/stack_frame.h"
 #include "third_party/blink/public/web/web_local_frame.h"
@@ -111,6 +112,12 @@ void ExtensionsRenderFrameObserver::DetailedConsoleMessageAdded(
     const base::string16& stack_trace_string,
     uint32_t line_number,
     int32_t severity_level) {
+  if (severity_level <
+      static_cast<int32_t>(extension_misc::kMinimumSeverityToReportError)) {
+    // We don't report certain low-severity errors.
+    return;
+  }
+
   base::string16 trimmed_message = message;
   StackTrace stack_trace = GetStackTraceFromMessage(
       &trimmed_message,