Refactor Pickle Read methods to use higher performance PickleIterator.

There was a lot of redundant error checking and initialization code in all Pickle Read methods because of the void** iterator type. This change replaces the void* iterator with PickleIterator, which encapsulates the read pointer so that less error checking and initialization code is needed for reading.

PickleIterator has all the necessary data to do the actual reading. The advantage of having it provide Read methods (as opposed to leaving them solely in the Pickle interface) is that the callers do not need to pass around the const Pickle* once they have a PickleIterator.

Followup CLs will refactor the call sites to remove const Pickle* arguments where they are now unnecessary. Then the Pickle::Read* methods can be removed entirely.

The alternative approach would have been to change the Pickle::Read methods to non-const and remove the iterator parameter (making Read methods advance an internal read pointer). Unfortunately, the const Read with iterator design is entrenched throughout the chromium code, making this a much more complex change with the same performance outcome.

BUG=13108

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

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

15 files changed, 139 insertions, 106 deletions

diff --git a/ipc/ipc_channel_posix.cc b/ipc/ipc_channel_posix.cc
index 1960cb0..4d6b8a02 100644
--- a/ipc/ipc_channel_posix.cc
+++ b/ipc/ipc_channel_posix.cc
@@ -1126,7 +1126,7 @@ void Channel::ChannelImpl::ClearInputFDs() {
 
 void Channel::ChannelImpl::HandleHelloMessage(const Message& msg) {
   // The Hello message contains only the process id.
-  void *iter = NULL;
+  PickleIterator iter(msg);
   int pid;
   if (!msg.ReadInt(&iter, &pid))
     NOTREACHED();
diff --git a/ipc/ipc_fuzzing_tests.cc b/ipc/ipc_fuzzing_tests.cc
index 05d177b..aa1fa5f 100644
--- a/ipc/ipc_fuzzing_tests.cc
+++ b/ipc/ipc_fuzzing_tests.cc
@@ -44,7 +44,7 @@ TEST(IPCMessageIntegrity, ReadBeyondBufferStr) {
   EXPECT_TRUE(m.WriteInt(v1));
   EXPECT_TRUE(m.WriteInt(v2));
 
-  void* iter = NULL;
+  PickleIterator iter(m);
   std::string vs;
   EXPECT_FALSE(m.ReadString(&iter, &vs));
 }
@@ -57,7 +57,7 @@ TEST(IPCMessageIntegrity, ReadBeyondBufferWStr) {
   EXPECT_TRUE(m.WriteInt(v1));
   EXPECT_TRUE(m.WriteInt(v2));
 
-  void* iter = NULL;
+  PickleIterator iter(m);
   std::wstring vs;
   EXPECT_FALSE(m.ReadWString(&iter, &vs));
 }
@@ -68,7 +68,7 @@ TEST(IPCMessageIntegrity, ReadBytesBadIterator) {
   EXPECT_TRUE(m.WriteInt(1));
   EXPECT_TRUE(m.WriteInt(2));
 
-  void* iter = NULL;
+  PickleIterator iter(m);
   const char* data = NULL;
   EXPECT_TRUE(m.ReadBytes(&iter, &data, sizeof(int)));
 }
@@ -84,7 +84,7 @@ TEST(IPCMessageIntegrity, ReadVectorNegativeSize) {
   EXPECT_TRUE(m.WriteInt(3));
 
   std::vector<double> vec;
-  void* iter = 0;
+  PickleIterator iter(m);
   EXPECT_FALSE(ReadParam(&m, &iter, &vec));
 }
 
@@ -97,7 +97,7 @@ TEST(IPCMessageIntegrity, ReadVectorTooLarge1) {
   EXPECT_TRUE(m.WriteInt64(2));
 
   std::vector<int64> vec;
-  void* iter = 0;
+  PickleIterator iter(m);
   EXPECT_FALSE(ReadParam(&m, &iter, &vec));
 }
 
@@ -111,7 +111,7 @@ TEST(IPCMessageIntegrity, ReadVectorTooLarge2) {
   EXPECT_TRUE(m.WriteInt64(2));
 
   std::vector<int64> vec;
-  void* iter = 0;
+  PickleIterator iter(m);
   EXPECT_FALSE(ReadParam(&m, &iter, &vec));
 }
 
@@ -212,7 +212,7 @@ class FuzzerClientListener : public SimpleListener {
       return false;
     int msg_value1 = 0;
     int msg_value2 = 0;
-    void* iter = NULL;
+    PickleIterator iter(*last_msg_);
     if (!last_msg_->ReadInt(&iter, &msg_value1))
       return false;
     if (!last_msg_->ReadInt(&iter, &msg_value2))
diff --git a/ipc/ipc_logging.cc b/ipc/ipc_logging.cc
index d50766c..b9ca828 100644
--- a/ipc/ipc_logging.cc
+++ b/ipc/ipc_logging.cc
@@ -109,7 +109,7 @@ void Logging::SetIPCSender(IPC::Message::Sender* sender) {
 
 void Logging::OnReceivedLoggingMessage(const Message& message) {
   std::vector<LogData> data;
-  void* iter = NULL;
+  PickleIterator iter(message);
   if (!ReadParam(&message, &iter, &data))
     return;
 
diff --git a/ipc/ipc_message.cc b/ipc/ipc_message.cc
index 703588d0..feec91a 100644
--- a/ipc/ipc_message.cc
+++ b/ipc/ipc_message.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -100,7 +100,7 @@ bool Message::WriteFileDescriptor(const base::FileDescriptor& descriptor) {
   }
 }
 
-bool Message::ReadFileDescriptor(void** iter,
+bool Message::ReadFileDescriptor(PickleIterator* iter,
                                  base::FileDescriptor* descriptor) const {
   int descriptor_index;
   if (!ReadInt(iter, &descriptor_index))
diff --git a/ipc/ipc_message.h b/ipc/ipc_message.h
index b630432..2598c8c 100644
--- a/ipc/ipc_message.h
+++ b/ipc/ipc_message.h
@@ -180,7 +180,8 @@ class IPC_EXPORT Message : public Pickle {
   bool WriteFileDescriptor(const base::FileDescriptor& descriptor);
   // Get a file descriptor from the message. Returns false on error.
   //   iter: a Pickle iterator to the current location in the message.
-  bool ReadFileDescriptor(void** iter, base::FileDescriptor* descriptor) const;
+  bool ReadFileDescriptor(PickleIterator* iter,
+                          base::FileDescriptor* descriptor) const;
 #endif
 
 #ifdef IPC_MESSAGE_LOG_ENABLED
diff --git a/ipc/ipc_message_unittest.cc b/ipc/ipc_message_unittest.cc
index 7339e30..2f50ee3 100644
--- a/ipc/ipc_message_unittest.cc
+++ b/ipc/ipc_message_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -21,7 +21,7 @@ TEST(IPCMessageTest, ListValue) {
   IPC::WriteParam(&msg, input);
 
   ListValue output;
-  void* iter = NULL;
+  PickleIterator iter(msg);
   EXPECT_TRUE(IPC::ReadParam(&msg, &iter, &output));
 
   EXPECT_TRUE(input.Equals(&output));
@@ -29,7 +29,7 @@ TEST(IPCMessageTest, ListValue) {
   // Also test the corrupt case.
   IPC::Message bad_msg(1, 2, IPC::Message::PRIORITY_NORMAL);
   bad_msg.WriteInt(99);
-  iter = NULL;
+  iter = PickleIterator(bad_msg);
   EXPECT_FALSE(IPC::ReadParam(&bad_msg, &iter, &output));
 }
 
@@ -56,7 +56,7 @@ TEST(IPCMessageTest, DictionaryValue) {
   IPC::WriteParam(&msg, input);
 
   DictionaryValue output;
-  void* iter = NULL;
+  PickleIterator iter(msg);
   EXPECT_TRUE(IPC::ReadParam(&msg, &iter, &output));
 
   EXPECT_TRUE(input.Equals(&output));
@@ -64,6 +64,6 @@ TEST(IPCMessageTest, DictionaryValue) {
   // Also test the corrupt case.
   IPC::Message bad_msg(1, 2, IPC::Message::PRIORITY_NORMAL);
   bad_msg.WriteInt(99);
-  iter = NULL;
+  iter = PickleIterator(bad_msg);
   EXPECT_FALSE(IPC::ReadParam(&bad_msg, &iter, &output));
 }
diff --git a/ipc/ipc_message_utils.cc b/ipc/ipc_message_utils.cc
index 03d326b..72437a9 100644
--- a/ipc/ipc_message_utils.cc
+++ b/ipc/ipc_message_utils.cc
@@ -23,7 +23,7 @@ const int kMaxRecursionDepth = 100;
 
 // Value serialization
 
-static bool ReadValue(const Message* m, void** iter, Value** value,
+static bool ReadValue(const Message* m, PickleIterator* iter, Value** value,
                       int recursion);
 
 static void WriteValue(Message* m, const Value* value, int recursion) {
@@ -102,7 +102,7 @@ static void WriteValue(Message* m, const Value* value, int recursion) {
 
 // Helper for ReadValue that reads a DictionaryValue into a pre-allocated
 // object.
-static bool ReadDictionaryValue(const Message* m, void** iter,
+static bool ReadDictionaryValue(const Message* m, PickleIterator* iter,
                                 DictionaryValue* value, int recursion) {
   int size;
   if (!ReadParam(m, iter, &size))
@@ -122,7 +122,7 @@ static bool ReadDictionaryValue(const Message* m, void** iter,
 
 // Helper for ReadValue that reads a ReadListValue into a pre-allocated
 // object.
-static bool ReadListValue(const Message* m, void** iter,
+static bool ReadListValue(const Message* m, PickleIterator* iter,
                           ListValue* value, int recursion) {
   int size;
   if (!ReadParam(m, iter, &size))
@@ -138,7 +138,7 @@ static bool ReadListValue(const Message* m, void** iter,
   return true;
 }
 
-static bool ReadValue(const Message* m, void** iter, Value** value,
+static bool ReadValue(const Message* m, PickleIterator* iter, Value** value,
                       int recursion) {
   if (recursion > kMaxRecursionDepth) {
     LOG(WARNING) << "Max recursion depth hit in ReadValue.";
@@ -238,7 +238,7 @@ void ParamTraits<unsigned short>::Write(Message* m, const param_type& p) {
   m->WriteBytes(&p, sizeof(param_type));
 }
 
-bool ParamTraits<unsigned short>::Read(const Message* m, void** iter,
+bool ParamTraits<unsigned short>::Read(const Message* m, PickleIterator* iter,
                                        param_type* r) {
   const char* data;
   if (!m->ReadBytes(iter, &data, sizeof(param_type)))
@@ -255,7 +255,7 @@ void ParamTraits<base::Time>::Write(Message* m, const param_type& p) {
   ParamTraits<int64>::Write(m, p.ToInternalValue());
 }
 
-bool ParamTraits<base::Time>::Read(const Message* m, void** iter,
+bool ParamTraits<base::Time>::Read(const Message* m, PickleIterator* iter,
                                    param_type* r) {
   int64 value;
   if (!ParamTraits<int64>::Read(m, iter, &value))
@@ -273,7 +273,7 @@ void ParamTraits<base::TimeDelta> ::Write(Message* m, const param_type& p) {
 }
 
 bool ParamTraits<base::TimeDelta> ::Read(const Message* m,
-                                         void** iter,
+                                         PickleIterator* iter,
                                          param_type* r) {
   int64 value;
   bool ret = ParamTraits<int64> ::Read(m, iter, &value);
@@ -292,7 +292,7 @@ void ParamTraits<base::TimeTicks> ::Write(Message* m, const param_type& p) {
 }
 
 bool ParamTraits<base::TimeTicks> ::Read(const Message* m,
-                                         void** iter,
+                                         PickleIterator* iter,
                                          param_type* r) {
   int64 value;
   bool ret = ParamTraits<int64> ::Read(m, iter, &value);
@@ -311,7 +311,7 @@ void ParamTraits<DictionaryValue>::Write(Message* m, const param_type& p) {
 }
 
 bool ParamTraits<DictionaryValue>::Read(
-    const Message* m, void** iter, param_type* r) {
+    const Message* m, PickleIterator* iter, param_type* r) {
   int type;
   if (!ReadParam(m, iter, &type) || type != Value::TYPE_DICTIONARY)
     return false;
@@ -330,7 +330,7 @@ void ParamTraits<ListValue>::Write(Message* m, const param_type& p) {
 }
 
 bool ParamTraits<ListValue>::Read(
-    const Message* m, void** iter, param_type* r) {
+    const Message* m, PickleIterator* iter, param_type* r) {
   int type;
   if (!ReadParam(m, iter, &type) || type != Value::TYPE_LIST)
     return false;
@@ -353,7 +353,7 @@ void ParamTraits<NullableString16>::Write(Message* m, const param_type& p) {
   WriteParam(m, p.is_null());
 }
 
-bool ParamTraits<NullableString16>::Read(const Message* m, void** iter,
+bool ParamTraits<NullableString16>::Read(const Message* m, PickleIterator* iter,
                                          param_type* r) {
   string16 string;
   if (!ReadParam(m, iter, &string))
@@ -384,7 +384,9 @@ void ParamTraits<FilePath>::Write(Message* m, const param_type& p) {
   ParamTraits<FilePath::StringType>::Write(m, p.value());
 }
 
-bool ParamTraits<FilePath>::Read(const Message* m, void** iter, param_type* r) {
+bool ParamTraits<FilePath>::Read(const Message* m,
+                                 PickleIterator* iter,
+                                 param_type* r) {
   FilePath::StringType value;
   if (!ParamTraits<FilePath::StringType>::Read(m, iter, &value))
     return false;
@@ -407,7 +409,8 @@ void ParamTraits<base::FileDescriptor>::Write(Message* m, const param_type& p) {
   }
 }
 
-bool ParamTraits<base::FileDescriptor>::Read(const Message* m, void** iter,
+bool ParamTraits<base::FileDescriptor>::Read(const Message* m,
+                                             PickleIterator* iter,
                                              param_type* r) {
   bool valid;
   if (!ReadParam(m, iter, &valid))
@@ -443,7 +446,8 @@ void ParamTraits<IPC::ChannelHandle>::Write(Message* m, const param_type& p) {
 #endif
 }
 
-bool ParamTraits<IPC::ChannelHandle>::Read(const Message* m, void** iter,
+bool ParamTraits<IPC::ChannelHandle>::Read(const Message* m,
+                                           PickleIterator* iter,
                                            param_type* r) {
   return ReadParam(m, iter, &r->name)
 #if defined(OS_POSIX)
@@ -484,7 +488,9 @@ void ParamTraits<LogData>::Write(Message* m, const param_type& p) {
   WriteParam(m, p.params);
 }
 
-bool ParamTraits<LogData>::Read(const Message* m, void** iter, param_type* r) {
+bool ParamTraits<LogData>::Read(const Message* m,
+                                PickleIterator* iter,
+                                param_type* r) {
   return
       ReadParam(m, iter, &r->channel) &&
       ReadParam(m, iter, &r->routing_id) &&
diff --git a/ipc/ipc_message_utils.h b/ipc/ipc_message_utils.h
index 1a7555b..3908b12 100644
--- a/ipc/ipc_message_utils.h
+++ b/ipc/ipc_message_utils.h
@@ -124,34 +124,33 @@ struct ChannelHandle;
 
 class MessageIterator {
  public:
-  explicit MessageIterator(const Message& m) : msg_(m), iter_(NULL) {
+  explicit MessageIterator(const Message& m) : iter_(m) {
   }
   int NextInt() const {
     int val = -1;
-    if (!msg_.ReadInt(&iter_, &val))
+    if (!iter_.ReadInt(&val))
       NOTREACHED();
     return val;
   }
   const std::string NextString() const {
     std::string val;
-    if (!msg_.ReadString(&iter_, &val))
+    if (!iter_.ReadString(&val))
       NOTREACHED();
     return val;
   }
   const std::wstring NextWString() const {
     std::wstring val;
-    if (!msg_.ReadWString(&iter_, &val))
+    if (!iter_.ReadWString(&val))
       NOTREACHED();
     return val;
   }
   void NextData(const char** data, int* length) const {
-    if (!msg_.ReadData(&iter_, data, length)) {
+    if (!iter_.ReadData(data, length)) {
       NOTREACHED();
     }
   }
  private:
-  const Message& msg_;
-  mutable void* iter_;
+  mutable PickleIterator iter_;
 };
 
 //-----------------------------------------------------------------------------
@@ -170,7 +169,8 @@ static inline void WriteParam(Message* m, const P& p) {
 }
 
 template <class P>
-static inline bool WARN_UNUSED_RESULT ReadParam(const Message* m, void** iter,
+static inline bool WARN_UNUSED_RESULT ReadParam(const Message* m,
+                                                PickleIterator* iter,
                                                 P* p) {
   typedef typename SimilarTypeTraits<P>::Type Type;
   return ParamTraits<Type>::Read(m, iter, reinterpret_cast<Type* >(p));
@@ -188,7 +188,8 @@ struct ParamTraits<bool> {
   static void Write(Message* m, const param_type& p) {
     m->WriteBool(p);
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r) {
     return m->ReadBool(iter, r);
   }
   static void Log(const param_type& p, std::string* l) {
@@ -202,7 +203,8 @@ struct ParamTraits<int> {
   static void Write(Message* m, const param_type& p) {
     m->WriteInt(p);
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r) {
     return m->ReadInt(iter, r);
   }
   IPC_EXPORT static void Log(const param_type& p, std::string* l);
@@ -214,7 +216,8 @@ struct ParamTraits<unsigned int> {
   static void Write(Message* m, const param_type& p) {
     m->WriteInt(p);
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r) {
     return m->ReadInt(iter, reinterpret_cast<int*>(r));
   }
   IPC_EXPORT static void Log(const param_type& p, std::string* l);
@@ -226,7 +229,8 @@ struct ParamTraits<long> {
   static void Write(Message* m, const param_type& p) {
     m->WriteLong(p);
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r) {
     return m->ReadLong(iter, r);
   }
   IPC_EXPORT static void Log(const param_type& p, std::string* l);
@@ -238,7 +242,8 @@ struct ParamTraits<unsigned long> {
   static void Write(Message* m, const param_type& p) {
     m->WriteLong(p);
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r) {
     return m->ReadLong(iter, reinterpret_cast<long*>(r));
   }
   IPC_EXPORT static void Log(const param_type& p, std::string* l);
@@ -250,7 +255,8 @@ struct ParamTraits<long long> {
   static void Write(Message* m, const param_type& p) {
     m->WriteInt64(static_cast<int64>(p));
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r) {
     return m->ReadInt64(iter, reinterpret_cast<int64*>(r));
   }
   IPC_EXPORT static void Log(const param_type& p, std::string* l);
@@ -262,7 +268,8 @@ struct ParamTraits<unsigned long long> {
   static void Write(Message* m, const param_type& p) {
     m->WriteInt64(p);
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r) {
     return m->ReadInt64(iter, reinterpret_cast<int64*>(r));
   }
   IPC_EXPORT static void Log(const param_type& p, std::string* l);
@@ -272,7 +279,7 @@ template <>
 struct IPC_EXPORT ParamTraits<unsigned short> {
   typedef unsigned short param_type;
   static void Write(Message* m, const param_type& p);
-  static bool Read(const Message* m, void** iter, param_type* r);
+  static bool Read(const Message* m, PickleIterator* iter, param_type* r);
   static void Log(const param_type& p, std::string* l);
 };
 
@@ -285,7 +292,8 @@ struct ParamTraits<float> {
   static void Write(Message* m, const param_type& p) {
     m->WriteData(reinterpret_cast<const char*>(&p), sizeof(param_type));
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r) {
     const char *data;
     int data_size;
     if (!m->ReadData(iter, &data, &data_size) ||
@@ -307,7 +315,8 @@ struct ParamTraits<double> {
   static void Write(Message* m, const param_type& p) {
     m->WriteData(reinterpret_cast<const char*>(&p), sizeof(param_type));
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r) {
     const char *data;
     int data_size;
     if (!m->ReadData(iter, &data, &data_size) ||
@@ -327,7 +336,7 @@ template <>
 struct IPC_EXPORT ParamTraits<base::Time> {
   typedef base::Time param_type;
   static void Write(Message* m, const param_type& p);
-  static bool Read(const Message* m, void** iter, param_type* r);
+  static bool Read(const Message* m, PickleIterator* iter, param_type* r);
   static void Log(const param_type& p, std::string* l);
 };
 
@@ -335,7 +344,7 @@ template <>
 struct IPC_EXPORT ParamTraits<base::TimeDelta> {
   typedef base::TimeDelta param_type;
   static void Write(Message* m, const param_type& p);
-  static bool Read(const Message* m, void** iter, param_type* r);
+  static bool Read(const Message* m, PickleIterator* iter, param_type* r);
   static void Log(const param_type& p, std::string* l);
 };
 
@@ -343,7 +352,7 @@ template <>
 struct IPC_EXPORT ParamTraits<base::TimeTicks> {
   typedef base::TimeTicks param_type;
   static void Write(Message* m, const param_type& p);
-  static bool Read(const Message* m, void** iter, param_type* r);
+  static bool Read(const Message* m, PickleIterator* iter, param_type* r);
   static void Log(const param_type& p, std::string* l);
 };
 
@@ -354,7 +363,8 @@ struct ParamTraits<LOGFONT> {
   static void Write(Message* m, const param_type& p) {
     m->WriteData(reinterpret_cast<const char*>(&p), sizeof(LOGFONT));
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r) {
     const char *data;
     int data_size = 0;
     bool result = m->ReadData(iter, &data, &data_size);
@@ -378,7 +388,8 @@ struct ParamTraits<MSG> {
   static void Write(Message* m, const param_type& p) {
     m->WriteData(reinterpret_cast<const char*>(&p), sizeof(MSG));
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r) {
     const char *data;
     int data_size = 0;
     bool result = m->ReadData(iter, &data, &data_size);
@@ -401,7 +412,7 @@ template <>
 struct IPC_EXPORT ParamTraits<base::DictionaryValue> {
   typedef base::DictionaryValue param_type;
   static void Write(Message* m, const param_type& p);
-  static bool Read(const Message* m, void** iter, param_type* r);
+  static bool Read(const Message* m, PickleIterator* iter, param_type* r);
   static void Log(const param_type& p, std::string* l);
 };
 
@@ -409,7 +420,7 @@ template <>
 struct IPC_EXPORT ParamTraits<base::ListValue> {
   typedef base::ListValue param_type;
   static void Write(Message* m, const param_type& p);
-  static bool Read(const Message* m, void** iter, param_type* r);
+  static bool Read(const Message* m, PickleIterator* iter, param_type* r);
   static void Log(const param_type& p, std::string* l);
 };
 
@@ -419,7 +430,8 @@ struct ParamTraits<std::string> {
   static void Write(Message* m, const param_type& p) {
     m->WriteString(p);
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r) {
     return m->ReadString(iter, r);
   }
   static void Log(const param_type& p, std::string* l) {
@@ -461,7 +473,8 @@ struct ParamTraits<std::vector<unsigned char> > {
                    static_cast<int>(p.size()));
     }
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r) {
     const char *data;
     int data_size = 0;
     if (!m->ReadData(iter, &data, &data_size) || data_size < 0)
@@ -486,7 +499,8 @@ struct ParamTraits<std::vector<char> > {
       m->WriteData(&p.front(), static_cast<int>(p.size()));
     }
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r) {
     const char *data;
     int data_size = 0;
     if (!m->ReadData(iter, &data, &data_size) || data_size < 0)
@@ -509,7 +523,8 @@ struct ParamTraits<std::vector<bool> > {
     for (size_t i = 0; i < p.size(); i++)
       WriteParam(m, p[i]);
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r) {
     int size;
     // ReadLength() checks for < 0 itself.
     if (!m->ReadLength(iter, &size))
@@ -540,7 +555,8 @@ struct ParamTraits<std::vector<P> > {
     for (size_t i = 0; i < p.size(); i++)
       WriteParam(m, p[i]);
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r) {
     int size;
     // ReadLength() checks for < 0 itself.
     if (!m->ReadLength(iter, &size))
@@ -573,7 +589,8 @@ struct ParamTraits<std::set<P> > {
     for (iter = p.begin(); iter != p.end(); ++iter)
       WriteParam(m, *iter);
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r) {
     int size;
     if (!m->ReadLength(iter, &size))
       return false;
@@ -602,7 +619,8 @@ struct ParamTraits<std::map<K, V> > {
       WriteParam(m, iter->second);
     }
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r) {
     int size;
     if (!ReadParam(m, iter, &size) || size < 0)
       return false;
@@ -628,7 +646,8 @@ struct ParamTraits<std::wstring> {
   static void Write(Message* m, const param_type& p) {
     m->WriteWString(p);
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r) {
     return m->ReadWString(iter, r);
   }
   IPC_EXPORT static void Log(const param_type& p, std::string* l);
@@ -641,7 +660,8 @@ struct ParamTraits<std::pair<A, B> > {
     WriteParam(m, p.first);
     WriteParam(m, p.second);
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r) {
     return ReadParam(m, iter, &r->first) && ReadParam(m, iter, &r->second);
   }
   static void Log(const param_type& p, std::string* l) {
@@ -657,7 +677,8 @@ template <>
 struct IPC_EXPORT ParamTraits<NullableString16> {
   typedef NullableString16 param_type;
   static void Write(Message* m, const param_type& p);
-  static bool Read(const Message* m, void** iter, param_type* r);
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r);
   static void Log(const param_type& p, std::string* l);
 };
 
@@ -670,7 +691,8 @@ struct ParamTraits<string16> {
   static void Write(Message* m, const param_type& p) {
     m->WriteString16(p);
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r) {
     return m->ReadString16(iter, r);
   }
   IPC_EXPORT static void Log(const param_type& p, std::string* l);
@@ -687,7 +709,8 @@ struct ParamTraits<HANDLE> {
     // bit systems.
     m->WriteUInt32(reinterpret_cast<uint32>(p));
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter,
+                   param_type* r) {
     DCHECK_EQ(sizeof(param_type), sizeof(uint32));
     return m->ReadUInt32(iter, reinterpret_cast<uint32*>(r));
   }
@@ -702,7 +725,7 @@ struct ParamTraits<HCURSOR> {
   static void Write(Message* m, const param_type& p) {
     m->WriteUInt32(reinterpret_cast<uint32>(p));
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter, param_type* r) {
     DCHECK_EQ(sizeof(param_type), sizeof(uint32));
     return m->ReadUInt32(iter, reinterpret_cast<uint32*>(r));
   }
@@ -717,7 +740,7 @@ struct ParamTraits<HACCEL> {
   static void Write(Message* m, const param_type& p) {
     m->WriteUInt32(reinterpret_cast<uint32>(p));
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter, param_type* r) {
     DCHECK_EQ(sizeof(param_type), sizeof(uint32));
     return m->ReadUInt32(iter, reinterpret_cast<uint32*>(r));
   }
@@ -730,7 +753,7 @@ struct ParamTraits<POINT> {
     m->WriteInt(p.x);
     m->WriteInt(p.y);
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter, param_type* r) {
     int x, y;
     if (!m->ReadInt(iter, &x) || !m->ReadInt(iter, &y))
       return false;
@@ -748,7 +771,7 @@ template <>
 struct IPC_EXPORT ParamTraits<FilePath> {
   typedef FilePath param_type;
   static void Write(Message* m, const param_type& p);
-  static bool Read(const Message* m, void** iter, param_type* r);
+  static bool Read(const Message* m, PickleIterator* iter, param_type* r);
   static void Log(const param_type& p, std::string* l);
 };
 
@@ -772,7 +795,7 @@ template<>
 struct IPC_EXPORT ParamTraits<base::FileDescriptor> {
   typedef base::FileDescriptor param_type;
   static void Write(Message* m, const param_type& p);
-  static bool Read(const Message* m, void** iter, param_type* r);
+  static bool Read(const Message* m, PickleIterator* iter, param_type* r);
   static void Log(const param_type& p, std::string* l);
 };
 #endif  // defined(OS_POSIX)
@@ -784,7 +807,7 @@ template<>
 struct IPC_EXPORT ParamTraits<IPC::ChannelHandle> {
   typedef ChannelHandle param_type;
   static void Write(Message* m, const param_type& p);
-  static bool Read(const Message* m, void** iter, param_type* r);
+  static bool Read(const Message* m, PickleIterator* iter, param_type* r);
   static void Log(const param_type& p, std::string* l);
 };
 
@@ -795,7 +818,7 @@ struct ParamTraits<XFORM> {
   static void Write(Message* m, const param_type& p) {
     m->WriteData(reinterpret_cast<const char*>(&p), sizeof(XFORM));
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter, param_type* r) {
     const char *data;
     int data_size = 0;
     bool result = m->ReadData(iter, &data, &data_size);
@@ -835,7 +858,7 @@ template <>
 struct IPC_EXPORT ParamTraits<LogData> {
   typedef LogData param_type;
   static void Write(Message* m, const param_type& p);
-  static bool Read(const Message* m, void** iter, param_type* r);
+  static bool Read(const Message* m, PickleIterator* iter, param_type* r);
   static void Log(const param_type& p, std::string* l) {
     // Doesn't make sense to implement this!
   }
@@ -849,7 +872,7 @@ struct ParamTraits<Message> {
     m->WriteInt(message_size);
     m->WriteData(reinterpret_cast<const char*>(p.data()), message_size);
   }
-  static bool Read(const Message* m, void** iter, Message* r) {
+  static bool Read(const Message* m, PickleIterator* iter, Message* r) {
     int size;
     if (!m->ReadInt(iter, &size))
       return false;
@@ -869,7 +892,7 @@ struct ParamTraits<Tuple0> {
   typedef Tuple0 param_type;
   static void Write(Message* m, const param_type& p) {
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter, param_type* r) {
     return true;
   }
   static void Log(const param_type& p, std::string* l) {
@@ -882,7 +905,7 @@ struct ParamTraits< Tuple1<A> > {
   static void Write(Message* m, const param_type& p) {
     WriteParam(m, p.a);
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter, param_type* r) {
     return ReadParam(m, iter, &r->a);
   }
   static void Log(const param_type& p, std::string* l) {
@@ -897,7 +920,7 @@ struct ParamTraits< Tuple2<A, B> > {
     WriteParam(m, p.a);
     WriteParam(m, p.b);
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter, param_type* r) {
     return (ReadParam(m, iter, &r->a) &&
             ReadParam(m, iter, &r->b));
   }
@@ -916,7 +939,7 @@ struct ParamTraits< Tuple3<A, B, C> > {
     WriteParam(m, p.b);
     WriteParam(m, p.c);
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter, param_type* r) {
     return (ReadParam(m, iter, &r->a) &&
             ReadParam(m, iter, &r->b) &&
             ReadParam(m, iter, &r->c));
@@ -939,7 +962,7 @@ struct ParamTraits< Tuple4<A, B, C, D> > {
     WriteParam(m, p.c);
     WriteParam(m, p.d);
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter, param_type* r) {
     return (ReadParam(m, iter, &r->a) &&
             ReadParam(m, iter, &r->b) &&
             ReadParam(m, iter, &r->c) &&
@@ -966,7 +989,7 @@ struct ParamTraits< Tuple5<A, B, C, D, E> > {
     WriteParam(m, p.d);
     WriteParam(m, p.e);
   }
-  static bool Read(const Message* m, void** iter, param_type* r) {
+  static bool Read(const Message* m, PickleIterator* iter, param_type* r) {
     return (ReadParam(m, iter, &r->a) &&
             ReadParam(m, iter, &r->b) &&
             ReadParam(m, iter, &r->c) &&
@@ -1054,7 +1077,7 @@ class ParamDeserializer : public MessageReplyDeserializer {
  public:
   explicit ParamDeserializer(const RefTuple& out) : out_(out) { }
 
-  bool SerializeOutputParameters(const IPC::Message& msg, void* iter) {
+  bool SerializeOutputParameters(const IPC::Message& msg, PickleIterator iter) {
     return ReadParam(&msg, &iter, &out_);
   }
 
diff --git a/ipc/ipc_message_utils_impl.h b/ipc/ipc_message_utils_impl.h
index f078b2a..0931e30 100644
--- a/ipc/ipc_message_utils_impl.h
+++ b/ipc/ipc_message_utils_impl.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
@@ -18,7 +18,7 @@ void MessageSchema<ParamType>::Write(Message* msg, const RefParam& p) {
 
 template <class ParamType>
 bool MessageSchema<ParamType>::Read(const Message* msg, Param* p) {
-  void* iter = NULL;
+  PickleIterator iter(*msg);
   if (ReadParam(msg, &iter, p))
     return true;
   NOTREACHED() << "Error deserializing message " << msg->type();
@@ -35,14 +35,14 @@ void SyncMessageSchema<SendParamType, ReplyParamType>::Write(
 template <class SendParamType, class ReplyParamType>
 bool SyncMessageSchema<SendParamType, ReplyParamType>::ReadSendParam(
     const Message* msg, SendParam* p) {
-  void* iter = SyncMessage::GetDataIterator(msg);
+  PickleIterator iter = SyncMessage::GetDataIterator(msg);
   return ReadParam(msg, &iter, p);
 }
 
 template <class SendParamType, class ReplyParamType>
 bool SyncMessageSchema<SendParamType, ReplyParamType>::ReadReplyParam(
     const Message* msg, typename TupleTypes<ReplyParam>::ValueTuple* p) {
-  void* iter = SyncMessage::GetDataIterator(msg);
+  PickleIterator iter = SyncMessage::GetDataIterator(msg);
   return ReadParam(msg, &iter, p);
 }
 
diff --git a/ipc/ipc_send_fds_test.cc b/ipc/ipc_send_fds_test.cc
index 3b845f8..124a3dc 100644
--- a/ipc/ipc_send_fds_test.cc
+++ b/ipc/ipc_send_fds_test.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -52,7 +52,7 @@ class MyChannelDescriptorListener : public IPC::Channel::Listener {
         num_fds_received_(0) {}
 
   virtual bool OnMessageReceived(const IPC::Message& message) {
-    void* iter = NULL;
+    PickleIterator iter(message);
 
     ++num_fds_received_;
     base::FileDescriptor descriptor;
diff --git a/ipc/ipc_sync_message.cc b/ipc/ipc_sync_message.cc
index 875f97b..eb0a917 100644
--- a/ipc/ipc_sync_message.cc
+++ b/ipc/ipc_sync_message.cc
@@ -74,10 +74,12 @@ bool SyncMessage::IsMessageReplyTo(const Message& msg, int request_id) {
   return GetMessageId(msg) == request_id;
 }
 
-void* SyncMessage::GetDataIterator(const Message* msg) {
-  void* iter = const_cast<char*>(msg->payload());
-  UpdateIter(&iter, kSyncMessageHeaderSize);
-  return iter;
+PickleIterator SyncMessage::GetDataIterator(const Message* msg) {
+  PickleIterator iter(*msg);
+  if (!iter.SkipBytes(kSyncMessageHeaderSize))
+    return PickleIterator();
+  else
+    return iter;
 }
 
 int SyncMessage::GetMessageId(const Message& msg) {
@@ -110,7 +112,7 @@ Message* SyncMessage::GenerateReply(const Message* msg) {
 bool SyncMessage::ReadSyncHeader(const Message& msg, SyncHeader* header) {
   DCHECK(msg.is_sync() || msg.is_reply());
 
-  void* iter = NULL;
+  PickleIterator iter(msg);
   bool result = msg.ReadInt(&iter, &header->message_id);
   if (!result) {
     NOTREACHED();
diff --git a/ipc/ipc_sync_message.h b/ipc/ipc_sync_message.h
index 087df0d..17a7324 100644
--- a/ipc/ipc_sync_message.h
+++ b/ipc/ipc_sync_message.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -61,7 +61,7 @@ class IPC_EXPORT SyncMessage : public Message {
 
   // Given a reply message, returns an iterator to the beginning of the data
   // (i.e. skips over the synchronous specific data).
-  static void* GetDataIterator(const Message* msg);
+  static PickleIterator GetDataIterator(const Message* msg);
 
   // Given a synchronous message (or its reply), returns its id.
   static int GetMessageId(const Message& msg);
@@ -90,7 +90,8 @@ class IPC_EXPORT MessageReplyDeserializer {
  private:
   // Derived classes need to implement this, using the given iterator (which
   // is skipped past the header for synchronous messages).
-  virtual bool SerializeOutputParameters(const Message& msg, void* iter) = 0;
+  virtual bool SerializeOutputParameters(const Message& msg,
+                                         PickleIterator iter) = 0;
 };
 
 // When sending a synchronous message, this structure contains an object
diff --git a/ipc/ipc_tests.cc b/ipc/ipc_tests.cc
index 8b22fc3..14259fa 100644
--- a/ipc/ipc_tests.cc
+++ b/ipc/ipc_tests.cc
@@ -140,7 +140,7 @@ TEST_F(IPCChannelTest, BasicMessageTest) {
   EXPECT_TRUE(m.WriteString(v2));
   EXPECT_TRUE(m.WriteWString(v3));
 
-  void* iter = NULL;
+  PickleIterator iter(m);
 
   int vi;
   std::string vs;
diff --git a/ipc/param_traits_macros.h b/ipc/param_traits_macros.h
index 05ce403..58e74f6 100644
--- a/ipc/param_traits_macros.h
+++ b/ipc/param_traits_macros.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -14,7 +14,7 @@
     struct IPC_MESSAGE_EXPORT ParamTraits<struct_name> { \
       typedef struct_name param_type; \
       static void Write(Message* m, const param_type& p); \
-      static bool Read(const Message* m, void** iter, param_type* p); \
+      static bool Read(const Message* m, PickleIterator* iter, param_type* p); \
       static void Log(const param_type& p, std::string* l); \
     }; \
   }
@@ -30,7 +30,7 @@
     struct IPC_MESSAGE_EXPORT ParamTraits<enum_name> { \
       typedef enum_name param_type; \
       static void Write(Message* m, const param_type& p); \
-      static bool Read(const Message* m, void** iter, param_type* p); \
+      static bool Read(const Message* m, PickleIterator* iter, param_type* p); \
       static void Log(const param_type& p, std::string* l); \
     }; \
   }
diff --git a/ipc/param_traits_read_macros.h b/ipc/param_traits_read_macros.h
index a78c880..9d89a92 100644
--- a/ipc/param_traits_read_macros.h
+++ b/ipc/param_traits_read_macros.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -27,7 +27,7 @@
 #undef IPC_STRUCT_TRAITS_END
 #define IPC_STRUCT_TRAITS_BEGIN(struct_name) \
   bool ParamTraits<struct_name>:: \
-      Read(const Message* m, void** iter, param_type* p) { \
+      Read(const Message* m, PickleIterator* iter, param_type* p) { \
     return
 #define IPC_STRUCT_TRAITS_MEMBER(name) ReadParam(m, iter, &p->name) &&
 #define IPC_STRUCT_TRAITS_PARENT(type) ParamTraits<type>::Read(m, iter, p) &&
@@ -36,7 +36,7 @@
 #undef IPC_ENUM_TRAITS
 #define IPC_ENUM_TRAITS(enum_name) \
   bool ParamTraits<enum_name>:: \
-      Read(const Message* m, void** iter, param_type* p) { \
+      Read(const Message* m, PickleIterator* iter, param_type* p) { \
     int type; \
     if (!m->ReadInt(iter, &type)) \
       return false; \