Remove MessageLoop destruction observer from hid_connection_linux.cc

device/hid/hid_connection_linux.cc

Index: device/hid/hid_connection_linux.cc
diff --git a/device/hid/hid_connection_linux.cc b/device/hid/hid_connection_linux.cc
index 5172b0b39ea6a366c22af751e3d48ebf5b933734..ec348ed6abc4cc5a429f74bd65c012c0ba0b096e 100644
--- a/device/hid/hid_connection_linux.cc
+++ b/device/hid/hid_connection_linux.cc
@@ -14,9 +14,8 @@
 
 #include "base/bind.h"
 #include "base/files/file_descriptor_watcher_posix.h"
-#include "base/files/file_path.h"
 #include "base/macros.h"
-#include "base/message_loop/message_loop.h"
+#include "base/memory/ptr_util.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ -33,44 +32,91 @@
 
 namespace device {
 
-class HidConnectionLinux::FileThreadHelper
-    : public base::MessageLoop::DestructionObserver {
+class HidConnectionLinux::FileThreadHelper {
  public:
-  FileThreadHelper(base::PlatformFile platform_file,
+  FileThreadHelper(base::ScopedFD fd,
                    scoped_refptr<HidDeviceInfo> device_info,
-                   base::WeakPtr<HidConnectionLinux> connection,
-                   scoped_refptr<base::SingleThreadTaskRunner> task_runner)
-      : platform_file_(platform_file),
+                   base::WeakPtr<HidConnectionLinux> connection)
+      : fd_(std::move(fd)),
         connection_(connection),
-        task_runner_(task_runner) {
+        origin_task_runner_(base::ThreadTaskRunnerHandle::Get()) {
+    sequence_checker_.DetachFromSequence();
     // Report buffers must always have room for the report ID.
     report_buffer_size_ = device_info->max_input_report_size() + 1;
     has_report_id_ = device_info->has_report_id();
   }
 
-  ~FileThreadHelper() override {
-    DCHECK(thread_checker_.CalledOnValidThread());
-    base::MessageLoop::current()->RemoveDestructionObserver(this);
-  }
+  ~FileThreadHelper() { DCHECK(sequence_checker_.CalledOnValidSequence()); }
 
   // Starts the FileDescriptorWatcher that reads input events from the device.
   // Must be called on a thread that has a base::MessageLoopForIO.
-  static void Start(std::unique_ptr<FileThreadHelper> self) {
+  void Start() {
+    DCHECK(sequence_checker_.CalledOnValidSequence());
     base::ThreadRestrictions::AssertIOAllowed();
-    self->thread_checker_.DetachFromThread();
 
-    self->file_watcher_ = base::FileDescriptorWatcher::WatchReadable(
-        self->platform_file_,
-        base::Bind(&FileThreadHelper::OnFileCanReadWithoutBlocking,
-                   base::Unretained(self.get())));
+    file_watcher_ = base::FileDescriptorWatcher::WatchReadable(
+        fd_.get(), base::Bind(&FileThreadHelper::OnFileCanReadWithoutBlocking,
+                              base::Unretained(this)));
+  }
+
+  void Write(scoped_refptr<net::IOBuffer> buffer,
+             size_t size,
+             const WriteCallback& callback) {
+    DCHECK(sequence_checker_.CalledOnValidSequence());
+    ssize_t result = HANDLE_EINTR(write(fd_.get(), buffer->data(), size));
+    if (result < 0) {
+      HID_PLOG(EVENT) << "Write failed";
+      origin_task_runner_->PostTask(FROM_HERE, base::Bind(callback, false));
+    } else {
+      if (static_cast<size_t>(result) != size)
+        HID_LOG(EVENT) << "Incomplete HID write: " << result << " != " << size;
+      origin_task_runner_->PostTask(FROM_HERE, base::Bind(callback, true));
+    }
+  }
 
-    // |self| is now owned by the current message loop.
-    base::MessageLoop::current()->AddDestructionObserver(self.release());
+  void GetFeatureReport(uint8_t report_id,
+                        scoped_refptr<net::IOBufferWithSize> buffer,
+                        const ReadCallback& callback) {
+    DCHECK(sequence_checker_.CalledOnValidSequence());
+    int result = HANDLE_EINTR(
+        ioctl(fd_.get(), HIDIOCGFEATURE(buffer->size()), buffer->data()));
+    if (result < 0) {
+      HID_PLOG(EVENT) << "Failed to get feature report";
+      origin_task_runner_->PostTask(FROM_HERE,
+                                    base::Bind(callback, false, nullptr, 0));
+    } else if (result == 0) {
+      HID_LOG(EVENT) << "Get feature result too short.";
+      origin_task_runner_->PostTask(FROM_HERE,
+                                    base::Bind(callback, false, nullptr, 0));
+    } else if (report_id == 0) {
+      // Linux adds a 0 to the beginning of the data received from the device.
+      scoped_refptr<net::IOBuffer> copied_buffer(new net::IOBuffer(result - 1));
+      memcpy(copied_buffer->data(), buffer->data() + 1, result - 1);
+      origin_task_runner_->PostTask(
+          FROM_HERE, base::Bind(callback, true, copied_buffer, result - 1));
+    } else {
+      origin_task_runner_->PostTask(FROM_HERE,
+                                    base::Bind(callback, true, buffer, result));
+    }
+  }
+
+  void SendFeatureReport(scoped_refptr<net::IOBuffer> buffer,
+                         size_t size,
+                         const WriteCallback& callback) {
+    DCHECK(sequence_checker_.CalledOnValidSequence());
+    int result =
+        HANDLE_EINTR(ioctl(fd_.get(), HIDIOCSFEATURE(size), buffer->data()));
+    if (result < 0) {
+      HID_PLOG(EVENT) << "Failed to send feature report";
+      origin_task_runner_->PostTask(FROM_HERE, base::Bind(callback, false));
+    } else {
+      origin_task_runner_->PostTask(FROM_HERE, base::Bind(callback, true));
+    }
   }
 
  private:
   void OnFileCanReadWithoutBlocking() {
-    DCHECK(thread_checker_.CalledOnValidThread());
+    DCHECK(sequence_checker_.CalledOnValidSequence());
 
     scoped_refptr<net::IOBuffer> buffer(new net::IOBuffer(report_buffer_size_));
     char* data = buffer->data();
@@ -82,7 +128,7 @@ class HidConnectionLinux::FileThreadHelper
       length--;
     }
 
-    ssize_t bytes_read = HANDLE_EINTR(read(platform_file_, data, length));
+    ssize_t bytes_read = HANDLE_EINTR(read(fd_.get(), data, length));
     if (bytes_read < 0) {
       if (errno != EAGAIN) {
         HID_PLOG(EVENT) << "Read failed";
@@ -100,23 +146,17 @@ class HidConnectionLinux::FileThreadHelper
       bytes_read++;
     }
 
-    task_runner_->PostTask(FROM_HERE,
-                           base::Bind(&HidConnectionLinux::ProcessInputReport,
-                                      connection_, buffer, bytes_read));
+    origin_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&HidConnectionLinux::ProcessInputReport,
+                              connection_, buffer, bytes_read));
   }
 
-  // base::MessageLoop::DestructionObserver:
-  void WillDestroyCurrentMessageLoop() override {
-    DCHECK(thread_checker_.CalledOnValidThread());
-    delete this;
-  }
-
-  base::ThreadChecker thread_checker_;
-  base::PlatformFile platform_file_;
+  base::SequenceChecker sequence_checker_;
+  base::ScopedFD fd_;
   size_t report_buffer_size_;
   bool has_report_id_;
   base::WeakPtr<HidConnectionLinux> connection_;
-  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+  const scoped_refptr<base::SequencedTaskRunner> origin_task_runner_;
   std::unique_ptr<base::FileDescriptorWatcher::Controller> file_watcher_;
 
   DISALLOW_COPY_AND_ASSIGN(FileThreadHelper);
@@ -124,38 +164,28 @@ class HidConnectionLinux::FileThreadHelper
 
 HidConnectionLinux::HidConnectionLinux(
     scoped_refptr<HidDeviceInfo> device_info,
-    base::File device_file,
-    scoped_refptr<base::SingleThreadTaskRunner> file_task_runner)
+    base::ScopedFD fd,
+    scoped_refptr<base::SequencedTaskRunner> blocking_task_runner)
     : HidConnection(device_info),
-      file_task_runner_(file_task_runner),
+      blocking_task_runner_(std::move(blocking_task_runner)),
       weak_factory_(this) {
-  task_runner_ = base::ThreadTaskRunnerHandle::Get();
-  device_file_ = std::move(device_file);
-
-  // The helper is passed a weak pointer to this connection so that it can be
-  // cleaned up after the connection is closed.
-  std::unique_ptr<FileThreadHelper> helper(
-      new FileThreadHelper(device_file_.GetPlatformFile(), device_info,
-                           weak_factory_.GetWeakPtr(), task_runner_));
-  helper_ = helper.get();
-  file_task_runner_->PostTask(
-      FROM_HERE, base::Bind(&FileThreadHelper::Start, base::Passed(&helper)));
+  helper_ = base::MakeUnique<FileThreadHelper>(std::move(fd), device_info,
+                                               weak_factory_.GetWeakPtr());
+  blocking_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&FileThreadHelper::Start, base::Unretained(helper_.get())));
 }
 
 HidConnectionLinux::~HidConnectionLinux() {
-  DCHECK(helper_ == nullptr);
+  DCHECK(sequence_checker_.CalledOnValidSequence());
 }
 
 void HidConnectionLinux::PlatformClose() {
-  // By closing the device file on the FILE thread (1) the requirement that
-  // base::File::Close is called on a thread where I/O is allowed is satisfied
-  // and (2) any tasks posted to this task runner that refer to this file will
+  // By closing the device on the blocking task runner 1) the requirement that
+  // base::ScopedFD is destroyed on a thread where I/O is allowed is satisfied
+  // and 2) any tasks posted to this task runner that refer to this file will
   // complete before it is closed.
-  file_task_runner_->DeleteSoon(FROM_HERE, helper_);
-  helper_ = nullptr;
-  file_task_runner_->PostTask(FROM_HERE,
-                              base::Bind(&HidConnectionLinux::CloseDevice,
-                                         base::Passed(&device_file_)));
+  blocking_task_runner_->DeleteSoon(FROM_HERE, helper_.release());
 
   while (!pending_reads_.empty()) {
     pending_reads_.front().callback.Run(false, NULL, 0);
@@ -175,13 +205,10 @@ void HidConnectionLinux::PlatformWrite(scoped_refptr<net::IOBuffer> buffer,
                                        const WriteCallback& callback) {
   // Linux expects the first byte of the buffer to always be a report ID so the
   // buffer can be used directly.
-  file_task_runner_->PostTask(
+  blocking_task_runner_->PostTask(
       FROM_HERE,
-      base::Bind(&HidConnectionLinux::BlockingWrite,
-                 device_file_.GetPlatformFile(), buffer, size,
-                 base::Bind(&HidConnectionLinux::FinishWrite,
-                            weak_factory_.GetWeakPtr(), size, callback),
-                 task_runner_));
+      base::Bind(&FileThreadHelper::Write, base::Unretained(helper_.get()),
+                 buffer, size, callback));
 }
 
 void HidConnectionLinux::PlatformGetFeatureReport(
@@ -194,14 +221,10 @@ void HidConnectionLinux::PlatformGetFeatureReport(
       new net::IOBufferWithSize(device_info()->max_feature_report_size() + 1));
   buffer->data()[0] = report_id;
 
-  file_task_runner_->PostTask(
+  blocking_task_runner_->PostTask(
       FROM_HERE,
-      base::Bind(
-          &HidConnectionLinux::BlockingIoctl, device_file_.GetPlatformFile(),
-          HIDIOCGFEATURE(buffer->size()), buffer,
-          base::Bind(&HidConnectionLinux::FinishGetFeatureReport,
-                     weak_factory_.GetWeakPtr(), report_id, buffer, callback),
-          task_runner_));
+      base::Bind(&FileThreadHelper::GetFeatureReport,
+                 base::Unretained(helper_.get()), report_id, buffer, callback));
 }
 
 void HidConnectionLinux::PlatformSendFeatureReport(
@@ -210,88 +233,10 @@ void HidConnectionLinux::PlatformSendFeatureReport(
     const WriteCallback& callback) {
   // Linux expects the first byte of the buffer to always be a report ID so the
   // buffer can be used directly.
-  file_task_runner_->PostTask(
+  blocking_task_runner_->PostTask(
       FROM_HERE,
-      base::Bind(&HidConnectionLinux::BlockingIoctl,
-                 device_file_.GetPlatformFile(), HIDIOCSFEATURE(size), buffer,
-                 base::Bind(&HidConnectionLinux::FinishSendFeatureReport,
-                            weak_factory_.GetWeakPtr(), callback),
-                 task_runner_));
-}
-
-void HidConnectionLinux::FinishWrite(size_t expected_size,
-                                     const WriteCallback& callback,
-                                     ssize_t result) {
-  if (result < 0) {
-    HID_PLOG(EVENT) << "Write failed";
-    callback.Run(false);
-  } else {
-    if (static_cast<size_t>(result) != expected_size) {
-      HID_LOG(EVENT) << "Incomplete HID write: " << result
-                     << " != " << expected_size;
-    }
-    callback.Run(true);
-  }
-}
-
-void HidConnectionLinux::FinishGetFeatureReport(
-    uint8_t report_id,
-    scoped_refptr<net::IOBuffer> buffer,
-    const ReadCallback& callback,
-    int result) {
-  if (result < 0) {
-    HID_PLOG(EVENT) << "Failed to get feature report";
-    callback.Run(false, NULL, 0);
-  } else if (result == 0) {
-    HID_LOG(EVENT) << "Get feature result too short.";
-    callback.Run(false, NULL, 0);
-  } else if (report_id == 0) {
-    // Linux adds a 0 to the beginning of the data received from the device.
-    scoped_refptr<net::IOBuffer> copied_buffer(new net::IOBuffer(result - 1));
-    memcpy(copied_buffer->data(), buffer->data() + 1, result - 1);
-    callback.Run(true, copied_buffer, result - 1);
-  } else {
-    callback.Run(true, buffer, result);
-  }
-}
-
-void HidConnectionLinux::FinishSendFeatureReport(const WriteCallback& callback,
-                                                 int result) {
-  if (result < 0) {
-    HID_PLOG(EVENT) << "Failed to send feature report";
-    callback.Run(false);
-  } else {
-    callback.Run(true);
-  }
-}
-
-// static
-void HidConnectionLinux::BlockingWrite(
-    base::PlatformFile platform_file,
-    scoped_refptr<net::IOBuffer> buffer,
-    size_t size,
-    const InternalWriteCallback& callback,
-    scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
-  base::ThreadRestrictions::AssertIOAllowed();
-  ssize_t result = HANDLE_EINTR(write(platform_file, buffer->data(), size));
-  task_runner->PostTask(FROM_HERE, base::Bind(callback, result));
-}
-
-// static
-void HidConnectionLinux::BlockingIoctl(
-    base::PlatformFile platform_file,
-    int request,
-    scoped_refptr<net::IOBuffer> buffer,
-    const IoctlCallback& callback,
-    scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
-  base::ThreadRestrictions::AssertIOAllowed();
-  int result = ioctl(platform_file, request, buffer->data());
-  task_runner->PostTask(FROM_HERE, base::Bind(callback, result));
-}
-
-// static
-void HidConnectionLinux::CloseDevice(base::File device_file) {
-  device_file.Close();
+      base::Bind(&FileThreadHelper::SendFeatureReport,
+                 base::Unretained(helper_.get()), buffer, size, callback));
 }
 
 void HidConnectionLinux::ProcessInputReport(scoped_refptr<net::IOBuffer> buffer,
@@ -311,9 +256,8 @@ void HidConnectionLinux::ProcessReadQueue() {
     PendingHidReport report = pending_reports_.front();
 
     pending_reports_.pop();
-    if (CompleteRead(report.buffer, report.size, read.callback)) {
+    if (CompleteRead(report.buffer, report.size, read.callback))
       pending_reads_.pop();
-    }
   }
 }