Fix failed apk unit test case MessagePumpLibeventTest.* on Android.

base/message_pump_libevent_unittest.cc

Index: base/message_pump_libevent_unittest.cc
diff --git a/base/message_pump_libevent_unittest.cc b/base/message_pump_libevent_unittest.cc
index acc33c077296c5667bf58af59d5eb8499b7687d1..a5380b7aa2fafa204c7917d863bcf0e2970b8ccb 100644
--- a/base/message_pump_libevent_unittest.cc
+++ b/base/message_pump_libevent_unittest.cc
@@ -6,6 +6,7 @@
 
 #include <unistd.h>
 
+#include "base/eintr_wrapper.h"
 #include "base/message_loop.h"
 #include "base/threading/thread.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -101,15 +102,23 @@ class DeleteWatcher : public MessagePumpLibevent::Watcher {
 };
 
 TEST_F(MessagePumpLibeventTest, DeleteWatcher) {
+  int pipefds[2];
+  int err = pipe(pipefds);
+  ASSERT_EQ(0, err);
+  int fd = pipefds[1];
   scoped_refptr<MessagePumpLibevent> pump(new MessagePumpLibevent);
   MessagePumpLibevent::FileDescriptorWatcher* watcher =
       new MessagePumpLibevent::FileDescriptorWatcher;
   DeleteWatcher delegate(watcher);
   pump->WatchFileDescriptor(
-      0, false, MessagePumpLibevent::WATCH_READ_WRITE, watcher, &delegate);
+      fd, false, MessagePumpLibevent::WATCH_READ_WRITE, watcher, &delegate);
 
   // Spoof a libevent notification.
   OnLibeventNotification(pump, watcher);
+  if (HANDLE_EINTR(close(pipefds[0])) < 0)
+    PLOG(ERROR) << "close";
+  if (HANDLE_EINTR(close(pipefds[1])) < 0)
+    PLOG(ERROR) << "close";
 }
 
 class StopWatcher : public MessagePumpLibevent::Watcher {
@@ -134,14 +143,22 @@ class StopWatcher : public MessagePumpLibevent::Watcher {
 };
 
 TEST_F(MessagePumpLibeventTest, StopWatcher) {
+  int pipefds[2];
+  int err = pipe(pipefds);
+  ASSERT_EQ(0, err);
+  int fd = pipefds[1];

[jar (doing other things), 2012/07/09 21:12:23]

I hate seeing growth of replicated code... 

WDYT of:

Put this creation/closing into the constructor and destructor of the test class.
 This way it is harder to "not clean up" in the test.

Given that DeleteWatcher and StopWatcher are almost identical (and grow larger
and yet more identical with this constructor/destructor addition), combine them
into one Watcher class, with an enum arg to the constructor indicating if it
should DELETE or just STOP.

You can then have two separately named derived clases StopWatcher and
DeleteWatcher that pass in the two choice DELETE vs STOP, and use them in the
test (as they are now).

Code gets bloated more and more as folks make these changes... and it would be
nice to clean as you fix.

WDYT?

[Shouqun Liu, 2012/07/10 05:42:41]

Yes, agree with you.  I have made the following changes:
1, move the creation/deletion of pipe into the SetUp/TearDown of test case class
MessagePumpLibeventTest to reducing replicated code.
2, Use a base class BaseWatcher to merge StopWatcher/DeleteWatcher, and
StopWatcher/DeleteWatcher derived from BaseWatcher, TYPE_STOP/TYPE_DELETE in
constructor to control which one should be used.
On 2012/07/09 21:12:23, jar wrote:

   scoped_refptr<MessagePumpLibevent> pump(new MessagePumpLibevent);
   MessagePumpLibevent::FileDescriptorWatcher watcher;
   StopWatcher delegate(&watcher);
   pump->WatchFileDescriptor(
-      0, false, MessagePumpLibevent::WATCH_READ_WRITE, &watcher, &delegate);
+      fd, false, MessagePumpLibevent::WATCH_READ_WRITE, &watcher, &delegate);
 
   // Spoof a libevent notification.
   OnLibeventNotification(pump, &watcher);
+  if (HANDLE_EINTR(close(pipefds[0])) < 0)
+    PLOG(ERROR) << "close";
+  if (HANDLE_EINTR(close(pipefds[1])) < 0)
+    PLOG(ERROR) << "close";
 }
 
 }  // namespace