Microoptimisation for OnLibeventNotification.

base/message_loop/message_pump_libevent.cc

Index: base/message_loop/message_pump_libevent.cc
diff --git a/base/message_loop/message_pump_libevent.cc b/base/message_loop/message_pump_libevent.cc
index b5b1fb7e81e860340920174309f968fe0048a0a6..2a4e107ff2ffaa436f151a7fc027baa0d4966710 100644
--- a/base/message_loop/message_pump_libevent.cc
+++ b/base/message_loop/message_pump_libevent.cc
@@ -55,13 +55,17 @@ MessagePumpLibevent::FileDescriptorWatcher::FileDescriptorWatcher()
     : event_(NULL),
       pump_(NULL),
       watcher_(NULL),
-      weak_factory_(this) {
+      was_destroyed_(NULL) {
 }
 
 MessagePumpLibevent::FileDescriptorWatcher::~FileDescriptorWatcher() {
   if (event_) {
     StopWatchingFileDescriptor();
   }
+  if (was_destroyed_) {
+    DCHECK(!*was_destroyed_);
+    *was_destroyed_ = true;
+  }
 }
 
 bool MessagePumpLibevent::FileDescriptorWatcher::StopWatchingFileDescriptor() {
@@ -340,23 +344,31 @@ bool MessagePumpLibevent::Init() {
 }
 
 // static
-void MessagePumpLibevent::OnLibeventNotification(int fd, short flags,
+void MessagePumpLibevent::OnLibeventNotification(int fd,
+                                                 short flags,
                                                  void* context) {
-  WeakPtr<FileDescriptorWatcher> controller =
-      static_cast<FileDescriptorWatcher*>(context)->weak_factory_.GetWeakPtr();
-  DCHECK(controller.get());
+  FileDescriptorWatcher* controller =
+      static_cast<FileDescriptorWatcher*>(context);
+  DCHECK(controller);
   TRACE_EVENT1("toplevel", "MessagePumpLibevent::OnLibeventNotification",
                "fd", fd);
 
   MessagePumpLibevent* pump = controller->pump();
   pump->processed_io_events_ = true;
 
-  if (flags & EV_WRITE) {
+  if ((flags & (EV_WRITE | EV_READ)) == (EV_WRITE | EV_READ)) {

[Lei Zhang, 2015/08/05 23:59:58]

silly nit: put EV_READ ahead of EV_WRITE. "R/W" is much more common than "W/R".
As in DVD-RW.

[Adam Rice, 2015/08/06 00:46:46]

Best nitpick ever. Done.

+    // Both callbacks will be called. It is necessary to check that |controller|
+    // is not destroyed.
+    bool controller_was_destroyed = false;
+    controller->was_destroyed_ = &controller_was_destroyed;
     controller->OnFileCanWriteWithoutBlocking(fd, pump);
-  }
-  // Check |controller| in case it's been deleted in
-  // controller->OnFileCanWriteWithoutBlocking().
-  if (controller.get() && flags & EV_READ) {
+    if (!controller_was_destroyed)
+      controller->OnFileCanReadWithoutBlocking(fd, pump);
+    if (!controller_was_destroyed)
+      controller->was_destroyed_ = NULL;

[Lei Zhang, 2015/08/05 23:59:58]

Please use nullptr in new code. I'm "meh" about line 58 since it's being
consistent with the lines above it.

[Adam Rice, 2015/08/06 00:46:46]

Done.

+  } else if (flags & EV_WRITE) {
+    controller->OnFileCanWriteWithoutBlocking(fd, pump);
+  } else if (flags & EV_READ) {
     controller->OnFileCanReadWithoutBlocking(fd, pump);
   }
 }