Code cleaning: Uses scoped_ptr<> to express ownership rather than writing 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

9 files changed, 107 insertions, 103 deletions

diff --git a/dbus/exported_object.cc b/dbus/exported_object.cc
index f9ddb62..b8cfbc5 100644
--- a/dbus/exported_object.cc
+++ b/dbus/exported_object.cc
@@ -218,16 +218,16 @@ DBusHandlerResult ExportedObject::HandleMessage(
                                  base::Bind(&ExportedObject::RunMethod,
                                             this,
                                             iter->second,
-                                            method_call.release(),
+                                            base::Passed(&method_call),
                                             start_time));
   } else {
     // If the D-Bus thread is not used, just call the method directly.
-    MethodCall* released_method_call = method_call.release();
-    iter->second.Run(released_method_call,
+    MethodCall* method = method_call.get();
+    iter->second.Run(method,
                      base::Bind(&ExportedObject::SendResponse,
                                 this,
                                 start_time,
-                                released_method_call));
+                                base::Passed(&method_call)));
   }
 
   // It's valid to say HANDLED here, and send a method response at a later
@@ -236,38 +236,37 @@ DBusHandlerResult ExportedObject::HandleMessage(
 }
 
 void ExportedObject::RunMethod(MethodCallCallback method_call_callback,
-                               MethodCall* method_call,
+                               scoped_ptr<MethodCall> method_call,
                                base::TimeTicks start_time) {
   bus_->AssertOnOriginThread();
-  method_call_callback.Run(method_call,
+  MethodCall* method = method_call.get();
+  method_call_callback.Run(method,
                            base::Bind(&ExportedObject::SendResponse,
                                       this,
                                       start_time,
-                                      method_call));
+                                      base::Passed(&method_call)));
 }
 
 void ExportedObject::SendResponse(base::TimeTicks start_time,
-                                  MethodCall* method_call,
-                                  Response* response) {
+                                  scoped_ptr<MethodCall> method_call,
+                                  scoped_ptr<Response> response) {
   DCHECK(method_call);
   if (bus_->HasDBusThread()) {
     bus_->PostTaskToDBusThread(FROM_HERE,
                                base::Bind(&ExportedObject::OnMethodCompleted,
                                           this,
-                                          method_call,
-                                          response,
+                                          base::Passed(&method_call),
+                                          base::Passed(&response),
                                           start_time));
   } else {
-    OnMethodCompleted(method_call, response, start_time);
+    OnMethodCompleted(method_call.Pass(), response.Pass(), start_time);
   }
 }
 
