Improvements to fatal JavaScript extension errors: (1) include the extension

chrome/renderer/extensions/module_system.cc

 // Copyright (c) 2012 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 "chrome/renderer/extensions/module_system.h"
 
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension_messages.h"
 #include "chrome/renderer/extensions/chrome_v8_context.h"
 #include "chrome/renderer/extensions/console.h"
 #include "content/public/renderer/render_view.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebFrame.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebScopedMicrotaskSuppression.h"
 
 namespace extensions {
 
 namespace {
 
 const char* kModuleSystem = "module_system";
 const char* kModuleName = "module_name";
 const char* kModuleField = "module_field";
 const char* kModulesField = "modules";
 
+// Prepends |extension_id| if it's non-empty to |message|.
+std::string PrependExtensionID(const std::string& extension_id,
+                               const std::string& message) {
+  std::string with_extension_id = message;
+  if (!extension_id.empty()) {
+    with_extension_id += "(";

[Jeffrey Yasskin, 2013/06/14 22:11:22]

Heh, this will put the extension_id after the message.

[not at google - send to devlin, 2013/06/14 22:27:44]

damnnnnnnn

+    with_extension_id += extension_id;
+    with_extension_id += ") ";
+  }
+  return with_extension_id;
+}
+
+void Fatal(const std::string& extension_id, const std::string& message) {
+  // Only crash extension processes. Ideally we would crash on *any* (i.e. web
+  // too), but given we hardly have code coverage for JS we should be careful
+  // and not break the web.
+  //
+  // NOTE: make single-process count as an extension process since it's for
+  // debugging, and we'll almost certainly want to use it to repro them.

[Jeffrey Yasskin, 2013/06/14 22:11:22]

"them" lacks an antecedent.

[not at google - send to devlin, 2013/06/14 22:27:44]

rephrased.

+  const CommandLine* command_line = CommandLine::ForCurrentProcess();
+  bool is_extension_process =
+      command_line->HasSwitch(switches::kExtensionProcess) ||
+      command_line->HasSwitch(switches::kSingleProcess);
+  std::string with_extension_id = PrependExtensionID(extension_id, message);
+  if (is_extension_process)
+    console::Fatal(v8::Context::GetCalling(), with_extension_id);
+  else
+    console::Error(v8::Context::GetCalling(), with_extension_id);
+}
+
+void Warn(const std::string& extension_id, const std::string& message) {
+  console::Warn(v8::Context::GetCalling(),
+                PrependExtensionID(extension_id, message));
+}
+
 // Default exception handler which logs the exception.
 class DefaultExceptionHandler : public ModuleSystem::ExceptionHandler {
  public:
+  explicit DefaultExceptionHandler(const std::string& extension_id)
+      : extension_id_(extension_id) {}
+
   // Fatally dumps the debug info from |try_catch| to the console.
   // Make sure this is never used for exceptions that originate in external
   // code!
   virtual void HandleUncaughtException(const v8::TryCatch& try_catch) OVERRIDE {
     v8::HandleScope handle_scope;
     std::string stack_trace = "<stack trace unavailable>";
     if (!try_catch.StackTrace().IsEmpty()) {
       v8::String::Utf8Value stack_value(try_catch.StackTrace());
       if (*stack_value)
         stack_trace.assign(*stack_value, stack_value.length());
       else
         stack_trace = "<could not convert stack trace to string>";
     }
-    console::Fatal(v8::Context::GetCalling(),
-                   CreateExceptionString(try_catch) + "{" + stack_trace + "}");
+    Fatal(extension_id_,
+          CreateExceptionString(try_catch) + "{" + stack_trace + "}");
   }
+
+ private:
+  std::string extension_id_;
 };
 
 } // namespace
 
 std::string ModuleSystem::ExceptionHandler::CreateExceptionString(
     const v8::TryCatch& try_catch) {
   v8::Handle<v8::Message> message(try_catch.Message());
   if (message.IsEmpty()) {
     return "try_catch has no message";
   }
@@ skipping 16 matching lines @@
                             message->GetLineNumber(),
                             error_message.c_str());
 }
 
 ModuleSystem::ModuleSystem(ChromeV8Context* context,
                            SourceMap* source_map)
     : ObjectBackedNativeHandler(context),
       context_(context),
       source_map_(source_map),
       natives_enabled_(0),
