Skip to content

Commit

Permalink
Code cleaning: Uses scoped_ptr<> to express ownership rather than wri…
Browse files Browse the repository at this point in the history
…ting ownership in comments.

Replaces Response* with scoped_ptr<Response> in dbus code and its related code.

BUG=163231
TEST=no regression / no behavior changes


Review URL: https://chromiumcodereview.appspot.com/12092061

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@181266 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
yuki@chromium.org committed Feb 7, 2013
1 parent d43177f commit 9b25d45
Show file tree
Hide file tree
Showing 27 changed files with 306 additions and 309 deletions.
3 changes: 1 addition & 2 deletions chrome/browser/chromeos/dbus/liveness_service_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,7 @@ void LivenessServiceProvider::CheckLiveness(
dbus::MethodCall* method_call,
dbus::ExportedObject::ResponseSender response_sender) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
dbus::Response* response = dbus::Response::FromMethodCall(method_call);
response_sender.Run(response);
response_sender.Run(dbus::Response::FromMethodCall(method_call));
}

} // namespace chromeos
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ void ProxyResolutionServiceProvider::ResolveProxyHandler(
!reader.PopString(&signal_interface) ||
!reader.PopString(&signal_name)) {
LOG(ERROR) << "Unexpected method call: " << method_call->ToString();
response_sender.Run(NULL);
response_sender.Run(scoped_ptr<dbus::Response>());
return;
}

Expand All @@ -250,8 +250,7 @@ void ProxyResolutionServiceProvider::ResolveProxyHandler(

// Send an empty response for now. We'll send a signal once the network proxy
// resolution is completed.
dbus::Response* response = dbus::Response::FromMethodCall(method_call);
response_sender.Run(response);
response_sender.Run(dbus::Response::FromMethodCall(method_call));
}

// static
Expand All @@ -261,7 +260,7 @@ void ProxyResolutionServiceProvider::CallResolveProxyHandler(
dbus::ExportedObject::ResponseSender response_sender) {
if (!provider_weak_ptr) {
LOG(WARNING) << "Called after the object is deleted";
response_sender.Run(NULL);
response_sender.Run(scoped_ptr<dbus::Response>());
return;
}
provider_weak_ptr->ResolveProxyHandler(method_call, response_sender);
Expand Down
19 changes: 10 additions & 9 deletions chrome/browser/chromeos/dbus/service_provider_test_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,17 @@ void ServiceProviderTestHelper::SetUp(
new dbus::MockObjectProxy(mock_bus_.get(),
kLibCrosServiceName,
dbus::ObjectPath(kLibCrosServicePath));
// |mock_object_proxy_|'s CallMethodAndBlock() will use
// |mock_object_proxy_|'s MockCallMethodAndBlock() will use
// MockCallMethodAndBlock() to return responses.
EXPECT_CALL(*mock_object_proxy_,
CallMethodAndBlock(
MockCallMethodAndBlock(
AllOf(
ResultOf(
std::mem_fun(&dbus::MethodCall::GetInterface),
kLibCrosServiceInterface),
kLibCrosServiceInterface),
ResultOf(
std::mem_fun(&dbus::MethodCall::GetMember),
exported_method_name)),
exported_method_name)),
_))
.WillOnce(Invoke(this,
&ServiceProviderTestHelper::MockCallMethodAndBlock));
Expand Down Expand Up @@ -102,9 +102,10 @@ void ServiceProviderTestHelper::SetUpReturnSignal(
signal_callback, on_connected_callback);
}

