From e7427f44dc5ebcfb88d7da2e174ef53f88bf2921 Mon Sep 17 00:00:00 2001 From: Miyoung Shin Date: Thu, 19 Sep 2019 03:39:32 +0000 Subject: [PATCH] Convert SerialPortManager to new Mojo types This CL converts SerialPortManager{Ptr, Request} in services, chrome and extensions to the new Mojo type. Bug: 955171 Change-Id: I548c09f2e0b28fea23ac83ddf630a271c7554c5e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1810439 Commit-Queue: Miyoung Shin Reviewed-by: Reilly Grant Reviewed-by: Elly Fong-Jones Reviewed-by: Oksana Zhuravlova Cr-Commit-Position: refs/heads/master@{#697895} --- .../serial/chrome_serial_browsertest.cc | 8 +++--- .../browser/serial/serial_chooser_context.cc | 15 +++++----- .../browser/serial/serial_chooser_context.h | 10 +++++-- .../serial_chooser_controller_unittest.cc | 8 +++--- .../browser/api/serial/serial_apitest.cc | 9 +++--- .../browser/api/serial/serial_port_manager.cc | 6 ++-- .../browser/api/serial/serial_port_manager.h | 3 +- .../cpp/test/fake_serial_port_manager.cc | 6 ++-- .../cpp/test/fake_serial_port_manager.h | 7 +++-- .../device/serial/serial_port_manager_impl.cc | 5 ++-- .../device/serial/serial_port_manager_impl.h | 7 +++-- .../serial_port_manager_impl_unittest.cc | 28 ++++++++++--------- 12 files changed, 62 insertions(+), 50 deletions(-) diff --git a/chrome/browser/serial/chrome_serial_browsertest.cc b/chrome/browser/serial/chrome_serial_browsertest.cc index 8617a7174c296d..33572c018e5d98 100644 --- a/chrome/browser/serial/chrome_serial_browsertest.cc +++ b/chrome/browser/serial/chrome_serial_browsertest.cc @@ -13,7 +13,7 @@ #include "content/public/common/content_switches.h" #include "content/public/test/browser_test_utils.h" #include "content/public/test/test_navigation_observer.h" -#include "mojo/public/cpp/bindings/interface_request.h" +#include "mojo/public/cpp/bindings/pending_remote.h" #include "net/test/embedded_test_server/embedded_test_server.h" #include "services/device/public/cpp/test/fake_serial_port_manager.h" #include "services/device/public/mojom/serial.mojom.h" @@ -33,10 +33,10 @@ class SerialTest : public InProcessBrowserTest { embedded_test_server()->ServeFilesFromSourceDirectory("content/test/data"); ASSERT_TRUE(embedded_test_server()->Start()); - device::mojom::SerialPortManagerPtr port_manager_ptr; - port_manager_.AddBinding(mojo::MakeRequest(&port_manager_ptr)); + mojo::PendingRemote port_manager; + port_manager_.AddReceiver(port_manager.InitWithNewPipeAndPassReceiver()); SerialChooserContextFactory::GetForProfile(browser()->profile()) - ->SetPortManagerForTesting(std::move(port_manager_ptr)); + ->SetPortManagerForTesting(std::move(port_manager)); GURL url = embedded_test_server()->GetURL("localhost", "/simple_page.html"); ui_test_utils::NavigateToURL(browser(), url); diff --git a/chrome/browser/serial/serial_chooser_context.cc b/chrome/browser/serial/serial_chooser_context.cc index 5f0ac95f9e95e1..69e29af0a5e658 100644 --- a/chrome/browser/serial/serial_chooser_context.cc +++ b/chrome/browser/serial/serial_chooser_context.cc @@ -10,6 +10,7 @@ #include "base/values.h" #include "chrome/browser/profiles/profile.h" #include "content/public/browser/system_connector.h" +#include "mojo/public/cpp/bindings/pending_remote.h" #include "services/device/public/mojom/constants.mojom.h" #include "services/service_manager/public/cpp/connector.h" @@ -175,7 +176,7 @@ device::mojom::SerialPortManager* SerialChooserContext::GetPortManager() { } void SerialChooserContext::SetPortManagerForTesting( - device::mojom::SerialPortManagerPtr manager) { + mojo::PendingRemote manager) { SetUpPortManagerConnection(std::move(manager)); } @@ -187,16 +188,16 @@ void SerialChooserContext::EnsurePortManagerConnection() { if (port_manager_) return; - device::mojom::SerialPortManagerPtr manager; - content::GetSystemConnector()->BindInterface(device::mojom::kServiceName, - mojo::MakeRequest(&manager)); + mojo::PendingRemote manager; + content::GetSystemConnector()->Connect( + device::mojom::kServiceName, manager.InitWithNewPipeAndPassReceiver()); SetUpPortManagerConnection(std::move(manager)); } void SerialChooserContext::SetUpPortManagerConnection( - device::mojom::SerialPortManagerPtr manager) { - port_manager_ = std::move(manager); - port_manager_.set_connection_error_handler( + mojo::PendingRemote manager) { + port_manager_.Bind(std::move(manager)); + port_manager_.set_disconnect_handler( base::BindOnce(&SerialChooserContext::OnPortManagerConnectionError, base::Unretained(this))); } diff --git a/chrome/browser/serial/serial_chooser_context.h b/chrome/browser/serial/serial_chooser_context.h index e98a06b9fac70b..38f0eacb163349 100644 --- a/chrome/browser/serial/serial_chooser_context.h +++ b/chrome/browser/serial/serial_chooser_context.h @@ -15,6 +15,8 @@ #include "base/memory/weak_ptr.h" #include "base/unguessable_token.h" #include "chrome/browser/permissions/chooser_context_base.h" +#include "mojo/public/cpp/bindings/pending_remote.h" +#include "mojo/public/cpp/bindings/remote.h" #include "services/device/public/mojom/serial.mojom.h" #include "third_party/blink/public/mojom/serial/serial.mojom.h" #include "url/gurl.h" @@ -55,12 +57,14 @@ class SerialChooserContext : public ChooserContextBase { device::mojom::SerialPortManager* GetPortManager(); - void SetPortManagerForTesting(device::mojom::SerialPortManagerPtr manager); + void SetPortManagerForTesting( + mojo::PendingRemote manager); base::WeakPtr AsWeakPtr(); private: void EnsurePortManagerConnection(); - void SetUpPortManagerConnection(device::mojom::SerialPortManagerPtr manager); + void SetUpPortManagerConnection( + mojo::PendingRemote manager); void OnPortManagerConnectionError(); void OnGetPorts(const url::Origin& requesting_origin, const url::Origin& embedding_origin, @@ -78,7 +82,7 @@ class SerialChooserContext : public ChooserContextBase { // Holds information about ports in |ephemeral_ports_|. std::map port_info_; - device::mojom::SerialPortManagerPtr port_manager_; + mojo::Remote port_manager_; base::WeakPtrFactory weak_factory_{this}; diff --git a/chrome/browser/ui/serial/serial_chooser_controller_unittest.cc b/chrome/browser/ui/serial/serial_chooser_controller_unittest.cc index 949f32463c61ca..36879e614c947c 100644 --- a/chrome/browser/ui/serial/serial_chooser_controller_unittest.cc +++ b/chrome/browser/ui/serial/serial_chooser_controller_unittest.cc @@ -9,7 +9,7 @@ #include "chrome/browser/serial/serial_chooser_context_factory.h" #include "chrome/test/base/chrome_render_view_host_test_harness.h" #include "chrome/test/base/testing_profile.h" -#include "mojo/public/cpp/bindings/interface_request.h" +#include "mojo/public/cpp/bindings/pending_remote.h" #include "services/device/public/cpp/test/fake_serial_port_manager.h" #include "services/device/public/mojom/serial.mojom.h" #include "testing/gmock/include/gmock/gmock.h" @@ -21,10 +21,10 @@ class SerialChooserControllerTest : public ChromeRenderViewHostTestHarness { void SetUp() override { ChromeRenderViewHostTestHarness::SetUp(); - device::mojom::SerialPortManagerPtr port_manager_ptr; - port_manager_.AddBinding(mojo::MakeRequest(&port_manager_ptr)); + mojo::PendingRemote port_manager; + port_manager_.AddReceiver(port_manager.InitWithNewPipeAndPassReceiver()); SerialChooserContextFactory::GetForProfile(profile()) - ->SetPortManagerForTesting(std::move(port_manager_ptr)); + ->SetPortManagerForTesting(std::move(port_manager)); } private: diff --git a/extensions/browser/api/serial/serial_apitest.cc b/extensions/browser/api/serial/serial_apitest.cc index 2a056c1664705b..6815c33ac5d62a 100644 --- a/extensions/browser/api/serial/serial_apitest.cc +++ b/extensions/browser/api/serial/serial_apitest.cc @@ -20,8 +20,9 @@ #include "extensions/common/api/serial.h" #include "extensions/common/switches.h" #include "extensions/test/result_catcher.h" -#include "mojo/public/cpp/bindings/binding_set.h" +#include "mojo/public/cpp/bindings/pending_receiver.h" #include "mojo/public/cpp/bindings/pending_remote.h" +#include "mojo/public/cpp/bindings/receiver_set.h" #include "mojo/public/cpp/system/data_pipe.h" #include "mojo/public/cpp/system/simple_watcher.h" #include "services/device/public/mojom/constants.mojom.h" @@ -289,8 +290,8 @@ class FakeSerialPortManager : public device::mojom::SerialPortManager { ~FakeSerialPortManager() override = default; - void Bind(device::mojom::SerialPortManagerRequest request) { - bindings_.AddBinding(this, std::move(request)); + void Bind(mojo::PendingReceiver receiver) { + receivers_.Add(this, std::move(receiver)); } private: @@ -321,7 +322,7 @@ class FakeSerialPortManager : public device::mojom::SerialPortManager { token, std::make_unique(std::move(port)))); } - mojo::BindingSet bindings_; + mojo::ReceiverSet receivers_; std::map> ports_; DISALLOW_COPY_AND_ASSIGN(FakeSerialPortManager); diff --git a/extensions/browser/api/serial/serial_port_manager.cc b/extensions/browser/api/serial/serial_port_manager.cc index b5990fe2001de9..05606ac24487a0 100644 --- a/extensions/browser/api/serial/serial_port_manager.cc +++ b/extensions/browser/api/serial/serial_port_manager.cc @@ -159,9 +159,9 @@ void SerialPortManager::EnsureConnection() { return; DCHECK(content::GetSystemConnector()); - content::GetSystemConnector()->BindInterface( - device::mojom::kServiceName, mojo::MakeRequest(&port_manager_)); - port_manager_.set_connection_error_handler( + content::GetSystemConnector()->Connect( + device::mojom::kServiceName, port_manager_.BindNewPipeAndPassReceiver()); + port_manager_.set_disconnect_handler( base::BindOnce(&SerialPortManager::OnPortManagerConnectionError, weak_factory_.GetWeakPtr())); } diff --git a/extensions/browser/api/serial/serial_port_manager.h b/extensions/browser/api/serial/serial_port_manager.h index ab8177908c2872..6e1d6e9bb3d6cd 100644 --- a/extensions/browser/api/serial/serial_port_manager.h +++ b/extensions/browser/api/serial/serial_port_manager.h @@ -14,6 +14,7 @@ #include "extensions/browser/api/api_resource_manager.h" #include "extensions/browser/browser_context_keyed_api_factory.h" #include "extensions/common/api/serial.h" +#include "mojo/public/cpp/bindings/remote.h" #include "services/device/public/mojom/serial.mojom.h" namespace content { @@ -80,7 +81,7 @@ class SerialPortManager : public BrowserContextKeyedAPI { std::vector devices); void OnPortManagerConnectionError(); - device::mojom::SerialPortManagerPtr port_manager_; + mojo::Remote port_manager_; content::BrowserThread::ID thread_id_; scoped_refptr connections_; content::BrowserContext* const context_; diff --git a/services/device/public/cpp/test/fake_serial_port_manager.cc b/services/device/public/cpp/test/fake_serial_port_manager.cc index dd77ccd5a4876f..2c4cf2b6ed8844 100644 --- a/services/device/public/cpp/test/fake_serial_port_manager.cc +++ b/services/device/public/cpp/test/fake_serial_port_manager.cc @@ -88,9 +88,9 @@ FakeSerialPortManager::FakeSerialPortManager() = default; FakeSerialPortManager::~FakeSerialPortManager() = default; -void FakeSerialPortManager::AddBinding( - mojom::SerialPortManagerRequest request) { - bindings_.AddBinding(this, std::move(request)); +void FakeSerialPortManager::AddReceiver( + mojo::PendingReceiver receiver) { + receivers_.Add(this, std::move(receiver)); } void FakeSerialPortManager::AddPort(mojom::SerialPortInfoPtr port) { diff --git a/services/device/public/cpp/test/fake_serial_port_manager.h b/services/device/public/cpp/test/fake_serial_port_manager.h index 3f52e08f6e7895..128fb394b87dd7 100644 --- a/services/device/public/cpp/test/fake_serial_port_manager.h +++ b/services/device/public/cpp/test/fake_serial_port_manager.h @@ -9,8 +9,9 @@ #include "base/macros.h" #include "base/unguessable_token.h" -#include "mojo/public/cpp/bindings/binding_set.h" +#include "mojo/public/cpp/bindings/pending_receiver.h" #include "mojo/public/cpp/bindings/pending_remote.h" +#include "mojo/public/cpp/bindings/receiver_set.h" #include "services/device/public/mojom/serial.mojom.h" namespace device { @@ -20,7 +21,7 @@ class FakeSerialPortManager : public mojom::SerialPortManager { FakeSerialPortManager(); ~FakeSerialPortManager() override; - void AddBinding(mojom::SerialPortManagerRequest request); + void AddReceiver(mojo::PendingReceiver receiver); void AddPort(mojom::SerialPortInfoPtr port); // mojom::SerialPortManager @@ -32,7 +33,7 @@ class FakeSerialPortManager : public mojom::SerialPortManager { private: std::map ports_; - mojo::BindingSet bindings_; + mojo::ReceiverSet receivers_; DISALLOW_COPY_AND_ASSIGN(FakeSerialPortManager); }; diff --git a/services/device/serial/serial_port_manager_impl.cc b/services/device/serial/serial_port_manager_impl.cc index bb31d1423c82b3..03c422b2f6e6f5 100644 --- a/services/device/serial/serial_port_manager_impl.cc +++ b/services/device/serial/serial_port_manager_impl.cc @@ -23,8 +23,9 @@ SerialPortManagerImpl::SerialPortManagerImpl( SerialPortManagerImpl::~SerialPortManagerImpl() = default; -void SerialPortManagerImpl::Bind(mojom::SerialPortManagerRequest request) { - bindings_.AddBinding(this, std::move(request)); +void SerialPortManagerImpl::Bind( + mojo::PendingReceiver receiver) { + receivers_.Add(this, std::move(receiver)); } void SerialPortManagerImpl::SetSerialEnumeratorForTesting( diff --git a/services/device/serial/serial_port_manager_impl.h b/services/device/serial/serial_port_manager_impl.h index 310d5d26d6fb64..95837d91f93dfd 100644 --- a/services/device/serial/serial_port_manager_impl.h +++ b/services/device/serial/serial_port_manager_impl.h @@ -9,8 +9,9 @@ #include "base/macros.h" #include "base/memory/ref_counted.h" -#include "mojo/public/cpp/bindings/binding_set.h" +#include "mojo/public/cpp/bindings/pending_receiver.h" #include "mojo/public/cpp/bindings/pending_remote.h" +#include "mojo/public/cpp/bindings/receiver_set.h" #include "services/device/public/mojom/serial.mojom.h" namespace base { @@ -32,7 +33,7 @@ class SerialPortManagerImpl : public mojom::SerialPortManager { scoped_refptr ui_task_runner); ~SerialPortManagerImpl() override; - void Bind(mojom::SerialPortManagerRequest request); + void Bind(mojo::PendingReceiver receiver); void SetSerialEnumeratorForTesting( std::unique_ptr fake_enumerator); @@ -49,7 +50,7 @@ class SerialPortManagerImpl : public mojom::SerialPortManager { scoped_refptr io_task_runner_; scoped_refptr ui_task_runner_; - mojo::BindingSet bindings_; + mojo::ReceiverSet receivers_; DISALLOW_COPY_AND_ASSIGN(SerialPortManagerImpl); }; diff --git a/services/device/serial/serial_port_manager_impl_unittest.cc b/services/device/serial/serial_port_manager_impl_unittest.cc index 9b0020e3a0019c..00f883d2a8b375 100644 --- a/services/device/serial/serial_port_manager_impl_unittest.cc +++ b/services/device/serial/serial_port_manager_impl_unittest.cc @@ -16,10 +16,10 @@ #include "base/test/bind_test_util.h" #include "base/test/task_environment.h" #include "base/threading/thread.h" -#include "mojo/public/cpp/bindings/interface_ptr.h" -#include "mojo/public/cpp/bindings/interface_request.h" +#include "mojo/public/cpp/bindings/pending_receiver.h" #include "mojo/public/cpp/bindings/pending_remote.h" -#include "mojo/public/cpp/bindings/strong_binding.h" +#include "mojo/public/cpp/bindings/remote.h" +#include "mojo/public/cpp/bindings/self_owned_receiver.h" #include "services/device/device_service_test_base.h" #include "services/device/public/mojom/constants.mojom.h" #include "services/device/public/mojom/serial.mojom.h" @@ -34,7 +34,7 @@ const base::FilePath kFakeDevicePath1(FILE_PATH_LITERAL("/dev/fakeserialmojo")); const base::FilePath kFakeDevicePath2(FILE_PATH_LITERAL("\\\\COM800\\")); void CreateAndBindOnBlockableRunner( - mojom::SerialPortManagerRequest request, + mojo::PendingReceiver receiver, scoped_refptr io_task_runner, scoped_refptr ui_task_runner) { auto manager = std::make_unique( @@ -43,7 +43,7 @@ void CreateAndBindOnBlockableRunner( fake_enumerator->AddDevicePath(kFakeDevicePath1); fake_enumerator->AddDevicePath(kFakeDevicePath2); manager->SetSerialEnumeratorForTesting(std::move(fake_enumerator)); - mojo::MakeStrongBinding(std::move(manager), std::move(request)); + mojo::MakeSelfOwnedReceiver(std::move(manager), std::move(receiver)); } } // namespace @@ -56,10 +56,11 @@ class SerialPortManagerImplTest : public DeviceServiceTestBase { protected: void SetUp() override { DeviceServiceTestBase::SetUp(); } - void BindSerialPortManager(mojom::SerialPortManagerRequest request) { + void BindSerialPortManager( + mojo::PendingReceiver receiver) { file_task_runner_->PostTask( FROM_HERE, - base::BindOnce(&CreateAndBindOnBlockableRunner, std::move(request), + base::BindOnce(&CreateAndBindOnBlockableRunner, std::move(receiver), io_task_runner_, base::ThreadTaskRunnerHandle::Get())); } @@ -69,8 +70,9 @@ class SerialPortManagerImplTest : public DeviceServiceTestBase { // This is to simply test that we can enumerate devices on the platform without // hanging or crashing. TEST_F(SerialPortManagerImplTest, SimpleConnectTest) { - mojom::SerialPortManagerPtr port_manager; - connector()->BindInterface(mojom::kServiceName, &port_manager); + mojo::Remote port_manager; + connector()->Connect(mojom::kServiceName, + port_manager.BindNewPipeAndPassReceiver()); base::RunLoop loop; port_manager->GetDevices(base::BindLambdaForTesting( @@ -90,8 +92,8 @@ TEST_F(SerialPortManagerImplTest, SimpleConnectTest) { } TEST_F(SerialPortManagerImplTest, GetDevices) { - mojom::SerialPortManagerPtr port_manager; - BindSerialPortManager(mojo::MakeRequest(&port_manager)); + mojo::Remote port_manager; + BindSerialPortManager(port_manager.BindNewPipeAndPassReceiver()); const std::set expected_paths = {kFakeDevicePath1, kFakeDevicePath2}; @@ -109,8 +111,8 @@ TEST_F(SerialPortManagerImplTest, GetDevices) { } TEST_F(SerialPortManagerImplTest, GetPort) { - mojom::SerialPortManagerPtr port_manager; - BindSerialPortManager(mojo::MakeRequest(&port_manager)); + mojo::Remote port_manager; + BindSerialPortManager(port_manager.BindNewPipeAndPassReceiver()); base::RunLoop loop; port_manager->GetDevices(base::BindLambdaForTesting(