Re-implement PageState serialization without a Blink API dependency.

content/common/page_state_serialization.cc

Index: content/common/page_state_serialization.cc
diff --git a/webkit/glue/glue_serialize_deprecated.cc b/content/common/page_state_serialization.cc
similarity index 19%
rename from webkit/glue/glue_serialize_deprecated.cc
rename to content/common/page_state_serialization.cc
index 62d4c8984f289a7ac0670d77d8ccd614b560d87a..13b323c547a5c7bd0b2d08a1742cd3f4640e49e2 100644
--- a/webkit/glue/glue_serialize_deprecated.cc
+++ b/content/common/page_state_serialization.cc
@@ -1,104 +1,213 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright (c) 2013 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.
 
-#include "webkit/glue/glue_serialize_deprecated.h"
+#include "content/common/page_state_serialization.h"
 
-#include <string>
+#include <algorithm>
+#include <limits>
 
 #include "base/pickle.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
-#include "googleurl/src/gurl.h"
-#include "third_party/WebKit/public/platform/WebData.h"
-#include "third_party/WebKit/public/platform/WebHTTPBody.h"
-#include "third_party/WebKit/public/platform/WebPoint.h"
-#include "third_party/WebKit/public/platform/WebString.h"
-#include "third_party/WebKit/public/platform/WebURL.h"
-#include "third_party/WebKit/public/platform/WebVector.h"
-#include "third_party/WebKit/public/web/WebHistoryItem.h"
-#include "third_party/WebKit/public/web/WebSerializedScriptValue.h"
 #include "ui/gfx/screen.h"
-#include "webkit/base/file_path_string_conversions.h"
 
