Revert "Attach UsbContext to libusb_device(_handle) pointers"

This reverts commit daa348fa.

Reason for revert: https://crbug.com/866782

Original change's description:
> Attach UsbContext to libusb_device(_handle) pointers
> 
> This change modifies ScopedLibusbDeviceRef and adds a new class
> ScopedLibusbDeviceHandle. This now explicitly own a reference to the
> UsbContext object (which reference counts a libusb_context) in addition
> to the libusb_device or libusb_device_handle they are wrapping.
> 
> This resolves potential use-after-frees possible when posting tasks
> with a ScopedLibusbDeviceRef since the UsbService the task is being
> posted to could be destroyed before the task is executed. The
> libusb_device would then be released after its libusb_context has been
> destroyed.
> 
> This is based on https://crrev.com/c/1131949 after I realized there
> where additional issues that needed to be addressed.
> 
> Bug: 838947
> Change-Id: Idee02828bf615bd477033e585fffe03cf4d20595
> Reviewed-on: https://chromium-review.googlesource.com/1145910
> Reviewed-by: Ken Rockot <rockot@chromium.org>
> Commit-Queue: Reilly Grant <reillyg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#577329}

TBR=rockot@chromium.org,reillyg@chromium.org

Change-Id: I4146987c84b34e1d3625e7e91cfc672afe98054c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 838947
Reviewed-on: https://chromium-review.googlesource.com/1147950Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577525}

device/usb/BUILD.gn

