unittests for Chromoting native messaging host.

BUG=232200

Review URL: https://chromiumcodereview.appspot.com/14979008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@203084 0039d316-1c4b-4281-b951-d872f2087c98

remoting/host/setup/native_messaging_host.cc

@@ -37,6 +37,7 @@ scoped_ptr<base::DictionaryValue> ConfigDictionaryFromMessage(
 namespace remoting {
 
 NativeMessagingHost::NativeMessagingHost(
+    scoped_ptr<DaemonController> daemon_controller,
     base::PlatformFile input,
     base::PlatformFile output,
     scoped_refptr<base::SingleThreadTaskRunner> caller_task_runner,
@@ -45,7 +46,7 @@ NativeMessagingHost::NativeMessagingHost(
       quit_closure_(quit_closure),
       native_messaging_reader_(input),
       native_messaging_writer_(output),
-      daemon_controller_(DaemonController::Create()),
+      daemon_controller_(daemon_controller.Pass()),
       weak_factory_(this) {
   weak_ptr_ = weak_factory_.GetWeakPtr();
 }
@@ -70,10 +71,16 @@ void NativeMessagingHost::Shutdown() {
 
 void NativeMessagingHost::ProcessMessage(scoped_ptr<base::Value> message) {
   DCHECK(caller_task_runner_->BelongsToCurrentThread());
+
+  // Don't process any more messages if Shutdown() has been called.
+  if (quit_closure_.is_null())
+    return;
+
   const base::DictionaryValue* message_dict;
   if (!message->GetAsDictionary(&message_dict)) {
     LOG(ERROR) << "Expected DictionaryValue";
     Shutdown();
+    return;
   }
 
   scoped_ptr<base::DictionaryValue> response_dict(new base::DictionaryValue());

remoting/host/setup/native_messaging_host.h

@@ -26,6 +26,7 @@ namespace remoting {
 class NativeMessagingHost {
  public:
   NativeMessagingHost(
+      scoped_ptr<DaemonController> daemon_controller,
       base::PlatformFile input,
       base::PlatformFile output,
       scoped_refptr<base::SingleThreadTaskRunner> caller_task_runner,
@@ -79,11 +80,10 @@ class NativeMessagingHost {
                                 DaemonController::AsyncResult result);
   void SendConfigResponse(scoped_ptr<base::DictionaryValue> response,
                           scoped_ptr<base::DictionaryValue> config);
-  void SendUsageStatsConsentResponse(
-      scoped_ptr<base::DictionaryValue> response,
-      bool supported,
-      bool allowed,
-      bool set_by_policy);
+  void SendUsageStatsConsentResponse(scoped_ptr<base::DictionaryValue> response,
+                                     bool supported,
+                                     bool allowed,
+                                     bool set_by_policy);
   void SendAsyncResult(scoped_ptr<base::DictionaryValue> response,
                        DaemonController::AsyncResult result);
 

remoting/host/setup/native_messaging_host_main.cc

@@ -33,7 +33,8 @@ int main(int argc, char** argv) {
 
   base::MessageLoop message_loop(base::MessageLoop::TYPE_IO);
   base::RunLoop run_loop;
-  remoting::NativeMessagingHost host(read_file, write_file,
+  remoting::NativeMessagingHost host(remoting::DaemonController::Create(),
+                                     read_file, write_file,
                                      message_loop.message_loop_proxy(),
                                      run_loop.QuitClosure());
   host.Start();

remoting/host/setup/native_messaging_reader.cc

@@ -73,8 +73,7 @@ NativeMessagingReader::Core::Core(
       read_task_runner_(read_task_runner) {
 }
 
-NativeMessagingReader::Core::~Core() {
-}
+NativeMessagingReader::Core::~Core() {}
 
 void NativeMessagingReader::Core::ReadMessage() {
   DCHECK(read_task_runner_->RunsTasksOnCurrentThread());
@@ -85,8 +84,11 @@ void NativeMessagingReader::Core::ReadMessage() {
     int read_result = read_stream_.ReadUntilComplete(
         reinterpret_cast<char*>(&message_length), kMessageHeaderSize);
     if (read_result != kMessageHeaderSize) {
-      LOG(ERROR) << "Failed to read message header, read returned "
-                 << read_result;
+      // 0 means EOF which is normal and should not be logged as an error.
+      if (read_result != 0) {
+        LOG(ERROR) << "Failed to read message header, read returned "
+                   << read_result;
+      }
       NotifyEof();
       return;
     }
@@ -98,8 +100,8 @@ void NativeMessagingReader::Core::ReadMessage() {
     }
 
     std::string message_json(message_length, '\0');
-    read_result = read_stream_.ReadUntilComplete(
-        string_as_array(&message_json), message_length);
+    read_result = read_stream_.ReadUntilComplete(string_as_array(&message_json),
+                                                 message_length);
     if (read_result != static_cast<int>(message_length)) {
       LOG(ERROR) << "Failed to read message body, read returned "
                  << read_result;
@@ -115,16 +117,17 @@ void NativeMessagingReader::Core::ReadMessage() {
     }
 
     // Notify callback of new message.
-    caller_task_runner_->PostTask(FROM_HERE, base::Bind(
-        &NativeMessagingReader::InvokeMessageCallback, reader_,
-        base::Passed(&message)));
+    caller_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&NativeMessagingReader::InvokeMessageCallback,
+                              reader_, base::Passed(&message)));
   }
 }
 
 void NativeMessagingReader::Core::NotifyEof() {
   DCHECK(read_task_runner_->RunsTasksOnCurrentThread());
-  caller_task_runner_->PostTask(FROM_HERE, base::Bind(
-      &NativeMessagingReader::InvokeEofCallback, reader_));
+  caller_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&NativeMessagingReader::InvokeEofCallback, reader_));
 }
 
 NativeMessagingReader::NativeMessagingReader(base::PlatformFile handle)
@@ -147,9 +150,9 @@ void NativeMessagingReader::Start(MessageCallback message_callback,
 
   // base::Unretained is safe since |core_| is only deleted via the
   // DeleteSoon task which is posted from this class's dtor.
-  read_task_runner_->PostTask(FROM_HERE, base::Bind(
-      &NativeMessagingReader::Core::ReadMessage,
-      base::Unretained(core_.get())));
+  read_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&NativeMessagingReader::Core::ReadMessage,
+                            base::Unretained(core_.get())));
 }
 
 void NativeMessagingReader::InvokeMessageCallback(

remoting/host/setup/native_messaging_reader_unittest.cc

+// Copyright 2013 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/setup/native_messaging_reader.h"
+
+#include "base/basictypes.h"
+#include "base/bind.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/message_loop.h"
+#include "base/platform_file.h"
+#include "base/run_loop.h"
+#include "base/values.h"
+#include "remoting/host/setup/test_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace remoting {
+
+class NativeMessagingReaderTest : public testing::Test {
+ public:
+  NativeMessagingReaderTest();
+  virtual ~NativeMessagingReaderTest();
+
+  virtual void SetUp() OVERRIDE;
+  virtual void TearDown() OVERRIDE;
+
+  // Starts the reader and runs the MessageLoop to completion.
+  void Run();
+
+  // MessageCallback passed to the Reader. Stores |message| so it can be
+  // verified by tests.
+  void OnMessage(scoped_ptr<base::Value> message);
+
+  // Writes a message (header+body) to the write-end of the pipe.
+  void WriteMessage(std::string message);
+
+  // Writes some data to the write-end of the pipe.
+  void WriteData(const char* data, int length);
+
+ protected:
+  scoped_ptr<NativeMessagingReader> reader_;
+  base::PlatformFile read_handle_;
+  base::PlatformFile write_handle_;
+  scoped_ptr<base::Value> message_;
+
+ private:
+  // MessageLoop declared here, since the NativeMessageReader ctor requires a
+  // MessageLoop to have been created.
+  base::MessageLoop message_loop_;
+  base::RunLoop run_loop_;
+};
+
+NativeMessagingReaderTest::NativeMessagingReaderTest()
+    : message_loop_(base::MessageLoop::TYPE_IO) {
+}
+
+NativeMessagingReaderTest::~NativeMessagingReaderTest() {}
+
+void NativeMessagingReaderTest::SetUp() {
+  ASSERT_TRUE(MakePipe(&read_handle_, &write_handle_));
+  reader_.reset(new NativeMessagingReader(read_handle_));
+}
+
+void NativeMessagingReaderTest::TearDown() {
+  // |read_handle_| is owned by NativeMessagingReader's FileStream, so don't
+  // try to close it here. Also, |write_handle_| gets closed by Run().
+}
+
+void NativeMessagingReaderTest::Run() {
+  // Close the write-end, so the reader doesn't block waiting for more data.
+  base::ClosePlatformFile(write_handle_);
+
+  // base::Unretained is safe since no further tasks can run after
+  // RunLoop::Run() returns.
+  reader_->Start(
+      base::Bind(&NativeMessagingReaderTest::OnMessage, base::Unretained(this)),
+      run_loop_.QuitClosure());
+  run_loop_.Run();
+}
+
+void NativeMessagingReaderTest::OnMessage(scoped_ptr<base::Value> message) {
+  message_ = message.Pass();
+}
+
+void NativeMessagingReaderTest::WriteMessage(std::string message) {
+  uint32 length = message.length();
+  WriteData(reinterpret_cast<char*>(&length), 4);
+  WriteData(message.data(), length);
+}
+
+void NativeMessagingReaderTest::WriteData(const char* data, int length) {
+  int written = base::WritePlatformFileAtCurrentPos(write_handle_, data,
+                                                    length);
+  ASSERT_EQ(length, written);
+}
+
+TEST_F(NativeMessagingReaderTest, GoodMessage) {
+  WriteMessage("{\"foo\": 42}");
+  Run();
+  EXPECT_TRUE(message_);
+  base::DictionaryValue* message_dict;
+  EXPECT_TRUE(message_->GetAsDictionary(&message_dict));
+  int result;
+  EXPECT_TRUE(message_dict->GetInteger("foo", &result));
+  EXPECT_EQ(42, result);
+}
+
+TEST_F(NativeMessagingReaderTest, InvalidLength) {
+  uint32 length = 0xffffffff;
+  WriteData(reinterpret_cast<char*>(&length), 4);
+  Run();
+  EXPECT_FALSE(message_);
+}
+
+TEST_F(NativeMessagingReaderTest, EmptyFile) {
+  Run();
+  EXPECT_FALSE(message_);
+}
+
+TEST_F(NativeMessagingReaderTest, ShortHeader) {
+  // Write only 3 bytes - the message length header is supposed to be 4 bytes.
+  WriteData("xxx", 3);
+  Run();
+  EXPECT_FALSE(message_);
+}
+
+TEST_F(NativeMessagingReaderTest, EmptyBody) {
+  uint32 length = 1;
+  WriteData(reinterpret_cast<char*>(&length), 4);
+  Run();
+  EXPECT_FALSE(message_);
+}
+
+TEST_F(NativeMessagingReaderTest, ShortBody) {
+  uint32 length = 2;
+  WriteData(reinterpret_cast<char*>(&length), 4);
+
+  // Only write 1 byte, where the header indicates there should be 2 bytes.
+  WriteData("x", 1);
+  Run();
+  EXPECT_FALSE(message_);
+}
+
+TEST_F(NativeMessagingReaderTest, InvalidJSON) {
+  std::string text = "{";
+  WriteMessage(text);
+  Run();
+  EXPECT_FALSE(message_);
+}
+
+TEST_F(NativeMessagingReaderTest, SecondMessage) {
+  WriteMessage("{}");
+  WriteMessage("{\"foo\": 42}");
+  Run();
+  EXPECT_TRUE(message_);
+  base::DictionaryValue* message_dict;
+  EXPECT_TRUE(message_->GetAsDictionary(&message_dict));
+  int result;
+  EXPECT_TRUE(message_dict->GetInteger("foo", &result));
+  EXPECT_EQ(42, result);
+}
+
+}  // namespace remoting

remoting/host/setup/native_messaging_writer_unittest.cc

+// Copyright 2013 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/setup/native_messaging_writer.h"
+
+#include "base/basictypes.h"
+#include "base/json/json_reader.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/platform_file.h"
+#include "base/stl_util.h"
+#include "base/values.h"
+#include "remoting/host/setup/test_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace remoting {
+
+class NativeMessagingWriterTest : public testing::Test {
+ public:
+  NativeMessagingWriterTest();
+  virtual ~NativeMessagingWriterTest();
+
+  virtual void SetUp() OVERRIDE;
+  virtual void TearDown() OVERRIDE;
+
+ protected:
+  scoped_ptr<NativeMessagingWriter> writer_;
+  base::PlatformFile read_handle_;
+  base::PlatformFile write_handle_;
+  bool read_handle_open_;
+};
+
+NativeMessagingWriterTest::NativeMessagingWriterTest() {}
+
+NativeMessagingWriterTest::~NativeMessagingWriterTest() {}
+
+void NativeMessagingWriterTest::SetUp() {
+  ASSERT_TRUE(MakePipe(&read_handle_, &write_handle_));
+  writer_.reset(new NativeMessagingWriter(write_handle_));
+  read_handle_open_ = true;
+}
+
+void NativeMessagingWriterTest::TearDown() {
+  // |write_handle_| is owned by NativeMessagingWriter's FileStream, so don't
+  // try to close it here. And close |read_handle_| only if it hasn't
+  // already been closed.
+  if (read_handle_open_)
+    base::ClosePlatformFile(read_handle_);
+}
+
+TEST_F(NativeMessagingWriterTest, GoodMessage) {
+  base::DictionaryValue message;
+  message.SetInteger("foo", 42);
+  EXPECT_TRUE(writer_->WriteMessage(message));
+
+  // Read from the pipe and verify the content.
+  uint32 length;
+  int read = base::ReadPlatformFileAtCurrentPos(
+      read_handle_, reinterpret_cast<char*>(&length), 4);
+  EXPECT_EQ(4, read);
+  std::string content(length, '\0');
+  read = base::ReadPlatformFileAtCurrentPos(read_handle_,
+                                            string_as_array(&content), length);
+  EXPECT_EQ(static_cast<int>(length), read);
+
+  // |content| should now contain serialized |message|.
+  scoped_ptr<base::Value> written_message(base::JSONReader::Read(content));
+  EXPECT_TRUE(message.Equals(written_message.get()));
+
+  // Nothing more should have been written. Close the write-end of the pipe,
+  // and verify the read end immediately hits EOF.
+  writer_.reset(NULL);
+  char unused;
+  read = base::ReadPlatformFileAtCurrentPos(read_handle_, &unused, 1);
+  EXPECT_LE(read, 0);
+}
+
+TEST_F(NativeMessagingWriterTest, SecondMessage) {
+  base::DictionaryValue message1;
+  base::DictionaryValue message2;
+  message2.SetInteger("foo", 42);
+  EXPECT_TRUE(writer_->WriteMessage(message1));
+  EXPECT_TRUE(writer_->WriteMessage(message2));
+  writer_.reset(NULL);
+
+  // Read two messages.
+  uint32 length;
+  int read;
+  std::string content;
+  for (int i = 0; i < 2; i++) {
+    read = base::ReadPlatformFileAtCurrentPos(
+        read_handle_, reinterpret_cast<char*>(&length), 4);
+    EXPECT_EQ(4, read) << "i = " << i;
+    content.resize(length);
+    read = base::ReadPlatformFileAtCurrentPos(read_handle_,
+                                              string_as_array(&content),
+                                              length);
+    EXPECT_EQ(static_cast<int>(length), read) << "i = " << i;
+  }
+
+  // |content| should now contain serialized |message2|.
+  scoped_ptr<base::Value> written_message2(base::JSONReader::Read(content));
+  EXPECT_TRUE(message2.Equals(written_message2.get()));
+}
+
+TEST_F(NativeMessagingWriterTest, FailedWrite) {
+  // Close the read end so that writing fails immediately.
+  base::ClosePlatformFile(read_handle_);
+  read_handle_open_ = false;
+
+  base::DictionaryValue message;
+  EXPECT_FALSE(writer_->WriteMessage(message));
+}
+
+}  // namespace remoting

remoting/host/setup/test_util.cc

+// Copyright 2013 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/setup/test_util.h"
+
+#if defined(OS_WIN)
+#include <windows.h>
+#elif defined(OS_POSIX)
+#include <unistd.h>
+#endif
+
+namespace remoting {
+
+bool MakePipe(base::PlatformFile* read_handle,
+              base::PlatformFile* write_handle) {
+#if defined(OS_WIN)
+  return CreatePipe(read_handle, write_handle, NULL, 0) != FALSE;
+#elif defined(OS_POSIX)
+  int fds[2];
+  if (pipe(fds) == 0) {
+    *read_handle = fds[0];
+    *write_handle = fds[1];
+    return true;
+  }
+  return false;
+#else
+#error Not implemented
+#endif
+}
+
+}  // namepsace remoting

remoting/host/setup/test_util.h

+// Copyright 2013 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.
+
+#ifndef REMOTING_HOST_SETUP_TEST_UTIL_H_
+#define REMOTING_HOST_SETUP_TEST_UTIL_H_
+
+#include "base/platform_file.h"
+
+namespace remoting {
+
+// Creates an anonymous, unidirectional pipe, returning true if successful. On
+// success, the caller is responsible for closing both ends using
+// base::ClosePlatformFile().
+bool MakePipe(base::PlatformFile* read_handle,
+              base::PlatformFile* write_handle);
+
+}  // namespace remoting
+
+#endif  // REMOTING_HOST_SETUP_TEST_UTIL_H_

remoting/remoting.gyp

@@ -612,6 +612,9 @@
             '../google_apis/google_apis.gyp:google_apis',
             'remoting_host',
           ],
+          'defines': [
+            'VERSION=<(version_full)',
+          ],
           'sources': [
             'host/setup/daemon_controller.h',
             'host/setup/daemon_controller_linux.cc',
@@ -621,10 +624,18 @@
             'host/setup/daemon_installer_win.h',
             'host/setup/host_starter.cc',
             'host/setup/host_starter.h',
+            'host/setup/native_messaging_host.cc',
+            'host/setup/native_messaging_host.h',
+            'host/setup/native_messaging_reader.cc',
+            'host/setup/native_messaging_reader.h',
+            'host/setup/native_messaging_writer.cc',
+            'host/setup/native_messaging_writer.h',
             'host/setup/oauth_helper.cc',
             'host/setup/oauth_helper.h',
             'host/setup/pin_validator.cc',
             'host/setup/pin_validator.h',
+            'host/setup/test_util.cc',
+            'host/setup/test_util.h',
             'host/setup/win/auth_code_getter.cc',
             'host/setup/win/auth_code_getter.h',
           ],
@@ -735,13 +746,7 @@
             'VERSION=<(version_full)',
           ],
           'sources': [
-            'host/setup/native_messaging_host.cc',
-            'host/setup/native_messaging_host.h',
             'host/setup/native_messaging_host_main.cc',
-            'host/setup/native_messaging_reader.cc',
-            'host/setup/native_messaging_reader.h',
-            'host/setup/native_messaging_writer.cc',
-            'host/setup/native_messaging_writer.h',
           ],
           'conditions': [
             ['OS=="linux" and linux_use_tcmalloc==1', {
@@ -2672,6 +2677,9 @@
         'host/resizing_host_observer_unittest.cc',
         'host/screen_resolution_unittest.cc',
         'host/server_log_entry_unittest.cc',
+        'host/setup/native_messaging_host_unittest.cc',
+        'host/setup/native_messaging_reader_unittest.cc',
+        'host/setup/native_messaging_writer_unittest.cc',
         'host/setup/oauth_helper_unittest.cc',
         'host/setup/pin_validator_unittest.cc',
         'host/token_validator_factory_impl_unittest.cc',