Skip to content

Commit

Permalink
VSO 6649949: Make KVO not drop notifications for *derived* nested sub…
Browse files Browse the repository at this point in the history 
…keys.
  • Loading branch information
@DHowett
DHowett authored and Raj Seshasankaran committed Mar 4, 2016
1 parent 39be972 commit 613a831
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 94 deletions.
43 changes: 32 additions & 11 deletions Frameworks/UnifiedFoundation/Foundation/NSKVOSupport.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,16 +157,32 @@ bool isKVOCapable(Class cls) {
constexpr static NSKVOMatchAnyContextTag NSKVOMatchAnyContext{};

class NSKVOConcreteNotifier : public NSKVONotifier {
private:
protected:
std::function<void(const NSKVONotifier*)> _deregistrationHook;
std::vector<std::shared_ptr<NSKVONotifier>> _dependentNotifiers;
std::vector<std::shared_ptr<NSKVONotifier>> _dependentNotifiers; // notifiers that were created on our behalf
std::vector<std::weak_ptr<NSKVONotifier>> _owningNotifiers; // notifiers that hold ownership stake in us

public:
void setDeregistrationHook(std::function<void(const NSKVONotifier*)>&& hook) {
_deregistrationHook = std::move(hook);
}
void addOwningNotifier(const std::shared_ptr<NSKVONotifier>& owningNotifier) {
_owningNotifiers.emplace_back(owningNotifier);
}
void addDependentNotifier(const std::shared_ptr<NSKVONotifier>& dependentNotifier) override {
_dependentNotifiers.emplace_back(dependentNotifier);
NSKVOConcreteNotifier* concreteDependent = dynamic_cast<NSKVOConcreteNotifier*>(dependentNotifier.get());
if (concreteDependent) {
concreteDependent->addOwningNotifier(shared_from_this());
}
}
void updateLinks(id instance) override {
for (const auto& owningNotifier: _owningNotifiers) {
auto lockedOwner(owningNotifier.lock());
if (lockedOwner) {
lockedOwner->updateLinks(instance);
}
}
}
void breakLink() override {
auto dependentNotifiersCopy = _dependentNotifiers;
Expand All @@ -181,13 +197,14 @@ void breakLink() override {

class NSKVOForwardingNotifier : public NSKVOConcreteNotifier {
private:
std::string _key;
std::shared_ptr<NSKVONotifier> _chainedNotifier;
std::function<std::shared_ptr<NSKVONotifier>(id)> _replacementLink;

public:
NSKVOForwardingNotifier(const std::shared_ptr<NSKVONotifier>& chainedNotifier,
NSKVOForwardingNotifier(const std::string& key, const std::shared_ptr<NSKVONotifier>& chainedNotifier,
const std::function<std::shared_ptr<NSKVONotifier>(id)>& replacementLink)
: _chainedNotifier(chainedNotifier), _replacementLink(replacementLink) {
: _key(key), _chainedNotifier(chainedNotifier), _replacementLink(replacementLink) {
}
void dispatch(bool prior) override {
_chainedNotifier->dispatch(prior);
Expand All @@ -198,7 +215,7 @@ bool matches(id observer, const std::string& keypath, NSKVOMatchAnyContextTag) o
bool matches(id observer, const std::string& keypath, void* context) override {
return _chainedNotifier->matches(observer, keypath, context);
}
void updateLinks(id newValue) override {
void updateLinks(id instance) override {
// updateLinks bears the responsibility of maintaining the notification chain when object values change.
// When called with the new value of the key for which this notifier has notified, it will
// redirect its next notification to newValue, or [instance valueForKey:@"changedKey"].
Expand All @@ -208,8 +225,10 @@ void updateLinks(id newValue) override {
if (nextChainInChain) {
nextChainInChain->breakLink();
}
id newValue = NSKVOClass::valueForKey(instance, _key);
_chainedNotifier = _replacementLink(newValue);
}
__super::updateLinks(instance);
}
void breakLink() override {
_chainedNotifier->breakLink();
Expand Down Expand Up @@ -249,7 +268,7 @@ bool matches(id observer, const std::string& keypath, void* context) override {
}

// As this is a terminal node, there are no further chained notifiers to renew.
void updateLinks(id newValue) override {
void updateLinks(id instance) override {
}

void dispatch(bool prior) override {
Expand Down Expand Up @@ -375,16 +394,18 @@ void dispatch(bool prior) override {

/* static */
std::shared_ptr<NSKVONotifier> NSKVOClass::_notifierForSubkeyOnInstance(id newValue,
const std::string& keyComponent,
const std::string& keypath,
const std::shared_ptr<NSKVONotifier>& leafNotifier) {
if (keypath.size() == 0) {
return leafNotifier;
}

if (!newValue) {
return std::make_shared<NSKVOForwardingNotifier>(leafNotifier,
return std::make_shared<NSKVOForwardingNotifier>(keyComponent, leafNotifier,
std::bind(&NSKVOClass::_notifierForSubkeyOnInstance,
std::placeholders::_1,
keyComponent,
keypath,
leafNotifier));
} else {
Expand Down Expand Up @@ -419,11 +440,11 @@ void dispatch(bool prior) override {
id descentInstance = valueForKey(instance, keyComponent);
remainingKeypath.erase(0, pointPosition + 1);

chainedNotifier = _notifierForSubkeyOnInstance(descentInstance, remainingKeypath, leafNotifier);
chainedNotifier = _notifierForSubkeyOnInstance(descentInstance, keyComponent, remainingKeypath, leafNotifier);
}

auto notifier(std::make_shared<NSKVOForwardingNotifier>(
chainedNotifier, std::bind(&NSKVOClass::_notifierForSubkeyOnInstance, std::placeholders::_1, remainingKeypath, leafNotifier)));
auto notifier(std::make_shared<NSKVOForwardingNotifier>(keyComponent,
chainedNotifier, std::bind(&NSKVOClass::_notifierForSubkeyOnInstance, std::placeholders::_1, keyComponent, remainingKeypath, leafNotifier)));

const auto& dependingKeys = valueDependingKeys(keyComponent);
for (const auto& dependingKey : dependingKeys) {
Expand Down Expand Up @@ -489,7 +510,7 @@ auto notifier(std::make_shared<NSKVOForwardingNotifier>(
lockedNotifier->dispatch(prior);
if (!prior) {
// TODO(DH): Look at caching these values, and only update if necessary.
lockedNotifier->updateLinks(valueForKey(instance, key));
lockedNotifier->updateLinks(instance);
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions Frameworks/include/NSKeyValueObserving-Internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ struct std::hash<std::tuple<id, std::string>> {
// should be removed.
struct NSKVOMatchAnyContextTag {};

class NSKVONotifier {
class NSKVONotifier: public std::enable_shared_from_this<NSKVONotifier> {
public:
// dispatch a notification towards the observer.
virtual void dispatch(bool) = 0;
Expand Down Expand Up @@ -74,6 +74,7 @@ struct NSKVOClass {

void _removeNotifier(id instance, const std::string& key, const NSKVONotifier* notifier);
static std::shared_ptr<NSKVONotifier> _notifierForSubkeyOnInstance(id newValue,
const std::string& keyComponent,
const std::string& keypath,
const std::shared_ptr<NSKVONotifier>& leafNotifier);
std::shared_ptr<NSKVONotifier> _linkedNotifierForLeaf(id instance,
Expand All @@ -87,7 +88,7 @@ struct NSKVOClass {

const std::unordered_set<std::string>& valueDependingKeys(const std::string& key);

id valueForKey(id instance, const std::string& key) {
static id valueForKey(id instance, const std::string& key) {
return [instance valueForKeyPath:[NSString stringWithUTF8String:key.c_str()]];
}

Expand Down
Loading

0 comments on commit 613a831

Please sign in to comment.