@@ -81,9 +81,6 @@ static_library("usb") {
 
   if (is_win || is_mac) {
     sources += [
-      "scoped_libusb_device_handle.cc",
-      "scoped_libusb_device_handle.h",
-      "scoped_libusb_device_ref.cc",
       "scoped_libusb_device_ref.h",
       "usb_context.cc",
       "usb_context.h",

device/usb/scoped_libusb_device_handle.cc

-// Copyright 2018 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 "device/usb/scoped_libusb_device_handle.h"
-
-#include "device/usb/usb_context.h"
-#include "third_party/libusb/src/libusb/libusb.h"
-
-namespace device {
-
-ScopedLibusbDeviceHandle::ScopedLibusbDeviceHandle(
-    libusb_device_handle* handle,
-    scoped_refptr<UsbContext> context)
-    : handle_(handle), context_(std::move(context)) {}
-
-ScopedLibusbDeviceHandle::ScopedLibusbDeviceHandle(
-    ScopedLibusbDeviceHandle&& other)
-    : handle_(other.handle_), context_(std::move(other.context_)) {
-  other.handle_ = nullptr;
-}
-
-ScopedLibusbDeviceHandle::~ScopedLibusbDeviceHandle() {
-  Reset();
-}
-
-void ScopedLibusbDeviceHandle::Reset() {
-  libusb_close(handle_);
-  handle_ = nullptr;
-  context_.reset();
-}
-
-bool ScopedLibusbDeviceHandle::IsValid() const {
-  return handle_ != nullptr;
-}
-
-}  // namespace device

device/usb/scoped_libusb_device_handle.h

-// Copyright 2018 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.
-
-#ifndef DEVICE_USB_SCOPED_LIBUSB_DEVICE_HANDLE_H_
-#define DEVICE_USB_SCOPED_LIBUSB_DEVICE_HANDLE_H_
-
-#include "base/macros.h"
-#include "base/memory/scoped_refptr.h"
-
-struct libusb_device_handle;
-
-namespace device {
-
-class UsbContext;
-
-// This class owns a reference to a libusb_device_handle as well as a reference
-// to the libusb_context. The libusb_context must outlive any
-// libusb_device_handle instances created from it.
-class ScopedLibusbDeviceHandle {
- public:
-  ScopedLibusbDeviceHandle(libusb_device_handle* handle,
-                           scoped_refptr<UsbContext> context);
-  ScopedLibusbDeviceHandle(ScopedLibusbDeviceHandle&& other);
-  ~ScopedLibusbDeviceHandle();
-
-  libusb_device_handle* get() const { return handle_; }
-
-  void Reset();
-  bool IsValid() const;
-
- private:
-  libusb_device_handle* handle_;
-  scoped_refptr<UsbContext> context_;
-
-  DISALLOW_COPY_AND_ASSIGN(ScopedLibusbDeviceHandle);
-};
-
-}  // namespace device
-
-#endif  // DEVICE_USB_SCOPED_LIBUSB_DEVICE_HANDLE_H_

device/usb/scoped_libusb_device_ref.cc

-// Copyright 2018 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 "device/usb/scoped_libusb_device_ref.h"
-
-#include "device/usb/usb_context.h"
-#include "third_party/libusb/src/libusb/libusb.h"
-
-namespace device {
-
-ScopedLibusbDeviceRef::ScopedLibusbDeviceRef(libusb_device* device,
-                                             scoped_refptr<UsbContext> context)
-    : device_(device), context_(std::move(context)) {}
-
-ScopedLibusbDeviceRef::ScopedLibusbDeviceRef(ScopedLibusbDeviceRef&& other)
-    : device_(other.device_), context_(std::move(other.context_)) {
-  other.device_ = nullptr;
-}
-
-ScopedLibusbDeviceRef::~ScopedLibusbDeviceRef() {
-  Reset();
-}
-
-void ScopedLibusbDeviceRef::Reset() {
-  libusb_unref_device(device_);
-  device_ = nullptr;
-  context_.reset();
-}
-
-bool ScopedLibusbDeviceRef::IsValid() const {
-  return device_ != nullptr;
-}
-
-bool operator==(const ScopedLibusbDeviceRef& ref, libusb_device* device) {
-  return ref.get() == device;
-}
-
-}  // namespace device

device/usb/scoped_libusb_device_ref.h

@@ -5,40 +5,19 @@
 #ifndef DEVICE_USB_SCOPED_LIBUSB_DEVICE_REF_H_
 #define DEVICE_USB_SCOPED_LIBUSB_DEVICE_REF_H_
 
-#include "base/macros.h"
-#include "base/memory/scoped_refptr.h"
-
-struct libusb_device;
+#include "base/scoped_generic.h"
+#include "third_party/libusb/src/libusb/libusb.h"
 
 namespace device {
 
-class UsbContext;
-
-// This class owns a reference to a libusb_device as well as a reference to
-// the libusb_context. The libusb_context must outlive any libusb_device
-// instances created from it.
-class ScopedLibusbDeviceRef {
- public:
-  ScopedLibusbDeviceRef(libusb_device* device,
-                        scoped_refptr<UsbContext> context);
-  ScopedLibusbDeviceRef(ScopedLibusbDeviceRef&& other);
-  ~ScopedLibusbDeviceRef();
-
-  libusb_device* get() const { return device_; }
-
-  scoped_refptr<UsbContext> GetContext() const { return context_; }
-
-  void Reset();
-  bool IsValid() const;
-
- private:
-  libusb_device* device_;
-  scoped_refptr<UsbContext> context_;
+struct LibusbDeviceRefTraits {
+  static libusb_device* InvalidValue() { return nullptr; }
 
-  DISALLOW_COPY_AND_ASSIGN(ScopedLibusbDeviceRef);
+  static void Free(libusb_device* device) { libusb_unref_device(device); }
 };
 
-bool operator==(const ScopedLibusbDeviceRef& ref, libusb_device* device);
+using ScopedLibusbDeviceRef =
+    base::ScopedGeneric<libusb_device*, LibusbDeviceRefTraits>;
 
 }  // namespace device
 

device/usb/usb_device_handle_impl.cc

@@ -311,7 +311,7 @@ UsbDeviceHandleImpl::Transfer::CreateControlTransfer(
   libusb_fill_control_setup(buffer->front(), type, request, value, index,
                             length);
   libusb_fill_control_transfer(transfer->platform_transfer_,
-                               device_handle->handle(), buffer->front(),
+                               device_handle->handle_, buffer->front(),
                                &UsbDeviceHandleImpl::Transfer::PlatformCallback,
                                transfer.get(), timeout);
 
@@ -341,7 +341,7 @@ UsbDeviceHandleImpl::Transfer::CreateBulkTransfer(
   }
 
   libusb_fill_bulk_transfer(
-      transfer->platform_transfer_, device_handle->handle(), endpoint,
+      transfer->platform_transfer_, device_handle->handle_, endpoint,
       buffer->front(), length, &UsbDeviceHandleImpl::Transfer::PlatformCallback,
       transfer.get(), timeout);
 
@@ -371,7 +371,7 @@ UsbDeviceHandleImpl::Transfer::CreateInterruptTransfer(
   }
 
   libusb_fill_interrupt_transfer(
-      transfer->platform_transfer_, device_handle->handle(), endpoint,
+      transfer->platform_transfer_, device_handle->handle_, endpoint,
       buffer->front(), length, &UsbDeviceHandleImpl::Transfer::PlatformCallback,
       transfer.get(), timeout);
 
@@ -401,10 +401,10 @@ UsbDeviceHandleImpl::Transfer::CreateIsochronousTransfer(
     return nullptr;
   }
 
-  libusb_fill_iso_transfer(
-      transfer->platform_transfer_, device_handle->handle(), endpoint,
-      buffer->front(), static_cast<int>(length), num_packets,
-      &Transfer::PlatformCallback, transfer.get(), timeout);
+  libusb_fill_iso_transfer(transfer->platform_transfer_, device_handle->handle_,
+                           endpoint, buffer->front(), static_cast<int>(length),
+                           num_packets, &Transfer::PlatformCallback,
+                           transfer.get(), timeout);
 
   for (size_t i = 0; i < packet_lengths.size(); ++i)
     transfer->platform_transfer_->iso_packet_desc[i].length = packet_lengths[i];
@@ -798,14 +798,16 @@ const UsbInterfaceDescriptor* UsbDeviceHandleImpl::FindInterfaceByEndpoint(
 }
 
 UsbDeviceHandleImpl::UsbDeviceHandleImpl(
+    scoped_refptr<UsbContext> context,
     scoped_refptr<UsbDeviceImpl> device,
-    ScopedLibusbDeviceHandle handle,
+    PlatformUsbDeviceHandle handle,
     scoped_refptr<base::SequencedTaskRunner> blocking_task_runner)
-    : device_(std::move(device)),
-      handle_(std::move(handle)),
+    : device_(device),
+      handle_(handle),
+      context_(context),
       task_runner_(base::ThreadTaskRunnerHandle::Get()),
       blocking_task_runner_(blocking_task_runner) {
-  DCHECK(handle_.IsValid()) << "Cannot create device with an invalid handle.";
+  DCHECK(handle) << "Cannot create device with NULL handle.";
 }
 
 UsbDeviceHandleImpl::~UsbDeviceHandleImpl() {
@@ -815,19 +817,17 @@ UsbDeviceHandleImpl::~UsbDeviceHandleImpl() {
   // any thread. libusb is not safe to reentrancy so be sure not to try to close
   // the device from inside a transfer completion callback.
   if (blocking_task_runner_->RunsTasksInCurrentSequence()) {
-    handle_.Reset();
+    libusb_close(handle_);
   } else {
-    blocking_task_runner_->PostTask(
-        FROM_HERE,
-        base::BindOnce(base::DoNothing::Once<ScopedLibusbDeviceHandle>(),
-                       std::move(handle_)));
+    blocking_task_runner_->PostTask(FROM_HERE,
+                                    base::BindOnce(&libusb_close, handle_));
   }
 }
 
 void UsbDeviceHandleImpl::SetConfigurationOnBlockingThread(
     int configuration_value,
     ResultCallback callback) {
-  int rv = libusb_set_configuration(handle(), configuration_value);
+  int rv = libusb_set_configuration(handle_, configuration_value);
   if (rv != LIBUSB_SUCCESS) {
     USB_LOG(EVENT) << "Failed to set configuration " << configuration_value
                    << ": " << ConvertPlatformUsbErrorToString(rv);
@@ -855,7 +855,7 @@ void UsbDeviceHandleImpl::SetConfigurationComplete(bool success,
 void UsbDeviceHandleImpl::ClaimInterfaceOnBlockingThread(
     int interface_number,
     ResultCallback callback) {
-  int rv = libusb_claim_interface(handle(), interface_number);
+  int rv = libusb_claim_interface(handle_, interface_number);
   scoped_refptr<InterfaceClaimer> interface_claimer;
   if (rv == LIBUSB_SUCCESS) {
     interface_claimer =
@@ -897,7 +897,7 @@ void UsbDeviceHandleImpl::SetInterfaceAlternateSettingOnBlockingThread(
     int interface_number,
     int alternate_setting,
     ResultCallback callback) {
-  int rv = libusb_set_interface_alt_setting(handle(), interface_number,
+  int rv = libusb_set_interface_alt_setting(handle_, interface_number,
                                             alternate_setting);
   if (rv != LIBUSB_SUCCESS) {
     USB_LOG(EVENT) << "Failed to set interface " << interface_number
@@ -930,7 +930,7 @@ void UsbDeviceHandleImpl::SetInterfaceAlternateSettingComplete(
 }
 
 void UsbDeviceHandleImpl::ResetDeviceOnBlockingThread(ResultCallback callback) {
-  int rv = libusb_reset_device(handle());
+  int rv = libusb_reset_device(handle_);
   if (rv != LIBUSB_SUCCESS) {
     USB_LOG(EVENT) << "Failed to reset device: "
                    << ConvertPlatformUsbErrorToString(rv);
@@ -941,7 +941,7 @@ void UsbDeviceHandleImpl::ResetDeviceOnBlockingThread(ResultCallback callback) {
 
 void UsbDeviceHandleImpl::ClearHaltOnBlockingThread(uint8_t endpoint,
                                                     ResultCallback callback) {
-  int rv = libusb_clear_halt(handle(), endpoint);
+  int rv = libusb_clear_halt(handle_, endpoint);
   if (rv != LIBUSB_SUCCESS) {
     USB_LOG(EVENT) << "Failed to clear halt: "
                    << ConvertPlatformUsbErrorToString(rv);

device/usb/usb_device_handle_impl.h

@@ -17,7 +17,6 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/threading/thread_checker.h"
-#include "device/usb/scoped_libusb_device_handle.h"
 #include "device/usb/usb_device_handle.h"
 #include "third_party/libusb/src/libusb/libusb.h"
 
@@ -35,8 +34,10 @@ struct EndpointMapValue {
   const UsbEndpointDescriptor* endpoint;
 };
 
+class UsbContext;
 class UsbDeviceImpl;
 
+typedef libusb_device_handle* PlatformUsbDeviceHandle;
 typedef libusb_iso_packet_descriptor* PlatformUsbIsoPacketDescriptor;
 typedef libusb_transfer* PlatformUsbTransferHandle;
 
@@ -89,13 +90,14 @@ class UsbDeviceHandleImpl : public UsbDeviceHandle {
 
   // This constructor is called by UsbDeviceImpl.
   UsbDeviceHandleImpl(
+      scoped_refptr<UsbContext> context,
       scoped_refptr<UsbDeviceImpl> device,
-      ScopedLibusbDeviceHandle handle,
+      PlatformUsbDeviceHandle handle,
       scoped_refptr<base::SequencedTaskRunner> blocking_task_runner);
 
   ~UsbDeviceHandleImpl() override;
 
-  libusb_device_handle* handle() const { return handle_.get(); }
+  PlatformUsbDeviceHandle handle() const { return handle_; }
 
  private:
   class InterfaceClaimer;
@@ -172,7 +174,7 @@ class UsbDeviceHandleImpl : public UsbDeviceHandle {
 
   scoped_refptr<UsbDeviceImpl> device_;
 
-  ScopedLibusbDeviceHandle handle_;
+  PlatformUsbDeviceHandle handle_;
 
   typedef std::map<int, scoped_refptr<InterfaceClaimer>> ClaimedInterfaceMap;
   ClaimedInterfaceMap claimed_interfaces_;
@@ -184,6 +186,10 @@ class UsbDeviceHandleImpl : public UsbDeviceHandle {
   typedef std::map<int, EndpointMapValue> EndpointMap;
   EndpointMap endpoint_map_;
 
+  // Retain the UsbContext so that the platform context will not be destroyed
+  // before this handle.
+  scoped_refptr<UsbContext> context_;
+
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
   scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_;
 

device/usb/usb_device_impl.cc

@@ -28,7 +28,8 @@
 
 namespace device {
 
-UsbDeviceImpl::UsbDeviceImpl(ScopedLibusbDeviceRef platform_device,
+UsbDeviceImpl::UsbDeviceImpl(scoped_refptr<UsbContext> context,
+                             ScopedLibusbDeviceRef platform_device,
                              const libusb_device_descriptor& descriptor)
     : UsbDevice(descriptor.bcdUSB,
                 descriptor.bDeviceClass,
@@ -40,8 +41,9 @@ UsbDeviceImpl::UsbDeviceImpl(ScopedLibusbDeviceRef platform_device,
                 base::string16(),
                 base::string16(),
                 base::string16()),
+      context_(std::move(context)),
       platform_device_(std::move(platform_device)) {
-  CHECK(platform_device_.IsValid()) << "platform_device must be valid";
+  CHECK(platform_device_.is_valid()) << "platform_device must be valid";
   ReadAllConfigurations();
   RefreshActiveConfiguration();
 }
@@ -102,14 +104,12 @@ void UsbDeviceImpl::OpenOnBlockingThread(
     scoped_refptr<base::TaskRunner> task_runner,
     scoped_refptr<base::SequencedTaskRunner> blocking_task_runner) {
   base::AssertBlockingAllowed();
-  libusb_device_handle* handle;
+  PlatformUsbDeviceHandle handle;
   const int rv = libusb_open(platform_device(), &handle);
-  ScopedLibusbDeviceHandle scoped_handle(handle, platform_device_.GetContext());
   if (LIBUSB_SUCCESS == rv) {
     task_runner->PostTask(
-        FROM_HERE,
-        base::BindOnce(&UsbDeviceImpl::Opened, this, std::move(scoped_handle),
-                       std::move(callback), blocking_task_runner));
+        FROM_HERE, base::BindOnce(&UsbDeviceImpl::Opened, this, handle,
+                                  std::move(callback), blocking_task_runner));
   } else {
     USB_LOG(EVENT) << "Failed to open device: "
                    << ConvertPlatformUsbErrorToString(rv);
@@ -119,12 +119,12 @@ void UsbDeviceImpl::OpenOnBlockingThread(
 }
 
 void UsbDeviceImpl::Opened(
-    ScopedLibusbDeviceHandle platform_handle,
+    PlatformUsbDeviceHandle platform_handle,
     OpenCallback callback,
     scoped_refptr<base::SequencedTaskRunner> blocking_task_runner) {
   DCHECK(thread_checker_.CalledOnValidThread());
   scoped_refptr<UsbDeviceHandle> device_handle = new UsbDeviceHandleImpl(
-      this, std::move(platform_handle), blocking_task_runner);
+      context_, this, platform_handle, blocking_task_runner);
   handles().push_back(device_handle.get());
   std::move(callback).Run(device_handle);
 }

device/usb/usb_device_impl.h

@@ -21,7 +21,9 @@
 #include "device/usb/usb_descriptors.h"
 #include "device/usb/usb_device.h"
 
+struct libusb_device;
 struct libusb_device_descriptor;
+struct libusb_device_handle;
 
 namespace base {
 class SequencedTaskRunner;
@@ -29,12 +31,15 @@ class SequencedTaskRunner;
 
 namespace device {
 
-class ScopedLibusbDeviceHandle;
 class UsbDeviceHandleImpl;
+class UsbContext;
+
+typedef struct libusb_device_handle* PlatformUsbDeviceHandle;
 
 class UsbDeviceImpl : public UsbDevice {
  public:
-  UsbDeviceImpl(ScopedLibusbDeviceRef platform_device,
+  UsbDeviceImpl(scoped_refptr<UsbContext> context,
+                ScopedLibusbDeviceRef platform_device,
                 const libusb_device_descriptor& descriptor);
 
   // UsbDevice implementation:
@@ -74,13 +79,15 @@ class UsbDeviceImpl : public UsbDevice {
       OpenCallback callback,
       scoped_refptr<base::TaskRunner> task_runner,
       scoped_refptr<base::SequencedTaskRunner> blocking_task_runner);
-  void Opened(ScopedLibusbDeviceHandle platform_handle,
+  void Opened(PlatformUsbDeviceHandle platform_handle,
               OpenCallback callback,
               scoped_refptr<base::SequencedTaskRunner> blocking_task_runner);
 
   base::ThreadChecker thread_checker_;
   bool visited_ = false;
 
+  // The libusb_context must not be released before the libusb_device.
+  const scoped_refptr<UsbContext> context_;
   const ScopedLibusbDeviceRef platform_device_;
 
   DISALLOW_COPY_AND_ASSIGN(UsbDeviceImpl);

device/usb/usb_service_impl.cc

@@ -131,7 +131,7 @@ void GetDeviceListOnBlockingThread(
   std::vector<ScopedLibusbDeviceRef> scoped_devices;
   scoped_devices.reserve(device_count);
   for (ssize_t i = 0; i < device_count; ++i)
-    scoped_devices.emplace_back(platform_devices[i], usb_context);
+    scoped_devices.emplace_back(platform_devices[i]);
 
   // Free the list but don't unref the devices because ownership has been
   // been transfered to the elements of |scoped_devices|.
@@ -231,7 +231,6 @@ UsbServiceImpl::UsbServiceImpl()
       device_observer_(this),
 #endif
       weak_factory_(this) {
-  weak_self_ = weak_factory_.GetWeakPtr();
   base::PostTaskWithTraits(
       FROM_HERE, kBlockingTaskTraits,
       base::Bind(&InitializeUsbContextOnBlockingThread, task_runner(),
@@ -362,7 +361,7 @@ void UsbServiceImpl::OnDeviceList(
       // Mark the existing device object visited and remove it from the list so
       // it will not be ignored.
       it->second->set_visited(true);
-      device.Reset();
+      device.reset();
     }
   }
 
@@ -383,7 +382,7 @@ void UsbServiceImpl::OnDeviceList(
   // that have been removed don't remain in |ignored_devices_| indefinitely.
   ignored_devices_.clear();
   for (auto& device : *devices) {
-    if (device.IsValid())
+    if (device.is_valid())
       ignored_devices_.push_back(std::move(device));
   }
 
@@ -442,8 +441,8 @@ void UsbServiceImpl::EnumerateDevice(ScopedLibusbDeviceRef platform_device,
 
   devices_being_enumerated_.insert(platform_device.get());
 
-  auto device = base::MakeRefCounted<UsbDeviceImpl>(std::move(platform_device),
-                                                    descriptor);
+  auto device = base::MakeRefCounted<UsbDeviceImpl>(
+      context_, std::move(platform_device), descriptor);
   base::OnceClosure add_device =
       base::BindOnce(&UsbServiceImpl::AddDevice, weak_factory_.GetWeakPtr(),
                      refresh_complete, device);
@@ -459,8 +458,7 @@ void UsbServiceImpl::EnumerateDevice(ScopedLibusbDeviceRef platform_device,
     libusb_ref_device(device->platform_device());
     base::OnceClosure enumeration_failed = base::BindOnce(
         &UsbServiceImpl::EnumerationFailed, weak_factory_.GetWeakPtr(),
-        ScopedLibusbDeviceRef(device->platform_device(), context_),
-        refresh_complete);
+        ScopedLibusbDeviceRef(device->platform_device()), refresh_complete);
 
     device->Open(base::BindOnce(
         &OnDeviceOpenedReadDescriptors, descriptor.iManufacturer,
@@ -520,18 +518,18 @@ int LIBUSB_CALL UsbServiceImpl::HotplugCallback(libusb_context* context,
   // libusb does not transfer ownership of |device_raw| to this function so a
   // reference must be taken here.
   libusb_ref_device(device_raw);
-  ScopedLibusbDeviceRef device(device_raw, self->context_);
+  ScopedLibusbDeviceRef device(device_raw);
 
   switch (event) {
     case LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED:
       self->task_runner()->PostTask(
           FROM_HERE, base::BindOnce(&UsbServiceImpl::OnPlatformDeviceAdded,
-                                    self->weak_self_, std::move(device)));
+                                    base::Unretained(self), std::move(device)));
       break;
     case LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT:
       self->task_runner()->PostTask(
           FROM_HERE, base::BindOnce(&UsbServiceImpl::OnPlatformDeviceRemoved,
-                                    self->weak_self_, std::move(device)));
+                                    base::Unretained(self), std::move(device)));
       break;
     default:
       NOTREACHED();

device/usb/usb_service_impl.h

@@ -122,10 +122,6 @@ class UsbServiceImpl :
   ScopedObserver<DeviceMonitorWin, DeviceMonitorWin::Observer> device_observer_;
 #endif  // OS_WIN
 
-  // This WeakPtr is used to safely post hotplug events back to the thread this
-  // object lives on.
-  base::WeakPtr<UsbServiceImpl> weak_self_;
-
   base::WeakPtrFactory<UsbServiceImpl> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(UsbServiceImpl);