Skip to content

Commit

Permalink
dbus: Use more OnceCallbacks in ObjectProxy
Browse files Browse the repository at this point in the history
Convert WaitForServiceToBeAvailableCallback and OnConnectedCallback to
OnceCallback.

Remove unused MockObjectProxy::WaitForServiceToBeAvailable().
Add MockObjectProxy::DoConnectToSignal() to workaround Gmock's
limitation around move-only arguments.
https://github.com/google/googletest/blob/master/googlemock/docs/CookBook.md#mocking-methods-that-use-move-only-types

BUG=740015
TEST=try

Change-Id: Ide16b01e57eac20f231d729ad570547376a7874d
Reviewed-on: https://chromium-review.googlesource.com/656480
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Commit-Queue: Ryo Hashimoto <hashimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501180}
  • Loading branch information
Ryo Hashimoto authored and Commit Bot committed Sep 12, 2017
1 parent 8b58011 commit 0fc9c71
Show file tree
Hide file tree
Showing 17 changed files with 147 additions and 144 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,8 @@ std::unique_ptr<dbus::Response> GetIdResponse(uint32_t id) {

ACTION_P(RegisterSignalCallback, callback_addr) {
*callback_addr = arg2;
arg3.Run("" /* interface_name */, "" /* signal_name */, true /* success */);
std::move(*arg3).Run("" /* interface_name */, "" /* signal_name */,
true /* success */);
}

ACTION_P2(OnNotify, verifier, id) {
Expand Down Expand Up @@ -242,14 +243,14 @@ class NotificationPlatformBridgeLinuxTest : public testing::Test {
.WillOnce(Return(ByMove(std::move(response))));

if (connect_signals) {
EXPECT_CALL(
*mock_notification_proxy_.get(),
ConnectToSignal(kFreedesktopNotificationsName, "ActionInvoked", _, _))
EXPECT_CALL(*mock_notification_proxy_.get(),
DoConnectToSignal(kFreedesktopNotificationsName,
"ActionInvoked", _, _))
.WillOnce(RegisterSignalCallback(&action_invoked_callback_));

EXPECT_CALL(*mock_notification_proxy_.get(),
ConnectToSignal(kFreedesktopNotificationsName,
"NotificationClosed", _, _))
DoConnectToSignal(kFreedesktopNotificationsName,
"NotificationClosed", _, _))
.WillOnce(RegisterSignalCallback(&notification_closed_callback_));
}

Expand Down
2 changes: 1 addition & 1 deletion chromeos/dbus/auth_policy_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ class AuthPolicyClientImpl : public AuthPolicyClient {
dbus::ObjectProxy::OnConnectedCallback on_connected_callback) override {
proxy_->ConnectToSignal(authpolicy::kAuthPolicyInterface,
std::move(signal_name), std::move(signal_callback),
on_connected_callback);
std::move(on_connected_callback));
}

protected:
Expand Down
9 changes: 5 additions & 4 deletions chromeos/dbus/biod/biod_client_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class BiodClientTest : public testing::Test {
.WillRepeatedly(Return(proxy_.get()));

// Save |client_|'s signal callback.
EXPECT_CALL(*proxy_.get(), ConnectToSignal(kInterface, _, _, _))
EXPECT_CALL(*proxy_.get(), DoConnectToSignal(kInterface, _, _, _))
.WillRepeatedly(Invoke(this, &BiodClientTest::ConnectToSignal));

client_.reset(BiodClient::Create(REAL_DBUS_CLIENT_IMPLEMENTATION));
Expand Down Expand Up @@ -176,12 +176,13 @@ class BiodClientTest : public testing::Test {
const std::string& interface_name,
const std::string& signal_name,
dbus::ObjectProxy::SignalCallback signal_callback,
dbus::ObjectProxy::OnConnectedCallback on_connected_callback) {
dbus::ObjectProxy::OnConnectedCallback* on_connected_callback) {
EXPECT_EQ(interface_name, kInterface);
signal_callbacks_[signal_name] = signal_callback;
message_loop_.task_runner()->PostTask(
FROM_HERE, base::Bind(on_connected_callback, interface_name,
signal_name, true /* success */));
FROM_HERE,
base::BindOnce(std::move(*on_connected_callback), interface_name,
signal_name, true /* success */));
}

// Handles calls to |proxy_|'s CallMethod().
Expand Down
98 changes: 41 additions & 57 deletions chromeos/dbus/cras_audio_client_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ class CrasAudioClientTest : public testing::Test {
// callback.
EXPECT_CALL(
*mock_cras_proxy_.get(),
ConnectToSignal(interface_name_, cras::kOutputMuteChanged, _, _))
DoConnectToSignal(interface_name_, cras::kOutputMuteChanged, _, _))
.WillRepeatedly(
Invoke(this, &CrasAudioClientTest::OnConnectToOutputMuteChanged));

Expand All @@ -291,47 +291,43 @@ class CrasAudioClientTest : public testing::Test {
// callback.
EXPECT_CALL(
*mock_cras_proxy_.get(),
ConnectToSignal(interface_name_, cras::kInputMuteChanged, _, _))
DoConnectToSignal(interface_name_, cras::kInputMuteChanged, _, _))
.WillRepeatedly(
Invoke(this, &CrasAudioClientTest::OnConnectToInputMuteChanged));

// Set an expectation so mock_cras_proxy's monitoring NodesChanged
// ConnectToSignal will use OnConnectToNodesChanged() to run the callback.
EXPECT_CALL(
*mock_cras_proxy_.get(),
ConnectToSignal(interface_name_, cras::kNodesChanged, _, _))
EXPECT_CALL(*mock_cras_proxy_.get(),
DoConnectToSignal(interface_name_, cras::kNodesChanged, _, _))
.WillRepeatedly(
Invoke(this, &CrasAudioClientTest::OnConnectToNodesChanged));

// Set an expectation so mock_cras_proxy's monitoring
// ActiveOutputNodeChanged ConnectToSignal will use
// OnConnectToActiveOutputNodeChanged() to run the callback.
EXPECT_CALL(
*mock_cras_proxy_.get(),
ConnectToSignal(interface_name_, cras::kActiveOutputNodeChanged, _, _))
.WillRepeatedly(
Invoke(this,
&CrasAudioClientTest::OnConnectToActiveOutputNodeChanged));
EXPECT_CALL(*mock_cras_proxy_.get(),
DoConnectToSignal(interface_name_,
cras::kActiveOutputNodeChanged, _, _))
.WillRepeatedly(Invoke(
this, &CrasAudioClientTest::OnConnectToActiveOutputNodeChanged));

// Set an expectation so mock_cras_proxy's monitoring
// ActiveInputNodeChanged ConnectToSignal will use
// OnConnectToActiveInputNodeChanged() to run the callback.
EXPECT_CALL(
*mock_cras_proxy_.get(),
ConnectToSignal(interface_name_, cras::kActiveInputNodeChanged, _, _))
.WillRepeatedly(
Invoke(this,
&CrasAudioClientTest::OnConnectToActiveInputNodeChanged));
DoConnectToSignal(interface_name_, cras::kActiveInputNodeChanged, _, _))
.WillRepeatedly(Invoke(
this, &CrasAudioClientTest::OnConnectToActiveInputNodeChanged));

