JSExport doesn't support constructors
authormhahnenberg@apple.com <mhahnenberg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 30 Oct 2013 17:58:12 +0000 (17:58 +0000)
committermhahnenberg@apple.com <mhahnenberg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 30 Oct 2013 17:58:12 +0000 (17:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=123380

Reviewed by Geoffrey Garen.

Support for constructor-style callbacks for the Objective-C API to JSC is currently limited to
Objective-C blocks. Any clients who try to call the constructor of a JSExport-ed Objective-C class
are met with a type error stating that it cannot be called as a constructor.

It would be nice to expand JSExport's functionality to support this idiom. It is a natural
extension to JSExport and would increase the expressiveness and simplicity in both Objective-C and
JavaScript client code.

The way we'll do this is to expand the capabilities of ObjCCallbackFunction and associated classes.
Instead of constructing a normal C API object for the constructor, we'll instead allocate a full-blown
ObjCCallbackFunction object which can already properly handle being invoked as a constructor.

* API/JSWrapperMap.mm:
(copyMethodsToObject):
(allocateConstructorForCustomClass):
(-[JSObjCClassInfo allocateConstructorAndPrototypeWithSuperClassInfo:]):
(tryUnwrapObjcObject):
* API/ObjCCallbackFunction.h:
(JSC::ObjCCallbackFunction::impl):
* API/ObjCCallbackFunction.mm:
(JSC::ObjCCallbackFunctionImpl::ObjCCallbackFunctionImpl):
(JSC::ObjCCallbackFunctionImpl::wrappedConstructor):
(JSC::ObjCCallbackFunctionImpl::isConstructible):
(JSC::ObjCCallbackFunction::getConstructData):
(JSC::ObjCCallbackFunctionImpl::name):
(JSC::ObjCCallbackFunctionImpl::call):
(objCCallbackFunctionForInvocation):
(objCCallbackFunctionForInit):
(tryUnwrapConstructor):
* API/tests/testapi.mm:
(-[TextXYZ initWithString:]):
(-[ClassA initWithA:]):
(-[ClassB initWithA:b:]):
(-[ClassC initWithA:]):
(-[ClassC initWithA:b:]):

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

Source/JavaScriptCore/API/JSWrapperMap.mm
Source/JavaScriptCore/API/ObjCCallbackFunction.h
Source/JavaScriptCore/API/ObjCCallbackFunction.mm
Source/JavaScriptCore/API/tests/testapi.mm
Source/JavaScriptCore/ChangeLog

index 9817bdb..c1df234 100644 (file)
 #import "WeakGCMap.h"
 #import <wtf/TCSpinLock.h>
 #import <wtf/Vector.h>
+#import <wtf/HashSet.h>
+
+#if OS(DARWIN)
+#include <mach-o/dyld.h>
+
+static const int32_t webkitFirstVersionWithInitConstructorSupport = 0x21A0400; // 538.4.0
+#endif
 
 @class JSObjCClassInfo;
 
@@ -163,6 +170,11 @@ static void copyMethodsToObject(JSContext *context, Class objcClass, Protocol *p
     forEachMethodInProtocol(protocol, YES, isInstanceMethod, ^(SEL sel, const char* types){
         const char* nameCStr = sel_getName(sel);
         NSString *name = @(nameCStr);
+        // Don't copy over init family methods because we handle those specially 
+        // for the purposes of hooking up the constructor correctly.
+        if ([name hasPrefix:@"init"])
+            return;
+
         if (accessorMethods && accessorMethods[name]) {
             JSObjectRef method = objCCallbackFunctionForMethod(context, objcClass, protocol, isInstanceMethod, sel, types);
             if (!method)
@@ -329,6 +341,59 @@ static void copyPrototypeProperties(JSContext *context, Class objcClass, Protoco
     [super dealloc];
 }
 
+static JSValue *allocateConstructorForCustomClass(JSContext *context, const char* className, Class cls)
+{
+#if OS(DARWIN)
+    if (NSVersionOfLinkTimeLibrary("JavaScriptCore") < webkitFirstVersionWithInitConstructorSupport)
+        return objectWithCustomBrand(context, [NSString stringWithFormat:@"%sConstructor", className], cls);
+#endif
+    // For each protocol that the class implements, gather all of the init family methods into a hash table.
+    __block HashMap<String, Protocol *> initTable;
+    Protocol *exportProtocol = getJSExportProtocol();
+    for (Class currentClass = cls; currentClass; currentClass = class_getSuperclass(currentClass)) {
+        forEachProtocolImplementingProtocol(currentClass, exportProtocol, ^(Protocol *protocol) {
+            forEachMethodInProtocol(protocol, YES, YES, ^(SEL selector, const char*) {
+                const char* name = sel_getName(selector);
+                if (![@(name) hasPrefix:@"init"])
+                    return;
+                initTable.set(name, protocol);
+            });
+        });
+    }
+
+    for (Class currentClass = cls; currentClass; currentClass = class_getSuperclass(currentClass)) {
+        __block unsigned numberOfInitsFound = 0;
+        __block SEL initMethod = 0;
+        __block Protocol *initProtocol = 0;
+        __block const char* types = 0;
+        forEachMethodInClass(currentClass, ^(Method method) {
+            SEL selector = method_getName(method);
+            const char* name = sel_getName(selector);
+            auto iter = initTable.find(name);
+
+            if (iter == initTable.end())
+                return;
+
+            numberOfInitsFound++;
+            initMethod = selector;
+            initProtocol = iter->value;
+            types = method_getTypeEncoding(method);
+        });
+
+        if (!numberOfInitsFound)
+            continue;
+
+        if (numberOfInitsFound > 1) {
+            NSLog(@"ERROR: Class %@ exported more than one init family method via JSExport. Class %@ will not have a callable JavaScript constructor function.", cls, cls);
+            break;
+        }
+
+        JSObjectRef method = objCCallbackFunctionForInit(context, cls, initProtocol, initMethod, types);
+        return [JSValue valueWithJSValueRef:method inContext:context];
+    }
+    return objectWithCustomBrand(context, [NSString stringWithFormat:@"%sConstructor", className], cls);
+}
+
 - (void)allocateConstructorAndPrototypeWithSuperClassInfo:(JSObjCClassInfo*)superClassInfo
 {
     ASSERT(!m_constructor || !m_prototype);
@@ -357,7 +422,7 @@ static void copyPrototypeProperties(JSContext *context, Class objcClass, Protoco
         if (m_constructor)
             constructor = [JSValue valueWithJSValueRef:toRef(m_constructor.get()) inContext:m_context];
         else
-            constructor = objectWithCustomBrand(m_context, [NSString stringWithFormat:@"%sConstructor", className], m_class);
+            constructor = allocateConstructorForCustomClass(m_context, className, m_class);
 
         JSContextRef cContext = [m_context JSGlobalContextRef];
         m_prototype = toJS(JSValueToObject(cContext, valueInternalValue(prototype), 0));
@@ -506,7 +571,7 @@ id tryUnwrapObjcObject(JSGlobalContextRef context, JSValueRef value)
     ASSERT(!exception);
     if (toJS(object)->inherits(JSC::JSCallbackObject<JSC::JSAPIWrapperObject>::info()))
         return (id)JSC::jsCast<JSC::JSAPIWrapperObject*>(toJS(object))->wrappedObject();
-    if (id target = tryUnwrapBlock(object))
+    if (id target = tryUnwrapConstructor(object))
         return target;
     return nil;
 }
index 5f94b35..046bf65 100644 (file)
@@ -34,8 +34,9 @@
 #if defined(__OBJC__)
 JSObjectRef objCCallbackFunctionForMethod(JSContext *, Class, Protocol *, BOOL isInstanceMethod, SEL, const char* types);
 JSObjectRef objCCallbackFunctionForBlock(JSContext *, id);
+JSObjectRef objCCallbackFunctionForInit(JSContext *, Class, Protocol *, SEL, const char* types);
 
-id tryUnwrapBlock(JSObjectRef);
+id tryUnwrapConstructor(JSObjectRef);
 #endif
 
 namespace JSC {
@@ -58,7 +59,7 @@ public:
 
     DECLARE_EXPORT_INFO;
 
-    ObjCCallbackFunctionImpl* impl() { return m_impl.get(); }
+    ObjCCallbackFunctionImpl* impl() const { return m_impl.get(); }
 
 protected:
     ObjCCallbackFunction(VM&, JSGlobalObject*, JSObjectCallAsFunctionCallback, JSObjectCallAsConstructorCallback, PassOwnPtr<ObjCCallbackFunctionImpl>);
index 58ac6a6..2203013 100644 (file)
@@ -385,6 +385,7 @@ public:
 };
 
 enum CallbackType {
+    CallbackInitMethod,
     CallbackInstanceMethod,
     CallbackClassMethod,
     CallbackBlock
@@ -401,7 +402,7 @@ public:
         , m_arguments(arguments)
         , m_result(result)
     {
-        ASSERT(type != CallbackInstanceMethod || instanceClass);
+        ASSERT((type != CallbackInstanceMethod && type != CallbackInitMethod) || instanceClass);
     }
 
     ~ObjCCallbackFunctionImpl()
@@ -416,6 +417,25 @@ public:
         return m_type == CallbackBlock ? [m_invocation target] : nil;
     }
 
+    id wrappedConstructor()
+    {
+        switch (m_type) {
+        case CallbackBlock:
+            return [m_invocation target];
+        case CallbackInitMethod:
+            return m_instanceClass;
+        default:
+            return nil;
+        }
+    }
+
+    bool isConstructible()
+    {
+        return !!wrappedBlock() || m_type == CallbackInitMethod;
+    }
+
+    String name();
+
 private:
     CallbackType m_type;
     Class m_instanceClass;
@@ -499,6 +519,7 @@ void ObjCCallbackFunction::destroy(JSCell* cell)
     static_cast<ObjCCallbackFunction*>(cell)->ObjCCallbackFunction::~ObjCCallbackFunction();
 }
 
+
 CallType ObjCCallbackFunction::getCallData(JSCell*, CallData& callData)
 {
     callData.native.function = APICallbackFunction::call<ObjCCallbackFunction>;
@@ -508,27 +529,47 @@ CallType ObjCCallbackFunction::getCallData(JSCell*, CallData& callData)
 ConstructType ObjCCallbackFunction::getConstructData(JSCell* cell, ConstructData& constructData)
 {
     ObjCCallbackFunction* callback = jsCast<ObjCCallbackFunction*>(cell);
-    if (!callback->impl()->wrappedBlock())
+    if (!callback->impl()->isConstructible())
         return Base::getConstructData(cell, constructData);
     constructData.native.function = APICallbackFunction::construct<ObjCCallbackFunction>;
     return ConstructTypeHost;
 }
 
+String ObjCCallbackFunctionImpl::name()
+{
+    if (m_type == CallbackInitMethod)
+        return class_getName(m_instanceClass);
+    return "";
+}
+
 JSValueRef ObjCCallbackFunctionImpl::call(JSContext *context, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
 {
     JSGlobalContextRef contextRef = [context JSGlobalContextRef];
 
+    id target;
     size_t firstArgument;
     switch (m_type) {
+    case CallbackInitMethod: {
+        RELEASE_ASSERT(!thisObject);
+        target = [m_instanceClass alloc];
+        if (!target || ![target isKindOfClass:m_instanceClass]) {
+            *exception = toRef(JSC::createTypeError(toJS(contextRef), "self type check failed for Objective-C instance method"));
+            return JSValueMakeUndefined(contextRef);
+        }
+        [m_invocation setTarget:target];
+        firstArgument = 2;
+        break;
+    }
     case CallbackInstanceMethod: {
-        id target = tryUnwrapObjcObject(contextRef, thisObject);
+        target = tryUnwrapObjcObject(contextRef, thisObject);
         if (!target || ![target isKindOfClass:m_instanceClass]) {
             *exception = toRef(JSC::createTypeError(toJS(contextRef), "self type check failed for Objective-C instance method"));
             return JSValueMakeUndefined(contextRef);
         }
         [m_invocation setTarget:target];
+        firstArgument = 2;
+        break;
     }
-    // fallthrough - firstArgument for CallbackInstanceMethod is also 2!
     case CallbackClassMethod:
         firstArgument = 2;
         break;
@@ -547,7 +588,18 @@ JSValueRef ObjCCallbackFunctionImpl::call(JSContext *context, JSObjectRef thisOb
 
     [m_invocation invoke];
 
-    return m_result->get(m_invocation.get(), context, exception);
+    JSValueRef result = m_result->get(m_invocation.get(), context, exception);
+
+    // Balance our call to -alloc with a call to -autorelease. We have to do this after calling -init
+    // because init family methods are allowed to release the allocated object and return something 
+    // else in its place.
+    if (m_type == CallbackInitMethod) {
+        id objcResult = tryUnwrapObjcObject(contextRef, result);
+        if (objcResult)
+            [objcResult autorelease];
+    }
+
+    return result;
 }
 
 } // namespace JSC
@@ -578,6 +630,7 @@ static JSObjectRef objCCallbackFunctionForInvocation(JSContext *context, NSInvoc
         return nil;
 
     switch (type) {
+    case CallbackInitMethod:
     case CallbackInstanceMethod:
     case CallbackClassMethod:
         // Methods are passed two implicit arguments - (id)self, and the selector.
@@ -611,7 +664,14 @@ static JSObjectRef objCCallbackFunctionForInvocation(JSContext *context, NSInvoc
     JSC::APIEntryShim shim(exec);
     OwnPtr<JSC::ObjCCallbackFunctionImpl> impl = adoptPtr(new JSC::ObjCCallbackFunctionImpl(invocation, type, instanceClass, arguments.release(), result.release()));
     // FIXME: Maybe we could support having the selector as the name of the function to make it a bit more user-friendly from the JS side?
-    return toRef(JSC::ObjCCallbackFunction::create(exec->vm(), exec->lexicalGlobalObject(), "", impl.release()));
+    return toRef(JSC::ObjCCallbackFunction::create(exec->vm(), exec->lexicalGlobalObject(), impl->name(), impl.release()));
+}
+
+JSObjectRef objCCallbackFunctionForInit(JSContext *context, Class cls, Protocol *protocol, SEL sel, const char* types)
+{
+    NSInvocation *invocation = [NSInvocation invocationWithMethodSignature:[NSMethodSignature signatureWithObjCTypes:types]];
+    [invocation setSelector:sel];
+    return objCCallbackFunctionForInvocation(context, invocation, CallbackInitMethod, cls, _protocol_getMethodTypeEncoding(protocol, sel, YES, YES));
 }
 
 JSObjectRef objCCallbackFunctionForMethod(JSContext *context, Class cls, Protocol *protocol, BOOL isInstanceMethod, SEL sel, const char* types)
@@ -636,11 +696,14 @@ JSObjectRef objCCallbackFunctionForBlock(JSContext *context, id target)
     return objCCallbackFunctionForInvocation(context, invocation, CallbackBlock, nil, signature);
 }
 
-id tryUnwrapBlock(JSObjectRef object)
+id tryUnwrapConstructor(JSObjectRef object)
 {
     if (!toJS(object)->inherits(JSC::ObjCCallbackFunction::info()))
         return nil;
-    return static_cast<JSC::ObjCCallbackFunction*>(toJS(object))->impl()->wrappedBlock();
+    JSC::ObjCCallbackFunctionImpl* impl = static_cast<JSC::ObjCCallbackFunction*>(toJS(object))->impl();
+    if (!impl->isConstructible())
+        return nil;
+    return impl->wrappedConstructor();
 }
 
 #endif
index 52858b3..3ef3eb3 100644 (file)
@@ -52,6 +52,7 @@ extern "C" void testObjectiveCAPI(void);
 @end
 
 @protocol TestObject <JSExport>
+- (id)init;
 @property int variable;
 @property (readonly) int six;
 @property CGPoint point;
@@ -103,6 +104,7 @@ JSExportAs(testArgumentTypes,
 bool testXYZTested = false;
 
 @protocol TextXYZ <JSExport>
+- (id)initWithString:(NSString*)string;
 @property int x;
 @property (readonly) int y;
 @property (assign) JSValue *onclick;
@@ -124,6 +126,16 @@ bool testXYZTested = false;
 @synthesize x;
 @synthesize y;
 @synthesize z;
+- (id)initWithString:(NSString*)string
+{
+    self = [super init];
+    if (!self)
+        return nil;
+
+    NSLog(@"%@", string);
+
+    return self;
+}
 - (void)test:(NSString *)message
 {
     testXYZTested = [message isEqual:@"test"] && x == 13 & y == 4 && z == 5;
@@ -284,6 +296,104 @@ static JSVirtualMachine *sharedInstance = nil;
 }
 @end
 
+@protocol InitA <JSExport>
+- (id)initWithA:(int)a;
+@end
+
+@protocol InitB <JSExport>
+- (id)initWithA:(int)a b:(int)b;
+@end
+
+@interface ClassA : NSObject<InitA>
+@end
+
+@interface ClassB : ClassA<InitB>
+@end
+
+@interface ClassC : ClassB<InitA, InitB>
+@end
+
+@interface ClassD : NSObject<InitA>
+- (id)initWithA:(int)a;
+@end
+
+@interface ClassE : ClassD
+- (id)initWithA:(int)a;
+@end
+
+@implementation ClassA {
+    int _a;
+}
+- (id)initWithA:(int)a
+{
+    self = [super init];
+    if (!self)
+        return nil;
+
+    _a = a;
+
+    return self;
+}
+@end
+
+@implementation ClassB {
+    int _b;
+}
+- (id)initWithA:(int)a b:(int)b
+{
+    self = [super initWithA:a];
+    if (!self)
+        return nil;
+
+    _b = b;
+
+    return self;
+}
+@end
+
+@implementation ClassC {
+    int _c;
+}
+- (id)initWithA:(int)a
+{
+    return [self initWithA:a b:0];
+}
+- (id)initWithA:(int)a b:(int)b
+{
+    self = [super initWithA:a b:b];
+    if (!self)
+        return nil;
+
+    _c = a + b;
+
+    return self;
+}
+@end
+
+@implementation ClassD
+
+- (id)initWithA:(int)a
+{
+    self = nil;
+    return [[ClassE alloc] initWithA:a];
+}
+@end
+
+@implementation ClassE {
+    int _a;
+}
+
+- (id)initWithA:(int)a
+{
+    self = [super init];
+    if (!self)
+        return nil;
+
+    _a = a;
+
+    return self;
+}
+@end
 static void checkResult(NSString *description, bool passed)
 {
     NSLog(@"TEST: \"%@\": %@", description, passed ? @"PASSED" : @"FAILED");
@@ -576,7 +686,7 @@ void testObjectiveCAPI()
         JSContext *context = [[JSContext alloc] init];
         context[@"TestObject"] = [TestObject class];
         JSValue *result = [context evaluateScript:@"String(TestObject)"];
-        checkResult(@"String(TestObject)", [result isEqualToObject:@"[object TestObjectConstructor]"]);
+        checkResult(@"String(TestObject)", [result isEqualToObject:@"function TestObject() {\n    [native code]\n}"]);
     }
 
     @autoreleasepool {
@@ -967,6 +1077,49 @@ void testObjectiveCAPI()
         checkResult(@"[JSContext currentArguments] == nil outside of callback", ![JSContext currentArguments]);
     }
 
+    @autoreleasepool {
+        JSContext *context = [[JSContext alloc] init];
+        context[@"TestObject"] = [TestObject class];
+        JSValue *testObject = [context evaluateScript:@"(new TestObject())"];
+        checkResult(@"testObject instanceof TestObject", [testObject isInstanceOf:context[@"TestObject"]]);
+
+        context[@"TextXYZ"] = [TextXYZ class];
+        JSValue *textObject = [context evaluateScript:@"(new TextXYZ(\"Called TextXYZ constructor!\"))"];
+        checkResult(@"textObject instanceof TextXYZ", [textObject isInstanceOf:context[@"TextXYZ"]]);
+    }
+
+    @autoreleasepool {
+        JSContext *context = [[JSContext alloc] init];
+        context[@"ClassA"] = [ClassA class];
+        context[@"ClassB"] = [ClassB class];
+        context[@"ClassC"] = [ClassC class]; // Should print error message about too many inits found.
+
+        JSValue *a = [context evaluateScript:@"(new ClassA(42))"];
+        checkResult(@"a instanceof ClassA", [a isInstanceOf:context[@"ClassA"]]);
+
+        JSValue *b = [context evaluateScript:@"(new ClassB(42, 53))"];
+        checkResult(@"b instanceof ClassB", [b isInstanceOf:context[@"ClassB"]]);
+
+        JSValue *canConstructClassC = [context evaluateScript:@"(function() { \
+            try { \
+                (new ClassC(1, 2)); \
+                return true; \
+            } catch(e) { \
+                return false; \
+            } \
+        })()"];
+        checkResult(@"shouldn't be able to construct ClassC", ![canConstructClassC toBool]);
+    }
+
+    @autoreleasepool {
+        JSContext *context = [[JSContext alloc] init];
+        context[@"ClassD"] = [ClassD class];
+        context[@"ClassE"] = [ClassE class];
+       
+        JSValue *d = [context evaluateScript:@"(new ClassD())"];
+        checkResult(@"Returning instance of ClassE from ClassD's init has correct class", [d isInstanceOf:context[@"ClassE"]]);
+    }
+
     currentThisInsideBlockGetterTest();
 }
 
index 5957d1f..6b97625 100644 (file)
@@ -1,3 +1,46 @@
+2013-10-25  Mark Hahnenberg  <mhahnenberg@apple.com>
+
+        JSExport doesn't support constructors
+        https://bugs.webkit.org/show_bug.cgi?id=123380
+
+        Reviewed by Geoffrey Garen.
+
+        Support for constructor-style callbacks for the Objective-C API to JSC is currently limited to 
+        Objective-C blocks. Any clients who try to call the constructor of a JSExport-ed Objective-C class 
+        are met with a type error stating that it cannot be called as a constructor.
+
+        It would be nice to expand JSExport's functionality to support this idiom. It is a natural 
+        extension to JSExport and would increase the expressiveness and simplicity in both Objective-C and 
+        JavaScript client code.
+
+        The way we'll do this is to expand the capabilities of ObjCCallbackFunction and associated classes. 
+        Instead of constructing a normal C API object for the constructor, we'll instead allocate a full-blown 
+        ObjCCallbackFunction object which can already properly handle being invoked as a constructor.
+
+        * API/JSWrapperMap.mm:
+        (copyMethodsToObject):
+        (allocateConstructorForCustomClass):
+        (-[JSObjCClassInfo allocateConstructorAndPrototypeWithSuperClassInfo:]):
+        (tryUnwrapObjcObject):
+        * API/ObjCCallbackFunction.h:
+        (JSC::ObjCCallbackFunction::impl):
+        * API/ObjCCallbackFunction.mm:
+        (JSC::ObjCCallbackFunctionImpl::ObjCCallbackFunctionImpl):
+        (JSC::ObjCCallbackFunctionImpl::wrappedConstructor):
+        (JSC::ObjCCallbackFunctionImpl::isConstructible):
+        (JSC::ObjCCallbackFunction::getConstructData):
+        (JSC::ObjCCallbackFunctionImpl::name):
+        (JSC::ObjCCallbackFunctionImpl::call):
+        (objCCallbackFunctionForInvocation):
+        (objCCallbackFunctionForInit):
+        (tryUnwrapConstructor):
+        * API/tests/testapi.mm:
+        (-[TextXYZ initWithString:]):
+        (-[ClassA initWithA:]):
+        (-[ClassB initWithA:b:]):
+        (-[ClassC initWithA:]):
+        (-[ClassC initWithA:b:]):
+
 2013-10-30  peavo@outlook.com  <peavo@outlook.com>
 
         [Win] Compile errors when enabling DFG JIT.