Add POSIX-specific MessageLoopForIO unit-tests.

Each POSIX(ish) platform uses a different underlying MessagePumpForIO
implementation, but all are required to implement a common interface
to support asynchronous I/O on file descriptors.

This CL adds a set of MessageLoopIoPosixTests, which exercise those
functions to verify the semantics of all the implementations. These
include existing tests moved from MessageLoopTest and
MessagePumpLibeventTest, and a new test for mid-callback teardown.

Bug: 
Change-Id: I2b7071107342489530076dfc611738a7ec39065c
Reviewed-on: https://chromium-review.googlesource.com/571518
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487337}

base/BUILD.gn

@@ -2088,6 +2088,7 @@ test("base_unittests") {
     "memory/shared_memory_win_unittest.cc",
     "memory/singleton_unittest.cc",
     "memory/weak_ptr_unittest.cc",
+    "message_loop/message_loop_io_posix_unittest.cc",
     "message_loop/message_loop_task_runner_unittest.cc",
     "message_loop/message_loop_unittest.cc",
     "message_loop/message_pump_glib_unittest.cc",

base/message_loop/message_loop_io_posix_unittest.cc

+// Copyright 2017 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/compiler_specific.h"
+#include "base/files/file.h"
+#include "base/files/file_util.h"
+#include "base/logging.h"
+#include "base/macros.h"
+#include "base/memory/ptr_util.h"
+#include "base/message_loop/message_loop.h"
+#include "base/posix/eintr_wrapper.h"
+#include "base/run_loop.h"
+#include "build/build_config.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace base {
+
+#if !defined(OS_NACL)
+
+namespace {
+
+class MessageLoopForIoPosixTest : public testing::Test {
+ public:
+  MessageLoopForIoPosixTest() {}
+
+  // testing::Test interface.
+  void SetUp() override {
+    // Create a file descriptor.  Doesn't need to be readable or writable,
+    // as we don't need to actually get any notifications.
+    // pipe() is just the easiest way to do it.
+    int pipefds[2];
+    int err = pipe(pipefds);
+    ASSERT_EQ(0, err);
+    read_fd_ = base::File(pipefds[0]);
+    write_fd_ = base::File(pipefds[1]);
+  }
+
+  base::File read_fd_;
+  base::File write_fd_;
+
+  DISALLOW_COPY_AND_ASSIGN(MessageLoopForIoPosixTest);
+};
+
+class TestHandler : public MessageLoopForIO::Watcher {
+ public:
+  void OnFileCanReadWithoutBlocking(int fd) override {
+    watcher_to_delete_ = nullptr;
+    is_readable_ = true;
+    MessageLoop::current()->QuitWhenIdle();
+  }
+  void OnFileCanWriteWithoutBlocking(int fd) override {
+    watcher_to_delete_ = nullptr;
+    is_writable_ = true;
+    MessageLoop::current()->QuitWhenIdle();
+  }
+
+  bool is_readable_ = false;
+  bool is_writable_ = false;
+
+  // If set then the contained watcher will be deleted on notification.
+  std::unique_ptr<MessageLoopForIO::FileDescriptorWatcher> watcher_to_delete_;
+};
+
+TEST_F(MessageLoopForIoPosixTest, FileDescriptorWatcherOutlivesMessageLoop) {
+  // Simulate a MessageLoop that dies before an FileDescriptorWatcher.
+  // This could happen when people use the Singleton pattern or atexit.
+
+  // Arrange for watcher to live longer than message loop.
+  MessageLoopForIO::FileDescriptorWatcher watcher(FROM_HERE);
+  TestHandler handler;
+  {
+    MessageLoopForIO message_loop;
+
+    message_loop.WatchFileDescriptor(write_fd_.GetPlatformFile(), true,
+                                     MessageLoopForIO::WATCH_WRITE, &watcher,
+                                     &handler);
+    // Don't run the message loop, just destroy it.
+  }
+
+  ASSERT_FALSE(handler.is_readable_);
+  ASSERT_FALSE(handler.is_writable_);
+}
+
+TEST_F(MessageLoopForIoPosixTest, FileDescriptorWatcherDoubleStop) {
+  // Verify that it's ok to call StopWatchingFileDescriptor().
+  // (Errors only showed up in valgrind.)
+
+  // Arrange for message loop to live longer than watcher.
+  MessageLoopForIO message_loop;
+  {
+    MessageLoopForIO::FileDescriptorWatcher watcher(FROM_HERE);
+
+    TestHandler handler;
+    message_loop.WatchFileDescriptor(write_fd_.GetPlatformFile(), true,
+                                     MessageLoopForIO::WATCH_WRITE, &watcher,
+                                     &handler);
+    ASSERT_TRUE(watcher.StopWatchingFileDescriptor());
+    ASSERT_TRUE(watcher.StopWatchingFileDescriptor());
+  }
+}
+
+TEST_F(MessageLoopForIoPosixTest, FileDescriptorWatcherDeleteInCallback) {
+  // Verify that it is OK to delete the FileDescriptorWatcher from within a
+  // callback.
+  MessageLoopForIO message_loop;
+
+  TestHandler handler;
+  handler.watcher_to_delete_ =
+      base::MakeUnique<MessageLoopForIO::FileDescriptorWatcher>(FROM_HERE);
+
+  message_loop.WatchFileDescriptor(write_fd_.GetPlatformFile(), true,
+                                   MessageLoopForIO::WATCH_WRITE,
+                                   handler.watcher_to_delete_.get(), &handler);
+  RunLoop().Run();
+}
+
+// Verify that basic readable notification works.
+TEST_F(MessageLoopForIoPosixTest, WatchReadable) {
+  MessageLoopForIO message_loop;
+  MessageLoopForIO::FileDescriptorWatcher watcher(FROM_HERE);
+  TestHandler handler;
+
+  // Watch the pipe for readability.
+  ASSERT_TRUE(MessageLoopForIO::current()->WatchFileDescriptor(
+      read_fd_.GetPlatformFile(), /* persistent= */ false,
+      MessageLoopForIO::WATCH_READ, &watcher, &handler));
+
+  // The pipe should not be readable when first created.
+  base::RunLoop().RunUntilIdle();
+  ASSERT_FALSE(handler.is_readable_);
+  ASSERT_FALSE(handler.is_writable_);
+
+  // Write a byte to the other end, making it readable.
+  const char buf = 0;
+  ASSERT_TRUE(
+      WriteFileDescriptor(write_fd_.GetPlatformFile(), &buf, sizeof(buf)));
+
+  // We don't want to assume that the read fd becomes readable the
+  // instant a bytes is written, so Run until quit by an event.
+  RunLoop().Run();
+
+  ASSERT_TRUE(handler.is_readable_);
+  ASSERT_FALSE(handler.is_writable_);
+}
+
+// Verify that watching a file descriptor for writability succeeds.
+TEST_F(MessageLoopForIoPosixTest, WatchWritable) {
+  MessageLoopForIO message_loop;
+  MessageLoopForIO::FileDescriptorWatcher watcher(FROM_HERE);
+  TestHandler handler;
+
+  // Watch the pipe for writability.
+  ASSERT_TRUE(MessageLoopForIO::current()->WatchFileDescriptor(
+      write_fd_.GetPlatformFile(), /* persistent= */ false,
+      MessageLoopForIO::WATCH_WRITE, &watcher, &handler));
+
+  // We should not receive a writable notification until we process events.
+  ASSERT_FALSE(handler.is_readable_);
+  ASSERT_FALSE(handler.is_writable_);
+
+  // The pipe should be writable immediately, but wait for the quit closure
+  // anyway, to be sure.
+  RunLoop().Run();
+
+  ASSERT_FALSE(handler.is_readable_);
+  ASSERT_TRUE(handler.is_writable_);
+}
+
+}  // namespace
+
+#endif  // !defined(OS_NACL)
+
+}  // namespace base

