Simplify threading in browser thread by making only ChromeThread deal with di...

chrome/browser/chrome_thread.h

Index: chrome/browser/chrome_thread.h
===================================================================
--- chrome/browser/chrome_thread.h	(revision 30037)
+++ chrome/browser/chrome_thread.h	(working copy)
@@ -1,11 +1,12 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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 CHROME_BROWSER_CHROME_THREAD_H__
-#define CHROME_BROWSER_CHROME_THREAD_H__
+#ifndef CHROME_BROWSER_CHROME_THREAD_H_
+#define CHROME_BROWSER_CHROME_THREAD_H_
 
 #include "base/lock.h"
+#include "base/task.h"
 #include "base/thread.h"
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -14,30 +15,29 @@
 // This class represents a thread that is known by a browser-wide name.  For
 // example, there is one IO thread for the entire browser process, and various
 // pieces of code find it useful to retrieve a pointer to the IO thread's
-// MessageLoop by name:
+// Invoke a task by thread ID:
 //
-//   MessageLoop* io_loop = ChromeThread::GetMessageLoop(ChromeThread::IO);
+//   ChromeThread::PostTask(ChromeThread::IO, FROM_HERE, task);
 //
-// On the UI thread, it is often preferable to obtain a pointer to a well-known
-// thread via the g_browser_process object, e.g. g_browser_process->io_thread();
+// The return value is false if the task couldn't be posted because the target
+// thread doesn't exist.  If this could lead to data loss, you need to check the
+// result and restructure the code to ensure it doesn't occur.
 //
-// Code that runs on a thread other than the UI thread must take extra care in
-// handling pointers to threads because many of the well-known threads are owned
-// by the UI thread and can be deallocated without notice.
-//
+// This class automatically handles the lifetime of different threads.
+// It's always safe to call PostTask on any thread.  If it's not yet created,
+// the task is deleted.  There are no race conditions.  If the thread that the
+// task is posted to is guaranteed to outlive the current thread, then no locks
+// are used.  You should never need to cache pointers to MessageLoops, since
+// they're not thread safe.
 class ChromeThread : public base::Thread {
  public:
   // An enumeration of the well-known threads.
+  // NOTE: threads must be listed in the order of their life-time, with each
+  // thread outliving every other thread below it.
   enum ID {

[darin (slow to review), 2009/10/27 00:06:52]

maybe we should have an identifier named CURRENT, so that the same
ChromeThread::PostTask interfaces can be used for posting a task back to the
current thread.  that would help encourage people to write code that can live on
any thread instead of code that only runs on a specific thread.

[jam, 2009/10/27 02:38:18]

Wouldn't this be a little confusing, since in the browser process people would
use ChromeThread::PostTask(ChromeThread::CURRENT while in other processes it
would be MessageLoop::current()->PostTask?

[darin (slow to review), 2009/10/27 04:43:33]

True, good point.

This sort of makes me wish that base/ provided the mechanism here, and that we
just had a way like we do with PathService to allow modules to extend the set of
items.  Then, we could use a consistent mechanism everywhere.

     // The main thread in the browser.
     UI,
 
-    // This is the thread that processes IPC and network messages.
-    IO,
-
-    // This is the thread that interacts with the file system.
-    FILE,
-
     // This is the thread that interacts with the database.
     DB,
 
@@ -45,6 +45,12 @@
     // NOT in --single-process mode.
     WEBKIT,
 
+    // This is the thread that interacts with the file system.
+    FILE,
+
+    // This is the thread that processes IPC and network messages.
+    IO,
+
 #if defined(OS_LINUX)
     // This thread has a second connection to the X server and is used to
     // process UI requests when routing the request to the UI thread would risk
@@ -62,21 +68,47 @@
   // to construct a ChromeThread that already exists.
   explicit ChromeThread(ID identifier);
 
-  // Special constructor for the main (UI) thread. We use a dummy thread here
-  // since the main thread already exists.
-  ChromeThread();
+  // Special constructor for the main (UI) thread and unittests. We use a dummy
+  // thread here since the main thread already exists.
+  ChromeThread(ID identifier, MessageLoop* message_loop);
 
   virtual ~ChromeThread();
 
-  // Callable on any thread, this helper function returns a pointer to the
-  // thread's MessageLoop.
-  //
-  // WARNING:
-  //   Nothing in this class prevents the MessageLoop object returned from this
-  //   function from being destroyed on another thread.  Use with care.
-  //
-  static MessageLoop* GetMessageLoop(ID identifier);
+  // These are the same methods in message_loop.h, but are guaranteed to either
+  // get posted to the MessageLoop if it's still alive, or be deleted otherwise.
+  // They return true iff the thread existed and the task was posted.

[darin (slow to review), 2009/10/27 00:06:52]

you should probably add a note here indicating that success in posting a task
does not ensure that it actually gets run.

you should probably also say that failure to post a task can only happen during
system shutdown.

[jam, 2009/10/27 02:38:18]

Done.
This isn't always the case (i.e. the thread wasn't started yet, like the webkit
thread).

[darin (slow to review), 2009/10/27 04:43:33]

OK

+  static bool PostTask(ID identifier,
+                       const tracked_objects::Location& from_here,
+                       Task* task);
+  static bool PostDelayedTask(ID identifier,
+                              const tracked_objects::Location& from_here,
+                              Task* task,
+                              int64 delay_ms);
+  static bool PostNonNestableTask(ID identifier,
+                                  const tracked_objects::Location& from_here,
+                                  Task* task);
+  static bool PostNonNestableDelayedTask(
+      ID identifier,
+      const tracked_objects::Location& from_here,
+      Task* task,
+      int64 delay_ms);
 
+  template <class T>
+  static bool DeleteSoon(ID identifier,
+                         const tracked_objects::Location& from_here,
+                         T* object) {
+    return PostNonNestableTask(
+        identifier, from_here, new DeleteTask<T>(object));
+  }
+
+  template <class T>
+  static bool ReleaseSoon(ID identifier,
+                          const tracked_objects::Location& from_here,
+                          T* object) {
+    return PostNonNestableTask(
+        identifier, from_here, new ReleaseTask<T>(object));
+  }
+
   // Callable on any thread.  Returns whether you're currently on a particular
   // thread.
   //
@@ -91,6 +123,17 @@
   // Common initialization code for the constructors.
   void Initialize();
 
+  // If the current message loop is one of the known threads, returns true and
+  // sets identifier to its ID.  Otherwise returns false.
+  static bool GetCurrentThreadIdentifier(ID* identifier);
+
+  static bool PostTaskHelper(
+      ID identifier,
+      const tracked_objects::Location& from_here,
+      Task* task,
+      int64 delay_ms,
+      bool nestable);
+
   // The identifier of this thread.  Only one thread can exist with a given
   // identifier at a given time.
   ID identifier_;
@@ -106,4 +149,4 @@
   static ChromeThread* chrome_threads_[ID_COUNT];
 };
 
-#endif  // #ifndef CHROME_BROWSER_CHROME_THREAD_H__
+#endif  // #ifndef CHROME_BROWSER_CHROME_THREAD_H_