Report Javascript Runtime Errors to the Error Console

TBR=brettw@chromium.org
(moving DEPS file from extensions/common/matcher to extensions/common).

BUG=21734

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

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

9 files changed, 284 insertions, 106 deletions

diff --git a/extensions/browser/extension_error.cc b/extensions/browser/extension_error.cc
index eaef221..9b1e95f 100644
--- a/extensions/browser/extension_error.cc
+++ b/extensions/browser/extension_error.cc
@@ -4,7 +4,6 @@
 
 #include "extensions/browser/extension_error.h"
 
-#include "base/json/json_reader.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
@@ -97,42 +96,21 @@ bool ManifestError::IsEqualImpl(const ExtensionError* rhs) const {
   return true;
 }
 
-RuntimeError::StackFrame::StackFrame() : line_number(-1), column_number(-1) {
-}
-
-RuntimeError::StackFrame::StackFrame(size_t frame_line,
-                                     size_t frame_column,
-                                     const string16& frame_url,
-                                     const string16& frame_function)
-    : line_number(frame_line),
-      column_number(frame_column),
-      url(frame_url),
-      function(frame_function) {
-}
-
-RuntimeError::StackFrame::~StackFrame() {
-}
-
-bool RuntimeError::StackFrame::operator==(
-    const RuntimeError::StackFrame& rhs) const {
-  return line_number == rhs.line_number &&
-         column_number == rhs.column_number &&
-         url == rhs.url &&
-         function == rhs.function;
-}
 RuntimeError::RuntimeError(bool from_incognito,
                            const string16& source,
                            const string16& message,
-                           logging::LogSeverity level,
-                           const string16& details)
+                           const StackTrace& stack_trace,
+                           const GURL& context_url,
+                           logging::LogSeverity level)
     : ExtensionError(ExtensionError::RUNTIME_ERROR,
-                     std::string(),  // We don't know the id yet.
+                     GURL(source).host(),
                      from_incognito,
                      level,
                      source,
-                     message) {
-  ParseDetails(details);
-  DetermineExtensionID();
+                     message),
+      context_url_(context_url),
+      stack_trace_(stack_trace) {
+  CleanUpInit();
 }
 
 RuntimeError::~RuntimeError() {
@@ -141,14 +119,14 @@ RuntimeError::~RuntimeError() {
 std::string RuntimeError::PrintForTest() const {
   std::string result = ExtensionError::PrintForTest() +
          "\n  Type:    RuntimeError"
-         "\n  Context: " + base::UTF16ToUTF8(execution_context_url_) +
+         "\n  Context: " + context_url_.spec() +
          "\n  Stack Trace: ";
   for (StackTrace::const_iterator iter = stack_trace_.begin();
        iter != stack_trace_.end(); ++iter) {
     result += "\n    {"
               "\n      Line:     " + base::IntToString(iter->line_number) +
               "\n      Column:   " + base::IntToString(iter->column_number) +
-              "\n      URL:      " + base::UTF16ToUTF8(iter->url) +
+              "\n      URL:      " + base::UTF16ToUTF8(iter->source) +
               "\n      Function: " + base::UTF16ToUTF8(iter->function) +
               "\n    }";
   }
@@ -163,49 +141,30 @@ bool RuntimeError::IsEqualImpl(const ExtensionError* rhs) const {
   // of displaying an old and inaccurate stack trace.
   return level_ == level_ &&
          source_ == source_ &&
-         execution_context_url_ == error->execution_context_url_ &&
+         context_url_ == error->context_url_ &&
          stack_trace_.size() == error->stack_trace_.size() &&
          (stack_trace_.empty() || stack_trace_[0] == error->stack_trace_[0]);
 }
 
-void RuntimeError::ParseDetails(const string16& details) {
-  scoped_ptr<base::Value> value(
-      base::JSONReader::Read(base::UTF16ToUTF8(details)));
-  const base::DictionaryValue* details_value;
-  const base::ListValue* trace_value = NULL;
-
-  // The |details| value should contain an execution context url and a stack
-  // trace.
-  if (!value.get() ||
-      !value->GetAsDictionary(&details_value) ||
-      !details_value->GetString(kExecutionContextURLKey,
-                                &execution_context_url_) ||
-      !details_value->GetList(kStackTraceKey, &trace_value)) {
-    NOTREACHED();
-    return;
+void RuntimeError::CleanUpInit() {
+  // If the error came from a generated background page, the "context" is empty
+  // because there's no visible URL. We should set context to be the generated
+  // background page in this case.
+  GURL source_url = GURL(source_);
+  if (context_url_.is_empty() &&
+      source_url.path() ==
+          std::string("/") + kGeneratedBackgroundPageFilename) {
+    context_url_ = source_url;
   }
 
-  int line = 0;
-  int column = 0;
-  string16 url;
-
-  for (size_t i = 0; i < trace_value->GetSize(); ++i) {
-    const base::DictionaryValue* frame_value = NULL;
-    CHECK(trace_value->GetDictionary(i, &frame_value));
-
-    frame_value->GetInteger(kLineNumberKey, &line);
-    frame_value->GetInteger(kColumnNumberKey, &column);
-    frame_value->GetString(kURLKey, &url);
-
-    string16 function;
-    frame_value->GetString(kFunctionNameKey, &function);  // This can be empty.
-    stack_trace_.push_back(StackFrame(line, column, url, function));
-  }
-}
-
-void RuntimeError::DetermineExtensionID() {
-  if (!GetExtensionIDFromGURL(GURL(source_), &extension_id_))
-    GetExtensionIDFromGURL(GURL(execution_context_url_), &extension_id_);
+  // In some instances (due to the fact that we're reusing error reporting from
+  // other systems), the source won't match up with the final entry in the stack
+  // trace. (For instance, in a browser action error, the source is the page -
+  // sometimes the background page - but the error is thrown from the script.)
+  // Make the source match the stack trace, since that is more likely the cause
+  // of the error.
+  if (!stack_trace_.empty() && source_ != stack_trace_[0].source)
+    source_ = stack_trace_[0].source;
 }
 
 }  // namespace extensions
diff --git a/extensions/browser/extension_error.h b/extensions/browser/extension_error.h
index c5be169..6ee7791 100644
--- a/extensions/browser/extension_error.h
+++ b/extensions/browser/extension_error.h
@@ -11,6 +11,8 @@
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "base/strings/string16.h"
+#include "extensions/common/stack_frame.h"
+#include "url/gurl.h"
 
 namespace extensions {
 
@@ -83,52 +85,27 @@ class ManifestError : public ExtensionError {
 
 class RuntimeError : public ExtensionError {
  public:
-  struct StackFrame {
-    size_t line_number;
-    size_t column_number;
-    // This is stored as a string (rather than a url) since it can be a
-    // Chrome script file (e.g., event_bindings.js).
-    base::string16 url;
-    base::string16 function;  // optional
-
-    // STL-Required constructor
-    StackFrame();
-
-    StackFrame(size_t frame_line,
-               size_t frame_column,
-               const base::string16& frame_url,
-               const base::string16& frame_function  /* can be empty */);
-
-    ~StackFrame();
-
-    bool operator==(const StackFrame& rhs) const;
-  };
-  typedef std::vector<StackFrame> StackTrace;
-
   RuntimeError(bool from_incognito,
                const base::string16& source,
                const base::string16& message,
-               logging::LogSeverity level,
-               const base::string16& details);
+               const StackTrace& stack_trace,
+               const GURL& context_url,
+               logging::LogSeverity level);
   virtual ~RuntimeError();
 
   virtual std::string PrintForTest() const OVERRIDE;
 
-  const base::string16& execution_context_url() const {
-      return execution_context_url_;
-  }
+  const GURL& context_url() const { return context_url_; }
   const StackTrace& stack_trace() const { return stack_trace_; }
  private:
   virtual bool IsEqualImpl(const ExtensionError* rhs) const OVERRIDE;
 
-  // Parse the JSON |details| passed to the error. This includes a stack trace
-  // and an execution context url.
-  void ParseDetails(const base::string16& details);
-  // Try to determine the ID of the extension. This may be obtained through the
-  // reported source, or through the execution context url.
-  void DetermineExtensionID();
+  // Since we piggy-back onto other error reporting systems (like V8 and
+  // WebKit), the reported information may need to be cleaned up in order to be
+  // in a consistent format.
+  void CleanUpInit();
 
-  base::string16 execution_context_url_;
+  GURL context_url_;
   StackTrace stack_trace_;
 
   DISALLOW_COPY_AND_ASSIGN(RuntimeError);
diff --git a/extensions/common/matcher/DEPS b/extensions/common/DEPS
index 972b087..972b087 100644
--- a/extensions/common/matcher/DEPS
+++ b/extensions/common/DEPS
diff --git a/extensions/common/constants.h b/extensions/common/constants.h
index 492ddbc..648663d 100644
--- a/extensions/common/constants.h
+++ b/extensions/common/constants.h
@@ -12,13 +12,13 @@ namespace extensions {
 // Scheme we serve extension content from.
 extern const char kExtensionScheme[];
 
-  // The name of the manifest inside an extension.
+// The name of the manifest inside an extension.
 extern const base::FilePath::CharType kManifestFilename[];
 
-  // The name of locale folder inside an extension.
+// The name of locale folder inside an extension.
 extern const base::FilePath::CharType kLocaleFolder[];
 
-  // The name of the messages file inside an extension.
+// The name of the messages file inside an extension.
 extern const base::FilePath::CharType kMessagesFilename[];
 
 // The base directory for subdirectories with platform-specific code.
diff --git a/extensions/common/extension_urls.cc b/extensions/common/extension_urls.cc
new file mode 100644
index 0000000..f354b6b
--- /dev/null
+++ b/extensions/common/extension_urls.cc
@@ -0,0 +1,16 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "extensions/common/extension_urls.h"
+
+#include "extensions/common/constants.h"
+#include "url/gurl.h"
+
+namespace extensions {
+
+bool IsSourceFromAnExtension(const base::string16& source) {
+  return GURL(source).SchemeIs(kExtensionScheme);
+}
+
+}  // namespace extensions
diff --git a/extensions/common/extension_urls.h b/extensions/common/extension_urls.h
new file mode 100644
index 0000000..7fa7e1e
--- /dev/null
+++ b/extensions/common/extension_urls.h
@@ -0,0 +1,20 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef EXTENSIONS_COMMON_EXTENSION_URLS_H_
+#define EXTENSIONS_COMMON_EXTENSION_URLS_H_
+
+#include "base/strings/string16.h"
+
+namespace extensions {
+
+// Determine whether or not a source came from an extension. |source| can link
+// to a page or a script, and can be external (e.g., "http://www.google.com"),
+// extension-related (e.g., "chrome-extension://<extension_id>/background.js"),
+// or internal (e.g., "event_bindings" or "schemaUtils").
+bool IsSourceFromAnExtension(const base::string16& source);
+
+}  // namespace extensions
+
+#endif  // EXTENSIONS_COMMON_EXTENSION_URLS_H_
diff --git a/extensions/common/stack_frame.cc b/extensions/common/stack_frame.cc
new file mode 100644
index 0000000..d287496
--- /dev/null
+++ b/extensions/common/stack_frame.cc
@@ -0,0 +1,80 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "extensions/common/stack_frame.h"
+
+#include <string>
+
+#include "base/strings/utf_string_conversions.h"
+#include "third_party/re2/re2/re2.h"
+
+namespace extensions {
+
+namespace {
+const char kAnonymousFunction[] = "(anonymous function)";
+}
+
+StackFrame::StackFrame() : line_number(1), column_number(1) {
+}
+
+StackFrame::StackFrame(const StackFrame& frame)
+    : line_number(frame.line_number),
+      column_number(frame.column_number),
+      source(frame.source),
+      function(frame.function) {
+}
+
+StackFrame::StackFrame(size_t line_number,
+                       size_t column_number,
+                       const base::string16& source,
+                       const base::string16& function)
+    : line_number(line_number),
+      column_number(column_number),
+      source(source),
+      function(function.empty() ? base::UTF8ToUTF16(kAnonymousFunction)
+                   : function) {
+}
+
+StackFrame::~StackFrame() {
+}
+
+// Create a stack frame from the passed text. The text must follow one of two
+// formats:
+//   - "function_name (source:line_number:column_number)"
+//   - "source:line_number:column_number"
+// (We have to recognize two formats because V8 will report stack traces in
+// both ways. If we reconcile this, we can clean this up.)
+// static
+scoped_ptr<StackFrame> StackFrame::CreateFromText(
+    const base::string16& frame_text) {
+  // We need to use utf8 for re2 matching.
+  std::string text = base::UTF16ToUTF8(frame_text);
+
+  size_t line = 1;
+  size_t column = 1;
+  std::string source;
+  std::string function;
+  if (!re2::RE2::FullMatch(text,
+                           "(.+) \\(([^\\(\\)]+):(\\d+):(\\d+)\\)",
+                           &function, &source, &line, &column) &&
+      !re2::RE2::FullMatch(text,
+                           "([^\\(\\)]+):(\\d+):(\\d+)",
+                           &source, &line, &column)) {
+    return scoped_ptr<StackFrame>();
+  }
+
+  return scoped_ptr<StackFrame>(new StackFrame(line,
+                                               column,
+                                               base::UTF8ToUTF16(source),
+                                               base::UTF8ToUTF16(function)));
+}
+
+bool StackFrame::operator==(const StackFrame& rhs) const {
+  return line_number == rhs.line_number &&
+         column_number == rhs.column_number &&
+         source == rhs.source &&
+         function == rhs.function;
+}
+
+}  // namespace extensions
diff --git a/extensions/common/stack_frame.h b/extensions/common/stack_frame.h
new file mode 100644
index 0000000..7052605
--- /dev/null
+++ b/extensions/common/stack_frame.h
@@ -0,0 +1,41 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef EXTENSIONS_COMMON_STACK_FRAME
+#define EXTENSIONS_COMMON_STACK_FRAME
+
+#include <vector>
+
+#include "base/memory/scoped_ptr.h"
+#include "base/strings/string16.h"
+
+namespace extensions {
+
+struct StackFrame {
+  StackFrame();
+  StackFrame(const StackFrame& frame);
+  StackFrame(size_t line_number,
+             size_t column_number,
+             const base::string16& source,
+             const base::string16& function);
+  ~StackFrame();
+
+  // Construct a stack frame from a reported plain-text frame.
+  static scoped_ptr<StackFrame> CreateFromText(
+      const base::string16& frame_text);
+
+  bool operator==(const StackFrame& rhs) const;
+
+  size_t line_number;
+  size_t column_number;
+  base::string16 source;
+  base::string16 function;  // optional
+};
+
+typedef std::vector<StackFrame> StackTrace;
+
+}  // namespace extensions
+
+#endif  // EXTENSIONS_COMMON_STACK_FRAME
+
diff --git a/extensions/common/stack_frame_unittest.cc b/extensions/common/stack_frame_unittest.cc
new file mode 100644
index 0000000..7dad047
--- /dev/null
+++ b/extensions/common/stack_frame_unittest.cc
@@ -0,0 +1,85 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "extensions/common/stack_frame.h"
+
+#include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/strings/string16.h"
+#include "base/strings/utf_string_conversions.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using base::UTF8ToUTF16;
+
+namespace extensions {
+
+namespace {
+
+void AssertStackFrameValid(const std::string& text,
+                           size_t line,
+                           size_t column,
+                           const std::string& source,
+                           const std::string& function) {
+  base::string16 utf16_text = UTF8ToUTF16(text);
+  scoped_ptr<StackFrame> frame = StackFrame::CreateFromText(utf16_text);
+
+  ASSERT_TRUE(frame.get()) << "Failed to create frame from '" << text << "'";
+  EXPECT_EQ(line, frame->line_number());
+  EXPECT_EQ(column, frame->column_number());
+  EXPECT_EQ(UTF8ToUTF16(source), frame->source());
+  EXPECT_EQ(UTF8ToUTF16(function), frame->function());
+}
+
+void AssertStackFrameInvalid(const std::string& text) {
+  base::string16 utf16_text = UTF8ToUTF16(text);
+  scoped_ptr<StackFrame> frame = StackFrame::CreateFromText(utf16_text);
+  ASSERT_FALSE(frame.get()) << "Errantly created frame from '" << text << "'";
+}
+
+}  // namespace
+
+TEST(StackFrameUnitTest, ParseStackFramesFromText) {
+  AssertStackFrameValid(
+      "function_name (https://www.url.com/foo.html:100:201)",
+      100u, 201u, "https://www.url.com/foo.html", "function_name");
+  AssertStackFrameValid(
+      "(anonymous function) (https://www.url.com/foo.html:100:201)",
+      100u, 201u, "https://www.url.com/foo.html", "(anonymous function)");
+  AssertStackFrameValid(
+      "Function.target.(anonymous function) (internals::SafeBuiltins:19:14)",
+      19u, 14u, "internals::SafeBuiltins",
+      "Function.target.(anonymous function)");
+  AssertStackFrameValid(
+      "internal-item:://fpgohbggpmcpeedljibghijiclejiklo/script.js:6:12",
+      6u, 12u, "internal-item:://fpgohbggpmcpeedljibghijiclejiklo/script.js",
+      "(anonymous function)");
+
+  // No delimiting ':' between line/column numbers.
+  AssertStackFrameInvalid(
+      "function_name (https://www.url.com/foo.html:100201)");
+  // No line number.
+  AssertStackFrameInvalid("function_name (https://www.url.com/foo.html::201)");
+  // No line number or delimiting ':'.
+  AssertStackFrameInvalid("function_name (https://www.url.com/foo.html201)");
+  // No leading '(' around url, line, column.
+  AssertStackFrameInvalid(
+      "function_name https://www.url.com/foo.html:100:201)");
+  // No trailing ')'.
+  AssertStackFrameInvalid(
+      "function_name (https://www.url.com/foo.html:100:201");
+  // Trailing ' '.
+  AssertStackFrameInvalid(
+      "function_name (https://www.url.com/foo.html:100:201) ");
+  // Invalid column number.
+  AssertStackFrameInvalid(
+      "function_name (https://www.url.com/foo.html:100:201a)");
+  // Negative column number.
+  AssertStackFrameInvalid(
+      "function_name (https://www.url.com/foo.html:100:-201)");
+  // Extra trailing ')'
+  AssertStackFrameInvalid(
+      "function_name (https://www.url.com/foo.html:100:201))");
+}
+
+}  // namespace extensions