[Chromoting] Introducing refcount-based life time management of the message loops in the service (d…

remoting/base/auto_message_loop.h

Index: remoting/base/auto_message_loop.h
diff --git a/remoting/base/auto_message_loop.h b/remoting/base/auto_message_loop.h
new file mode 100644
index 0000000000000000000000000000000000000000..5e0544beb98d1cf0318eccb0f816caaff68dee4a
--- /dev/null
+++ b/remoting/base/auto_message_loop.h
@@ -0,0 +1,62 @@
+// 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_AUTO_MESSAGE_LOOP_H_
+#define REMOTING_BASE_AUTO_MESSAGE_LOOP_H_
+
+#include "base/basictypes.h"
+#include "base/callback.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/message_loop.h"
+#include "base/single_thread_task_runner.h"
+
+class MessageLoop;
+
+namespace remoting {
+
+// This class provides automatic life time management for a single thread task
+// runner. The task runner is stopped when the last reference to
+// |AutoMessageLoop| is dropped.

[Wez, 2012/08/28 17:34:53]

nit: It doesn't actually do that any more, though; it invokes a caller-supplied
callback on a supplied TaskRunner, which may or may not quit it?

[alexeypa (please no reviews), 2012/08/28 19:18:51]

It is technically true. It is hard to figure out the purpose of the class from
the above description. The purpose is the life time management. The fact the it
is implemented via a callback it just the unfortunate result of too many ways to
stop using a message loop.

+class AutoMessageLoop : public base::SingleThreadTaskRunner {

[Wez, 2012/08/28 17:34:53]

nit: AutoMessageLoop doesn't seem the right name for this any more ->
AutoThreadTaskRunner?

But do we really need this class to be so general-purpose; as a result it
doesn't actually _do_ much!

[alexeypa (please no reviews), 2012/08/28 19:18:51]

Sounds like a good name. AutoMessageLoop is shorter (while still long). Let talk
offline.
Well, it provides the only guarantee we want from it - keep the loop alive while
we need it (assuming the callback is doing what it should do).

+ public:
+  // |parent| is optional and may be NULL. |stop_callback| can be called on any
+  // thread.

[Wez, 2012/08/28 17:34:53]

nit: Clarify what |stop_callback| is for / when it'll be called, either here, or
in the class description.

[alexeypa (please no reviews), 2012/08/28 19:18:51]

Done.

+  AutoMessageLoop(base::SingleThreadTaskRunner* parent,

[Wez, 2012/08/28 17:34:53]

nit: scoped_refptr<base::SingleThreadTaskRunner>, to make ref-counting explicit.

[Wez, 2012/08/28 17:34:53]

Why do you need the |parent| reference? The purpose is to keep it alive but
that's something that objects which need it should be doing; in what sense does
the child thread need its parent to persist while it runs?

[alexeypa (please no reviews), 2012/08/28 19:18:51]

Done.

[alexeypa (please no reviews), 2012/08/28 19:18:51]

This is a way to express parent-child relations between message loops. For
instance if the callback posts a task to the parent loop in order to stop the
child one (like it was done in the 1st iteration of this CL).

+                  base::SingleThreadTaskRunner* task_runner,
+                  const base::Closure& stop_callback);
+
+  // SingleThreadTaskRunner implementation
+  virtual bool PostDelayedTask(const tracked_objects::Location& from_here,
+                               const base::Closure& task,
+                               base::TimeDelta delay) OVERRIDE;
+  virtual bool PostNonNestableDelayedTask(
+      const tracked_objects::Location& from_here,
+      const base::Closure& task,
+      base::TimeDelta delay) OVERRIDE;
+  virtual bool RunsTasksOnCurrentThread() const OVERRIDE;
+
+ protected:
+  // Quits the message loop by invoking |stop_callback_|.
+  virtual void OnDestruct() const OVERRIDE;
+
+ private:
+  friend class base::DeleteHelper<AutoMessageLoop>;
+
+  virtual ~AutoMessageLoop();
+
+  // An optional reference to the parent message loop to keep it alive.
+  scoped_refptr<base::SingleThreadTaskRunner> parent_;
+
+  // This callback quits |task_runner_|. It can be run on any thread.
+  base::Closure stop_callback_;
+
+  // The wrapped task runner.
+  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+
+  DISALLOW_COPY_AND_ASSIGN(AutoMessageLoop);
+};
+
+}  // namespace remoting
+
+#endif  // REMOTING_BASE_AUTO_MESSAGE_LOOP_H_