aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Thomas Van Lenten <thomasvl@google.com>2016-05-24 10:04:21 -0400
committerGravatar Thomas Van Lenten <thomasvl@google.com>2016-05-24 10:04:21 -0400
commit016b5c7bc2bceefb672c9880b8c5cf037c003c5d (patch)
tree900862a07c3487b75cb571b526f7e183715e398e
parent45ef69578f7f36dd200edd894b53bc1ab55becc7 (diff)
parent911c457618377e675bbc89abbda97ffc13af1306 (diff)
Merge pull request #117 from thomasvl/drop_kvo_checks
Remove GTM_PERFORM_KVO_CHECKS.
-rw-r--r--Foundation/GTMNSObject+KeyValueObserving.h6
-rw-r--r--Foundation/GTMNSObject+KeyValueObserving.m146
-rw-r--r--Foundation/GTMNSObject+KeyValueObservingTest.m53
3 files changed, 1 insertions, 204 deletions
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 <Foundation/Foundation.h>
-#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
-