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

chromeos/components/tether/ble_connection_manager_unittest.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chromeos/components/tether/ble_connection_manager.h"
 
 #include "base/logging.h"
 #include "base/timer/mock_timer.h"
 #include "chromeos/components/tether/ble_constants.h"
 #include "chromeos/components/tether/proto/tether.pb.h"
@@ skipping 667 matching lines @@
       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 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_,
               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]));
 
@@ skipping 496 matching lines @@
        cryptauth::SecureChannel::Status::DISCONNECTED},
       {test_devices_[1], cryptauth::SecureChannel::Status::CONNECTING,
        cryptauth::SecureChannel::Status::DISCONNECTED},
       {test_devices_[2], cryptauth::SecureChannel::Status::CONNECTING,
        cryptauth::SecureChannel::Status::DISCONNECTED}});
 }
 
 }  // namespace tether
 
 }  // namespace cryptauth