BleConnectionManager: Check when receiving BLE advertisements if a SecureChannel already exists. No…

chromeos/components/tether/ble_connection_manager_unittest.cc

Index: chromeos/components/tether/ble_connection_manager_unittest.cc
diff --git a/chromeos/components/tether/ble_connection_manager_unittest.cc b/chromeos/components/tether/ble_connection_manager_unittest.cc
index 6ce9b41a3ec45201bbea31425c6c59c92372823b..4b5b7dd08c7bd2af197486e5123e36336aa98fed 100644
--- a/chromeos/components/tether/ble_connection_manager_unittest.cc
+++ b/chromeos/components/tether/ble_connection_manager_unittest.cc
@@ -685,6 +685,60 @@ TEST_F(BleConnectionManagerTest, TestSuccessfulConnection_SendAndReceive) {
   VerifyDeviceNotRegistered(test_devices_[0]);
 }
 
+// Test for fix to crbug.com/706640. This test will crash without the fix.

[Kyle Horimoto, 2017/04/06 21:17:30]

In addition to just testing that it doesn't crash, can you test that only one
channel is created?

[Ryan Hansberry, 2017/04/06 22:14:59]

Good call, done.

+TEST_F(BleConnectionManagerTest,
+       TestSuccessfulConnection_SendAndReceive_MultipleAdvertisementsReceived) {
+  EXPECT_CALL(*mock_ble_scanner_,
+              RegisterScanFilterForDevice(test_devices_[0]));
+  EXPECT_CALL(*mock_ble_advertiser_,
+              StartAdvertisingToDevice(test_devices_[0]));
+  EXPECT_CALL(*mock_ble_scanner_,
+              UnregisterScanFilterForDevice(test_devices_[0]));
+  EXPECT_CALL(*mock_ble_advertiser_, StopAdvertisingToDevice(test_devices_[0]));
+
+  manager_->RegisterRemoteDevice(test_devices_[0],
+                                 MessageType::TETHER_AVAILABILITY_REQUEST);
+  VerifyAdvertisingTimeoutSet(test_devices_[0]);
+  VerifyConnectionStateChanges(std::vector<SecureChannelStatusChange>{
+      {test_devices_[0], cryptauth::SecureChannel::Status::DISCONNECTED,
+       cryptauth::SecureChannel::Status::CONNECTING}});
+
+  fake_secure_channel_factory_->SetExpectedDeviceAddress(
+      std::string(kBluetoothAddress1));
+
+  // Simulate multiple advertisements being received:
+  mock_ble_scanner_->SimulateScanResults(std::string(kBluetoothAddress1),
+                                         test_devices_[0]);
+  mock_ble_scanner_->SimulateScanResults(std::string(kBluetoothAddress1),
+                                         test_devices_[0]);
+  mock_ble_scanner_->SimulateScanResults(std::string(kBluetoothAddress1),
+                                         test_devices_[0]);
+
+  FakeSecureChannel* channel = GetChannelForDevice(test_devices_[0]);
+  AuthenticateChannel(test_devices_[0]);
+
+  manager_->SendMessage(test_devices_[0], "request1");

[Kyle Horimoto, 2017/04/06 21:17:30]

Do we need the stuff from here onwards? Seems unrelated to this test. If you
want to keep it, maybe try simulating more scan results during this section to
ensure that nothing goes wrong.

[Ryan Hansberry, 2017/04/06 22:14:59]

Nope, no need for it. Removed.

+  VerifyLastMessageSent(channel, "request1", 1);
+
+  channel->ReceiveMessage(std::string(kTetherFeature), "response1");
+  VerifyReceivedMessages(
+      std::vector<ReceivedMessage>{{test_devices_[0], "response1"}});
+
+  manager_->SendMessage(test_devices_[0], "request2");
+  VerifyLastMessageSent(channel, "request2", 2);
+
+  channel->ReceiveMessage(std::string(kTetherFeature), "response2");
+  VerifyReceivedMessages(
+      std::vector<ReceivedMessage>{{test_devices_[0], "response2"}});
+
+  manager_->UnregisterRemoteDevice(test_devices_[0],
+                                   MessageType::TETHER_AVAILABILITY_REQUEST);
+  VerifyConnectionStateChanges(std::vector<SecureChannelStatusChange>{
+      {test_devices_[0], cryptauth::SecureChannel::Status::AUTHENTICATED,
+       cryptauth::SecureChannel::Status::DISCONNECTED}});
+  VerifyDeviceNotRegistered(test_devices_[0]);
+}
+
 TEST_F(BleConnectionManagerTest,
        TestSuccessfulConnection_MultipleConnectionReasons) {
   EXPECT_CALL(*mock_ble_scanner_,