Restart the host when the third party auth certificate changes

remoting/host/linux/certificate_watcher_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 "base/bind.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/logging.h"
+#include "base/message_loop/message_loop.h"
+

[Sergey Ulanov, 2016/03/30 21:02:46]

nit: remove this empty line

[Yuwei, 2016/03/31 17:40:06]

Done.

+#include "remoting/base/auto_thread.h"
+#include "remoting/host/linux/certificate_watcher.h"

[Sergey Ulanov, 2016/03/30 21:02:45]

this should be first include:
https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes

[Yuwei, 2016/03/31 17:40:05]

Done.

+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace remoting {
+const std::string WATCH_FILENAME = "testfile.txt";

[Sergey Ulanov, 2016/03/30 21:02:45]

kWatchFilename
https://google.github.io/styleguide/cppguide.html#Constant_Names

[Sergey Ulanov, 2016/03/30 21:02:46]

static constant of std::string are not allowed because they require static
initializers. You can use char array:

const char kWatchFilename[] = "testfile.txt".

[Yuwei, 2016/03/31 17:40:05]

Done.

+const std::string TEST_WRITE_STRING = "TEST STRING";
+
+class TestSetup {
+ public:

[Sergey Ulanov, 2016/03/30 21:02:46]

None of these fields and methods need to be public. Make the protected?

[Yuwei, 2016/03/31 17:40:06]

Done.

+  TestSetup()
+      : watcher_loop_(),
+        watcher_runner_(

[Sergey Ulanov, 2016/03/30 21:02:46]

You don't need this. MessageLoop already has a task runner.

[Yuwei, 2016/03/31 17:40:05]

Done.

+            new AutoThreadTaskRunner(watcher_loop_.task_runner(),
+                                     base::MessageLoop::QuitWhenIdleClosure())),
+        temp_dir(),

[Sergey Ulanov, 2016/03/30 21:02:46]

Don't need this initializer - the compiler will call the default constructor
implicitly.

[Yuwei, 2016/03/31 17:40:05]

Done.

+        watch_path(CreateAndGetUniqueTempDir().AppendASCII(WATCH_FILENAME)),
+        watcher(0,
+                base::Bind(&TestSetup::OnRestart, base::Unretained(this)),
+                base::Bind(&TestSetup::OnDeferred, base::Unretained(this)),
+                watch_path) {}
+
+  void Start() {
+    watcher.StartOn(watcher_runner_, base::WeakPtr<HostStatusMonitor>());
+  }
+
+  void Stop() { watcher.Stop(); }
+
+  void RunLoop() { watcher_loop_.Run(); }
+
+  void TouchFile() {
+    watcher_runner_->PostTask(FROM_HERE, base::Bind(&TestSetup::TouchFileOnIO,
+                                                    base::Unretained(this)));
+  }
+
+  base::MessageLoopForIO watcher_loop_;

[Sergey Ulanov, 2016/03/30 21:02:46]

The convention is to call MessageLoop instances in tests message_loop_

[Yuwei, 2016/03/31 17:40:05]

Done.

+  const scoped_refptr<AutoThreadTaskRunner> watcher_runner_;
+  base::ScopedTempDir temp_dir;
+  base::FilePath watch_path;
+  CertificateWatcher watcher;

[Sergey Ulanov, 2016/03/30 21:02:45]

watcher_ with '_' suffix. Same for other members

[Yuwei, 2016/03/31 17:40:05]

Done.

+  int restart_count = 0;
+
+ private:
+  const base::FilePath& CreateAndGetUniqueTempDir() {
+    EXPECT_TRUE(temp_dir.CreateUniqueTempDir());
+    return temp_dir.path();
+  }
+
+  void TouchFileOnIO() {
+    EXPECT_TRUE(watcher_runner_->BelongsToCurrentThread());
+    base::WriteFile(watch_path, TEST_WRITE_STRING.c_str(),
+                    TEST_WRITE_STRING.length());
+  }
+
+  void OnRestart() {
+    EXPECT_TRUE(watcher_runner_->BelongsToCurrentThread());
+    restart_count++;
+    // watcher_loop_.QuitNow();

[Sergey Ulanov, 2016/03/30 21:02:45]

Please remove commented code.

[Yuwei, 2016/03/31 17:40:05]

Done.

+    watcher_loop_.QuitNow();
+  }
+
+  void OnDeferred() {
+    EXPECT_TRUE(watcher_runner_->BelongsToCurrentThread());
+    watcher_loop_.QuitNow();
+  }
+};
+
+TEST(CertificateWatcherTest, StartAndStop) {
+  TestSetup setup;
+
+  EXPECT_EQ(0, setup.restart_count);
+  setup.Start();
+  setup.TouchFile();
+  setup.RunLoop();
+  EXPECT_EQ(1, setup.restart_count);
+  setup.Stop();
+  EXPECT_EQ(1, setup.restart_count);
+}
+
+TEST(CertificateWatcherTest, InhibitDeferRestart) {
+  TestSetup setup;
+
+  setup.Start();
+  EXPECT_EQ(0, setup.restart_count);
+  setup.watcher.Inhibit();
+  EXPECT_EQ(0, setup.restart_count);
+  setup.TouchFile();
+  setup.RunLoop();
+  EXPECT_EQ(0, setup.restart_count);
+  setup.watcher.Uninhibit();
+  setup.RunLoop();
+  EXPECT_EQ(1, setup.restart_count);
+}
+
+TEST(CertificateWatcherTest, UninhibitAndRestart) {
+  TestSetup setup;
+
+  setup.Start();
+  EXPECT_EQ(0, setup.restart_count);
+  setup.watcher.Inhibit();
+  EXPECT_EQ(0, setup.restart_count);
+  setup.watcher.Uninhibit();
+  EXPECT_EQ(0, setup.restart_count);
+  setup.TouchFile();
+  setup.RunLoop();
+  EXPECT_EQ(1, setup.restart_count);
+}
+
+} // namespace remoting