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

4 files changed, 15 insertions, 19 deletions

diff --git a/ui/base/clipboard/clipboard_unittest.cc b/ui/base/clipboard/clipboard_unittest.cc
index f00d4fd..c586d51 100644
--- a/ui/base/clipboard/clipboard_unittest.cc
+++ b/ui/base/clipboard/clipboard_unittest.cc
@@ -445,7 +445,7 @@ TEST_F(ClipboardTest, DataTest) {
   ASSERT_FALSE(output.empty());
 
   Pickle read_pickle(output.data(), output.size());
-  void* iter = NULL;
+  PickleIterator iter(read_pickle);
   std::string unpickled_string;
   ASSERT_TRUE(read_pickle.ReadString(&iter, &unpickled_string));
   EXPECT_EQ(payload, unpickled_string);
@@ -482,7 +482,7 @@ TEST_F(ClipboardTest, MultipleDataTest) {
   ASSERT_FALSE(output2.empty());
 
   Pickle read_pickle2(output2.data(), output2.size());
-  void* iter2 = NULL;
+  PickleIterator iter2(read_pickle2);
   std::string unpickled_string2;
   ASSERT_TRUE(read_pickle2.ReadString(&iter2, &unpickled_string2));
   EXPECT_EQ(payload2, unpickled_string2);
@@ -504,7 +504,7 @@ TEST_F(ClipboardTest, MultipleDataTest) {
   ASSERT_FALSE(output1.empty());
 
   Pickle read_pickle1(output1.data(), output1.size());
-  void* iter1 = NULL;
+  PickleIterator iter1(read_pickle1);
   std::string unpickled_string1;
   ASSERT_TRUE(read_pickle1.ReadString(&iter1, &unpickled_string1));
   EXPECT_EQ(payload1, unpickled_string1);
diff --git a/ui/base/clipboard/custom_data_helper.cc b/ui/base/clipboard/custom_data_helper.cc
index 88abf0f..5200de8 100644
--- a/ui/base/clipboard/custom_data_helper.cc
+++ b/ui/base/clipboard/custom_data_helper.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,24 +19,20 @@ namespace {
 class SkippablePickle : public Pickle {
  public:
   SkippablePickle(const void* data, size_t data_len);
-  bool SkipString16(void** iter);
+  bool SkipString16(PickleIterator* iter);
 };
 
 SkippablePickle::SkippablePickle(const void* data, size_t data_len)
     : Pickle(reinterpret_cast<const char*>(data), data_len) {
 }
 
-bool SkippablePickle::SkipString16(void** iter) {
+bool SkippablePickle::SkipString16(PickleIterator* iter) {
   DCHECK(iter);
 
   int len;
   if (!ReadLength(iter, &len))
     return false;
-  if (!IteratorHasRoomFor(*iter, len * sizeof(char16)))
-    return false;
-
-  UpdateIter(iter, len * sizeof(char16));
-  return true;
+  return iter->SkipBytes(len * sizeof(char16));
 }
 
 }  // namespace
@@ -45,7 +41,7 @@ void ReadCustomDataTypes(const void* data,
                          size_t data_length,
                          std::vector<string16>* types) {
   SkippablePickle pickle(data, data_length);
-  void* iter = NULL;
+  PickleIterator iter(pickle);
 
   size_t size = 0;
   if (!pickle.ReadSize(&iter, &size))
@@ -71,7 +67,7 @@ void ReadCustomDataForType(const void* data,
                            const string16& type,
                            string16* result) {
   SkippablePickle pickle(data, data_length);
-  void* iter = NULL;
+  PickleIterator iter(pickle);
 
   size_t size = 0;
   if (!pickle.ReadSize(&iter, &size))
@@ -94,7 +90,7 @@ void ReadCustomDataIntoMap(const void* data,
                            size_t data_length,
                            std::map<string16, string16>* result) {
   Pickle pickle(reinterpret_cast<const char*>(data), data_length);
-  void* iter = NULL;
+  PickleIterator iter(pickle);
 
   size_t size = 0;
   if (!pickle.ReadSize(&iter, &size))
diff --git a/ui/base/dragdrop/gtk_dnd_util.cc b/ui/base/dragdrop/gtk_dnd_util.cc
index d70aac8..71d6923 100644
--- a/ui/base/dragdrop/gtk_dnd_util.cc
+++ b/ui/base/dragdrop/gtk_dnd_util.cc
@@ -216,7 +216,7 @@ bool ExtractNamedURL(GtkSelectionData* selection_data,
       reinterpret_cast<const char*>(
           gtk_selection_data_get_data(selection_data)),
       gtk_selection_data_get_length(selection_data));
-  void* iter = NULL;
+  PickleIterator iter(data);
   std::string title_utf8, url_utf8;
   if (!data.ReadString(&iter, &title_utf8) ||
       !data.ReadString(&iter, &url_utf8)) {
diff --git a/ui/base/dragdrop/os_exchange_data_win_unittest.cc b/ui/base/dragdrop/os_exchange_data_win_unittest.cc
index 42b5daf..abc67c7 100644
--- a/ui/base/dragdrop/os_exchange_data_win_unittest.cc
+++ b/ui/base/dragdrop/os_exchange_data_win_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.
 
@@ -350,11 +350,11 @@ TEST(OSExchangeDataTest, TestPickledData) {
 
   Pickle restored_pickle;
   EXPECT_TRUE(copy.GetPickledData(test_cf, &restored_pickle));
-  void* p_iterator = NULL;
+  PickleIterator iterator(restored_pickle);
   int value;
-  EXPECT_TRUE(restored_pickle.ReadInt(&p_iterator, &value));
+  EXPECT_TRUE(restored_pickle.ReadInt(&iterator, &value));
   EXPECT_EQ(1, value);
-  EXPECT_TRUE(restored_pickle.ReadInt(&p_iterator, &value));
+  EXPECT_TRUE(restored_pickle.ReadInt(&iterator, &value));
   EXPECT_EQ(2, value);
 }