base/message_loop/message_loop_unittest.cc

@@ -729,78 +729,6 @@ TEST(MessageLoopTest, HighResolutionTimer) {
 
 #endif  // defined(OS_WIN)
 
-#if defined(OS_POSIX) && !defined(OS_NACL)
-
-namespace {
-
-class QuitDelegate : public MessageLoopForIO::Watcher {
- public:
-  void OnFileCanWriteWithoutBlocking(int fd) override {
-    MessageLoop::current()->QuitWhenIdle();
-  }
-  void OnFileCanReadWithoutBlocking(int fd) override {
-    MessageLoop::current()->QuitWhenIdle();
-  }
-};
-
-TEST(MessageLoopTest, FileDescriptorWatcherOutlivesMessageLoop) {
-  // Simulate a MessageLoop that dies before an FileDescriptorWatcher.
-  // This could happen when people use the Singleton pattern or atexit.
-
-  // Create a file descriptor.  Doesn't need to be readable or writable,
-  // as we don't need to actually get any notifications.
-  // pipe() is just the easiest way to do it.
-  int pipefds[2];
-  int err = pipe(pipefds);
-  ASSERT_EQ(0, err);
-  int fd = pipefds[1];
-  {
-    // Arrange for controller to live longer than message loop.
-    MessageLoopForIO::FileDescriptorWatcher controller(FROM_HERE);
-    {
-      MessageLoopForIO message_loop;
-
-      QuitDelegate delegate;
-      message_loop.WatchFileDescriptor(fd,
-          true, MessageLoopForIO::WATCH_WRITE, &controller, &delegate);
-      // and don't run the message loop, just destroy it.
-    }
-  }
-  if (IGNORE_EINTR(close(pipefds[0])) < 0)
-    PLOG(ERROR) << "close";
-  if (IGNORE_EINTR(close(pipefds[1])) < 0)
-    PLOG(ERROR) << "close";
-}
-
-TEST(MessageLoopTest, FileDescriptorWatcherDoubleStop) {
-  // Verify that it's ok to call StopWatchingFileDescriptor().
-  // (Errors only showed up in valgrind.)
-  int pipefds[2];
-  int err = pipe(pipefds);
-  ASSERT_EQ(0, err);
-  int fd = pipefds[1];
-  {
-    // Arrange for message loop to live longer than controller.
-    MessageLoopForIO message_loop;
-    {
-      MessageLoopForIO::FileDescriptorWatcher controller(FROM_HERE);
-
-      QuitDelegate delegate;
-      message_loop.WatchFileDescriptor(fd,
-          true, MessageLoopForIO::WATCH_WRITE, &controller, &delegate);
-      controller.StopWatchingFileDescriptor();
-    }
-  }
-  if (IGNORE_EINTR(close(pipefds[0])) < 0)
-    PLOG(ERROR) << "close";
-  if (IGNORE_EINTR(close(pipefds[1])) < 0)
-    PLOG(ERROR) << "close";
-}
-
-}  // namespace
-
-#endif  // defined(OS_POSIX) && !defined(OS_NACL)
-
 namespace {
 // Inject a test point for recording the destructor calls for Closure objects
 // send to MessageLoop::PostTask(). It is awkward usage since we are trying to

base/message_loop/message_pump_libevent_unittest.cc

@@ -277,87 +277,6 @@ TEST_F(MessagePumpLibeventTest, QuitWatcher) {
       BindOnce(&WaitableEventWatcher::StopWatching, Owned(watcher.release())));
 }
 
-class CaptureAndQuitWatcher : public BaseWatcher {
- public:
-  CaptureAndQuitWatcher(MessagePumpLibevent::FileDescriptorWatcher* controller,
-                        base::Closure quit_closure)
-      : BaseWatcher(controller), quit_closure_(std::move(quit_closure)) {}
-
-  void OnFileCanReadWithoutBlocking(int /* fd */) override {
-    is_readable_ = true;
-    quit_closure_.Run();
-  }
-  void OnFileCanWriteWithoutBlocking(int /* fd */) override {
-    is_writable_ = true;
-    quit_closure_.Run();
-  }
-
-  bool is_readable_ = false;
-  bool is_writable_ = false;
-
- private:
-  base::Closure quit_closure_;
-};
-
-// Verify that basic readable notification works.
-TEST_F(MessagePumpLibeventTest, WatchReadable) {
-  // Tear-down the old MessageLoop before creating the replacement.
-  ui_loop_.reset();
-
-  ui_loop_ = base::MakeUnique<MessageLoopForIO>();
-  MessagePumpLibevent::FileDescriptorWatcher watcher(FROM_HERE);
-  RunLoop run_loop;
-  CaptureAndQuitWatcher delegate(&watcher, run_loop.QuitClosure());
-
-  // Watch the pipe for readability.
-  ASSERT_TRUE(MessageLoopForIO::current()->WatchFileDescriptor(
-      pipefds_[0], /* persistent= */ false, MessageLoopForIO::WATCH_READ,
-      &watcher, &delegate));
-
-  // The pipe should not be readable when first created.
-  base::RunLoop().RunUntilIdle();
-  ASSERT_FALSE(delegate.is_readable_);
-  ASSERT_FALSE(delegate.is_writable_);
-
-  // Write a byte to the other end, making it readable.
-  const char buf = 0;
-  ASSERT_TRUE(WriteFileDescriptor(pipefds_[1], &buf, sizeof(buf)));
-
-  // We don't want to assume that the read fd becomes readable the
-  // instant a bytes is written, so Run until quit by an event.
-  run_loop.Run();
-
-  ASSERT_TRUE(delegate.is_readable_);
-  ASSERT_FALSE(delegate.is_writable_);
-}
-
-// Verify that watching a file descriptor for writability succeeds.
-TEST_F(MessagePumpLibeventTest, WatchWritable) {
-  // Tear-down the old MessageLoop before creating the replacement.
-  ui_loop_.reset();
-
-  ui_loop_ = base::MakeUnique<MessageLoopForIO>();
-  MessagePumpLibevent::FileDescriptorWatcher watcher(FROM_HERE);
-  RunLoop run_loop;
-  CaptureAndQuitWatcher delegate(&watcher, run_loop.QuitClosure());
-
-  // Watch the pipe for writability.
-  ASSERT_TRUE(MessageLoopForIO::current()->WatchFileDescriptor(
-      pipefds_[1], /* persistent= */ false, MessageLoopForIO::WATCH_WRITE,
-      &watcher, &delegate));
-
-  // We should not receive a writable notification until we process events.
-  ASSERT_FALSE(delegate.is_readable_);
-  ASSERT_FALSE(delegate.is_writable_);
-
-  // The pipe should be writable immediately, so no need to wait for
-  // the quit closure.
-  run_loop.RunUntilIdle();
-
-  ASSERT_FALSE(delegate.is_readable_);
-  ASSERT_TRUE(delegate.is_writable_);
-}
-
 }  // namespace
 
 }  // namespace base