Refactor the daemon controller so that the callbacks are called on the caller thread.

remoting/host/setup/native_messaging_host_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_host.h"
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/stl_util.h"
 #include "base/strings/stringize_macros.h"
 #include "base/values.h"
 #include "google_apis/gaia/gaia_oauth_client.h"
 #include "net/base/file_stream.h"
 #include "net/base/net_util.h"
+#include "remoting/base/auto_thread_task_runner.h"
 #include "remoting/host/pin_hash.h"
 #include "remoting/host/setup/test_util.h"
 #include "remoting/protocol/pairing_registry.h"
 #include "remoting/protocol/protocol_mock_objects.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using remoting::protocol::MockPairingRegistryDelegate;
 using remoting::protocol::PairingRegistry;
 using remoting::protocol::SynchronousPairingRegistry;
 
@@ skipping 94 matching lines @@
   EXPECT_TRUE(response->GetString("type", &value));
   EXPECT_EQ("startDaemonResponse", value);
   EXPECT_TRUE(response->GetString("result", &value));
   EXPECT_EQ("OK", value);
 }
 
 }  // namespace
 
 namespace remoting {
 
-class MockDaemonController : public DaemonController {
+class MockDaemonControllerDelegate : public DaemonController::Delegate {
  public:
-  MockDaemonController();
-  virtual ~MockDaemonController();
+  MockDaemonControllerDelegate();
+  virtual ~MockDaemonControllerDelegate();
 
-  virtual State GetState() OVERRIDE;
-  virtual void GetConfig(const GetConfigCallback& callback) OVERRIDE;
-  virtual void SetConfigAndStart(scoped_ptr<base::DictionaryValue> config,
-                                 bool consent,
-                                 const CompletionCallback& callback) OVERRIDE;
-  virtual void UpdateConfig(scoped_ptr<base::DictionaryValue> config,
-                            const CompletionCallback& callback) OVERRIDE;
-  virtual void Stop(const CompletionCallback& callback) OVERRIDE;
+  // DaemonController::Delegate interface.
+  virtual DaemonController::State GetState() OVERRIDE;
+  virtual scoped_ptr<base::DictionaryValue> GetConfig() OVERRIDE;
+  virtual void SetConfigAndStart(
+      scoped_ptr<base::DictionaryValue> config,
+      bool consent,
+      const DaemonController::CompletionCallback& done) OVERRIDE;
+  virtual void UpdateConfig(
+      scoped_ptr<base::DictionaryValue> config,
+      const DaemonController::CompletionCallback& done) OVERRIDE;
+  virtual void Stop(const DaemonController::CompletionCallback& done) OVERRIDE;
   virtual void SetWindow(void* window_handle) OVERRIDE;
-  virtual void GetVersion(const GetVersionCallback& callback) OVERRIDE;
-  virtual void GetUsageStatsConsent(
-      const GetUsageStatsConsentCallback& callback) OVERRIDE;
+  virtual std::string GetVersion() OVERRIDE;
+  virtual DaemonController::UsageStatsConsent GetUsageStatsConsent() OVERRIDE;
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(MockDaemonController);
+  DISALLOW_COPY_AND_ASSIGN(MockDaemonControllerDelegate);
 };
 
-MockDaemonController::MockDaemonController() {}
+MockDaemonControllerDelegate::MockDaemonControllerDelegate() {}
 
-MockDaemonController::~MockDaemonController() {}
+MockDaemonControllerDelegate::~MockDaemonControllerDelegate() {}
 
-DaemonController::State MockDaemonController::GetState() {
+DaemonController::State MockDaemonControllerDelegate::GetState() {
   return DaemonController::STATE_STARTED;
 }
 
-void MockDaemonController::GetConfig(const GetConfigCallback& callback) {
-  scoped_ptr<base::DictionaryValue> config(new base::DictionaryValue());
-  callback.Run(config.Pass());
+scoped_ptr<base::DictionaryValue> MockDaemonControllerDelegate::GetConfig() {
+  return scoped_ptr<base::DictionaryValue>(new base::DictionaryValue());
 }
 
-void MockDaemonController::SetConfigAndStart(
-    scoped_ptr<base::DictionaryValue> config, bool consent,
-    const CompletionCallback& callback) {
+void MockDaemonControllerDelegate::SetConfigAndStart(
+    scoped_ptr<base::DictionaryValue> config,
+    bool consent,
+    const DaemonController::CompletionCallback& done) {
 
   // Verify parameters passed in.
   if (consent && config && config->HasKey("start")) {
-    callback.Run(DaemonController::RESULT_OK);
+    done.Run(DaemonController::RESULT_OK);
   } else {
-    callback.Run(DaemonController::RESULT_FAILED);
+    done.Run(DaemonController::RESULT_FAILED);
   }
 }
 
-void MockDaemonController::UpdateConfig(
+void MockDaemonControllerDelegate::UpdateConfig(
     scoped_ptr<base::DictionaryValue> config,
-    const CompletionCallback& callback) {
+    const DaemonController::CompletionCallback& done) {
   if (config && config->HasKey("update")) {
-    callback.Run(DaemonController::RESULT_OK);
+    done.Run(DaemonController::RESULT_OK);
   } else {
-    callback.Run(DaemonController::RESULT_FAILED);
+    done.Run(DaemonController::RESULT_FAILED);
   }
 }
 
-void MockDaemonController::Stop(const CompletionCallback& callback) {
-  callback.Run(DaemonController::RESULT_OK);
+void MockDaemonControllerDelegate::Stop(
+    const DaemonController::CompletionCallback& done) {
+  done.Run(DaemonController::RESULT_OK);
 }
 
-void MockDaemonController::SetWindow(void* window_handle) {}
+void MockDaemonControllerDelegate::SetWindow(void* window_handle) {}
 
-void MockDaemonController::GetVersion(const GetVersionCallback& callback) {
+std::string MockDaemonControllerDelegate::GetVersion() {
   // Unused - NativeMessagingHost returns the compiled-in version string
   // instead of calling this method.
+  NOTREACHED();
+  return std::string();
 }
 
-void MockDaemonController::GetUsageStatsConsent(
-    const GetUsageStatsConsentCallback& callback) {
-  callback.Run(true, true, true);
+DaemonController::UsageStatsConsent
+MockDaemonControllerDelegate::GetUsageStatsConsent() {
+  DaemonController::UsageStatsConsent consent;
+  consent.supported = true;
+  consent.allowed = true;
+  consent.set_by_policy = true;
+  return consent;
 }
 
 class NativeMessagingHostTest : public testing::Test {
  public:
   NativeMessagingHostTest();
   virtual ~NativeMessagingHostTest();
 
   virtual void SetUp() OVERRIDE;
   virtual void TearDown() OVERRIDE;
 
   void Run();
 
+  // Deletes |host_|.
+  void DeleteHost();
+
   scoped_ptr<base::DictionaryValue> ReadMessageFromOutputPipe();
 
   void WriteMessageToInputPipe(const base::Value& message);
 
   // The Host process should shut down when it receives a malformed request.
   // This is tested by sending a known-good request, followed by |message|,
   // followed by the known-good request again. The response file should only
   // contain a single response from the first good request.
   void TestBadRequest(const base::Value& message);
 
  protected:
-  // Reference to the MockDaemonController, which is owned by |host_|.
-  MockDaemonController* daemon_controller_;
+  // Reference to the MockDaemonControllerDelegate, which is owned by |host_|.
+  MockDaemonControllerDelegate* daemon_controller_delegate_;
 
  private:
   // Each test creates two unidirectional pipes: "input" and "output".
   // NativeMessagingHost reads from input_read_handle and writes to
   // output_write_handle. The unittest supplies data to input_write_handle, and
   // verifies output from output_read_handle.
   //
   // unittest -> [input] -> NativeMessagingHost -> [output] -> unittest
   base::PlatformFile input_read_handle_;
   base::PlatformFile input_write_handle_;
   base::PlatformFile output_read_handle_;
   base::PlatformFile output_write_handle_;
 
   base::MessageLoop message_loop_;
   base::RunLoop run_loop_;
   scoped_ptr<remoting::NativeMessagingHost> host_;
 
   DISALLOW_COPY_AND_ASSIGN(NativeMessagingHostTest);
 };
 
 NativeMessagingHostTest::NativeMessagingHostTest()
     : message_loop_(base::MessageLoop::TYPE_IO) {}
 
 NativeMessagingHostTest::~NativeMessagingHostTest() {}
 
 void NativeMessagingHostTest::SetUp() {
   ASSERT_TRUE(MakePipe(&input_read_handle_, &input_write_handle_));
   ASSERT_TRUE(MakePipe(&output_read_handle_, &output_write_handle_));
 
-  daemon_controller_ = new MockDaemonController();
-  scoped_ptr<DaemonController> daemon_controller(daemon_controller_);
+  // Arrange to run |message_loop_| until no components depend on it.
+  scoped_refptr<AutoThreadTaskRunner> task_runner = new AutoThreadTaskRunner(
+      message_loop_.message_loop_proxy(), run_loop_.QuitClosure());
+
+  daemon_controller_delegate_ = new MockDaemonControllerDelegate();
+  scoped_refptr<DaemonController> daemon_controller(
+      new DaemonController(
+          scoped_ptr<DaemonController::Delegate>(daemon_controller_delegate_)));
 
   scoped_refptr<PairingRegistry> pairing_registry =
       new SynchronousPairingRegistry(scoped_ptr<PairingRegistry::Delegate>(
           new MockPairingRegistryDelegate()));
 
-  host_.reset(new NativeMessagingHost(daemon_controller.Pass(),
-                                      pairing_registry,
-                                      scoped_ptr<remoting::OAuthClient>(),
-                                      input_read_handle_, output_write_handle_,
-                                      message_loop_.message_loop_proxy(),
-                                      run_loop_.QuitClosure()));
+  host_.reset(new NativeMessagingHost(
+      daemon_controller,
+      pairing_registry,
+      scoped_ptr<remoting::OAuthClient>(),
+      input_read_handle_, output_write_handle_,
+      task_runner,
+      base::Bind(&NativeMessagingHostTest::DeleteHost,
+                 base::Unretained(this))));
 }
 
 void NativeMessagingHostTest::TearDown() {
   // The NativeMessagingHost dtor closes the handles that are passed to it.
   // |input_write_handle_| gets closed just before starting the host. So the
   // only handle left to close is |output_read_handle_|.
   base::ClosePlatformFile(output_read_handle_);
 }
 
 void NativeMessagingHostTest::Run() {
   // Close the write-end of input, so that the host sees EOF after reading
   // messages and won't block waiting for more input.
   base::ClosePlatformFile(input_write_handle_);
   host_->Start();
   run_loop_.Run();
+}
 
+void NativeMessagingHostTest::DeleteHost() {
   // Destroy |host_| so that it closes its end of the output pipe, so that
   // TestBadRequest() will see EOF and won't block waiting for more data.
   host_.reset();
 }
 
 scoped_ptr<base::DictionaryValue>
 NativeMessagingHostTest::ReadMessageFromOutputPipe() {
   uint32 length;
   int read_result = base::ReadPlatformFileAtCurrentPos(
       output_read_handle_, reinterpret_cast<char*>(&length), sizeof(length));
@@ skipping 216 matching lines @@
 
 // Verify rejection if startDaemon request has no "consent" parameter.
 TEST_F(NativeMessagingHostTest, StartDaemonNoConsent) {
   base::DictionaryValue message;
   message.SetString("type", "startDaemon");
   message.Set("config", base::DictionaryValue().DeepCopy());
   TestBadRequest(message);
 }
 
 }  // namespace remoting