Index: extensions/browser/extension_error.cc |
diff --git a/extensions/browser/extension_error.cc b/extensions/browser/extension_error.cc |
index 8b6196c4200967605b3eb51f0653035bd4e88744..4e15ba0af2ef6fec1dc776a5e05ffb11c0a5b2b4 100644 |
--- a/extensions/browser/extension_error.cc |
+++ b/extensions/browser/extension_error.cc |
@@ -4,11 +4,13 @@ |
#include "extensions/browser/extension_error.h" |
-#include "base/json/json_reader.h" |
#include "base/strings/string_number_conversions.h" |
+#include "base/strings/string_split.h" |
+#include "base/strings/string_util.h" |
#include "base/strings/utf_string_conversions.h" |
#include "base/values.h" |
#include "extensions/common/constants.h" |
+#include "third_party/re2/re2/re2.h" |
#include "url/gurl.h" |
using base::string16; |
@@ -23,6 +25,8 @@ const char kURLKey[] = "url"; |
const char kFunctionNameKey[] = "functionName"; |
const char kExecutionContextURLKey[] = "executionContextURL"; |
const char kStackTraceKey[] = "stackTrace"; |
+const char kStackFrameDelimiter[] = "\n at "; |
+const char kAnonymousFunction[] = "(anonymous function)"; |
// Try to retrieve an extension ID from a |url|. On success, returns true and |
// populates |extension_id| with the ID. On failure, returns false and leaves |
@@ -77,10 +81,6 @@ std::string ManifestParsingError::PrintForTest() const { |
"\n Type: ManifestParsingError"; |
} |
-JavascriptRuntimeError::StackFrame::StackFrame() : line_number(-1), |
- column_number(-1) { |
-} |
- |
JavascriptRuntimeError::StackFrame::StackFrame(size_t frame_line, |
size_t frame_column, |
const string16& frame_url, |
@@ -94,19 +94,54 @@ JavascriptRuntimeError::StackFrame::StackFrame(size_t frame_line, |
JavascriptRuntimeError::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. |
Yoyo Zhou
2013/08/23 22:54:54
nit: )
Devlin
2013/08/23 23:44:30
Done.
|
+scoped_ptr<JavascriptRuntimeError::StackFrame> |
+JavascriptRuntimeError::StackFrame::CreateFromText(const string16& utf16_text) { |
+ // We need to use utf8 for re2 matching. |
+ std::string text = base::UTF16ToUTF8(utf16_text); |
+ |
+ size_t line = -1; |
+ size_t column = -1; |
+ std::string url; |
+ std::string function; |
+ if (!re2::RE2::FullMatch(text, |
+ "(.+) \\(([^\\(\\)]+):(\\d+):(\\d+)\\)", |
+ &function, &url, &line, &column) && |
+ !re2::RE2::FullMatch(text, |
+ "([^\\(\\)]+):(\\d+):(\\d+)", |
+ &url, &line, &column)) { |
+ return scoped_ptr<StackFrame>(); |
+ } |
+ |
+ if (function.empty()) |
+ function = kAnonymousFunction; |
+ |
+ return scoped_ptr<StackFrame>( |
+ new StackFrame( |
+ line, column, base::UTF8ToUTF16(url), base::UTF8ToUTF16(function))); |
+} |
+ |
JavascriptRuntimeError::JavascriptRuntimeError(bool from_incognito, |
const string16& source, |
const string16& message, |
- logging::LogSeverity level, |
- const string16& details) |
+ const string16& stack_trace, |
+ int32 line_number, |
+ const GURL& context_url, |
+ logging::LogSeverity level) |
: ExtensionError(ExtensionError::JAVASCRIPT_RUNTIME_ERROR, |
- std::string(), // We don't know the id yet. |
+ GURL(source).host(), |
from_incognito, |
source, |
message), |
+ context_url_(context_url), |
level_(level) { |
- ParseDetails(details); |
- DetermineExtensionID(); |
+ GetStackTrace(message, stack_trace, line_number); |
+ CleanUpInit(); |
} |
JavascriptRuntimeError::~JavascriptRuntimeError() { |
@@ -115,58 +150,78 @@ JavascriptRuntimeError::~JavascriptRuntimeError() { |
std::string JavascriptRuntimeError::PrintForTest() const { |
std::string result = ExtensionError::PrintForTest() + |
"\n Type: JavascriptRuntimeError" |
- "\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 Function: " + base::UTF16ToUTF8(iter->function) + |
+ "\n Line: " + base::IntToString((*iter)->line_number) + |
+ "\n Column: " + base::IntToString((*iter)->column_number) + |
+ "\n URL: " + base::UTF16ToUTF8((*iter)->url) + |
+ "\n Function: " + base::UTF16ToUTF8((*iter)->function) + |
"\n }"; |
} |
return result; |
} |
-void JavascriptRuntimeError::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 JavascriptRuntimeError::GetStackTrace(const string16& message, |
+ const string16& stack_trace, |
+ int32 line_number) { |
+ std::vector<string16> pieces; |
+ size_t index = 0; |
+ |
+ // If the message includes a stack trace, it's indicative (heuristically) of |
+ // a stack trace which we included ourselves in the error message. Since the |
Yoyo Zhou
2013/08/23 22:54:54
A pointer to the relevant part of the code would b
Devlin
2013/08/23 23:44:30
I think any comment-point would be flimsy and coul
|
+ // thrown error is actually from our own code, the supplied stack trace |
+ // is more detailed, and should be used. |
+ if (message.find(base::UTF8ToUTF16(kStackFrameDelimiter)) != string16::npos) { |
+ base::SplitStringUsingSubstr(message, |
+ base::UTF8ToUTF16(kStackFrameDelimiter), |
+ &pieces); |
+ message_ = pieces[0]; |
+ index = 1; |
+ } else if (!stack_trace.empty()) { |
+ // If we didn't include a stack trace in the message, we should use the |
+ // other, if possible. |
Yoyo Zhou
2013/08/23 22:54:54
"the other" - can you provide some indication of w
Devlin
2013/08/23 23:44:30
See above.
|
+ base::SplitStringUsingSubstr(stack_trace, |
+ base::UTF8ToUTF16(kStackFrameDelimiter), |
+ &pieces); |
} |
- 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)); |
+ // If we got a stack trace, parse each frame from the text. |
+ if (index < pieces.size()) { |
+ for (; index < pieces.size(); ++index) { |
+ scoped_ptr<StackFrame> frame = StackFrame::CreateFromText(pieces[index]); |
+ if (frame.get()) |
+ stack_trace_.push_back(frame.release()); |
+ } |
+ } else { // Otherwise, mock one up from the given line/url. |
+ stack_trace_.push_back(new StackFrame(line_number, |
+ 1, // column number |
+ source_, |
+ UTF8ToUTF16(kAnonymousFunction))); |
} |
} |
-void JavascriptRuntimeError::DetermineExtensionID() { |
- if (!GetExtensionIDFromGURL(GURL(source_), &extension_id_)) |
- GetExtensionIDFromGURL(GURL(execution_context_url_), &extension_id_); |
+void JavascriptRuntimeError::CleanUpInit() { |
+ // If the error came from a background page, the "context" is empty because |
Yoyo Zhou
2013/08/23 22:54:54
from a generated background page?
(What happens wh
Devlin
2013/08/23 23:44:30
Ah, yes, generated background page. Whoops.
|
+ // there's no visible URL. We should set context to be the 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; |
+ } |
+ |
+ // 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]->url) |
+ source_ = stack_trace_[0]->url; |
} |
} // namespace extensions |