REGRESSION(r204565): WKObject is broken

https://bugs.webkit.org/show_bug.cgi?id=174736 <rdar://problem/33246169> 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
WebKit · Jul 22, 2017 · 4b5bca1 · 4b5bca1
1 parent db0f4c3
commit 4b5bca1
Show file tree

Hide file tree

Showing 6 changed files with 178 additions and 90 deletions.
diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
@@ -1,3 +1,50 @@
+2017-07-22  Chris Dumez  <cdumez@apple.com>
+
+        REGRESSION(r204565): WKObject is broken
+        https://bugs.webkit.org/show_bug.cgi?id=174736
+        <rdar://problem/33246169>
+
+        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  <cdumez@apple.com>
 
         Drop IDBDatabaseException class

diff --git 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<id <WKObject>>(object)._apiObject;
 }
 

diff --git 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 <WKObject>
+@interface WKObject : NSProxy <WKObject>
 
 - (NSObject *)_web_createTarget NS_RETURNS_RETAINED;
 

diff --git 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 <wtf/ObjcRuntimeExtras.h>
+
+@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<API::Object*>(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<NSUInteger>(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<id>(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<id>(self, selector, object) : nil;
+    initializeTargetIfNeeded(self);
+
+    return _target;
 }
 
-- (id)performSelector:(SEL)selector withObject:(id)object1 withObject:(id)object2
+- (NSString *)description
 {
-    return selector ? wtfObjcMsgSend<id>(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<API::Object*>(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
@@ -1,3 +1,16 @@
+2017-07-22  Chris Dumez  <cdumez@apple.com>
+
+        REGRESSION(r204565): WKObject is broken
+        https://bugs.webkit.org/show_bug.cgi?id=174736
+        <rdar://problem/33246169>
+
+        Reviewed by Dan Bernstein.
+
+        Add API test that used to crash.
+
+        * TestWebKitAPI/Tests/WebKit2Cocoa/WKObject.mm:
+        (TestWebKitAPI::TEST):
+
 2017-07-22  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [WTF] Extend ThreadGroup::add results from bool to ThreadGroupAddResult