Fix design shortcomings in Message classes.

- Prohibit to instantiate Message class.
  Rationale: this is not corresponding to any D-Bus message types.

- Get rid of Message::reset_raw_message()
  Rationale: this was breaking encapsulation. For instance, It was possible
  to inject a DBUS_MESSAGE_TYPE_ERROR raw message to a MethodCall message,
  which should not be allowed.

- Prohibit to instantiate Response/ErrorResponse with NULL raw message.
  Rationale: Message objects should be backed up by valid raw messages.

- Change Object::CallMethodAndBlock() to return Response*.
  Rationale: the original API requred a Response object with raw_message_ set
  to NULL, which we no longer allow.

- Add message_type header to ToString().

BUG=90036
TEST=dbus_unittests


Review URL: http://codereview.chromium.org/7709009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@97983 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 225 insertions, 202 deletions

diff --git a/dbus/end_to_end_sync_unittest.cc b/dbus/end_to_end_sync_unittest.cc
index 704845e..fe0489a 100644
--- a/dbus/end_to_end_sync_unittest.cc
+++ b/dbus/end_to_end_sync_unittest.cc
@@ -58,14 +58,13 @@ TEST_F(EndToEndSyncTest, Echo) {
   writer.AppendString(kHello);
 
   // Call the method.
-  dbus::Response response;
   const int timeout_ms = dbus::ObjectProxy::TIMEOUT_USE_DEFAULT;
-  const bool success =
-      object_proxy_->CallMethodAndBlock(&method_call, timeout_ms, &response);
-  ASSERT_TRUE(success);
+  scoped_ptr<dbus::Response> response(
+      object_proxy_->CallMethodAndBlock(&method_call, timeout_ms));
+  ASSERT_TRUE(response.get());
 
   // Check the response. kHello should be echoed back.
-  dbus::MessageReader reader(&response);
+  dbus::MessageReader reader(response.get());
   std::string returned_message;
   ASSERT_TRUE(reader.PopString(&returned_message));
   EXPECT_EQ(kHello, returned_message);
@@ -80,30 +79,27 @@ TEST_F(EndToEndSyncTest, Timeout) {
   writer.AppendString(kHello);
 
   // Call the method with timeout of 0ms.
-  dbus::Response response;
   const int timeout_ms = 0;
-  const bool success =
-      object_proxy_->CallMethodAndBlock(&method_call, timeout_ms, &response);
+  scoped_ptr<dbus::Response> response(
+      object_proxy_->CallMethodAndBlock(&method_call, timeout_ms));
   // Should fail because of timeout.
-  ASSERT_FALSE(success);
+  ASSERT_FALSE(response.get());
 }
 
 TEST_F(EndToEndSyncTest, NonexistentMethod) {
   dbus::MethodCall method_call("org.chromium.TestInterface", "Nonexistent");
 
-  dbus::Response response;
   const int timeout_ms = dbus::ObjectProxy::TIMEOUT_USE_DEFAULT;
-  const bool success =
-      object_proxy_->CallMethodAndBlock(&method_call, timeout_ms, &response);
-  ASSERT_FALSE(success);
+  scoped_ptr<dbus::Response> response(
+      object_proxy_->CallMethodAndBlock(&method_call, timeout_ms));
+  ASSERT_FALSE(response.get());
 }
 
 TEST_F(EndToEndSyncTest, BrokenMethod) {
   dbus::MethodCall method_call("org.chromium.TestInterface", "BrokenMethod");
 
-  dbus::Response response;
   const int timeout_ms = dbus::ObjectProxy::TIMEOUT_USE_DEFAULT;
-  const bool success =
-      object_proxy_->CallMethodAndBlock(&method_call, timeout_ms, &response);
-  ASSERT_FALSE(success);
+  scoped_ptr<dbus::Response> response(
+      object_proxy_->CallMethodAndBlock(&method_call, timeout_ms));
+  ASSERT_FALSE(response.get());
 }
diff --git a/dbus/message.cc b/dbus/message.cc
index 8de6583..2837ce5 100644
--- a/dbus/message.cc
+++ b/dbus/message.cc
@@ -45,6 +45,11 @@ Message::~Message() {
     dbus_message_unref(raw_message_);
 }
 
+void Message::Init(DBusMessage* raw_message) {
+  DCHECK(!raw_message_);
+  raw_message_ = raw_message;
+}
+
 Message::MessageType Message::GetMessageType() {
   if (!raw_message_)
     return MESSAGE_INVALID;
@@ -52,10 +57,21 @@ Message::MessageType Message::GetMessageType() {
   return static_cast<Message::MessageType>(type);
 }
 
-void Message::reset_raw_message(DBusMessage* raw_message) {
-  if (raw_message_)
-    dbus_message_unref(raw_message_);
-  raw_message_ = raw_message;
+std::string Message::GetMessageTypeAsString() {
+  switch (GetMessageType()) {
+    case MESSAGE_INVALID:
+      return "MESSAGE_INVALID";
+    case MESSAGE_METHOD_CALL:
+      return "MESSAGE_METHOD_CALL";
+    case MESSAGE_METHOD_RETURN:
+      return "MESSAGE_METHOD_RETURN";
+    case MESSAGE_SIGNAL:
+      return "MESSAGE_SIGNAL";
+    case MESSAGE_ERROR:
+      return "MESSAGE_ERROR";
+  }
+  NOTREACHED();
+  return "";
 }
 
 std::string Message::ToStringInternal(const std::string& indent,
@@ -203,6 +219,7 @@ std::string Message::ToString() {
 
   // Generate headers first.
   std::string headers;
+  AppendStringHeader("message_type", GetMessageTypeAsString(), &headers);
   AppendStringHeader("destination", GetDestination(), &headers);
   AppendStringHeader("path", GetPath(), &headers);
   AppendStringHeader("interface", GetInterface(), &headers);
@@ -312,22 +329,20 @@ uint32 Message::GetReplySerial() {
 MethodCall::MethodCall(const std::string& interface_name,
                        const std::string& method_name)
     : Message() {
-  reset_raw_message(dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_CALL));
+  Init(dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_CALL));
 
   SetInterface(interface_name);
   SetMember(method_name);
 }
 
+MethodCall::MethodCall() : Message() {
+}
+
 MethodCall* MethodCall::FromRawMessage(DBusMessage* raw_message) {
   DCHECK_EQ(DBUS_MESSAGE_TYPE_METHOD_CALL, dbus_message_get_type(raw_message));
 
-  const char* interface = dbus_message_get_interface(raw_message);
-  const char* member = dbus_message_get_member(raw_message);
-  std::string interface_string = interface ? interface : "";
-  std::string member_string = member ? member : "";
-
-  MethodCall* method_call = new MethodCall(interface_string, member_string);
-  method_call->reset_raw_message(raw_message);
+  MethodCall* method_call = new MethodCall;
+  method_call->Init(raw_message);
   return method_call;
 }
 
@@ -337,22 +352,20 @@ MethodCall* MethodCall::FromRawMessage(DBusMessage* raw_message) {
 Signal::Signal(const std::string& interface_name,
                const std::string& method_name)
     : Message() {
-  reset_raw_message(dbus_message_new(DBUS_MESSAGE_TYPE_SIGNAL));
+  Init(dbus_message_new(DBUS_MESSAGE_TYPE_SIGNAL));
 
   SetInterface(interface_name);
   SetMember(method_name);
 }
 
+Signal::Signal() : Message() {
+}
+
 Signal* Signal::FromRawMessage(DBusMessage* raw_message) {
   DCHECK_EQ(DBUS_MESSAGE_TYPE_SIGNAL, dbus_message_get_type(raw_message));
 
-  const char* interface = dbus_message_get_interface(raw_message);
-  const char* member = dbus_message_get_member(raw_message);
-  std::string interface_string = interface ? interface : "";
-  std::string member_string = member ? member : "";
-
-  Signal* signal = new Signal(interface_string, member_string);
-  signal->reset_raw_message(raw_message);
+  Signal* signal = new Signal;
+  signal->Init(raw_message);
   return signal;
 }
 
@@ -363,13 +376,26 @@ Signal* Signal::FromRawMessage(DBusMessage* raw_message) {
 Response::Response() : Message() {
 }
 
+Response* Response::FromRawMessage(DBusMessage* raw_message) {
+  DCHECK_EQ(DBUS_MESSAGE_TYPE_METHOD_RETURN,
+            dbus_message_get_type(raw_message));
+
+  Response* response = new Response;
+  response->Init(raw_message);
+  return response;
+}
+
 Response* Response::FromMethodCall(MethodCall* method_call) {
   Response* response = new Response;
-  response->reset_raw_message(
-      dbus_message_new_method_return(method_call->raw_message()));
+  response->Init(dbus_message_new_method_return(method_call->raw_message()));
   return response;
 }
 
+Response* Response::CreateEmpty() {
+  Response* response = new Response;
+  response->Init(dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN));
+  return response;
+}
 //
 // ErrorResponse implementation.
 //
@@ -377,15 +403,22 @@ Response* Response::FromMethodCall(MethodCall* method_call) {
 ErrorResponse::ErrorResponse() : Message() {
 }
 
+ErrorResponse* ErrorResponse::FromRawMessage(DBusMessage* raw_message) {
+  DCHECK_EQ(DBUS_MESSAGE_TYPE_ERROR, dbus_message_get_type(raw_message));
+
+  ErrorResponse* response = new ErrorResponse;
+  response->Init(raw_message);
+  return response;
+}
+
 ErrorResponse* ErrorResponse::FromMethodCall(
     MethodCall* method_call,
     const std::string& error_name,
     const std::string& error_message) {
   ErrorResponse* response = new ErrorResponse;
-  response->reset_raw_message(
-      dbus_message_new_error(method_call->raw_message(),
-                             error_name.c_str(),
-                             error_message.c_str()));
+  response->Init(dbus_message_new_error(method_call->raw_message(),
+                                        error_name.c_str(),
+                                        error_message.c_str()));
   return response;
 }
 
diff --git a/dbus/message.h b/dbus/message.h
index 58162a3..0a692de 100644
--- a/dbus/message.h
+++ b/dbus/message.h
@@ -17,8 +17,8 @@ namespace dbus {
 class MessageWriter;
 class MessageReader;
 
-// Message is the base class of D-Bus message types. Client code should
-// usually use sub classes such as MethodCall and Response instead.
+// Message is the base class of D-Bus message types. Client code must use
+// sub classes such as MethodCall and Response instead.
 //
 // The class name Message is very generic, but there should be no problem
 // as the class is inside 'dbus' namespace. We chose to name this way, as
@@ -59,20 +59,15 @@ class Message {
     VARIANT = DBUS_TYPE_VARIANT,
   };
 
-  // Creates a Message. The internal raw message is NULL until it's set
-  // from outside by reset_raw_message().
-  Message();
-  virtual ~Message();
-
   // Returns the type of the message. Returns MESSAGE_INVALID if
   // raw_message_ is NULL.
   MessageType GetMessageType();
 
-  DBusMessage* raw_message() { return raw_message_; }
+  // Returns the type of the message as string like "MESSAGE_METHOD_CALL"
+  // for instance.
+  std::string GetMessageTypeAsString();
 
-  // Resets raw_message_ with the given raw message. Takes the ownership
-  // of raw_message. raw_message_ will be unref'ed in the destructor.
-  void reset_raw_message(DBusMessage* raw_message);
+  DBusMessage* raw_message() { return raw_message_; }
 
   // Sets the destination, the path, the interface, the member, etc.
   void SetDestination(const std::string& destination);
@@ -102,6 +97,14 @@ class Message {
   // debugging.
   std::string ToString();
 
+ protected:
+  // This class cannot be instantiated. Use sub classes instead.
+  Message();
+  virtual ~Message();
+
+  // Initializes the message with the given raw message.
+  void Init(DBusMessage* raw_message);
+
  private:
   // Helper function used in ToString().
   std::string ToStringInternal(const std::string& indent,
@@ -123,8 +126,7 @@ class MethodCall : public Message {
   //
   //   MethodCall method_call(DBUS_INTERFACE_INTROSPECTABLE, "Get");
   //
-  // The constructor creates the internal raw_message_, so the client
-  // doesn't need to set this with reset_raw_message().
+  // The constructor creates the internal raw message.
   MethodCall(const std::string& interface_name,
              const std::string& method_name);
 
@@ -133,6 +135,11 @@ class MethodCall : public Message {
   // returned object. Takes the ownership of |raw_message|.
   static MethodCall* FromRawMessage(DBusMessage* raw_message);
 
+ private:
+  // Creates a method call message. The internal raw message is NULL.
+  // Only used internally.
+  MethodCall();
+
   DISALLOW_COPY_AND_ASSIGN(MethodCall);
 };
 
@@ -148,8 +155,7 @@ class Signal : public Message {
   //
   //   Signal signal(DBUS_INTERFACE_INTROSPECTABLE, "PropertiesChanged");
   //
-  // The constructor creates the internal raw_message_, so the client
-  // doesn't need to set this with reset_raw_message().
+  // The constructor creates the internal raw_message_.
   Signal(const std::string& interface_name,
          const std::string& method_name);
 
@@ -157,23 +163,37 @@ class Signal : public Message {
   // DBUS_MESSAGE_TYPE_SIGNAL. The caller must delete the returned
   // object. Takes the ownership of |raw_message|.
   static Signal* FromRawMessage(DBusMessage* raw_message);
+
+ private:
+  // Creates a signal message. The internal raw message is NULL.
+  // Only used internally.
+  Signal();
+
+  DISALLOW_COPY_AND_ASSIGN(Signal);
 };
 
 // Response is a type of message used for receiving a response from a
 // method via D-Bus.
 class Response : public Message {
  public:
-  // Creates a Response message. The internal raw message is NULL.
-  // Classes that implment method calls need to set the raw message once a
-  // response is received from the server. See object_proxy.h.
-  Response();
+  // 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);
 
   // 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 creaed Response with an empty payload. The caller
+  // must delete the returned object. Useful for testing.
+  static Response* CreateEmpty();
+
  private:
+  // Creates a Response message. The internal raw message is NULL.
+  Response();
+
   DISALLOW_COPY_AND_ASSIGN(Response);
 };
 
@@ -181,10 +201,10 @@ class Response : public Message {
 // caller of a method.
 class ErrorResponse: public Message {
  public:
-  // Creates a ErrorResponse message. The internal raw message is NULL.
-  // Classes that implment method calls need to set the raw message once a
-  // response is received from the server. See object_proxy.h.
-  ErrorResponse();
+  // 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);
 
   // Returns a newly created ErrorResponse from the given method call, the
   // error name, and the error message.  The error name looks like
@@ -195,6 +215,9 @@ class ErrorResponse: public Message {
                                        const std::string& error_message);
 
  private:
+  // Creates an ErrorResponse message. The internal raw message is NULL.
+  ErrorResponse();
+
   DISALLOW_COPY_AND_ASSIGN(ErrorResponse);
 };
 
diff --git a/dbus/message_unittest.cc b/dbus/message_unittest.cc
index 6e45e55..473a6ef 100644
--- a/dbus/message_unittest.cc
+++ b/dbus/message_unittest.cc
@@ -12,12 +12,11 @@
 // Test that a byte can be properly written and read. We only have this
 // test for byte, as repeating this for other basic types is too redundant.
 TEST(MessageTest, AppendAndPopByte) {
-  dbus::Message message;
-  message.reset_raw_message(dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_CALL));
-  dbus::MessageWriter writer(&message);
+  scoped_ptr<dbus::Response> message(dbus::Response::CreateEmpty());
+  dbus::MessageWriter writer(message.get());
   writer.AppendByte(123);  // The input is 123.
 
-  dbus::MessageReader reader(&message);
+  dbus::MessageReader reader(message.get());
   ASSERT_TRUE(reader.HasMoreData());  // Should have data to read.
   ASSERT_EQ(dbus::Message::BYTE, reader.GetDataType());
 
@@ -36,9 +35,8 @@ TEST(MessageTest, AppendAndPopByte) {
 
 // Check all basic types can be properly written and read.
 TEST(MessageTest, AppendAndPopBasicDataTypes) {
-  dbus::Message message;
-  message.reset_raw_message(dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_CALL));
-  dbus::MessageWriter writer(&message);
+  scoped_ptr<dbus::Response> message(dbus::Response::CreateEmpty());
+  dbus::MessageWriter writer(message.get());
 
   // Append 0, 1, 2, 3, 4, 5, 6, 7, 8, "string", "/object/path".
   writer.AppendByte(0);
@@ -65,7 +63,7 @@ TEST(MessageTest, AppendAndPopBasicDataTypes) {
   std::string string_value;
   std::string object_path_value;
 
-  dbus::MessageReader reader(&message);
+  dbus::MessageReader reader(message.get());
   ASSERT_TRUE(reader.HasMoreData());
   ASSERT_TRUE(reader.PopByte(&byte_value));
   ASSERT_TRUE(reader.PopBool(&bool_value));
@@ -96,9 +94,8 @@ TEST(MessageTest, AppendAndPopBasicDataTypes) {
 
 // Check all variant types can be properly written and read.
 TEST(MessageTest, AppendAndPopVariantDataTypes) {
-  dbus::Message message;
-  message.reset_raw_message(dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_CALL));
-  dbus::MessageWriter writer(&message);
+  scoped_ptr<dbus::Response> message(dbus::Response::CreateEmpty());
+  dbus::MessageWriter writer(message.get());
 
   // Append 0, 1, 2, 3, 4, 5, 6, 7, 8, "string", "/object/path".
   writer.AppendVariantOfByte(0);
@@ -125,7 +122,7 @@ TEST(MessageTest, AppendAndPopVariantDataTypes) {
   std::string string_value;
   std::string object_path_value;
 
-  dbus::MessageReader reader(&message);
+  dbus::MessageReader reader(message.get());
   ASSERT_TRUE(reader.HasMoreData());
   ASSERT_TRUE(reader.PopVariantOfByte(&byte_value));
   ASSERT_TRUE(reader.PopVariantOfBool(&bool_value));
@@ -155,16 +152,15 @@ TEST(MessageTest, AppendAndPopVariantDataTypes) {
 }
 
 TEST(MessageTest, ArrayOfBytes) {
-  dbus::Message message;
-  message.reset_raw_message(dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_CALL));
-  dbus::MessageWriter writer(&message);
+  scoped_ptr<dbus::Response> message(dbus::Response::CreateEmpty());
+  dbus::MessageWriter writer(message.get());
   std::vector<uint8> bytes;
   bytes.push_back(1);
   bytes.push_back(2);
   bytes.push_back(3);
   writer.AppendArrayOfBytes(bytes.data(), bytes.size());
 
-  dbus::MessageReader reader(&message);
+  dbus::MessageReader reader(message.get());
   uint8* output_bytes = NULL;
   size_t length = 0;
   ASSERT_TRUE(reader.PopArrayOfBytes(&output_bytes, &length));
@@ -176,16 +172,15 @@ TEST(MessageTest, ArrayOfBytes) {
 }
 
 TEST(MessageTest, ArrayOfObjectPaths) {
-  dbus::Message message;
-  message.reset_raw_message(dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_CALL));
-  dbus::MessageWriter writer(&message);
+  scoped_ptr<dbus::Response> message(dbus::Response::CreateEmpty());
+  dbus::MessageWriter writer(message.get());
   std::vector<std::string> object_paths;
   object_paths.push_back("/object/path/1");
   object_paths.push_back("/object/path/2");
   object_paths.push_back("/object/path/3");
   writer.AppendArrayOfObjectPaths(object_paths);
 
-  dbus::MessageReader reader(&message);
+  dbus::MessageReader reader(message.get());
   std::vector<std::string> output_object_paths;
   ASSERT_TRUE(reader.PopArrayOfObjectPaths(&output_object_paths));
   ASSERT_FALSE(reader.HasMoreData());
@@ -199,19 +194,18 @@ TEST(MessageTest, ArrayOfObjectPaths) {
 // test for array, as repeating this for other container types is too
 // redundant.
 TEST(MessageTest, OpenArrayAndPopArray) {
-  dbus::Message message;
-  message.reset_raw_message(dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_CALL));
-  dbus::MessageWriter writer(&message);
-  dbus::MessageWriter array_writer(&message);
+  scoped_ptr<dbus::Response> message(dbus::Response::CreateEmpty());
+  dbus::MessageWriter writer(message.get());
+  dbus::MessageWriter array_writer(message.get());
   writer.OpenArray("s", &array_writer);  // Open an array of strings.
   array_writer.AppendString("foo");
   array_writer.AppendString("bar");
   array_writer.AppendString("baz");
   writer.CloseContainer(&array_writer);
 
-  dbus::MessageReader reader(&message);
+  dbus::MessageReader reader(message.get());
   ASSERT_EQ(dbus::Message::ARRAY, reader.GetDataType());
-  dbus::MessageReader array_reader(&message);
+  dbus::MessageReader array_reader(message.get());
   ASSERT_TRUE(reader.PopArray(&array_reader));
   ASSERT_FALSE(reader.HasMoreData());  // Should not have more data to read.
 
@@ -229,17 +223,16 @@ TEST(MessageTest, OpenArrayAndPopArray) {
 // Create a complex message using array, struct, variant, dict entry, and
 // make sure it can be read properly.
 TEST(MessageTest, CreateComplexMessageAndReadIt) {
-  dbus::Message message;
-  message.reset_raw_message(dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_CALL));
-  dbus::MessageWriter writer(&message);
+  scoped_ptr<dbus::Response> message(dbus::Response::CreateEmpty());
+  dbus::MessageWriter writer(message.get());
   {
-    dbus::MessageWriter array_writer(&message);
+    dbus::MessageWriter array_writer(message.get());
     // Open an array of variants.
     writer.OpenArray("v", &array_writer);
     {
       // The first value in the array.
       {
-        dbus::MessageWriter variant_writer(&message);
+        dbus::MessageWriter variant_writer(message.get());
         // Open a variant of a boolean.
         array_writer.OpenVariant("b", &variant_writer);
         variant_writer.AppendBool(true);
@@ -248,11 +241,11 @@ TEST(MessageTest, CreateComplexMessageAndReadIt) {
 
       // The second value in the array.
       {
-        dbus::MessageWriter variant_writer(&message);
+        dbus::MessageWriter variant_writer(message.get());
         // Open a variant of a struct that contains a string and an int32.
         array_writer.OpenVariant("(si)", &variant_writer);
         {
-          dbus::MessageWriter struct_writer(&message);
+          dbus::MessageWriter struct_writer(message.get());
           variant_writer.OpenStruct(&struct_writer);
           struct_writer.AppendString("string");
           struct_writer.AppendInt32(123);
@@ -263,16 +256,16 @@ TEST(MessageTest, CreateComplexMessageAndReadIt) {
 
       // The third value in the array.
       {
-        dbus::MessageWriter variant_writer(&message);
+        dbus::MessageWriter variant_writer(message.get());
         // Open a variant of an array of string-to-int64 dict entries.
         array_writer.OpenVariant("a{sx}", &variant_writer);
         {
           // Opens an array of string-to-int64 dict entries.
-          dbus::MessageWriter dict_array_writer(&message);
+          dbus::MessageWriter dict_array_writer(message.get());
           variant_writer.OpenArray("{sx}", &dict_array_writer);
           {
             // Opens a string-to-int64 dict entries.
-            dbus::MessageWriter dict_entry_writer(&message);
+            dbus::MessageWriter dict_entry_writer(message.get());
             dict_array_writer.OpenDictEntry(&dict_entry_writer);
             dict_entry_writer.AppendString("foo");
             dict_entry_writer.AppendInt64(GG_INT64_C(1234567890123456789));
@@ -286,7 +279,8 @@ TEST(MessageTest, CreateComplexMessageAndReadIt) {
     writer.CloseContainer(&array_writer);
   }
   // What we have created looks like this:
-  EXPECT_EQ("signature: av\n"
+  EXPECT_EQ("message_type: MESSAGE_METHOD_RETURN\n"
+            "signature: av\n"
             "\n"
             "array [\n"
             "  variant     bool true\n"
@@ -301,10 +295,10 @@ TEST(MessageTest, CreateComplexMessageAndReadIt) {
             "      }\n"
             "    ]\n"
             "]\n",
-            message.ToString());
+            message->ToString());
 
-  dbus::MessageReader reader(&message);
-  dbus::MessageReader array_reader(&message);
+  dbus::MessageReader reader(message.get());
+  dbus::MessageReader array_reader(message.get());
   ASSERT_TRUE(reader.PopArray(&array_reader));
 
   // The first value in the array.
@@ -314,10 +308,10 @@ TEST(MessageTest, CreateComplexMessageAndReadIt) {
 
   // The second value in the array.
   {
-    dbus::MessageReader variant_reader(&message);
+    dbus::MessageReader variant_reader(message.get());
     ASSERT_TRUE(array_reader.PopVariant(&variant_reader));
     {
-      dbus::MessageReader struct_reader(&message);
+      dbus::MessageReader struct_reader(message.get());
       ASSERT_TRUE(variant_reader.PopStruct(&struct_reader));
       std::string string_value;
       ASSERT_TRUE(struct_reader.PopString(&string_value));
@@ -332,13 +326,13 @@ TEST(MessageTest, CreateComplexMessageAndReadIt) {
 
   // The third value in the array.
   {
-    dbus::MessageReader variant_reader(&message);
+    dbus::MessageReader variant_reader(message.get());
     ASSERT_TRUE(array_reader.PopVariant(&variant_reader));
     {
-      dbus::MessageReader dict_array_reader(&message);
+      dbus::MessageReader dict_array_reader(message.get());
       ASSERT_TRUE(variant_reader.PopArray(&dict_array_reader));
       {
-        dbus::MessageReader dict_entry_reader(&message);
+        dbus::MessageReader dict_entry_reader(message.get());
         ASSERT_TRUE(dict_array_reader.PopDictEntry(&dict_entry_reader));
         std::string string_value;
         ASSERT_TRUE(dict_entry_reader.PopString(&string_value));
@@ -355,23 +349,19 @@ TEST(MessageTest, CreateComplexMessageAndReadIt) {
   ASSERT_FALSE(reader.HasMoreData());
 }
 
-TEST(MessageTest, Message) {
-  dbus::Message message;
-  EXPECT_TRUE(message.raw_message() == NULL);
-  EXPECT_EQ(dbus::Message::MESSAGE_INVALID, message.GetMessageType());
-}
-
 TEST(MessageTest, MethodCall) {
   dbus::MethodCall method_call("com.example.Interface", "SomeMethod");
   EXPECT_TRUE(method_call.raw_message() != NULL);
   EXPECT_EQ(dbus::Message::MESSAGE_METHOD_CALL, method_call.GetMessageType());
+  EXPECT_EQ("MESSAGE_METHOD_CALL", method_call.GetMessageTypeAsString());
   method_call.SetDestination("com.example.Service");
   method_call.SetPath("/com/example/Object");
 
   dbus::MessageWriter writer(&method_call);
   writer.AppendString("payload");
 
-  EXPECT_EQ("destination: com.example.Service\n"
+  EXPECT_EQ("message_type: MESSAGE_METHOD_CALL\n"
+            "destination: com.example.Service\n"
             "path: /com/example/Object\n"
             "interface: com.example.Interface\n"
             "member: SomeMethod\n"
@@ -396,12 +386,14 @@ TEST(MessageTest, Signal) {
   dbus::Signal signal("com.example.Interface", "SomeSignal");
   EXPECT_TRUE(signal.raw_message() != NULL);
   EXPECT_EQ(dbus::Message::MESSAGE_SIGNAL, signal.GetMessageType());
+  EXPECT_EQ("MESSAGE_SIGNAL", signal.GetMessageTypeAsString());
   signal.SetPath("/com/example/Object");
 
   dbus::MessageWriter writer(&signal);
   writer.AppendString("payload");
 
-  EXPECT_EQ("path: /com/example/Object\n"
+  EXPECT_EQ("message_type: MESSAGE_SIGNAL\n"
+            "path: /com/example/Object\n"
             "interface: com.example.Interface\n"
             "member: SomeSignal\n"
             "signature: s\n"
@@ -422,11 +414,10 @@ TEST(MessageTest, Signal_FromRawMessage) {
 }
 
 TEST(MessageTest, Response) {
-  dbus::Response response;
-  EXPECT_TRUE(response.raw_message() == NULL);
-  response.reset_raw_message(
-      dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_RETURN));
-  EXPECT_EQ(dbus::Message::MESSAGE_METHOD_RETURN, response.GetMessageType());
+  scoped_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
+  EXPECT_TRUE(response->raw_message());
+  EXPECT_EQ(dbus::Message::MESSAGE_METHOD_RETURN, response->GetMessageType());
+  EXPECT_EQ("MESSAGE_METHOD_RETURN", response->GetMessageTypeAsString());
 }
 
 TEST(MessageTest, Response_FromMethodCall) {
@@ -437,18 +428,11 @@ TEST(MessageTest, Response_FromMethodCall) {
   scoped_ptr<dbus::Response> response(
       dbus::Response::FromMethodCall(&method_call));
   EXPECT_EQ(dbus::Message::MESSAGE_METHOD_RETURN, response->GetMessageType());
+  EXPECT_EQ("MESSAGE_METHOD_RETURN", response->GetMessageTypeAsString());
   // The serial should be copied to the reply serial.
   EXPECT_EQ(kSerial, response->GetReplySerial());
 }
 
-TEST(MessageTest, ErrorResponse) {
-  dbus::ErrorResponse error_response;
-  EXPECT_TRUE(error_response.raw_message() == NULL);
-  error_response.reset_raw_message(
-      dbus_message_new(DBUS_MESSAGE_TYPE_ERROR));
-  EXPECT_EQ(dbus::Message::MESSAGE_ERROR, error_response.GetMessageType());
-}
-
 TEST(MessageTest, ErrorResponse_FromMethodCall) {
   const uint32 kSerial = 123;
 const char kErrorMessage[] = "error message";
@@ -461,6 +445,7 @@ const char kErrorMessage[] = "error message";
                                           DBUS_ERROR_FAILED,
                                           kErrorMessage));
   EXPECT_EQ(dbus::Message::MESSAGE_ERROR, error_response->GetMessageType());
+  EXPECT_EQ("MESSAGE_ERROR", error_response->GetMessageTypeAsString());
   // The serial should be copied to the reply serial.
   EXPECT_EQ(kSerial, error_response->GetReplySerial());
 
@@ -471,39 +456,33 @@ const char kErrorMessage[] = "error message";
   EXPECT_EQ(kErrorMessage, error_message);
 }
 
-TEST(MessageTest, ToString_EmptyMessage) {
-  dbus::Message message;
-  EXPECT_EQ("", message.ToString());
-}
-
 TEST(MessageTest, GetAndSetHeaders) {
-  dbus::Message message;
-  message.reset_raw_message(dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_CALL));
-
-  EXPECT_EQ("", message.GetDestination());
-  EXPECT_EQ("", message.GetPath());
-  EXPECT_EQ("", message.GetInterface());
-  EXPECT_EQ("", message.GetMember());
-  EXPECT_EQ("", message.GetErrorName());
-  EXPECT_EQ("", message.GetSender());
-  EXPECT_EQ(0U, message.GetSerial());
-  EXPECT_EQ(0U, message.GetReplySerial());
-
-  message.SetDestination("org.chromium.destination");
-  message.SetPath("/org/chromium/path");
-  message.SetInterface("org.chromium.interface");
-  message.SetMember("member");
-  message.SetErrorName("org.chromium.error");
-  message.SetSender(":1.2");
-  message.SetSerial(123);
-  message.SetReplySerial(456);
-
-  EXPECT_EQ("org.chromium.destination", message.GetDestination());
-  EXPECT_EQ("/org/chromium/path", message.GetPath());
-  EXPECT_EQ("org.chromium.interface", message.GetInterface());
-  EXPECT_EQ("member", message.GetMember());
-  EXPECT_EQ("org.chromium.error", message.GetErrorName());
-  EXPECT_EQ(":1.2", message.GetSender());
-  EXPECT_EQ(123U, message.GetSerial());
-  EXPECT_EQ(456U, message.GetReplySerial());
+  scoped_ptr<dbus::Response> message(dbus::Response::CreateEmpty());
+
+  EXPECT_EQ("", message->GetDestination());
+  EXPECT_EQ("", message->GetPath());
+  EXPECT_EQ("", message->GetInterface());
+  EXPECT_EQ("", message->GetMember());
+  EXPECT_EQ("", message->GetErrorName());
+  EXPECT_EQ("", message->GetSender());
+  EXPECT_EQ(0U, message->GetSerial());
+  EXPECT_EQ(0U, message->GetReplySerial());
+
+  message->SetDestination("org.chromium.destination");
+  message->SetPath("/org/chromium/path");
+  message->SetInterface("org.chromium.interface");
+  message->SetMember("member");
+  message->SetErrorName("org.chromium.error");
+  message->SetSender(":1.2");
+  message->SetSerial(123);
+  message->SetReplySerial(456);
+
+  EXPECT_EQ("org.chromium.destination", message->GetDestination());
+  EXPECT_EQ("/org/chromium/path", message->GetPath());
+  EXPECT_EQ("org.chromium.interface", message->GetInterface());
+  EXPECT_EQ("member", message->GetMember());
+  EXPECT_EQ("org.chromium.error", message->GetErrorName());
+  EXPECT_EQ(":1.2", message->GetSender());
+  EXPECT_EQ(123U, message->GetSerial());
+  EXPECT_EQ(456U, message->GetReplySerial());
 }
diff --git a/dbus/object_proxy.cc b/dbus/object_proxy.cc
index 0ab3ce5..7452b1d 100644
--- a/dbus/object_proxy.cc
+++ b/dbus/object_proxy.cc
@@ -44,13 +44,12 @@ 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.
-bool ObjectProxy::CallMethodAndBlock(MethodCall* method_call,
-                                     int timeout_ms,
-                                     Response* response) {
+Response* ObjectProxy::CallMethodAndBlock(MethodCall* method_call,
+                                          int timeout_ms) {
   bus_->AssertOnDBusThread();
 
   if (!bus_->Connect())
-    return false;
+    return NULL;
 
   method_call->SetDestination(service_name_);
   method_call->SetPath(object_path_);
@@ -65,11 +64,10 @@ bool ObjectProxy::CallMethodAndBlock(MethodCall* method_call,
   if (!response_message) {
     LOG(ERROR) << "Failed to call method: "
                << (error.is_set() ? error.message() : "");
-    return false;
+    return NULL;
   }
-  response->reset_raw_message(response_message);
 
-  return true;
+  return Response::FromRawMessage(response_message);
 }
 
 void ObjectProxy::CallMethod(MethodCall* method_call,
@@ -184,50 +182,45 @@ void ObjectProxy::OnPendingCallIsComplete(DBusPendingCall* pending_call,
   bus_->AssertOnDBusThread();
 
   DBusMessage* response_message = dbus_pending_call_steal_reply(pending_call);
-
-  if (!response_message) {
-    // This shouldn't happen but just in case.
-    LOG(ERROR) << "The response message is not received for some reason";
-    Response* response = NULL;
-    base::Closure task = base::Bind(&ObjectProxy::RunResponseCallback,
-                                    this,
-                                    response_callback,
-                                    response);
-    bus_->PostTaskToOriginThread(FROM_HERE, task);
-    return;
-  }
-
-  // The response message will be deleted in RunResponseCallback().
-  Response* response = new Response;
-  response->reset_raw_message(response_message);
+  // |response_message| will be unref'ed in RunResponseCallback().
+  // Bind() won't compile if we pass response_message as-is.
+  // See CallMethod() for details.
   base::Closure task = base::Bind(&ObjectProxy::RunResponseCallback,
                                   this,
                                   response_callback,
-                                  response);
+                                  static_cast<void*>(response_message));
   bus_->PostTaskToOriginThread(FROM_HERE, task);
 }
 
 void ObjectProxy::RunResponseCallback(ResponseCallback response_callback,
-                                      Response* response) {
+                                      void* in_response_message) {
   bus_->AssertOnOriginThread();
+  DBusMessage* response_message =
+      static_cast<DBusMessage*>(in_response_message);
 
-  if (!response) {
+  if (!response_message) {
     // The response is not received.
     response_callback.Run(NULL);
-  } else if (response->GetMessageType() == Message::MESSAGE_ERROR) {
+  } else if (dbus_message_get_type(response_message) ==
+             DBUS_MESSAGE_TYPE_ERROR) {
+    // This will take |response_message| and release (unref) it.
+    scoped_ptr<dbus::ErrorResponse> error_response(
+        dbus::ErrorResponse::FromRawMessage(response_message));
     // Error message may contain the error message as string.
-    dbus::MessageReader reader(response);
+    dbus::MessageReader reader(error_response.get());
     std::string error_message;
     reader.PopString(&error_message);
-    LOG(ERROR) << "Failed to call method: " << response->GetErrorName()
+    LOG(ERROR) << "Failed to call method: " << error_response->GetErrorName()
                << ": " << error_message;
     // We don't give the error message to the callback.
     response_callback.Run(NULL);
   } else {
+    // This will take |response_message| and release (unref) it.
+    scoped_ptr<dbus::Response> response(
+        dbus::Response::FromRawMessage(response_message));
     // The response is successfuly received.
-    response_callback.Run(response);
+    response_callback.Run(response.get());
   }
-  delete response;  // It's ok to delete NULL.
 }
 
 void ObjectProxy::OnPendingCallIsCompleteThunk(DBusPendingCall* pending_call,
diff --git a/dbus/object_proxy.h b/dbus/object_proxy.h
index 70e412df..ae9bd9a3 100644
--- a/dbus/object_proxy.h
+++ b/dbus/object_proxy.h
@@ -60,12 +60,11 @@ class ObjectProxy : public base::RefCountedThreadSafe<ObjectProxy> {
       OnConnectedCallback;
 
   // Calls the method of the remote object and blocks until the response
-  // is returned.
+  // is returned. Returns NULL on error.
   //
   // BLOCKING CALL.
-  virtual bool CallMethodAndBlock(MethodCall* method_call,
-                                  int timeout_ms,
-                                  Response* response);
+  virtual Response* CallMethodAndBlock(MethodCall* method_call,
+                                       int timeout_ms);
 
   // Requests to call the method of the remote object.
   //
@@ -132,7 +131,7 @@ class ObjectProxy : public base::RefCountedThreadSafe<ObjectProxy> {
 
   // Runs the response callback with the given response object.
   void RunResponseCallback(ResponseCallback response_callback,
-                           Response* response);
+                           void* response_message);
 
   // Redirects the function call to OnPendingCallIsComplete().
   static void OnPendingCallIsCompleteThunk(DBusPendingCall* pending_call,