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, 19 insertions, 22 deletions

diff --git a/crypto/secure_hash.h b/crypto/secure_hash.h
index 4a49380..28b78c1 100644
--- a/crypto/secure_hash.h
+++ b/crypto/secure_hash.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.
 
@@ -10,6 +10,7 @@
 #include "crypto/crypto_export.h"
 
 class Pickle;
+class PickleIterator;
 
 namespace crypto {
 
@@ -36,7 +37,7 @@ class CRYPTO_EXPORT SecureHash {
   // |data_iterator| allows this to be used as part of a larger pickle.
   // |pickle| holds the saved data.
   // Returns success or failure.
-  virtual bool Deserialize(void** data_iterator, Pickle* pickle) = 0;
+  virtual bool Deserialize(PickleIterator* data_iterator) = 0;
 
  protected:
   SecureHash() {}
diff --git a/crypto/secure_hash_default.cc b/crypto/secure_hash_default.cc
index 6607dfe..834a276 100644
--- a/crypto/secure_hash_default.cc
+++ b/crypto/secure_hash_default.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.
 
@@ -36,7 +36,7 @@ class SecureHashSHA256NSS : public SecureHash {
   }
 
   virtual bool Serialize(Pickle* pickle);
-  virtual bool Deserialize(void** data_iterator, Pickle* pickle);
+  virtual bool Deserialize(PickleIterator* data_iterator);
 
  private:
   SHA256Context ctx_;
@@ -55,26 +55,23 @@ bool SecureHashSHA256NSS::Serialize(Pickle* pickle) {
   return true;
 }
 
-bool SecureHashSHA256NSS::Deserialize(void** data_iterator, Pickle* pickle) {
-  if (!pickle)
-    return false;
-
+bool SecureHashSHA256NSS::Deserialize(PickleIterator* data_iterator) {
   int version;
-  if (!pickle->ReadInt(data_iterator, &version))
+  if (!data_iterator->ReadInt(&version))
     return false;
 
   if (version > kSecureHashVersion)
     return false;  // We don't know how to deal with this.
 
   std::string type;
-  if (!pickle->ReadString(data_iterator, &type))
+  if (!data_iterator->ReadString(&type))
     return false;
 
   if (type != kSHA256Descriptor)
     return false;  // It's the wrong kind.
 
   const char* data = NULL;
-  if (!pickle->ReadBytes(data_iterator, &data, sizeof(ctx_)))
+  if (!data_iterator->ReadBytes(&data, sizeof(ctx_)))
     return false;
 
   memcpy(&ctx_, data, sizeof(ctx_));
diff --git a/crypto/secure_hash_openssl.cc b/crypto/secure_hash_openssl.cc
index 098bf27..a542e22 100644
--- a/crypto/secure_hash_openssl.cc
+++ b/crypto/secure_hash_openssl.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.
 
@@ -40,7 +40,7 @@ class SecureHashSHA256OpenSSL : public SecureHash {
   }
 
   virtual bool Serialize(Pickle* pickle);
-  virtual bool Deserialize(void** data_iterator, Pickle* pickle);
+  virtual bool Deserialize(PickleIterator* data_iterator);
 
  private:
   SHA256_CTX ctx_;
@@ -59,27 +59,26 @@ bool SecureHashSHA256OpenSSL::Serialize(Pickle* pickle) {
   return true;
 }
 
-bool SecureHashSHA256OpenSSL::Deserialize(void** data_iterator,
-                                          Pickle* pickle) {
-  if (!pickle)
+bool SecureHashSHA256OpenSSL::Deserialize(PickleIterator* data_iterator) {
+  if (!data_iterator)
     return false;
 
   int version;
-  if (!pickle->ReadInt(data_iterator, &version))
+  if (!data_iterator->ReadInt(&version))
     return false;
 
   if (version > kSecureHashVersion)
     return false;  // We don't know how to deal with this.
 
   std::string type;
-  if (!pickle->ReadString(data_iterator, &type))
+  if (!data_iterator->ReadString(&type))
     return false;
 
   if (type != kSHA256Descriptor)
     return false;  // It's the wrong kind.
 
   const char* data = NULL;
-  if (!pickle->ReadBytes(data_iterator, &data, sizeof(ctx_)))
+  if (!data_iterator->ReadBytes(&data, sizeof(ctx_)))
     return false;
 
   memcpy(&ctx_, data, sizeof(ctx_));
diff --git a/crypto/secure_hash_unittest.cc b/crypto/secure_hash_unittest.cc
index 2adddfb..42b9ade3 100644
--- a/crypto/secure_hash_unittest.cc
+++ b/crypto/secure_hash_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.
 
@@ -62,8 +62,8 @@ TEST(SecureHashTest, TestSerialization) {
 
   ctx1->Finish(output1, sizeof(output1));
 
-  void* data_iterator = NULL;
-  EXPECT_TRUE(ctx2->Deserialize(&data_iterator, &pickle));
+  PickleIterator data_iterator(pickle);
+  EXPECT_TRUE(ctx2->Deserialize(&data_iterator));
   ctx2->Update(input4.data(), input4.size());
   ctx2->Update(input5.data(), input5.size());