Add GetPort(...) for device::mojom::SerialPortManager.

A new method GetPort(...) for SerialPortManager Mojo interface is
added. The argument list for SerialPort::Open(...) method has been
changed and the related code has been adjusted accordingly.

BUG=908833

Change-Id: Iba156b1abb0e636ee623cd3de6eae6d0773722ff
Reviewed-on: https://chromium-review.googlesource.com/c/1369711Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Donna Wu <donna.wu@intel.com>
Cr-Commit-Position: refs/heads/master@{#615905}

extensions/browser/api/serial/serial_api.cc

@@ -87,11 +87,11 @@ ExtensionFunction::ResponseAction SerialGetDevicesFunction::Run() {
   content::ServiceManagerConnection::GetForProcess()
       ->GetConnector()
       ->BindInterface(device::mojom::kServiceName,
-                      mojo::MakeRequest(&enumerator_));
-  enumerator_.set_connection_error_handler(
+                      mojo::MakeRequest(&port_manager_));
+  port_manager_.set_connection_error_handler(
       base::BindOnce(&SerialGetDevicesFunction::OnGotDevices, this,
                      std::vector<device::mojom::SerialPortInfoPtr>()));
-  enumerator_->GetDevices(
+  port_manager_->GetDevices(
       base::BindOnce(&SerialGetDevicesFunction::OnGotDevices, this));
   return RespondLater();
 }
@@ -102,7 +102,7 @@ void SerialGetDevicesFunction::OnGotDevices(
       serial::GetDevices::Results::Create(
           mojo::ConvertTo<std::vector<serial::DeviceInfo>>(devices));
   Respond(ArgumentList(std::move(results)));
-  enumerator_.reset();
+  port_manager_.reset();
 }
 
 SerialConnectFunction::SerialConnectFunction() {}
@@ -134,10 +134,12 @@ bool SerialConnectFunction::Prepare() {
 
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(content::ServiceManagerConnection::GetForProcess());
+  device::mojom::SerialPortManagerPtr port_manager;
   content::ServiceManagerConnection::GetForProcess()
       ->GetConnector()
       ->BindInterface(device::mojom::kServiceName,
-                      mojo::MakeRequest(&io_handler_info_));
+                      mojo::MakeRequest(&port_manager));
+  port_manager->GetPort(params_->path, mojo::MakeRequest(&serial_port_info_));
 
   serial_event_dispatcher_ = SerialEventDispatcher::Get(browser_context());
   DCHECK(serial_event_dispatcher_);
@@ -148,7 +150,7 @@ bool SerialConnectFunction::Prepare() {
 void SerialConnectFunction::AsyncWorkStart() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   connection_ = std::make_unique<SerialConnection>(
-      params_->path, extension_->id(), std::move(io_handler_info_));
+      extension_->id(), std::move(serial_port_info_));
   connection_->Open(*params_->options,
                     base::BindOnce(&SerialConnectFunction::OnConnected, this));
 }

extensions/browser/api/serial/serial_api.h

@@ -52,7 +52,7 @@ class SerialGetDevicesFunction : public UIThreadExtensionFunction {
  private:
   void OnGotDevices(std::vector<device::mojom::SerialPortInfoPtr> devices);
 
-  device::mojom::SerialPortManagerPtr enumerator_;
+  device::mojom::SerialPortManagerPtr port_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(SerialGetDevicesFunction);
 };
@@ -86,7 +86,7 @@ class SerialConnectFunction : public SerialAsyncApiFunction {
   // ApiResourceManager<SerialConnection> upon success.
   std::unique_ptr<SerialConnection> connection_;
 
-  device::mojom::SerialPortPtrInfo io_handler_info_;
+  device::mojom::SerialPortPtrInfo serial_port_info_;
 };
 
 class SerialUpdateFunction : public SerialAsyncApiFunction {

extensions/browser/api/serial/serial_apitest.cc

@@ -50,30 +50,9 @@ using testing::Return;
 namespace extensions {
 namespace {
 
-class FakeSerialPortManager : public device::mojom::SerialPortManager {
- public:
-  FakeSerialPortManager() = default;
-  ~FakeSerialPortManager() override = default;
-
- private:
-  // device::mojom::SerialPortManager methods:
-  void GetDevices(GetDevicesCallback callback) override {
-    std::vector<device::mojom::SerialPortInfoPtr> devices;
-    auto device0 = device::mojom::SerialPortInfo::New();
-    device0->path = "/dev/fakeserialmojo";
-    auto device1 = device::mojom::SerialPortInfo::New();
-    device1->path = "\\\\COM800\\";
-    devices.push_back(std::move(device0));
-    devices.push_back(std::move(device1));
-    std::move(callback).Run(std::move(devices));
-  }
-
-  DISALLOW_COPY_AND_ASSIGN(FakeSerialPortManager);
-};
-
 class FakeSerialPort : public device::mojom::SerialPort {
  public:
-  FakeSerialPort() {
+  FakeSerialPort(const std::string& path) : path_(path) {
     options_.bitrate = 9600;
     options_.data_bits = device::mojom::SerialDataBits::EIGHT;
     options_.parity_bit = device::mojom::SerialParityBit::NO_PARITY;
@@ -85,8 +64,7 @@ class FakeSerialPort : public device::mojom::SerialPort {
 
  private:
   // device::mojom::SerialPort methods:
-  void Open(const std::string& port,
-            device::mojom::SerialConnectionOptionsPtr options,
+  void Open(device::mojom::SerialConnectionOptionsPtr options,
             OpenCallback callback) override {
     DoConfigurePort(*options);
     std::move(callback).Run(true);
@@ -111,8 +89,7 @@ class FakeSerialPort : public device::mojom::SerialPort {
       std::move(pending_read_callback_).Run(std::vector<uint8_t>(), reason);
     }
   }
-  void CancelWrite(device::mojom::SerialSendError reason) override {
-  }
+  void CancelWrite(device::mojom::SerialSendError reason) override {}
   void Flush(FlushCallback callback) override { std::move(callback).Run(true); }
   void GetControlSignals(GetControlSignalsCallback callback) override {
     auto signals = device::mojom::SerialPortControlSignals::New();
@@ -182,6 +159,7 @@ class FakeSerialPort : public device::mojom::SerialPort {
 
   // Currently applied connection options.
   device::mojom::SerialConnectionOptions options_;
+  std::string path_;
   std::vector<uint8_t> buffer_;
   FakeSerialPort::ReadCallback pending_read_callback_;
   uint32_t pending_read_bytes_ = 0;
@@ -189,6 +167,33 @@ class FakeSerialPort : public device::mojom::SerialPort {
   DISALLOW_COPY_AND_ASSIGN(FakeSerialPort);
 };
 
+class FakeSerialPortManager : public device::mojom::SerialPortManager {
+ public:
+  FakeSerialPortManager() = default;
+  ~FakeSerialPortManager() override = default;
+
+ private:
+  // device::mojom::SerialPortManager methods:
+  void GetDevices(GetDevicesCallback callback) override {
+    std::vector<device::mojom::SerialPortInfoPtr> devices;
+    auto device0 = device::mojom::SerialPortInfo::New();
+    device0->path = "/dev/fakeserialmojo";
+    auto device1 = device::mojom::SerialPortInfo::New();
+    device1->path = "\\\\COM800\\";
+    devices.push_back(std::move(device0));
+    devices.push_back(std::move(device1));
+    std::move(callback).Run(std::move(devices));
+  }
+
+  void GetPort(const std::string& path,
+               device::mojom::SerialPortRequest request) override {
+    mojo::MakeStrongBinding(std::make_unique<FakeSerialPort>(path),
+                            std::move(request));
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(FakeSerialPortManager);
+};
+
 class SerialApiTest : public ExtensionApiTest {
  public:
   SerialApiTest() {
@@ -200,9 +205,6 @@ class SerialApiTest : public ExtensionApiTest {
         device::mojom::kServiceName,
         base::BindRepeating(&SerialApiTest::BindSerialPortManager,
                             base::Unretained(this)));
-    service_manager::ServiceBinding::OverrideInterfaceBinderForTesting(
-        device::mojom::kServiceName,
-        base::BindRepeating(&SerialApiTest::BindSerialPort));
 #endif
   }
 
@@ -236,11 +238,6 @@ class SerialApiTest : public ExtensionApiTest {
                             std::move(request));
   }
 
-  static void BindSerialPort(device::mojom::SerialPortRequest request) {
-    mojo::MakeStrongBinding(std::make_unique<FakeSerialPort>(),
-                            std::move(request));
-  }
-
   bool fail_enumerator_request_ = false;
 };
 

extensions/browser/api/serial/serial_connection.cc

@@ -163,11 +163,9 @@ ApiResourceManager<SerialConnection>::GetFactoryInstance() {
 }
 
 SerialConnection::SerialConnection(
-    const std::string& port,
     const std::string& owner_extension_id,
-    device::mojom::SerialPortPtrInfo io_handler_info)
+    device::mojom::SerialPortPtrInfo serial_port_info)
     : ApiResource(owner_extension_id),
-      port_(port),
       persistent_(false),
       buffer_size_(kDefaultBufferSize),
       receive_timeout_(0),
@@ -175,15 +173,15 @@ SerialConnection::SerialConnection(
       paused_(false),
       weak_factory_(this) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  DCHECK(io_handler_info.is_valid());
-  io_handler_.Bind(std::move(io_handler_info));
-  io_handler_.set_connection_error_handler(base::BindOnce(
+  DCHECK(serial_port_info.is_valid());
+  serial_port_.Bind(std::move(serial_port_info));
+  serial_port_.set_connection_error_handler(base::BindOnce(
       &SerialConnection::OnConnectionError, base::Unretained(this)));
 }
 
 SerialConnection::~SerialConnection() {
-  io_handler_->CancelRead(device::mojom::SerialReceiveError::DISCONNECTED);
-  io_handler_->CancelWrite(device::mojom::SerialSendError::DISCONNECTED);
+  serial_port_->CancelRead(device::mojom::SerialReceiveError::DISCONNECTED);
+  serial_port_->CancelWrite(device::mojom::SerialSendError::DISCONNECTED);
 }
 
 bool SerialConnection::IsPersistent() const {
@@ -203,16 +201,16 @@ void SerialConnection::set_send_timeout(int send_timeout) {
 }
 
 void SerialConnection::set_paused(bool paused) {
-  DCHECK(io_handler_);
+  DCHECK(serial_port_);
   paused_ = paused;
   if (paused) {
-    io_handler_->CancelRead(device::mojom::SerialReceiveError::NONE);
+    serial_port_->CancelRead(device::mojom::SerialReceiveError::NONE);
   }
 }
 
 void SerialConnection::set_connection_error_handler(
     base::OnceClosure connection_error_handler) {
-  if (io_handler_.encountered_error()) {
+  if (serial_port_.encountered_error()) {
     // Already being disconnected, run client's error handler immediatelly.
     std::move(connection_error_handler).Run();
     return;
@@ -223,7 +221,7 @@ void SerialConnection::set_connection_error_handler(
 void SerialConnection::Open(const api::serial::ConnectionOptions& options,
                             OpenCompleteCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  DCHECK(io_handler_);
+  DCHECK(serial_port_);
 
   if (options.persistent.get())
     set_persistent(*options.persistent);
@@ -235,8 +233,8 @@ void SerialConnection::Open(const api::serial::ConnectionOptions& options,
     set_receive_timeout(*options.receive_timeout);
   if (options.send_timeout.get())
     set_send_timeout(*options.send_timeout);
-  io_handler_->Open(
-      port_, device::mojom::SerialConnectionOptions::From(options),
+  serial_port_->Open(
+      device::mojom::SerialConnectionOptions::From(options),
       mojo::WrapCallbackWithDefaultInvokeIfNotRun(std::move(callback), false));
 }
 
@@ -244,14 +242,14 @@ bool SerialConnection::Receive(ReceiveCompleteCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (receive_complete_)
     return false;
-  DCHECK(io_handler_);
+  DCHECK(serial_port_);
   receive_complete_ = std::move(callback);
-  io_handler_->Read(buffer_size_,
-                    mojo::WrapCallbackWithDefaultInvokeIfNotRun(
-                        base::BindOnce(&SerialConnection::OnAsyncReadComplete,
-                                       weak_factory_.GetWeakPtr()),
-                        std::vector<uint8_t>(),
-                        device::mojom::SerialReceiveError::DISCONNECTED));
+  serial_port_->Read(buffer_size_,
+                     mojo::WrapCallbackWithDefaultInvokeIfNotRun(
+                         base::BindOnce(&SerialConnection::OnAsyncReadComplete,
+                                        weak_factory_.GetWeakPtr()),
+                         std::vector<uint8_t>(),
+                         device::mojom::SerialReceiveError::DISCONNECTED));
   receive_timeout_task_.Cancel();
   if (receive_timeout_ > 0) {
     receive_timeout_task_.Reset(base::Bind(&SerialConnection::OnReceiveTimeout,
@@ -268,14 +266,14 @@ bool SerialConnection::Send(const std::vector<uint8_t>& data,
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (send_complete_)
     return false;
-  DCHECK(io_handler_);
+  DCHECK(serial_port_);
   send_complete_ = std::move(callback);
-  io_handler_->Write(data,
-                     mojo::WrapCallbackWithDefaultInvokeIfNotRun(
-                         base::BindOnce(&SerialConnection::OnAsyncWriteComplete,
-                                        weak_factory_.GetWeakPtr()),
-                         static_cast<uint32_t>(0),
-                         device::mojom::SerialSendError::DISCONNECTED));
+  serial_port_->Write(
+      data, mojo::WrapCallbackWithDefaultInvokeIfNotRun(
+                base::BindOnce(&SerialConnection::OnAsyncWriteComplete,
+                               weak_factory_.GetWeakPtr()),
+                static_cast<uint32_t>(0),
+                device::mojom::SerialSendError::DISCONNECTED));
   send_timeout_task_.Cancel();
   if (send_timeout_ > 0) {
     send_timeout_task_.Reset(base::Bind(&SerialConnection::OnSendTimeout,
@@ -290,7 +288,7 @@ bool SerialConnection::Send(const std::vector<uint8_t>& data,
 void SerialConnection::Configure(const api::serial::ConnectionOptions& options,
                                  ConfigureCompleteCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  DCHECK(io_handler_);
+  DCHECK(serial_port_);
   if (options.persistent.get())
     set_persistent(*options.persistent);
   if (options.name.get())
@@ -301,15 +299,15 @@ void SerialConnection::Configure(const api::serial::ConnectionOptions& options,
     set_receive_timeout(*options.receive_timeout);
   if (options.send_timeout.get())
     set_send_timeout(*options.send_timeout);
-  io_handler_->ConfigurePort(
+  serial_port_->ConfigurePort(
       device::mojom::SerialConnectionOptions::From(options),
       mojo::WrapCallbackWithDefaultInvokeIfNotRun(std::move(callback), false));
-  io_handler_->CancelRead(device::mojom::SerialReceiveError::NONE);
+  serial_port_->CancelRead(device::mojom::SerialReceiveError::NONE);
 }
 
 void SerialConnection::GetInfo(GetInfoCompleteCallback callback) const {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  DCHECK(io_handler_);
+  DCHECK(serial_port_);
 
   auto info = std::make_unique<api::serial::ConnectionInfo>();
   info->paused = paused_;
@@ -337,19 +335,19 @@ void SerialConnection::GetInfo(GetInfoCompleteCallback callback) const {
         std::move(callback).Run(true, std::move(info));
       },
       std::move(callback), std::move(info));
-  io_handler_->GetPortInfo(mojo::WrapCallbackWithDefaultInvokeIfNotRun(
+  serial_port_->GetPortInfo(mojo::WrapCallbackWithDefaultInvokeIfNotRun(
       std::move(resp_callback), nullptr));
 }
 
 void SerialConnection::Flush(FlushCompleteCallback callback) const {
-  DCHECK(io_handler_);
-  return io_handler_->Flush(
+  DCHECK(serial_port_);
+  return serial_port_->Flush(
       mojo::WrapCallbackWithDefaultInvokeIfNotRun(std::move(callback), false));
 }
 
 void SerialConnection::GetControlSignals(
     GetControlSignalsCompleteCallback callback) const {
-  DCHECK(io_handler_);
+  DCHECK(serial_port_);
   auto resp_callback = base::BindOnce(
       [](GetControlSignalsCompleteCallback callback,
          device::mojom::SerialPortControlSignalsPtr signals) {
@@ -366,41 +364,41 @@ void SerialConnection::GetControlSignals(
         std::move(callback).Run(std::move(control_signals));
       },
       std::move(callback));
-  io_handler_->GetControlSignals(mojo::WrapCallbackWithDefaultInvokeIfNotRun(
+  serial_port_->GetControlSignals(mojo::WrapCallbackWithDefaultInvokeIfNotRun(
       std::move(resp_callback), nullptr));
 }
 
 void SerialConnection::SetControlSignals(
     const api::serial::HostControlSignals& control_signals,
     SetControlSignalsCompleteCallback callback) {
-  DCHECK(io_handler_);
-  io_handler_->SetControlSignals(
+  DCHECK(serial_port_);
+  serial_port_->SetControlSignals(
       device::mojom::SerialHostControlSignals::From(control_signals),
       mojo::WrapCallbackWithDefaultInvokeIfNotRun(std::move(callback), false));
 }
 
 void SerialConnection::SetBreak(SetBreakCompleteCallback callback) {
-  DCHECK(io_handler_);
-  io_handler_->SetBreak(
+  DCHECK(serial_port_);
+  serial_port_->SetBreak(
       mojo::WrapCallbackWithDefaultInvokeIfNotRun(std::move(callback), false));
 }
 
 void SerialConnection::ClearBreak(ClearBreakCompleteCallback callback) {
-  DCHECK(io_handler_);
-  io_handler_->ClearBreak(
+  DCHECK(serial_port_);
+  serial_port_->ClearBreak(
       mojo::WrapCallbackWithDefaultInvokeIfNotRun(std::move(callback), false));
 }
 
 void SerialConnection::OnReceiveTimeout() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  DCHECK(io_handler_);
-  io_handler_->CancelRead(device::mojom::SerialReceiveError::TIMEOUT);
+  DCHECK(serial_port_);
+  serial_port_->CancelRead(device::mojom::SerialReceiveError::TIMEOUT);
 }
 
 void SerialConnection::OnSendTimeout() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  DCHECK(io_handler_);
-  io_handler_->CancelWrite(device::mojom::SerialSendError::TIMEOUT);
+  DCHECK(serial_port_);
+  serial_port_->CancelWrite(device::mojom::SerialSendError::TIMEOUT);
 }
 
 void SerialConnection::OnAsyncReadComplete(

extensions/browser/api/serial/serial_connection.h

@@ -65,9 +65,8 @@ class SerialConnection : public ApiResource {
   using ClearBreakCompleteCallback =
       device::mojom::SerialPort::ClearBreakCallback;
 
-  SerialConnection(const std::string& port,
-                   const std::string& owner_extension_id,
-                   device::mojom::SerialPortPtrInfo io_handler_info);
+  SerialConnection(const std::string& owner_extension_id,
+                   device::mojom::SerialPortPtrInfo serial_port_info);
   ~SerialConnection() override;
 
   // ApiResource override.
@@ -154,20 +153,17 @@ class SerialConnection : public ApiResource {
   // Handles a send timeout.
   void OnSendTimeout();
 
-  // Receives read completion notification from the |io_handler_|.
+  // Receives read completion notification from the |serial_port_|.
   void OnAsyncReadComplete(const std::vector<uint8_t>& data,
                            device::mojom::SerialReceiveError error);
 
-  // Receives write completion notification from the |io_handler_|.
+  // Receives write completion notification from the |serial_port_|.
   void OnAsyncWriteComplete(uint32_t bytes_sent,
                             device::mojom::SerialSendError error);
 
-  // Handles |io_handler_| connection error.
+  // Handles |serial_port_| connection error.
   void OnConnectionError();
 
-  // The pathname of the serial device.
-  std::string port_;
-
   // Flag indicating whether or not the connection should persist when
   // its host app is suspended.
   bool persistent_;
@@ -205,8 +201,8 @@ class SerialConnection : public ApiResource {
   base::CancelableClosure send_timeout_task_;
 
   // Mojo interface ptr corresponding with remote asynchronous I/O handler.
-  device::mojom::SerialPortPtr io_handler_;
-  // Closure which is set by client and will be called when |io_handler_|
+  device::mojom::SerialPortPtr serial_port_;
+  // Closure which is set by client and will be called when |serial_port_|
   // connection encountered an error.
   base::OnceClosure connection_error_handler_;
 

extensions/browser/api/webcam_private/visca_webcam.cc

@@ -171,21 +171,21 @@ void ViscaWebcam::Open(const std::string& path,
                        const std::string& extension_id,
                        const OpenCompleteCallback& open_callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  device::mojom::SerialPortManagerPtr port_manager;
   device::mojom::SerialPortPtrInfo port_ptr_info;
   DCHECK(content::ServiceManagerConnection::GetForProcess());
   content::ServiceManagerConnection::GetForProcess()
       ->GetConnector()
       ->BindInterface(device::mojom::kServiceName,
-                      mojo::MakeRequest(&port_ptr_info));
+                      mojo::MakeRequest(&port_manager));
+  port_manager->GetPort(path, mojo::MakeRequest(&port_ptr_info));
   base::PostTaskWithTraits(
       FROM_HERE, {BrowserThread::IO},
       base::Bind(&ViscaWebcam::OpenOnIOThread, weak_ptr_factory_.GetWeakPtr(),
-                 path, extension_id, base::Passed(&port_ptr_info),
-                 open_callback));
+                 extension_id, base::Passed(&port_ptr_info), open_callback));
 }
 
-void ViscaWebcam::OpenOnIOThread(const std::string& path,
-                                 const std::string& extension_id,
+void ViscaWebcam::OpenOnIOThread(const std::string& extension_id,
                                  device::mojom::SerialPortPtrInfo port_ptr_info,
                                  const OpenCompleteCallback& open_callback) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
@@ -205,7 +205,7 @@ void ViscaWebcam::OpenOnIOThread(const std::string& path,
   options.stop_bits = api::serial::STOP_BITS_ONE;
 
   serial_connection_.reset(
-      new SerialConnection(path, extension_id, std::move(port_ptr_info)));
+      new SerialConnection(extension_id, std::move(port_ptr_info)));
   serial_connection_->Open(
       options, base::BindOnce(&ViscaWebcam::OnConnected,
                               weak_ptr_factory_.GetWeakPtr(), open_callback));

extensions/browser/api/webcam_private/visca_webcam.h

@@ -46,8 +46,7 @@ class ViscaWebcam : public Webcam {
   // Private because WebCam is base::RefCounted.
   ~ViscaWebcam() override;
 
-  void OpenOnIOThread(const std::string& path,
-                      const std::string& extension_id,
+  void OpenOnIOThread(const std::string& extension_id,
                       device::mojom::SerialPortPtrInfo port_ptr_info,
                       const OpenCompleteCallback& open_callback);
 

extensions/browser/api/webcam_private/visca_webcam_unittest.cc

@@ -19,7 +19,7 @@ namespace {
 class TestSerialConnection : public SerialConnection {
  public:
   TestSerialConnection(device::mojom::SerialPortPtrInfo port_ptr_info)
-      : SerialConnection("dummy_path", "dummy_id", std::move(port_ptr_info)) {}
+      : SerialConnection("dummy_id", std::move(port_ptr_info)) {}
   ~TestSerialConnection() override {}
 
   void SetReceiveBuffer(const std::vector<uint8_t>& receive_buffer) {

services/device/device_service.cc

@@ -22,7 +22,6 @@
 #include "services/device/geolocation/public_ip_address_location_notifier.h"
 #include "services/device/power_monitor/power_monitor_message_broadcaster.h"
 #include "services/device/public/mojom/battery_monitor.mojom.h"
-#include "services/device/serial/serial_port_impl.h"
 #include "services/device/serial/serial_port_manager_impl.h"
 #include "services/device/time_zone_monitor/time_zone_monitor.h"
 #include "services/device/wake_lock/wake_lock_provider.h"
@@ -145,8 +144,6 @@ void DeviceService::OnStart() {
       &DeviceService::BindWakeLockProviderRequest, base::Unretained(this)));
   registry_.AddInterface<mojom::SerialPortManager>(base::Bind(
       &DeviceService::BindSerialPortManagerRequest, base::Unretained(this)));
-  registry_.AddInterface<mojom::SerialPort>(base::Bind(
-      &DeviceService::BindSerialPortRequest, base::Unretained(this)));
   registry_.AddInterface<mojom::UsbDeviceManager>(base::Bind(
       &DeviceService::BindUsbDeviceManagerRequest, base::Unretained(this)));
 
@@ -315,17 +312,6 @@ void DeviceService::BindSerialPortManagerRequest(
 #endif
 }
 
-void DeviceService::BindSerialPortRequest(mojom::SerialPortRequest request) {
-#if (defined(OS_LINUX) && defined(USE_UDEV)) || defined(OS_WIN) || \
-    defined(OS_MACOSX)
-  if (io_task_runner_) {
-    io_task_runner_->PostTask(
-        FROM_HERE, base::Bind(&SerialPortImpl::Create, base::Passed(&request),
-                              base::ThreadTaskRunnerHandle::Get()));
-  }
-#endif
-}
-
 void DeviceService::BindUsbDeviceManagerRequest(
     mojom::UsbDeviceManagerRequest request) {
   if (!usb_device_manager_)

services/device/device_service.h

@@ -161,8 +161,6 @@ class DeviceService : public service_manager::Service {
 
   void BindSerialPortManagerRequest(mojom::SerialPortManagerRequest request);
 
-  void BindSerialPortRequest(mojom::SerialPortRequest request);
-
   void BindUsbDeviceManagerRequest(mojom::UsbDeviceManagerRequest request);
 
   service_manager::ServiceBinding service_binding_;

services/device/manifest.json

@@ -21,10 +21,7 @@
         "device:nfc": [ "device.mojom.NFCProvider" ],
         "device:power_monitor": [ "device.mojom.PowerMonitor" ],
         "device:screen_orientation": [ "device.mojom.ScreenOrientationListener" ],
-        "device:serial": [
-          "device.mojom.SerialPortManager",
-          "device.mojom.SerialPort"
-        ],
+        "device:serial": [ "device.mojom.SerialPortManager" ],
         "device:time_zone_monitor": [ "device.mojom.TimeZoneMonitor" ],
         "device:usb": [ "device.mojom.UsbDeviceManager" ],
         "device:vibration": [ "device.mojom.VibrationManager" ],

services/device/public/mojom/serial.mojom

@@ -87,12 +87,13 @@ struct SerialPortControlSignals {
 // Discovers and enumerates serial devices available to the host.
 interface SerialPortManager {
   GetDevices() => (array<SerialPortInfo> devices);
+  GetPort(string path, SerialPort& port_request);
 };
 
 // Performs asynchronous I/O on serial devices.
 interface SerialPort {
   // Initiates an Open of the device then returns result.
-  Open(string port, SerialConnectionOptions options) => (bool success);
+  Open(SerialConnectionOptions options) => (bool success);
 
   // Performs a Read operation then returns retrieved data and the
   // result. Note that the Read may succeed partially, means that even if

services/device/serial/serial_port_impl.cc

@@ -13,22 +13,25 @@ namespace device {
 
 // static
 void SerialPortImpl::Create(
+    const std::string& path,
     mojom::SerialPortRequest request,
     scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner) {
-  mojo::MakeStrongBinding(std::make_unique<SerialPortImpl>(ui_task_runner),
-                          std::move(request));
+  mojo::MakeStrongBinding(
+      std::make_unique<SerialPortImpl>(path, ui_task_runner),
+      std::move(request));
 }
 
 SerialPortImpl::SerialPortImpl(
+    const std::string& path,
     scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner)
-    : io_handler_(device::SerialIoHandler::Create(ui_task_runner)) {}
+    : path_(path),
+      io_handler_(device::SerialIoHandler::Create(ui_task_runner)) {}
 
 SerialPortImpl::~SerialPortImpl() = default;
 
-void SerialPortImpl::Open(const std::string& port,
-                          mojom::SerialConnectionOptionsPtr options,
+void SerialPortImpl::Open(mojom::SerialConnectionOptionsPtr options,
                           OpenCallback callback) {
-  io_handler_->Open(port, *options, std::move(callback));
+  io_handler_->Open(path_, *options, std::move(callback));
 }
 
 void SerialPortImpl::Read(uint32_t bytes, ReadCallback callback) {

services/device/serial/serial_port_impl.h

@@ -23,17 +23,18 @@ namespace device {
 class SerialPortImpl : public mojom::SerialPort {
  public:
   static void Create(
+      const std::string& path,
       mojom::SerialPortRequest request,
       scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner);
 
   explicit SerialPortImpl(
+      const std::string& path,
       scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner);
   ~SerialPortImpl() override;
 
  private:
   // mojom::SerialPort methods:
-  void Open(const std::string& port,
-            mojom::SerialConnectionOptionsPtr options,
+  void Open(mojom::SerialConnectionOptionsPtr options,
             OpenCallback callback) override;
   void Read(uint32_t bytes, ReadCallback callback) override;
   void Write(const std::vector<uint8_t>& data, WriteCallback callback) override;
@@ -49,6 +50,7 @@ class SerialPortImpl : public mojom::SerialPort {
   void SetBreak(SetBreakCallback callback) override;
   void ClearBreak(ClearBreakCallback callback) override;
 
+  std::string path_;
   scoped_refptr<device::SerialIoHandler> io_handler_;
 
   DISALLOW_COPY_AND_ASSIGN(SerialPortImpl);

services/device/serial/serial_port_impl_unittest.cc

@@ -19,7 +19,15 @@ class SerialPortImplTest : public DeviceServiceTestBase {
   SerialPortImplTest() = default;
   ~SerialPortImplTest() override = default;
 
- private:
+ protected:
+  void SetUp() override {
+    DeviceServiceTestBase::SetUp();
+    connector()->BindInterface(mojom::kServiceName, &port_manager_);
+  }
+
+  void TearDown() override { port_manager_.reset(); }
+
+  mojom::SerialPortManagerPtr port_manager_;
   DISALLOW_COPY_AND_ASSIGN(SerialPortImplTest);
 };
 
@@ -27,9 +35,9 @@ class SerialPortImplTest : public DeviceServiceTestBase {
 // Device Service and bind the serial SerialPort interface correctly.
 // TODO(leonhsl): figure out how to add more robust tests.
 TEST_F(SerialPortImplTest, SimpleConnectTest) {
-  mojom::SerialPortPtr io_handler;
-  connector()->BindInterface(mojom::kServiceName, &io_handler);
-  io_handler.FlushForTesting();
+  mojom::SerialPortPtr serial_port;
+  port_manager_->GetPort("", mojo::MakeRequest(&serial_port));
+  serial_port.FlushForTesting();
 }
 
 }  // namespace

services/device/serial/serial_port_manager_impl.cc

@@ -12,9 +12,12 @@ namespace device {
 
 namespace {
 
-void CreateAndBindOnBlockableRunner(mojom::SerialPortManagerRequest request) {
-  mojo::MakeStrongBinding(std::make_unique<SerialPortManagerImpl>(),
-                          std::move(request));
+void CreateAndBindOnBlockableRunner(
+    mojom::SerialPortManagerRequest request,
+    scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner) {
+  mojo::MakeStrongBinding(
+      std::make_unique<SerialPortManagerImpl>(std::move(ui_task_runner)),
+      std::move(request));
 }
 
 }  // namespace
@@ -28,11 +31,15 @@ void SerialPortManagerImpl::Create(mojom::SerialPortManagerRequest request) {
           {base::MayBlock(), base::TaskPriority::BEST_EFFORT});
   blockable_sequence_runner->PostTask(
       FROM_HERE,
-      base::BindOnce(&CreateAndBindOnBlockableRunner, std::move(request)));
+      base::BindOnce(&CreateAndBindOnBlockableRunner, std::move(request),
+                     base::ThreadTaskRunnerHandle::Get()));
 }
 
-SerialPortManagerImpl::SerialPortManagerImpl()
-    : enumerator_(device::SerialDeviceEnumerator::Create()) {}
+// SerialPortManagerImpl must be created in a blockable runner.
+SerialPortManagerImpl::SerialPortManagerImpl(
+    scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner)
+    : enumerator_(device::SerialDeviceEnumerator::Create()),
+      ui_task_runner_(std::move(ui_task_runner)) {}
 
 SerialPortManagerImpl::~SerialPortManagerImpl() = default;
 
@@ -41,4 +48,9 @@ void SerialPortManagerImpl::GetDevices(GetDevicesCallback callback) {
   std::move(callback).Run(enumerator_->GetDevices());
 }
 
+void SerialPortManagerImpl::GetPort(const std::string& path,
+                                    mojom::SerialPortRequest request) {
+  SerialPortImpl::Create(path, std::move(request), ui_task_runner_);
+}
+
 }  // namespace device

services/device/serial/serial_port_manager_impl.h

@@ -8,8 +8,10 @@
 #include <memory>
 
 #include "base/macros.h"
+#include "base/memory/ref_counted.h"
 #include "device/serial/serial_device_enumerator.h"
 #include "services/device/public/mojom/serial.mojom.h"
+#include "services/device/serial/serial_port_impl.h"
 
 namespace device {
 
@@ -20,15 +22,20 @@ class SerialPortManagerImpl : public mojom::SerialPortManager {
  public:
   static void Create(mojom::SerialPortManagerRequest request);
 
-  SerialPortManagerImpl();
+  SerialPortManagerImpl(
+      scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner);
   ~SerialPortManagerImpl() override;
 
  private:
   // mojom::SerialPortManager methods:
   void GetDevices(GetDevicesCallback callback) override;
+  void GetPort(const std::string& path,
+               mojom::SerialPortRequest request) override;
 
   std::unique_ptr<device::SerialDeviceEnumerator> enumerator_;
 
+  scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner_;
+
   DISALLOW_COPY_AND_ASSIGN(SerialPortManagerImpl);
 };
 

services/device/serial/serial_port_manager_impl_unittest.cc

@@ -22,12 +22,12 @@ class SerialPortManagerImplTest : public DeviceServiceTestBase {
  protected:
   void SetUp() override {
     DeviceServiceTestBase::SetUp();
-    connector()->BindInterface(mojom::kServiceName, &enumerator_);
+    connector()->BindInterface(mojom::kServiceName, &port_manager_);
   }
 
-  void TearDown() override { enumerator_.reset(); }
+  void TearDown() override { port_manager_.reset(); }
 
-  mojom::SerialPortManagerPtr enumerator_;
+  mojom::SerialPortManagerPtr port_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(SerialPortManagerImplTest);
 };
@@ -37,7 +37,7 @@ class SerialPortManagerImplTest : public DeviceServiceTestBase {
 // correctly.
 // TODO(leonhsl): figure out how to add more robust tests.
 TEST_F(SerialPortManagerImplTest, SimpleConnectTest) {
-  enumerator_.FlushForTesting();
+  port_manager_.FlushForTesting();
 }
 
 }  // namespace