[Extensions Bindings] Add an ExceptionHandler class

extensions/renderer/bindings/exception_handler.h

Index: extensions/renderer/bindings/exception_handler.h
diff --git a/extensions/renderer/bindings/exception_handler.h b/extensions/renderer/bindings/exception_handler.h
new file mode 100644
index 0000000000000000000000000000000000000000..6f2a72269c8204b73c93685faad0876c505f8836
--- /dev/null
+++ b/extensions/renderer/bindings/exception_handler.h
@@ -0,0 +1,62 @@
+// Copyright 2017 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_RENDERER_BINDINGS_EXCEPTION_HANDLER
+#define EXTENSIONS_RENDERER_BINDINGS_EXCEPTION_HANDLER
+
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "base/macros.h"
+#include "base/optional.h"
+#include "extensions/renderer/bindings/api_binding_types.h"
+#include "v8/include/v8.h"
+
+namespace extensions {
+
+// A class to handle uncaught exceptions encountered in the bindings system
+// while running untrusted code, such as exceptions thrown during callback
+// execution or event handling.
+class ExceptionHandler {
+ public:
+  ExceptionHandler(const binding::AddConsoleError& add_console_error,
+                   const binding::RunJSFunction& run_js);
+  ~ExceptionHandler();
+
+  // Handles an exception in the given |context|. |message| is a message to
+  // prefix the error message with, e.g. "Exception in response to foo".
+  // The |try_catch| is the TryCatch that caught the exception.
+  void HandleException(v8::Local<v8::Context> context,
+                       const std::string& message,
+                       const v8::TryCatch& try_catch);
+
+  // Sets a custom handler for the given context, which will be notified of
+  // exceptions thrown.
+  void SetHandlerForContext(v8::Local<v8::Context> context,

[Devlin, 2017/06/29 01:36:01]

Note: This isn't actually used in this CL (though it's tested), but it will be
used in a followup.  Including the usage here made the CL too large, but
omitting it completely seemed unnecessary.

+                            v8::Local<v8::Function> handler);
+
+  // Notifies the ExceptionHandler that the given |context| should be
+  // invalidated.
+  void InvalidateContext(v8::Local<v8::Context> context);
+
+ private:
+  // Returns the custom handler for the given |context|, or an empty handle if
+  // no custom handle exists.
+  v8::Local<v8::Function> GetCustomHandler(v8::Local<v8::Context> context);
+
+  using CustomHandlerList =
+      std::vector<std::pair<v8::Global<v8::Context>, v8::Global<v8::Function>>>;

[jbroman, 2017/06/30 18:35:31]

This seems to roughly re-implement PerContextData (stores per-context data,
dumps it on context invalidation). Is there an advantage to this approach?

[Devlin, 2017/07/06 16:49:22]

Brevity :)  It was actually quite a bit shorter than the PerContextData
approach, and we didn't need the advantage of being able to access the
PerContextData by only a context (whereas in other cases we've had, e.g., static
methods that need to access context data, these methods always go directly
through the ExceptionHandler).  But I can understand the desire to use the tools
we have.

I've added a patch that uses PerContextData.  Let me know which you prefer; I'm
fine with either.

[jbroman, 2017/07/06 21:09:18]

I think I mildly prefer the PerContextData version because we don't have to
explicitly remember to handle invalidation etc.

+  CustomHandlerList custom_handlers_;
+
+  binding::AddConsoleError add_console_error_;
+
+  binding::RunJSFunction run_js_;
+
+  DISALLOW_COPY_AND_ASSIGN(ExceptionHandler);
+};
+
+}  // namespace extensions
+
+#endif  // EXTENSIONS_RENDERER_BINDINGS_EXCEPTION_HANDLER