Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5cc2a723 authored by Etienne Ruffieux's avatar Etienne Ruffieux
Browse files

Handle AVRCP SDP search collisions 

If remote tries to connect at the same time as the local
device the SDP search will fail. We need to handle this by
closing the incoming connection and letting the outgoing
connection process.

Bug: 298146372
Tag: #feature
Test: atest AvrcpConnectionHandlerTest --iterations 1000
Change-Id: Ie8218b7f0a9c16d7642b40f145ab173960089bf8
parent 0cfacee8

system/profile/avrcp/connection_handler.cc

+13 −3
Original line number Diff line number Diff line
@@ -395,9 +395,19 @@ void ConnectionHandler::AcceptorControlCb(uint8_t handle, uint8_t event, 
        }

      };



      SdpLookup(*peer_addr, base::Bind(sdp_lambda, this, handle), false);



      if (SdpLookup(*peer_addr, base::Bind(sdp_lambda, this, handle), false)) {

        avrc_->OpenBrowse(handle, AVCT_ACP);

      } else {

        // SDP search failed, this could be due to a collision between outgoing

        // and incoming connection. In any case, we need to reject the current

        // connection.

        LOG(ERROR) << __PRETTY_FUNCTION__

                   << ": SDP search failed for handle: " << loghex(handle)

                   << ", closing connection";

        DisconnectDevice(*peer_addr);

      }

      // Open for the next incoming connection. The handle will not be the same

      // as this one which will be closed when the device is disconnected.

      AvrcpConnect(false, RawAddress::kAny);

    } break;

system/profile/avrcp/tests/avrcp_connection_handler_test.cc

+116 −8
Original line number Diff line number Diff line
@@ -166,8 +166,10 @@ TEST_F(AvrcpConnectionHandlerTest, disconnectAfterCleanupTest) { 
  conn_cb.ctrl_cback.Run(1, AVRC_CLOSE_IND_EVT, 0, &RawAddress::kAny);

};



// Check that we can handle having a remote device connect to us, start SDP, and

// open another acceptor connection

/**

 * Check that we can handle having a remote device connect to us, start SDP, and

 * open another acceptor connection.

 */

