CRBProtocolObservers can now be mutated while forwarding methods.

base/ios/crb_protocol_observers_unittest.mm

Index: base/ios/crb_protocol_observers_unittest.mm
diff --git a/base/ios/crb_protocol_observers_unittest.mm b/base/ios/crb_protocol_observers_unittest.mm
index d235c98b21f0919a2742cd40eb07e371486a5345..5f1105122943f27143e484e30f400e422a576bf6 100644
--- a/base/ios/crb_protocol_observers_unittest.mm
+++ b/base/ios/crb_protocol_observers_unittest.mm
@@ -18,6 +18,10 @@
 
 @optional
 - (void)optionalMethod;
+- (void)mutateByAddingObserver:(id<TestObserver>)observer;
+- (void)mutateByRemovingObserver:(id<TestObserver>)observer;
+- (void)nestedMutateByAddingObserver:(id<TestObserver>)observer;
+- (void)nestedMutateByRemovingObserver:(id<TestObserver>)observer;
 
 @end
 
@@ -31,6 +35,13 @@
 @property(nonatomic, readonly) BOOL optionalMethodInvoked;
 @end
 
+@interface TestMutateObserver : TestCompleteObserver
+
+- (instancetype)initWithObserver:(CRBProtocolObservers*)observer
+    NS_DESIGNATED_INITIALIZER;
+
+@end
+
 namespace {
 
 class CRBProtocolObserversTest : public PlatformTest {
@@ -50,11 +61,16 @@ class CRBProtocolObserversTest : public PlatformTest {
     complete_observer_.reset([[TestCompleteObserver alloc] init]);
     EXPECT_FALSE([complete_observer_ requiredMethodInvoked]);
     EXPECT_FALSE([complete_observer_ optionalMethodInvoked]);
+
+    mutate_observer_.reset(
+        [[TestMutateObserver alloc] initWithObserver:observers_.get()]);
+    EXPECT_FALSE([mutate_observer_ requiredMethodInvoked]);
   }
 
   base::scoped_nsobject<id> observers_;
   base::scoped_nsobject<TestPartialObserver> partial_observer_;
   base::scoped_nsobject<TestCompleteObserver> complete_observer_;
+  base::scoped_nsobject<TestMutateObserver> mutate_observer_;
 };
 
 // Verifies basic functionality of -[CRBProtocolObservers addObserver:] and
@@ -119,6 +135,86 @@ TEST_F(CRBProtocolObserversTest, WeakReference) {
   EXPECT_FALSE(weak_observer.get());
 }
 
+// Verifies that an observer can safely remove itself as observer while being
+// notified.
+TEST_F(CRBProtocolObserversTest, SelfMutateObservers) {
+  [observers_ addObserver:mutate_observer_];
+  EXPECT_FALSE([observers_ empty]);
+
+  [observers_ requiredMethod];
+  EXPECT_TRUE([mutate_observer_ requiredMethodInvoked]);
+
+  [mutate_observer_ reset];
+
+  [observers_ nestedMutateByRemovingObserver:mutate_observer_];
+  EXPECT_FALSE([mutate_observer_ requiredMethodInvoked]);
+
+  [observers_ addObserver:partial_observer_];
+
+  [observers_ requiredMethod];
+  EXPECT_FALSE([mutate_observer_ requiredMethodInvoked]);
+  EXPECT_TRUE([partial_observer_ requiredMethodInvoked]);
+
+  [observers_ removeObserver:partial_observer_];
+  EXPECT_TRUE([observers_ empty]);
+}
+
+// Verifies that - [CRBProtocolObservers addObserver:] and
+// - [CRBProtocolObservers removeObserver:] can be called while methods are
+// being forwarded.
+TEST_F(CRBProtocolObserversTest, MutateObservers) {
+  // Indirectly add an observer while forwarding an observer method.
+  [observers_ addObserver:mutate_observer_];
+
+  [observers_ mutateByAddingObserver:partial_observer_];
+  EXPECT_FALSE([partial_observer_ requiredMethodInvoked]);
+
+  // Check that methods are correctly forwared to the indirectly added observer.
+  [mutate_observer_ reset];
+  [observers_ requiredMethod];
+  EXPECT_TRUE([mutate_observer_ requiredMethodInvoked]);
+  EXPECT_TRUE([partial_observer_ requiredMethodInvoked]);
+
+  [mutate_observer_ reset];
+  [partial_observer_ reset];
+
+  // Indirectly remove an observer while forwarding an observer method.
+  [observers_ mutateByRemovingObserver:partial_observer_];
+
+  // Check that method is not forwared to the indirectly removed observer.
+  [observers_ requiredMethod];
+  EXPECT_TRUE([mutate_observer_ requiredMethodInvoked]);
+  EXPECT_FALSE([partial_observer_ requiredMethodInvoked]);
+}
+
+// Verifies that - [CRBProtocolObservers addObserver:] and
+// - [CRBProtocolObservers removeObserver:] can be called while methods are
+// being forwarded with a nested invocation depth > 0.
+TEST_F(CRBProtocolObserversTest, NestedMutateObservers) {
+  // Indirectly add an observer while forwarding an observer method.
+  [observers_ addObserver:mutate_observer_];
+
+  [observers_ nestedMutateByAddingObserver:partial_observer_];
+  EXPECT_FALSE([partial_observer_ requiredMethodInvoked]);
+
+  // Check that methods are correctly forwared to the indirectly added observer.
+  [mutate_observer_ reset];
+  [observers_ requiredMethod];
+  EXPECT_TRUE([mutate_observer_ requiredMethodInvoked]);
+  EXPECT_TRUE([partial_observer_ requiredMethodInvoked]);
+
+  [mutate_observer_ reset];
+  [partial_observer_ reset];
+
+  // Indirectly remove an observer while forwarding an observer method.
+  [observers_ nestedMutateByRemovingObserver:partial_observer_];
+
+  // Check that method is not forwared to the indirectly removed observer.
+  [observers_ requiredMethod];
+  EXPECT_TRUE([mutate_observer_ requiredMethodInvoked]);
+  EXPECT_FALSE([partial_observer_ requiredMethodInvoked]);
+}
+
 }  // namespace
 
 @implementation TestPartialObserver {
@@ -157,3 +253,33 @@ TEST_F(CRBProtocolObserversTest, WeakReference) {
 }
 
 @end
+
+@implementation TestMutateObserver {
+  __weak id _observers;
+}
+
+- (instancetype)initWithObserver:(CRBProtocolObservers*)observers {
+  self = [super init];
+  if (self) {
+    _observers = observers;
+  }
+  return self;
+}
+
+- (void)mutateByAddingObserver:(id<TestObserver>)observer {
+  [_observers addObserver:observer];
+}
+
+- (void)mutateByRemovingObserver:(id<TestObserver>)observer {
+  [_observers removeObserver:observer];
+}
+
+- (void)nestedMutateByAddingObserver:(id<TestObserver>)observer {
+  [_observers mutateByAddingObserver:observer];
+}
+
+- (void)nestedMutateByRemovingObserver:(id<TestObserver>)observer {
+  [_observers mutateByRemovingObserver:observer];
+}
+
+@end