From 911c457618377e675bbc89abbda97ffc13af1306 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Tue, 24 May 2016 10:02:33 -0400 Subject: Remove GTM_PERFORM_KVO_CHECKS. They have become too invasive and cause more issues than they were solving, especially on iOS where they seem to interact very badly with the accessibility provided by the OS. --- Foundation/GTMNSObject+KeyValueObserving.h | 6 - Foundation/GTMNSObject+KeyValueObserving.m | 146 +------------------------ Foundation/GTMNSObject+KeyValueObservingTest.m | 53 --------- 3 files changed, 1 insertion(+), 204 deletions(-) (limited to 'Foundation') diff --git a/Foundation/GTMNSObject+KeyValueObserving.h b/Foundation/GTMNSObject+KeyValueObserving.h index 079717f..9aa1fbb 100644 --- a/Foundation/GTMNSObject+KeyValueObserving.h +++ b/Foundation/GTMNSObject+KeyValueObserving.h @@ -32,12 +32,6 @@ #import -#ifndef GTM_PERFORM_KVO_CHECKS -// Controls whether KVO checking code is on in debug. See -// GTMNSObject+KeyValueObserving.m for details -#define GTM_PERFORM_KVO_CHECKS 1 -#endif - // If you read the articles above you will see that doing KVO correctly // is actually pretty tricky, and that Apple's documentation may not be // completely clear as to how things should be used. Use the methods below diff --git a/Foundation/GTMNSObject+KeyValueObserving.m b/Foundation/GTMNSObject+KeyValueObserving.m index 70f5310..c4a589d 100644 --- a/Foundation/GTMNSObject+KeyValueObserving.m +++ b/Foundation/GTMNSObject+KeyValueObserving.m @@ -397,17 +397,6 @@ static void SwizzleMethodsInClass(Class cls, SEL sel1, SEL sel2) { method_exchangeImplementations(m1, m2); } -#if GTM_PERFORM_KVO_CHECKS - -// This is only used when GTM_PERFORM_KVO_CHECKS is on. -static void SwizzleClassMethodsInClass(Class cls, SEL sel1, SEL sel2) { - Method m1 = class_getClassMethod(cls, sel1); - Method m2 = class_getClassMethod(cls, sel2); - method_exchangeImplementations(m1, m2); -} - -#endif // GTM_PERFORM_KVO_CHECKS - // This category exists to attempt to help deal with tricky KVO issues. // KVO is a wonderful technology in some ways, but is extremely fragile and // allows developers a lot of freedom to shoot themselves in the foot. @@ -415,43 +404,12 @@ static void SwizzleClassMethodsInClass(Class cls, SEL sel1, SEL sel2) { // debugging it. // These are some tools that we have found useful when working with KVO. Note // that these tools are only on in Debug builds. -// -// We have divided these tools up into two categories: Checks and Debugs. -// -// Debugs -// Debugs are mainly for logging all the KVO/KVC that is occurring in your -// application. To enable our KVO debugging, set the GTMDebugKVO environment +// To enable our KVO debugging, set the GTMDebugKVO environment // variable to 1 and you will get a whole pile of KVO logging that may help you // track down problems. // bash - export GTMDebugKVO=1 // csh/tcsh - setenv GTMDebugKVO 1 // -// Checks -// First we believe that instance variables should be private by default, -// and that any KVO should be done via accessors. Apple by default allows KVO -// to get at instance variables directly. Since our coding standards define -// that instance variables should be @private, we feel that KVO shouldn't be -// breaking this encapsulation. Unfortunately the @private, @protected -// designators are a compile time convention only, and don't get carried over -// into the runtime, so there's no way to check on an individual iVar what -// it's visibility is. We therefore assume that an instance variable is private, -// and disallow KVO access to instance variables. The problem with most KVO -// issues is that they occur at runtime and unless you execute that case you -// may never see the bug until it's too late. We try to force KVO issues to -// rear their head at the time of the observing if at all possible. -// Checks are on by default in debug builds. They can be turned off by defining -// the compile flag GTM_PERFORM_KVO_CHECKS to 0 -// i.e. #define GTM_PERFORM_KVO_CHECKS 0, or set it -// in GCC_PREPROCESSOR_DEFINITIONS. -// -// Checks work at a couple of different levels. -// The most restrictive of the checks is that we set -// |accessInstanceVariablesDirectly| to NO by default. This means that if you -// attempt to perform KVO on an instance variable, you will get an exception -// thrown. -// Also, when adding an observer, we check to see if any member of the path -// starts or ends with _ which by convention denotes an instance variable. If so -// we warn you about attempting to access a ivar directly. @interface NSObject (GTMDebugKeyValueObserving) - (void)_gtmDebugAddObserver:(NSObject *)observer @@ -471,20 +429,6 @@ static void SwizzleClassMethodsInClass(Class cls, SEL sel1, SEL sel2) { - (void)_gtmDebugWillChangeValueForKey:(NSString*)key; - (void)_gtmDebugDidChangeValueForKey:(NSString*)key; -#if GTM_PERFORM_KVO_CHECKS - -- (void)_gtmCheckAddObserver:(NSObject *)observer - forKeyPath:(NSString *)keyPath - options:(NSKeyValueObservingOptions)options - context:(void *)context; -- (void)_gtmCheckAddObserver:(NSObject *)observer - toObjectsAtIndexes:(NSIndexSet *)indexes - forKeyPath:(NSString *)keyPath - options:(NSKeyValueObservingOptions)options - context:(void *)context; -+ (BOOL)_gtmAccessInstanceVariablesDirectly; - -#endif // GTM_PERFORM_KVO_CHECKS @end @implementation NSObject (GTMDebugKeyValueObserving) @@ -497,17 +441,6 @@ GTM_METHOD_CHECK(NSArray, GTM_METHOD_CHECK(NSArray, _gtmDebugArrayRemoveObserver:fromObjectsAtIndexes:forKeyPath:); -#if GTM_PERFORM_KVO_CHECKS - -GTM_METHOD_CHECK(NSObject, - _gtmCheckAddObserver:forKeyPath:options:context:); -GTM_METHOD_CHECK(NSArray, - _gtmCheckAddObserver:toObjectsAtIndexes:forKeyPath:options:context:); -GTM_METHOD_CHECK(NSObject, - _gtmAccessInstanceVariablesDirectly); - -#endif // GTM_PERFORM_KVO_CHECKS - + (void)load { NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init]; NSDictionary *env = [[NSProcessInfo processInfo] environment]; @@ -542,20 +475,6 @@ GTM_METHOD_CHECK(NSObject, @selector(removeObserver:fromObjectsAtIndexes:forKeyPath:), @selector(_gtmDebugArrayRemoveObserver:fromObjectsAtIndexes:forKeyPath:)); } -#if GTM_PERFORM_KVO_CHECKS - cls = [NSObject class]; - SwizzleMethodsInClass(cls, - @selector(addObserver:forKeyPath:options:context:), - @selector(_gtmCheckAddObserver:forKeyPath:options:context:)); - SwizzleClassMethodsInClass(cls, - @selector(accessInstanceVariablesDirectly), - @selector(_gtmAccessInstanceVariablesDirectly)); - cls = [NSArray class]; - SwizzleMethodsInClass(cls, - @selector(addObserver:toObjectsAtIndexes:forKeyPath:options:context:), - @selector(_gtmCheckAddObserver:toObjectsAtIndexes:forKeyPath:options:context:)); - -#endif // GTM_PERFORM_KVO_CHECKS [pool drain]; } @@ -608,69 +527,6 @@ GTM_METHOD_CHECK(NSObject, [self _gtmDebugDidChangeValueForKey:key]; } -#if GTM_PERFORM_KVO_CHECKS - -- (void)_gtmCheckAddObserver:(NSObject *)observer - forKeyPath:(NSString *)keyPath - options:(NSKeyValueObservingOptions)options - context:(void *)context { - NSArray *keyPathElements = [keyPath componentsSeparatedByString:@"."]; - NSString *element; - GTM_FOREACH_OBJECT(element, keyPathElements) { - if ([element hasPrefix:@"_"] || [element hasSuffix:@"_"]) { - _GTMDevLog(@"warning: %@ is registering an observation on what appears " - @"to be a private ivar of %@ (or a sub keyed object) with " - @"element %@ of keyPath %@.", observer, self, element, - keyPath); - } - } - [self _gtmCheckAddObserver:observer - forKeyPath:keyPath - options:options - context:context]; -} - -- (void)_gtmCheckAddObserver:(NSObject *)observer - toObjectsAtIndexes:(NSIndexSet *)indexes - forKeyPath:(NSString *)keyPath - options:(NSKeyValueObservingOptions)options - context:(void *)context { - NSArray *keyPathElements = [keyPath componentsSeparatedByString:@"."]; - NSString *element; - GTM_FOREACH_OBJECT(element, keyPathElements) { - if ([element hasPrefix:@"_"] || [element hasSuffix:@"_"]) { - _GTMDevLog(@"warning: %@ is registering an observation on what appears " - @"to be a private ivar of %@ (or a sub keyed object) with " - @"element %@ of keyPath %@.", observer, self, element, - keyPath); - } - } - [self _gtmCheckAddObserver:observer - toObjectsAtIndexes:indexes - forKeyPath:keyPath - options:options - context:context]; -} - - -+ (BOOL)_gtmAccessInstanceVariablesDirectly { - // Apple has lots of "bad" direct instance variable accesses, so we - // only want to check our code, as opposed to library code. iOS simulator - // builds copy the app into the user's home directory. Xcode 4 also changes - // the default location of the output directory. Don't count being within - // the user's home and under "/Library/" as being a system library. - - // If this turns out to be slow, we may want to consider a cache to speed - // things up. - NSBundle *bundle = [NSBundle bundleForClass:self]; - NSString *path = [bundle bundlePath]; - BOOL hasLibrary = [path rangeOfString:@"/Library/"].location != NSNotFound; - BOOL startsWithUser = [path hasPrefix:@"/Users/"]; - return !startsWithUser && hasLibrary; -} - -#endif // GTM_PERFORM_KVO_CHECKS - @end #endif // DEBUG diff --git a/Foundation/GTMNSObject+KeyValueObservingTest.m b/Foundation/GTMNSObject+KeyValueObservingTest.m index ba59a96..43a1bba 100644 --- a/Foundation/GTMNSObject+KeyValueObservingTest.m +++ b/Foundation/GTMNSObject+KeyValueObservingTest.m @@ -127,56 +127,3 @@ } @end - -#if GTM_PERFORM_KVO_CHECKS -@interface GTMNSObject_KeyValueObservingChecksTest: GTMTestCase { - @private - id value_; - id _value2; - GTM_WEAK NSArray *value3_; - GTM_WEAK NSString *value4; -} -- (NSString *)value4; -@end - -@implementation GTMNSObject_KeyValueObservingChecksTest - -- (void)setUp { - value_ = nil; - _value2 = nil; -} - -- (void)testAddingObserver { - [GTMUnitTestDevLogDebug expectPattern:@"warning:.*"]; - [self addObserver:self forKeyPath:@"value_" options:0 context:NULL]; - [GTMUnitTestDevLogDebug expectPattern:@"warning:.*"]; - [self addObserver:self forKeyPath:@"_value2" options:0 context:NULL]; - value3_ = [NSArray arrayWithObject:@"foo"]; - NSIndexSet *set = [NSIndexSet indexSetWithIndex:0]; - [GTMUnitTestDevLogDebug expectPattern:@"warning:.*"]; - [value3_ addObserver:self toObjectsAtIndexes:set forKeyPath:@"_fronttest" - options:0 context:NULL]; - [GTMUnitTestDevLogDebug expectPattern:@"warning:.*"]; - [value3_ addObserver:self toObjectsAtIndexes:set forKeyPath:@"backtest_" - options:0 context:NULL]; -#if DEBUG - // Should only throw in debug - STAssertThrows([self valueForKey:@"value_"], nil); -#else - STAssertNoThrow([self valueForKey:@"value_"], nil); -#endif - value4 = @"Hello"; - STAssertEqualObjects([self valueForKey:@"value4"], @"Hello", nil); - [self removeObserver:self forKeyPath:@"value_"]; - [self removeObserver:self forKeyPath:@"_value2"]; - [value3_ removeObserver:self fromObjectsAtIndexes:set forKeyPath:@"_fronttest"]; - [value3_ removeObserver:self fromObjectsAtIndexes:set forKeyPath:@"backtest_"]; -} - -- (NSString *)value4 { - return value4; -} -@end - -#endif // GTM_PERFORM_KVO_CHECKS - -- cgit v1.2.3