// Set an expectation so mock_cras_proxy's monitoring
// OutputNodeVolumeChanged ConnectToSignal will use
// OnConnectToOutputNodeVolumeChanged() to run the callback.
EXPECT_CALL(
*mock_cras_proxy_.get(),
ConnectToSignal(interface_name_, cras::kOutputNodeVolumeChanged, _, _))
.WillRepeatedly(
Invoke(this,
&CrasAudioClientTest::OnConnectToOutputNodeVolumeChanged));
EXPECT_CALL(*mock_cras_proxy_.get(),
DoConnectToSignal(interface_name_,
cras::kOutputNodeVolumeChanged, _, _))
.WillRepeatedly(Invoke(
this, &CrasAudioClientTest::OnConnectToOutputNodeVolumeChanged));

// Set an expectation so mock_bus's GetObjectProxy() for the given
// service name and the object path will return mock_cras_proxy_.
Expand Down Expand Up @@ -438,14 +434,12 @@ class CrasAudioClientTest : public testing::Test {
const std::string& interface_name,
const std::string& signal_name,
const dbus::ObjectProxy::SignalCallback& signal_callback,
const dbus::ObjectProxy::OnConnectedCallback& on_connected_callback) {
dbus::ObjectProxy::OnConnectedCallback* on_connected_callback) {
output_mute_changed_handler_ = signal_callback;
const bool success = true;
message_loop_.task_runner()->PostTask(FROM_HERE,
base::Bind(on_connected_callback,
interface_name,
signal_name,
success));
message_loop_.task_runner()->PostTask(
FROM_HERE, base::BindOnce(std::move(*on_connected_callback),
interface_name, signal_name, success));
}

