Title: [189140] trunk/Source/WebCore
Revision
189140
Author
cdu...@apple.com
Date
2015-08-28 19:31:23 -0700 (Fri, 28 Aug 2015)

Log Message

JSCallbackData::invokeCallback() should return the Exception to the caller
https://bugs.webkit.org/show_bug.cgi?id=148591

Reviewed by Mark Lam.

JSCallbackData::invokeCallback() calls a callback function. If this
function throws an exception, it will report it and clear it on the VM.
However, in the case of NodeFilter, the DOM specification clearly states
that we are supposed to rethrow the exception [1].

Once way to support this is to have JSCallbackData::invokeCallback()
return the Exception to the caller and let the caller decide what to do
with it (i.e. report it or rethrow it).

There is no actual behavior change in this patch. This slight
refactoring is in preparation for Bug 148415.

[1] https://dom.spec.whatwg.org/#traversal

Some more context at:
https://www.w3.org/Bugs/Public/show_bug.cgi?id=17713#c16

* bindings/js/JSCallbackData.cpp:
(WebCore::JSCallbackData::invokeCallback):
* bindings/js/JSCallbackData.h:
* bindings/js/JSCustomSQLStatementErrorCallback.cpp:
(WebCore::JSSQLStatementErrorCallback::handleEvent):
* bindings/scripts/CodeGeneratorJS.pm:
(GenerateCallbackImplementation):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (189139 => 189140)


--- trunk/Source/WebCore/ChangeLog	2015-08-29 02:28:51 UTC (rev 189139)
+++ trunk/Source/WebCore/ChangeLog	2015-08-29 02:31:23 UTC (rev 189140)
@@ -1,3 +1,35 @@
+2015-08-28  Chris Dumez  <cdu...@apple.com>
+
+        JSCallbackData::invokeCallback() should return the Exception to the caller
+        https://bugs.webkit.org/show_bug.cgi?id=148591
+
+        Reviewed by Mark Lam.
+
+        JSCallbackData::invokeCallback() calls a callback function. If this
+        function throws an exception, it will report it and clear it on the VM.
+        However, in the case of NodeFilter, the DOM specification clearly states
+        that we are supposed to rethrow the exception [1].
+
+        Once way to support this is to have JSCallbackData::invokeCallback()
+        return the Exception to the caller and let the caller decide what to do
+        with it (i.e. report it or rethrow it).
+
+        There is no actual behavior change in this patch. This slight
+        refactoring is in preparation for Bug 148415.
+
+        [1] https://dom.spec.whatwg.org/#traversal
+
+        Some more context at:
+        https://www.w3.org/Bugs/Public/show_bug.cgi?id=17713#c16
+
+        * bindings/js/JSCallbackData.cpp:
+        (WebCore::JSCallbackData::invokeCallback):
+        * bindings/js/JSCallbackData.h:
+        * bindings/js/JSCustomSQLStatementErrorCallback.cpp:
+        (WebCore::JSSQLStatementErrorCallback::handleEvent):
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (GenerateCallbackImplementation):
+
 2015-08-28  Bem Jones-Bey  <bjone...@adobe.com>
 
         [CSS Shapes] Remove unused CSSBasicShape::m_referenceBox

Modified: trunk/Source/WebCore/bindings/js/JSCallbackData.cpp (189139 => 189140)


--- trunk/Source/WebCore/bindings/js/JSCallbackData.cpp	2015-08-29 02:28:51 UTC (rev 189139)
+++ trunk/Source/WebCore/bindings/js/JSCallbackData.cpp	2015-08-29 02:31:23 UTC (rev 189140)
@@ -43,13 +43,13 @@
     delete static_cast<JSCallbackData*>(context);
 }
 
-JSValue JSCallbackData::invokeCallback(MarkedArgumentBuffer& args, CallbackType method, PropertyName functionName, bool* raisedException)
+JSValue JSCallbackData::invokeCallback(MarkedArgumentBuffer& args, CallbackType method, PropertyName functionName, NakedPtr<Exception>& returnedException)
 {
     ASSERT(callback());
-    return invokeCallback(callback(), args, method, functionName, raisedException);
+    return invokeCallback(callback(), args, method, functionName, returnedException);
 }
 
-JSValue JSCallbackData::invokeCallback(JSValue thisValue, MarkedArgumentBuffer& args, CallbackType method, PropertyName functionName, bool* raisedException)
+JSValue JSCallbackData::invokeCallback(JSValue thisValue, MarkedArgumentBuffer& args, CallbackType method, PropertyName functionName, NakedPtr<Exception>& returnedException)
 {
     ASSERT(callback());
     ASSERT(globalObject());
@@ -84,20 +84,13 @@
 
     InspectorInstrumentationCookie cookie = JSMainThreadExecState::instrumentFunctionCall(context, callType, callData);
 
-    NakedPtr<Exception> exception;
+    returnedException = nullptr;
     JSValue result = context->isDocument()
-        ? JSMainThreadExecState::call(exec, function, callType, callData, thisValue, args, exception)
-        : JSC::call(exec, function, callType, callData, thisValue, args, exception);
+        ? JSMainThreadExecState::call(exec, function, callType, callData, thisValue, args, returnedException)
+        : JSC::call(exec, function, callType, callData, thisValue, args, returnedException);
 
     InspectorInstrumentation::didCallFunction(cookie, context);
 
-    if (exception) {
-        reportException(exec, exception);
-        if (raisedException)
-            *raisedException = true;
-        return result;
-    }
-
     return result;
 }
 

