Title: [207328] trunk/Source/WebCore
Revision
207328
Author
commit-qu...@webkit.org
Date
2016-10-13 23:58:28 -0700 (Thu, 13 Oct 2016)

Log Message

Binding generated code for private operations should assert for casted-this checks
https://bugs.webkit.org/show_bug.cgi?id=163326

Patch by Youenn Fablet <you...@apple.com> on 2016-10-13
Reviewed by Darin Adler.

Covered by existing tests.

Private operations are not exposed to user scripts and are only called by built-in scripts or other WebKit-controlled code.
The call sites already ensure that the caller is of the right type so there is no need to do that work twice.

Introducing a casted-this-error Assert mode for casted-this checks, which may be reused for other binding generated code.
Updated binding generator to use that mode for private operations.

* bindings/js/JSDOMBinding.h:
(WebCore::BindingCaller::callPromiseOperation):
(WebCore::BindingCaller::callOperation):
* bindings/scripts/CodeGeneratorJS.pm:
(GenerateImplementation):
* bindings/scripts/test/JS/JSTestGlobalObject.cpp:
(WebCore::jsTestGlobalObjectInstanceFunctionTestPrivateFunction):
* bindings/scripts/test/JS/JSTestObj.cpp:
(WebCore::jsTestObjPrototypeFunctionPrivateMethod):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (207327 => 207328)


--- trunk/Source/WebCore/ChangeLog	2016-10-14 06:31:55 UTC (rev 207327)
+++ trunk/Source/WebCore/ChangeLog	2016-10-14 06:58:28 UTC (rev 207328)
@@ -1,3 +1,28 @@
+2016-10-13  Youenn Fablet  <you...@apple.com>
+
+        Binding generated code for private operations should assert for casted-this checks
+        https://bugs.webkit.org/show_bug.cgi?id=163326
+
+        Reviewed by Darin Adler.
+
+        Covered by existing tests.
+
+        Private operations are not exposed to user scripts and are only called by built-in scripts or other WebKit-controlled code.
+        The call sites already ensure that the caller is of the right type so there is no need to do that work twice.
+
+        Introducing a casted-this-error Assert mode for casted-this checks, which may be reused for other binding generated code.
+        Updated binding generator to use that mode for private operations.
+
+        * bindings/js/JSDOMBinding.h:
+        (WebCore::BindingCaller::callPromiseOperation):
+        (WebCore::BindingCaller::callOperation):
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (GenerateImplementation):
+        * bindings/scripts/test/JS/JSTestGlobalObject.cpp:
+        (WebCore::jsTestGlobalObjectInstanceFunctionTestPrivateFunction):
+        * bindings/scripts/test/JS/JSTestObj.cpp:
+        (WebCore::jsTestObjPrototypeFunctionPrivateMethod):
+
 2016-10-13  Carlos Garcia Campos  <cgar...@igalia.com>
 
         WebView and WebPage URLs not updated after URL is modified by InjectedBundlePageResourceLoadClient::willSendRequestForFrame

Modified: trunk/Source/WebCore/bindings/js/JSDOMBinding.h (207327 => 207328)


--- trunk/Source/WebCore/bindings/js/JSDOMBinding.h	2016-10-14 06:31:55 UTC (rev 207327)
+++ trunk/Source/WebCore/bindings/js/JSDOMBinding.h	2016-10-14 06:58:28 UTC (rev 207328)
@@ -331,7 +331,7 @@
 template<typename T> struct NativeValueTraits;
 
 
