Add ExceptionHandler to ModuleSystem, remove heap allocated v8::TryCatch.

BUG=152389


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

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

7 files changed, 68 insertions, 11 deletions

diff --git a/chrome/renderer/extensions/event_unittest.cc b/chrome/renderer/extensions/event_unittest.cc
index 62bbf09..edd47c0 100644
--- a/chrome/renderer/extensions/event_unittest.cc
+++ b/chrome/renderer/extensions/event_unittest.cc
@@ -151,7 +151,7 @@ TEST_F(EventUnittest, NamedEventDispatch) {
       "var e = new event.Event('myevent');"
       "var called = false;"
       "e.addListener(function() { called = true; });"
-      "chromeHidden.Event.dispatchJSON('myevent', []);"
+      "chromeHidden.Event.dispatchEvent('myevent', []);"
       "assert.AssertTrue(called);");
   module_system_->Require("test");
 }
diff --git a/chrome/renderer/extensions/module_system.cc b/chrome/renderer/extensions/module_system.cc
index 594f645..69b2c78 100644
--- a/chrome/renderer/extensions/module_system.cc
+++ b/chrome/renderer/extensions/module_system.cc
@@ -63,6 +63,12 @@ bool ModuleSystem::IsPresentInCurrentContext() {
   return !module_system.IsEmpty() && !module_system->IsUndefined();
 }
 
+void ModuleSystem::HandleException(const v8::TryCatch& try_catch) {
+  DumpException(try_catch);
+  if (exception_handler_.get())
+    exception_handler_->HandleUncaughtException();
+}
+
 // static
 void ModuleSystem::DumpException(const v8::TryCatch& try_catch) {
   v8::HandleScope handle_scope;
@@ -143,7 +149,7 @@ v8::Handle<v8::Value> ModuleSystem::RequireForJsInner(
     try_catch.SetCaptureMessage(true);
     func->Call(global, 3, args);
     if (try_catch.HasCaught()) {
-      DumpException(try_catch);
+      HandleException(try_catch);
       return v8::Undefined();
     }
   }
@@ -174,7 +180,7 @@ void ModuleSystem::CallModuleMethod(const std::string& module_name,
     try_catch.SetCaptureMessage(true);
     func->Call(global, 0, NULL);
     if (try_catch.HasCaught())
-      DumpException(try_catch);
+      HandleException(try_catch);
   }
 }
 
@@ -251,13 +257,13 @@ v8::Handle<v8::Value> ModuleSystem::RunString(v8::Handle<v8::String> code,
   try_catch.SetCaptureMessage(true);
   v8::Handle<v8::Script> script(v8::Script::New(code, name));
   if (try_catch.HasCaught()) {
-    DumpException(try_catch);
+    HandleException(try_catch);
     return handle_scope.Close(result);
   }
 
   result = script->Run();
   if (try_catch.HasCaught())
-    DumpException(try_catch);
+    HandleException(try_catch);
 
   return handle_scope.Close(result);
 }
