Changeset 207328 in webkit

Timestamp:

Oct 13, 2016 11:58:28 PM (8 years ago)

Author:

commit-queue@webkit.org

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 <​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):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• bindings/js/JSDOMBinding.h (modified) (5 diffs)
• bindings/scripts/CodeGeneratorJS.pm (modified) (2 diffs)
• bindings/scripts/test/JS/JSTestGlobalObject.cpp (modified) (1 diff)
• bindings/scripts/test/JS/JSTestObj.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/bindings/js/JSDOMBinding.h

@@ -332 +332 @@
 
 
-enum class CastedThisErrorBehavior { Throw, ReturnEarly, RejectPromise };
+enum class CastedThisErrorBehavior { Throw, ReturnEarly, RejectPromise, Assert };
 
 template<typename JSClass>
@@ -344 +344 @@
     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)
     {
@@ -350 +350 @@
         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.
@@ -365 +366 @@
         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)
@@ -372 +373 @@
             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.

trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm

@@ -3678 +3678 @@
                 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");
@@ -3685 +3687 @@
                     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");

trunk/Source/WebCore/bindings/scripts/test/JS/JSTestGlobalObject.cpp

@@ -459 +459 @@
 EncodedJSValue JSC_HOST_CALL jsTestGlobalObjectInstanceFunctionTestPrivateFunction(ExecState* state)
 {
-    return BindingCaller<JSTestGlobalObject>::callOperation<jsTestGlobalObjectInstanceFunctionTestPrivateFunctionCaller>(state, "testPrivateFunction");
+    return BindingCaller<JSTestGlobalObject>::callOperation<jsTestGlobalObjectInstanceFunctionTestPrivateFunctionCaller, CastedThisErrorBehavior::Assert>(state, "testPrivateFunction");
 }
 

trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp

@@ -5353 +5353 @@
 EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionPrivateMethod(ExecState* state)
 {
-    return BindingCaller<JSTestObj>::callOperation<jsTestObjPrototypeFunctionPrivateMethodCaller>(state, "privateMethod");
+    return BindingCaller<JSTestObj>::callOperation<jsTestObjPrototypeFunctionPrivateMethodCaller, CastedThisErrorBehavior::Assert>(state, "privateMethod");
 }