Title: [192162] trunk/Source/WebKit2
Revision
192162
Author
ander...@apple.com
Date
2015-11-09 10:35:19 -0800 (Mon, 09 Nov 2015)

Log Message

Rework the way allowed argument classes are stored
https://bugs.webkit.org/show_bug.cgi?id=150992

Reviewed by Darin Adler.

Add a separate MethodInfo class so we have someplace to store the reply block arguments.
Use HashSet<Class> instead of NSSet. No functionality change intended.

* Shared/API/Cocoa/WKRemoteObjectCoder.mm:
(-[WKRemoteObjectDecoder decodeValueOfObjCType:at:]):
(decodeObjectFromObjectStream):
(checkIfClassIsAllowed):
(decodeInvocationArguments):
(decodeObject):
(-[WKRemoteObjectDecoder decodeObjectOfClasses:forKey:]):
(-[WKRemoteObjectDecoder allowedClasses]):
* Shared/API/Cocoa/_WKRemoteObjectInterface.mm:
(isContainerClass):
(propertyListClasses):
(initializeMethod):
(initializeMethods):
(-[_WKRemoteObjectInterface initWithProtocol:identifier:]):
(classesForSelectorArgument):
(-[_WKRemoteObjectInterface classesForSelector:argumentIndex:]):
(-[_WKRemoteObjectInterface setClasses:forSelector:argumentIndex:]):
(-[_WKRemoteObjectInterface _allowedArgumentClassesForSelector:]):
(allowedArgumentClassesForMethod): Deleted.
(initializeAllowedArgumentClasses): Deleted.
* Shared/API/Cocoa/_WKRemoteObjectInterfaceInternal.h:

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (192161 => 192162)


--- trunk/Source/WebKit2/ChangeLog	2015-11-09 18:10:29 UTC (rev 192161)
+++ trunk/Source/WebKit2/ChangeLog	2015-11-09 18:35:19 UTC (rev 192162)
@@ -1,3 +1,35 @@
+2015-11-06  Anders Carlsson  <ander...@apple.com>
+
+        Rework the way allowed argument classes are stored
+        https://bugs.webkit.org/show_bug.cgi?id=150992
+
+        Reviewed by Darin Adler.
+
+        Add a separate MethodInfo class so we have someplace to store the reply block arguments.
+        Use HashSet<Class> instead of NSSet. No functionality change intended.
+
+        * Shared/API/Cocoa/WKRemoteObjectCoder.mm:
+        (-[WKRemoteObjectDecoder decodeValueOfObjCType:at:]):
+        (decodeObjectFromObjectStream):
+        (checkIfClassIsAllowed):
+        (decodeInvocationArguments):
+        (decodeObject):
+        (-[WKRemoteObjectDecoder decodeObjectOfClasses:forKey:]):
+        (-[WKRemoteObjectDecoder allowedClasses]):
+        * Shared/API/Cocoa/_WKRemoteObjectInterface.mm:
+        (isContainerClass):
+        (propertyListClasses):
+        (initializeMethod):
+        (initializeMethods):
+        (-[_WKRemoteObjectInterface initWithProtocol:identifier:]):
+        (classesForSelectorArgument):
+        (-[_WKRemoteObjectInterface classesForSelector:argumentIndex:]):
+        (-[_WKRemoteObjectInterface setClasses:forSelector:argumentIndex:]):
+        (-[_WKRemoteObjectInterface _allowedArgumentClassesForSelector:]):
+        (allowedArgumentClassesForMethod): Deleted.
+        (initializeAllowedArgumentClasses): Deleted.
+        * Shared/API/Cocoa/_WKRemoteObjectInterfaceInternal.h:
+
 2015-11-09  Csaba Osztrogonác  <o...@webkit.org>
 
         Unreviewed CMake buildfix after r192113.

Modified: trunk/Source/WebKit2/Shared/API/Cocoa/WKRemoteObjectCoder.mm (192161 => 192162)


--- trunk/Source/WebKit2/Shared/API/Cocoa/WKRemoteObjectCoder.mm	2015-11-09 18:10:29 UTC (rev 192161)
+++ trunk/Source/WebKit2/Shared/API/Cocoa/WKRemoteObjectCoder.mm	2015-11-09 18:35:19 UTC (rev 192162)
@@ -358,7 +358,7 @@
     const API::Array* _objectStream;
     size_t _objectStreamPosition;
 
