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

6 files changed, 50 insertions, 40 deletions

diff --git a/ppapi/proxy/ppapi_param_traits.cc b/ppapi/proxy/ppapi_param_traits.cc
index 484cb9b..7c13710 100644
--- a/ppapi/proxy/ppapi_param_traits.cc
+++ b/ppapi/proxy/ppapi_param_traits.cc
@@ -39,7 +39,7 @@ namespace {
 // add it to the vector one at a time.
 template<typename T>
 bool ReadVectorWithoutCopy(const Message* m,
-                           void** iter,
+                           PickleIterator* iter,
                            std::vector<T>* output) {
   // This part is just a copy of the the default ParamTraits vector Read().
   int size;
@@ -81,7 +81,9 @@ void ParamTraits<PP_Bool>::Write(Message* m, const param_type& p) {
 }
 
 // static
-bool ParamTraits<PP_Bool>::Read(const Message* m, void** iter, param_type* r) {
+bool ParamTraits<PP_Bool>::Read(const Message* m,
+                                PickleIterator* iter,
+                                param_type* r) {
   // We specifically want to be strict here about what types of input we accept,
   // which ParamTraits<bool> does for us. We don't want to deserialize "2" into
   // a PP_Bool, for example.
@@ -109,7 +111,7 @@ void ParamTraits<PP_FileInfo>::Write(Message* m, const param_type& p) {
 }
 
 // static
-bool ParamTraits<PP_FileInfo>::Read(const Message* m, void** iter,
+bool ParamTraits<PP_FileInfo>::Read(const Message* m, PickleIterator* iter,
                                         param_type* r) {
   int type, system_type;
   if (!ParamTraits<int64_t>::Read(m, iter, &r->size) ||
@@ -148,7 +150,7 @@ void ParamTraits<PP_NetAddress_Private>::Write(Message* m,
 
 // static
 bool ParamTraits<PP_NetAddress_Private>::Read(const Message* m,
-                                              void** iter,
+                                              PickleIterator* iter,
                                               param_type* p) {
   uint16 size;
   if (!ReadParam(m, iter, &size))
@@ -181,7 +183,7 @@ void ParamTraits<PP_ObjectProperty>::Write(Message* m, const param_type& p) {
 
 // static
 bool ParamTraits<PP_ObjectProperty>::Read(const Message* m,
-                                          void** iter,
+                                          PickleIterator* iter,
                                           param_type* r) {
   // FIXME(brettw);
   return true;
@@ -220,7 +222,7 @@ void ParamTraits<ppapi::proxy::PPBFlash_DrawGlyphs_Params>::Write(
 // static
 bool ParamTraits<ppapi::proxy::PPBFlash_DrawGlyphs_Params>::Read(
     const Message* m,
-    void** iter,
+    PickleIterator* iter,
     param_type* r) {
   return
       ParamTraits<PP_Instance>::Read(m, iter, &r->instance) &&
@@ -265,7 +267,7 @@ void ParamTraits<ppapi::PPB_FileRef_CreateInfo>::Write(Message* m,
 
 // static
 bool ParamTraits<ppapi::PPB_FileRef_CreateInfo>::Read(const Message* m,
-                                                      void** iter,
+                                                      PickleIterator* iter,
                                                       param_type* r) {
   return
       ParamTraits<ppapi::HostResource>::Read(m, iter, &r->resource) &&
@@ -296,7 +298,7 @@ void ParamTraits<ppapi::proxy::PPBURLLoader_UpdateProgress_Params>::Write(
 // static
 bool ParamTraits<ppapi::proxy::PPBURLLoader_UpdateProgress_Params>::Read(
     const Message* m,
-    void** iter,
+    PickleIterator* iter,
     param_type* r) {
   return
       ParamTraits<PP_Instance>::Read(m, iter, &r->instance) &&
@@ -324,7 +326,7 @@ void ParamTraits<ppapi::proxy::SerializedDirEntry>::Write(Message* m,
 
 // static
 bool ParamTraits<ppapi::proxy::SerializedDirEntry>::Read(const Message* m,
-                                                         void** iter,
+                                                         PickleIterator* iter,
                                                          param_type* r) {
   return ParamTraits<std::string>::Read(m, iter, &r->name) &&
          ParamTraits<bool>::Read(m, iter, &r->is_dir);
@@ -354,7 +356,7 @@ void ParamTraits<ppapi::proxy::SerializedFontDescription>::Write(
 // static
 bool ParamTraits<ppapi::proxy::SerializedFontDescription>::Read(
     const Message* m,
-    void** iter,
+    PickleIterator* iter,
     param_type* r) {
   return
       ParamTraits<ppapi::proxy::SerializedVar>::Read(m, iter, &r->face) &&
@@ -384,7 +386,7 @@ void ParamTraits<ppapi::HostResource>::Write(Message* m,
 
 // static
 bool ParamTraits<ppapi::HostResource>::Read(const Message* m,
-                                            void** iter,
+                                            PickleIterator* iter,
                                             param_type* r) {
   PP_Instance instance;
   PP_Resource resource;
@@ -410,7 +412,7 @@ void ParamTraits<ppapi::proxy::SerializedVar>::Write(Message* m,
 
 // static
 bool ParamTraits<ppapi::proxy::SerializedVar>::Read(const Message* m,
-                                                    void** iter,
+                                                    PickleIterator* iter,
                                                     param_type* r) {
   return r->ReadFromMessage(m, iter);
 }
@@ -431,7 +433,7 @@ void ParamTraits< std::vector<ppapi::proxy::SerializedVar> >::Write(
 // static
 bool ParamTraits< std::vector<ppapi::proxy::SerializedVar> >::Read(
     const Message* m,
-    void** iter,
+    PickleIterator* iter,
     param_type* r) {
   return ReadVectorWithoutCopy(m, iter, r);
 }
@@ -453,7 +455,7 @@ void ParamTraits< std::vector<ppapi::PPB_FileRef_CreateInfo> >::Write(
 // static
 bool ParamTraits< std::vector<ppapi::PPB_FileRef_CreateInfo> >::Read(
     const Message* m,
-    void** iter,
+    PickleIterator* iter,
     param_type* r) {
   return ReadVectorWithoutCopy(m, iter, r);
 }
@@ -475,7 +477,7 @@ void ParamTraits<ppapi::proxy::SerializedFlashMenu>::Write(
 
 // static
 bool ParamTraits<ppapi::proxy::SerializedFlashMenu>::Read(const Message* m,
-                                                          void** iter,
+                                                          PickleIterator* iter,
                                                           param_type* r) {
   return r->ReadFromMessage(m, iter);
 }
diff --git a/ppapi/proxy/ppapi_param_traits.h b/ppapi/proxy/ppapi_param_traits.h
index 63121f0..40e4e5b 100644
--- a/ppapi/proxy/ppapi_param_traits.h
+++ b/ppapi/proxy/ppapi_param_traits.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.
 
@@ -42,7 +42,7 @@ template<>
 struct ParamTraits<PP_Bool> {
   typedef PP_Bool 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);
 };
 
@@ -50,7 +50,7 @@ template<>
 struct ParamTraits<PP_FileInfo> {
   typedef PP_FileInfo 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);
 };
 
@@ -58,7 +58,7 @@ template <>
 struct PPAPI_PROXY_EXPORT ParamTraits<PP_NetAddress_Private> {
   typedef PP_NetAddress_Private 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);
 };
 
@@ -66,7 +66,7 @@ template<>
 struct ParamTraits<PP_ObjectProperty> {
   typedef PP_ObjectProperty 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);
 };
 
@@ -74,7 +74,7 @@ template<>
 struct ParamTraits<ppapi::proxy::PPBFlash_DrawGlyphs_Params> {
   typedef ppapi::proxy::PPBFlash_DrawGlyphs_Params 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);
 };
 
@@ -82,7 +82,7 @@ template<>
 struct ParamTraits<ppapi::PPB_FileRef_CreateInfo> {
   typedef ppapi::PPB_FileRef_CreateInfo 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);
 };
 
@@ -90,7 +90,7 @@ template<>
 struct ParamTraits<ppapi::proxy::PPBURLLoader_UpdateProgress_Params> {
   typedef ppapi::proxy::PPBURLLoader_UpdateProgress_Params 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);
 };
 
@@ -98,7 +98,7 @@ template<>
 struct ParamTraits<ppapi::proxy::SerializedDirEntry> {
   typedef ppapi::proxy::SerializedDirEntry 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);
 };
 
@@ -106,7 +106,7 @@ template<>
 struct ParamTraits<ppapi::proxy::SerializedFontDescription> {
   typedef ppapi::proxy::SerializedFontDescription 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);
 };
 
@@ -114,7 +114,7 @@ template<>
 struct ParamTraits<ppapi::HostResource> {
   typedef ppapi::HostResource 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);
 };
 
@@ -122,7 +122,7 @@ template<>
 struct ParamTraits<ppapi::proxy::SerializedVar> {
   typedef ppapi::proxy::SerializedVar 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);
 };
 
@@ -130,7 +130,7 @@ template<>
 struct ParamTraits< std::vector<ppapi::proxy::SerializedVar> > {
   typedef std::vector<ppapi::proxy::SerializedVar> 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);
 };
 
@@ -138,7 +138,7 @@ template<>
 struct ParamTraits< std::vector<ppapi::PPB_FileRef_CreateInfo> > {
   typedef std::vector<ppapi::PPB_FileRef_CreateInfo> 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);
 };
 
@@ -146,7 +146,7 @@ template<>
 struct ParamTraits<ppapi::proxy::SerializedFlashMenu> {
   typedef ppapi::proxy::SerializedFlashMenu 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);
 };
 
diff --git a/ppapi/proxy/serialized_flash_menu.cc b/ppapi/proxy/serialized_flash_menu.cc
index c29f9c1..2784485 100644
--- a/ppapi/proxy/serialized_flash_menu.cc
+++ b/ppapi/proxy/serialized_flash_menu.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.
 
@@ -19,7 +19,7 @@ const int kMaxMenuDepth = 2;
 bool CheckMenu(int depth, const PP_Flash_Menu* menu);
 void FreeMenu(const PP_Flash_Menu* menu);
 void WriteMenu(IPC::Message* m, const PP_Flash_Menu* menu);
-PP_Flash_Menu* ReadMenu(int depth, const IPC::Message* m, void** iter);
+PP_Flash_Menu* ReadMenu(int depth, const IPC::Message* m, PickleIterator* iter);
 
 bool CheckMenuItem(int depth, const PP_Flash_MenuItem* item) {
   if (item->type == PP_FLASH_MENUITEM_TYPE_SUBMENU)
@@ -77,7 +77,7 @@ void FreeMenu(const PP_Flash_Menu* menu) {
 
 bool ReadMenuItem(int depth,
                   const IPC::Message* m,
-                  void** iter,
+                  PickleIterator* iter,
                   PP_Flash_MenuItem* menu_item) {
   uint32_t type;
   if (!m->ReadUInt32(iter, &type))
@@ -105,7 +105,9 @@ bool ReadMenuItem(int depth,
   return true;
 }
 
-PP_Flash_Menu* ReadMenu(int depth, const IPC::Message* m, void** iter) {
+PP_Flash_Menu* ReadMenu(int depth,
+                        const IPC::Message* m,
+                        PickleIterator* iter) {
   if (depth > kMaxMenuDepth)
     return NULL;
   ++depth;
@@ -158,7 +160,8 @@ void SerializedFlashMenu::WriteToMessage(IPC::Message* m) const {
   WriteMenu(m, pp_menu_);
 }
 
-bool SerializedFlashMenu::ReadFromMessage(const IPC::Message* m, void** iter) {
+bool SerializedFlashMenu::ReadFromMessage(const IPC::Message* m,
+                                          PickleIterator* iter) {
   DCHECK(!pp_menu_);
   pp_menu_ = ReadMenu(0, m, iter);
   if (!pp_menu_)
diff --git a/ppapi/proxy/serialized_flash_menu.h b/ppapi/proxy/serialized_flash_menu.h
index 4be22bf..f7ad6b6 100644
--- a/ppapi/proxy/serialized_flash_menu.h
+++ b/ppapi/proxy/serialized_flash_menu.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.
 
@@ -12,6 +12,8 @@
 #include "base/memory/scoped_ptr.h"
 #include "ppapi/proxy/ppapi_proxy_export.h"
 
+class PickleIterator;
+
 struct PP_Flash_Menu;
 
 namespace IPC {
@@ -31,7 +33,7 @@ class PPAPI_PROXY_EXPORT SerializedFlashMenu {
   const PP_Flash_Menu* pp_menu() const { return pp_menu_; }
 
   void WriteToMessage(IPC::Message* m) const;
-  bool ReadFromMessage(const IPC::Message* m, void** iter);
+  bool ReadFromMessage(const IPC::Message* m, PickleIterator* iter);
 
  private:
   const PP_Flash_Menu* pp_menu_;
diff --git a/ppapi/proxy/serialized_var.cc b/ppapi/proxy/serialized_var.cc
index eb20333..7b2a8e3 100644
--- a/ppapi/proxy/serialized_var.cc
+++ b/ppapi/proxy/serialized_var.cc
@@ -147,7 +147,8 @@ void SerializedVar::Inner::WriteToMessage(IPC::Message* m) const {
   }
 }
 
-bool SerializedVar::Inner::ReadFromMessage(const IPC::Message* m, void** iter) {
+bool SerializedVar::Inner::ReadFromMessage(const IPC::Message* m,
+                                           PickleIterator* iter) {
 #ifndef NDEBUG
   // We should only deserialize something once or will end up with leaked
   // references.
diff --git a/ppapi/proxy/serialized_var.h b/ppapi/proxy/serialized_var.h
index 28b7c8b..ffcf0da 100644
--- a/ppapi/proxy/serialized_var.h
+++ b/ppapi/proxy/serialized_var.h
@@ -14,6 +14,8 @@
 #include "ppapi/c/pp_var.h"
 #include "ppapi/proxy/ppapi_proxy_export.h"
 
+class PickleIterator;
+
 namespace IPC {
 class Message;
 }
@@ -72,7 +74,7 @@ class PPAPI_PROXY_EXPORT SerializedVar {
   void WriteToMessage(IPC::Message* m) const {
     inner_->WriteToMessage(m);
   }
-  bool ReadFromMessage(const IPC::Message* m, void** iter) {
+  bool ReadFromMessage(const IPC::Message* m, PickleIterator* iter) {
     return inner_->ReadFromMessage(m, iter);
   }
 
@@ -106,7 +108,7 @@ class PPAPI_PROXY_EXPORT SerializedVar {
     void ForceSetVarValueForTest(PP_Var value);
 
     void WriteToMessage(IPC::Message* m) const;
-    bool ReadFromMessage(const IPC::Message* m, void** iter);
+    bool ReadFromMessage(const IPC::Message* m, PickleIterator* iter);
 
     // Sets the cleanup mode. See the CleanupMode enum below. These functions
     // are not just a simple setter in order to require that the appropriate