From 4b5bca14adf3033809dd7a0a8a7ba634646cf7ea Mon Sep 17 00:00:00 2001 From: Chris Dumez Date: Sat, 22 Jul 2017 17:31:57 +0000 Subject: [PATCH] REGRESSION(r204565): WKObject is broken https://bugs.webkit.org/show_bug.cgi?id=174736 Reviewed by Dan Bernstein. Source/WebKit: Revert r204565 as making WKObject a root class caused unexpected crashes. Instead, we now have WKObject inherit from NSProxy (instead of previously NSObject) and we forward calls to the target. We also need to provide an implementation for private methods such as isNSString__ to address the issue with NSStrings that r204565 was trying to fix. * Shared/Cocoa/APIObject.mm: (API::Object::unwrap): * Shared/Cocoa/WKObject.h: * Shared/Cocoa/WKObject.mm: (-[WKObject dealloc]): (-[WKObject hash]): (-[WKObject isKindOfClass:]): (-[WKObject isMemberOfClass:]): (-[WKObject respondsToSelector:]): (-[WKObject conformsToProtocol:]): (-[WKObject forwardingTargetForSelector:]): (-[WKObject description]): (-[WKObject debugDescription]): (-[WKObject classForCoder]): (-[WKObject classForKeyedArchiver]): (-[WKObject _web_createTarget]): (-[WKObject forwardInvocation:]): (-[WKObject methodSignatureForSelector:]): (-[WKObject isNSObject__]): (-[WKObject isNSArray__]): (-[WKObject isNSCFConstantString__]): (-[WKObject isNSData__]): (-[WKObject isNSDate__]): (-[WKObject isNSDictionary__]): (-[WKObject isNSNumber__]): (-[WKObject isNSOrderedSet__]): (-[WKObject isNSSet__]): (-[WKObject isNSString__]): (-[WKObject isNSTimeZone__]): (-[WKObject isNSValue__]): Tools: Add API test that used to crash. * TestWebKitAPI/Tests/WebKit2Cocoa/WKObject.mm: (TestWebKitAPI::TEST): Canonical link: https://commits.webkit.org/191573@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@219764 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Source/WebKit/ChangeLog | 47 +++++ Source/WebKit/Shared/Cocoa/APIObject.mm | 3 - Source/WebKit/Shared/Cocoa/WKObject.h | 3 +- Source/WebKit/Shared/Cocoa/WKObject.mm | 191 ++++++++++-------- Tools/ChangeLog | 13 ++ .../Tests/WebKit2Cocoa/WKObject.mm | 11 + 6 files changed, 178 insertions(+), 90 deletions(-) diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog index 92fed553b1ed..1d2d757348f2 100644 --- a/Source/WebKit/ChangeLog +++ b/Source/WebKit/ChangeLog @@ -1,3 +1,50 @@ +2017-07-22 Chris Dumez + + REGRESSION(r204565): WKObject is broken + https://bugs.webkit.org/show_bug.cgi?id=174736 + + + Reviewed by Dan Bernstein. + + Revert r204565 as making WKObject a root class caused unexpected crashes. + Instead, we now have WKObject inherit from NSProxy (instead of previously + NSObject) and we forward calls to the target. + + We also need to provide an implementation for private methods such as + isNSString__ to address the issue with NSStrings that r204565 was trying + to fix. + + * Shared/Cocoa/APIObject.mm: + (API::Object::unwrap): + * Shared/Cocoa/WKObject.h: + * Shared/Cocoa/WKObject.mm: + (-[WKObject dealloc]): + (-[WKObject hash]): + (-[WKObject isKindOfClass:]): + (-[WKObject isMemberOfClass:]): + (-[WKObject respondsToSelector:]): + (-[WKObject conformsToProtocol:]): + (-[WKObject forwardingTargetForSelector:]): + (-[WKObject description]): + (-[WKObject debugDescription]): + (-[WKObject classForCoder]): + (-[WKObject classForKeyedArchiver]): + (-[WKObject _web_createTarget]): + (-[WKObject forwardInvocation:]): + (-[WKObject methodSignatureForSelector:]): + (-[WKObject isNSObject__]): + (-[WKObject isNSArray__]): + (-[WKObject isNSCFConstantString__]): + (-[WKObject isNSData__]): + (-[WKObject isNSDate__]): + (-[WKObject isNSDictionary__]): + (-[WKObject isNSNumber__]): + (-[WKObject isNSOrderedSet__]): + (-[WKObject isNSSet__]): + (-[WKObject isNSString__]): + (-[WKObject isNSTimeZone__]): + (-[WKObject isNSValue__]): + 2017-07-21 Chris Dumez Drop IDBDatabaseException class diff --git a/Source/WebKit/Shared/Cocoa/APIObject.mm b/Source/WebKit/Shared/Cocoa/APIObject.mm index 9b0c27bc9af3..1e03cc6dfe78 100644 --- a/Source/WebKit/Shared/Cocoa/APIObject.mm +++ b/Source/WebKit/Shared/Cocoa/APIObject.mm @@ -334,9 +334,6 @@ if (!object) return nullptr; - ASSERT([(id)object conformsToProtocol:@protocol(WKObject)]); - ASSERT([(id)object respondsToSelector:@selector(_apiObject)]); - return &static_cast>(object)._apiObject; } diff --git a/Source/WebKit/Shared/Cocoa/WKObject.h b/Source/WebKit/Shared/Cocoa/WKObject.h index 0e81d0fafaeb..1beaa0a9973c 100644 --- a/Source/WebKit/Shared/Cocoa/WKObject.h +++ b/Source/WebKit/Shared/Cocoa/WKObject.h @@ -52,8 +52,7 @@ void* wrap(API::Object*); @end -NS_ROOT_CLASS -@interface WKObject +@interface WKObject : NSProxy - (NSObject *)_web_createTarget NS_RETURNS_RETAINED; diff --git a/Source/WebKit/Shared/Cocoa/WKObject.mm b/Source/WebKit/Shared/Cocoa/WKObject.mm index f4d641805f80..fa8e73505666 100644 --- a/Source/WebKit/Shared/Cocoa/WKObject.mm +++ b/Source/WebKit/Shared/Cocoa/WKObject.mm @@ -29,18 +29,33 @@ #if WK_API_ENABLED #import "APIObject.h" -#import "objcSPI.h" -#import + +@interface NSObject () +- (BOOL)isNSArray__; +- (BOOL)isNSCFConstantString__; +- (BOOL)isNSData__; +- (BOOL)isNSDate__; +- (BOOL)isNSDictionary__; +- (BOOL)isNSObject__; +- (BOOL)isNSOrderedSet__; +- (BOOL)isNSNumber__; +- (BOOL)isNSSet__; +- (BOOL)isNSString__; +- (BOOL)isNSTimeZone__; +- (BOOL)isNSValue__; +@end @implementation WKObject { - Class _isa; BOOL _hasInitializedTarget; NSObject *_target; } -+ (Class)class +- (void)dealloc { - return self; + static_cast(object_getIndexedIvars(self))->~Object(); + [_target release]; + + [super dealloc]; } static inline void initializeTargetIfNeeded(WKObject *self) @@ -52,27 +67,6 @@ static inline void initializeTargetIfNeeded(WKObject *self) self->_target = [self _web_createTarget]; } -// MARK: Methods used by the Objective-C runtime - -- (id)forwardingTargetForSelector:(SEL)selector -{ - initializeTargetIfNeeded(self); - - return _target; -} - -- (BOOL)allowsWeakReference -{ - return !_objc_rootIsDeallocating(self); -} - -- (BOOL)retainWeakReference -{ - return _objc_rootTryRetain(self); -} - -// MARK: NSObject protocol implementation - - (BOOL)isEqual:(id)object { if (object == self) @@ -90,146 +84,173 @@ - (NSUInteger)hash { initializeTargetIfNeeded(self); - return _target ? [_target hash] : reinterpret_cast(self); + return _target ? [_target hash] : [super hash]; } -- (Class)superclass +- (BOOL)isKindOfClass:(Class)aClass { initializeTargetIfNeeded(self); - return _target ? [_target superclass] : class_getSuperclass(object_getClass(self)); + return [_target isKindOfClass:aClass]; } -- (Class)class +- (BOOL)isMemberOfClass:(Class)aClass { initializeTargetIfNeeded(self); - return _target ? [_target class] : object_getClass(self); + return [_target isMemberOfClass:aClass]; } -- (instancetype)self +- (BOOL)respondsToSelector:(SEL)selector { - return self; + initializeTargetIfNeeded(self); + + return [_target respondsToSelector:selector] || [super respondsToSelector:selector]; } -- (id)performSelector:(SEL)selector +- (BOOL)conformsToProtocol:(Protocol *)protocol { - return selector ? wtfObjcMsgSend(self, selector) : nil; + initializeTargetIfNeeded(self); + + return [_target conformsToProtocol:protocol] || [super conformsToProtocol:protocol]; } -- (id)performSelector:(SEL)selector withObject:(id)object +- (id)forwardingTargetForSelector:(SEL)selector { - return selector ? wtfObjcMsgSend(self, selector, object) : nil; + initializeTargetIfNeeded(self); + + return _target; } -- (id)performSelector:(SEL)selector withObject:(id)object1 withObject:(id)object2 +- (NSString *)description { - return selector ? wtfObjcMsgSend(self, selector, object1, object2) : nil; + initializeTargetIfNeeded(self); + + return _target ? [_target description] : [super description]; } -- (BOOL)isProxy +- (NSString *)debugDescription { - return NO; + initializeTargetIfNeeded(self); + + return _target ? [_target debugDescription] : [self description]; } -- (BOOL)isKindOfClass:(Class)aClass +- (Class)classForCoder { initializeTargetIfNeeded(self); - return [_target isKindOfClass:aClass]; + return [_target classForCoder]; } -- (BOOL)isMemberOfClass:(Class)aClass +- (Class)classForKeyedArchiver { initializeTargetIfNeeded(self); - return [_target isMemberOfClass:aClass]; + return [_target classForKeyedArchiver]; } -- (BOOL)respondsToSelector:(SEL)selector +- (NSObject *)_web_createTarget +{ + return nil; +} + +- (void)forwardInvocation:(NSInvocation *)invocation { initializeTargetIfNeeded(self); - return [_target respondsToSelector:selector] || (selector && class_respondsToSelector(object_getClass(self), selector)); + [invocation invokeWithTarget:_target]; } -+ (BOOL)conformsToProtocol:(Protocol *)protocol +- (NSMethodSignature *)methodSignatureForSelector:(SEL)sel { - if (!protocol) - return NO; + initializeTargetIfNeeded(self); - for (Class cls = self; cls; cls = class_getSuperclass(cls)) { - if (class_conformsToProtocol(cls, protocol)) - return YES; - } + return [_target methodSignatureForSelector:sel]; +} - return NO; +- (BOOL)isNSObject__ +{ + initializeTargetIfNeeded(self); + + return [_target isNSObject__]; } -- (BOOL)conformsToProtocol:(Protocol *)protocol +- (BOOL)isNSArray__ { initializeTargetIfNeeded(self); - if ([_target conformsToProtocol:protocol]) - return YES; + return [_target isNSArray__]; +} - if (!protocol) - return NO; +- (BOOL)isNSCFConstantString__ +{ + initializeTargetIfNeeded(self); - for (Class cls = object_getClass(self); cls; cls = class_getSuperclass(cls)) { - if (class_conformsToProtocol(cls, protocol)) - return YES; - } + return [_target isNSCFConstantString__]; +} - return NO; +- (BOOL)isNSData__ +{ + initializeTargetIfNeeded(self); + + return [_target isNSData__]; } -- (NSString *)description +- (BOOL)isNSDate__ { initializeTargetIfNeeded(self); - return _target ? [_target description] : [NSString stringWithFormat:@"<%s %p>", class_getName(object_getClass(self)), self]; + return [_target isNSDate__]; } -- (NSString *)debugDescription +- (BOOL)isNSDictionary__ { initializeTargetIfNeeded(self); - return _target ? [_target debugDescription] : [self description]; + return [_target isNSDictionary__]; } -- (instancetype)retain +- (BOOL)isNSNumber__ { - return _objc_rootRetain(self); + initializeTargetIfNeeded(self); + + return [_target isNSNumber__]; } -- (oneway void)release +- (BOOL)isNSOrderedSet__ { - if (_objc_rootReleaseWasZero(self)) { - static_cast(object_getIndexedIvars(self))->~Object(); - [_target release]; - _objc_rootDealloc(self); - } + initializeTargetIfNeeded(self); + + return [_target isNSOrderedSet__]; } -- (instancetype)autorelease +- (BOOL)isNSSet__ { - return _objc_rootAutorelease(self); + initializeTargetIfNeeded(self); + + return [_target isNSSet__]; } -- (NSUInteger)retainCount +- (BOOL)isNSString__ { - return _objc_rootRetainCount(self); + initializeTargetIfNeeded(self); + + return [_target isNSString__]; } -- (NSZone *)zone +- (BOOL)isNSTimeZone__ { - return NSDefaultMallocZone(); + initializeTargetIfNeeded(self); + + return [_target isNSTimeZone__]; } -- (NSObject *)_web_createTarget +- (BOOL)isNSValue__ { - return nil; + initializeTargetIfNeeded(self); + + return [_target isNSValue__]; } #pragma mark WKObject protocol implementation diff --git a/Tools/ChangeLog b/Tools/ChangeLog index 2e8888450190..9eca74d1241b 100644 --- a/Tools/ChangeLog +++ b/Tools/ChangeLog @@ -1,3 +1,16 @@ +2017-07-22 Chris Dumez + + REGRESSION(r204565): WKObject is broken + https://bugs.webkit.org/show_bug.cgi?id=174736 + + + Reviewed by Dan Bernstein. + + Add API test that used to crash. + + * TestWebKitAPI/Tests/WebKit2Cocoa/WKObject.mm: + (TestWebKitAPI::TEST): + 2017-07-22 Yusuke Suzuki [WTF] Extend ThreadGroup::add results from bool to ThreadGroupAddResult diff --git a/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKObject.mm b/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKObject.mm index fabf56ca36e2..98f2b6bf6158 100644 --- a/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKObject.mm +++ b/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKObject.mm @@ -31,6 +31,7 @@ #import #import +#import namespace TestWebKitAPI { @@ -57,6 +58,16 @@ ASSERT_TRUE([wkObjectClass conformsToProtocol:@protocol(NSObject)]); } +TEST(WebKit2, WKObject_classInDictionary) +{ + Class wkObjectClass = NSClassFromString(@"WKObject"); + ASSERT_NE((Class)0, wkObjectClass); + + auto map = adoptNS([[NSMapTable alloc] initWithKeyOptions:NSMapTableStrongMemory valueOptions:NSMapTableWeakMemory capacity:0]); + [map setObject:@"test" forKey:wkObjectClass]; + ASSERT_TRUE([@"test" isEqualToString:(NSString *)[map objectForKey: wkObjectClass]]); +} + } // namespace TestWebKitAPI #endif