Skip to content

Commit

Permalink
Convert SerialPortManager to new Mojo types
Browse files Browse the repository at this point in the history
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 <myid.shin@igalia.com>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697895}
  • Loading branch information
MyidShin authored and Commit Bot committed Sep 19, 2019
1 parent 7ffb1d6 commit e7427f4
Show file tree
Hide file tree
Showing 12 changed files with 62 additions and 50 deletions.
8 changes: 4 additions & 4 deletions chrome/browser/serial/chrome_serial_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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<device::mojom::SerialPortManager> 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);
Expand Down
15 changes: 8 additions & 7 deletions chrome/browser/serial/serial_chooser_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -175,7 +176,7 @@ device::mojom::SerialPortManager* SerialChooserContext::GetPortManager() {
}

void SerialChooserContext::SetPortManagerForTesting(
device::mojom::SerialPortManagerPtr manager) {
mojo::PendingRemote<device::mojom::SerialPortManager> manager) {
SetUpPortManagerConnection(std::move(manager));
}

Expand All @@ -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<device::mojom::SerialPortManager> 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<device::mojom::SerialPortManager> manager) {
port_manager_.Bind(std::move(manager));
port_manager_.set_disconnect_handler(
base::BindOnce(&SerialChooserContext::OnPortManagerConnectionError,
base::Unretained(this)));
}
Expand Down
10 changes: 7 additions & 3 deletions chrome/browser/serial/serial_chooser_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -55,12 +57,14 @@ class SerialChooserContext : public ChooserContextBase {

device::mojom::SerialPortManager* GetPortManager();

void SetPortManagerForTesting(device::mojom::SerialPortManagerPtr manager);
void SetPortManagerForTesting(
mojo::PendingRemote<device::mojom::SerialPortManager> manager);
base::WeakPtr<SerialChooserContext> AsWeakPtr();

private:
void EnsurePortManagerConnection();
void SetUpPortManagerConnection(device::mojom::SerialPortManagerPtr manager);
void SetUpPortManagerConnection(
mojo::PendingRemote<device::mojom::SerialPortManager> manager);
void OnPortManagerConnectionError();
void OnGetPorts(const url::Origin& requesting_origin,
const url::Origin& embedding_origin,
Expand All @@ -78,7 +82,7 @@ class SerialChooserContext : public ChooserContextBase {
// Holds information about ports in |ephemeral_ports_|.
std::map<base::UnguessableToken, base::Value> port_info_;

device::mojom::SerialPortManagerPtr port_manager_;
mojo::Remote<device::mojom::SerialPortManager> port_manager_;

base::WeakPtrFactory<SerialChooserContext> weak_factory_{this};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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<device::mojom::SerialPortManager> port_manager;
port_manager_.AddReceiver(port_manager.InitWithNewPipeAndPassReceiver());
SerialChooserContextFactory::GetForProfile(profile())
->SetPortManagerForTesting(std::move(port_manager_ptr));
->SetPortManagerForTesting(std::move(port_manager));
}

private:
Expand Down
9 changes: 5 additions & 4 deletions extensions/browser/api/serial/serial_apitest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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<device::mojom::SerialPortManager> receiver) {
receivers_.Add(this, std::move(receiver));
}

private:
Expand Down Expand Up @@ -321,7 +322,7 @@ class FakeSerialPortManager : public device::mojom::SerialPortManager {
token, std::make_unique<FakeSerialPort>(std::move(port))));
}

mojo::BindingSet<device::mojom::SerialPortManager> bindings_;
mojo::ReceiverSet<device::mojom::SerialPortManager> receivers_;
std::map<base::UnguessableToken, std::unique_ptr<FakeSerialPort>> ports_;

DISALLOW_COPY_AND_ASSIGN(FakeSerialPortManager);
Expand Down
6 changes: 3 additions & 3 deletions extensions/browser/api/serial/serial_port_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
Expand Down
3 changes: 2 additions & 1 deletion extensions/browser/api/serial/serial_port_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -80,7 +81,7 @@ class SerialPortManager : public BrowserContextKeyedAPI {
std::vector<device::mojom::SerialPortInfoPtr> devices);
void OnPortManagerConnectionError();

device::mojom::SerialPortManagerPtr port_manager_;
mojo::Remote<device::mojom::SerialPortManager> port_manager_;
content::BrowserThread::ID thread_id_;
scoped_refptr<ConnectionData> connections_;
content::BrowserContext* const context_;
Expand Down
6 changes: 3 additions & 3 deletions services/device/public/cpp/test/fake_serial_port_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<mojom::SerialPortManager> receiver) {
receivers_.Add(this, std::move(receiver));
}