// Checks the requested interface name and signal name.
Expand All @@ -454,14 +448,12 @@ class CrasAudioClientTest : public testing::Test {
const std::string& interface_name,
const std::string& signal_name,
const dbus::ObjectProxy::SignalCallback& signal_callback,
const dbus::ObjectProxy::OnConnectedCallback& on_connected_callback) {
dbus::ObjectProxy::OnConnectedCallback* on_connected_callback) {
input_mute_changed_handler_ = signal_callback;
const bool success = true;
message_loop_.task_runner()->PostTask(FROM_HERE,
base::Bind(on_connected_callback,
interface_name,
signal_name,
success));
message_loop_.task_runner()->PostTask(
FROM_HERE, base::BindOnce(std::move(*on_connected_callback),
interface_name, signal_name, success));
}

// Checks the requested interface name and signal name.
Expand All @@ -470,14 +462,12 @@ class CrasAudioClientTest : public testing::Test {
const std::string& interface_name,
const std::string& signal_name,
const dbus::ObjectProxy::SignalCallback& signal_callback,
const dbus::ObjectProxy::OnConnectedCallback& on_connected_callback) {
dbus::ObjectProxy::OnConnectedCallback* on_connected_callback) {
nodes_changed_handler_ = signal_callback;
const bool success = true;
message_loop_.task_runner()->PostTask(FROM_HERE,
base::Bind(on_connected_callback,
interface_name,
signal_name,
success));
message_loop_.task_runner()->PostTask(
FROM_HERE, base::BindOnce(std::move(*on_connected_callback),
interface_name, signal_name, success));
}

// Checks the requested interface name and signal name.
Expand All @@ -486,14 +476,12 @@ class CrasAudioClientTest : public testing::Test {
const std::string& interface_name,
const std::string& signal_name,
const dbus::ObjectProxy::SignalCallback& signal_callback,
const dbus::ObjectProxy::OnConnectedCallback& on_connected_callback) {
dbus::ObjectProxy::OnConnectedCallback* on_connected_callback) {
active_output_node_changed_handler_ = signal_callback;
const bool success = true;
message_loop_.task_runner()->PostTask(FROM_HERE,
base::Bind(on_connected_callback,
interface_name,
signal_name,
success));
message_loop_.task_runner()->PostTask(
FROM_HERE, base::BindOnce(std::move(*on_connected_callback),
interface_name, signal_name, success));
}

// Checks the requested interface name and signal name.
Expand All @@ -502,14 +490,12 @@ class CrasAudioClientTest : public testing::Test {
const std::string& interface_name,
const std::string& signal_name,
const dbus::ObjectProxy::SignalCallback& signal_callback,
const dbus::ObjectProxy::OnConnectedCallback& on_connected_callback) {
dbus::ObjectProxy::OnConnectedCallback* on_connected_callback) {
active_input_node_changed_handler_ = signal_callback;
const bool success = true;
message_loop_.task_runner()->PostTask(FROM_HERE,
base::Bind(on_connected_callback,
interface_name,
signal_name,
success));
message_loop_.task_runner()->PostTask(
FROM_HERE, base::BindOnce(std::move(*on_connected_callback),
interface_name, signal_name, success));
}