-      exception_handler_(new DefaultExceptionHandler()) {
+      exception_handler_(
+          new DefaultExceptionHandler(context->GetExtensionID())) {
   RouteFunction("require",
       base::Bind(&ModuleSystem::RequireForJs, base::Unretained(this)));
   RouteFunction("requireNative",
       base::Bind(&ModuleSystem::RequireNative, base::Unretained(this)));
 
   v8::Handle<v8::Object> global(context->v8_context()->Global());
   global->SetHiddenValue(v8::String::New(kModulesField), v8::Object::New());
   global->SetHiddenValue(v8::String::New(kModuleSystem),
                          v8::External::New(this));
 }
@@ skipping 57 matching lines @@
   v8::Context::Scope context_scope(context()->v8_context());
 
   v8::Handle<v8::Object> global(context()->v8_context()->Global());
 
   // The module system might have been deleted. This can happen if a different
   // context keeps a reference to us, but our frame is destroyed (e.g.
   // background page keeps reference to chrome object in a closed popup).
   v8::Handle<v8::Value> modules_value =
       global->GetHiddenValue(v8::String::New(kModulesField));
   if (modules_value.IsEmpty() || modules_value->IsUndefined()) {
-    console::Warn(v8::Context::GetCalling(), "Extension view no longer exists");
+    Warn(context_->GetExtensionID(), "Extension view no longer exists");
     return v8::Undefined();
   }
 
   v8::Handle<v8::Object> modules(v8::Handle<v8::Object>::Cast(modules_value));
   v8::Handle<v8::Value> exports(modules->Get(module_name));
   if (!exports->IsUndefined())
     return handle_scope.Close(exports);
 
   std::string module_name_str = *v8::String::AsciiValue(module_name);
   v8::Handle<v8::Value> source(GetSource(module_name_str));
   if (source.IsEmpty() || source->IsUndefined()) {
-    console::Error(v8::Context::GetCalling(),
-                   "No source for require(" + module_name_str + ")");
+    Fatal(context_->GetExtensionID(),
+          "No source for require(" + module_name_str + ")");
     return v8::Undefined();
   }
   v8::Handle<v8::String> wrapped_source(WrapSource(
       v8::Handle<v8::String>::Cast(source)));
   // Modules are wrapped in (function(){...}) so they always return functions.
   v8::Handle<v8::Value> func_as_value = RunString(wrapped_source, module_name);
   if (func_as_value.IsEmpty() || func_as_value->IsUndefined()) {
-    console::Error(v8::Context::GetCalling(),
-                   "Bad source for require(" + module_name_str + ")");
+    Fatal(context_->GetExtensionID(),
+          "Bad source for require(" + module_name_str + ")");
     return v8::Undefined();
   }
 
   v8::Handle<v8::Function> func = v8::Handle<v8::Function>::Cast(func_as_value);
 
   exports = v8::Object::New();
   v8::Handle<v8::Object> natives(NewInstance());
   v8::Handle<v8::Value> args[] = {
     natives->Get(v8::String::NewSymbol("require")),
     natives->Get(v8::String::NewSymbol("requireNative")),
@@ skipping 41 matching lines @@
   v8::Context::Scope context_scope(context()->v8_context());
 
   v8::Local<v8::Value> module;
   {
     NativesEnabledScope natives_enabled(this);
     module = v8::Local<v8::Value>::New(
         RequireForJsInner(v8::String::New(module_name.c_str())));
   }
 
   if (module.IsEmpty() || !module->IsObject()) {
-    console::Error(
-        v8::Context::GetCalling(),
-        "Failed to get module " + module_name + " to call " + method_name);
+    Fatal(context_->GetExtensionID(),
+          "Failed to get module " + module_name + " to call " + method_name);
     return handle_scope.Close(v8::Undefined());
   }
 
   v8::Local<v8::Value> value =
       v8::Handle<v8::Object>::Cast(module)->Get(
           v8::String::New(method_name.c_str()));
   if (value.IsEmpty() || !value->IsFunction()) {
-    console::Error(v8::Context::GetCalling(),
-                   module_name + "." + method_name + " is not a function");
+    Fatal(context_->GetExtensionID(),
+          module_name + "." + method_name + " is not a function");
     return handle_scope.Close(v8::Undefined());
   }
 
   v8::Handle<v8::Function> func = v8::Handle<v8::Function>::Cast(value);
   v8::Local<v8::Value> result;
   {
     v8::TryCatch try_catch;
     try_catch.SetCaptureMessage(true);
     result = context_->CallFunction(func, argc, argv);
     if (try_catch.HasCaught())
@@ skipping 43 matching lines @@
   v8::HandleScope handle_scope;
   v8::Handle<v8::Object> parameters = v8::Handle<v8::Object>::Cast(info.Data());
   // This context should be the same as context()->v8_context().
   v8::Handle<v8::Context> context = parameters->CreationContext();
   v8::Handle<v8::Object> global(context->Global());
   v8::Handle<v8::Value> module_system_value =
       global->GetHiddenValue(v8::String::New(kModuleSystem));
   if (module_system_value.IsEmpty() || !module_system_value->IsExternal()) {
     // ModuleSystem has been deleted.
     // TODO(kalman): See comment in header file.
-    console::Warn(v8::Context::GetCalling(),
-                  "Module system has been deleted, does extension view exist?");
+    Warn("", "Module system has been deleted, does extension view exist?");
     return;
   }
 
   ModuleSystem* module_system = static_cast<ModuleSystem*>(
       v8::Handle<v8::External>::Cast(module_system_value)->Value());
 
   std::string name = *v8::String::AsciiValue(
       parameters->Get(v8::String::New(kModuleName))->ToString());
 
   // Switch to our v8 context because we need functions created while running
@@ skipping 11 matching lines @@
     // require_function will have already logged this, we don't need to.
     return;
   }
 
   v8::Handle<v8::Object> module = v8::Handle<v8::Object>::Cast(module_value);
   v8::Handle<v8::String> field =
       parameters->Get(v8::String::New(kModuleField))->ToString();
 
   if (!module->Has(field)) {
     std::string field_str = *v8::String::AsciiValue(field);
-    console::Fatal(v8::Context::GetCalling(),
-                   "Lazy require of " + name + "." + field_str + " did not " +
-                   "set the " + field_str + " field");
+    Fatal(module_system->context_->GetExtensionID(),
+          "Lazy require of " + name + "." + field_str + " did not " +
+              "set the " + field_str + " field");
     return;
   }
 
   v8::Local<v8::Value> new_field = module->Get(field);
   if (try_catch.HasCaught()) {
     module_system->HandleException(try_catch);
     return;
   }
 
   // Ok for it to be undefined, among other things it's how bindings signify
@@ skipping 79 matching lines @@
 }
 
 v8::Handle<v8::Value> ModuleSystem::RequireNativeFromString(
     const std::string& native_name) {
   if (natives_enabled_ == 0) {
     // HACK: if in test throw exception so that we can test the natives-disabled
     // logic; however, under normal circumstances, this is programmer error so
     // we could crash.
     if (exception_handler_)
       return v8::ThrowException(v8::String::New("Natives disabled"));
-    console::Fatal(v8::Context::GetCalling(),
-                   "Natives disabled for requireNative(" + native_name + ")");
+    Fatal(context_->GetExtensionID(),
+          "Natives disabled for requireNative(" + native_name + ")");
     return v8::Undefined();
   }
 
   if (overridden_native_handlers_.count(native_name) > 0u)
     return RequireForJsInner(v8::String::New(native_name.c_str()));
 
   NativeHandlerMap::iterator i = native_handler_map_.find(native_name);
   if (i == native_handler_map_.end()) {
-    console::Fatal(
-        v8::Context::GetCalling(),
-        "Couldn't find native for requireNative(" + native_name + ")");
+    Fatal(context_->GetExtensionID(),
+          "Couldn't find native for requireNative(" + native_name + ")");
     return v8::Undefined();
   }
   return i->second->NewInstance();
 }
 
 v8::Handle<v8::String> ModuleSystem::WrapSource(v8::Handle<v8::String> source) {
   v8::HandleScope handle_scope;
   v8::Handle<v8::String> left = v8::String::New(
       "(function(require, requireNative, exports) {'use strict';");
   v8::Handle<v8::String> right = v8::String::New("\n})");
   return handle_scope.Close(
       v8::String::Concat(left, v8::String::Concat(source, right)));
 }
 
 }  // namespace extensions