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

14 files changed, 21 insertions, 17 deletions

diff --git a/net/base/x509_certificate.cc b/net/base/x509_certificate.cc
index 0a301e0..cb9b798 100644
--- a/net/base/x509_certificate.cc
+++ b/net/base/x509_certificate.cc
@@ -324,7 +324,7 @@ X509Certificate* X509Certificate::CreateFromBytes(const char* data,
 
 // static
 X509Certificate* X509Certificate::CreateFromPickle(const Pickle& pickle,
-                                                   void** pickle_iter,
+                                                   PickleIterator* pickle_iter,
                                                    PickleType type) {
   OSCertHandle cert_handle = ReadOSCertHandleFromPickle(pickle, pickle_iter);
   if (!cert_handle)
diff --git a/net/base/x509_certificate.h b/net/base/x509_certificate.h
index 0a603b1..9e00e71 100644
--- a/net/base/x509_certificate.h
+++ b/net/base/x509_certificate.h
@@ -37,6 +37,7 @@ struct CERTCertificateStr;
 #endif
 
 class Pickle;
+class PickleIterator;
 
 namespace crypto {
 class RSAPrivateKey;
@@ -181,7 +182,7 @@ class NET_EXPORT X509Certificate
   //
   // The returned pointer must be stored in a scoped_refptr<X509Certificate>.
   static X509Certificate* CreateFromPickle(const Pickle& pickle,
-                                           void** pickle_iter,
+                                           PickleIterator* pickle_iter,
                                            PickleType type);
 
   // Parses all of the certificates possible from |data|. |format| is a
@@ -554,7 +555,7 @@ class NET_EXPORT X509Certificate
   // libraries, nor acceptable to CreateFromBytes(). Returns an invalid
   // handle, NULL, on failure.
   static OSCertHandle ReadOSCertHandleFromPickle(const Pickle& pickle,
-                                                 void** pickle_iter);
+                                                 PickleIterator* pickle_iter);
 
   // Writes a single certificate to |pickle|. Returns false on failure.
   static bool WriteOSCertHandleToPickle(OSCertHandle handle, Pickle* pickle);
diff --git a/net/base/x509_certificate_mac.cc b/net/base/x509_certificate_mac.cc
index 8541973..afd622f 100644
--- a/net/base/x509_certificate_mac.cc
+++ b/net/base/x509_certificate_mac.cc
@@ -1565,7 +1565,7 @@ CFArrayRef X509Certificate::CreateOSCertChainForCert() const {
 // static
 X509Certificate::OSCertHandle
 X509Certificate::ReadOSCertHandleFromPickle(const Pickle& pickle,
-                                            void** pickle_iter) {
+                                            PickleIterator* pickle_iter) {
   const char* data;
   int length;
   if (!pickle.ReadData(pickle_iter, &data, &length))
diff --git a/net/base/x509_certificate_nss.cc b/net/base/x509_certificate_nss.cc
index bc72304..edded15 100644
--- a/net/base/x509_certificate_nss.cc
+++ b/net/base/x509_certificate_nss.cc
@@ -1146,7 +1146,7 @@ SHA1Fingerprint X509Certificate::CalculateCAFingerprint(
 // static
 X509Certificate::OSCertHandle
 X509Certificate::ReadOSCertHandleFromPickle(const Pickle& pickle,
-                                            void** pickle_iter) {
+                                            PickleIterator* pickle_iter) {
   const char* data;
   int length;
   if (!pickle.ReadData(pickle_iter, &data, &length))
diff --git a/net/base/x509_certificate_openssl.cc b/net/base/x509_certificate_openssl.cc
index 3efb989..6eef06e 100644
--- a/net/base/x509_certificate_openssl.cc
+++ b/net/base/x509_certificate_openssl.cc
@@ -650,7 +650,7 @@ bool X509Certificate::IsSameOSCert(X509Certificate::OSCertHandle a,
 // static
 X509Certificate::OSCertHandle
 X509Certificate::ReadOSCertHandleFromPickle(const Pickle& pickle,
-                                            void** pickle_iter) {
+                                            PickleIterator* pickle_iter) {
   const char* data;
   int length;
   if (!pickle.ReadData(pickle_iter, &data, &length))
diff --git a/net/base/x509_certificate_unittest.cc b/net/base/x509_certificate_unittest.cc
index 7e25cbb..a69df0a 100644
--- a/net/base/x509_certificate_unittest.cc
+++ b/net/base/x509_certificate_unittest.cc
@@ -1026,7 +1026,7 @@ TEST(X509CertificateTest, Pickle) {
   Pickle pickle;
   cert->Persist(&pickle);
 
-  void* iter = NULL;
+  PickleIterator iter(pickle);
   scoped_refptr<X509Certificate> cert_from_pickle =
       X509Certificate::CreateFromPickle(
           pickle, &iter, X509Certificate::PICKLETYPE_CERTIFICATE_CHAIN);
diff --git a/net/base/x509_certificate_win.cc b/net/base/x509_certificate_win.cc
index 3c80457..a6b186e 100644
--- a/net/base/x509_certificate_win.cc
+++ b/net/base/x509_certificate_win.cc
@@ -1061,7 +1061,7 @@ SHA1Fingerprint X509Certificate::CalculateCAFingerprint(
 // static
 X509Certificate::OSCertHandle
 X509Certificate::ReadOSCertHandleFromPickle(const Pickle& pickle,
-                                            void** pickle_iter) {
+                                            PickleIterator* pickle_iter) {
   const char* data;
   int length;
   if (!pickle.ReadData(pickle_iter, &data, &length))
diff --git a/net/http/http_response_headers.cc b/net/http/http_response_headers.cc
index 4916498..33d78da 100644
--- a/net/http/http_response_headers.cc
+++ b/net/http/http_response_headers.cc
@@ -169,7 +169,8 @@ HttpResponseHeaders::HttpResponseHeaders(const std::string& raw_input)
                                    GetAllHttpResponseCodes());
 }
 
-HttpResponseHeaders::HttpResponseHeaders(const Pickle& pickle, void** iter)
+HttpResponseHeaders::HttpResponseHeaders(const Pickle& pickle,
+                                         PickleIterator* iter)
     : response_code_(-1) {
   std::string raw_input;
   if (pickle.ReadString(iter, &raw_input))
diff --git a/net/http/http_response_headers.h b/net/http/http_response_headers.h
index 0ab162e..6fbc0bb 100644
--- a/net/http/http_response_headers.h
+++ b/net/http/http_response_headers.h
@@ -16,6 +16,7 @@
 #include "net/http/http_version.h"
 
 class Pickle;
+class PickleIterator;
 
 namespace base {
 class Time;
@@ -52,7 +53,7 @@ class NET_EXPORT HttpResponseHeaders
   // Initializes from the representation stored in the given pickle.  The data
   // for this object is found relative to the given pickle_iter, which should
   // be passed to the pickle's various Read* methods.
-  HttpResponseHeaders(const Pickle& pickle, void** pickle_iter);
+  HttpResponseHeaders(const Pickle& pickle, PickleIterator* pickle_iter);
 
   // Appends a representation of this object to the given pickle.
   // The options argument can be a combination of PersistOptions.
diff --git a/net/http/http_response_headers_unittest.cc b/net/http/http_response_headers_unittest.cc
index 775946c..f7c5e6f5 100644
--- a/net/http/http_response_headers_unittest.cc
+++ b/net/http/http_response_headers_unittest.cc
@@ -475,7 +475,7 @@ TEST(HttpResponseHeadersTest, Persist) {
     Pickle pickle;
     parsed1->Persist(&pickle, tests[i].options);
 
-    void* iter = NULL;
+    PickleIterator iter(pickle);
     scoped_refptr<net::HttpResponseHeaders> parsed2(
         new net::HttpResponseHeaders(pickle, &iter));
 
diff --git a/net/http/http_response_info.cc b/net/http/http_response_info.cc
index 951e2b6..542bdb0 100644
--- a/net/http/http_response_info.cc
+++ b/net/http/http_response_info.cc
@@ -116,7 +116,7 @@ HttpResponseInfo& HttpResponseInfo::operator=(const HttpResponseInfo& rhs) {
 
 bool HttpResponseInfo::InitFromPickle(const Pickle& pickle,
                                       bool* response_truncated) {
-  void* iter = NULL;
+  PickleIterator iter(pickle);
 
   // read flags and verify version
   int flags;
diff --git a/net/http/http_vary_data.cc b/net/http/http_vary_data.cc
index 882dc78..c716ca0 100644
--- a/net/http/http_vary_data.cc
+++ b/net/http/http_vary_data.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.
 
@@ -64,7 +64,7 @@ bool HttpVaryData::Init(const HttpRequestInfo& request_info,
   return is_valid_ = true;
 }
 
-bool HttpVaryData::InitFromPickle(const Pickle& pickle, void** iter) {
+bool HttpVaryData::InitFromPickle(const Pickle& pickle, PickleIterator* iter) {
   is_valid_ = false;
   const char* data;
   if (pickle.ReadBytes(iter, &data, sizeof(request_digest_))) {
diff --git a/net/http/http_vary_data.h b/net/http/http_vary_data.h
index baa4d88..4e7a542 100644
--- a/net/http/http_vary_data.h
+++ b/net/http/http_vary_data.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 "net/base/net_export.h"
 
 class Pickle;
+class PickleIterator;
 
 namespace net {
 
@@ -52,7 +53,7 @@ class NET_EXPORT_PRIVATE HttpVaryData {
   // is_valid() will return true.  Otherwise, false is returned to indicate
   // that this object is marked as invalid.
   //
-  bool InitFromPickle(const Pickle& pickle, void** pickle_iter);
+  bool InitFromPickle(const Pickle& pickle, PickleIterator* pickle_iter);
 
   // Call this method to persist the vary data. Illegal to call this on an
   // invalid object.
diff --git a/net/socket/ssl_host_info.cc b/net/socket/ssl_host_info.cc
index bc4a43e..f4edcc6 100644
--- a/net/socket/ssl_host_info.cc
+++ b/net/socket/ssl_host_info.cc
@@ -64,7 +64,7 @@ bool SSLHostInfo::ParseInner(const std::string& data) {
   State* state = mutable_state();
 
   Pickle p(data.data(), data.size());
-  void* iter = NULL;
+  PickleIterator iter(p);
 
   int num_der_certs;
   if (!p.ReadInt(&iter, &num_der_certs) ||