Build a new and better block-based KVO API

Fixes #1046
ish-app · Nov 15, 2020 · 26f4c91 · 26f4c91
1 parent a36ddff
commit 26f4c91
Show file tree

Hide file tree

Showing 11 changed files with 111 additions and 155 deletions.
diff --git a/app/AboutAppearanceViewController.m b/app/AboutAppearanceViewController.m
@@ -8,6 +8,7 @@
 #import "AboutAppearanceViewController.h"
 #import "FontPickerViewController.h"
 #import "UserPreferences.h"
+#import "NSObject+SaneKVO.h"
 
 static NSString *const ThemeNameCellIdentifier = @"Theme Name";
 static NSString *const FontSizeCellIdentifier = @"Font Size";
@@ -21,14 +22,11 @@ @implementation AboutAppearanceViewController
 
 - (void)viewDidLoad {
     [super viewDidLoad];
-    [[UserPreferences shared] addObserver:self forKeyPath:@"theme" options:NSKeyValueObservingOptionNew context:nil];
-    [[UserPreferences shared] addObserver:self forKeyPath:@"fontSize" options:NSKeyValueObservingOptionNew context:nil];
-    [[UserPreferences shared] addObserver:self forKeyPath:@"fontFamily" options:NSKeyValueObservingOptionNew context:nil];
-}
-
-- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context {
-    [self.tableView reloadData];
-    [self setNeedsStatusBarAppearanceUpdate];
+    [UserPreferences.shared observe:@[@"theme", @"fontSize", @"fontFamily"]
+                            options:0 owner:self usingBlock:^(typeof(self) self) {
+        [self.tableView reloadData];
+        [self setNeedsStatusBarAppearanceUpdate];
+    }];
 }
 
 #pragma mark - Table view data source

diff --git a/app/AboutExternalKeyboardViewController.m b/app/AboutExternalKeyboardViewController.m
@@ -7,6 +7,7 @@
 
 #import "AboutExternalKeyboardViewController.h"
 #import "UserPreferences.h"
+#import "NSObject+SaneKVO.h"
 
 const int kCapsLockMappingSection = 0;
 
@@ -23,15 +24,13 @@ @implementation AboutExternalKeyboardViewController
 
 - (void)viewDidLoad {
     [super viewDidLoad];
-    [UserPreferences.shared addObserver:self forKeyPath:@"capsLockMapping" options:NSKeyValueObservingOptionNew context:nil];
-    [UserPreferences.shared addObserver:self forKeyPath:@"optionMapping" options:NSKeyValueObservingOptionNew context:nil];
+    [UserPreferences.shared observe:@[@"capsLockMapping", @"optionMapping"]
+                            options:0 owner:self usingBlock:^(typeof(self) self) {
+        [self.tableView reloadData];
+    }];
     [self _update];
 }
 
-- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary<NSKeyValueChangeKey,id> *)change context:(void *)context {
-    [self.tableView reloadData];
-}
-
 - (void)_update {
     self.optionMetaSwitch.on = UserPreferences.shared.optionMapping == OptionMapEsc;
     self.backtickEscapeSwitch.on = UserPreferences.shared.backtickMapEscape;

diff --git a/app/AboutNavigationController.m b/app/AboutNavigationController.m
@@ -7,6 +7,7 @@
 
 #import "AboutNavigationController.h"
 #import "UserPreferences.h"
+#import "NSObject+SaneKVO.h"
 
 @interface AboutNavigationController ()
 
@@ -16,20 +17,17 @@ @implementation AboutNavigationController
 
 - (void)viewDidLoad {
     [super viewDidLoad];
-    [[UserPreferences shared] addObserver:self forKeyPath:@"theme" options:NSKeyValueObservingOptionNew|NSKeyValueObservingOptionInitial context:nil];
-}
-
-- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary<NSKeyValueChangeKey,id> *)change context:(void *)context {
-    if (@available(iOS 13, *)) {
-        if ([keyPath isEqualToString:@"theme"]) {
+    [UserPreferences.shared observe:@[@"theme"] options:NSKeyValueObservingOptionInitial
+                              owner:self usingBlock:^(typeof(self) self) {
+        if (@available(iOS 13, *)) {
             UIKeyboardAppearance appearance = UserPreferences.shared.theme.keyboardAppearance;
             if (appearance == UIKeyboardAppearanceDark) {
                 self.overrideUserInterfaceStyle = UIUserInterfaceStyleDark;
             } else {
                 self.overrideUserInterfaceStyle = UIUserInterfaceStyleLight;
             }
         }
-    }
+    }];
 }
 
 @end
diff --git a/app/AboutViewController.m b/app/AboutViewController.m
@@ -9,6 +9,7 @@
 #import "AboutViewController.h"
 #import "UserPreferences.h"
 #import "AppGroup.h"
+#import "NSObject+SaneKVO.h"
 
 @interface AboutViewController ()
 @property (weak, nonatomic) IBOutlet UITableViewCell *capsLockMappingCell;
@@ -46,12 +47,10 @@ - (void)viewDidLoad {
                           [[NSBundle mainBundle] objectForInfoDictionaryKey:@"CFBundleShortVersionString"],
                           [[NSBundle mainBundle] objectForInfoDictionaryKey:@"CFBundleVersion"]];
 
-    UserPreferences *prefs = [UserPreferences shared];
-    NSKeyValueObservingOptions opts = NSKeyValueObservingOptionNew;
-    [prefs addObserver:self forKeyPath:@"capsLockMapping" options:opts context:nil];
-    [prefs addObserver:self forKeyPath:@"fontSize" options:opts context:nil];
-    [prefs addObserver:self forKeyPath:@"launchCommand" options:opts context:nil];
-    [prefs addObserver:self forKeyPath:@"bootCommand" options:opts context:nil];
+    [UserPreferences.shared observe:@[@"capsLockMapping", @"fontSize", @"launchCommand", @"bootCommand"]
+                            options:0 owner:self usingBlock:^(typeof(self) self) {
+        [self _updatePreferenceUI];
+    }];
 }
 
 - (IBAction)dismiss:(id)sender {
@@ -63,14 +62,6 @@ - (void)exitRecovery:(id)sender {
     exit(0);
 }
 
-- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context {
-    if ([object isKindOfClass:[UserPreferences class]]) {
-        [self _updatePreferenceUI];
-    } else {
-        [super observeValueForKeyPath:keyPath ofObject:object change:change context:context];
-    }
-}
-
 - (void)_updatePreferenceUI {
     UserPreferences *prefs = UserPreferences.shared;
     self.themeCell.detailTextLabel.text = prefs.theme.presetName;

diff --git a/app/AppDelegate.m b/app/AppDelegate.m
@@ -16,6 +16,7 @@
 #import "SceneDelegate.h"
 #import "PasteboardDevice.h"
 #import "LocationDevice.h"
+#import "NSObject+SaneKVO.h"
 #import "Roots.h"
 #import "TerminalViewController.h"
 #import "UserPreferences.h"
@@ -208,7 +209,10 @@ - (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(
     extern const char *uname_version;
     uname_version = self.unameVersion.UTF8String;
 
-    [UserPreferences.shared addObserver:self forKeyPath:@"shouldDisableDimming" options:NSKeyValueObservingOptionInitial context:nil];
+    [UserPreferences.shared observe:@[@"shouldDisableDimming"] options:NSKeyValueObservingOptionInitial
+                              owner:self usingBlock:^(typeof(self) self) {
+        UIApplication.sharedApplication.idleTimerDisabled = UserPreferences.shared.shouldDisableDimming;
+    }];
 
     struct sockaddr_in6 address = {
         .sin6_len = sizeof(address),
@@ -237,10 +241,6 @@ - (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(
     return YES;
 }
 
-- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary<NSKeyValueChangeKey,id> *)change context:(void *)context {
-    UIApplication.sharedApplication.idleTimerDisabled = UserPreferences.shared.shouldDisableDimming;
-}
-
 - (void)application:(UIApplication *)application didDiscardSceneSessions:(NSSet<UISceneSession *> *)sceneSessions API_AVAILABLE(ios(13.0)) {
     for (UISceneSession *sceneSession in sceneSessions) {
         NSString *terminalUUID = sceneSession.stateRestorationActivity.userInfo[@"TerminalUUID"];

diff --git a/app/NSObject+SaneKVO.h b/app/NSObject+SaneKVO.h
@@ -9,8 +9,25 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+@interface KVOObservation : NSObject {
+    BOOL _enabled;
+    __weak id _object;
+    NSString *_keyPath;
+    void (^_block)(void);
+}
+- (void)disable;
+@end
+
 @interface NSObject (SaneKVO)
 
+- (KVOObservation *)observe:(NSString *)keyPath
+                    options:(NSKeyValueObservingOptions)options
+                 usingBlock:(void (^)(void))block;
+- (void)observe:(NSArray<NSString *> *)keyPaths
+        options:(NSKeyValueObservingOptions)options
+          owner:(id)owner
+     usingBlock:(void (^)(id self))block;
+
 @end
 
 NS_ASSUME_NONNULL_END
diff --git a/app/NSObject+SaneKVO.m b/app/NSObject+SaneKVO.m
@@ -8,113 +8,65 @@
 #import <objc/runtime.h>
 #import "NSObject+SaneKVO.h"
 
-static void *kKVOObject = &kKVOObject;
-
-@interface KVOObserver : NSObject {
-    @public
-    NSString *keyPath;
-    void *context;
-    __weak id object;
-}
-@end
-@implementation KVOObserver
-@end
-
-@interface KVOObject : NSObject
-
-- (instancetype)initWithOwner:(id)owner;
-- (BOOL)removeMatchingObserver:(BOOL (^)(KVOObserver *))test;
-@property (nonatomic, weak) id owner;
-@property NSMutableArray<KVOObserver *> *observers;
+static void *kKVOObservations = &kKVOObservations;
 
+@interface KVOObservation ()
+- (instancetype)initWithKeyPath:(NSString *)keyPath object:(id)object block:(void (^)(void))block;
 @end
 
 @implementation NSObject (SaneKVO)
 
-- (id)sane_createObserver {
-    @synchronized (self) {
-        KVOObject *observer = objc_getAssociatedObject(self, kKVOObject);
-        if (observer == nil) {
-            observer = [[KVOObject alloc] initWithOwner:self];
-            objc_setAssociatedObject(self, kKVOObject, observer, OBJC_ASSOCIATION_RETAIN_NONATOMIC);
-        }
-        return observer;
-    }
-}
-- (id)sane_observer {
-    return objc_getAssociatedObject(self, kKVOObject);
+- (KVOObservation *)observe:(NSString *)keyPath options:(NSKeyValueObservingOptions)options usingBlock:(void (^)(void))block {
+    KVOObservation *observation = [[KVOObservation alloc] initWithKeyPath:keyPath object:self block:block];
+    [self addObserver:observation forKeyPath:keyPath options:options context:NULL];
+    return observation;
 }
 
-- (void)sane_addObserver:(NSObject *)observer forKeyPath:(NSString *)keyPath options:(NSKeyValueObservingOptions)options context:(void *)context {
-    KVOObject *o = [observer sane_createObserver];
-    @synchronized (o) {
-        KVOObserver *obs = [KVOObserver new];
-        obs->object = self;
-        obs->keyPath = keyPath;
-        obs->context = context;
-        [o.observers addObject:obs];
-    }
-    [self sane_addObserver:o forKeyPath:keyPath options:options context:context];
-}
-- (void)sane_removeObserver:(NSObject *)observer forKeyPath:(NSString *)keyPath {
-    KVOObject *o = [observer sane_observer];
-    if ([o removeMatchingObserver:^BOOL(KVOObserver *obs) {
-        return [obs->keyPath isEqualToString:keyPath];
-    }]) {
-        [self sane_removeObserver:o forKeyPath:keyPath];
-    }
-}
-- (void)sane_removeObserver:(NSObject *)observer forKeyPath:(NSString *)keyPath context:(void *)context {
-    KVOObject *o = [observer sane_observer];
-    if ([o removeMatchingObserver:^BOOL(KVOObserver *obs) {
-        return [obs->keyPath isEqualToString:keyPath] && obs->context == context;
-    }]) {
-        [self sane_removeObserver:o forKeyPath:keyPath context:context];
+- (void)observe:(NSArray<NSString *> *)keyPaths options:(NSKeyValueObservingOptions)options owner:(id)owner usingBlock:(void (^)(id self))block {
+    __weak id weakOwner = owner;
+    void (^newBlock)(void) = ^{
+        id owner = weakOwner;
+        NSAssert(owner, @"kvo notification shouldn't come to dead object");
+        block(owner);
+    };
+    @synchronized (owner) {
+        for (NSString *keyPath in keyPaths) {
+            NSMutableSet *observations = objc_getAssociatedObject(owner, kKVOObservations);
+            if (observations == nil) {
+                observations = [NSMutableSet new];
+                objc_setAssociatedObject(owner, kKVOObservations, observations, OBJC_ASSOCIATION_RETAIN_NONATOMIC);
+            }
+            [observations addObject:[self observe:keyPath options:options usingBlock:newBlock]];
+        }
     }
 }
 
-+ (void)load {
-    [self swizzle:@selector(addObserver:forKeyPath:options:context:) with:@selector(sane_addObserver:forKeyPath:options:context:)];
-    [self swizzle:@selector(removeObserver:forKeyPath:context:) with:@selector(sane_removeObserver:forKeyPath:context:)];
-    [self swizzle:@selector(removeObserver:forKeyPath:) with:@selector(sane_removeObserver:forKeyPath:)];
-}
-+ (void)swizzle:(SEL)method with:(SEL)replacement {
-    method_exchangeImplementations(class_getInstanceMethod(self, method),
-                                   class_getInstanceMethod(self, replacement));
-}
-
 @end
 
-@implementation KVOObject
+@implementation KVOObservation
 
-- (instancetype)initWithOwner:(id)owner {
+- (instancetype)initWithKeyPath:(NSString *)keyPath object:(id)object block:(void (^)(void))block {
     if (self = [super init]) {
-        _owner = owner;
-        _observers = [NSMutableArray new];
+        _keyPath = keyPath;
+        _object = object;
+        _block = block;
+        _enabled = YES;
     }
     return self;
 }
 
 - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary<NSKeyValueChangeKey,id> *)change context:(void *)context {
-    [_owner observeValueForKeyPath:keyPath ofObject:object change:change context:context];
+    _block();
 }
 
-- (BOOL)removeMatchingObserver:(BOOL (^)(KVOObserver *))test {
-    @synchronized (self) {
-        NSUInteger i = [_observers indexOfObjectPassingTest:^BOOL(KVOObserver * _Nonnull obj, NSUInteger idx, BOOL * _Nonnull stop) {
-            return test(obj);
-        }];
-        if (i == NSNotFound)
-            return NO;
-        [_observers removeObjectAtIndex:i];
-        return YES;
+- (void)disable {
+    if (_enabled) {
+        [_object removeObserver:self forKeyPath:_keyPath context:NULL];
+        _enabled = NO;
     }
 }
-
 - (void)dealloc {
-    for (KVOObserver *obs in _observers) {
-        [obs->object removeObserver:self forKeyPath:obs->keyPath context:obs->context   ];
-    }
+    [self disable];
 }
 
 @end
diff --git a/app/Roots.m b/app/Roots.m
@@ -8,6 +8,7 @@
 #import <FileProvider/FileProvider.h>
 #import "Roots.h"
 #import "AppGroup.h"
+#import "NSObject+SaneKVO.h"
 #include "tools/fakefs.h"
 
 static NSURL *RootsDir() {
@@ -50,25 +51,24 @@ - (instancetype)init {
                 NSAssert(NO, @"failed to import alpine, error %@", error);
             }
         }
-        [self addObserver:self forKeyPath:@"roots" options:0 context:nil];
+        [self observe:@[@"roots"] options:0 owner:self usingBlock:^(typeof(self) self) {
+            if (self.defaultRoot == nil && self.roots.count)
+                self.defaultRoot = self.roots[0];
+            [self syncFileProviderDomains];
+        }];
         [self syncFileProviderDomains];
 
-        self.defaultRoot = [NSUserDefaults.standardUserDefaults stringForKey:kDefaultRoot];
-        [self addObserver:self forKeyPath:@"defaultRoot" options:0 context:nil];
         if ((!self.defaultRoot || ![self.roots containsObject:self.defaultRoot]) && self.roots.count)
             self.defaultRoot = self.roots.firstObject;
     }
     return self;
 }
 
-- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary<NSKeyValueChangeKey,id> *)change context:(void *)context {
-    if ([keyPath isEqualToString:@"defaultRoot"]) {
-        [NSUserDefaults.standardUserDefaults setObject:self.defaultRoot forKey:kDefaultRoot];
-    } else if ([keyPath isEqualToString:@"roots"]) {
-        if (self.defaultRoot == nil && self.roots.count)
-            self.defaultRoot = self.roots[0];
-        [self syncFileProviderDomains];
-    }
+- (NSString *)defaultRoot {
+    return [NSUserDefaults.standardUserDefaults stringForKey:kDefaultRoot];
+}
+- (void)setDefaultRoot:(NSString *)defaultRoot {
+    [NSUserDefaults.standardUserDefaults setObject:defaultRoot forKey:kDefaultRoot];
 }
 
 - (NSURL *)rootUrl:(NSString *)name {