[webkit-reviews] review requested: [Bug 39378] [chromium] Provide a way to catch exceptions thrown while interacting with a NPObject via WebBindings methods. : [Attachment 56565] v3 patch

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has asked  for review:
Bug 39378: [chromium] Provide a way to catch exceptions thrown while
interacting with a NPObject via WebBindings methods.
https://bugs.webkit.org/show_bug.cgi?id=39378

Attachment 56565: v3 patch
https://bugs.webkit.org/attachment.cgi?id=56565&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
This is a slight modification to the V2 patch:

1- No longer ReThrow the exception if there is not an ExceptionHandler.
This was causing problems since the next entered script context would
see the re-thrown exception.  That is clearly undesirable since the
exception could be unrelated to the next entered script context.

2- Call SetVerbose(true) if there is not a registere ExceptionHandler.
This way the exception will be logged to the JS console if it is not
being externally handled.

Interdiff:

diff -u WebCore/bindings/v8/V8NPUtils.cpp WebCore/bindings/v8/V8NPUtils.cpp
--- WebCore/bindings/v8/V8NPUtils.cpp (working copy)
+++ WebCore/bindings/v8/V8NPUtils.cpp (working copy)
@@ -152,6 +152,12 @@
     delete doomed;
 }

+ExceptionCatcher::ExceptionCatcher()
+{
+    if (!topHandler)
+	 m_tryCatch.SetVerbose(true);
+}
+
 ExceptionCatcher::~ExceptionCatcher()
 {
     if (!m_tryCatch.HasCaught())
@@ -159,8 +165,6 @@

     if (topHandler)
	 topHandler->handler(topHandler->data,
*v8::String::Utf8Value(m_tryCatch.Exception()));
-    else
-	 m_tryCatch.ReThrow();
 }

 } // namespace WebCore
diff -u WebCore/bindings/v8/V8NPUtils.h WebCore/bindings/v8/V8NPUtils.h
--- WebCore/bindings/v8/V8NPUtils.h (working copy)
+++ WebCore/bindings/v8/V8NPUtils.h (working copy)
@@ -60,6 +60,7 @@
 // current ExceptionHandler.
 class ExceptionCatcher {
 public:
+    ExceptionCatcher();
     ~ExceptionCatcher();
 private:
     v8::TryCatch m_tryCatch;