// Checks the requested interface name and signal name.
Expand All @@ -518,14 +504,12 @@ class CrasAudioClientTest : public testing::Test {
const std::string& interface_name,
const std::string& signal_name,
const dbus::ObjectProxy::SignalCallback& signal_callback,
const dbus::ObjectProxy::OnConnectedCallback& on_connected_callback) {
dbus::ObjectProxy::OnConnectedCallback* on_connected_callback) {
output_node_volume_changed_handler_ = signal_callback;
const bool success = true;
message_loop_.task_runner()->PostTask(FROM_HERE,
base::Bind(on_connected_callback,
interface_name,
signal_name,
success));
message_loop_.task_runner()->PostTask(
FROM_HERE, base::BindOnce(std::move(*on_connected_callback),
interface_name, signal_name, success));
}

// Checks the content of the method call and returns the response.
Expand Down
4 changes: 2 additions & 2 deletions chromeos/dbus/fake_auth_policy_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,8 @@ void FakeAuthPolicyClient::ConnectToSignal(
const std::string& signal_name,
dbus::ObjectProxy::SignalCallback signal_callback,
dbus::ObjectProxy::OnConnectedCallback on_connected_callback) {
on_connected_callback.Run(authpolicy::kAuthPolicyInterface, signal_name,
true /* success */);
std::move(on_connected_callback)
.Run(authpolicy::kAuthPolicyInterface, signal_name, true /* success */);
PostDelayedClosure(
base::BindOnce(RunSignalCallback, authpolicy::kAuthPolicyInterface,
signal_name, signal_callback),
Expand Down
12 changes: 5 additions & 7 deletions chromeos/dbus/gsm_sms_client_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,8 @@ class GsmSMSClientTest : public testing::Test {
// Set an expectation so mock_proxy's ConnectToSignal() will use
// OnConnectToSignal() to run the callback.
EXPECT_CALL(*mock_proxy_.get(),
ConnectToSignal(modemmanager::kModemManagerSMSInterface,
modemmanager::kSMSReceivedSignal,
_,
_))
DoConnectToSignal(modemmanager::kModemManagerSMSInterface,
modemmanager::kSMSReceivedSignal, _, _))
.WillRepeatedly(Invoke(this, &GsmSMSClientTest::OnConnectToSignal));

// Set an expectation so mock_bus's GetObjectProxy() for the given
Expand Down Expand Up @@ -174,12 +172,12 @@ class GsmSMSClientTest : public testing::Test {
const std::string& interface_name,
const std::string& signal_name,
const dbus::ObjectProxy::SignalCallback& signal_callback,
const dbus::ObjectProxy::OnConnectedCallback& on_connected_callback) {
dbus::ObjectProxy::OnConnectedCallback* on_connected_callback) {
sms_received_callback_ = signal_callback;
const bool success = true;
message_loop_.task_runner()->PostTask(
FROM_HERE, base::Bind(on_connected_callback, interface_name,
signal_name, success));
FROM_HERE, base::BindOnce(std::move(*on_connected_callback),
interface_name, signal_name, success));
}
};

Expand Down
17 changes: 8 additions & 9 deletions chromeos/dbus/modem_messaging_client_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,12 @@ class ModemMessagingClientTest : public testing::Test {

// Set an expectation so mock_proxy's ConnectToSignal() will use
// OnConnectToSignal() to run the callback.
EXPECT_CALL(*mock_proxy_.get(),
ConnectToSignal(modemmanager::kModemManager1MessagingInterface,
modemmanager::kSMSAddedSignal,
_,
_))
EXPECT_CALL(
*mock_proxy_.get(),
DoConnectToSignal(modemmanager::kModemManager1MessagingInterface,
modemmanager::kSMSAddedSignal, _, _))
.WillRepeatedly(
Invoke(this, &ModemMessagingClientTest::OnConnectToSignal));
Invoke(this, &ModemMessagingClientTest::OnConnectToSignal));

// Set an expectation so mock_bus's GetObjectProxy() for the given
// service name and the object path will return mock_proxy_.
Expand Down Expand Up @@ -154,12 +153,12 @@ class ModemMessagingClientTest : public testing::Test {
const std::string& interface_name,
const std::string& signal_name,
const dbus::ObjectProxy::SignalCallback& signal_callback,
const dbus::ObjectProxy::OnConnectedCallback& on_connected_callback) {
dbus::ObjectProxy::OnConnectedCallback* on_connected_callback) {
sms_received_callback_ = signal_callback;
const bool success = true;
message_loop_.task_runner()->PostTask(
FROM_HERE, base::Bind(on_connected_callback, interface_name,
signal_name, success));
FROM_HERE, base::BindOnce(std::move(*on_connected_callback),
interface_name, signal_name, success));
}
};

