- Revision
- 158762
- Author
- [email protected]
- Date
- 2013-11-06 11:21:47 -0800 (Wed, 06 Nov 2013)
Log Message
JSExport doesn't support constructors
https://bugs.webkit.org/show_bug.cgi?id=123380
Reviewed by Geoffrey Garen.
Needed another linked-on-or-after check for when we're deciding whether
we should copy over init family methods.
Factored out the link time checks into a separate function so that they can be cached.
Factored out the check for init-family method selectors into helper function and changed it to
match the description in the clang docs, namely that there can be underscores at the beginning
and the first letter after 'init' part of the selector (if there is one) must be a capital letter.
Updated tests to make sure we don't treat "initialize" as an init-family method and that we do
treat "_init" as an init-family method.
* API/JSWrapperMap.h:
* API/JSWrapperMap.mm:
(isInitFamilyMethod):
(shouldSkipMethodWithName):
(copyMethodsToObject):
(allocateConstructorForCustomClass):
(supportsInitMethodConstructors):
* API/tests/testapi.mm:
(-[ClassA initialize]):
(-[ClassD initialize]):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/API/JSWrapperMap.h (158761 => 158762)
--- trunk/Source/_javascript_Core/API/JSWrapperMap.h 2013-11-06 19:01:30 UTC (rev 158761)
+++ trunk/Source/_javascript_Core/API/JSWrapperMap.h 2013-11-06 19:21:47 UTC (rev 158762)
@@ -41,6 +41,7 @@
id tryUnwrapObjcObject(JSGlobalContextRef, JSValueRef);
+bool supportsInitMethodConstructors();
Protocol *getJSExportProtocol();
Class getNSBlockClass();
Modified: trunk/Source/_javascript_Core/API/JSWrapperMap.mm (158761 => 158762)
--- trunk/Source/_javascript_Core/API/JSWrapperMap.mm 2013-11-06 19:01:30 UTC (rev 158761)
+++ trunk/Source/_javascript_Core/API/JSWrapperMap.mm 2013-11-06 19:21:47 UTC (rev 158762)
@@ -42,11 +42,9 @@
#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;
@@ -159,6 +157,44 @@
}];
}
+static bool isInitFamilyMethod(NSString *name)
+{
+ NSUInteger i = 0;
+
+ // Skip over initial underscores.
+ for (; i < [name length]; ++i) {
+ if ([name characterAtIndex:i] != '_')
+ break;
+ }
+
+ // Match 'init'.
+ NSUInteger initIndex = 0;
+ NSString* init = @"init";
+ for (; i < [name length] && initIndex < [init length]; ++i, ++initIndex) {
+ if ([name characterAtIndex:i] != [init characterAtIndex:initIndex])
+ return false;
+ }
+
+ // We didn't match all of 'init'.
+ if (initIndex < [init length])
+ return false;
+
+ // If we're at the end or the next character is a capital letter then this is an init-family selector.
+ return i == [name length] || [[NSCharacterSet uppercaseLetterCharacterSet] characterIsMember:[name characterAtIndex:i]];
+}
+
+static bool shouldSkipMethodWithName(NSString *name)
+{
+ // For clients that don't support init-based constructors just copy
+ // over the init method as we would have before.
+ if (!supportsInitMethodConstructors())
+ return false;
+
+ // Skip over init family methods because we handle those specially
+ // for the purposes of hooking up the constructor correctly.
+ return isInitFamilyMethod(name);
+}
+
// This method will iterate over the set of required methods in the protocol, and:
// * Determine a property name (either via a renameMap or default conversion).
// * If an accessorMap is provided, and contains this name, store the method in the map.
@@ -170,9 +206,8 @@
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"])
+
+ if (shouldSkipMethodWithName(name))
return;
if (accessorMethods && accessorMethods[name]) {
@@ -343,10 +378,9 @@
static JSValue *allocateConstructorForCustomClass(JSContext *context, const char* className, Class cls)
{
-#if OS(DARWIN)
- if (NSVersionOfLinkTimeLibrary("_javascript_Core") < webkitFirstVersionWithInitConstructorSupport)
+ if (!supportsInitMethodConstructors())
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();
@@ -354,7 +388,7 @@
forEachProtocolImplementingProtocol(currentClass, exportProtocol, ^(Protocol *protocol) {
forEachMethodInProtocol(protocol, YES, YES, ^(SEL selector, const char*) {
const char* name = sel_getName(selector);
- if (![@(name) hasPrefix:@"init"])
+ if (!isInitFamilyMethod(@(name)))
return;
initTable.set(name, protocol);
});
@@ -582,6 +616,14 @@
@implementation JSExport
@end
+bool supportsInitMethodConstructors()
+{
+ static int32_t versionOfLinkTimeLibrary = 0;
+ if (!versionOfLinkTimeLibrary)
+ versionOfLinkTimeLibrary = NSVersionOfLinkTimeLibrary("_javascript_Core");
+ return versionOfLinkTimeLibrary >= webkitFirstVersionWithInitConstructorSupport;
+}
+
Protocol *getJSExportProtocol()
{
static Protocol *protocol = objc_getProtocol("JSExport");
Modified: trunk/Source/_javascript_Core/API/tests/testapi.mm (158761 => 158762)
--- trunk/Source/_javascript_Core/API/tests/testapi.mm 2013-11-06 19:01:30 UTC (rev 158761)
+++ trunk/Source/_javascript_Core/API/tests/testapi.mm 2013-11-06 19:21:47 UTC (rev 158762)
@@ -298,12 +298,17 @@
@protocol InitA <JSExport>
- (id)initWithA:(int)a;
+- (int)initialize;
@end
@protocol InitB <JSExport>
- (id)initWithA:(int)a b:(int)b;
@end
+@protocol InitC <JSExport>
+- (id)_init;
+@end
+
@interface ClassA : NSObject<InitA>
@end
@@ -313,6 +318,9 @@
@interface ClassC : ClassB<InitA, InitB>
@end
+@interface ClassCPrime : ClassB<InitA, InitC>
+@end
+
@interface ClassD : NSObject<InitA>
- (id)initWithA:(int)a;
@end
@@ -334,6 +342,10 @@
return self;
}
+- (int)initialize
+{
+ return 42;
+}
@end
@implementation ClassB {
@@ -370,6 +382,20 @@
}
@end
+@implementation ClassCPrime
+- (id)initWithA:(int)a
+{
+ self = [super initWithA:a b:0];
+ if (!self)
+ return nil;
+ return self;
+}
+- (id)_init
+{
+ return [self initWithA:42];
+}
+@end
+
@implementation ClassD
- (id)initWithA:(int)a
@@ -377,6 +403,10 @@
self = nil;
return [[ClassE alloc] initWithA:a];
}
+- (int)initialize
+{
+ return 0;
+}
@end
@implementation ClassE {
@@ -1093,9 +1123,11 @@
context[@"ClassA"] = [ClassA class];
context[@"ClassB"] = [ClassB class];
context[@"ClassC"] = [ClassC class]; // Should print error message about too many inits found.
+ context[@"ClassCPrime"] = [ClassCPrime class]; // Ditto.
JSValue *a = [context evaluateScript:@"(new ClassA(42))"];
checkResult(@"a instanceof ClassA", [a isInstanceOf:context[@"ClassA"]]);
+ checkResult(@"a.initialize() is callable", [[a invokeMethod:@"initialize" withArguments:@[]] toInt32] == 42);
JSValue *b = [context evaluateScript:@"(new ClassB(42, 53))"];
checkResult(@"b instanceof ClassB", [b isInstanceOf:context[@"ClassB"]]);
@@ -1109,6 +1141,15 @@
} \
})()"];
checkResult(@"shouldn't be able to construct ClassC", ![canConstructClassC toBool]);
+ JSValue *canConstructClassCPrime = [context evaluateScript:@"(function() { \
+ try { \
+ (new ClassCPrime(1)); \
+ return true; \
+ } catch(e) { \
+ return false; \
+ } \
+ })()"];
+ checkResult(@"shouldn't be able to construct ClassCPrime", ![canConstructClassCPrime toBool]);
}
@autoreleasepool {
Modified: trunk/Source/_javascript_Core/ChangeLog (158761 => 158762)
--- trunk/Source/_javascript_Core/ChangeLog 2013-11-06 19:01:30 UTC (rev 158761)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-11-06 19:21:47 UTC (rev 158762)
@@ -1,3 +1,33 @@
+2013-11-06 Mark Hahnenberg <[email protected]>
+
+ JSExport doesn't support constructors
+ https://bugs.webkit.org/show_bug.cgi?id=123380
+
+ Reviewed by Geoffrey Garen.
+
+ Needed another linked-on-or-after check for when we're deciding whether
+ we should copy over init family methods.
+
+ Factored out the link time checks into a separate function so that they can be cached.
+
+ Factored out the check for init-family method selectors into helper function and changed it to
+ match the description in the clang docs, namely that there can be underscores at the beginning
+ and the first letter after 'init' part of the selector (if there is one) must be a capital letter.
+
+ Updated tests to make sure we don't treat "initialize" as an init-family method and that we do
+ treat "_init" as an init-family method.
+
+ * API/JSWrapperMap.h:
+ * API/JSWrapperMap.mm:
+ (isInitFamilyMethod):
+ (shouldSkipMethodWithName):
+ (copyMethodsToObject):
+ (allocateConstructorForCustomClass):
+ (supportsInitMethodConstructors):
+ * API/tests/testapi.mm:
+ (-[ClassA initialize]):
+ (-[ClassD initialize]):
+
2013-11-06 Michael Saboff <[email protected]>
Change ctiTrampoline into a thunk