diff --git a/chrome/renderer/extensions/module_system.h b/chrome/renderer/extensions/module_system.h
index e371865..4f79084 100644
--- a/chrome/renderer/extensions/module_system.h
+++ b/chrome/renderer/extensions/module_system.h
@@ -37,10 +37,17 @@ class ModuleSystem : public NativeHandler {
  public:
   class SourceMap {
    public:
+    virtual ~SourceMap() {}
     virtual v8::Handle<v8::Value> GetSource(const std::string& name) = 0;
     virtual bool Contains(const std::string& name) = 0;
   };
 
+  class ExceptionHandler {
+   public:
+    virtual ~ExceptionHandler() {}
+    virtual void HandleUncaughtException() = 0;
+  };
+
   // Enables native bindings for the duration of its lifetime.
   class NativesEnabledScope {
    public:
@@ -99,9 +106,16 @@ class ModuleSystem : public NativeHandler {
                     const std::string& module_name,
                     const std::string& module_field);
 
+  void set_exception_handler(scoped_ptr<ExceptionHandler> handler) {
+    exception_handler_ = handler.Pass();
+  }
+
  private:
   typedef std::map<std::string, linked_ptr<NativeHandler> > NativeHandlerMap;
 
+  // Called when an exception is thrown but not caught.
+  void HandleException(const v8::TryCatch& try_catch);
+
   // Ensure that require_ has been evaluated from require.js.
   void EnsureRequireLoaded();
 
@@ -139,6 +153,9 @@ class ModuleSystem : public NativeHandler {
   // pinned natives as enabled.
   int natives_enabled_;
 
+  // Called when an exception is thrown but not caught in JS.
+  scoped_ptr<ExceptionHandler> exception_handler_;
+
   std::set<std::string> overridden_native_handlers_;
 
   DISALLOW_COPY_AND_ASSIGN(ModuleSystem);
diff --git a/chrome/renderer/extensions/module_system_unittest.cc b/chrome/renderer/extensions/module_system_unittest.cc
index 9bf7471..da233b8 100644
--- a/chrome/renderer/extensions/module_system_unittest.cc
+++ b/chrome/renderer/extensions/module_system_unittest.cc
@@ -31,6 +31,36 @@ class CounterNatives : public NativeHandler {
   int counter_;
 };
 
+class TestExceptionHandler : public ModuleSystem::ExceptionHandler {
+ public:
+  TestExceptionHandler()
+      : handled_exception_(false) {
+  }
+
+  virtual void HandleUncaughtException() {
+    handled_exception_ = true;
+  }
+
+  bool handled_exception() const { return handled_exception_; }
+
+ private:
+  bool handled_exception_;
+};
+
+TEST_F(ModuleSystemTest, TestExceptionHandling) {
+  ModuleSystem::NativesEnabledScope natives_enabled_scope(module_system_.get());
+  TestExceptionHandler* handler = new TestExceptionHandler;
+  scoped_ptr<ModuleSystem::ExceptionHandler> scoped_handler(handler);
+  ASSERT_FALSE(handler->handled_exception());
+  module_system_->set_exception_handler(scoped_handler.Pass());
+
+  RegisterModule("test", "throw 'hi';");
+  module_system_->Require("test");
+  ASSERT_TRUE(handler->handled_exception());
+
+  ExpectNoAssertionsMade();
+}
+
 TEST_F(ModuleSystemTest, TestRequire) {
   ModuleSystem::NativesEnabledScope natives_enabled_scope(module_system_.get());
   RegisterModule("add", "exports.Add = function(x, y) { return x + y; };");
diff --git a/chrome/renderer/resource_bundle_source_map.h b/chrome/renderer/resource_bundle_source_map.h
index b77b32f..9fcf01f 100644
--- a/chrome/renderer/resource_bundle_source_map.h
+++ b/chrome/renderer/resource_bundle_source_map.h
@@ -22,7 +22,7 @@ namespace ui {
 class ResourceBundleSourceMap : public extensions::ModuleSystem::SourceMap {
  public:
   explicit ResourceBundleSourceMap(const ui::ResourceBundle* resource_bundle);
-  ~ResourceBundleSourceMap();
+  virtual ~ResourceBundleSourceMap();
 
   virtual v8::Handle<v8::Value> GetSource(const std::string& name) OVERRIDE;
   virtual bool Contains(const std::string& name) OVERRIDE;
diff --git a/chrome/test/base/module_system_test.cc b/chrome/test/base/module_system_test.cc
index 1f7b75b..365ea7f 100644
--- a/chrome/test/base/module_system_test.cc
+++ b/chrome/test/base/module_system_test.cc
@@ -75,6 +75,13 @@ class StringSourceMap : public ModuleSystem::SourceMap {
   std::map<std::string, std::string> source_map_;
 };
 
+class FailsOnException : public ModuleSystem::ExceptionHandler {
+ public:
+  virtual void HandleUncaughtException() {
+    FAIL();
+  }
+};
+
 ModuleSystemTest::ModuleSystemTest()
     : context_(v8::Context::New()),
       source_map_(new StringSourceMap()),
@@ -84,7 +91,8 @@ ModuleSystemTest::ModuleSystemTest()
   module_system_.reset(new ModuleSystem(context_, source_map_.get()));
   module_system_->RegisterNativeHandler("assert", scoped_ptr<NativeHandler>(
       assert_natives_));
-  try_catch_.SetCaptureMessage(true);
+  module_system_->set_exception_handler(
+      scoped_ptr<ModuleSystem::ExceptionHandler>(new FailsOnException));
 }
 
 ModuleSystemTest::~ModuleSystemTest() {
@@ -112,9 +120,6 @@ void ModuleSystemTest::OverrideNativeHandler(const std::string& name,
 }
 
 void ModuleSystemTest::TearDown() {
-  if (try_catch_.HasCaught())
-    ModuleSystem::DumpException(try_catch_);
-  EXPECT_FALSE(try_catch_.HasCaught());
   // All tests must assert at least once unless otherwise specified.
   EXPECT_EQ(should_assertions_be_made_,
             assert_natives_->assertion_made());
diff --git a/chrome/test/base/module_system_test.h b/chrome/test/base/module_system_test.h
index bc9832c..457832b 100644
--- a/chrome/test/base/module_system_test.h
+++ b/chrome/test/base/module_system_test.h
@@ -52,7 +52,6 @@ class ModuleSystemTest : public testing::Test {
 
   v8::Persistent<v8::Context> context_;
   v8::HandleScope handle_scope_;
-  v8::TryCatch try_catch_;
   AssertNatives* assert_natives_;
   scoped_ptr<StringSourceMap> source_map_;
   scoped_ptr<extensions::ModuleSystem> module_system_;