-using WebKit::WebData;
-using WebKit::WebHistoryItem;
-using WebKit::WebHTTPBody;
-using WebKit::WebPoint;
-using WebKit::WebSerializedScriptValue;
-using WebKit::WebString;
-using WebKit::WebUChar;
-using WebKit::WebVector;
+namespace content {
+namespace {
 
-namespace webkit_glue {
+//-----------------------------------------------------------------------------
+
+void AppendDataToHttpBody(ExplodedHttpBody* http_body, const char* data,
+                          int data_length) {
+  ExplodedHttpBodyElement element;
+  element.type = WebKit::WebHTTPBody::Element::TypeData;
+  element.data.assign(data, data_length);
+  http_body->elements.push_back(element);
+}
+
+void AppendFileRangeToHttpBody(ExplodedHttpBody* http_body,
+                               const base::NullableString16& file_path,
+                               int file_start,
+                               int file_length,
+                               double file_modification_time) {
+  ExplodedHttpBodyElement element;
+  element.type = WebKit::WebHTTPBody::Element::TypeFile;
+  element.file_path = file_path;
+  element.file_start = file_start;
+  element.file_length = file_length;
+  element.file_modification_time = file_modification_time;
+  http_body->elements.push_back(element);
+}
+
+void AppendURLRangeToHttpBody(ExplodedHttpBody* http_body,
+                              const GURL& url,
+                              int file_start,
+                              int file_length,
+                              double file_modification_time) {
+  ExplodedHttpBodyElement element;
+  element.type = WebKit::WebHTTPBody::Element::TypeURL;
+  element.url = url;
+  element.file_start = file_start;
+  element.file_length = file_length;
+  element.file_modification_time = file_modification_time;
+  http_body->elements.push_back(element);
+}
+
+void AppendBlobToHttpBody(ExplodedHttpBody* http_body, const GURL& url) {
+  ExplodedHttpBodyElement element;
+  element.type = WebKit::WebHTTPBody::Element::TypeBlob;
+  element.url = url;
+  http_body->elements.push_back(element);
+}
+
+//----------------------------------------------------------------------------
+
+void ExtractReferencedFilesFromHttpBody(
+    const std::vector<ExplodedHttpBodyElement>& elements,
+    std::vector<base::NullableString16>* referenced_files) {
+  for (size_t i = 0; i < elements.size(); ++i) {
+    if (elements[i].type == WebKit::WebHTTPBody::Element::TypeFile)
+      referenced_files->push_back(elements[i].file_path);
+  }
+}
 
-namespace {
+bool ExtractReferencedFilesFromDocumentState(
+    const std::vector<base::NullableString16>& state,

[jamesr, 2013/06/21 23:24:53]

could we call this document_state?

[darin (slow to review), 2013/06/24 08:02:10]

Done.

+    std::vector<base::NullableString16>* referenced_files) {
+  if (state.empty())
+    return true;
 
-enum IncludeFormData {
-  NEVER_INCLUDE_FORM_DATA,
-  INCLUDE_FORM_DATA_WITHOUT_PASSWORDS,
-  ALWAYS_INCLUDE_FORM_DATA
-};
+  // This algorithm is adapted from Blink's core/html/FormController.cpp code.
+  // We only care about how that code worked when this code snapshot was taken
+  // as this code is only needed for backwards compat.

[jamesr, 2013/06/21 23:24:53]

could we have a link to that file at that rev?

[darin (slow to review), 2013/06/24 08:02:10]

Done.

+
+  size_t index = 0;
+
+  if (state.size() < 3)
+    return false;
+
+  index++;  // Skip over magic signature.
+  index++;  // Skip over form key.
+
+  size_t item_count;
+  if (!base::StringToSizeT(state[index++].string(), &item_count))
+    return false;
+
+  while (item_count--) {
+    if (index + 1 >= state.size())
+      return false;
+
+    state[index++];  // name

[jamesr, 2013/06/21 23:24:53]

the state[] access here is a bit confusing - how about just "index++;" ?

[darin (slow to review), 2013/06/24 08:02:10]

Done.  Yeah, there was no point to the state[] bit.

+    const base::NullableString16& type = state[index++];
+
+    if (index >= state.size())
+      return false;
+
+    size_t value_size;
+    if (!base::StringToSizeT(state[index++].string(), &value_size))

[jamesr, 2013/06/21 23:24:53]

should we check state[index].is_null() ? is the string() value for a is_null()
nullable_string ever meaningful?

[darin (slow to review), 2013/06/24 08:02:10]

If the string is null, then the string() accessor will return an empty string. 
This isn't actually enforced by base::NullableString16.  I have it on my TODO
list to make it so.  In this case, we can count on it because all of the
NullableString16's came from the ReadString function in this file.

+      return false;
+
+    if (index + value_size > state.size())
+      return false;
+
+    if (EqualsASCII(type.string(), "file")) {
+      if (value_size != 2)
+        return false;
+
+      referenced_files->push_back(state[index++]);
+      index++;  // Skip over display name.
+    } else {
+      index += value_size;
+    }
+  }
+
+  return true;
+}
+
+bool RecursivelyExtractReferencedFiles(
+    const ExplodedFrameState& frame_state,
+    std::vector<base::NullableString16>* referenced_files) {
+  if (!frame_state.http_body.is_null) {
+    ExtractReferencedFilesFromHttpBody(frame_state.http_body.elements,
+                                       referenced_files);
+  }
+
+  if (!ExtractReferencedFilesFromDocumentState(frame_state.document_state,
+                                               referenced_files))
+    return false;
+
+  for (size_t i = 0; i < frame_state.children.size(); ++i) {
+    if (!RecursivelyExtractReferencedFiles(frame_state.children[i],
+                                           referenced_files))
+      return false;
+  }
+
+  return true;
+}
+
+//----------------------------------------------------------------------------
 
 struct SerializeObject {
-  SerializeObject() : version(0) {}
+  SerializeObject()
+      : version(0),
+        parse_error(false) {
+  }
+
   SerializeObject(const char* data, int len)
-      : pickle(data, len), version(0) { iter = PickleIterator(pickle); }
+      : pickle(data, len),
+        version(0),
+        parse_error(false) {
+    iter = PickleIterator(pickle);
+  }
 
   std::string GetAsString() {
     return std::string(static_cast<const char*>(pickle.data()), pickle.size());
   }
 
   Pickle pickle;
-  mutable PickleIterator iter;
-  mutable int version;
+  PickleIterator iter;
+  int version;
+  bool parse_error;
 };
 
-// TODO(mpcomplete): obsolete versions 1 and 2 after 1/1/2008.
-// Version ID used in reading/writing history items.
-// 1: Initial revision.
-// 2: Added case for NULL string versus "". Version 2 code can read Version 1
-//    data, but not vice versa.
-// 3: Version 2 was broken, it stored number of WebUChars, not number of bytes.
-//    This version checks and reads v1 and v2 correctly.
-// 4: Adds support for storing FormData::identifier().
-// 5: Adds support for empty FormData
-// 6: Adds support for documentSequenceNumbers
-// 7: Adds support for stateObject
-// 8: Adds support for file range and modification time
-// 9: Adds support for itemSequenceNumbers
-// 10: Adds support for blob
-// 11: Adds support for pageScaleFactor
-// 12: Adds support for hasPasswordData in HTTP body
+// Version ID of serialized format.
+// 11: Min version
+// 12: Adds support for contains_passwords in HTTP body
 // 13: Adds support for URL (FileSystem URL)
 // 14: Adds list of referenced files, version written only for first item.
-// Should be const, but unit tests may modify it.
 //
 // NOTE: If the version is -1, then the pickle contains only a URL string.
-// See CreateHistoryStateForURL.
+// See ReadPageState.
 //
-int kVersion = 14;
+const int kMinVersion = 11;
+const int kCurrentVersion = 14;
+
+// A bunch of convenience functions to read/write to SerializeObjects.  The
+// de-serializers assume the input data will be in the correct format and fall
+// back to returning safe defaults when not.
 
-// A bunch of convenience functions to read/write to SerializeObjects.
-// The serializers assume the input data is in the correct format and so does
-// no error checking.
 void WriteData(const void* data, int length, SerializeObject* obj) {
   obj->pickle.WriteData(static_cast<const char*>(data), length);
 }
 
-void ReadData(const SerializeObject* obj, const void** data, int* length) {
+void ReadData(SerializeObject* obj, const void** data, int* length) {
   const char* tmp;
   if (obj->pickle.ReadData(&obj->iter, &tmp, length)) {
     *data = tmp;
   } else {
+    obj->parse_error = true;
     *data = NULL;
     *length = 0;
   }
 }
 
-bool ReadBytes(const SerializeObject* obj, const void** data, int length) {
+bool ReadBytes(SerializeObject* obj, const void** data, int length) {
   const char *tmp;
-  if (!obj->pickle.ReadBytes(&obj->iter, &tmp, length))
+  if (!obj->pickle.ReadBytes(&obj->iter, &tmp, length)) {
+    obj->parse_error = true;
     return false;
+  }
   *data = tmp;
   return true;
 }
@@ -107,14 +216,15 @@ void WriteInteger(int data, SerializeObject* obj) {
   obj->pickle.WriteInt(data);
 }
 
-int ReadInteger(const SerializeObject* obj) {
+int ReadInteger(SerializeObject* obj) {
   int tmp;
   if (obj->pickle.ReadInt(&obj->iter, &tmp))
     return tmp;
+  obj->parse_error = true;
   return 0;
 }
 
-void ConsumeInteger(const SerializeObject* obj) {
+void ConsumeInteger(SerializeObject* obj) {
   int unused ALLOW_UNUSED = ReadInteger(obj);
 }
 
@@ -122,17 +232,19 @@ void WriteInteger64(int64 data, SerializeObject* obj) {
   obj->pickle.WriteInt64(data);
 }
 
-int64 ReadInteger64(const SerializeObject* obj) {
+int64 ReadInteger64(SerializeObject* obj) {
   int64 tmp = 0;
-  obj->pickle.ReadInt64(&obj->iter, &tmp);
-  return tmp;
+  if (obj->pickle.ReadInt64(&obj->iter, &tmp))
+    return tmp;
+  obj->parse_error = true;
+  return 0;
 }
 
 void WriteReal(double data, SerializeObject* obj) {
   WriteData(&data, sizeof(double), obj);

[jamesr, 2013/06/21 23:24:53]

hmm, we're reading/writing the double's bits raw, but also using the
implementation-defined value std::numeric_limits<double>::quite_NaN(). would
anything bad happen if after a toolchain upgrade the quite_NaN bit pattern
changed and we read it as a different sort of NaN? maybe we should re-normalize
all NaN values to the quite_NaN() value on reading?  i'm not really sure when
the difference between that NaN and other NaNs would matter

[darin (slow to review), 2013/06/24 08:02:10]

In at least the case where Read/WriteReal is used for the file_modification_time
field, we probably need its notion of "unitialized value" to match the return
value of Blink's invalidFileTime() method.  That's where the use of quiet_NaN
comes from.  Unfortunately, I'm having to clone some default initializers, which
may be a point of fragility.

At any rate, the double format seems to have been stable for a long time as we
seem to have been serializing/de-serializing them since the start without a
hitch.

 }
 
-double ReadReal(const SerializeObject* obj) {
+double ReadReal(SerializeObject* obj) {
   const void* tmp = NULL;
   int length = 0;
   double value = 0.0;
@@ -140,6 +252,8 @@ double ReadReal(const SerializeObject* obj) {
   if (tmp && length >= static_cast<int>(sizeof(double))) {
     // Use memcpy, as tmp may not be correctly aligned.
     memcpy(&value, tmp, sizeof(double));
+  } else {
+    obj->parse_error = true;
   }
   return value;
 }
@@ -148,10 +262,11 @@ void WriteBoolean(bool data, SerializeObject* obj) {
   obj->pickle.WriteInt(data ? 1 : 0);
 }
 
-bool ReadBoolean(const SerializeObject* obj) {
+bool ReadBoolean(SerializeObject* obj) {
   bool tmp;
   if (obj->pickle.ReadBool(&obj->iter, &tmp))
     return tmp;
+  obj->parse_error = true;
   return false;
 }
 
@@ -159,349 +274,243 @@ void WriteGURL(const GURL& url, SerializeObject* obj) {
   obj->pickle.WriteString(url.possibly_invalid_spec());
 }
 
-GURL ReadGURL(const SerializeObject* obj) {
+GURL ReadGURL(SerializeObject* obj) {
   std::string spec;
   if (obj->pickle.ReadString(&obj->iter, &spec))
     return GURL(spec);
+  obj->parse_error = true;
   return GURL();
 }
 
-// Read/WriteString pickle the WebString as <int length><WebUChar* data>.
-// If length == -1, then the WebString itself is NULL (WebString()).
-// Otherwise the length is the number of WebUChars (not bytes) in the WebString.
-void WriteString(const WebString& str, SerializeObject* obj) {
-  base::string16 string = str;
-  const char16* data = string.data();
-  size_t length_in_uchars = string.length();
-  size_t length_in_bytes = length_in_uchars * sizeof(char16);
-  switch (kVersion) {
-    case 1:
-      // Version 1 writes <length in bytes><string data>.
-      // It saves WebString() and "" as "".
-      obj->pickle.WriteInt(length_in_bytes);
-      obj->pickle.WriteBytes(data, length_in_bytes);
-      break;
-    case 2:
-      // Version 2 writes <length in WebUChar><string data>.
-      // It uses -1 in the length field to mean WebString().
-      if (str.isNull()) {
-        obj->pickle.WriteInt(-1);
-      } else {
-        obj->pickle.WriteInt(length_in_uchars);
-        obj->pickle.WriteBytes(data, length_in_bytes);
-      }
-      break;
-    default:
-      // Version 3+ writes <length in bytes><string data>.
-      // It uses -1 in the length field to mean WebString().
-      if (str.isNull()) {
-        obj->pickle.WriteInt(-1);
-      } else {
-        obj->pickle.WriteInt(length_in_bytes);
-        obj->pickle.WriteBytes(data, length_in_bytes);
-      }
-      break;
+// WriteString pickles the NullableString16 as <int length><char16* data>.
+// If length == -1, then the NullableString16 itself is null.  Otherwise the
+// length is the number of char16 (not bytes) in the NullableString16.
+void WriteString(const base::NullableString16& str, SerializeObject* obj) {
+  const char16* data = str.string().data();
+  size_t length_in_bytes = str.string().length() * sizeof(char16);
+  if (str.is_null()) {
+    obj->pickle.WriteInt(-1);
+  } else {
+    obj->pickle.WriteInt(length_in_bytes);
+    obj->pickle.WriteBytes(data, length_in_bytes);
   }
 }
 
-// This reads a serialized WebString from obj. If a string can't be read,
-// WebString() is returned.
-const WebUChar* ReadStringNoCopy(const SerializeObject* obj, int* num_chars) {
-  int length;
-
-  // Versions 1, 2, and 3 all start with an integer.
-  if (!obj->pickle.ReadInt(&obj->iter, &length))
+// This reads a serialized NullableString16 from obj. If a string can't be
+// read, NULL is returned.
+const char16* ReadStringNoCopy(SerializeObject* obj, int* num_chars) {
+  int length_in_bytes;
+  if (!obj->pickle.ReadInt(&obj->iter, &length_in_bytes)) {
+    obj->parse_error = true;
     return NULL;
+  }
 
-  // Starting with version 2, -1 means WebString().
-  if (length == -1)
+  if (length_in_bytes == -1)
     return NULL;
 
-  // In version 2, the length field was the length in WebUChars.
-  // In version 1 and 3 it is the length in bytes.
-  int bytes = length;
-  if (obj->version == 2)
-    bytes *= sizeof(WebUChar);
-
   const void* data;
-  if (!ReadBytes(obj, &data, bytes))
+  if (!ReadBytes(obj, &data, length_in_bytes)) {
+    obj->parse_error = true;
     return NULL;
+  }
 
   if (num_chars)
-    *num_chars = bytes / sizeof(WebUChar);
-  return static_cast<const WebUChar*>(data);
+    *num_chars = length_in_bytes / sizeof(char16);
+  return static_cast<const char16*>(data);
 }
 
-WebString ReadString(const SerializeObject* obj) {
+base::NullableString16 ReadString(SerializeObject* obj) {
   int num_chars;
-  const WebUChar* chars = ReadStringNoCopy(obj, &num_chars);
-  return chars ? WebString(chars, num_chars) : WebString();
+  const char16* chars = ReadStringNoCopy(obj, &num_chars);
+  return chars ?
+      base::NullableString16(base::string16(chars, num_chars), false) :
+      base::NullableString16();
 }
 
-void ConsumeString(const SerializeObject* obj) {
-  const WebUChar* unused ALLOW_UNUSED = ReadStringNoCopy(obj, NULL);
+void ConsumeString(SerializeObject* obj) {
+  const char16* unused ALLOW_UNUSED = ReadStringNoCopy(obj, NULL);
 }
 
-// Writes a Vector of Strings into a SerializeObject for serialization.
+// Writes a Vector of strings into a SerializeObject for serialization.
 void WriteStringVector(
-    const WebVector<WebString>& data, SerializeObject* obj) {
+    const std::vector<base::NullableString16>& data, SerializeObject* obj) {
   WriteInteger(static_cast<int>(data.size()), obj);

[jamesr, 2013/06/21 23:24:53]

what if data.size() > INT_MAX? it looks like this code will write out a
truncated value and then try to write out lots and lots of objects. the Pickle
reserve will almost certainly die trying to allocate memory, but it might be
safer to error out early here.  the reader is even more aggressive - it'll
reject all values >= INT_MAX / sizeof(b::NS16)

[darin (slow to review), 2013/06/24 08:02:10]

Yeah, good catch.  This code doesn't do much at all to ensure that it is writing
out good data.  I'll add a CHECK instead of trying to plumb an early return due
to error.

-  for (size_t i = 0, c = data.size(); i < c; ++i) {
-    unsigned ui = static_cast<unsigned>(i);  // sigh
-    WriteString(data[ui], obj);
+  for (size_t i = 0; i < data.size(); ++i) {
+    WriteString(data[i], obj);
   }
 }
 
-WebVector<WebString> ReadStringVector(const SerializeObject* obj) {
-  int num_elements = ReadInteger(obj);
-  WebVector<WebString> result(static_cast<size_t>(num_elements));
-  for (int i = 0; i < num_elements; ++i)
-    result[i] = ReadString(obj);
-  return result;
-}
+void ReadStringVector(SerializeObject* obj,
+                      std::vector<base::NullableString16>* result) {
+  size_t num_elements = static_cast<size_t>(ReadInteger(obj));
 
-void ConsumeStringVector(const SerializeObject* obj) {
-  int num_elements = ReadInteger(obj);
-  for (int i = 0; i < num_elements; ++i)
-    ConsumeString(obj);
+  // Ensure that num_elements makes sense.
+  if (INT_MAX / sizeof(base::NullableString16) <= num_elements) {

[jamesr, 2013/06/21 23:26:53]

rather than duplicating this check for the resize() below maybe we could have a
ReadSize() function that read an integer, verified that it was in the range [0,
remaining_pickle_length / sizeof(field)] and then cast it to size_t to use in
the rest of the code.

[darin (slow to review), 2013/06/24 08:02:10]

I went with a ReadAndValidateVectorSize method.  It doesn't divide by
sizeof(field) because it is not clear that sizeof(field) is meaningful in
determining how much data should be left in the pickle.  For example, a
NullableString16 is 8 bytes in size, but it is serialized with only 4 bytes when
its value is null.  Ditto for the way that ExplodedHTTPBody is serialized.

+    obj->parse_error = true;
+    return;
+  }
+
+  result->resize(num_elements);
+  for (size_t i = 0; i < num_elements; ++i)
+    (*result)[i] = ReadString(obj);
 }
 
-// Writes a FormData object into a SerializeObject for serialization.
-void WriteFormData(const WebHTTPBody& http_body, SerializeObject* obj) {
-  WriteBoolean(!http_body.isNull(), obj);
+// Writes an ExplodedHttpBody object into a SerializeObject for serialization.
+void WriteHttpBody(const ExplodedHttpBody& http_body, SerializeObject* obj) {
+  WriteBoolean(!http_body.is_null, obj);
 
-  if (http_body.isNull())
+  if (http_body.is_null)
     return;
 
-  WriteInteger(static_cast<int>(http_body.elementCount()), obj);
-  WebHTTPBody::Element element;
-  for (size_t i = 0; http_body.elementAt(i, element); ++i) {
+  WriteInteger(static_cast<int>(http_body.elements.size()), obj);

[jamesr, 2013/06/21 23:24:53]

this could write out a bogus value if size() is ridiculous

[darin (slow to review), 2013/06/24 08:02:10]

OK.  I went with a function named WriteAndValidateVectorSize.

+  for (size_t i = 0; i < http_body.elements.size(); ++i) {
+    const ExplodedHttpBodyElement& element = http_body.elements[i];
     WriteInteger(element.type, obj);
-    if (element.type == WebHTTPBody::Element::TypeData) {
+    if (element.type == WebKit::WebHTTPBody::Element::TypeData) {
       WriteData(element.data.data(), static_cast<int>(element.data.size()),
                 obj);
-    } else if (element.type == WebHTTPBody::Element::TypeFile) {
-      WriteString(element.filePath, obj);
-      WriteInteger64(element.fileStart, obj);
-      WriteInteger64(element.fileLength, obj);
-      WriteReal(element.modificationTime, obj);
-    } else if (element.type == WebHTTPBody::Element::TypeURL) {
+    } else if (element.type == WebKit::WebHTTPBody::Element::TypeFile) {
+      WriteString(element.file_path, obj);
+      WriteInteger64(element.file_start, obj);
+      WriteInteger64(element.file_length, obj);
+      WriteReal(element.file_modification_time, obj);
+    } else if (element.type == WebKit::WebHTTPBody::Element::TypeURL) {
       WriteGURL(element.url, obj);
-      WriteInteger64(element.fileStart, obj);
-      WriteInteger64(element.fileLength, obj);
-      WriteReal(element.modificationTime, obj);
+      WriteInteger64(element.file_start, obj);
+      WriteInteger64(element.file_length, obj);
+      WriteReal(element.file_modification_time, obj);
     } else {
       WriteGURL(element.url, obj);
     }
   }
-  WriteInteger64(http_body.identifier(), obj);
-  WriteBoolean(http_body.containsPasswordData(), obj);
+  WriteInteger64(http_body.identifier, obj);
+  WriteBoolean(http_body.contains_passwords, obj);
 }
 
-WebHTTPBody ReadFormData(const SerializeObject* obj) {
-  // In newer versions, an initial boolean indicates if we have form data.
-  if (obj->version >= 5 && !ReadBoolean(obj))
-    return WebHTTPBody();
+void ReadHttpBody(SerializeObject* obj, ExplodedHttpBody* http_body) {
+  // An initial boolean indicates if we have an HTTP body.
+  if (!ReadBoolean(obj))
+    return;
+  http_body->is_null = false;
 
-  // In older versions, 0 elements implied no form data.
   int num_elements = ReadInteger(obj);
-  if (num_elements == 0 && obj->version < 5)
-    return WebHTTPBody();
-
-  WebHTTPBody http_body;
-  http_body.initialize();
 
   for (int i = 0; i < num_elements; ++i) {
     int type = ReadInteger(obj);
-    if (type == WebHTTPBody::Element::TypeData) {
+    if (type == WebKit::WebHTTPBody::Element::TypeData) {
       const void* data;
       int length = -1;
       ReadData(obj, &data, &length);
-      if (length >= 0)
-        http_body.appendData(WebData(static_cast<const char*>(data), length));
-    } else if (type == WebHTTPBody::Element::TypeFile) {
-      WebString file_path = ReadString(obj);
-      long long file_start = 0;
-      long long file_length = -1;
-      double modification_time = 0.0;
-      if (obj->version >= 8) {
-        file_start = ReadInteger64(obj);
-        file_length = ReadInteger64(obj);
-        modification_time = ReadReal(obj);
+      if (length >= 0) {
+        AppendDataToHttpBody(http_body, static_cast<const char*>(data),
+                             length);
       }
-      http_body.appendFileRange(file_path, file_start, file_length,
-                                modification_time);
-    } else if (type == WebHTTPBody::Element::TypeURL) {
+    } else if (type == WebKit::WebHTTPBody::Element::TypeFile) {
+      base::NullableString16 file_path = ReadString(obj);
+      int64 file_start = ReadInteger64(obj);
+      int64 file_length = ReadInteger64(obj);
+      double file_modification_time = ReadReal(obj);
+      AppendFileRangeToHttpBody(http_body, file_path, file_start, file_length,
+                                file_modification_time);
+    } else if (type == WebKit::WebHTTPBody::Element::TypeURL) {
       GURL url = ReadGURL(obj);
-      long long file_start = 0;
-      long long file_length = -1;
-      double modification_time = 0.0;
-      file_start = ReadInteger64(obj);
-      file_length = ReadInteger64(obj);
-      modification_time = ReadReal(obj);
-      http_body.appendURLRange(url, file_start, file_length,
-                               modification_time);
-    } else if (obj->version >= 10) {
+      int64 file_start = ReadInteger64(obj);
+      int64 file_length = ReadInteger64(obj);
+      double file_modification_time = ReadReal(obj);
+      AppendURLRangeToHttpBody(http_body, url, file_start, file_length,
+                               file_modification_time);
+    } else if (type == WebKit::WebHTTPBody::Element::TypeBlob) {
       GURL blob_url = ReadGURL(obj);
-      http_body.appendBlob(blob_url);
+      AppendBlobToHttpBody(http_body, blob_url);
     }
   }
-  if (obj->version >= 4)
-    http_body.setIdentifier(ReadInteger64(obj));
+  http_body->identifier = ReadInteger64(obj);
 
   if (obj->version >= 12)
-    http_body.setContainsPasswordData(ReadBoolean(obj));
-
-  return http_body;
+    http_body->contains_passwords = ReadBoolean(obj);
 }
 
-// Writes the HistoryItem data into the SerializeObject object for
+// Writes the ExplodedFrameState data into the SerializeObject object for
 // serialization.
-void WriteHistoryItem(
-    const WebHistoryItem& item, SerializeObject* obj, bool is_top) {
+void WriteFrameState(
+    const ExplodedFrameState& state, SerializeObject* obj, bool is_top) {
   // WARNING: This data may be persisted for later use. As such, care must be
   // taken when changing the serialized format. If a new field needs to be
   // written, only adding at the end will make it easier to deal with loading
   // older versions. Similarly, this should NOT save fields with sensitive
   // data, such as password fields.
 
-  if (kVersion >= 14) {
-    if (is_top) {
-      WriteInteger(kVersion, obj);
-
-      // Insert the list of referenced files, so they can be extracted easily
-      // from the serialized data (avoiding the need to call into Blink again).
-      WriteStringVector(item.getReferencedFilePaths(), obj);
-    }
-  } else {
-    WriteInteger(kVersion, obj);
-  }
-
-  WriteString(item.urlString(), obj);
-  WriteString(item.originalURLString(), obj);
-  WriteString(item.target(), obj);
-  WriteString(item.parent(), obj);
-  WriteString(item.title(), obj);
-  WriteString(item.alternateTitle(), obj);
-  WriteReal(item.lastVisitedTime(), obj);
-  WriteInteger(item.scrollOffset().x, obj);
-  WriteInteger(item.scrollOffset().y, obj);
-  WriteBoolean(item.isTargetItem(), obj);
-  WriteInteger(item.visitCount(), obj);
-  WriteString(item.referrer(), obj);
-
-  WriteStringVector(item.documentState(), obj);
-
-  if (kVersion >= 11)
-    WriteReal(item.pageScaleFactor(), obj);
-  if (kVersion >= 9)
-    WriteInteger64(item.itemSequenceNumber(), obj);
-  if (kVersion >= 6)
-    WriteInteger64(item.documentSequenceNumber(), obj);
-  if (kVersion >= 7) {
-    bool has_state_object = !item.stateObject().isNull();
-    WriteBoolean(has_state_object, obj);
-    if (has_state_object)
-      WriteString(item.stateObject().toString(), obj);
-  }
-
-  WriteFormData(item.httpBody(), obj);
-  WriteString(item.httpContentType(), obj);
-  if (kVersion < 14)
-    WriteString(item.referrer(), obj);
+  WriteString(state.url_string, obj);
+  WriteString(state.original_url_string, obj);
+  WriteString(state.target, obj);
+  WriteString(state.parent, obj);
+  WriteString(state.title, obj);
+  WriteString(state.alternate_title, obj);
+  WriteReal(state.visited_time, obj);
+  WriteInteger(state.scroll_offset.x(), obj);
+  WriteInteger(state.scroll_offset.y(), obj);
+  WriteBoolean(state.is_target_item, obj);
+  WriteInteger(state.visit_count, obj);
+  WriteString(state.referrer, obj);
+
+  WriteStringVector(state.document_state, obj);
+
+  WriteReal(state.page_scale_factor, obj);
+  WriteInteger64(state.item_sequence_number, obj);
+  WriteInteger64(state.document_sequence_number, obj);
+
+  bool has_state_object = !state.state_object.is_null();
+  WriteBoolean(has_state_object, obj);

[jamesr, 2013/06/21 23:24:53]

hmm, why is this different from the other base::NullableString16()s? if we're
storing the nullity for this out-of-band in the serialized format, should it
just be a bool+string16 pair in the struct to reflect that? if we're just
matching the way this was serialized on disk in the past i think it deserves a
comment indicating that up to version XXX (14, i guess?) this string is
serialized differently from other strings on the struct

[darin (slow to review), 2013/06/24 08:02:10]

Added a comment.  Previously, we were serializing a WebSerializedScriptValue,
which could be null.  We called its toString() method and then serialized that. 
Hence, two checks for null as you could have a null WebSerializedScriptValue or
a non-null WebSerializedScriptValue with a null string value.  Ugh.  This subtle
distinction is now lost, which seems fine to me.

+  if (has_state_object)
+    WriteString(state.state_object, obj);
+
+  WriteHttpBody(state.http_body, obj);
+  WriteString(state.http_body.http_content_type, obj);

[jamesr, 2013/06/21 23:24:53]

why isn't this part of WriteHttpBody() ?

[darin (slow to review), 2013/06/24 08:02:10]

Just historical.  I would move it, but it is actually a bit awkward to move
since {Read,Write}HttpBody have early returns when the HTTP body is null.  The
HTTP content type still needs to be read / written even when the HTTP body is
null, which is a quirk of the format to say the least :-(

I added a comment.

 
   // Subitems
-  const WebVector<WebHistoryItem>& children = item.children();
+  const std::vector<ExplodedFrameState>& children = state.children;
   WriteInteger(static_cast<int>(children.size()), obj);
   for (size_t i = 0, c = children.size(); i < c; ++i)
-    WriteHistoryItem(children[i], obj, false);
+    WriteFrameState(children[i], obj, false);
 }
 
-// Creates a new HistoryItem tree based on the serialized string.
-// Assumes the data is in the format returned by WriteHistoryItem.
-WebHistoryItem ReadHistoryItem(
-    const SerializeObject* obj,
-    IncludeFormData include_form_data,
-    bool include_scroll_offset,
-    bool is_top) {
-  if (is_top) {
-    obj->version = ReadInteger(obj);
+void ReadFrameState(SerializeObject* obj, bool is_top,
+                    ExplodedFrameState* state) {
+  if (obj->version < 14 && !is_top)
+    ConsumeInteger(obj);  // Skip over redundant version field.
 
-    if (obj->version == -1) {
-      GURL url = ReadGURL(obj);
-      WebHistoryItem item;
-      item.initialize();
-      item.setURLString(WebString::fromUTF8(url.possibly_invalid_spec()));
-      return item;
-    }
+  state->url_string = ReadString(obj);
+  state->original_url_string = ReadString(obj);
+  state->target = ReadString(obj);
+  state->parent = ReadString(obj);
+  state->title = ReadString(obj);
+  state->alternate_title = ReadString(obj);
+  state->visited_time = ReadReal(obj);
 
-    if (obj->version > kVersion || obj->version < 1)
-      return WebHistoryItem();
+  int x = ReadInteger(obj);
+  int y = ReadInteger(obj);
+  state->scroll_offset = gfx::Point(x, y);
 
-    if (obj->version >= 14)
-      ConsumeStringVector(obj);  // Skip over list of referenced files.
-  } else if (obj->version < 14) {
-    ConsumeInteger(obj);  // Skip over redundant version field.
-  }
+  state->is_target_item = ReadBoolean(obj);
+  state->visit_count = ReadInteger(obj);
+  state->referrer = ReadString(obj);
 
-  WebHistoryItem item;
-  item.initialize();
+  ReadStringVector(obj, &state->document_state);
 
-  item.setURLString(ReadString(obj));
-  item.setOriginalURLString(ReadString(obj));
-  item.setTarget(ReadString(obj));
-  item.setParent(ReadString(obj));
-  item.setTitle(ReadString(obj));
-  item.setAlternateTitle(ReadString(obj));
-  item.setLastVisitedTime(ReadReal(obj));
+  state->page_scale_factor = ReadReal(obj);
+  state->item_sequence_number = ReadInteger64(obj);
+  state->document_sequence_number = ReadInteger64(obj);
 
-  int x = ReadInteger(obj);
-  int y = ReadInteger(obj);
-  if (include_scroll_offset)
-    item.setScrollOffset(WebPoint(x, y));
-
-  item.setIsTargetItem(ReadBoolean(obj));
-  item.setVisitCount(ReadInteger(obj));
-  item.setReferrer(ReadString(obj));
-
-  item.setDocumentState(ReadStringVector(obj));
-
-  if (obj->version >= 11)
-    item.setPageScaleFactor(ReadReal(obj));
-  if (obj->version >= 9)
-    item.setItemSequenceNumber(ReadInteger64(obj));
-  if (obj->version >= 6)
-    item.setDocumentSequenceNumber(ReadInteger64(obj));
-  if (obj->version >= 7) {
-    bool has_state_object = ReadBoolean(obj);
-    if (has_state_object) {
-      item.setStateObject(
-          WebSerializedScriptValue::fromString(ReadString(obj)));
-    }
-  }
+  bool has_state_object = ReadBoolean(obj);
+  if (has_state_object)
+    state->state_object = ReadString(obj);
 
-  // The extra referrer string is read for backwards compat.
-  const WebHTTPBody& http_body = ReadFormData(obj);
-  const WebString& http_content_type = ReadString(obj);
+  ReadHttpBody(obj, &state->http_body);
+  state->http_body.http_content_type = ReadString(obj);
 
   if (obj->version < 14)
     ConsumeString(obj);  // Skip unused referrer string.
 
-  if (include_form_data == ALWAYS_INCLUDE_FORM_DATA ||
-      (include_form_data == INCLUDE_FORM_DATA_WITHOUT_PASSWORDS &&
-       !http_body.isNull() && !http_body.containsPasswordData())) {
-    // Include the full HTTP body.
-    item.setHTTPBody(http_body);
-    item.setHTTPContentType(http_content_type);
-  } else if (!http_body.isNull()) {
-    // Don't include the data in the HTTP body, but include its identifier. This
-    // enables fetching data from the cache.
-    WebHTTPBody empty_http_body;
-    empty_http_body.initialize();
-    empty_http_body.setIdentifier(http_body.identifier());
-    item.setHTTPBody(empty_http_body);
-  }
-
 #if defined(OS_ANDROID)
   if (obj->version == 11) {
     // Now-unused values that shipped in this version of Chrome for Android when
@@ -509,14 +518,13 @@ WebHistoryItem ReadHistoryItem(
     ReadReal(obj);
     ReadBoolean(obj);
 
-    // In this version, pageScaleFactor included deviceScaleFactor and scroll
+    // In this version, page_scale_factor included deviceScaleFactor and scroll
     // offsets were premultiplied by pageScaleFactor.
-    if (item.pageScaleFactor()) {
-      if (include_scroll_offset)
-        item.setScrollOffset(
-            WebPoint(item.scrollOffset().x / item.pageScaleFactor(),
-                     item.scrollOffset().y / item.pageScaleFactor()));
-      item.setPageScaleFactor(item.pageScaleFactor() /
+    if (state->page_scale_factor) {
+      state->scroll_offset =
+          gfx::Point(state->scroll_offset.x() / state->page_scale_factor,
+                     state->scroll_offset.y() / state->page_scale_factor);
+      state->page_scale_factor = (state->page_scale_factor /
           gfx::Screen::GetNativeScreen()->GetPrimaryDisplay()
               .device_scale_factor());
     }
@@ -525,146 +533,103 @@ WebHistoryItem ReadHistoryItem(
 
   // Subitems
   int num_children = ReadInteger(obj);
+  state->children.resize(num_children);

[jamesr, 2013/06/21 23:24:53]

i think we need to validate this number a bit more.  if it's negative, the
resize() call will implicitly convert to a (probably really big) unsigned number
and try to allocate that which'll crash.  we could make sure it's <= INT_MAX /
sizeof(ExplodedFrameState), or even better make sure that the length isn't such
that we'll have to read past the end of the pickle to plausibly decode this
number of frame state entries

[darin (slow to review), 2013/06/24 08:02:10]

It turns out that the cursor position of the PickleIterator is not publicly
exposed, which would be needed to write such ideal checks.

I can check against the entire payload size of the pickle to at least perform a
ball-park check.

   for (int i = 0; i < num_children; ++i)
-    item.appendToChildren(ReadHistoryItem(obj,
-                                          include_form_data,
-                                          include_scroll_offset,
-                                          false));
-
-  return item;
+    ReadFrameState(obj, false, &state->children[i]);
 }
 
-// Reconstruct a HistoryItem from a string, using our JSON Value deserializer.
-// This assumes that the given serialized string has all the required key,value
-// pairs, and does minimal error checking. The form data of the post is restored
-// if |include_form_data| is |ALWAYS_INCLUDE_FORM_DATA| or if the data doesn't
-// contain passwords and |include_form_data| is
-// |INCLUDE_FORM_DATA_WITHOUT_PASSWORDS|. Otherwise the form data is empty. If
-// |include_scroll_offset| is true, the scroll offset is restored.
-WebHistoryItem HistoryItemFromString(
-    const std::string& serialized_item,
-    IncludeFormData include_form_data,
-    bool include_scroll_offset) {
-  if (serialized_item.empty())
-    return WebHistoryItem();
-
-  SerializeObject obj(serialized_item.data(),
-                      static_cast<int>(serialized_item.length()));
-  return ReadHistoryItem(&obj, include_form_data, include_scroll_offset, true);
+void WritePageState(const ExplodedPageState& state, SerializeObject* obj) {
+  WriteInteger(obj->version, obj);
+  WriteStringVector(state.referenced_files, obj);
+  WriteFrameState(state.top, obj, true);
 }
 
-void ToFilePathVector(const WebVector<WebString>& input,
-                      std::vector<base::FilePath>* output) {
-  for (size_t i = 0; i < input.size(); ++i)
-    output->push_back(webkit_base::WebStringToFilePath(input[i]));
-}
-
-}  // namespace
+void ReadPageState(SerializeObject* obj, ExplodedPageState* state) {
+  obj->version = ReadInteger(obj);
 
-// Serialize a HistoryItem to a string, using our JSON Value serializer.
-std::string HistoryItemToString(const WebHistoryItem& item) {
-  if (item.isNull())
-    return std::string();
+  if (obj->version == -1) {
+    GURL url = ReadGURL(obj);
+    state->top.url_string =
+        base::NullableString16(UTF8ToUTF16(url.possibly_invalid_spec()), false);

[jamesr, 2013/06/21 23:24:53]

might be worth a comment that GURL::possibly_invalid_spec() will always return
valid UTF8 code no matter what the input to explain why the UTF8ToUTF16() call
doesn't have to be checked

[darin (slow to review), 2013/06/24 08:02:10]

Done.  I'm so used to coding with GURL ;-)

+    return;
+  }
 
-  SerializeObject obj;
-  WriteHistoryItem(item, &obj, true);
-  return obj.GetAsString();
-}
+  if (obj->version > kCurrentVersion || obj->version < kMinVersion) {
+    obj->parse_error = true;
+    return;
+  }
 
-WebHistoryItem HistoryItemFromString(const std::string& serialized_item) {
-  return HistoryItemFromString(serialized_item, ALWAYS_INCLUDE_FORM_DATA, true);
-}
+  if (obj->version >= 14)
+    ReadStringVector(obj, &state->referenced_files);
 
-std::vector<base::FilePath> FilePathsFromHistoryState(
-    const std::string& content_state) {
-  // TODO(darin): We should avoid using the WebKit API here, so that we do not
-  // need to have WebKit initialized before calling this method.
+  ReadFrameState(obj, true, &state->top);
 
-  std::vector<base::FilePath> result;
+  if (obj->version < 14)
+    RecursivelyExtractReferencedFiles(state->top, &state->referenced_files);
 
-  // In newer versions of the format, the set of referenced files is computed
-  // at serialization time.
-  SerializeObject obj(content_state.data(),
-                      static_cast<int>(content_state.length()));
-  obj.version = ReadInteger(&obj);
+  // De-dupe
+  state->referenced_files.erase(
+      std::unique(state->referenced_files.begin(),
+                  state->referenced_files.end()),
+      state->referenced_files.end());
+}
 
-  if (obj.version > kVersion || obj.version < 1)
-    return result;
+}  // namespace
 
-  if (obj.version >= 14) {
-    ToFilePathVector(ReadStringVector(&obj), &result);
-  } else {
-    // TODO(darin): Delete this code path after we branch for M29.
-    const WebHistoryItem& item =
-        HistoryItemFromString(content_state, ALWAYS_INCLUDE_FORM_DATA, true);
-    if (!item.isNull())
-      ToFilePathVector(item.getReferencedFilePaths(), &result);
-  }
-  return result;
+ExplodedHttpBodyElement::ExplodedHttpBodyElement()
+    : type(WebKit::WebHTTPBody::Element::TypeData),
+      file_start(0),
+      file_length(-1),
+      file_modification_time(std::numeric_limits<double>::quiet_NaN()) {
 }
 
-// For testing purposes only.
-void HistoryItemToVersionedString(const WebHistoryItem& item, int version,
-                                  std::string* serialized_item) {
-  if (item.isNull()) {
-    serialized_item->clear();
-    return;
-  }
+ExplodedHttpBodyElement::~ExplodedHttpBodyElement() {
+}
 
-  // Temporarily change the version.
-  int real_version = kVersion;
-  kVersion = version;
+ExplodedHttpBody::ExplodedHttpBody()
+    : identifier(0),
+      contains_passwords(false),
+      is_null(true) {
+}
 
-  SerializeObject obj;
-  WriteHistoryItem(item, &obj, true);
-  *serialized_item = obj.GetAsString();
+ExplodedHttpBody::~ExplodedHttpBody() {
+}
 
-  kVersion = real_version;
+ExplodedFrameState::ExplodedFrameState()
+    : item_sequence_number(0),
+      document_sequence_number(0),
+      visit_count(0),
+      visited_time(0.0),
+      page_scale_factor(0.0),
+      is_target_item(false) {
 }
 
-int HistoryItemCurrentVersion() {
-  return kVersion;
+ExplodedFrameState::~ExplodedFrameState() {
 }
 
-std::string RemovePasswordDataFromHistoryState(
-    const std::string& content_state) {
-  // TODO(darin): We should avoid using the WebKit API here, so that we do not
-  // need to have WebKit initialized before calling this method.
-  const WebHistoryItem& item =
-      HistoryItemFromString(
-          content_state, INCLUDE_FORM_DATA_WITHOUT_PASSWORDS, true);
-  if (item.isNull()) {
-    // Couldn't parse the string, return an empty string.
-    return std::string();
-  }
+ExplodedPageState::ExplodedPageState() {
+}
 
-  return HistoryItemToString(item);
+ExplodedPageState::~ExplodedPageState() {
 }
 
-std::string RemoveScrollOffsetFromHistoryState(
-    const std::string& content_state) {
-  // TODO(darin): We should avoid using the WebKit API here, so that we do not
-  // need to have WebKit initialized before calling this method.
-  const WebHistoryItem& item =
-      HistoryItemFromString(content_state, ALWAYS_INCLUDE_FORM_DATA, false);
-  if (item.isNull()) {
-    // Couldn't parse the string, return an empty string.
-    return std::string();
-  }
+bool DecodePageState(const std::string& encoded, ExplodedPageState* exploded) {
+  *exploded = ExplodedPageState();
+
+  if (encoded.empty())
+    return true;
 
-  return HistoryItemToString(item);
+  SerializeObject obj(encoded.data(), static_cast<int>(encoded.size()));
+  ReadPageState(&obj, exploded);
+  return !obj.parse_error;
 }
 
-std::string CreateHistoryStateForURL(const GURL& url) {
-  // We avoid using the WebKit API here, so that we do not need to have WebKit
-  // initialized before calling this method.  Instead, we write a simple
-  // serialization of the given URL with a dummy version number of -1.  This
-  // will be interpreted by ReadHistoryItem as a request to create a default
-  // WebHistoryItem.
+bool EncodePageState(const ExplodedPageState& exploded, std::string* encoded) {
   SerializeObject obj;
-  WriteInteger(-1, &obj);
-  WriteGURL(url, &obj);
-  return obj.GetAsString();
+  obj.version = kCurrentVersion;
+  WritePageState(exploded, &obj);
+  *encoded = obj.GetAsString();
+  return true;
 }
 
-}  // namespace webkit_glue
+}  // namespace content