Binding generated code for private operations should assert for casted-this checks
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Oct 2016 06:58:28 +0000 (06:58 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Oct 2016 06:58:28 +0000 (06:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=163326

Patch by Youenn Fablet <youenn@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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@207328 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSDOMBinding.h
Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
Source/WebCore/bindings/scripts/test/JS/JSTestGlobalObject.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp

index aaaaed6..927b054 100644 (file)
@@ -1,3 +1,28 @@
+2016-10-13  Youenn Fablet  <youenn@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  <cgarcia@igalia.com>
 
         WebView and WebPage URLs not updated after URL is modified by InjectedBundlePageResourceLoadClient::willSendRequestForFrame
index be4abc3..83d3061 100644 (file)
@@ -331,7 +331,7 @@ template<JSC::NativeFunction, int length> JSC::EncodedJSValue nonCachingStaticFu
 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 @@ struct BindingCaller {
     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,13 +365,14 @@ struct BindingCaller {
         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);
             // 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);
index 5ad98e9..35bd09c 100644 (file)
@@ -3677,16 +3677,22 @@ END
             } 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");
                 } 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");
index cff6cfa..6e57202 100644 (file)
@@ -458,7 +458,7 @@ static inline JSC::EncodedJSValue jsTestGlobalObjectInstanceFunctionTestPrivateF
 
 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)
index 9763c6e..e9e138d 100644 (file)
@@ -5352,7 +5352,7 @@ static inline JSC::EncodedJSValue jsTestObjPrototypeFunctionPrivateMethodCaller(
 
 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)