Introducing remoting::Stoppable helper base class implementing asynchronous shutdown on a specific t

remoting/base/shutdownable.h

Index: remoting/base/shutdownable.h
diff --git a/remoting/base/shutdownable.h b/remoting/base/shutdownable.h
new file mode 100644
index 0000000000000000000000000000000000000000..c701c2814181ff6bf614353f31dbdeaf86d13368
--- /dev/null
+++ b/remoting/base/shutdownable.h
@@ -0,0 +1,64 @@
+// Copyright (c) 2012 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_BASE_SHUTDOWNABLE_H_
+#define REMOTING_BASE_SHUTDOWNABLE_H_
+
+#include "base/basictypes.h"
+#include "base/callback.h"
+#include "base/memory/ref_counted.h"
+
+namespace base {
+class SingleThreadTaskRunner;
+}  // namespace base
+
+namespace remoting {
+
+// A wrapper implementing two-step shutdown for an object.

[Sergey Ulanov, 2012/07/30 19:46:22]

suggest rewording it as "A helper base class that implements asynchronous
shutdown on a specific thread."

[alexeypa (please no reviews), 2012/07/30 20:41:43]

Done.

+class Shutdownable {

[Sergey Ulanov, 2012/07/30 19:46:22]

Maybe call it Stoppable? - it's easier to write and easier to read.

[alexeypa (please no reviews), 2012/07/30 20:41:43]

Done.

+ public:
+  typedef base::Callback<void(Shutdownable*)> Callback;

[Sergey Ulanov, 2012/07/30 19:46:22]

Why not just use base::Closure? If users of that interface need to get reference
to the calling object they can always use currying when creating the callback.

[alexeypa (please no reviews), 2012/07/30 20:41:43]

It was just a way to get |this| and pass a callback instance to the constructor.
I agree that base::Closure looks better.

+
+  // Constructs an object and stores the callback to be posted to |task_runner|
+  // once the object has been shutdown completely.
+  explicit Shutdownable(scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+                        const Callback& done);
+  virtual ~Shutdownable();
+
+  // Initiates shutdown. It can be called by both the owner of the object and
+  // the object itself resulting in the same shutdown sequence.
+  void Shutdown();
+
+ protected:
+  // Completes shutdown by posting the completion task on the caller's message
+  // loop.
+  void CompleteShutdown();
+
+  // Derived classes can override this method to implement additional shutdown
+  // logic.

[Sergey Ulanov, 2012/07/30 19:46:22]

Do we need this default implementation? If a class inherits from this class then
we can assume that it needs some stopping logic, so would it be better to make
this method pure virtual?

[alexeypa (please no reviews), 2012/07/30 20:41:43]

Done.

+  virtual void DoShutdown();
+
+  enum State {
+    kRunning,
+    kShutdown,
+    kStopped

[Sergey Ulanov, 2012/07/30 19:46:22]

it should be either kShuttingDown/kShutdown or kStopping/kStopped. IMO it's
better to rename this interface to Stoppable and use kStopping/kStopped.

[alexeypa (please no reviews), 2012/07/30 20:41:43]

Done.

+  };
+
+  State get_shutdownable_state() const { return state_; }

[Sergey Ulanov, 2012/07/30 19:46:22]

drop get_, just shutdownable_state()

[alexeypa (please no reviews), 2012/07/30 20:41:43]

Done.

+
+ private:
+  // The target task runner where the shutdown notification will be posted.
+  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+
+  // The shutdown notification callback.
+  Callback done_;
+
+  State state_;
+
+  DISALLOW_COPY_AND_ASSIGN(Shutdownable);
+};
+
+}  // namespace remoting
+
+#endif  // REMOTING_BASE_SHUTDOWNABLE_H_