Changeset 244999 in webkit


Ignore:
Timestamp:
May 6, 2019 6:23:39 PM (5 years ago)
Author:
keith_miller@apple.com
Message:

JSWrapperMap should check if existing prototype properties are wrappers when copying exported methods.
https://bugs.webkit.org/show_bug.cgi?id=197324
<rdar://problem/50253144>

Reviewed by Saam Barati.

The current implementation prevents using JSExport to shadow a
method from a super class. This was because we would only add a
method if the prototype didn't already claim to have the
property. Normally this would only happen if an Objective-C super
class already exported a ObjCCallbackFunction for the method,
however, if the user exports a property that is already on
Object.prototype the overriden method won't be exported.

This patch fixes the object prototype issue by checking if the
property on the prototype chain is an ObjCCallbackFunction, if
it's not then it adds an override.

  • API/JSWrapperMap.mm:

(copyMethodsToObject):

  • API/tests/testapi.mm:

(-[ToStringClass toString]):
(-[ToStringClass other]):
(-[ToStringSubclass toString]):
(-[ToStringSubclassNoProtocol toString]):
(testToString):
(testObjectiveCAPI):

Location:
trunk/Source/JavaScriptCore
Files:
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/API/JSWrapperMap.mm

    r244323 r244999  
    269269            if (!name)
    270270                name = selectorToPropertyName(nameCStr);
    271             if ([object hasProperty:name])
     271            auto exec = toJS([context JSGlobalContextRef]);
     272            JSValue *existingMethod = object[name];
     273            // ObjCCallbackFunction does a dynamic lookup for the
     274            // selector before calling the method. In order to save
     275            // memory we only put one callback object in any givin
     276            // prototype chain. However, to handle the client trying
     277            // to override normal builtins e.g. "toString" we check if
     278            // the existing value on the prototype chain is an ObjC
     279            // callback already.
     280            if ([existingMethod isObject] && JSC::jsDynamicCast<JSC::ObjCCallbackFunction*>(exec->vm(), toJS(exec, [existingMethod JSValueRef])))
    272281                return;
    273282            JSObjectRef method = objCCallbackFunctionForMethod(context, objcClass, protocol, isInstanceMethod, sel, types);
  • trunk/Source/JavaScriptCore/API/tests/testapi.mm

    r244620 r244999  
    25362536}
    25372537
     2538
     2539@protocol ToString <JSExport>
     2540- (NSString *)toString;
     2541@end
     2542
     2543@interface ToStringClass : NSObject<ToString>
     2544@end
     2545
     2546@implementation ToStringClass
     2547- (NSString *)toString
     2548{
     2549    return @"foo";
     2550}
     2551@end
     2552
     2553@interface ToStringSubclass : ToStringClass<ToString>
     2554@end
     2555
     2556@implementation ToStringSubclass
     2557- (NSString *)toString
     2558{
     2559    return @"baz";
     2560}
     2561@end
     2562
     2563@interface ToStringSubclassNoProtocol : ToStringClass
     2564@end
     2565
     2566@implementation ToStringSubclassNoProtocol
     2567- (NSString *)toString
     2568{
     2569    return @"baz";
     2570}
     2571@end
     2572
     2573static void testToString()
     2574{
     2575    @autoreleasepool {
     2576        JSContext *context = [[JSContext alloc] init];
     2577
     2578        JSValue *toStringClass = [JSValue valueWithObject:[[ToStringClass alloc] init] inContext:context];
     2579        checkResult(@"exporting a property with the same name as a builtin on Object.prototype should still be exported", [[toStringClass invokeMethod:@"toString" withArguments:@[]] isEqualToObject:@"foo"]);
     2580        checkResult(@"converting an object with an exported custom toObject property should use that method", [[toStringClass toString] isEqualToString:@"foo"]);
     2581
     2582        toStringClass = [JSValue valueWithObject:[[ToStringSubclass alloc] init] inContext:context];
     2583        checkResult(@"Calling a method on a derived class should call the derived implementation", [[toStringClass invokeMethod:@"toString" withArguments:@[]] isEqualToObject:@"baz"]);
     2584        checkResult(@"Converting an object with an exported custom toObject property should use that method", [[toStringClass toString] isEqualToString:@"baz"]);
     2585        context[@"toStringValue"] = toStringClass;
     2586        JSValue *hasOwnProperty = [context evaluateScript:@"toStringValue.__proto__.hasOwnProperty('toString')"];
     2587        checkResult(@"A subclass that exports a method exported by a super class shouldn't have a duplicate prototype method", [hasOwnProperty toBool]);
     2588
     2589        toStringClass = [JSValue valueWithObject:[[ToStringSubclassNoProtocol alloc] init] inContext:context];
     2590        checkResult(@"Calling a method on a derived class should call the derived implementation even when not exported on the derived class", [[toStringClass invokeMethod:@"toString" withArguments:@[]] isEqualToObject:@"baz"]);
     2591    }
     2592}
     2593
    25382594#define RUN(test) do { \
    25392595        if (!shouldRun(#test)) \
     
    25562612    RUN(checkNegativeNSIntegers());
    25572613    RUN(runJITThreadLimitTests());
     2614    RUN(testToString());
    25582615
    25592616    RUN(testLoaderResolvesAbsoluteScriptURL());
  • trunk/Source/JavaScriptCore/ChangeLog

    r244996 r244999  
     12019-05-06  Keith Miller  <keith_miller@apple.com>
     2
     3        JSWrapperMap should check if existing prototype properties are wrappers when copying exported methods.
     4        https://bugs.webkit.org/show_bug.cgi?id=197324
     5        <rdar://problem/50253144>
     6
     7        Reviewed by Saam Barati.
     8
     9        The current implementation prevents using JSExport to shadow a
     10        method from a super class. This was because we would only add a
     11        method if the prototype didn't already claim to have the
     12        property. Normally this would only happen if an Objective-C super
     13        class already exported a ObjCCallbackFunction for the method,
     14        however, if the user exports a property that is already on
     15        Object.prototype the overriden method won't be exported.
     16
     17        This patch fixes the object prototype issue by checking if the
     18        property on the prototype chain is an ObjCCallbackFunction, if
     19        it's not then it adds an override.
     20
     21        * API/JSWrapperMap.mm:
     22        (copyMethodsToObject):
     23        * API/tests/testapi.mm:
     24        (-[ToStringClass toString]):
     25        (-[ToStringClass other]):
     26        (-[ToStringSubclass toString]):
     27        (-[ToStringSubclassNoProtocol toString]):
     28        (testToString):
     29        (testObjectiveCAPI):
     30
    1312019-05-06  Yusuke Suzuki  <ysuzuki@apple.com>
    232
Note: See TracChangeset for help on using the changeset viewer.