Don't serialize frame id or original url history parameters. They are obsolete.

BUG=341425,340277
TEST=PageStateSerializationTest.BackwardsCompat_v18

Review URL: https://codereview.chromium.org/192813004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@256089 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 38 insertions, 25 deletions

diff --git a/content/common/page_state_serialization.cc b/content/common/page_state_serialization.cc
index 9e57150..edef549 100644
--- a/content/common/page_state_serialization.cc
+++ b/content/common/page_state_serialization.cc
@@ -189,12 +189,14 @@ struct SerializeObject {
 // 16: Switched from blob urls to blob uuids.
 // 17: Add a target frame id number.
 // 18: Add referrer policy.
+// 19: Remove target frame id, which was a bad idea, and original url string,
+//         which is no longer used.
 //
 // NOTE: If the version is -1, then the pickle contains only a URL string.
 // See ReadPageState.
 //
 const int kMinVersion = 11;
-const int kCurrentVersion = 18;
+const int kCurrentVersion = 19;
 
 // 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
@@ -243,6 +245,10 @@ int64 ReadInteger64(SerializeObject* obj) {
   return 0;
 }
 
+void ConsumeInteger64(SerializeObject* obj) {
+  int64 unused ALLOW_UNUSED = ReadInteger64(obj);
+}
+
 void WriteReal(double data, SerializeObject* obj) {
   WriteData(&data, sizeof(double), obj);
 }
@@ -493,7 +499,6 @@ void WriteFrameState(
   // data, such as password fields.
 
   WriteString(state.url_string, obj);
-  WriteString(state.original_url_string, obj);
   WriteString(state.target, obj);
   WriteInteger(state.scroll_offset.x(), obj);
   WriteInteger(state.scroll_offset.y(), obj);
@@ -504,7 +509,6 @@ void WriteFrameState(
   WriteReal(state.page_scale_factor, obj);
   WriteInteger64(state.item_sequence_number, obj);
   WriteInteger64(state.document_sequence_number, obj);
-  WriteInteger64(state.target_frame_id, obj);
   WriteInteger(state.referrer_policy, obj);
 
   bool has_state_object = !state.state_object.is_null();
@@ -532,7 +536,10 @@ void ReadFrameState(SerializeObject* obj, bool is_top,
     ConsumeInteger(obj);  // Skip over redundant version field.
 
   state->url_string = ReadString(obj);
-  state->original_url_string = ReadString(obj);
+
+  if (obj->version < 19)
+    ConsumeString(obj);  // Skip obsolete original url string field.
+
   state->target = ReadString(obj);
   if (obj->version < 15) {
     ConsumeString(obj);  // Skip obsolete parent field.
@@ -556,8 +563,10 @@ void ReadFrameState(SerializeObject* obj, bool is_top,
   state->page_scale_factor = ReadReal(obj);
   state->item_sequence_number = ReadInteger64(obj);
   state->document_sequence_number = ReadInteger64(obj);
-  if (obj->version >= 17)
-    state->target_frame_id = ReadInteger64(obj);
+
+  if (obj->version >= 17 && obj->version < 19)
+    ConsumeInteger64(obj); // Skip obsolete target frame id number.
+
   if (obj->version >= 18) {
     state->referrer_policy =
         static_cast<blink::WebReferrerPolicy>(ReadInteger(obj));
@@ -621,7 +630,7 @@ void ReadPageState(SerializeObject* obj, ExplodedPageState* state) {
   if (obj->version == -1) {
     GURL url = ReadGURL(obj);
     // NOTE: GURL::possibly_invalid_spec() always returns valid UTF-8.
-    state->top.url_string = state->top.original_url_string =
+    state->top.url_string =
         base::NullableString16(
             base::UTF8ToUTF16(url.possibly_invalid_spec()), false);
     return;
@@ -671,7 +680,6 @@ ExplodedHttpBody::~ExplodedHttpBody() {
 ExplodedFrameState::ExplodedFrameState()
     : item_sequence_number(0),
       document_sequence_number(0),
-      target_frame_id(0),
       page_scale_factor(0.0),
       referrer_policy(blink::WebReferrerPolicyDefault) {
 }
diff --git a/content/common/page_state_serialization.h b/content/common/page_state_serialization.h
index 1a4ddc85..43efb53 100644
--- a/content/common/page_state_serialization.h
+++ b/content/common/page_state_serialization.h
@@ -43,7 +43,6 @@ struct CONTENT_EXPORT ExplodedHttpBody {
 
 struct CONTENT_EXPORT ExplodedFrameState {
   base::NullableString16 url_string;
-  base::NullableString16 original_url_string;
   base::NullableString16 referrer;
   base::NullableString16 target;
   base::NullableString16 state_object;
@@ -51,7 +50,6 @@ struct CONTENT_EXPORT ExplodedFrameState {
   gfx::Point scroll_offset;
   int64 item_sequence_number;
   int64 document_sequence_number;
-  int64 target_frame_id;
   double page_scale_factor;
   blink::WebReferrerPolicy referrer_policy;
   ExplodedHttpBody http_body;
diff --git a/content/common/page_state_serialization_unittest.cc b/content/common/page_state_serialization_unittest.cc
index cce9347..2b5a181 100644
--- a/content/common/page_state_serialization_unittest.cc
+++ b/content/common/page_state_serialization_unittest.cc
@@ -67,7 +67,6 @@ void ExpectEquality(const ExplodedHttpBody& a, const ExplodedHttpBody& b) {
 template <>
 void ExpectEquality(const ExplodedFrameState& a, const ExplodedFrameState& b) {
   EXPECT_EQ(a.url_string, b.url_string);
-  EXPECT_EQ(a.original_url_string, b.original_url_string);
   EXPECT_EQ(a.referrer, b.referrer);
   EXPECT_EQ(a.referrer_policy, b.referrer_policy);
   EXPECT_EQ(a.target, b.target);
@@ -76,7 +75,6 @@ void ExpectEquality(const ExplodedFrameState& a, const ExplodedFrameState& b) {
   EXPECT_EQ(a.scroll_offset, b.scroll_offset);
   EXPECT_EQ(a.item_sequence_number, b.item_sequence_number);
   EXPECT_EQ(a.document_sequence_number, b.document_sequence_number);
-  EXPECT_EQ(a.target_frame_id, b.target_frame_id);
   EXPECT_EQ(a.page_scale_factor, b.page_scale_factor);
   ExpectEquality(a.http_body, b.http_body);
   ExpectEquality(a.children, b.children);
@@ -94,7 +92,6 @@ class PageStateSerializationTest : public testing::Test {
   void PopulateFrameState(ExplodedFrameState* frame_state) {
     // Invent some data for the various fields.
     frame_state->url_string = NS16("http://dev.chromium.org/");
-    frame_state->original_url_string = frame_state->url_string;
     frame_state->referrer = NS16("https://www.google.com/search?q=dev.chromium.org");
     frame_state->referrer_policy = blink::WebReferrerPolicyAlways;
     frame_state->target = NS16("foo");
@@ -106,7 +103,6 @@ class PageStateSerializationTest : public testing::Test {
     frame_state->scroll_offset = gfx::Point(0, 100);
     frame_state->item_sequence_number = 1;
     frame_state->document_sequence_number = 2;
-    frame_state->target_frame_id = 3;
     frame_state->page_scale_factor = 2.0;
   }
 
@@ -137,7 +133,6 @@ class PageStateSerializationTest : public testing::Test {
       ExplodedFrameState* frame_state,
       bool is_child) {
     frame_state->url_string = NS16("http://chromium.org/");
-    frame_state->original_url_string = frame_state->url_string;
     frame_state->referrer = NS16("http://google.com/");
     frame_state->referrer_policy = blink::WebReferrerPolicyDefault;
     if (!is_child)
@@ -145,7 +140,6 @@ class PageStateSerializationTest : public testing::Test {
     frame_state->scroll_offset = gfx::Point(42, -42);
     frame_state->item_sequence_number = 123;
     frame_state->document_sequence_number = 456;
-    frame_state->target_frame_id = 0;
     frame_state->page_scale_factor = 2.0f;
 
     frame_state->document_state.push_back(
@@ -424,5 +418,9 @@ TEST_F(PageStateSerializationTest, BackwardsCompat_v16) {
   TestBackwardsCompat(16);
 }
 
+TEST_F(PageStateSerializationTest, BackwardsCompat_v18) {
+  TestBackwardsCompat(18);
+}
+
 }  // namespace
 }  // namespace content
diff --git a/content/public/common/page_state.cc b/content/public/common/page_state.cc
index 761e4f6..3cf1659 100644
--- a/content/public/common/page_state.cc
+++ b/content/public/common/page_state.cc
@@ -65,8 +65,7 @@ PageState PageState::CreateFromEncodedData(const std::string& data) {
 PageState PageState::CreateFromURL(const GURL& url) {
   ExplodedPageState state;
 
-  state.top.url_string = state.top.original_url_string =
-      ToNullableString16(url.possibly_invalid_spec());
+  state.top.url_string = ToNullableString16(url.possibly_invalid_spec());
 
   return ToPageState(state);
 }
@@ -79,8 +78,7 @@ PageState PageState::CreateForTesting(
     const base::FilePath* optional_body_file_path) {
   ExplodedPageState state;
 
-  state.top.url_string = state.top.original_url_string =
-      ToNullableString16(url.possibly_invalid_spec());
+  state.top.url_string = ToNullableString16(url.possibly_invalid_spec());
 
   if (optional_body_data || optional_body_file_path) {
     state.top.http_body.is_null = false;
diff --git a/content/public/renderer/history_item_serialization.cc b/content/public/renderer/history_item_serialization.cc
index 1c4c1dd..ef1c2a2d 100644
--- a/content/public/renderer/history_item_serialization.cc
+++ b/content/public/renderer/history_item_serialization.cc
@@ -82,7 +82,6 @@ void AppendHTTPBodyElement(const ExplodedHttpBodyElement& element,
 bool RecursivelyGenerateFrameState(const WebHistoryItem& item,
                                    ExplodedFrameState* state) {
   state->url_string = item.urlString();
-  state->original_url_string = item.originalURLString();
   state->referrer = item.referrer();
   state->referrer_policy = item.referrerPolicy();
   state->target = item.target();
@@ -92,7 +91,6 @@ bool RecursivelyGenerateFrameState(const WebHistoryItem& item,
   state->item_sequence_number = item.itemSequenceNumber();
   state->document_sequence_number =
       item.documentSequenceNumber();
-  state->target_frame_id = item.targetFrameID();
   state->page_scale_factor = item.pageScaleFactor();
   ToNullableString16Vector(item.documentState(), &state->document_state);
 
@@ -122,7 +120,6 @@ bool RecursivelyGenerateFrameState(const WebHistoryItem& item,
 bool RecursivelyGenerateHistoryItem(const ExplodedFrameState& state,
                                     WebHistoryItem* item) {
   item->setURLString(state.url_string);
-  item->setOriginalURLString(state.original_url_string);
   item->setReferrer(state.referrer, state.referrer_policy);
   item->setTarget(state.target);
   if (!state.state_object.is_null()) {
@@ -141,8 +138,6 @@ bool RecursivelyGenerateHistoryItem(const ExplodedFrameState& state,
   if (state.document_sequence_number)
     item->setDocumentSequenceNumber(state.document_sequence_number);
 
-  item->setTargetFrameID(state.target_frame_id);
-
   item->setHTTPContentType(state.http_body.http_content_type);
   if (!state.http_body.is_null) {
     WebHTTPBody http_body;
diff --git a/content/test/data/page_state/serialized_v18.dat b/content/test/data/page_state/serialized_v18.dat
new file mode 100644
index 0000000..4345c44
--- /dev/null
+++ b/content/test/data/page_state/serialized_v18.dat
@@ -0,0 +1,16 @@
+dAMAABMAAAABAAAAEAAAAGYAaQBsAGUALgB0AHgAdAAoAAAAaAB0AHQAcAA6AC8ALwBjAGgAcgBv
+AG0AaQB1AG0ALgBvAHIAZwAvAAwAAAB0AGEAcgBnAGUAdAAqAAAA1v///yQAAABoAHQAdABwADoA
+LwAvAGcAbwBvAGcAbABlAC4AYwBvAG0ALwAIAAAAYAAAAAoADQA/ACUAIABXAGUAYgBLAGkAdAAg
+AHMAZQByAGkAYQBsAGkAegBlAGQAIABmAG8AcgBtACAAcwB0AGEAdABlACAAdgBlAHIAcwBpAG8A
+bgAgADgAIAAKAA0APQAmABAAAABmAG8AcgBtACAAawBlAHkAAgAAADEAAAAGAAAAZgBvAG8AAAAI
+AAAAZgBpAGwAZQACAAAAMgAAABAAAABmAGkAbABlAC4AdAB4AHQAFgAAAGQAaQBzAHAAbABhAHkA
+TgBhAG0AZQAAAAgAAAAAAAAAAAAAQHsAAAAAAAAAyAEAAAAAAAABAAAAAAAAAAEAAAADAAAAAAAA
+ABAAAABmaXJzdCBkYXRhIGJsb2NrAQAAABAAAABmAGkAbABlAC4AdAB4AHQAAAAAAAAAAAD/////
+/////wgAAAAAAAAAAAD4fwAAAAAPAAAAZGF0YSB0aGUgc2Vjb25kABUDAAAAAAAAAAAAAA4AAABm
+AG8AbwAvAGIAYQByAAAAAQAAACgAAABoAHQAdABwADoALwAvAGMAaAByAG8AbQBpAHUAbQAuAG8A
+cgBnAC8A/////yoAAADW////JAAAAGgAdAB0AHAAOgAvAC8AZwBvAG8AZwBsAGUALgBjAG8AbQAv
+AAgAAABgAAAACgANAD8AJQAgAFcAZQBiAEsAaQB0ACAAcwBlAHIAaQBhAGwAaQB6AGUAZAAgAGYA
+bwByAG0AIABzAHQAYQB0AGUAIAB2AGUAcgBzAGkAbwBuACAAOAAgAAoADQA9ACYAEAAAAGYAbwBy
+AG0AIABrAGUAeQACAAAAMQAAAAYAAABmAG8AbwAAAAgAAABmAGkAbABlAAIAAAAyAAAAEAAAAGYA
+aQBsAGUALgB0AHgAdAAWAAAAZABpAHMAcABsAGEAeQBOAGEAbQBlAAAACAAAAAAAAAAAAABAewAA
+AAAAAADIAQAAAAAAAAEAAAAAAAAAAAAAAP////8AAAAA