Use ChannelMojo for remote security key channels.

remoting/host/security_key/security_key_auth_handler_win_unittest.cc

Index: remoting/host/security_key/security_key_auth_handler_win_unittest.cc
diff --git a/remoting/host/security_key/security_key_auth_handler_win_unittest.cc b/remoting/host/security_key/security_key_auth_handler_win_unittest.cc
index af5c5b8c3d2d91a1e2439e4c9b08382b1276d789..7716f0ee0238138d0fb458fdf3cc6dbdc52c45e6 100644
--- a/remoting/host/security_key/security_key_auth_handler_win_unittest.cc
+++ b/remoting/host/security_key/security_key_auth_handler_win_unittest.cc
@@ -17,6 +17,7 @@
 #include "ipc/ipc_listener.h"
 #include "ipc/ipc_message.h"
 #include "ipc/ipc_message_macros.h"
+#include "mojo/edk/test/scoped_ipc_support.h"
 #include "remoting/host/host_mock_objects.h"
 #include "remoting/host/security_key/fake_security_key_ipc_client.h"
 #include "remoting/host/security_key/fake_security_key_ipc_server.h"
@@ -49,18 +50,20 @@ class SecurityKeyAuthHandlerWinTest : public testing::Test {
   void SendMessageToClient(int connection_id, const std::string& data);
 
   // Creates a new security key connection on the object under test.
-  void CreateSecurityKeyConnection(const std::string& channel_name);
+  void CreateSecurityKeyConnection(
+      const mojo::edk::NamedPlatformHandle& channel_handle);
 
   // Sets |desktop_session_id_| to the id for the current Windows session.
   void InitializeDesktopSessionId();
 
-  // Uses |fake_ipc_client| to connect to the initial IPC server channel, it
-  // then validates internal state of the object under test and closes the
-  // connection based on |close_connection|.
-  void EstablishInitialIpcConnection(FakeSecurityKeyIpcClient* fake_ipc_client,
-                                     int expected_connection_id,
-                                     const std::string& channel_name,
-                                     bool close_connection);
+  // Uses |fake_ipc_client| to connect to the IPC server channel, it then
+  // validates internal state of the object under test and closes the connection
+  // based on |close_connection|.
+  void EstablishIpcConnection(
+      FakeSecurityKeyIpcClient* fake_ipc_client,
+      int expected_connection_id,
+      const mojo::edk::NamedPlatformHandle& channel_handle,
+      bool close_connection);
 
   // Sends a security key response message using |fake_ipc_server| and
   // validates the state of the object under test.
@@ -81,13 +84,15 @@ class SecurityKeyAuthHandlerWinTest : public testing::Test {
       const base::WeakPtr<FakeSecurityKeyIpcServer>& fake_ipc_server,
       int connection_id);
 
-  // Returns a unique IPC channel name which prevents conflicts when running
+  // Returns a unique IPC channel handle which prevents conflicts when running
   // tests concurrently.
-  std::string GetUniqueTestChannelName();
+  std::string GetUniqueTestChannelHandle();
 
   // IPC tests require a valid MessageLoop to run.
   base::MessageLoopForIO message_loop_;
 
+  mojo::edk::test::ScopedIPCSupport ipc_support_;
+
   // Used to allow |message_loop_| to run during tests.  The instance is reset
   // after each stage of the tests has been completed.
   std::unique_ptr<base::RunLoop> run_loop_;
@@ -118,7 +123,8 @@ class SecurityKeyAuthHandlerWinTest : public testing::Test {
 };
 
 SecurityKeyAuthHandlerWinTest::SecurityKeyAuthHandlerWinTest()
-    : run_loop_(new base::RunLoop()) {
+    : ipc_support_(message_loop_.task_runner()),
+      run_loop_(new base::RunLoop()) {
   auth_handler_ = remoting::SecurityKeyAuthHandler::Create(
       &mock_client_session_details_,
       base::Bind(&SecurityKeyAuthHandlerWinTest::SendMessageToClient,
@@ -146,14 +152,13 @@ void SecurityKeyAuthHandlerWinTest::SendMessageToClient(
 }
 
 void SecurityKeyAuthHandlerWinTest::CreateSecurityKeyConnection(
-    const std::string& channel_name) {
+    const mojo::edk::NamedPlatformHandle& channel_handle) {
   ASSERT_EQ(0u, auth_handler_->GetActiveConnectionCountForTest());
 
-  remoting::SetSecurityKeyIpcChannelNameForTest(channel_name);
+  remoting::SetSecurityKeyIpcChannelForTest(channel_handle);
 
   // Create a new SecurityKey IPC Server connection.
   auth_handler_->CreateSecurityKeyConnection();
-  ASSERT_TRUE(IPC::Channel::IsNamedServerInitialized(channel_name));
 
   InitializeDesktopSessionId();
 }
@@ -166,25 +171,18 @@ void SecurityKeyAuthHandlerWinTest::InitializeDesktopSessionId() {
       .WillByDefault(testing::Return(desktop_session_id_));
 }
 
-void SecurityKeyAuthHandlerWinTest::EstablishInitialIpcConnection(
+void SecurityKeyAuthHandlerWinTest::EstablishIpcConnection(
     FakeSecurityKeyIpcClient* fake_ipc_client,
     int expected_connection_id,
-    const std::string& channel_name,
+    const mojo::edk::NamedPlatformHandle& channel_handle,
     bool close_connection) {
   size_t expected_connection_count =
       auth_handler_->GetActiveConnectionCountForTest() + 1;
 
-  ASSERT_TRUE(fake_ipc_client->ConnectViaIpc(channel_name));
-  // Client and Server will each signal us once when OnChannelConenect() is
-  // called so we wait on complete twice.  The order in which each is signaled
-  // is not important.
-  WaitForOperationComplete();
+  ASSERT_FALSE(auth_handler_->IsValidConnectionId(expected_connection_id));
+  ASSERT_TRUE(fake_ipc_client->ConnectViaIpc(channel_handle));
   WaitForOperationComplete();
 
-  // Verify the connection details have been passed to the client.
-  std::string new_channel_name = fake_ipc_client->last_message_received();
-  ASSERT_FALSE(new_channel_name.empty());
-
   // Verify the internal state of the SecurityKeyAuthHandler is correct.
   ASSERT_TRUE(auth_handler_->IsValidConnectionId(expected_connection_id));
   ASSERT_EQ(expected_connection_count,
@@ -256,7 +254,7 @@ void SecurityKeyAuthHandlerWinTest::CloseSecurityKeySessionIpcChannel(
   ASSERT_FALSE(fake_ipc_server.get());
 }
 
-std::string SecurityKeyAuthHandlerWinTest::GetUniqueTestChannelName() {
+std::string SecurityKeyAuthHandlerWinTest::GetUniqueTestChannelHandle() {
   std::string channel_name("Uber_Awesome_Super_Mega_Test_Channel.");
   channel_name.append(IPC::Channel::GenerateUniqueRandomChannelID());
 
@@ -264,24 +262,22 @@ std::string SecurityKeyAuthHandlerWinTest::GetUniqueTestChannelName() {
 }
 
 TEST_F(SecurityKeyAuthHandlerWinTest, HandleSingleSecurityKeyRequest) {
-  std::string channel_name(GetUniqueTestChannelName());
-  CreateSecurityKeyConnection(channel_name);
+  mojo::edk::NamedPlatformHandle channel_handle(GetUniqueTestChannelHandle());
+  CreateSecurityKeyConnection(channel_handle);
+
+  ASSERT_FALSE(auth_handler_->IsValidConnectionId(kConnectionId1));
 
   // Create a fake client and connect to the IPC server channel.
   FakeSecurityKeyIpcClient fake_ipc_client(
       base::Bind(&SecurityKeyAuthHandlerWinTest::OperationComplete,
                  base::Unretained(this)));
-  EstablishInitialIpcConnection(&fake_ipc_client, kConnectionId1, channel_name,
-                                /*close_connection=*/true);
-
-  // Connect to the private IPC server channel created for this client.
-  std::string new_channel_name = fake_ipc_client.last_message_received();
+  EstablishIpcConnection(&fake_ipc_client, kConnectionId1, channel_handle,
+                         /*close_connection=*/true);
 
   // Retrieve the IPC server instance created when the client connected.
   base::WeakPtr<FakeSecurityKeyIpcServer> fake_ipc_server =
       ipc_server_factory_.GetIpcServerObject(kConnectionId1);
   ASSERT_TRUE(fake_ipc_server.get());
-  ASSERT_EQ(new_channel_name, fake_ipc_server->channel_name());
 
   fake_ipc_server->set_send_response_callback(
       base::Bind(&SecurityKeyAuthHandlerWinTest::OperationComplete,
@@ -299,8 +295,8 @@ TEST_F(SecurityKeyAuthHandlerWinTest, HandleSingleSecurityKeyRequest) {
 }
 
 TEST_F(SecurityKeyAuthHandlerWinTest, HandleConcurrentSecurityKeyRequests) {
-  std::string channel_name(GetUniqueTestChannelName());
-  CreateSecurityKeyConnection(channel_name);
+  mojo::edk::NamedPlatformHandle channel_handle(GetUniqueTestChannelHandle());
+  CreateSecurityKeyConnection(channel_handle);
 
   // Create fake clients and connect each to the IPC server channel.
   FakeSecurityKeyIpcClient fake_ipc_client_1(
@@ -310,27 +306,18 @@ TEST_F(SecurityKeyAuthHandlerWinTest, HandleConcurrentSecurityKeyRequests) {
       base::Bind(&SecurityKeyAuthHandlerWinTest::OperationComplete,
                  base::Unretained(this)));
 
-  EstablishInitialIpcConnection(&fake_ipc_client_1, kConnectionId1,
-                                channel_name,
-                                /*close_connection=*/true);
-  EstablishInitialIpcConnection(&fake_ipc_client_2, kConnectionId2,
-                                channel_name,
-                                /*close_connection=*/true);
-
-  // Verify the connection details have been passed to the client.
-  std::string channel_name_1 = fake_ipc_client_1.last_message_received();
-  std::string channel_name_2 = fake_ipc_client_2.last_message_received();
-  ASSERT_NE(channel_name_1, channel_name_2);
+  EstablishIpcConnection(&fake_ipc_client_1, kConnectionId1, channel_handle,
+                         /*close_connection=*/true);
+  EstablishIpcConnection(&fake_ipc_client_2, kConnectionId2, channel_handle,
+                         /*close_connection=*/true);
 
   base::WeakPtr<FakeSecurityKeyIpcServer> fake_ipc_server_1 =
       ipc_server_factory_.GetIpcServerObject(kConnectionId1);
   ASSERT_TRUE(fake_ipc_server_1.get());
-  ASSERT_EQ(channel_name_1, fake_ipc_server_1->channel_name());
 
   base::WeakPtr<FakeSecurityKeyIpcServer> fake_ipc_server_2 =
       ipc_server_factory_.GetIpcServerObject(kConnectionId2);
   ASSERT_TRUE(fake_ipc_server_2.get());
-  ASSERT_EQ(channel_name_2, fake_ipc_server_2->channel_name());
 
   fake_ipc_server_1->set_send_response_callback(
       base::Bind(&SecurityKeyAuthHandlerWinTest::OperationComplete,
@@ -361,25 +348,20 @@ TEST_F(SecurityKeyAuthHandlerWinTest, HandleConcurrentSecurityKeyRequests) {
 }
 
 TEST_F(SecurityKeyAuthHandlerWinTest, HandleSequentialSecurityKeyRequests) {
-  std::string channel_name(GetUniqueTestChannelName());
-  CreateSecurityKeyConnection(channel_name);
+  mojo::edk::NamedPlatformHandle channel_handle(GetUniqueTestChannelHandle());
+  CreateSecurityKeyConnection(channel_handle);
 
   // Create fake clients to connect to the IPC server channel.
   FakeSecurityKeyIpcClient fake_ipc_client_1(
       base::Bind(&SecurityKeyAuthHandlerWinTest::OperationComplete,
                  base::Unretained(this)));
 
-  EstablishInitialIpcConnection(&fake_ipc_client_1, kConnectionId1,
-                                channel_name,
-                                /*close_connection=*/true);
-
-  // Verify the connection details have been passed to the client.
-  std::string channel_name_1 = fake_ipc_client_1.last_message_received();
+  EstablishIpcConnection(&fake_ipc_client_1, kConnectionId1, channel_handle,
+                         /*close_connection=*/true);
 
   base::WeakPtr<FakeSecurityKeyIpcServer> fake_ipc_server_1 =
       ipc_server_factory_.GetIpcServerObject(kConnectionId1);
   ASSERT_TRUE(fake_ipc_server_1.get());
-  ASSERT_EQ(channel_name_1, fake_ipc_server_1->channel_name());
 
   fake_ipc_server_1->set_send_response_callback(
       base::Bind(&SecurityKeyAuthHandlerWinTest::OperationComplete,
@@ -400,17 +382,12 @@ TEST_F(SecurityKeyAuthHandlerWinTest, HandleSequentialSecurityKeyRequests) {
   FakeSecurityKeyIpcClient fake_ipc_client_2(
       base::Bind(&SecurityKeyAuthHandlerWinTest::OperationComplete,
                  base::Unretained(this)));
-  EstablishInitialIpcConnection(&fake_ipc_client_2, kConnectionId2,
-                                channel_name,
-                                /*close_connection=*/true);
-
-  std::string channel_name_2 = fake_ipc_client_2.last_message_received();
-  ASSERT_NE(channel_name_1, channel_name_2);
+  EstablishIpcConnection(&fake_ipc_client_2, kConnectionId2, channel_handle,
+                         /*close_connection=*/true);
 
   base::WeakPtr<FakeSecurityKeyIpcServer> fake_ipc_server_2 =
       ipc_server_factory_.GetIpcServerObject(kConnectionId2);
   ASSERT_TRUE(fake_ipc_server_2.get());
-  ASSERT_EQ(channel_name_2, fake_ipc_server_2->channel_name());
 
   fake_ipc_server_2->set_send_response_callback(
       base::Bind(&SecurityKeyAuthHandlerWinTest::OperationComplete,
@@ -428,86 +405,22 @@ TEST_F(SecurityKeyAuthHandlerWinTest, HandleSequentialSecurityKeyRequests) {
   CloseSecurityKeySessionIpcChannel(fake_ipc_server_2, kConnectionId2);
 }
 
-TEST_F(SecurityKeyAuthHandlerWinTest,
-       ClientNeverDisconnectsFromInitialIpcChannel) {
-  const int kLowConnectionTimeoutInMs = 25;
-  auth_handler_->SetRequestTimeoutForTest(
-      base::TimeDelta::FromMilliseconds(kLowConnectionTimeoutInMs));
-
-  std::string channel_name(GetUniqueTestChannelName());
-  CreateSecurityKeyConnection(channel_name);
-
-  // Create a fake client and connect to the IPC server channel.
-  FakeSecurityKeyIpcClient fake_ipc_client(
-      base::Bind(&SecurityKeyAuthHandlerWinTest::OperationComplete,
-                 base::Unretained(this)));
-  EstablishInitialIpcConnection(&fake_ipc_client, kConnectionId1, channel_name,
-                                /*close_connection=*/false);
-
-  // Don't close the channel here, instead wait for the SecurityKeyAuthHandler
-  // to close the connection due to the timeout.
-  WaitForOperationComplete();
-
-  // Verify the connection that was set up still exists.
-  ASSERT_TRUE(auth_handler_->IsValidConnectionId(kConnectionId1));
-  ASSERT_EQ(1u, auth_handler_->GetActiveConnectionCountForTest());
-
-  // Attempt to connect again after the error.
-  EstablishInitialIpcConnection(&fake_ipc_client, kConnectionId2, channel_name,
-                                /*close_connection=*/true);
-}
-
-TEST_F(SecurityKeyAuthHandlerWinTest, HandleSecurityKeyRequestTimeout) {
-  std::string channel_name(GetUniqueTestChannelName());
-  CreateSecurityKeyConnection(channel_name);
-
-  // Create a fake client and connect to the IPC server channel.
-  FakeSecurityKeyIpcClient fake_ipc_client(
-      base::Bind(&SecurityKeyAuthHandlerWinTest::OperationComplete,
-                 base::Unretained(this)));
-  EstablishInitialIpcConnection(&fake_ipc_client, kConnectionId1, channel_name,
-                                /*close_connection=*/true);
-
-  // Connect to the private IPC server channel created for this client.
-  std::string new_channel_name = fake_ipc_client.last_message_received();
-
-  // Retrieve the IPC server instance created when the client connected.
-  base::WeakPtr<FakeSecurityKeyIpcServer> fake_ipc_server =
-      ipc_server_factory_.GetIpcServerObject(kConnectionId1);
-  ASSERT_TRUE(fake_ipc_server.get());
-  ASSERT_EQ(new_channel_name, fake_ipc_server->channel_name());
-
-  fake_ipc_server->set_send_response_callback(
-      base::Bind(&SecurityKeyAuthHandlerWinTest::OperationComplete,
-                 base::Unretained(this)));
-
-  // Simulate a timeout and verify the IPC server is cleaned up.
-  CloseSecurityKeySessionIpcChannel(fake_ipc_server, kConnectionId1);
-
-  // Attempt to connect again after the error.
-  EstablishInitialIpcConnection(&fake_ipc_client, kConnectionId2, channel_name,
-                                /*close_connection=*/true);
-}
-
 TEST_F(SecurityKeyAuthHandlerWinTest, HandleSecurityKeyErrorResponse) {
-  std::string channel_name(GetUniqueTestChannelName());
-  CreateSecurityKeyConnection(channel_name);
+  mojo::edk::NamedPlatformHandle channel_handle(GetUniqueTestChannelHandle());
+  CreateSecurityKeyConnection(channel_handle);
+  ASSERT_EQ(0u, auth_handler_->GetActiveConnectionCountForTest());
 
   // Create a fake client and connect to the IPC server channel.
   FakeSecurityKeyIpcClient fake_ipc_client(
       base::Bind(&SecurityKeyAuthHandlerWinTest::OperationComplete,
                  base::Unretained(this)));
-  EstablishInitialIpcConnection(&fake_ipc_client, kConnectionId1, channel_name,
-                                /*close_connection=*/true);
-
-  // Connect to the private IPC server channel created for this client.
-  std::string new_channel_name = fake_ipc_client.last_message_received();
+  EstablishIpcConnection(&fake_ipc_client, kConnectionId1, channel_handle,
+                         /*close_connection=*/true);
 
   // Retrieve the IPC server instance created when the client connected.
   base::WeakPtr<FakeSecurityKeyIpcServer> fake_ipc_server =
       ipc_server_factory_.GetIpcServerObject(kConnectionId1);
   ASSERT_TRUE(fake_ipc_server.get());
-  ASSERT_EQ(new_channel_name, fake_ipc_server->channel_name());
 
   fake_ipc_server->set_send_response_callback(
       base::Bind(&SecurityKeyAuthHandlerWinTest::OperationComplete,
@@ -528,28 +441,8 @@ TEST_F(SecurityKeyAuthHandlerWinTest, HandleSecurityKeyErrorResponse) {
   ASSERT_EQ(0u, auth_handler_->GetActiveConnectionCountForTest());
 
   // Attempt to connect again after the error.
-  EstablishInitialIpcConnection(&fake_ipc_client, kConnectionId2, channel_name,
-                                /*close_connection=*/true);
-}
-
-TEST_F(SecurityKeyAuthHandlerWinTest, IpcConnectionFailsFromInvalidSession) {

[joedow, 2016/11/03 22:25:18]

We still want this to work (i.e. Ipc connections from the wrong session should
be rejected).  Why was this test removed?

If the functionality is preserved and has just been moved, then the member and
function which tracks |desktop_sessions_id_| should also be removed.

[Sam McNally, 2016/11/04 02:51:09]

SecurityKeyAuthHandlerWin no longer receives initial connections.
SecurityKeyIpcServerImpl has the same check for its connections and that's still
tested.
Done.

-  std::string channel_name(GetUniqueTestChannelName());
-  CreateSecurityKeyConnection(channel_name);
-
-  // Set the current session id to a 'different' session.
-  desktop_session_id_ += 1;
-
-  // Create a fake client and connect to the IPC server channel.
-  FakeSecurityKeyIpcClient fake_ipc_client(
-      base::Bind(&SecurityKeyAuthHandlerWinTest::OperationComplete,
-                 base::Unretained(this)));
-  ASSERT_TRUE(fake_ipc_client.ConnectViaIpc(channel_name));
-  // Wait for the error callback to be signaled.
-  WaitForOperationComplete();
-
-  // Verify the connection was not set up.
-  ASSERT_FALSE(auth_handler_->IsValidConnectionId(kConnectionId1));
-  ASSERT_EQ(0u, auth_handler_->GetActiveConnectionCountForTest());
+  EstablishIpcConnection(&fake_ipc_client, kConnectionId2, channel_handle,
+                         /*close_connection=*/true);
 }
 
 }  // namespace remoting