Adding the base ChromotingInstance implementation and unittests. This class will be used by the ap…

remoting/test/chromoting_instance_unittest.cc

+// Copyright 2015 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 <string>
+
+#include "base/message_loop/message_loop.h"
+#include "remoting/protocol/fake_connection_to_host.h"
+#include "remoting/protocol/protocol_mock_objects.h"
+#include "remoting/test/chromoting_instance.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+const char kTestUserName[] = "test_user@faux_address.com";
+const char kAccessToken[] = "faux_access_token";
+}
+
+namespace remoting {
+namespace test {
+
+using testing::_;
+
+// Provides base functionality for the ChromotingInstance Tests below.  This
+// fixture also creates an IO MessageLoop, if necessary, for use by the
+// ChromotingInstance.  Overrides a subset of the RemoteConnectionObserver
+// interface to track connection status changes for result verification.
+class ChromotingInstanceTest :
+    public ::testing::Test,
+    public RemoteConnectionObserver {
+ public:
+  ChromotingInstanceTest();
+  ~ChromotingInstanceTest() override;
+
+ protected:
+  // Test interface.
+  void SetUp() override;
+  void TearDown() override;
+
+  // Used for result verification.
+  bool is_connected_to_host_;
+  protocol::ConnectionToHost::State connection_state_;
+  protocol::ErrorCode error_code_;
+
+  // Used for simulating different conditions for the ChromotingInstance.
+  RemoteHostInfo remote_host_info_;
+  FakeConnectionToHost fake_connection_to_host_;
+  protocol::MockConnectionToHost* mock_connection_to_host_;
+
+  scoped_ptr<remoting::test::ChromotingInstance> chromoting_instance_;
+
+ private:
+  // RemoteConnectionObserver interface.
+  void ConnectionStateChanged(
+      protocol::ConnectionToHost::State state,
+      protocol::ErrorCode error_code) override;
+  void ConnectedToRemoteHost() override;
+  void DisconnectedFromRemoteHost() override;
+
+  // Used as return values in ON_CALL overrides for MockConnectionToHost.
+  protocol::MockHostStub mock_host_stub_;
+  protocol::SessionConfig session_config_;
+
+  scoped_ptr<base::MessageLoopForIO> message_loop_;

[Sergey Ulanov, 2015/03/09 06:44:41]

Usually message_loop_ is defined as a first field in tests, so it outlives other
objects.

[Sergey Ulanov, 2015/03/09 06:44:41]

Don't need this to be a pointer, see my next comment.

[joedow, 2015/03/09 21:09:26]

Done.

[joedow, 2015/03/09 21:09:26]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(ChromotingInstanceTest);
+};
+
+ChromotingInstanceTest::ChromotingInstanceTest() :
+    is_connected_to_host_(false),
+    connection_state_(protocol::ConnectionToHost::INITIALIZING),
+    error_code_(protocol::OK),
+    mock_connection_to_host_(nullptr) {}
+
+ChromotingInstanceTest::~ChromotingInstanceTest() {}
+
+void ChromotingInstanceTest::SetUp() {
+  if (!base::MessageLoop::current()) {
+    // Create a temporary message loop if the current thread does not already

[Sergey Ulanov, 2015/03/09 06:44:41]

This code is never expected to be invoked with a MessageLoop created somewhere
else. DOn't need this check.

[joedow, 2015/03/09 21:09:26]

Done.

+    // have one so we can use its task runner to create a request object.
+    message_loop_.reset(new base::MessageLoopForIO);
+  }
+
+  chromoting_instance_.reset(new remoting::test::ChromotingInstance);
+  chromoting_instance_->AddRemoteConnectionObserver(this);
+
+  // Pass ownership of the MockConnectionToHost to the chromoting instance but
+  // keep the ptr around so we can use it to simulate state changes.  It will
+  // remain valid until |chromoting_instance_| is destroyed.
+  mock_connection_to_host_ = new protocol::MockConnectionToHost();
+  chromoting_instance_->SetConnectionToHostForTests(
+      make_scoped_ptr(mock_connection_to_host_));
+
+  session_config_ = protocol::SessionConfig::ForTest();
+  remote_host_info_.remote_host_status = kRemoteHostStatusReady;
+
+  // We want to redirect some of the calls to the mock objects to real objects
+  // which implement a simplified version of the method so ON_CALL is used.
+  ON_CALL(*mock_connection_to_host_,
+      ConnectPtr(_, _, _, _, _)).WillByDefault(testing::Invoke(
+          &fake_connection_to_host_, &FakeConnectionToHost::ConnectPtr));

[Sergey Ulanov, 2015/03/09 06:44:41]

Do you need MockConnectionToHost, given that there is FakeConnectionToHost?
GMOCK usage is discouraged. See
https://groups.google.com/a/chromium.org/d/topic/chromium-dev/-KH_IP0rIWQ/dis...
. This is the case when GMOCK usage makes code less readable. It's also
confusing to call FakeConnectionToHost here.

[joedow, 2015/03/09 21:09:26]

Thanks for this info, I see GMOCK used often in the chromoting code base and the
GMock docs don't mention any of this...I'll remove the mocks since they aren't
super useful and just complicate the test code.

+  ON_CALL(*mock_connection_to_host_,
+      OnSessionStateChange(_)).WillByDefault(testing::Invoke(
+          &fake_connection_to_host_,
+          &FakeConnectionToHost::OnSessionStateChange));
+  ON_CALL(*mock_connection_to_host_,
+      OnVideoChannelStatus(_)).WillByDefault(testing::Invoke(
+          &fake_connection_to_host_,
+          &FakeConnectionToHost::OnVideoChannelStatus));
+  ON_CALL(*mock_connection_to_host_, config()).WillByDefault(
+      testing::ReturnRef(session_config_));
+  ON_CALL(*mock_connection_to_host_, host_stub()).WillByDefault(
+      testing::Return(&mock_host_stub_));
+
+  EXPECT_CALL(mock_host_stub_, SetCapabilities(_))
+      .Times(testing::AtMost(1));
+  EXPECT_CALL(*mock_connection_to_host_, ConnectPtr(_, _, _, _, _))
+      .Times(testing::AtMost(1));
+  EXPECT_CALL(*mock_connection_to_host_, OnSessionStateChange(_))
+      .Times(testing::AnyNumber());
+  EXPECT_CALL(*mock_connection_to_host_, OnVideoChannelStatus(_))
+      .Times(testing::AnyNumber());
+  EXPECT_CALL(*mock_connection_to_host_, config())
+      .Times(testing::AnyNumber());
+  EXPECT_CALL(*mock_connection_to_host_, host_stub())
+      .Times(testing::AnyNumber());
+}
+
+void ChromotingInstanceTest::TearDown() {
+  chromoting_instance_->RemoveRemoteConnectionObserver(this);
+  mock_connection_to_host_ = nullptr;
+
+  // The chromoting instance must be destroyed before the message loop.
+  chromoting_instance_.reset();
+
+  // The LibjingleTransportFactory destroys the PortAllocator via a DeleteSoon
+  // operation.  If we do not allow the message loop to run here, we run the
+  // risk of the DeleteSoon task being dropped and incurring a memory leak.
+  message_loop_->RunUntilIdle();
+
+  // Now that the message loop is clear, we can destroy it.
+  message_loop_.reset();
+}
+
+void ChromotingInstanceTest::ConnectionStateChanged(
+    protocol::ConnectionToHost::State state,
+    protocol::ErrorCode error_code) {
+  connection_state_ = state;
+  error_code_ = error_code;
+}
+
+void ChromotingInstanceTest::ConnectedToRemoteHost() {
+  is_connected_to_host_ = true;
+}
+
+void ChromotingInstanceTest::DisconnectedFromRemoteHost() {
+  is_connected_to_host_ = false;
+}
+
+TEST_F(ChromotingInstanceTest, StartConnectionAndDisconnect) {
+  chromoting_instance_->StartConnection(
+      kTestUserName,
+      kAccessToken,
+      remote_host_info_);
+  EXPECT_EQ(protocol::ConnectionToHost::State::CONNECTING, connection_state_);
+  EXPECT_EQ(protocol::ErrorCode::OK, error_code_);
+  EXPECT_FALSE(is_connected_to_host_);
+
+  // Simulate an AUTHENTICATED message being sent from the Jingle session.
+  mock_connection_to_host_->OnSessionStateChange(
+      protocol::Session::AUTHENTICATED);
+  EXPECT_EQ(protocol::ConnectionToHost::State::AUTHENTICATED,
+      connection_state_);
+  EXPECT_EQ(protocol::ErrorCode::OK, error_code_);
+  EXPECT_FALSE(is_connected_to_host_);
+
+  // Simulate a CONNECTED message being sent from the Jingle session.
+  mock_connection_to_host_->OnSessionStateChange(protocol::Session::CONNECTED);
+  EXPECT_EQ(protocol::ConnectionToHost::State::CONNECTED, connection_state_);
+  EXPECT_EQ(protocol::ErrorCode::OK, error_code_);
+  EXPECT_FALSE(is_connected_to_host_);
+
+  // The chromoting instance will consider its connection to the host complete
+  // once the video channel is active.
+  mock_connection_to_host_->OnVideoChannelStatus(true);
+  EXPECT_EQ(protocol::ConnectionToHost::State::CONNECTED, connection_state_);
+  EXPECT_EQ(protocol::ErrorCode::OK, error_code_);
+  EXPECT_TRUE(is_connected_to_host_);
+
+  chromoting_instance_->EndConnection();
+  EXPECT_EQ(protocol::ConnectionToHost::State::CLOSED, connection_state_);
+  EXPECT_EQ(protocol::ErrorCode::OK, error_code_);
+  EXPECT_FALSE(is_connected_to_host_);
+}
+
+TEST_F(ChromotingInstanceTest, StartConnectionThenFailWithAuthenticationError) {
+  chromoting_instance_->StartConnection(
+      kTestUserName,
+      kAccessToken,
+      remote_host_info_);
+  EXPECT_EQ(protocol::ConnectionToHost::State::CONNECTING, connection_state_);
+  EXPECT_EQ(protocol::ErrorCode::OK, error_code_);
+  EXPECT_FALSE(is_connected_to_host_);
+
+  fake_connection_to_host_.set_session_state_change_error_code(
+      protocol::AUTHENTICATION_FAILED);
+  mock_connection_to_host_->OnSessionStateChange(protocol::Session::FAILED);
+  EXPECT_EQ(protocol::ConnectionToHost::State::FAILED, connection_state_);
+  EXPECT_EQ(protocol::ErrorCode::AUTHENTICATION_FAILED, error_code_);
+  EXPECT_FALSE(is_connected_to_host_);
+
+  // Close the connection via the ChromotingInstance and verify the error state
+  // is persisted.
+  chromoting_instance_->EndConnection();
+  EXPECT_EQ(protocol::ConnectionToHost::State::FAILED, connection_state_);
+  EXPECT_EQ(protocol::ErrorCode::AUTHENTICATION_FAILED, error_code_);
+  EXPECT_FALSE(is_connected_to_host_);
+}
+
+TEST_F(ChromotingInstanceTest, StartConnectionThenFailWithUnknownError) {
+  chromoting_instance_->StartConnection(
+      kTestUserName,
+      kAccessToken,
+      remote_host_info_);
+  EXPECT_EQ(protocol::ConnectionToHost::State::CONNECTING, connection_state_);
+  EXPECT_EQ(protocol::ErrorCode::OK, error_code_);
+  EXPECT_FALSE(is_connected_to_host_);
+
+  // Simulate an AUTHENTICATED message being sent from the Jingle session.
+  mock_connection_to_host_->OnSessionStateChange(
+      protocol::Session::AUTHENTICATED);
+  EXPECT_EQ(protocol::ConnectionToHost::State::AUTHENTICATED,
+      connection_state_);
+  EXPECT_EQ(protocol::ErrorCode::OK, error_code_);
+  EXPECT_FALSE(is_connected_to_host_);
+
+  // Simulate a CONNECTED message being sent from the Jingle session.
+  mock_connection_to_host_->OnSessionStateChange(protocol::Session::CONNECTED);
+  EXPECT_EQ(protocol::ConnectionToHost::State::CONNECTED, connection_state_);
+  EXPECT_EQ(protocol::ErrorCode::OK, error_code_);
+  EXPECT_FALSE(is_connected_to_host_);
+
+  // The chromoting instance will consider its connection to the host complete
+  // once the video channel is active.
+  mock_connection_to_host_->OnVideoChannelStatus(true);
+  EXPECT_EQ(protocol::ConnectionToHost::State::CONNECTED, connection_state_);
+  EXPECT_EQ(protocol::ErrorCode::OK, error_code_);
+  EXPECT_TRUE(is_connected_to_host_);
+
+  fake_connection_to_host_.set_session_state_change_error_code(
+      protocol::UNKNOWN_ERROR);
+  mock_connection_to_host_->OnSessionStateChange(protocol::Session::FAILED);
+  EXPECT_EQ(protocol::ConnectionToHost::State::FAILED, connection_state_);
+  EXPECT_EQ(protocol::ErrorCode::UNKNOWN_ERROR, error_code_);
+  EXPECT_FALSE(is_connected_to_host_);
+
+  // Close the connection via the ChromotingInstance and verify the error state
+  // is persisted.
+  chromoting_instance_->EndConnection();
+  EXPECT_EQ(protocol::ConnectionToHost::State::FAILED, connection_state_);
+  EXPECT_EQ(protocol::ErrorCode::UNKNOWN_ERROR, error_code_);
+  EXPECT_FALSE(is_connected_to_host_);
+}
+
+}  // namespace test
+}  // namespace remoting