-enum class CastedThisErrorBehavior { Throw, ReturnEarly, RejectPromise };
+enum class CastedThisErrorBehavior { Throw, ReturnEarly, RejectPromise, Assert };
 
 template<typename JSClass>
 struct BindingCaller {
@@ -343,16 +343,17 @@
     static JSClass* castForAttribute(JSC::ExecState&, JSC::EncodedJSValue);
     static JSClass* castForOperation(JSC::ExecState&);
 
-    template<PromiseOperationCallerFunction operationCaller>
+    template<PromiseOperationCallerFunction operationCaller, CastedThisErrorBehavior shouldThrow = CastedThisErrorBehavior::RejectPromise>
     static JSC::EncodedJSValue callPromiseOperation(JSC::ExecState* state, Ref<DeferredPromise>&& promise, const char* operationName)
     {
         ASSERT(state);
         auto throwScope = DECLARE_THROW_SCOPE(state->vm());
         auto* thisObject = castForOperation(*state);
-        if (UNLIKELY(!thisObject)) {
+        if (shouldThrow != CastedThisErrorBehavior::Assert && UNLIKELY(!thisObject)) {
             ASSERT(JSClass::info());
             return rejectPromiseWithThisTypeError(promise.get(), JSClass::info()->className, operationName);
         }
+        ASSERT(thisObject);
         ASSERT_GC_OBJECT_INHERITS(thisObject, JSClass::info());
         // FIXME: We should refactor the binding generated code to use references for state and thisObject.
         return operationCaller(state, thisObject, WTFMove(promise), throwScope);
@@ -364,7 +365,7 @@
         ASSERT(state);
         auto throwScope = DECLARE_THROW_SCOPE(state->vm());
         auto* thisObject = castForOperation(*state);
-        if (UNLIKELY(!thisObject)) {
+        if (shouldThrow != CastedThisErrorBehavior::Assert && UNLIKELY(!thisObject)) {
             ASSERT(JSClass::info());
             if (shouldThrow == CastedThisErrorBehavior::Throw)
                 return throwThisTypeError(*state, throwScope, JSClass::info()->className, operationName);
@@ -371,6 +372,7 @@
             // For custom promise-returning operations
             return rejectPromiseWithThisTypeError(*state, JSClass::info()->className, operationName);
         }
+        ASSERT(thisObject);
         ASSERT_GC_OBJECT_INHERITS(thisObject, JSClass::info());
         // FIXME: We should refactor the binding generated code to use references for state and thisObject.
         return operationCaller(state, thisObject, throwScope);

Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (207327 => 207328)


--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2016-10-14 06:31:55 UTC (rev 207327)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2016-10-14 06:58:28 UTC (rev 207328)
@@ -3677,7 +3677,9 @@
             } else {
                 my $methodName = $function->signature->name;
                 if (IsReturningPromise($function) && !$isCustom) {
-                    push(@implContent, "    return BindingCaller<$className>::callPromiseOperation<${functionName}Caller>(state, WTFMove(promise), \"${methodName}\");\n");
+                    my $templateParameters = "${functionName}Caller";
+                    $templateParameters .= ", CastedThisErrorBehavior::Assert" if ($function->signature->extendedAttributes->{PrivateIdentifier} and not $function->signature->extendedAttributes->{PublicIdentifier});
+                    push(@implContent, "    return BindingCaller<$className>::callPromiseOperation<${templateParameters}>(state, WTFMove(promise), \"${methodName}\");\n");
                     push(@implContent, "}\n");
                     push(@implContent, "\n");
                     push(@implContent, "static inline JSC::EncodedJSValue ${functionName}Caller(JSC::ExecState* state, ${className}* castedThis, Ref<DeferredPromise>&& promise, JSC::ThrowScope& throwScope)\n");
@@ -3684,9 +3686,13 @@
                 } else {
                     my $classParameterType = $className eq "JSEventTarget" ? "JSEventTargetWrapper*" : "${className}*";
                     my $templateParameters = "${functionName}Caller";
-                    # FIXME: We need this specific handling for custom promise-returning functions.
-                    # It would be better to have the casted-this code calling the promise-specific code.
-                    $templateParameters .= ", CastedThisErrorBehavior::RejectPromise" if IsReturningPromise($function);
+                    if ($function->signature->extendedAttributes->{PrivateIdentifier} and not $function->signature->extendedAttributes->{PublicIdentifier}) {
+                        $templateParameters .= ", CastedThisErrorBehavior::Assert";
+                    } elsif (IsReturningPromise($function)) {
+                        # FIXME: We need this specific handling for custom promise-returning functions.
+                        # It would be better to have the casted-this code calling the promise-specific code.
+                        $templateParameters .= ", CastedThisErrorBehavior::RejectPromise" if IsReturningPromise($function);
+                    }
 
                     push(@implContent, "    return BindingCaller<$className>::callOperation<${templateParameters}>(state, \"${methodName}\");\n");
                     push(@implContent, "}\n");

Modified: trunk/Source/WebCore/bindings/scripts/test/JS/JSTestGlobalObject.cpp (207327 => 207328)


--- trunk/Source/WebCore/bindings/scripts/test/JS/JSTestGlobalObject.cpp	2016-10-14 06:31:55 UTC (rev 207327)
+++ trunk/Source/WebCore/bindings/scripts/test/JS/JSTestGlobalObject.cpp	2016-10-14 06:58:28 UTC (rev 207328)
@@ -458,7 +458,7 @@
 
 EncodedJSValue JSC_HOST_CALL jsTestGlobalObjectInstanceFunctionTestPrivateFunction(ExecState* state)
 {
-    return BindingCaller<JSTestGlobalObject>::callOperation<jsTestGlobalObjectInstanceFunctionTestPrivateFunctionCaller>(state, "testPrivateFunction");
+    return BindingCaller<JSTestGlobalObject>::callOperation<jsTestGlobalObjectInstanceFunctionTestPrivateFunctionCaller, CastedThisErrorBehavior::Assert>(state, "testPrivateFunction");
 }
 
 static inline JSC::EncodedJSValue jsTestGlobalObjectInstanceFunctionTestPrivateFunctionCaller(JSC::ExecState* state, JSTestGlobalObject* castedThis, JSC::ThrowScope& throwScope)

Modified: trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp (207327 => 207328)


--- trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp	2016-10-14 06:31:55 UTC (rev 207327)
+++ trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp	2016-10-14 06:58:28 UTC (rev 207328)
@@ -5352,7 +5352,7 @@
 
 EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionPrivateMethod(ExecState* state)
 {
-    return BindingCaller<JSTestObj>::callOperation<jsTestObjPrototypeFunctionPrivateMethodCaller>(state, "privateMethod");
+    return BindingCaller<JSTestObj>::callOperation<jsTestObjPrototypeFunctionPrivateMethodCaller, CastedThisErrorBehavior::Assert>(state, "privateMethod");
 }
 
 static inline JSC::EncodedJSValue jsTestObjPrototypeFunctionPrivateMethodCaller(JSC::ExecState* state, JSTestObj* castedThis, JSC::ThrowScope& throwScope)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to