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

13 files changed, 49 insertions, 48 deletions

diff --git a/webkit/fileapi/file_system_directory_database.cc b/webkit/fileapi/file_system_directory_database.cc
index 88152e2..217b98f 100644
--- a/webkit/fileapi/file_system_directory_database.cc
+++ b/webkit/fileapi/file_system_directory_database.cc
@@ -47,7 +47,7 @@ bool PickleFromFileInfo(
 bool FileInfoFromPickle(
     const Pickle& pickle,
     fileapi::FileSystemDirectoryDatabase::FileInfo* info) {
-  void* iter = NULL;
+  PickleIterator iter(pickle);
   std::string data_path;
   std::string name;
   int64 internal_time;
diff --git a/webkit/fileapi/file_system_usage_cache.cc b/webkit/fileapi/file_system_usage_cache.cc
index 46bbdff..18d13c7 100644
--- a/webkit/fileapi/file_system_usage_cache.cc
+++ b/webkit/fileapi/file_system_usage_cache.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.
 
@@ -131,7 +131,7 @@ int64 FileSystemUsageCache::Read(const FilePath& usage_file_path,
       file_util::ReadFile(usage_file_path, buffer, kUsageFileSize))
     return -1;
   Pickle read_pickle(buffer, kUsageFileSize);
-  void* iter = NULL;
+  PickleIterator iter(read_pickle);
   int64 fs_usage;
 
   if (!read_pickle.ReadBytes(&iter, &header, kUsageFileHeaderSize) ||
diff --git a/webkit/glue/glue_serialize.cc b/webkit/glue/glue_serialize.cc
index 04a4ac4..606ff61 100644
--- a/webkit/glue/glue_serialize.cc
+++ b/webkit/glue/glue_serialize.cc
@@ -32,16 +32,16 @@ namespace webkit_glue {
 
 namespace {
 struct SerializeObject {
-  SerializeObject() : iter(NULL), version(0) {}
+  SerializeObject() : version(0) {}
   SerializeObject(const char* data, int len)
-      : pickle(data, len), iter(NULL), version(0) {}
+      : pickle(data, len), version(0) { iter = PickleIterator(pickle); }
 
   std::string GetAsString() {
     return std::string(static_cast<const char*>(pickle.data()), pickle.size());
   }
 
   Pickle pickle;
-  mutable void* iter;
+  mutable PickleIterator iter;
   mutable int version;
 };
 
diff --git a/webkit/glue/npruntime_util.cc b/webkit/glue/npruntime_util.cc
index a408770e..4c448ae 100644
--- a/webkit/glue/npruntime_util.cc
+++ b/webkit/glue/npruntime_util.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-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.
 
@@ -26,22 +26,22 @@ bool SerializeNPIdentifier(NPIdentifier identifier, Pickle* pickle) {
   return pickle->WriteInt(number);
 }
 
-bool DeserializeNPIdentifier(const Pickle& pickle, void** pickle_iter,
+bool DeserializeNPIdentifier(PickleIterator* pickle_iter,
                              NPIdentifier* identifier) {
   bool is_string;
-  if (!pickle.ReadBool(pickle_iter, &is_string))
+  if (!pickle_iter->ReadBool(&is_string))
     return false;
 
   if (is_string) {
     const char* data;
     int data_len;
-    if (!pickle.ReadData(pickle_iter, &data, &data_len))
+    if (!pickle_iter->ReadData(&data, &data_len))
       return false;
     DCHECK_EQ((static_cast<size_t>(data_len)), strlen(data) + 1);
     *identifier = WebBindings::getStringIdentifier(data);
   } else {
     int number;
-    if (!pickle.ReadInt(pickle_iter, &number))
+    if (!pickle_iter->ReadInt(&number))
       return false;
     *identifier = WebBindings::getIntIdentifier(number);
   }
diff --git a/webkit/glue/npruntime_util.h b/webkit/glue/npruntime_util.h
index 586aab1..401f771 100644
--- a/webkit/glue/npruntime_util.h
+++ b/webkit/glue/npruntime_util.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.
 
@@ -9,14 +9,14 @@
 #include "webkit/glue/webkit_glue_export.h"
 
 class Pickle;
+class PickleIterator;
 
 namespace webkit_glue {
 
 // Efficiently serialize/deserialize a NPIdentifier
 WEBKIT_GLUE_EXPORT bool SerializeNPIdentifier(NPIdentifier identifier,
                                               Pickle* pickle);
-WEBKIT_GLUE_EXPORT bool DeserializeNPIdentifier(const Pickle& pickle,
-                                                void** pickle_iter,
+WEBKIT_GLUE_EXPORT bool DeserializeNPIdentifier(PickleIterator* pickle_iter,
                                                 NPIdentifier* identifier);
 
 }  // namespace webkit_glue
diff --git a/webkit/glue/webcursor.cc b/webkit/glue/webcursor.cc
index 292ca0d..94c2781 100644
--- a/webkit/glue/webcursor.cc
+++ b/webkit/glue/webcursor.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.
 
@@ -70,18 +70,18 @@ void WebCursor::GetCursorInfo(WebCursorInfo* cursor_info) const {
 #endif
 }
 
-bool WebCursor::Deserialize(const Pickle* pickle, void** iter) {
+bool WebCursor::Deserialize(PickleIterator* iter) {
   int type, hotspot_x, hotspot_y, size_x, size_y, data_len;
 
   const char* data;
 
   // Leave |this| unmodified unless we are going to return success.
-  if (!pickle->ReadInt(iter, &type) ||
-      !pickle->ReadInt(iter, &hotspot_x) ||
-      !pickle->ReadInt(iter, &hotspot_y) ||
-      !pickle->ReadLength(iter, &size_x) ||
-      !pickle->ReadLength(iter, &size_y) ||
-      !pickle->ReadData(iter, &data, &data_len))
+  if (!iter->ReadInt(&type) ||
+      !iter->ReadInt(&hotspot_x) ||
+      !iter->ReadInt(&hotspot_y) ||
+      !iter->ReadLength(&size_x) ||
+      !iter->ReadLength(&size_y) ||
+      !iter->ReadData(&data, &data_len))
     return false;
 
   // Ensure the size is sane, and there is enough data.
@@ -111,7 +111,7 @@ bool WebCursor::Deserialize(const Pickle* pickle, void** iter) {
       }
     }
   }
-  return DeserializePlatformData(pickle, iter);
+  return DeserializePlatformData(iter);
 }
 
 bool WebCursor::Serialize(Pickle* pickle) const {
diff --git a/webkit/glue/webcursor.h b/webkit/glue/webcursor.h
index 0461a3c..0dfddcf 100644
--- a/webkit/glue/webcursor.h
+++ b/webkit/glue/webcursor.h
@@ -30,6 +30,7 @@ struct Cursor;
 #endif
 
 class Pickle;
+class PickleIterator;
 
 namespace WebKit {
 class WebImage;
@@ -55,7 +56,7 @@ class WEBKIT_GLUE_EXPORT WebCursor {
   void GetCursorInfo(WebKit::WebCursorInfo* cursor_info) const;
 
   // Serialization / De-serialization
-  bool Deserialize(const Pickle* pickle, void** iter);
+  bool Deserialize(PickleIterator* iter);
   bool Serialize(Pickle* pickle) const;
 
   // Returns true if GetCustomCursor should be used to allocate a platform
@@ -114,7 +115,7 @@ class WEBKIT_GLUE_EXPORT WebCursor {
 
   // Platform specific Serialization / De-serialization
   bool SerializePlatformData(Pickle* pickle) const;
-  bool DeserializePlatformData(const Pickle* pickle, void** iter);
+  bool DeserializePlatformData(PickleIterator* iter);
 
   // Returns true if the platform data in the current cursor object
   // matches that of the cursor passed in.
diff --git a/webkit/glue/webcursor_android.cc b/webkit/glue/webcursor_android.cc
index 774133b..54a8a1d 100644
--- a/webkit/glue/webcursor_android.cc
+++ b/webkit/glue/webcursor_android.cc
@@ -14,7 +14,7 @@ bool WebCursor::SerializePlatformData(Pickle* pickle) const {
   return true;
 }
 
-bool WebCursor::DeserializePlatformData(const Pickle* pickle, void** iter) {
+bool WebCursor::DeserializePlatformData(PickleIterator* iter) {
   return true;
 }
 
diff --git a/webkit/glue/webcursor_aura.cc b/webkit/glue/webcursor_aura.cc
index 7610aff..08719c1 100644
--- a/webkit/glue/webcursor_aura.cc
+++ b/webkit/glue/webcursor_aura.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.
 
@@ -113,7 +113,7 @@ bool WebCursor::SerializePlatformData(Pickle* pickle) const {
   return true;
 }
 
-bool WebCursor::DeserializePlatformData(const Pickle* pickle, void** iter) {
+bool WebCursor::DeserializePlatformData(PickleIterator* iter) {
   return true;
 }
 
diff --git a/webkit/glue/webcursor_gtk.cc b/webkit/glue/webcursor_gtk.cc
index d46d6e2..e9b4a12 100644
--- a/webkit/glue/webcursor_gtk.cc
+++ b/webkit/glue/webcursor_gtk.cc
@@ -199,7 +199,7 @@ bool WebCursor::SerializePlatformData(Pickle* pickle) const {
   return true;
 }
 
-bool WebCursor::DeserializePlatformData(const Pickle* pickle, void** iter) {
+bool WebCursor::DeserializePlatformData(PickleIterator* iter) {
   return true;
 }
 
diff --git a/webkit/glue/webcursor_mac.mm b/webkit/glue/webcursor_mac.mm
index fe65480..1e72e0b 100644
--- a/webkit/glue/webcursor_mac.mm
+++ b/webkit/glue/webcursor_mac.mm
@@ -493,7 +493,7 @@ bool WebCursor::SerializePlatformData(Pickle* pickle) const {
   return true;
 }
 
-bool WebCursor::DeserializePlatformData(const Pickle* pickle, void** iter) {
+bool WebCursor::DeserializePlatformData(PickleIterator* iter) {
   return true;
 }
 
diff --git a/webkit/glue/webcursor_unittest.cc b/webkit/glue/webcursor_unittest.cc
index 00f2760..830905b 100644
--- a/webkit/glue/webcursor_unittest.cc
+++ b/webkit/glue/webcursor_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.
 
@@ -26,8 +26,8 @@ TEST(WebCursorTest, OKCursorSerialization) {
   ok_custom_pickle.WriteUInt32(0);
   // Custom Windows message.
   ok_custom_pickle.WriteUInt32(0);
-  void* iter = NULL;
-  EXPECT_TRUE(custom_cursor.Deserialize(&ok_custom_pickle, &iter));
+  PickleIterator iter(ok_custom_pickle);
+  EXPECT_TRUE(custom_cursor.Deserialize(&iter));
 
 #if defined(TOOLKIT_USES_GTK)
   // On GTK+ using platforms, we should get a real native GdkCursor object back
@@ -51,8 +51,8 @@ TEST(WebCursorTest, BrokenCursorSerialization) {
   // Data len not including enough data for a 1x1 image.
   short_custom_pickle.WriteInt(3);
   short_custom_pickle.WriteUInt32(0);
-  void* iter = NULL;
-  EXPECT_FALSE(custom_cursor.Deserialize(&short_custom_pickle, &iter));
+  PickleIterator iter(short_custom_pickle);
+  EXPECT_FALSE(custom_cursor.Deserialize(&iter));
 
   // This custom cursor has enough data but is too big.
   Pickle large_custom_pickle;
@@ -68,8 +68,8 @@ TEST(WebCursorTest, BrokenCursorSerialization) {
   large_custom_pickle.WriteInt(kTooBigSize * 4);
   for (int i = 0; i < kTooBigSize; ++i)
     large_custom_pickle.WriteUInt32(0);
-  iter = NULL;
-  EXPECT_FALSE(custom_cursor.Deserialize(&large_custom_pickle, &iter));
+  iter = PickleIterator(large_custom_pickle);
+  EXPECT_FALSE(custom_cursor.Deserialize(&iter));
 
   // This custom cursor uses negative lengths.
   Pickle neg_custom_pickle;
@@ -85,8 +85,8 @@ TEST(WebCursorTest, BrokenCursorSerialization) {
   neg_custom_pickle.WriteUInt32(0);
   // Custom Windows message.
   neg_custom_pickle.WriteUInt32(0);
-  iter = NULL;
-  EXPECT_FALSE(custom_cursor.Deserialize(&neg_custom_pickle, &iter));
+  iter = PickleIterator(neg_custom_pickle);
+  EXPECT_FALSE(custom_cursor.Deserialize(&iter));
 }
 
 #if defined(OS_WIN)
@@ -97,8 +97,8 @@ TEST(WebCursorTest, WindowsCursorConversion) {
   win32_custom_cursor.InitFromExternalCursor(
       reinterpret_cast<HCURSOR>(1000));
   EXPECT_TRUE(win32_custom_cursor.Serialize(&win32_custom_pickle));
-  void* iter = NULL;
-  EXPECT_TRUE(custom_cursor.Deserialize(&win32_custom_pickle, &iter));
+  PickleIterator iter(win32_custom_pickle);
+  EXPECT_TRUE(custom_cursor.Deserialize(&iter));
   EXPECT_EQ(reinterpret_cast<HCURSOR>(1000), custom_cursor.GetCursor(NULL));
 }
 #endif  // OS_WIN
@@ -121,8 +121,8 @@ TEST(WebCursorTest, ClampHotspot) {
     ok_custom_pickle.WriteUInt32(0);
   // Custom Windows message.
   ok_custom_pickle.WriteUInt32(0);
-  void* iter = NULL;
-  ASSERT_TRUE(custom_cursor.Deserialize(&ok_custom_pickle, &iter));
+  PickleIterator iter(ok_custom_pickle);
+  ASSERT_TRUE(custom_cursor.Deserialize(&iter));
 
   // Convert to WebCursorInfo, make sure the hotspot got clamped.
   WebCursorInfo info;
@@ -154,8 +154,8 @@ TEST(WebCursorTest, EmptyImage) {
 
   // Make sure we can read this on all platforms; it is technicaally a valid
   // cursor.
-  void* iter = NULL;
-  ASSERT_TRUE(custom_cursor.Deserialize(&broken_cursor_pickle, &iter));
+  PickleIterator iter(broken_cursor_pickle);
+  ASSERT_TRUE(custom_cursor.Deserialize(&iter));
 
 #if defined(TOOLKIT_USES_GTK)
   // On GTK+ using platforms, we make sure that we get NULL back from this
diff --git a/webkit/glue/webcursor_win.cc b/webkit/glue/webcursor_win.cc
index 4d8e520..9b334a8 100644
--- a/webkit/glue/webcursor_win.cc
+++ b/webkit/glue/webcursor_win.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.
 
@@ -216,8 +216,8 @@ bool WebCursor::SerializePlatformData(Pickle* pickle) const {
   return pickle->WriteUInt32(reinterpret_cast<uint32>(external_cursor_));
 }
 
-bool WebCursor::DeserializePlatformData(const Pickle* pickle, void** iter) {
-  return pickle->ReadUInt32(iter, reinterpret_cast<uint32*>(&external_cursor_));
+bool WebCursor::DeserializePlatformData(PickleIterator* iter) {
+  return iter->ReadUInt32(reinterpret_cast<uint32*>(&external_cursor_));
 }
 
 bool WebCursor::IsPlatformDataEqual(const WebCursor& other) const {