Restart the host when the third party auth certificate changes

remoting/host/linux/certificate_watcher.h

+// 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.
+
+#ifndef REMOTING_HOST_LINUX_CERTIFICATE_WATCHER_H_
+#define REMOTING_HOST_LINUX_CERTIFICATE_WATCHER_H_
+
+#include "base/files/file_path.h"
+#include "base/files/file_path_watcher.h"
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "base/timer/timer.h"
+#include "remoting/base/auto_thread_task_runner.h"

[Sergey Ulanov, 2016/04/05 21:12:56]

Don't need this.

[Yuwei, 2016/04/05 22:18:55]

Done.

+#include "remoting/host/host_status_monitor.h"
+#include "remoting/host/host_status_observer.h"
+
+namespace remoting {
+
+// This class watches the cert database and kills the host when a change of the
+// database is detected. The runner script will restart the host when the host
+// is killed then the new host will capture any new changes of the database.
+//
+// Acceptable false positives will be caused by desktop sessions and other
+// external programs.
+//
+// Implements HostStatusObserver to defer restart action when the host is
+// connected to a client.
+class CertificateWatcher : public remoting::HostStatusObserver,
+                           public base::SupportsWeakPtr<CertificateWatcher> {

[Sergey Ulanov, 2016/04/05 21:12:56]

Please use WeakPtrFactory<> instead of SupportsWeakPtr<>

[Yuwei, 2016/04/05 22:18:55]

Done.

+ public:
+  CertificateWatcher(
+      const base::Closure& restart_action,
+      scoped_refptr<base::SingleThreadTaskRunner> io_task_runner);
+
+  ~CertificateWatcher() override;
+
+  // Starts watching file changes on file thread and calling restart on
+  // network thread if necessary.

[Sergey Ulanov, 2016/04/05 21:12:56]

Suggest rewording:
 // Starts watching file changes calling |restart_action| when the
 // host should be restarted.

No need to mention threads since from the consumer's point of view this is a
single-thread class.

[Yuwei, 2016/04/05 21:53:01]

This comment may be unnecessary information for the consumer but I may disagree
that a consumer can view this class as a single-thread class since the
constructor takes in a task runner and the thread must be running when cleaning
up the file watcher...

[Yuwei, 2016/04/05 22:18:55]

Done.

+  // returns true if the watcher has successfully started.

[Sergey Ulanov, 2016/04/05 21:12:56]

Don't need to return anything as it never fails.

[Yuwei, 2016/04/05 22:18:55]

Done.

+  bool Start();
+
+  // Stops watching file changes and cleans up all the resources being used.
+  // Will do nothing if the certificate watcher is currently not running.
+  // You might not need to explicitly call this function since the destructor
+  // will call it.
+  // The message loop MUST be running after this function is called, otherwise
+  // there will be memory leaks.
+  void Stop();

[Sergey Ulanov, 2016/04/05 21:12:56]

I don't think you need this method. For the tests just allocate and destroy the
watcher on the heap to control when it frees the resources.

+
+  // Sets the monitor to observe connection/disconnection events to toggle
+  // the inhibit mode. Should be called after the watcher starts.
+  // Adds |this| as an observer to the monitor.
+  // Removes |this| as an observer from the old monitor if it is not null.
+  void SetMonitor(base::WeakPtr<HostStatusMonitor> monitor);
+
+  // HostStatusObserver interface.
+  void OnClientConnected(const std::string& jid) override;
+  void OnClientDisconnected(const std::string& jid) override;
+
+  // Will only work before the watcher starts.
+  void SetDelayForTests(const base::TimeDelta& delay);
+  void SetWatchPathForTests(const base::FilePath& watch_path);
+
+ private:
+  // Returns true if the watcher has started.
+  bool is_started() const;
+
+  // Callback passed to |file_watcher_|. Run in IO thread.

[Sergey Ulanov, 2016/04/05 21:12:56]

s/Run/Runs/

[Yuwei, 2016/04/05 22:18:55]

Done.

+  static void OnCertDirectoryChanged(
+      scoped_refptr<base::SingleThreadTaskRunner> network_task_runner,
+      base::WeakPtr<CertificateWatcher> watcher_, const base::FilePath& path,
+      bool error);
+
+  // Run in network thread.
+  void DirectoryChanged(const base::FilePath& path, bool error);
+
+  // Called by |restart_timer_| when it's time to reset the host.
+  // It will defer restart if |inhibit_restart_scheduled_| flag is set to true.
+  void OnTimer();
+
+  // Reference to the monitor
+  base::WeakPtr<HostStatusMonitor> monitor_;
+
+  // Called when a restart is scheduled.
+  base::Closure restart_action_;
+
+  // The runner that runs everything other than the file watcher.
+  scoped_refptr<base::SingleThreadTaskRunner> caller_task_runner_;
+
+  // The runner that runs the file watcher.
+  scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
+
+  bool inhibit_mode_ = false;
+
+  bool restart_pending_ = false;
+
+  // Path of the certificate files/directories.
+  base::FilePath cert_watch_path_;
+
+  // The file watcher to watch changes inside the certificate folder.
+  scoped_ptr<base::FilePathWatcher> file_watcher_;
+
+  // The time to wait to restart when it is scheduled.
+  base::TimeDelta delay_;
+
+  // Timer to delay the restart action.
+  scoped_ptr<base::DelayTimer> restart_timer_;
+
+  DISALLOW_COPY_AND_ASSIGN(CertificateWatcher);
+};
+
+}  // namespace remoting
+
+#endif  // REMOTING_HOST_LINUX_CERTIFICATE_WATCHER_H_