Modified: trunk/Source/WebCore/bindings/js/JSCallbackData.h (189139 => 189140)


--- trunk/Source/WebCore/bindings/js/JSCallbackData.h	2015-08-29 02:28:51 UTC (rev 189139)
+++ trunk/Source/WebCore/bindings/js/JSCallbackData.h	2015-08-29 02:31:23 UTC (rev 189140)
@@ -67,8 +67,8 @@
     JSDOMGlobalObject* globalObject() { return m_globalObject.get(); }
     
     enum class CallbackType { Function, Object, FunctionOrObject };
-    JSC::JSValue invokeCallback(JSC::MarkedArgumentBuffer&, CallbackType, JSC::PropertyName functionName, bool* raisedException = nullptr);
-    JSC::JSValue invokeCallback(JSC::JSValue thisValue, JSC::MarkedArgumentBuffer&, CallbackType, JSC::PropertyName functionName, bool* raisedException = nullptr);
+    JSC::JSValue invokeCallback(JSC::MarkedArgumentBuffer&, CallbackType, JSC::PropertyName functionName, NakedPtr<JSC::Exception>& returnedException);
+    JSC::JSValue invokeCallback(JSC::JSValue thisValue, JSC::MarkedArgumentBuffer&, CallbackType, JSC::PropertyName functionName, NakedPtr<JSC::Exception>& returnedException);
 
 private:
     JSC::Strong<JSC::JSObject> m_callback;

Modified: trunk/Source/WebCore/bindings/js/JSCustomSQLStatementErrorCallback.cpp (189139 => 189140)


--- trunk/Source/WebCore/bindings/js/JSCustomSQLStatementErrorCallback.cpp	2015-08-29 02:28:51 UTC (rev 189139)
+++ trunk/Source/WebCore/bindings/js/JSCustomSQLStatementErrorCallback.cpp	2015-08-29 02:31:23 UTC (rev 189140)
@@ -32,6 +32,7 @@
 #include "JSSQLError.h"
 #include "JSSQLTransaction.h"
 #include "ScriptExecutionContext.h"
+#include <runtime/Exception.h>
 #include <runtime/JSLock.h>
 #include <wtf/Ref.h>
 
@@ -53,9 +54,11 @@
     args.append(toJS(exec, m_data->globalObject(), transaction));
     args.append(toJS(exec, m_data->globalObject(), error));
 
-    bool raisedException = false;
-    JSValue result = m_data->invokeCallback(args, JSCallbackData::CallbackType::Function, Identifier(), &raisedException);
-    if (raisedException) {
+    NakedPtr<Exception> returnedException;
+    JSValue result = m_data->invokeCallback(args, JSCallbackData::CallbackType::Function, Identifier(), returnedException);
+    if (returnedException) {
+        reportException(exec, returnedException);
+
         // The spec says:
         // "If the error callback returns false, then move on to the next statement..."
         // "Otherwise, the error callback did not return false, or there was no error callback"

Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (189139 => 189140)


--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2015-08-29 02:28:51 UTC (rev 189139)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2015-08-29 02:31:23 UTC (rev 189140)
@@ -3662,7 +3662,7 @@
                 push(@implContent, "    args.append(" . NativeToJSValue($param, 1, $interfaceName, $paramName, "m_data") . ");\n");
             }
 
-            push(@implContent, "\n    bool raisedException = false;\n");
+            push(@implContent, "\n    NakedPtr<Exception> returnedException;\n");
 
             my $propertyToLookup = "Identifier::fromString(exec, \"${functionName}\")";
             my $invokeMethod = "JSCallbackData::CallbackType::FunctionOrObject";
@@ -3678,8 +3678,13 @@
                 # https://heycam.github.io/webidl/#es-user-objects
                 $invokeMethod = "JSCallbackData::CallbackType::Object";
             }
-            push(@implContent, "    m_data->invokeCallback(args, $invokeMethod, $propertyToLookup, &raisedException);\n");
-            push(@implContent, "    return !raisedException;\n");
+            push(@implContent, "    m_data->invokeCallback(args, $invokeMethod, $propertyToLookup, returnedException);\n");
+
+            # FIXME: We currently just report the exception. We should probably add an extended attribute to indicate when
+            # we want the exception to be rethrown instead.
+            push(@implContent, "    if (returnedException)\n");
+            push(@implContent, "        reportException(exec, returnedException);\n");
+            push(@implContent, "    return !returnedException;\n");
             push(@implContent, "}\n");
         }
     }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to