Re-land: Add missing check for quitting from MessagePumpLibevent

If a MessagePumpLibevent has no tasks to run, it will call libevent's
event_base_loop with EVLOOP_ONCE.  However, it may turn out that the
event that finally runs actually quits the MessageLoop.  In this case,
there is an off-by-one error where MessagePumpLibevent will call the
MessageLoop's DoWork function once before realizing that it was actually
asked to quit.  Add a check to prevent this from happening.

BUG=473693

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

Cr-Commit-Position: refs/heads/master@{#323834}

2 files changed, 73 insertions, 3 deletions

diff --git a/base/message_loop/message_pump_libevent.cc b/base/message_loop/message_pump_libevent.cc
index d52025a..743b4f7 100644
--- a/base/message_loop/message_pump_libevent.cc
+++ b/base/message_loop/message_pump_libevent.cc
@@ -274,6 +274,9 @@ void MessagePumpLibevent::Run(Delegate* delegate) {
         delayed_work_time_ = TimeTicks();
       }
     }
+
+    if (!keep_running_)
+      break;
   }
 }
 
diff --git a/base/message_loop/message_pump_libevent_unittest.cc b/base/message_loop/message_pump_libevent_unittest.cc
index 1058ee8..65d7217 100644
--- a/base/message_loop/message_pump_libevent_unittest.cc
+++ b/base/message_loop/message_pump_libevent_unittest.cc
@@ -7,9 +7,14 @@
 #include <unistd.h>
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/files/file_util.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/run_loop.h"
+#include "base/synchronization/waitable_event.h"
+#include "base/synchronization/waitable_event_watcher.h"
 #include "base/threading/thread.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/libevent/event.h"
@@ -19,7 +24,7 @@ namespace base {
 class MessagePumpLibeventTest : public testing::Test {
  protected:
   MessagePumpLibeventTest()
-      : ui_loop_(MessageLoop::TYPE_UI),
+      : ui_loop_(new MessageLoop(MessageLoop::TYPE_UI)),
         io_thread_("MessagePumpLibeventTestIOThread") {}
   ~MessagePumpLibeventTest() override {}
 
@@ -38,7 +43,6 @@ class MessagePumpLibeventTest : public testing::Test {
       PLOG(ERROR) << "close";
   }
 
-  MessageLoop* ui_loop() { return &ui_loop_; }
   MessageLoopForIO* io_loop() const {
     return static_cast<MessageLoopForIO*>(io_thread_.message_loop());
   }
@@ -50,9 +54,9 @@ class MessagePumpLibeventTest : public testing::Test {
   }
 
   int pipefds_[2];
+  scoped_ptr<MessageLoop> ui_loop_;
 
  private:
-  MessageLoop ui_loop_;
   Thread io_thread_;
 };
 
@@ -194,6 +198,69 @@ TEST_F(MessagePumpLibeventTest, NestedPumpWatcher) {
   OnLibeventNotification(pump.get(), &watcher);
 }
 
+void FatalClosure() {
+  FAIL() << "Reached fatal closure.";
+}
+
+class QuitWatcher : public BaseWatcher {
+ public:
+  QuitWatcher(MessagePumpLibevent::FileDescriptorWatcher* controller,
+              RunLoop* run_loop)
+      : BaseWatcher(controller), run_loop_(run_loop) {}
+  ~QuitWatcher() override {}
+
+  void OnFileCanReadWithoutBlocking(int /* fd */) override {
+    // Post a fatal closure to the MessageLoop before we quit it.
+    MessageLoop::current()->PostTask(FROM_HERE, Bind(&FatalClosure));
+
+    // Now quit the MessageLoop.
+    run_loop_->Quit();
+  }
+
+ private:
+  RunLoop* run_loop_;  // weak
+};
+
+void WriteFDWrapper(const int fd,
+                    const char* buf,
+                    int size,
+                    WaitableEvent* event) {
+  ASSERT_TRUE(WriteFileDescriptor(fd, buf, size));
+}
+
+// Tests that MessagePumpLibevent quits immediately when it is quit from
+// libevent's event_base_loop().
+TEST_F(MessagePumpLibeventTest, QuitWatcher) {
+  // Delete the old MessageLoop so that we can manage our own one here.
+  ui_loop_.reset();
+
+  MessagePumpLibevent* pump = new MessagePumpLibevent;  // owned by |loop|.
+  MessageLoop loop(make_scoped_ptr(pump));
+  RunLoop run_loop;
+  MessagePumpLibevent::FileDescriptorWatcher controller;
+  QuitWatcher delegate(&controller, &run_loop);
+  WaitableEvent event(false /* manual_reset */, false /* initially_signaled */);
+  WaitableEventWatcher watcher;
+
+  // Tell the pump to watch the pipe.
+  pump->WatchFileDescriptor(pipefds_[0], false, MessagePumpLibevent::WATCH_READ,
+                            &controller, &delegate);
+
+  // Make the IO thread wait for |event| before writing to pipefds[1].
+  const char buf = 0;
+  const WaitableEventWatcher::EventCallback write_fd_task =
+      Bind(&WriteFDWrapper, pipefds_[1], &buf, 1);
+  io_loop()->PostTask(FROM_HERE,
+                      Bind(IgnoreResult(&WaitableEventWatcher::StartWatching),
+                           Unretained(&watcher), &event, write_fd_task));
+
+  // Queue |event| to signal on |loop|.
+  loop.PostTask(FROM_HERE, Bind(&WaitableEvent::Signal, Unretained(&event)));
+
+  // Now run the MessageLoop.
+  run_loop.Run();
+}
+
 }  // namespace
 
 }  // namespace base