Add thread check to WatchFileDescriptor.

I had a case where I was accidentally calling WatchFileDescriptor from another
thread, and it took me a while to debug why I was having problems.
This check would've immediately caught my error for me.

BUG=none
TEST=build

Review URL: http://codereview.chromium.org/6410035

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@85271 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 74 insertions, 1 deletions

diff --git a/base/base.gyp b/base/base.gyp
index 0e3c72d..8514894 100644
--- a/base/base.gyp
+++ b/base/base.gyp
@@ -154,6 +154,7 @@
         'message_loop_proxy_impl_unittest.cc',
         'message_loop_unittest.cc',
         'message_pump_glib_unittest.cc',
+        'message_pump_libevent_unittest.cc',
         'metrics/field_trial_unittest.cc',
         'metrics/histogram_unittest.cc',
         'metrics/stats_table_unittest.cc',
@@ -275,6 +276,7 @@
             'dir_reader_posix_unittest.cc',
             'file_descriptor_shuffle_unittest.cc',
             'threading/worker_pool_posix_unittest.cc',
+            'message_pump_libevent_unittest.cc',
           ],
         }, {  # OS != "win"
           'sources/': [
diff --git a/base/message_pump_libevent.cc b/base/message_pump_libevent.cc
index 5154233..24eba1e 100644
--- a/base/message_pump_libevent.cc
+++ b/base/message_pump_libevent.cc
@@ -139,6 +139,9 @@ bool MessagePumpLibevent::WatchFileDescriptor(int fd,
   DCHECK(controller);
   DCHECK(delegate);
   DCHECK(mode == WATCH_READ || mode == WATCH_WRITE || mode == WATCH_READ_WRITE);
+  // WatchFileDescriptor should be called on the pump thread. It is not
+  // threadsafe, and your watcher may never be registered.
+  DCHECK(watch_file_descriptor_caller_checker_.CalledOnValidThread());
 
   int event_mask = persistent ? EV_PERSIST : 0;
   if ((mode & WATCH_READ) != 0) {
diff --git a/base/message_pump_libevent.h b/base/message_pump_libevent.h
index d58d902..16c58a2 100644
--- a/base/message_pump_libevent.h
+++ b/base/message_pump_libevent.h
@@ -9,6 +9,7 @@
 #include "base/basictypes.h"
 #include "base/message_pump.h"
 #include "base/observer_list.h"
+#include "base/threading/thread_checker.h"
 #include "base/time.h"
 
 // Declare structs we need from libevent.h rather than including it
@@ -105,6 +106,7 @@ class BASE_API MessagePumpLibevent : public MessagePump {
   // If an error occurs while calling this method in a cumulative fashion, the
   // event previously attached to |controller| is aborted.
   // Returns true on success.
+  // Must be called on the same thread the message_pump is running on.
   // TODO(dkegel): switch to edge-triggered readiness notification
   bool WatchFileDescriptor(int fd,
                            bool persistent,
@@ -157,7 +159,7 @@ class BASE_API MessagePumpLibevent : public MessagePump {
   event* wakeup_event_;
 
   ObserverList<IOObserver> io_observers_;
-
+  ThreadChecker watch_file_descriptor_caller_checker_;
   DISALLOW_COPY_AND_ASSIGN(MessagePumpLibevent);
 };
 
diff --git a/base/message_pump_libevent_unittest.cc b/base/message_pump_libevent_unittest.cc
new file mode 100644
index 0000000..2d7e1fd
--- /dev/null
+++ b/base/message_pump_libevent_unittest.cc
@@ -0,0 +1,66 @@
+// Copyright (c) 2011 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/message_pump_libevent.h"
+
+#include <unistd.h>
+
+#include "base/message_loop.h"
+#include "base/threading/thread.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+class MessagePumpLibeventTest : public testing::Test {
+ public:
+  MessagePumpLibeventTest()
+      : ui_loop_(MessageLoop::TYPE_UI),
+        io_thread_("MessagePumpLibeventTestIOThread") {}
+  virtual ~MessagePumpLibeventTest() {}
+
+  virtual void SetUp() {
+    base::Thread::Options options(MessageLoop::TYPE_IO, 0);
+    ASSERT_TRUE(io_thread_.StartWithOptions(options));
+    ASSERT_EQ(MessageLoop::TYPE_IO, io_thread_.message_loop()->type());
+  }
+
+  MessageLoop* ui_loop() { return &ui_loop_; }
+  MessageLoopForIO* io_loop() const {
+    return static_cast<MessageLoopForIO*>(io_thread_.message_loop());
+  }
+
+ private:
+  MessageLoop ui_loop_;
+  base::Thread io_thread_;
+  DISALLOW_COPY_AND_ASSIGN(MessagePumpLibeventTest);
+};
+
+// Concrete implementation of base::MessagePumpLibevent::Watcher that does
+// nothing useful.
+class StupidWatcher : public base::MessagePumpLibevent::Watcher {
+ public:
+  virtual ~StupidWatcher() {}
+
+  // base:MessagePumpLibEvent::Watcher interface
+  virtual void OnFileCanReadWithoutBlocking(int fd) {}
+  virtual void OnFileCanWriteWithoutBlocking(int fd) {}
+};
+
+}  // namespace
+
+#if GTEST_HAS_DEATH_TEST
+
+// Test to make sure that we catch calling WatchFileDescriptor off of the
+// wrong thread.
+TEST_F(MessagePumpLibeventTest, TestWatchingFromBadThread) {
+  base::MessagePumpLibevent::FileDescriptorWatcher watcher;
+  StupidWatcher delegate;
+
+  ASSERT_DEBUG_DEATH(io_loop()->WatchFileDescriptor(
+      STDOUT_FILENO, false, MessageLoopForIO::WATCH_READ, &watcher, &delegate),
+      "Check failed: "
+      "watch_file_descriptor_caller_checker_.CalledOnValidThread()");
+}
+
+#endif  // GTEST_HAS_DEATH_TEST