Expand Down
9 changes: 5 additions & 4 deletions chromeos/dbus/power_manager_client_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ class PowerManagerClientTest : public testing::Test {
.WillRepeatedly(Return(proxy_.get()));

// Save |client_|'s signal and name-owner-changed callbacks.
EXPECT_CALL(*proxy_.get(), ConnectToSignal(kInterface, _, _, _))
EXPECT_CALL(*proxy_.get(), DoConnectToSignal(kInterface, _, _, _))
.WillRepeatedly(Invoke(this, &PowerManagerClientTest::ConnectToSignal));
EXPECT_CALL(*proxy_.get(), SetNameOwnerChangedCallback(_))
.WillRepeatedly(SaveArg<0>(&name_owner_changed_callback_));
Expand Down Expand Up @@ -300,13 +300,14 @@ class PowerManagerClientTest : public testing::Test {
const std::string& interface_name,
const std::string& signal_name,
dbus::ObjectProxy::SignalCallback signal_callback,
dbus::ObjectProxy::OnConnectedCallback on_connected_callback) {
dbus::ObjectProxy::OnConnectedCallback* on_connected_callback) {
CHECK_EQ(interface_name, power_manager::kPowerManagerInterface);
signal_callbacks_[signal_name] = signal_callback;

message_loop_.task_runner()->PostTask(
FROM_HERE, base::Bind(on_connected_callback, interface_name,
signal_name, true /* success */));
FROM_HERE,
base::BindOnce(std::move(*on_connected_callback), interface_name,
signal_name, true /* success */));
}

// Handles calls to |proxy_|'s CallMethod() method to register suspend delays.
Expand Down
9 changes: 5 additions & 4 deletions chromeos/dbus/services/service_provider_test_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,12 @@ void ServiceProviderTestHelper::SetUpReturnSignal(
// |mock_object_proxy_|'s ConnectToSignal will use
// MockConnectToSignal().
EXPECT_CALL(*mock_object_proxy_.get(),
ConnectToSignal(interface_name, signal_name, _, _))
DoConnectToSignal(interface_name, signal_name, _, _))
.WillOnce(Invoke(this, &ServiceProviderTestHelper::MockConnectToSignal));

mock_object_proxy_->ConnectToSignal(interface_name, signal_name,
signal_callback, on_connected_callback);
signal_callback,
std::move(on_connected_callback));
}

std::unique_ptr<dbus::Response> ServiceProviderTestHelper::CallMethod(
Expand Down Expand Up @@ -141,9 +142,9 @@ void ServiceProviderTestHelper::MockConnectToSignal(
const std::string& interface_name,
const std::string& signal_name,
dbus::ObjectProxy::SignalCallback signal_callback,
dbus::ObjectProxy::OnConnectedCallback connected_callback) {
dbus::ObjectProxy::OnConnectedCallback* connected_callback) {
// Tell the callback that the object proxy is connected to the signal.
connected_callback.Run(interface_name, signal_name, true);
std::move(*connected_callback).Run(interface_name, signal_name, true);
// Capture the callback, so we can run this at a later time.
on_signal_callback_ = signal_callback;
}
Expand Down
2 changes: 1 addition & 1 deletion chromeos/dbus/services/service_provider_test_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class ServiceProviderTestHelper {
const std::string& interface_name,
const std::string& signal_name,
dbus::ObjectProxy::SignalCallback signal_callback,
dbus::ObjectProxy::OnConnectedCallback connected_callback);
dbus::ObjectProxy::OnConnectedCallback* connected_callback);

// Behaves as |mock_exported_object_|'s SendSignal().
void MockSendSignal(dbus::Signal* signal);
Expand Down
Loading

0 comments on commit 0fc9c71

Please sign in to comment.