Fix intermittent falure in Darwin browsing test.

We are running the test against Matter applications that are starting up, and
during startup we do some silly unregistering/re-registering of DNS-SD
advertisements.  The test was counting all the registrations, not subtracting
the un-registrations, and asserting that the resulting count was not too high.
But sometimes it is, if we get a "unregister/re-register" pair in there.

The fix:

1) Fixes the test to better track un-registration bits.
2) Fixes the OnBleScanRemove code to call the right delegate method.
3) Fixes OnBrowseRemove to not call the delegate if we have not notified about
   the add for the relevant result.  We could have just worked around this in
   the test, but notifying "removed" about an object with all fields set to nil
   just doesn't make much sense.

Author: bzbarsky-apple

src/darwin/Framework/CHIP/MTRCommissionableBrowser.mm

@@ -268,9 +268,14 @@ void OnBrowseRemove(DnssdService service) override
         // If there is nothing else to resolve for the given instance name, just remove it
         // too and informs the delegate that it is gone.
         if ([interfaces count] == 0) {
-            dispatch_async(mDispatchQueue, ^{
-                [mDelegate controller:mController didRemoveCommissionableDevice:result];
-            });
+            // If result.instanceName is nil, that means we never notified our
+            // delegate about this result (because we did not get that far in
+            // resolving it), so don't bother notifying about the removal either.
+            if (result.instanceName != nil) {
+                dispatch_async(mDispatchQueue, ^{
+                    [mDelegate controller:mController didRemoveCommissionableDevice:result];
+                });
+            }
 
             mDiscoveredResults[key] = nil;
         }
@@ -325,7 +330,7 @@ void OnBleScanRemove(BLE_CONNECTION_OBJECT connObj) override
         MATTER_LOG_METRIC(kMetricBLEDevicesRemoved, ++mBLEDevicesRemoved);
 
         dispatch_async(mDispatchQueue, ^{
-            [mDelegate controller:mController didFindCommissionableDevice:result];
+            [mDelegate controller:mController didRemoveCommissionableDevice:result];
         });
     }
 

src/darwin/Framework/CHIPTests/MTRCommissionableBrowserTests.m

@@ -36,9 +36,27 @@
 // Singleton controller we use.
 static MTRDeviceController * sController = nil;
 
+static NSString * kInstanceNameKey = @"instanceName";
+static NSString * kVendorIDKey = @"vendorID";
+static NSString * kProductIDKey = @"productID";
+static NSString * kDiscriminatorKey = @"discriminator";
+static NSString * kCommissioningModeKey = @"commissioningMode";
+
+static NSDictionary<NSString *, id> * ResultSnapshot(MTRCommissionableBrowserResult * result)
+{
+    return @{
+        kInstanceNameKey : result.instanceName,
+        kVendorIDKey : result.vendorID,
+        kProductIDKey : result.productID,
+        kDiscriminatorKey : result.discriminator,
+        kCommissioningModeKey : @(result.commissioningMode),
+    };
+}
+
 @interface DeviceScannerDelegate : NSObject <MTRCommissionableBrowserDelegate>
 @property (nonatomic, nullable) XCTestExpectation * expectation;
-@property (nonatomic) NSNumber * resultsCount;
+@property (nonatomic) NSMutableArray<NSDictionary<NSString *, id> *> * results;
+@property (nonatomic) NSMutableArray<NSDictionary<NSString *, id> *> * removedResults;
 
 - (instancetype)initWithExpectation:(XCTestExpectation *)expectation;
 - (void)controller:(MTRDeviceController *)controller didFindCommissionableDevice:(MTRCommissionableBrowserResult *)device;
@@ -52,8 +70,9 @@ - (instancetype)initWithExpectation:(XCTestExpectation *)expectation
         return nil;
     }
 
-    _resultsCount = 0;
     _expectation = expectation;
+    _results = [[NSMutableArray alloc] init];
+    _removedResults = [[NSMutableArray alloc] init];
     return self;
 }
 
@@ -63,8 +82,9 @@ - (instancetype)init
         return nil;
     }
 
-    _resultsCount = 0;
     _expectation = nil;
+    _results = [[NSMutableArray alloc] init];
+    _removedResults = [[NSMutableArray alloc] init];
     return self;
 }
 
@@ -77,12 +97,26 @@ - (void)controller:(MTRDeviceController *)controller didFindCommissionableDevice
         return;
     }
 
-    _resultsCount = @(_resultsCount.unsignedLongValue + 1);
-    if ([_resultsCount isEqual:@(kExpectedDiscoveredDevicesCount)]) {
+    __auto_type * snapshot = ResultSnapshot(device);
+
+    XCTAssertFalse([_results containsObject:snapshot], @"Newly discovered device %@ should not be in results already.", snapshot);
+
+    [_results addObject:snapshot];
+
+    if (_results.count == kExpectedDiscoveredDevicesCount) {
+        // Do some sanity checking on our results and removedResults to make
+        // sure we really only saw the relevant set of things.
+        NSSet<NSDictionary<NSString *, id> *> * finalResultsSet = [NSSet setWithArray:_results];
+        NSSet<NSDictionary<NSString *, id> *> * allResultsSet = [finalResultsSet copy];
+        allResultsSet = [allResultsSet setByAddingObjectsFromArray:_removedResults];
+
+        // Ensure that we just saw the same results as our final set popping in and out if things
+        // ever got removed here.
+        XCTAssertEqualObjects(finalResultsSet, allResultsSet);
         [self.expectation fulfill];
     }
 
-    XCTAssertLessThanOrEqual(_resultsCount.unsignedLongValue, kExpectedDiscoveredDevicesCount);
+    XCTAssertLessThanOrEqual(_results.count, kExpectedDiscoveredDevicesCount);
 
     __auto_type instanceName = device.instanceName;
     __auto_type vendorId = device.vendorID;
@@ -108,6 +142,12 @@ - (void)controller:(MTRDeviceController *)controller didRemoveCommissionableDevi
 
     NSLog(
         @"Removed Device (%@) with discriminator: %@ (vendor: %@, product: %@)", instanceName, discriminator, vendorId, productId);
+
+    __auto_type * snapshot = ResultSnapshot(device);
+    XCTAssertTrue([_results containsObject:snapshot], @"Removed device %@ is not something we found before", snapshot);
+
+    [_removedResults addObject:snapshot];
+    [_results removeObject:snapshot];
 }
 @end