dbus::Response* ServiceProviderTestHelper::CallMethod(
scoped_ptr<dbus::Response> ServiceProviderTestHelper::CallMethod(
dbus::MethodCall* method_call) {
return mock_object_proxy_->CallMethodAndBlock(method_call,
return mock_object_proxy_->CallMethodAndBlock(
method_call,
dbus::ObjectProxy::TIMEOUT_USE_DEFAULT);
}

Expand Down Expand Up @@ -155,12 +156,12 @@ void ServiceProviderTestHelper::MockSendSignal(dbus::Signal* signal) {
on_signal_callback_.Run(signal);
}

void ServiceProviderTestHelper::OnResponse(dbus::Response* response) {
response_.reset(response);
void ServiceProviderTestHelper::OnResponse(
scoped_ptr<dbus::Response> response) {
response_ = response.Pass();
response_received_ = true;
if (message_loop_.is_running())
message_loop_.Quit();
}

} // namespace chromeos

10 changes: 5 additions & 5 deletions chrome/browser/chromeos/dbus/service_provider_test_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class ServiceProviderTestHelper {
dbus::ObjectProxy::OnConnectedCallback on_connected_callback);

// Calls tested dbus method.
dbus::Response* CallMethod(dbus::MethodCall* method_call);
scoped_ptr<dbus::Response> CallMethod(dbus::MethodCall* method_call);

// Cleanups helper. Should be called after |CallMethod()|.
void TearDown();
Expand All @@ -66,8 +66,9 @@ class ServiceProviderTestHelper {
dbus::ExportedObject::OnExportedCallback on_exported_callback);

// Calls exported method and waits for a response for |mock_object_proxy_|.
dbus::Response* MockCallMethodAndBlock(dbus::MethodCall* method_call,
::testing::Unused);
dbus::Response* MockCallMethodAndBlock(
dbus::MethodCall* method_call,
::testing::Unused);

// Behaves as |mock_object_proxy_|'s ConnectToSignal().
void MockConnectToSignal(
Expand All @@ -80,7 +81,7 @@ class ServiceProviderTestHelper {
void MockSendSignal(dbus::Signal* signal);

// Receives a response and makes it available to MockCallMethodAndBlock().
void OnResponse(dbus::Response* response);
void OnResponse(scoped_ptr<dbus::Response> response);

scoped_refptr<dbus::MockBus> mock_bus_;
scoped_refptr<dbus::MockExportedObject> mock_exported_object_;
Expand All @@ -95,4 +96,3 @@ class ServiceProviderTestHelper {
} // namespace chromeos

#endif // CHROME_BROWSER_CHROMEOS_DBUS_SERVICE_PROVIDER_TEST_HELPER_H_

Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ void NativeBackendKWalletTest::SetUp() {
"org.kde.klauncher",
dbus::ObjectPath("/KLauncher"));
EXPECT_CALL(*mock_klauncher_proxy_,
CallMethodAndBlock(_, _))
MockCallMethodAndBlock(_, _))
.WillRepeatedly(Invoke(this,
&NativeBackendKWalletTest::KLauncherMethodCall));

Expand All @@ -260,7 +260,7 @@ void NativeBackendKWalletTest::SetUp() {
"org.kde.kwalletd",
dbus::ObjectPath("/modules/kwalletd"));
EXPECT_CALL(*mock_kwallet_proxy_,
CallMethodAndBlock(_, _))
MockCallMethodAndBlock(_, _))
.WillRepeatedly(Invoke(this,
&NativeBackendKWalletTest::KWalletMethodCall));

Expand Down Expand Up @@ -312,13 +312,13 @@ dbus::Response* NativeBackendKWalletTest::KLauncherMethodCall(
if (kwallet_runnable_)
kwallet_running_ = true;

dbus::Response* response = dbus::Response::CreateEmpty();
dbus::MessageWriter writer(response);
scoped_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
dbus::MessageWriter writer(response.get());
writer.AppendInt32(klauncher_ret_);
writer.AppendString(""); // dbus_name
writer.AppendString(klauncher_error_);
writer.AppendInt32(1234); // pid
return response;
return response.release();
}

dbus::Response* NativeBackendKWalletTest::KWalletMethodCall(
Expand All @@ -327,14 +327,14 @@ dbus::Response* NativeBackendKWalletTest::KWalletMethodCall(
return NULL;
EXPECT_EQ("org.kde.KWallet", method_call->GetInterface());

dbus::Response* response = NULL;
scoped_ptr<dbus::Response> response;
if (method_call->GetMember() == "isEnabled") {
response = dbus::Response::CreateEmpty();
dbus::MessageWriter writer(response);
dbus::MessageWriter writer(response.get());
writer.AppendBool(kwallet_enabled_);
} else if (method_call->GetMember() == "networkWallet") {
response = dbus::Response::CreateEmpty();
dbus::MessageWriter writer(response);
dbus::MessageWriter writer(response.get());
writer.AppendString("test_wallet"); // Should match |open| below.
} else if (method_call->GetMember() == "open") {
dbus::MessageReader reader(method_call);
Expand All @@ -346,7 +346,7 @@ dbus::Response* NativeBackendKWalletTest::KWalletMethodCall(
EXPECT_TRUE(reader.PopString(&app_name));
EXPECT_EQ("test_wallet", wallet_name); // Should match |networkWallet|.
response = dbus::Response::CreateEmpty();
dbus::MessageWriter writer(response);
dbus::MessageWriter writer(response.get());
writer.AppendInt32(1); // Can be anything but kInvalidKWalletHandle.
} else if (method_call->GetMember() == "hasFolder" ||
method_call->GetMember() == "createFolder") {
Expand All @@ -359,7 +359,7 @@ dbus::Response* NativeBackendKWalletTest::KWalletMethodCall(
EXPECT_TRUE(reader.PopString(&app_name));
EXPECT_NE(NativeBackendKWalletStub::kInvalidKWalletHandle, handle);
response = dbus::Response::CreateEmpty();
dbus::MessageWriter writer(response);
dbus::MessageWriter writer(response.get());
if (method_call->GetMember() == "hasFolder")
writer.AppendBool(wallet_.hasFolder(folder_name));
else
Expand All @@ -377,7 +377,7 @@ dbus::Response* NativeBackendKWalletTest::KWalletMethodCall(
EXPECT_TRUE(reader.PopString(&app_name));
EXPECT_NE(NativeBackendKWalletStub::kInvalidKWalletHandle, handle);
response = dbus::Response::CreateEmpty();
dbus::MessageWriter writer(response);
dbus::MessageWriter writer(response.get());
if (method_call->GetMember() == "hasEntry")
writer.AppendBool(wallet_.hasEntry(folder_name, key));
else
Expand All @@ -394,7 +394,7 @@ dbus::Response* NativeBackendKWalletTest::KWalletMethodCall(
std::vector<std::string> entries;
if (wallet_.entryList(folder_name, &entries)) {
response = dbus::Response::CreateEmpty();
dbus::MessageWriter writer(response);
dbus::MessageWriter writer(response.get());
writer.AppendArrayOfStrings(entries);
}
} else if (method_call->GetMember() == "readEntry") {
Expand All @@ -411,7 +411,7 @@ dbus::Response* NativeBackendKWalletTest::KWalletMethodCall(
TestKWallet::Blob value;
if (wallet_.readEntry(folder_name, key, &value)) {
response = dbus::Response::CreateEmpty();
dbus::MessageWriter writer(response);
dbus::MessageWriter writer(response.get());
writer.AppendArrayOfBytes(value.data(), value.size());
}
} else if (method_call->GetMember() == "writeEntry") {
Expand All @@ -429,14 +429,14 @@ dbus::Response* NativeBackendKWalletTest::KWalletMethodCall(
EXPECT_TRUE(reader.PopString(&app_name));
EXPECT_NE(NativeBackendKWalletStub::kInvalidKWalletHandle, handle);
response = dbus::Response::CreateEmpty();
dbus::MessageWriter writer(response);
dbus::MessageWriter writer(response.get());
writer.AppendInt32(
wallet_.writeEntry(folder_name, key,
TestKWallet::Blob(bytes, length)) ? 0 : 1);
}

EXPECT_FALSE(response == NULL);
return response;
EXPECT_FALSE(response.get() == NULL);
return response.release();
}

void NativeBackendKWalletTest::CheckPasswordForms(
Expand Down
8 changes: 4 additions & 4 deletions chromeos/dbus/blocking_method_caller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace {

// This function is a part of CallMethodAndBlock implementation.
void CallMethodAndBlockInternal(
dbus::Response** response,
scoped_ptr<dbus::Response>* response,
base::ScopedClosureRunner* signaler,
dbus::ObjectProxy* proxy,
dbus::MethodCall* method_call) {
Expand All @@ -36,7 +36,7 @@ BlockingMethodCaller::BlockingMethodCaller(dbus::Bus* bus,
BlockingMethodCaller::~BlockingMethodCaller() {
}

dbus::Response* BlockingMethodCaller::CallMethodAndBlock(
scoped_ptr<dbus::Response> BlockingMethodCaller::CallMethodAndBlock(
dbus::MethodCall* method_call) {
// on_blocking_method_call_->Signal() will be called when |signaler| is
// destroyed.
Expand All @@ -46,7 +46,7 @@ dbus::Response* BlockingMethodCaller::CallMethodAndBlock(
base::ScopedClosureRunner* signaler =
new base::ScopedClosureRunner(signal_task);

dbus::Response* response = NULL;
scoped_ptr<dbus::Response> response;
bus_->PostTaskToDBusThread(
FROM_HERE,
base::Bind(&CallMethodAndBlockInternal,
Expand All @@ -57,7 +57,7 @@ dbus::Response* BlockingMethodCaller::CallMethodAndBlock(
// http://crbug.com/125360
base::ThreadRestrictions::ScopedAllowWait allow_wait;
on_blocking_method_call_.Wait();
return response;
return response.Pass();
}

} // namespace chromeos
6 changes: 2 additions & 4 deletions chromeos/dbus/blocking_method_caller.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@
#include "base/callback.h"
#include "base/synchronization/waitable_event.h"
#include "chromeos/chromeos_export.h"
#include "dbus/message.h"

namespace dbus {

class Bus;
class ObjectProxy;
class MethodCall;
class Response;

} // namespace dbus

Expand All @@ -29,8 +28,7 @@ class CHROMEOS_EXPORT BlockingMethodCaller {
virtual ~BlockingMethodCaller();

// Calls the method and blocks until it returns.
// The caller is responsible to delete the returned object.
dbus::Response* CallMethodAndBlock(dbus::MethodCall* method_call);
scoped_ptr<dbus::Response> CallMethodAndBlock(dbus::MethodCall* method_call);

private:
dbus::Bus* bus_;
Expand Down
8 changes: 4 additions & 4 deletions chromeos/dbus/blocking_method_caller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class BlockingMethodCallerTest : public testing::Test {

// Set an expectation so mock_proxy's CallMethodAndBlock() will use
// CreateMockProxyResponse() to return responses.
EXPECT_CALL(*mock_proxy_, CallMethodAndBlock(_, _))
EXPECT_CALL(*mock_proxy_, MockCallMethodAndBlock(_, _))
.WillRepeatedly(Invoke(
this, &BlockingMethodCallerTest::CreateMockProxyResponse));

Expand Down Expand Up @@ -78,10 +78,10 @@ class BlockingMethodCallerTest : public testing::Test {
dbus::MessageReader reader(method_call);
std::string text_message;
if (reader.PopString(&text_message)) {
dbus::Response* response = dbus::Response::CreateEmpty();
dbus::MessageWriter writer(response);
scoped_ptr<dbus::Response> response = dbus::Response::CreateEmpty();
dbus::MessageWriter writer(response.get());
writer.AppendString(text_message);
return response;
return response.release();
}
}

Expand Down
Loading

0 comments on commit 9b25d45

Please sign in to comment.