void FakeSerialPortManager::AddPort(mojom::SerialPortInfoPtr port) {
Expand Down
7 changes: 4 additions & 3 deletions services/device/public/cpp/test/fake_serial_port_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -20,7 +21,7 @@ class FakeSerialPortManager : public mojom::SerialPortManager {
FakeSerialPortManager();
~FakeSerialPortManager() override;

void AddBinding(mojom::SerialPortManagerRequest request);
void AddReceiver(mojo::PendingReceiver<mojom::SerialPortManager> receiver);
void AddPort(mojom::SerialPortInfoPtr port);

// mojom::SerialPortManager
Expand All @@ -32,7 +33,7 @@ class FakeSerialPortManager : public mojom::SerialPortManager {

private:
std::map<base::UnguessableToken, mojom::SerialPortInfoPtr> ports_;
mojo::BindingSet<mojom::SerialPortManager> bindings_;
mojo::ReceiverSet<mojom::SerialPortManager> receivers_;

DISALLOW_COPY_AND_ASSIGN(FakeSerialPortManager);
};
Expand Down
5 changes: 3 additions & 2 deletions services/device/serial/serial_port_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<mojom::SerialPortManager> receiver) {
receivers_.Add(this, std::move(receiver));
}

void SerialPortManagerImpl::SetSerialEnumeratorForTesting(
Expand Down
7 changes: 4 additions & 3 deletions services/device/serial/serial_port_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -32,7 +33,7 @@ class SerialPortManagerImpl : public mojom::SerialPortManager {
scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner);
~SerialPortManagerImpl() override;

void Bind(mojom::SerialPortManagerRequest request);
void Bind(mojo::PendingReceiver<mojom::SerialPortManager> receiver);
void SetSerialEnumeratorForTesting(
std::unique_ptr<SerialDeviceEnumerator> fake_enumerator);

Expand All @@ -49,7 +50,7 @@ class SerialPortManagerImpl : public mojom::SerialPortManager {
scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner_;

mojo::BindingSet<SerialPortManager> bindings_;
mojo::ReceiverSet<SerialPortManager> receivers_;

DISALLOW_COPY_AND_ASSIGN(SerialPortManagerImpl);
};
Expand Down
28 changes: 15 additions & 13 deletions services/device/serial/serial_port_manager_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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<mojom::SerialPortManager> receiver,
scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner) {
auto manager = std::make_unique<SerialPortManagerImpl>(
Expand All @@ -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
Expand All @@ -56,10 +56,11 @@ class SerialPortManagerImplTest : public DeviceServiceTestBase {
protected:
void SetUp() override { DeviceServiceTestBase::SetUp(); }

void BindSerialPortManager(mojom::SerialPortManagerRequest request) {
void BindSerialPortManager(
mojo::PendingReceiver<mojom::SerialPortManager> 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()));
}

Expand All @@ -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<mojom::SerialPortManager> port_manager;
connector()->Connect(mojom::kServiceName,
port_manager.BindNewPipeAndPassReceiver());

base::RunLoop loop;
port_manager->GetDevices(base::BindLambdaForTesting(
Expand All @@ -90,8 +92,8 @@ TEST_F(SerialPortManagerImplTest, SimpleConnectTest) {
}

TEST_F(SerialPortManagerImplTest, GetDevices) {
mojom::SerialPortManagerPtr port_manager;
BindSerialPortManager(mojo::MakeRequest(&port_manager));
mojo::Remote<mojom::SerialPortManager> port_manager;
BindSerialPortManager(port_manager.BindNewPipeAndPassReceiver());
const std::set<base::FilePath> expected_paths = {kFakeDevicePath1,
kFakeDevicePath2};

Expand All @@ -109,8 +111,8 @@ TEST_F(SerialPortManagerImplTest, GetDevices) {
}

TEST_F(SerialPortManagerImplTest, GetPort) {
mojom::SerialPortManagerPtr port_manager;
BindSerialPortManager(mojo::MakeRequest(&port_manager));
mojo::Remote<mojom::SerialPortManager> port_manager;
BindSerialPortManager(port_manager.BindNewPipeAndPassReceiver());

base::RunLoop loop;
port_manager->GetDevices(base::BindLambdaForTesting(
Expand Down

0 comments on commit e7427f4

Please sign in to comment.