-void ExportedObject::OnMethodCompleted(MethodCall* method_call,
-                                       Response* response,
+void ExportedObject::OnMethodCompleted(scoped_ptr<MethodCall> method_call,
+                                       scoped_ptr<Response> response,
                                        base::TimeTicks start_time) {
   bus_->AssertOnDBusThread();
-  scoped_ptr<MethodCall> method_call_deleter(method_call);
-  scoped_ptr<Response> response_deleter(response);
 
   // Record if the method call is successful, or not. 1 if successful.
   UMA_HISTOGRAM_ENUMERATION("DBus.ExportedMethodHandleSuccess",
@@ -283,7 +282,7 @@ void ExportedObject::OnMethodCompleted(MethodCall* method_call,
     // Something bad happened in the method call.
     scoped_ptr<dbus::ErrorResponse> error_response(
         ErrorResponse::FromMethodCall(
-            method_call,
+            method_call.get(),
             DBUS_ERROR_FAILED,
             "error occurred in " + method_call->GetMember()));
     bus_->Send(error_response->raw_message(), NULL);
diff --git a/dbus/exported_object.h b/dbus/exported_object.h
index 4e74ddb..89a4133 100644
--- a/dbus/exported_object.h
+++ b/dbus/exported_object.h
@@ -41,12 +41,7 @@ class CHROME_DBUS_EXPORT ExportedObject
   // Called to send a response from an exported method. |response| is the
   // response message. Callers should pass NULL in the event of an error that
   // prevents the sending of a response.
-  //
-  // ResponseSender takes ownership of |response| hence client code should
-  // not delete |response|.
-  // TODO(satorux): Change this to take scoped_ptr<Response> to make
-  // ownership clearer. crbug.com/163231
-  typedef base::Callback<void (Response* response)> ResponseSender;
+  typedef base::Callback<void (scoped_ptr<Response> response)> ResponseSender;
 
   // Called when an exported method is called. |method_call| is the request
   // message. |sender| is the callback that's used to send a response.
@@ -134,20 +129,20 @@ class CHROME_DBUS_EXPORT ExportedObject
 
   // Runs the method. Helper function for HandleMessage().
   void RunMethod(MethodCallCallback method_call_callback,
-                 MethodCall* method_call,
+                 scoped_ptr<MethodCall> method_call,
                  base::TimeTicks start_time);
 
   // Callback invoked by service provider to send a response to a method call.
   // Can be called immediately from a MethodCallCallback to implement a
   // synchronous service or called later to implement an asynchronous service.
   void SendResponse(base::TimeTicks start_time,
-                    MethodCall* method_call,
-                    Response* response);
+                    scoped_ptr<MethodCall> method_call,
+                    scoped_ptr<Response> response);
 
   // Called on completion of the method run from SendResponse().
   // Takes ownership of |method_call| and |response|.
-  void OnMethodCompleted(MethodCall* method_call,
-                         Response* response,
+  void OnMethodCompleted(scoped_ptr<MethodCall> method_call,
+                         scoped_ptr<Response> response,
                          base::TimeTicks start_time);
 
   // Called when the object is unregistered.
diff --git a/dbus/message.cc b/dbus/message.cc
index afcd932..ce53a48 100644
--- a/dbus/message.cc
+++ b/dbus/message.cc
@@ -400,26 +400,27 @@ Signal* Signal::FromRawMessage(DBusMessage* raw_message) {
 Response::Response() : Message() {
 }
 
-Response* Response::FromRawMessage(DBusMessage* raw_message) {
+scoped_ptr<Response> Response::FromRawMessage(DBusMessage* raw_message) {
   DCHECK_EQ(DBUS_MESSAGE_TYPE_METHOD_RETURN,
             dbus_message_get_type(raw_message));
 
-  Response* response = new Response;
+  scoped_ptr<Response> response(new Response);
   response->Init(raw_message);
-  return response;
+  return response.Pass();
 }
 
-Response* Response::FromMethodCall(MethodCall* method_call) {
-  Response* response = new Response;
+scoped_ptr<Response> Response::FromMethodCall(MethodCall* method_call) {
+  scoped_ptr<Response> response(new Response);
   response->Init(dbus_message_new_method_return(method_call->raw_message()));
-  return response;
+  return response.Pass();
 }
 
-Response* Response::CreateEmpty() {
-  Response* response = new Response;
+scoped_ptr<Response> Response::CreateEmpty() {
+  scoped_ptr<Response> response(new Response);
   response->Init(dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN));
-  return response;
+  return response.Pass();
 }
+
 //
 // ErrorResponse implementation.
 //
@@ -427,23 +428,24 @@ Response* Response::CreateEmpty() {
 ErrorResponse::ErrorResponse() : Response() {
 }
 
-ErrorResponse* ErrorResponse::FromRawMessage(DBusMessage* raw_message) {
+scoped_ptr<ErrorResponse> ErrorResponse::FromRawMessage(
+    DBusMessage* raw_message) {
   DCHECK_EQ(DBUS_MESSAGE_TYPE_ERROR, dbus_message_get_type(raw_message));
 
-  ErrorResponse* response = new ErrorResponse;
+  scoped_ptr<ErrorResponse> response(new ErrorResponse);
   response->Init(raw_message);
-  return response;
+  return response.Pass();
 }
 
-ErrorResponse* ErrorResponse::FromMethodCall(
+scoped_ptr<ErrorResponse> ErrorResponse::FromMethodCall(
     MethodCall* method_call,
     const std::string& error_name,
     const std::string& error_message) {
-  ErrorResponse* response = new ErrorResponse;
+  scoped_ptr<ErrorResponse> response(new ErrorResponse);
   response->Init(dbus_message_new_error(method_call->raw_message(),
                                         error_name.c_str(),
                                         error_message.c_str()));
-  return response;
+  return response.Pass();
 }
 
 //
diff --git a/dbus/message.h b/dbus/message.h
index 42d6860..54ca036 100644
--- a/dbus/message.h
+++ b/dbus/message.h
@@ -10,6 +10,7 @@
 #include <dbus/dbus.h>
 
 #include "base/basictypes.h"
+#include "base/memory/scoped_ptr.h"
 #include "dbus/dbus_export.h"
 #include "dbus/file_descriptor.h"
 #include "dbus/object_path.h"
@@ -200,18 +201,17 @@ class CHROME_DBUS_EXPORT Signal : public Message {
 class CHROME_DBUS_EXPORT Response : public Message {
  public:
   // Returns a newly created Response from the given raw message of the
-  // type DBUS_MESSAGE_TYPE_METHOD_RETURN. The caller must delete the
-  // returned object. Takes the ownership of |raw_message|.
-  static Response* FromRawMessage(DBusMessage* raw_message);
+  // type DBUS_MESSAGE_TYPE_METHOD_RETURN. Takes the ownership of |raw_message|.
+  static scoped_ptr<Response> FromRawMessage(DBusMessage* raw_message);
 
-  // Returns a newly created Response from the given method call. The
-  // caller must delete the returned object. Used for implementing
-  // exported methods.
-  static Response* FromMethodCall(MethodCall* method_call);
+  // Returns a newly created Response from the given method call.
+  // Used for implementing exported methods. Does NOT take the ownership of
+  // |method_call|.
+  static scoped_ptr<Response> FromMethodCall(MethodCall* method_call);
 
-  // Returns a newly created Response with an empty payload. The caller
-  // must delete the returned object. Useful for testing.
-  static Response* CreateEmpty();
+  // Returns a newly created Response with an empty payload.
+  // Useful for testing.
+  static scoped_ptr<Response> CreateEmpty();
 
  protected:
   // Creates a Response message. The internal raw message is NULL.
@@ -226,17 +226,17 @@ class CHROME_DBUS_EXPORT Response : public Message {
 class CHROME_DBUS_EXPORT ErrorResponse: public Response {
  public:
   // Returns a newly created Response from the given raw message of the
-  // type DBUS_MESSAGE_TYPE_METHOD_RETURN. The caller must delete the
-  // returned object. Takes the ownership of |raw_message|.
-  static ErrorResponse* FromRawMessage(DBusMessage* raw_message);
+  // type DBUS_MESSAGE_TYPE_METHOD_RETURN. Takes the ownership of |raw_message|.
+  static scoped_ptr<ErrorResponse> FromRawMessage(DBusMessage* raw_message);
 
   // Returns a newly created ErrorResponse from the given method call, the
   // error name, and the error message.  The error name looks like
   // "org.freedesktop.DBus.Error.Failed". Used for returning an error to a
-  // failed method call.
-  static ErrorResponse* FromMethodCall(MethodCall* method_call,
-                                       const std::string& error_name,
-                                       const std::string& error_message);
+  // failed method call. Does NOT take the ownership of |method_call|.
+  static scoped_ptr<ErrorResponse> FromMethodCall(
+      MethodCall* method_call,
+      const std::string& error_name,
+      const std::string& error_message);
 
  private:
   // Creates an ErrorResponse message. The internal raw message is NULL.
diff --git a/dbus/mock_object_proxy.h b/dbus/mock_object_proxy.h
index e820f45..fcad860 100644
--- a/dbus/mock_object_proxy.h
+++ b/dbus/mock_object_proxy.h
@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "dbus/message.h"
 #include "dbus/object_path.h"
 #include "dbus/object_proxy.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -20,8 +21,17 @@ class MockObjectProxy : public ObjectProxy {
                   const std::string& service_name,
                   const ObjectPath& object_path);
 
-  MOCK_METHOD2(CallMethodAndBlock, Response*(MethodCall* method_call,
-                                             int timeout_ms));
+  // GMock doesn't support the return type of scoped_ptr<> because scoped_ptr is
+  // uncopyable. This is a workaround which defines |MockCallMethodAndBlock| as
+  // a mock method and makes |CallMethodAndBlock| call the mocked method.
+  // Use |MockCallMethodAndBlock| for setting/testing expectations.
+  MOCK_METHOD2(MockCallMethodAndBlock, Response*(MethodCall* method_call,
+                                                 int timeout_ms));
+  virtual scoped_ptr<Response> CallMethodAndBlock(MethodCall* method_call,
+                                                  int timeout_ms) OVERRIDE {
+    return scoped_ptr<Response>(MockCallMethodAndBlock(method_call,
+                                                       timeout_ms));
+  }
   MOCK_METHOD3(CallMethod, void(MethodCall* method_call,
                                 int timeout_ms,
                                 ResponseCallback callback));
diff --git a/dbus/mock_unittest.cc b/dbus/mock_unittest.cc
index cf644df..f860082 100644
--- a/dbus/mock_unittest.cc
+++ b/dbus/mock_unittest.cc
@@ -39,7 +39,7 @@ class MockTest : 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, &MockTest::CreateMockProxyResponse));
 
     // Set an expectation so mock_proxy's CallMethod() will use
@@ -91,10 +91,10 @@ class MockTest : 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();
       }
     }
 
diff --git a/dbus/object_proxy.cc b/dbus/object_proxy.cc
index b20f544..9a84f25 100644
--- a/dbus/object_proxy.cc
+++ b/dbus/object_proxy.cc
@@ -63,14 +63,14 @@ ObjectProxy::~ObjectProxy() {
 // Originally we tried to make |method_call| a const reference, but we
 // gave up as dbus_connection_send_with_reply_and_block() takes a
 // non-const pointer of DBusMessage as the second parameter.
-Response* ObjectProxy::CallMethodAndBlock(MethodCall* method_call,
-                                          int timeout_ms) {
+scoped_ptr<Response> ObjectProxy::CallMethodAndBlock(MethodCall* method_call,
+                                                     int timeout_ms) {
   bus_->AssertOnDBusThread();
 
   if (!bus_->Connect() ||
       !method_call->SetDestination(service_name_) ||
       !method_call->SetPath(object_path_))
-    return NULL;
+    return scoped_ptr<Response>();
 
   DBusMessage* request_message = method_call->raw_message();
 
@@ -93,7 +93,7 @@ Response* ObjectProxy::CallMethodAndBlock(MethodCall* method_call,
                          method_call->GetMember(),
                          error.is_set() ? error.name() : "unknown error type",
                          error.is_set() ? error.message() : "");
-    return NULL;
+    return scoped_ptr<Response>();
   }
   // Record time spent for the method call. Don't include failures.
   UMA_HISTOGRAM_TIMES("DBus.SyncMethodCallTime",
diff --git a/dbus/object_proxy.h b/dbus/object_proxy.h
index 801c577..61576e6 100644
--- a/dbus/object_proxy.h
+++ b/dbus/object_proxy.h
@@ -81,11 +81,10 @@ class CHROME_DBUS_EXPORT ObjectProxy
 
   // Calls the method of the remote object and blocks until the response
   // is returned. Returns NULL on error.
-  // The caller is responsible to delete the returned object.
   //
   // BLOCKING CALL.
-  virtual Response* CallMethodAndBlock(MethodCall* method_call,
-                                       int timeout_ms);
+  virtual scoped_ptr<Response> CallMethodAndBlock(MethodCall* method_call,
+                                                  int timeout_ms);
 
   // Requests to call the method of the remote object.
   //
diff --git a/dbus/test_service.cc b/dbus/test_service.cc
index 0d99d57..d9dbf82 100644
--- a/dbus/test_service.cc
+++ b/dbus/test_service.cc
@@ -243,14 +243,14 @@ void TestService::Echo(MethodCall* method_call,
   MessageReader reader(method_call);
   std::string text_message;
   if (!reader.PopString(&text_message)) {
-    response_sender.Run(NULL);
+    response_sender.Run(scoped_ptr<dbus::Response>());
     return;
   }
 
-  Response* response = Response::FromMethodCall(method_call);
-  MessageWriter writer(response);
+  scoped_ptr<Response> response = Response::FromMethodCall(method_call);
+  MessageWriter writer(response.get());
   writer.AppendString(text_message);
-  response_sender.Run(response);
+  response_sender.Run(response.Pass());
 }
 
 void TestService::SlowEcho(
@@ -275,7 +275,7 @@ void TestService::AsyncEcho(
 void TestService::BrokenMethod(
     MethodCall* method_call,
     dbus::ExportedObject::ResponseSender response_sender) {
-  response_sender.Run(NULL);
+  response_sender.Run(scoped_ptr<dbus::Response>());
 }
 
 
@@ -285,7 +285,7 @@ void TestService::GetAllProperties(
   MessageReader reader(method_call);
   std::string interface;
   if (!reader.PopString(&interface)) {
-    response_sender.Run(NULL);
+    response_sender.Run(scoped_ptr<dbus::Response>());
     return;
   }
 
@@ -300,8 +300,8 @@ void TestService::GetAllProperties(
   //   "Objects": Variant<[objectpath:"/TestObjectPath"]>
   // ]
 
-  Response* response = Response::FromMethodCall(method_call);
-  MessageWriter writer(response);
+  scoped_ptr<Response> response = Response::FromMethodCall(method_call);
+  MessageWriter writer(response.get());
 
   MessageWriter array_writer(NULL);
   MessageWriter dict_entry_writer(NULL);
@@ -343,7 +343,7 @@ void TestService::GetAllProperties(
 
   writer.CloseContainer(&array_writer);
 
-  response_sender.Run(response);
+  response_sender.Run(response.Pass());
 }
 
 void TestService::GetProperty(
@@ -352,39 +352,39 @@ void TestService::GetProperty(
   MessageReader reader(method_call);
   std::string interface;
   if (!reader.PopString(&interface)) {
-    response_sender.Run(NULL);
+    response_sender.Run(scoped_ptr<dbus::Response>());
     return;
   }
 
   std::string name;
   if (!reader.PopString(&name)) {
-    response_sender.Run(NULL);
+    response_sender.Run(scoped_ptr<dbus::Response>());
     return;
   }
 
   if (name == "Name") {
     // Return the previous value for the "Name" property:
     // Variant<"TestService">
-    Response* response = Response::FromMethodCall(method_call);
-    MessageWriter writer(response);
+    scoped_ptr<Response> response = Response::FromMethodCall(method_call);
+    MessageWriter writer(response.get());
 
     writer.AppendVariantOfString("TestService");
 
-    response_sender.Run(response);
+    response_sender.Run(response.Pass());
   } else if (name == "Version") {
     // Return a new value for the "Version" property:
     // Variant<20>
-    Response* response = Response::FromMethodCall(method_call);
-    MessageWriter writer(response);
+    scoped_ptr<Response> response = Response::FromMethodCall(method_call);
+    MessageWriter writer(response.get());
 
     writer.AppendVariantOfInt16(20);
 
-    response_sender.Run(response);
+    response_sender.Run(response.Pass());
   } else if (name == "Methods") {
     // Return the previous value for the "Methods" property:
     // Variant<["Echo", "SlowEcho", "AsyncEcho", "BrokenMethod"]>
-    Response* response = Response::FromMethodCall(method_call);
-    MessageWriter writer(response);
+    scoped_ptr<Response> response = Response::FromMethodCall(method_call);
+    MessageWriter writer(response.get());
     MessageWriter variant_writer(NULL);
     MessageWriter variant_array_writer(NULL);
 
@@ -397,12 +397,12 @@ void TestService::GetProperty(
     variant_writer.CloseContainer(&variant_array_writer);
     writer.CloseContainer(&variant_writer);
 
-    response_sender.Run(response);
+    response_sender.Run(response.Pass());
   } else if (name == "Objects") {
     // Return the previous value for the "Objects" property:
     // Variant<[objectpath:"/TestObjectPath"]>
-    Response* response = Response::FromMethodCall(method_call);
-    MessageWriter writer(response);
+    scoped_ptr<Response> response = Response::FromMethodCall(method_call);
+    MessageWriter writer(response.get());
     MessageWriter variant_writer(NULL);
     MessageWriter variant_array_writer(NULL);
 
@@ -412,10 +412,10 @@ void TestService::GetProperty(
     variant_writer.CloseContainer(&variant_array_writer);
     writer.CloseContainer(&variant_writer);
 
-    response_sender.Run(response);
+    response_sender.Run(response.Pass());
   } else {
     // Return error.
-    response_sender.Run(NULL);
+    response_sender.Run(scoped_ptr<dbus::Response>());
     return;
   }
 }
@@ -426,31 +426,30 @@ void TestService::SetProperty(
   MessageReader reader(method_call);
   std::string interface;
   if (!reader.PopString(&interface)) {
-    response_sender.Run(NULL);
+    response_sender.Run(scoped_ptr<dbus::Response>());
     return;
   }
 
   std::string name;
   if (!reader.PopString(&name)) {
-    response_sender.Run(NULL);
+    response_sender.Run(scoped_ptr<dbus::Response>());
     return;
   }
 
   if (name != "Name") {
-    response_sender.Run(NULL);
+    response_sender.Run(scoped_ptr<dbus::Response>());
     return;
   }
 
   std::string value;
   if (!reader.PopVariantOfString(&value)) {
-    response_sender.Run(NULL);
+    response_sender.Run(scoped_ptr<dbus::Response>());
     return;
   }
 
   SendPropertyChangedSignal(value);
 
-  Response* response = Response::FromMethodCall(method_call);
-  response_sender.Run(response);
+  response_sender.Run(Response::FromMethodCall(method_call));
 }
 
 void TestService::SendPropertyChangedSignal(const std::string& name) {