Update IPC message handling for SecurityKeyIpcClient class

remoting/host/security_key/security_key_ipc_client_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 "remoting/host/security_key/security_key_ipc_client.h"
 
 #include <memory>
 #include <string>
 
 #include "base/bind.h"
@@ skipping 40 matching lines @@
   void ClientMessageReceived(const std::string& response_payload);
 
  protected:
   // testing::Test interface.
   void SetUp() override;
 
   // Waits until the current |run_loop_| instance is signaled, then resets it.
   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.
   void SendRequestAndResponse(const std::string& request_data,
                               const std::string& response_data);
 
   // Creates a unique IPC channel name to use for testing.
   std::string GenerateUniqueTestChannelName();
 
   // IPC tests require a valid MessageLoop to run.
@@ skipping 96 matching lines @@
     const std::string& response_payload) {
   last_message_received_ = response_payload;
   OperationComplete(/*failed=*/false);
 }
 
 std::string SecurityKeyIpcClientTest::GenerateUniqueTestChannelName() {
   return GetChannelNamePathPrefixForTest() + kValidIpcChannelName +
          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(
       GenerateUniqueTestChannelName());
   security_key_ipc_client_.SetIpcChannelHandleForTest(channel_handle);
   ASSERT_TRUE(fake_ipc_server_.CreateChannel(
       channel_handle,
       /*request_timeout=*/base::TimeDelta::FromMilliseconds(500)));
 
   ASSERT_TRUE(security_key_ipc_client_.CheckForSecurityKeyIpcServerChannel());
 
   // Establish the IPC channel so we can begin sending and receiving security
   // key messages.
   security_key_ipc_client_.EstablishIpcConnection(
       base::Bind(&SecurityKeyIpcClientTest::ConnectionStateHandler,
                  base::Unretained(this)),
       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(
     const std::string& request_data,
     const std::string& response_data) {
   ASSERT_TRUE(security_key_ipc_client_.SendSecurityKeyRequest(
       request_data, base::Bind(&SecurityKeyIpcClientTest::ClientMessageReceived,
                                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!");
 
   security_key_ipc_client_.CloseIpcConnection();
 }
 
 TEST_F(SecurityKeyIpcClientTest, GenerateLargeSecurityKeyRequest) {
-  EstablishConnection(/*expect_success=*/true);
+  EstablishConnection(/*expect_connected=*/true, /*expect_error=*/false);
 
   SendRequestAndResponse(std::string(kLargeMessageSizeBytes, 'Y'),
                          std::string(kLargeMessageSizeBytes, 'Z'));
 
   security_key_ipc_client_.CloseIpcConnection();
 }
 
 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'));
 
   security_key_ipc_client_.CloseIpcConnection();
 }
 
 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!");
   SendRequestAndResponse("Auth me 3!", "You've been authed thrice!");
 
   security_key_ipc_client_.CloseIpcConnection();
 }
 
 TEST_F(SecurityKeyIpcClientTest, ServerClosesConnectionAfterRequestTimeout) {
-  EstablishConnection(/*expect_success=*/true);
+  EstablishConnection(/*expect_connected=*/true, /*expect_error=*/false);
   fake_ipc_server_.CloseChannel();
   WaitForOperationComplete();
   ASSERT_FALSE(operation_failed_);
 }
 
 TEST_F(SecurityKeyIpcClientTest,
        SecondSecurityKeyRequestBeforeFirstResponseReceived) {
-  EstablishConnection(/*expect_success=*/true);
+  EstablishConnection(/*expect_connected=*/true, /*expect_error=*/false);
 
   ASSERT_TRUE(security_key_ipc_client_.SendSecurityKeyRequest(
       "First Request",
       base::Bind(&SecurityKeyIpcClientTest::ClientMessageReceived,
                  base::Unretained(this))));
   WaitForOperationComplete();
   ASSERT_FALSE(operation_failed_);
 
   ASSERT_FALSE(security_key_ipc_client_.SendSecurityKeyRequest(
       "Second Request",
       base::Bind(&SecurityKeyIpcClientTest::ClientMessageReceived,
                  base::Unretained(this))));
 }
 
 TEST_F(SecurityKeyIpcClientTest, ReceiveSecurityKeyResponseWithEmptyPayload) {
-  EstablishConnection(/*expect_success=*/true);
+  EstablishConnection(/*expect_connected=*/true, /*expect_error=*/false);
 
   ASSERT_TRUE(security_key_ipc_client_.SendSecurityKeyRequest(
       "Valid request",
       base::Bind(&SecurityKeyIpcClientTest::ClientMessageReceived,
                  base::Unretained(this))));
   WaitForOperationComplete();
   ASSERT_FALSE(operation_failed_);
 
   ASSERT_TRUE(fake_ipc_server_.SendResponse(""));
   WaitForOperationComplete();
@@ skipping 16 matching lines @@
   // not exist).
   security_key_ipc_client_.EstablishIpcConnection(
       base::Bind(&SecurityKeyIpcClientTest::ConnectionStateHandler,
                  base::Unretained(this)),
       base::Bind(&SecurityKeyIpcClientTest::OperationComplete,
                  base::Unretained(this), /*failed=*/true));
   WaitForOperationComplete();
   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);
-  simulate_invalid_session_ = true;
 
   // 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)
 
 }  // namespace remoting