Update IPC message handling for SecurityKeyIpcClient class

remoting/host/security_key/security_key_ipc_client_unittest.cc

Index: remoting/host/security_key/security_key_ipc_client_unittest.cc
diff --git a/remoting/host/security_key/security_key_ipc_client_unittest.cc b/remoting/host/security_key/security_key_ipc_client_unittest.cc
index 7b1cfd46126cc4e763dc67642d4ee1745555e21a..cf62fa31ade0692cd010edb8e0b3d24f9c7cfb71 100644
--- a/remoting/host/security_key/security_key_ipc_client_unittest.cc
+++ b/remoting/host/security_key/security_key_ipc_client_unittest.cc
@@ -58,9 +58,10 @@ class SecurityKeyIpcClientTest : public testing::Test {
   void WaitForOperationComplete();
 
   // Sets up an active IPC connection between |security_key_ipc_client_|
-  // and |fake_ipc_server_|.  |expect_success| defines whether the operation
-  // is expected to succeed or fail.
-  void EstablishConnection(bool expect_success);
+  // and |fake_ipc_server_|.  |expect_connected| defines whether the operation
+  // is result in a usable IPC connection.  |expect_error| defines whether the
+  // the error callback should be invoked during the connection process.
+  void EstablishConnection(bool expect_connected, bool expect_error);
 
   // Sends a security key request from |security_key_ipc_client_| and
   // a response from |fake_ipc_server_| and verifies the payloads for both.
@@ -177,7 +178,8 @@ std::string SecurityKeyIpcClientTest::GenerateUniqueTestChannelName() {
          IPC::Channel::GenerateUniqueRandomChannelID();
 }
 
-void SecurityKeyIpcClientTest::EstablishConnection(bool expect_success) {
+void SecurityKeyIpcClientTest::EstablishConnection(bool expect_connected,
+                                                   bool expect_error) {
   // Start up the security key forwarding session IPC channel first, that way
   // we can provide the channel using the fake SecurityKeyAuthHandler later on.
   mojo::edk::NamedPlatformHandle channel_handle(
@@ -197,8 +199,8 @@ void SecurityKeyIpcClientTest::EstablishConnection(bool expect_success) {
       base::Bind(&SecurityKeyIpcClientTest::OperationComplete,
                  base::Unretained(this), /*failed=*/true));
   WaitForOperationComplete();
-  ASSERT_EQ(connection_established_, expect_success);
-  ASSERT_NE(operation_failed_, expect_success);
+  ASSERT_EQ(expect_connected, connection_established_);
+  ASSERT_EQ(expect_error, operation_failed_);
 }
 
 void SecurityKeyIpcClientTest::SendRequestAndResponse(
@@ -209,17 +211,17 @@ void SecurityKeyIpcClientTest::SendRequestAndResponse(
                                base::Unretained(this))));
   WaitForOperationComplete();
   ASSERT_FALSE(operation_failed_);
-  ASSERT_EQ(last_connection_id_received_, kTestConnectionId);
-  ASSERT_EQ(last_message_received_, request_data);
+  ASSERT_EQ(kTestConnectionId, last_connection_id_received_);
+  ASSERT_EQ(request_data, last_message_received_);
 
   ASSERT_TRUE(fake_ipc_server_.SendResponse(response_data));
   WaitForOperationComplete();
   ASSERT_FALSE(operation_failed_);
-  ASSERT_EQ(last_message_received_, response_data);
+  ASSERT_EQ(response_data, last_message_received_);
 }
 
 TEST_F(SecurityKeyIpcClientTest, GenerateSingleSecurityKeyRequest) {
-  EstablishConnection(/*expect_success=*/true);
+  EstablishConnection(/*expect_connected=*/true, /*expect_error=*/false);
 
   SendRequestAndResponse("Auth me!", "You've been authed!");
 
@@ -227,7 +229,7 @@ TEST_F(SecurityKeyIpcClientTest, GenerateSingleSecurityKeyRequest) {
 }
 
 TEST_F(SecurityKeyIpcClientTest, GenerateLargeSecurityKeyRequest) {
-  EstablishConnection(/*expect_success=*/true);
+  EstablishConnection(/*expect_connected=*/true, /*expect_error=*/false);
 
   SendRequestAndResponse(std::string(kLargeMessageSizeBytes, 'Y'),
                          std::string(kLargeMessageSizeBytes, 'Z'));
@@ -236,7 +238,7 @@ TEST_F(SecurityKeyIpcClientTest, GenerateLargeSecurityKeyRequest) {
 }
 
 TEST_F(SecurityKeyIpcClientTest, GenerateReallyLargeSecurityKeyRequest) {
-  EstablishConnection(/*expect_success=*/true);
+  EstablishConnection(/*expect_connected=*/true, /*expect_error=*/false);
 
   SendRequestAndResponse(std::string(kLargeMessageSizeBytes * 2, 'Y'),
                          std::string(kLargeMessageSizeBytes * 2, 'Z'));
@@ -245,7 +247,7 @@ TEST_F(SecurityKeyIpcClientTest, GenerateReallyLargeSecurityKeyRequest) {
 }
 
 TEST_F(SecurityKeyIpcClientTest, GenerateMultipleSecurityKeyRequest) {
-  EstablishConnection(/*expect_success=*/true);
+  EstablishConnection(/*expect_connected=*/true, /*expect_error=*/false);
 
   SendRequestAndResponse("Auth me 1!", "You've been authed once!");
   SendRequestAndResponse("Auth me 2!", "You've been authed twice!");
@@ -255,7 +257,7 @@ TEST_F(SecurityKeyIpcClientTest, GenerateMultipleSecurityKeyRequest) {
 }
 
 TEST_F(SecurityKeyIpcClientTest, ServerClosesConnectionAfterRequestTimeout) {
-  EstablishConnection(/*expect_success=*/true);
+  EstablishConnection(/*expect_connected=*/true, /*expect_error=*/false);
   fake_ipc_server_.CloseChannel();
   WaitForOperationComplete();
   ASSERT_FALSE(operation_failed_);
@@ -263,7 +265,7 @@ TEST_F(SecurityKeyIpcClientTest, ServerClosesConnectionAfterRequestTimeout) {
 
 TEST_F(SecurityKeyIpcClientTest,
        SecondSecurityKeyRequestBeforeFirstResponseReceived) {
-  EstablishConnection(/*expect_success=*/true);
+  EstablishConnection(/*expect_connected=*/true, /*expect_error=*/false);
 
   ASSERT_TRUE(security_key_ipc_client_.SendSecurityKeyRequest(
       "First Request",
@@ -279,7 +281,7 @@ TEST_F(SecurityKeyIpcClientTest,
 }
 
 TEST_F(SecurityKeyIpcClientTest, ReceiveSecurityKeyResponseWithEmptyPayload) {
-  EstablishConnection(/*expect_success=*/true);
+  EstablishConnection(/*expect_connected=*/true, /*expect_error=*/false);
 
   ASSERT_TRUE(security_key_ipc_client_.SendSecurityKeyRequest(
       "Valid request",
@@ -316,16 +318,90 @@ TEST_F(SecurityKeyIpcClientTest, NonExistentIpcServerChannel) {
   ASSERT_TRUE(operation_failed_);
 }
 
+TEST_F(SecurityKeyIpcClientTest, MultipleConnectionReadyMessagesReceived) {
+  EstablishConnection(/*expect_connected=*/true, /*expect_error=*/false);
+  ASSERT_FALSE(operation_failed_);
+
+  // Send a second ConnectionReady message to trigger the error callback.
+  SendConnectionMessage();
+  WaitForOperationComplete();
+  ASSERT_TRUE(operation_failed_);
+
+  // Send a third message to ensure no crash occurs both callbacks will have
+  // been called and cleared so we don't wait for the operation to complete.
+  SendConnectionMessage();
+  ASSERT_TRUE(operation_failed_);
+}
+
+TEST_F(SecurityKeyIpcClientTest, UnexpectedInvalidSessionMessagesReceived) {
+  EstablishConnection(/*expect_connected=*/true, /*expect_error=*/false);
+  ASSERT_FALSE(operation_failed_);
+
+  // Send an InvalidSession message to trigger the error callback.
+  simulate_invalid_session_ = true;
+  SendConnectionMessage();
+  WaitForOperationComplete();
+  ASSERT_TRUE(operation_failed_);
+
+  // Send a third message to ensure no crash occurs both callbacks will have
+  // been called and cleared so we don't wait for the operation to complete.
+  SendConnectionMessage();
+  ASSERT_TRUE(operation_failed_);
+}
+
 #if defined(OS_WIN)
 TEST_F(SecurityKeyIpcClientTest, SecurityKeyIpcServerRunningInWrongSession) {
   // Set the expected session Id to a different session than we are running in.
   security_key_ipc_client_.SetExpectedIpcServerSessionIdForTest(session_id_ +
                                                                 1);
+
+  // Attempting to establish a connection should fail here since the IPC Server
+  // is 'running' in a different session than expected.  The success callback
+  // will be called by the IPC server since it thinks the connection is valid,
+  // but we will have already started tearing down the connection since it
+  // failed at the client end.
+  EstablishConnection(/*expect_connected=*/true, /*expect_error=*/true);
+}
+
+TEST_F(SecurityKeyIpcClientTest, SecurityKeyIpcClientRunningInWrongSession) {
+  // Attempting to establish a connection should fail here since the IPC Client
+  // is 'running' in the non-remoted session.
   simulate_invalid_session_ = true;
+  EstablishConnection(/*expect_connected=*/false, /*expect_error=*/false);
+}
 
+TEST_F(SecurityKeyIpcClientTest, MultipleInvalidSessionMessagesReceived) {
   // Attempting to establish a connection should fail here since the IPC Server
   // is 'running' in a different session than expected.
-  EstablishConnection(/*expect_success=*/false);
+  simulate_invalid_session_ = true;
+  EstablishConnection(/*expect_connected=*/false, /*expect_error=*/false);
+
+  SendConnectionMessage();
+  WaitForOperationComplete();
+  ASSERT_TRUE(operation_failed_);
+
+  // Send a third message to ensure no crash occurs both callbacks will have
+  // been called and cleared so we don't wait for the operation to complete.
+  SendConnectionMessage();
+  ASSERT_TRUE(operation_failed_);
+}
+
+TEST_F(SecurityKeyIpcClientTest, UnexpectedConnectionReadyMessagesReceived) {
+  // Attempting to establish a connection should fail here since the IPC Server
+  // is 'running' in a different session than expected.
+  simulate_invalid_session_ = true;
+  EstablishConnection(/*expect_connected=*/false, /*expect_error=*/false);
+
+  // Send an InvalidSession message to trigger the error callback.
+  simulate_invalid_session_ = false;
+  SendConnectionMessage();
+  WaitForOperationComplete();
+  ASSERT_TRUE(operation_failed_);
+
+  // Send a third message to ensure no crash occurs both callbacks will have
+  // been called and cleared so we don't wait for the operation to complete.
+  SendConnectionMessage();
+  ASSERT_TRUE(operation_failed_);
 }
 #endif  // defined(OS_WIN)