Move referrer policy to a different field when serializing.

components/sessions/serialized_navigation_entry.cc

Index: components/sessions/serialized_navigation_entry.cc
diff --git a/components/sessions/serialized_navigation_entry.cc b/components/sessions/serialized_navigation_entry.cc
index 4b041427bdca4ac042b1326acfb4094fe4e8f059..d78394d069e39162e404f4a9831502cb0734e5f4 100644
--- a/components/sessions/serialized_navigation_entry.cc
+++ b/components/sessions/serialized_navigation_entry.cc
@@ -36,8 +36,19 @@ SerializedNavigationEntry SerializedNavigationEntry::FromSyncData(
   SerializedNavigationEntry navigation;
   navigation.index_ = index;
   navigation.unique_id_ = sync_data.unique_id();
-  navigation.referrer_url_ = GURL(sync_data.referrer());
-  navigation.referrer_policy_ = sync_data.referrer_policy();
+  if (sync_data.has_correct_referrer_policy()) {

[Andrew T Wilson (Slow), 2015/01/29 11:02:13]

Does this mean that if you have an old client and you modify the referrer
policy, the change won't take effect on newer clients because they are only
looking at "correct_referrer_policy" now?

[jochen (gone - plz use gerrit), 2015/01/29 12:04:44]

The entries aren't modified by clients.

+    navigation.referrer_url_ = GURL(sync_data.referrer());
+    navigation.referrer_policy_ = sync_data.correct_referrer_policy();
+  } else {
+    int mapped_referrer_policy;
+    if (SerializedNavigationDriver::Get()->MapReferrerPolicyToNewValues(
+            sync_data.referrer_policy(), &mapped_referrer_policy)) {
+      navigation.referrer_url_ = GURL(sync_data.referrer());
+    } else {
+      navigation.referrer_url_ = GURL();
+    }
+    navigation.referrer_policy_ = mapped_referrer_policy;
+  }
   navigation.virtual_url_ = GURL(sync_data.virtual_url());
   navigation.title_ = base::UTF8ToUTF16(sync_data.title());
   navigation.encoded_page_state_ = sync_data.state();
@@ -183,12 +194,13 @@ enum TypeMask {
 //
 // type_mask (has_post_data_)
 // referrer_url_
-// referrer_policy_
+// referrer_policy_ (broken, crbug.com/450589)
 // original_request_url_
 // is_overriding_user_agent_
 // timestamp_
 // search_terms_
 // http_status_code_
+// referrer_policy_
 
 void SerializedNavigationEntry::WriteToPickle(int max_size,
                                               Pickle* pickle) const {
@@ -210,11 +222,16 @@ void SerializedNavigationEntry::WriteToPickle(int max_size,
   const int type_mask = has_post_data_ ? HAS_POST_DATA : 0;
   pickle->WriteInt(type_mask);
 
-  WriteStringToPickle(
-      pickle, &bytes_written, max_size,
-      referrer_url_.is_valid() ? referrer_url_.spec() : std::string());
-
-  pickle->WriteInt(referrer_policy_);
+  int mapped_referrer_policy;
+  if (SerializedNavigationDriver::Get()->MapReferrerPolicyToOldValues(
+          referrer_policy_, &mapped_referrer_policy)) {

[Andrew T Wilson (Slow), 2015/01/29 11:02:13]

Fine as-is, but I wonder if the following is easier to read:

if (SerializedNavigationDriver::Get()->Map...() && referrer_url_.is_valid()) {
  WriteStringToPickle(pickle, &bytes_written, max_size, referrer_url_.spec());
} else {
  WriteStringToPickle(pickle, &bytes_written, max_size, std::string());
}

+    WriteStringToPickle(
+        pickle, &bytes_written, max_size,
+        referrer_url_.is_valid() ? referrer_url_.spec() : std::string());
+  } else {
+    WriteStringToPickle(pickle, &bytes_written, max_size, std::string());
+  }
+  pickle->WriteInt(mapped_referrer_policy);
 
   // Save info required to override the user agent.
   WriteStringToPickle(
@@ -227,6 +244,8 @@ void SerializedNavigationEntry::WriteToPickle(int max_size,
   WriteString16ToPickle(pickle, &bytes_written, max_size, search_terms_);
 
   pickle->WriteInt(http_status_code_);
+
+  pickle->WriteInt(referrer_policy_);
 }
 
 bool SerializedNavigationEntry::ReadFromPickle(PickleIterator* iterator) {
@@ -258,9 +277,20 @@ bool SerializedNavigationEntry::ReadFromPickle(PickleIterator* iterator) {
 
     // The "referrer policy" property was added even later, so we fall back to
     // the default policy if the property is not present.
-    if (!iterator->ReadInt(&referrer_policy_))
+    //
+    // Note: due to crbug.com/450589 this value might be incorrect, and a
+    // corrected version is stored later in the pickle.
+    if (!iterator->ReadInt(&referrer_policy_)) {
       referrer_policy_ =
           SerializedNavigationDriver::Get()->GetDefaultReferrerPolicy();
+    } else {
+      int mapped_referrer_policy;
+      if (!SerializedNavigationDriver::Get()->MapReferrerPolicyToNewValues(
+              referrer_policy_, &mapped_referrer_policy)) {
+        referrer_url_ = GURL();

[marja, 2015/01/29 10:54:20]

Why do we want to do this also in the case where we do have a correct referrer
policy too?

[jochen (gone - plz use gerrit), 2015/01/29 10:59:31]

good point. moved this code block down.

+      }
+      referrer_policy_ = mapped_referrer_policy;
+    }
 
     // If the original URL can't be found, leave it empty.
     std::string original_request_url_spec;
@@ -285,6 +315,11 @@ bool SerializedNavigationEntry::ReadFromPickle(PickleIterator* iterator) {
 
     if (!iterator->ReadInt(&http_status_code_))
       http_status_code_ = 0;
+
+    // Correct referrer policy (if present).
+    int correct_referrer_policy;
+    if (iterator->ReadInt(&correct_referrer_policy))
+      referrer_policy_ = correct_referrer_policy;
   }
 
   SerializedNavigationDriver::Get()->Sanitize(this);
@@ -299,8 +334,15 @@ bool SerializedNavigationEntry::ReadFromPickle(PickleIterator* iterator) {
 sync_pb::TabNavigation SerializedNavigationEntry::ToSyncData() const {
   sync_pb::TabNavigation sync_data;
   sync_data.set_virtual_url(virtual_url_.spec());
-  sync_data.set_referrer(referrer_url_.spec());
-  sync_data.set_referrer_policy(referrer_policy_);
+  int mapped_referrer_policy;
+  if (SerializedNavigationDriver::Get()->MapReferrerPolicyToOldValues(
+          referrer_policy_, &mapped_referrer_policy)) {
+    sync_data.set_referrer(referrer_url_.spec());
+  } else {
+    sync_data.set_referrer(std::string());
+  }
+  sync_data.set_referrer_policy(mapped_referrer_policy);
+  sync_data.set_correct_referrer_policy(referrer_policy_);
   sync_data.set_title(base::UTF16ToUTF8(title_));
 
   // Page transition core.