Migrate to base::FileDescriptionWatcher in dbus/bus.cc.

dbus/bus.cc

 // Copyright (c) 2012 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 "dbus/bus.h"
 
 #include <stddef.h>
 
+#include <memory>
+
 #include "base/bind.h"
+#include "base/files/file_descriptor_watcher_posix.h"
 #include "base/logging.h"
-#include "base/message_loop/message_loop.h"
 #include "base/stl_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "dbus/exported_object.h"
 #include "dbus/message.h"
 #include "dbus/object_manager.h"
 #include "dbus/object_path.h"
@@ skipping 13 matching lines @@
 const char kNameOwnerChangedSignal[] = "NameOwnerChanged";
 
 // The match rule used to filter for changes to a given service name owner.
 const char kServiceNameOwnerChangeMatchRule[] =
     "type='signal',interface='org.freedesktop.DBus',"
     "member='NameOwnerChanged',path='/org/freedesktop/DBus',"
     "sender='org.freedesktop.DBus',arg0='%s'";
 
 // The class is used for watching the file descriptor used for D-Bus
 // communication.
-class Watch : public base::MessagePumpLibevent::Watcher {
+class Watch {
  public:
-  explicit Watch(DBusWatch* watch)
-      : raw_watch_(watch), file_descriptor_watcher_(FROM_HERE) {
+  explicit Watch(DBusWatch* watch) : raw_watch_(watch) {
     dbus_watch_set_data(raw_watch_, this, NULL);
   }
 
-  ~Watch() override { dbus_watch_set_data(raw_watch_, NULL, NULL); }
+  ~Watch() { dbus_watch_set_data(raw_watch_, NULL, NULL); }
 
   // Returns true if the underlying file descriptor is ready to be watched.
   bool IsReadyToBeWatched() {
     return dbus_watch_get_enabled(raw_watch_);
   }
 
   // Starts watching the underlying file descriptor.
   void StartWatching() {
     const int file_descriptor = dbus_watch_get_unix_fd(raw_watch_);
-    const int flags = dbus_watch_get_flags(raw_watch_);
+    const unsigned int flags = dbus_watch_get_flags(raw_watch_);
 
-    base::MessageLoopForIO::Mode mode = base::MessageLoopForIO::WATCH_READ;
-    if ((flags & DBUS_WATCH_READABLE) && (flags & DBUS_WATCH_WRITABLE))
-      mode = base::MessageLoopForIO::WATCH_READ_WRITE;
-    else if (flags & DBUS_WATCH_READABLE)
-      mode = base::MessageLoopForIO::WATCH_READ;
-    else if (flags & DBUS_WATCH_WRITABLE)
-      mode = base::MessageLoopForIO::WATCH_WRITE;
-    else
-      NOTREACHED();
-
-    const bool persistent = true;  // Watch persistently.
-    const bool success = base::MessageLoopForIO::current()->WatchFileDescriptor(
-        file_descriptor, persistent, mode, &file_descriptor_watcher_, this);
-    CHECK(success) << "Unable to allocate memory";
+    // Using base::Unretained(this) is safe because watches are automatically
+    // canceled when |read_watcher_| and |write_watcher_| are destroyed.
+    if (flags & DBUS_WATCH_READABLE) {
+      read_watcher_ = base::FileDescriptorWatcher::WatchReadable(

[satorux1, 2017/04/12 01:02:30]

The function comment for WatchReadable() says:


  // To call these methods, a FileDescriptorWatcher must have been      
  // instantiated on the current thread and SequencedTaskRunnerHandle::IsSet()
  // must return true.                                                          
  

It wasn't clear to me where FileDescriptorWatcher is instantiated. I took a
quick look and found that a thread with MessageLoopForIO has an instance:

https://cs.chromium.org/chromium/src/base/threading/thread.cc?q=new%5C+Filede...

Is FileDescriptorWatcher also instantiated in "TaskScheduler threads"?

[fdoray, 2017/04/12 12:39:47]

MessageLoopForIO and TaskScheduler threads instantiate a
base::FileDescriptorWatcher and hence allow calls to
base::FileDescriptorWatcher::WatchReadable/Writable. "FileDescriptorWatcher" in
base/files/file_descriptor_watcher_posix.h refers to
base::FileDescriptorWatcher.

I want to make base::FileDescriptorWatcher work everywhere and to remove
MessageLoopForIO::FileDescriptorWatcher, but there's a lot of work to do before
we get there.

+          file_descriptor,
+          base::Bind(&Watch::OnFileReady, base::Unretained(this),
+                     DBUS_WATCH_READABLE));
+    }
+    if (flags & DBUS_WATCH_WRITABLE) {
+      write_watcher_ = base::FileDescriptorWatcher::WatchWritable(
+          file_descriptor,
+          base::Bind(&Watch::OnFileReady, base::Unretained(this),
+                     DBUS_WATCH_WRITABLE));
+    }
   }
 
   // Stops watching the underlying file descriptor.
   void StopWatching() {
-    file_descriptor_watcher_.StopWatchingFileDescriptor();
+    read_watcher_.reset();
+    write_watcher_.reset();
   }
 
  private:
-  // Implement MessagePumpLibevent::Watcher.
-  void OnFileCanReadWithoutBlocking(int file_descriptor) override {
-    const bool success = dbus_watch_handle(raw_watch_, DBUS_WATCH_READABLE);
-    CHECK(success) << "Unable to allocate memory";
-  }
-
-  // Implement MessagePumpLibevent::Watcher.
-  void OnFileCanWriteWithoutBlocking(int file_descriptor) override {
-    const bool success = dbus_watch_handle(raw_watch_, DBUS_WATCH_WRITABLE);
-    CHECK(success) << "Unable to allocate memory";
+  void OnFileReady(unsigned int flags) {
+    CHECK(dbus_watch_handle(raw_watch_, flags)) << "Unable to allocate memory";
   }
 
   DBusWatch* raw_watch_;
-  base::MessagePumpLibevent::FileDescriptorWatcher file_descriptor_watcher_;
+  std::unique_ptr<base::FileDescriptorWatcher::Controller> read_watcher_;
+  std::unique_ptr<base::FileDescriptorWatcher::Controller> write_watcher_;
 };
 
 // The class is used for monitoring the timeout used for D-Bus method
 // calls.
 //
 // Unlike Watch, Timeout is a ref counted object, to ensure that |this| of
 // the object is is alive when HandleTimeout() is called. It's unlikely
 // but it may be possible that HandleTimeout() is called after
 // Bus::OnRemoveTimeout(). That's why we don't simply delete the object in
 // Bus::OnRemoveTimeout().
@@ skipping 935 matching lines @@
 }
 
 void Bus::OnToggleWatch(DBusWatch* raw_watch) {
   AssertOnDBusThread();
 
   Watch* watch = static_cast<Watch*>(dbus_watch_get_data(raw_watch));
   if (watch->IsReadyToBeWatched()) {
     watch->StartWatching();
   } else {
     // It's safe to call this if StartWatching() wasn't called, per
     // message_pump_libevent.h.

[satorux1, 2017/04/14 01:25:33]

update this comment?

[fdoray, 2017/04/18 12:43:16]

Done.

     watch->StopWatching();
   }
 }
 
 dbus_bool_t Bus::OnAddTimeout(DBusTimeout* raw_timeout) {
   AssertOnDBusThread();
 
   // timeout will be deleted when raw_timeout is removed in
   // OnRemoveTimeoutThunk().
   Timeout* timeout = new Timeout(raw_timeout);
@@ skipping 143 matching lines @@
                              kNameOwnerChangedSignal)) {
     Bus* self = static_cast<Bus*>(data);
     self->OnServiceOwnerChanged(message);
   }
   // Always return unhandled to let others, e.g. ObjectProxies, handle the same
   // signal.
   return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 }
 
 }  // namespace dbus