libevent message-pump: Fix a particular case of nested message-loop runs.

If Quit() is called on the message-pump to terminate a nested loop, followed
by a call to Run() again to start another nested loop, in the same iteration
of the loop, then make sure it all works correctly.

We have this scenario with nested menus, modal dialogs, drag-drop etc. in
the browser UI. So this is a prerequisite fix to be able to use the
libevent based message-pump on Chrome OS (and get rid of glib dep).

BUG=240715
R=sky@chromium.org
TBR=darin@chromium.org

Review URL: https://codereview.chromium.org/245923005

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

base/message_loop/message_pump_libevent.cc

@@ -217,7 +217,7 @@ static void timer_callback(int fd, short events, void *context)
 
 // Reentrant!
 void MessagePumpLibevent::Run(Delegate* delegate) {
-  DCHECK(keep_running_) << "Quit must have been called outside of Run!";
+  AutoReset<bool> auto_reset_keep_running(&keep_running_, true);
   AutoReset<bool> auto_reset_in_run(&in_run_, true);
 
   // event_base_loopexit() + EVLOOP_ONCE is leaky, see http://crbug.com/25641.
@@ -275,12 +275,10 @@ void MessagePumpLibevent::Run(Delegate* delegate) {
       }
     }
   }
-
-  keep_running_ = true;
 }
 
 void MessagePumpLibevent::Quit() {
-  DCHECK(in_run_);
+  DCHECK(in_run_) << "Quit was called outside of Run!";
   // Tell both libevent and Run that they should break out of their loops.
   keep_running_ = false;
   ScheduleWork();

base/message_loop/message_pump_libevent_unittest.cc

@@ -6,8 +6,10 @@
 
 #include <unistd.h>
 
+#include "base/bind.h"
 #include "base/message_loop/message_loop.h"
 #include "base/posix/eintr_wrapper.h"
+#include "base/run_loop.h"
 #include "base/threading/thread.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/libevent/event.h"
@@ -81,6 +83,12 @@ TEST_F(MessagePumpLibeventTest, TestWatchingFromBadThread) {
       "watch_file_descriptor_caller_checker_.CalledOnValidThread\\(\\)");
 }
 
+TEST_F(MessagePumpLibeventTest, QuitOutsideOfRun) {
+  scoped_ptr<MessagePumpLibevent> pump(new MessagePumpLibevent);
+  ASSERT_DEATH(pump->Quit(), "Check failed: in_run_. "
+                             "Quit was called outside of Run!");
+}
+
 #endif  // GTEST_HAS_DEATH_TEST && !defined(NDEBUG)
 
 class BaseWatcher : public MessagePumpLibevent::Watcher {
@@ -157,6 +165,41 @@ TEST_F(MessagePumpLibeventTest, StopWatcher) {
   OnLibeventNotification(pump.get(), &watcher);
 }
 
+void QuitMessageLoopAndStart(const Closure& quit_closure) {
+  quit_closure.Run();
+
+  MessageLoop::ScopedNestableTaskAllower allow(MessageLoop::current());
+  RunLoop runloop;
+  MessageLoop::current()->PostTask(FROM_HERE, runloop.QuitClosure());
+  runloop.Run();
+}
+
+class NestedPumpWatcher : public MessagePumpLibevent::Watcher {
+ public:
+  NestedPumpWatcher() {}
+  virtual ~NestedPumpWatcher() {}
+
+  virtual void OnFileCanReadWithoutBlocking(int /* fd */) OVERRIDE {
+    RunLoop runloop;
+    MessageLoop::current()->PostTask(FROM_HERE, Bind(&QuitMessageLoopAndStart,
+                                                     runloop.QuitClosure()));
+    runloop.Run();
+  }
+
+  virtual void OnFileCanWriteWithoutBlocking(int /* fd */) OVERRIDE {}
+};
+
+TEST_F(MessagePumpLibeventTest, NestedPumpWatcher) {
+  scoped_ptr<MessagePumpLibevent> pump(new MessagePumpLibevent);
+  MessagePumpLibevent::FileDescriptorWatcher watcher;
+  NestedPumpWatcher delegate;
+  pump->WatchFileDescriptor(pipefds_[1],
+      false, MessagePumpLibevent::WATCH_READ, &watcher, &delegate);
+
+  // Spoof a libevent notification.
+  OnLibeventNotification(pump.get(), &watcher);
+}
+
 }  // namespace
 
 }  // namespace base