-    NSSet *_allowedClasses;
+    const HashSet<Class>* _allowedClasses;
 }
 
 - (id)initWithInterface:(_WKRemoteObjectInterface *)interface rootObjectDictionary:(const API::Dictionary*)rootObjectDictionary
@@ -381,7 +381,7 @@
     switch (*type) {
     // int
     case 'i':
-        *static_cast<int*>(data) = [decodeObjectFromObjectStream(self, [NSSet setWithObject:[NSNumber class]]) intValue];
+        *static_cast<int*>(data) = [decodeObjectFromObjectStream(self, { [NSNumber class] }) intValue];
         break;
 
     default:
@@ -404,9 +404,9 @@
     return [self decodeObjectOfClasses:nil forKey:key];
 }
 
-static id decodeObject(WKRemoteObjectDecoder *, const API::Dictionary*, NSSet *allowedClasses);
+static id decodeObject(WKRemoteObjectDecoder *, const API::Dictionary*, const HashSet<Class>& allowedClasses);
 
-static id decodeObjectFromObjectStream(WKRemoteObjectDecoder *decoder, NSSet *allowedClasses)
+static id decodeObjectFromObjectStream(WKRemoteObjectDecoder *decoder, const HashSet<Class>& allowedClasses)
 {
     if (!decoder->_objectStream)
         return nil;
@@ -421,15 +421,19 @@
 
 static void checkIfClassIsAllowed(WKRemoteObjectDecoder *decoder, Class objectClass)
 {
-    NSSet *allowedClasses = decoder->_allowedClasses;
+    auto* allowedClasses = decoder->_allowedClasses;
+    if (!allowedClasses)
+        return;
 
-    // Check if the class or any of its superclasses are in the allowed classes set.
-    for (Class cls = objectClass; cls; cls = class_getSuperclass(cls)) {
-        if ([allowedClasses containsObject:cls])
+    if (allowedClasses->contains(objectClass))
+        return;
+
+    for (Class superclass = class_getSuperclass(objectClass); superclass; superclass = class_getSuperclass(superclass)) {
+        if (allowedClasses->contains(superclass))
             return;
     }
 
-    [NSException raise:NSInvalidUnarchiveOperationException format:@"Object of class \"%@\" is not allowed. Allowed classes are \"%@\"", objectClass, allowedClasses];
+    [NSException raise:NSInvalidUnarchiveOperationException format:@"Object of class \"%@\" is not allowed. Allowed classes are \"%@\"", objectClass, decoder.allowedClasses];
 }
 
 static void validateClass(WKRemoteObjectDecoder *decoder, Class objectClass)
@@ -445,7 +449,7 @@
     [decoder validateClassSupportsSecureCoding:objectClass];
 }
 
-static void decodeInvocationArguments(WKRemoteObjectDecoder *decoder, NSInvocation *invocation, const Vector<RetainPtr<NSSet>>& allowedArgumentClasses)
+static void decodeInvocationArguments(WKRemoteObjectDecoder *decoder, NSInvocation *invocation, const Vector<HashSet<Class>>& allowedArgumentClasses)
 {
     NSMethodSignature *methodSignature = invocation.methodSignature;
     NSUInteger argumentCount = methodSignature.numberOfArguments;
@@ -460,63 +464,63 @@
         switch (*type) {
         // double
         case 'd': {
-            double value = [decodeObjectFromObjectStream(decoder, [NSSet setWithObject:[NSNumber class]]) doubleValue];
+            double value = [decodeObjectFromObjectStream(decoder, { [NSNumber class] }) doubleValue];
             [invocation setArgument:&value atIndex:i];
             break;
         }
 
         // float
         case 'f': {
-            float value = [decodeObjectFromObjectStream(decoder, [NSSet setWithObject:[NSNumber class]]) floatValue];
+            float value = [decodeObjectFromObjectStream(decoder, { [NSNumber class] }) floatValue];
             [invocation setArgument:&value atIndex:i];
             break;
         }
 
         // int
         case 'i': {
-            int value = [decodeObjectFromObjectStream(decoder, [NSSet setWithObject:[NSNumber class]]) intValue];
+            int value = [decodeObjectFromObjectStream(decoder, { [NSNumber class] }) intValue];
             [invocation setArgument:&value atIndex:i];
             break;
         }
 
         // unsigned
         case 'I': {
-            unsigned value = [decodeObjectFromObjectStream(decoder, [NSSet setWithObject:[NSNumber class]]) unsignedIntValue];
+            unsigned value = [decodeObjectFromObjectStream(decoder, { [NSNumber class] }) unsignedIntValue];
             [invocation setArgument:&value atIndex:i];
             break;
         }
 
         // char
         case 'c': {
-            char value = [decodeObjectFromObjectStream(decoder, [NSSet setWithObject:[NSNumber class]]) charValue];
+            char value = [decodeObjectFromObjectStream(decoder, { [NSNumber class] }) charValue];
             [invocation setArgument:&value atIndex:i];
             break;
         }
 
         // bool
         case 'B': {
-            bool value = [decodeObjectFromObjectStream(decoder, [NSSet setWithObject:[NSNumber class]]) boolValue];
+            bool value = [decodeObjectFromObjectStream(decoder, { [NSNumber class] }) boolValue];
             [invocation setArgument:&value atIndex:i];
             break;
         }
 
         // long
         case 'q': {
-            long value = [decodeObjectFromObjectStream(decoder, [NSSet setWithObject:[NSNumber class]]) longValue];
+            long value = [decodeObjectFromObjectStream(decoder, { [NSNumber class] }) longValue];
             [invocation setArgument:&value atIndex:i];
             break;
         }
 
         // unsigned long
         case 'Q': {
-            unsigned long value = [decodeObjectFromObjectStream(decoder, [NSSet setWithObject:[NSNumber class]]) unsignedLongValue];
+            unsigned long value = [decodeObjectFromObjectStream(decoder, { [NSNumber class] }) unsignedLongValue];
             [invocation setArgument:&value atIndex:i];
             break;
         }
             
         // Objective-C object
         case '@': {
-            NSSet *allowedClasses = allowedArgumentClasses[i - 2].get();
+            auto& allowedClasses = allowedArgumentClasses[i - 2];
 
             id value = decodeObjectFromObjectStream(decoder, allowedClasses);
             [invocation setArgument:&value atIndex:i];
@@ -608,7 +612,7 @@
     return [result autorelease];
 }
 
-static id decodeObject(WKRemoteObjectDecoder *decoder, const API::Dictionary* dictionary, NSSet *allowedClasses)
+static id decodeObject(WKRemoteObjectDecoder *decoder, const API::Dictionary* dictionary, const HashSet<Class>& allowedClasses)
 {
     if (!dictionary)
         return nil;
@@ -616,10 +620,10 @@
     TemporaryChange<const API::Dictionary*> dictionaryChange(decoder->_currentDictionary, dictionary);
 
     // If no allowed classes were listed, just use the currently allowed classes.
-    if (!allowedClasses)
+    if (allowedClasses.isEmpty())
         return decodeObject(decoder);
 
-    TemporaryChange<NSSet *> allowedClassesChange(decoder->_allowedClasses, allowedClasses);
+    TemporaryChange<const HashSet<Class>*> allowedClassesChange(decoder->_allowedClasses, &allowedClasses);
     return decodeObject(decoder);
 }
 
@@ -695,12 +699,23 @@
 
 - (id)decodeObjectOfClasses:(NSSet *)classes forKey:(NSString *)key
 {
-    return decodeObject(self, _currentDictionary->get<API::Dictionary>(escapeKey(key)), classes);
+    HashSet<Class> allowedClasses;
+    for (Class allowedClass in classes)
+        allowedClasses.add(allowedClass);
+
+    return decodeObject(self, _currentDictionary->get<API::Dictionary>(escapeKey(key)), allowedClasses);
 }
 
 - (NSSet *)allowedClasses
 {
-    return _allowedClasses;
+    if (!_allowedClasses)
+        return [NSSet set];
+
+    auto result = adoptNS([[NSMutableSet alloc] init]);
+    for (Class allowedClass : *_allowedClasses)
+        [result addObject:allowedClass];
+
+    return result.autorelease();
 }
 
 @end

Modified: trunk/Source/WebKit2/Shared/API/Cocoa/_WKRemoteObjectInterface.mm (192161 => 192162)


--- trunk/Source/WebKit2/Shared/API/Cocoa/_WKRemoteObjectInterface.mm	2015-11-09 18:10:29 UTC (rev 192161)
+++ trunk/Source/WebKit2/Shared/API/Cocoa/_WKRemoteObjectInterface.mm	2015-11-09 18:35:19 UTC (rev 192162)
@@ -24,14 +24,15 @@
  */
 
 #import "config.h"
-#import "_WKRemoteObjectInterface.h"
+#import "_WKRemoteObjectInterfaceInternal.h"
 
 #if WK_API_ENABLED
 
 #import <objc/runtime.h>
 #import <wtf/HashMap.h>
+#import <wtf/NeverDestroyed.h>
+#import <wtf/RetainPtr.h>
 #import <wtf/Vector.h>
-#import <wtf/RetainPtr.h>
 
 extern "C"
 const char *_protocol_getMethodTypeEncoding(Protocol *p, SEL sel, BOOL isRequiredMethod, BOOL isInstanceMethod);
@@ -40,88 +41,116 @@
 - (Class)_classForObjectAtArgumentIndex:(NSInteger)idx;
 @end
 
+struct MethodInfo {
+    Vector<HashSet<Class>> allowedArgumentClasses;
+
+    // FIXME: This should have allowed reply argument classes too.
+};
+
 @implementation _WKRemoteObjectInterface {
-    HashMap<SEL, Vector<RetainPtr<NSSet>>> _allowedArgumentClasses;
     RetainPtr<NSString> _identifier;
+
+    HashMap<SEL, MethodInfo> _methods;
 }
 
 static bool isContainerClass(Class objectClass)
 {
     // FIXME: Add more classes here if needed.
-    static NSSet *containerClasses = [[NSSet alloc] initWithObjects:[NSArray class], [NSDictionary class], nil];
+    static LazyNeverDestroyed<HashSet<Class>> containerClasses;
+    static dispatch_once_t onceToken;
+    dispatch_once(&onceToken, ^{
+        containerClasses.construct(std::initializer_list<Class> { [NSArray class], [NSDictionary class] });
+    });
 
-    return [containerClasses containsObject:objectClass];
+    return containerClasses->contains(objectClass);
 }
 
-static NSSet *propertyListClasses()
+static HashSet<Class>& propertyListClasses()
 {
-    // FIXME: Add more property list classes if needed.
-    static NSSet *propertyListClasses = [[NSSet alloc] initWithObjects:[NSArray class], [NSDictionary class], [NSNumber class], [NSString class], nil];
+    static LazyNeverDestroyed<HashSet<Class>> propertyListClasses;
+    static dispatch_once_t onceToken;
+    dispatch_once(&onceToken, ^{
+        propertyListClasses.construct(std::initializer_list<Class> { [NSArray class], [NSDictionary class], [NSNumber class], [NSString class] });
+    });
 
     return propertyListClasses;
 }
 
-static Vector<RetainPtr<NSSet>> allowedArgumentClassesForMethod(NSMethodSignature *methodSignature)
+static void initializeMethod(MethodInfo& methodInfo, NSMethodSignature *methodSignature, bool forReplyBlock)
 {
-    Vector<RetainPtr<NSSet>> result;
+    Vector<HashSet<Class>> allowedClasses;
 
-    NSSet *emptySet = [NSSet set];
+    NSUInteger firstArgument = forReplyBlock ? 1 : 2;
+    NSUInteger argumentCount = methodSignature.numberOfArguments;
 
-    // We ignore self and _cmd.
-    NSUInteger argumentCount = methodSignature.numberOfArguments - 2;
+    for (NSUInteger i = firstArgument; i < argumentCount; ++i) {
+        const char* argumentType = [methodSignature getArgumentTypeAtIndex:i];
 
-    result.reserveInitialCapacity(argumentCount);
-
-    for (NSUInteger i = 0; i < argumentCount; ++i) {
-        const char* type = [methodSignature getArgumentTypeAtIndex:i + 2];
-
-        if (*type != '@') {
+        if (*argumentType != '@') {
             // This is a non-object type; we won't allow any classes to be decoded for it.
-            result.uncheckedAppend(emptySet);
+            allowedClasses.uncheckedAppend({ });
             continue;
         }
 
-        Class objectClass = [methodSignature _classForObjectAtArgumentIndex:i + 2];
+        Class objectClass = [methodSignature _classForObjectAtArgumentIndex:i];
         if (!objectClass) {
-            result.uncheckedAppend(emptySet);
+            allowedClasses.append({ });
             continue;
         }
 
         if (isContainerClass(objectClass)) {
             // For container classes, we allow all simple property list classes.
-            result.uncheckedAppend(propertyListClasses());
+            allowedClasses.append(propertyListClasses());
             continue;
         }
 
-        result.uncheckedAppend([NSSet setWithObject:objectClass]);
+        allowedClasses.append({ objectClass });
     }
 
-    return result;
+    methodInfo.allowedArgumentClasses = WTF::move(allowedClasses);
 }
 
-static void initializeAllowedArgumentClasses(_WKRemoteObjectInterface *interface, bool requiredMethods)
+static void initializeMethods(_WKRemoteObjectInterface *interface, Protocol *protocol, bool requiredMethods)
 {
     unsigned methodCount;
-    struct objc_method_description *methods = protocol_copyMethodDescriptionList(interface->_protocol, requiredMethods, true, &methodCount);
+    struct objc_method_description *methods = protocol_copyMethodDescriptionList(protocol, requiredMethods, true, &methodCount);
 
     for (unsigned i = 0; i < methodCount; ++i) {
         SEL selector = methods[i].name;
 
-        const char* types = _protocol_getMethodTypeEncoding(interface->_protocol, selector, requiredMethods, true);
-        if (!types)
+        ASSERT(!interface->_methods.contains(selector));
+        MethodInfo& methodInfo = interface->_methods.add(selector, MethodInfo()).iterator->value;
+
+        const char* methodTypeEncoding = _protocol_getMethodTypeEncoding(protocol, selector, requiredMethods, true);
+        if (!methodTypeEncoding)
             [NSException raise:NSInvalidArgumentException format:@"Could not find method type encoding for method \"%s\"", sel_getName(selector)];
 
-        NSMethodSignature *methodSignature = [NSMethodSignature signatureWithObjCTypes:types];
-        interface->_allowedArgumentClasses.set(selector, allowedArgumentClassesForMethod(methodSignature));
+        NSMethodSignature *methodSignature = [NSMethodSignature signatureWithObjCTypes:methodTypeEncoding];
+
+        initializeMethod(methodInfo, methodSignature, false);
     }
 
     free(methods);
 }
 
-static void initializeAllowedArgumentClasses(_WKRemoteObjectInterface *interface)
+static void initializeMethods(_WKRemoteObjectInterface *interface, Protocol *protocol)
 {
-    initializeAllowedArgumentClasses(interface, true);
-    initializeAllowedArgumentClasses(interface, false);
+    unsigned conformingProtocolCount;
+    Protocol** conformingProtocols = protocol_copyProtocolList(interface->_protocol, &conformingProtocolCount);
+
+    for (unsigned i = 0; i < conformingProtocolCount; ++i) {
+        Protocol* conformingProtocol = conformingProtocols[i];
+
+        if (conformingProtocol == @protocol(NSObject))
+            continue;
+
+        initializeMethods(interface, conformingProtocol);
+    }
+
+    free(conformingProtocols);
+
+    initializeMethods(interface, protocol, true);
+    initializeMethods(interface, protocol, false);
 }
 
 - (id)initWithProtocol:(Protocol *)protocol identifier:(NSString *)identifier
@@ -132,7 +161,7 @@
     _protocol = protocol;
     _identifier = adoptNS([identifier copy]);
 
-    initializeAllowedArgumentClasses(self);
+    initializeMethods(self, _protocol);
 
     return self;
 }
@@ -152,26 +181,38 @@
     return [NSString stringWithFormat:@"<%@: %p; protocol = \"%@\"; identifier = \"%@\">", NSStringFromClass(self.class), self, _identifier.get(), NSStringFromProtocol(_protocol)];
 }
 
-static RetainPtr<NSSet>& classesForSelectorArgument(_WKRemoteObjectInterface *interface, SEL selector, NSUInteger argumentIndex)
+static HashSet<Class>& classesForSelectorArgument(_WKRemoteObjectInterface *interface, SEL selector, NSUInteger argumentIndex)
 {
-    auto it = interface->_allowedArgumentClasses.find(selector);
-    if (it == interface->_allowedArgumentClasses.end())
+    auto it = interface->_methods.find(selector);
+    if (it == interface->_methods.end())
         [NSException raise:NSInvalidArgumentException format:@"Interface does not contain selector \"%s\"", sel_getName(selector)];
 
-    if (argumentIndex >= it->value.size())
+    MethodInfo& methodInfo = it->value;
+    auto& allowedArgumentClasses = methodInfo.allowedArgumentClasses;
+
+    if (argumentIndex >= allowedArgumentClasses.size())
         [NSException raise:NSInvalidArgumentException format:@"Argument index %ld is out of range for selector \"%s\"", (unsigned long)argumentIndex, sel_getName(selector)];
 
-    return it->value[argumentIndex];
+    return allowedArgumentClasses[argumentIndex];
 }
 
 - (NSSet *)classesForSelector:(SEL)selector argumentIndex:(NSUInteger)argumentIndex
 {
-    return [[classesForSelectorArgument(self, selector, argumentIndex).get() retain] autorelease];
+    auto result = adoptNS([[NSMutableSet alloc] init]);
+
+    for (auto allowedClass : classesForSelectorArgument(self, selector, argumentIndex))
+        [result addObject:allowedClass];
+
+    return result.autorelease();
 }
 
 - (void)setClasses:(NSSet *)classes forSelector:(SEL)selector argumentIndex:(NSUInteger)argumentIndex
 {
-    classesForSelectorArgument(self, selector, argumentIndex) = adoptNS([classes copy]);
+    HashSet<Class> allowedClasses;
+    for (Class allowedClass in classes)
+        allowedClasses.add(allowedClass);
+
+    classesForSelectorArgument(self, selector, argumentIndex) = WTF::move(allowedClasses);
 }
 
 static const char* methodArgumentTypeEncodingForSelector(Protocol *protocol, SEL selector)
@@ -198,12 +239,11 @@
     return [NSMethodSignature signatureWithObjCTypes:types];
 }
 
-- (const Vector<RetainPtr<NSSet>>&)_allowedArgumentClassesForSelector:(SEL)selector
+- (const Vector<HashSet<Class>>&)_allowedArgumentClassesForSelector:(SEL)selector
 {
-    auto it = _allowedArgumentClasses.find(selector);
-    ASSERT(it != _allowedArgumentClasses.end());
+    ASSERT(_methods.contains(selector));
 
-    return it->value;
+    return _methods.find(selector)->value.allowedArgumentClasses;
 }
 
 @end

Modified: trunk/Source/WebKit2/Shared/API/Cocoa/_WKRemoteObjectInterfaceInternal.h (192161 => 192162)


--- trunk/Source/WebKit2/Shared/API/Cocoa/_WKRemoteObjectInterfaceInternal.h	2015-11-09 18:10:29 UTC (rev 192161)
+++ trunk/Source/WebKit2/Shared/API/Cocoa/_WKRemoteObjectInterfaceInternal.h	2015-11-09 18:35:19 UTC (rev 192162)
@@ -28,12 +28,14 @@
 #if WK_API_ENABLED
 
 #import <wtf/Forward.h>
+#import <wtf/HashSet.h>
 #import <wtf/RetainPtr.h>
+#import <wtf/Vector.h>
 
 @interface _WKRemoteObjectInterface ()
 
 - (NSMethodSignature *)_methodSignatureForSelector:(SEL)selector;
-- (const Vector<RetainPtr<NSSet>>&)_allowedArgumentClassesForSelector:(SEL)selector;
+- (const Vector<HashSet<Class>>&)_allowedArgumentClassesForSelector:(SEL)selector;
 
 @end
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to