TEST_F(AvrcpConnectionHandlerTest, remoteDeviceConnectionTest) {

  // Set an Expectation that Open will be called twice as an acceptor and save

  // the connection callback once it is called.
@@ -210,8 +212,10 @@ TEST_F(AvrcpConnectionHandlerTest, remoteDeviceConnectionTest) { 
  ConnectionHandler::CleanUp();

}



// Check that when a device does not support absolute volume, that the

// handler reports that via the volume interface.

/**

 *  Check that when a device does not support absolute volume, that the

 * handler reports that via the volume interface.

 */

TEST_F(AvrcpConnectionHandlerTest, noAbsoluteVolumeTest) {

  // Set an Expectation that Open will be called twice as an acceptor and save

  // the connection callback once it is called.
@@ -246,8 +250,10 @@ TEST_F(AvrcpConnectionHandlerTest, noAbsoluteVolumeTest) { 
  ConnectionHandler::CleanUp();

}



// Check that when a device does support absolute volume, that the handler

// doesn't report it. Instead that will be left up to the device.

/**

 *  Check that when a device does support absolute volume, that the handler

 * doesn't report it. Instead that will be left up to the device.

 */

TEST_F(AvrcpConnectionHandlerTest, absoluteVolumeTest) {

  // Set an Expectation that Open will be called twice as an acceptor and save

  // the connection callback once it is called.
@@ -314,8 +320,10 @@ TEST_F(AvrcpConnectionHandlerTest, disconnectTest) { 
  ConnectionHandler::CleanUp();

}



// Check that we can handle having a remote device connect to us, start SDP, and

// open another acceptor connection

/**

 * Check that we can handle having a remote device connect to us, start SDP, and

 * open another acceptor connection.

 */

TEST_F(AvrcpConnectionHandlerTest, multipleRemoteDeviceConnectionTest) {

  // Set an Expectation that Open will be called three times as an acceptor and

  // save the connection callback once it is called.
@@ -534,5 +542,105 @@ TEST_F(AvrcpConnectionHandlerTest, disconnectWhileDoingSdpTest) { 
  ConnectionHandler::CleanUp();

}



/**

 * Check that when an incoming connection happens at the same time as the

 * that the SDP search for initiator is running the collision is handled.

 */

TEST_F(AvrcpConnectionHandlerTest, connectionCollisionTest) {

  tAVRC_CONN_CB conn_cb;

  EXPECT_CALL(mock_avrcp_, Open(_, _, RawAddress::kAny))

      .Times(2)

      .WillOnce(

          DoAll(SetArgPointee<0>(1), SaveArgPointee<1>(&conn_cb), Return(0)))

      .WillOnce(

          DoAll(SetArgPointee<0>(2), SaveArgPointee<1>(&conn_cb), Return(0)));



  // Initialize the interface

  auto bound_callback = base::Bind(&MockFunction<void(device_ptr)>::Call,

                                   base::Unretained(&device_cb));

  ASSERT_TRUE(ConnectionHandler::Initialize(bound_callback, &mock_avrcp_,

                                            &mock_sdp_, &mock_volume_));

  connection_handler_ = ConnectionHandler::Get();



  // Check that the callback was sent with us as the acceptor

  ASSERT_EQ(conn_cb.conn, 1);



  // Set an Expectations that SDP will be performed

  tAVRC_FIND_CBACK sdp_cb;

  SetUpSdp(&sdp_cb, false, false);



  connection_handler_->ConnectDevice(RawAddress::kAny);



  // Set an expectation that a device will be created

  EXPECT_CALL(device_cb, Call(_)).Times(1);



  // Set an Expectations that SDP search will be performed again but will fail

  EXPECT_CALL(mock_avrcp_, FindService(_, _, _, _))

      .Times(1)

      .WillOnce(DoAll(SaveArg<3>(&sdp_cb), Return(1)));



  // Set an expectation that the incoming connection will be closed

  EXPECT_CALL(mock_avrcp_, Close(1));



  // Call the callback with a message saying that a remote device has connected

  conn_cb.ctrl_cback.Run(1, AVRC_OPEN_IND_EVT, 0, &RawAddress::kAny);



  // Set an expectation that cleanup will close the last connection

  EXPECT_CALL(mock_avrcp_, Close(_));



  // Run the SDP callback with status success

  sdp_cb.Run(0);



  connection_handler_ = nullptr;

  ConnectionHandler::CleanUp();

}



/**

 * Check that we are not proceeding with the connection if the SDP search

 * failed.

 */

TEST_F(AvrcpConnectionHandlerTest, acceptorSdpSearchFailTest) {

  // Set an Expectation that Open will be called twice as an acceptor and

  // save the connection callback once it is called.

  tAVRC_CONN_CB conn_cb;

  EXPECT_CALL(mock_avrcp_, Open(_, _, RawAddress::kAny))

      .Times(2)

      .WillOnce(

          DoAll(SetArgPointee<0>(1), SaveArgPointee<1>(&conn_cb), Return(0)))

      .WillOnce(

          DoAll(SetArgPointee<0>(2), SaveArgPointee<1>(&conn_cb), Return(0)));



  // Initialize the interface

  auto bound_callback = base::Bind(&MockFunction<void(device_ptr)>::Call,

                                   base::Unretained(&device_cb));

  ASSERT_TRUE(ConnectionHandler::Initialize(bound_callback, &mock_avrcp_,

                                            &mock_sdp_, &mock_volume_));

  connection_handler_ = ConnectionHandler::Get();



  // Check that the callback was sent with us as the acceptor

  ASSERT_EQ(conn_cb.conn, 1);



  // Set an expectation that a device will be created

  EXPECT_CALL(device_cb, Call(_)).Times(1);



  // Set an expectation that SDP search will be performed but will fail

  tAVRC_FIND_CBACK sdp_cb;

  EXPECT_CALL(mock_avrcp_, FindService(_, _, _, _))

      .Times(1)

      .WillOnce(DoAll(SaveArg<3>(&sdp_cb), Return(1)));



  // Set an expectation that the incoming connection will be closed

  EXPECT_CALL(mock_avrcp_, Close(1));



  // Call the callback with a message saying that a remote device has connected

  conn_cb.ctrl_cback.Run(1, AVRC_OPEN_IND_EVT, 0, &RawAddress::kAny);



  // Set an expectation that cleanup will close the last connection

  EXPECT_CALL(mock_avrcp_, Close(_));



  connection_handler_ = nullptr;

  ConnectionHandler::CleanUp();

}



}  // namespace avrcp

}  // namespace bluetooth