Rewrites SerializedNavigationEntry to not have any //content member variables.

components/sessions/serialized_navigation_entry.cc

 // Copyright 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 "components/sessions/serialized_navigation_entry.h"
 
 #include "base/pickle.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/public/browser/favicon_status.h"
 #include "content/public/browser/navigation_controller.h"
@@ skipping 10 matching lines @@
 
 SerializedNavigationEntry::SerializedNavigationEntry()
     : index_(-1),
       unique_id_(0),
       transition_type_(ui::PAGE_TRANSITION_TYPED),
       has_post_data_(false),
       post_id_(-1),
       is_overriding_user_agent_(false),
       http_status_code_(0),
       is_restored_(false),
-      blocked_state_(STATE_INVALID) {}
+      blocked_state_(STATE_INVALID) {
+  referrer_policy_ = GetDefaultReferrerPolicy();
+}
 
 SerializedNavigationEntry::~SerializedNavigationEntry() {}
 
 // static
 SerializedNavigationEntry SerializedNavigationEntry::FromNavigationEntry(
     int index,
     const NavigationEntry& entry) {
   SerializedNavigationEntry navigation;
   navigation.index_ = index;
   navigation.unique_id_ = entry.GetUniqueID();
-  navigation.referrer_ = entry.GetReferrer();
+  navigation.referrer_url_ = entry.GetReferrer().url;
+  navigation.referrer_policy_ = entry.GetReferrer().policy;
   navigation.virtual_url_ = entry.GetVirtualURL();
   navigation.title_ = entry.GetTitle();
-  navigation.page_state_ = entry.GetPageState();
+  navigation.encoded_page_state_ = entry.GetPageState().ToEncodedData();
   navigation.transition_type_ = entry.GetTransitionType();
   navigation.has_post_data_ = entry.GetHasPostData();
   navigation.post_id_ = entry.GetPostID();
   navigation.original_request_url_ = entry.GetOriginalRequestURL();
   navigation.is_overriding_user_agent_ = entry.GetIsOverridingUserAgent();
   navigation.timestamp_ = entry.GetTimestamp();
   navigation.is_restored_ = entry.IsRestored();
   // If you want to navigate a named frame in Chrome, you will first need to
   // add support for persisting it. It is currently only used for layout tests.
   CHECK(entry.GetFrameToNavigate().empty());
   entry.GetExtraData(kSearchTermsKey, &navigation.search_terms_);
   if (entry.GetFavicon().valid)
     navigation.favicon_url_ = entry.GetFavicon().url;
   navigation.http_status_code_ = entry.GetHttpStatusCode();
   navigation.redirect_chain_ = entry.GetRedirectChain();
 
   return navigation;
 }
 
 SerializedNavigationEntry SerializedNavigationEntry::FromSyncData(
     int index,
     const sync_pb::TabNavigation& sync_data) {
   SerializedNavigationEntry navigation;
   navigation.index_ = index;
   navigation.unique_id_ = sync_data.unique_id();
-  navigation.referrer_ = content::Referrer(
-      GURL(sync_data.referrer()),
-      static_cast<blink::WebReferrerPolicy>(sync_data.referrer_policy()));
+  navigation.referrer_url_ = GURL(sync_data.referrer());
+  navigation.referrer_policy_ = sync_data.referrer_policy();
   navigation.virtual_url_ = GURL(sync_data.virtual_url());
   navigation.title_ = base::UTF8ToUTF16(sync_data.title());
-  navigation.page_state_ =
-      content::PageState::CreateFromEncodedData(sync_data.state());
+  navigation.encoded_page_state_ = sync_data.state();
 
   uint32 transition = 0;
   if (sync_data.has_page_transition()) {
     switch (sync_data.page_transition()) {
       case sync_pb::SyncEnums_PageTransition_LINK:
         transition = ui::PAGE_TRANSITION_LINK;
         break;
       case sync_pb::SyncEnums_PageTransition_TYPED:
         transition = ui::PAGE_TRANSITION_TYPED;
         break;
@@ skipping 115 matching lines @@
   HAS_POST_DATA = 1
 };
 
 }  // namespace
 
 // Pickle order:
 //
 // index_
 // virtual_url_
 // title_
-// page_state_
+// encoded_page_state_

[rohitrao (ping after 24h), 2014/10/17 15:15:54]

The PageState was always written to the pickle as an encoded string, so this
should not change the disk representation at all.

 // transition_type_
 //
 // Added on later:
 //
 // type_mask (has_post_data_)
-// referrer_
+// referrer_url_
+// referrer_policy_

[rohitrao (ping after 24h), 2014/10/17 15:15:53]

I've added referrer_policy_ here to be explicit, but we were always writing the
url and policy to the pickle separately, so this should not change the disk
representation at all.

 // original_request_url_
 // is_overriding_user_agent_
 // timestamp_
 // search_terms_
 // http_status_code_
 
 void SerializedNavigationEntry::WriteToPickle(int max_size,
                                               Pickle* pickle) const {
   pickle->WriteInt(index_);
 
   int bytes_written = 0;
 
   WriteStringToPickle(pickle, &bytes_written, max_size,
                       virtual_url_.spec());
 
   WriteString16ToPickle(pickle, &bytes_written, max_size, title_);
 
-  content::PageState page_state = page_state_;
-  if (has_post_data_)
-    page_state = page_state.RemovePasswordData();
-
-  WriteStringToPickle(pickle, &bytes_written, max_size,
-                      page_state.ToEncodedData());
+  const std::string encoded_page_state = GetSanitizedPageStateForPickle();
+  WriteStringToPickle(pickle, &bytes_written, max_size, encoded_page_state);
 
   pickle->WriteInt(transition_type_);
 
   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());
+      referrer_url_.is_valid() ? referrer_url_.spec() : std::string());
 
-  pickle->WriteInt(referrer_.policy);
+  pickle->WriteInt(referrer_policy_);
 
   // Save info required to override the user agent.
   WriteStringToPickle(
       pickle, &bytes_written, max_size,
       original_request_url_.is_valid() ?
       original_request_url_.spec() : std::string());
   pickle->WriteBool(is_overriding_user_agent_);
   pickle->WriteInt64(timestamp_.ToInternalValue());
 
   WriteString16ToPickle(pickle, &bytes_written, max_size, search_terms_);
 
   pickle->WriteInt(http_status_code_);
 }
 
 bool SerializedNavigationEntry::ReadFromPickle(PickleIterator* iterator) {
   *this = SerializedNavigationEntry();
-  std::string virtual_url_spec, page_state_data;
+  std::string virtual_url_spec;
   int transition_type_int = 0;
   if (!iterator->ReadInt(&index_) ||
       !iterator->ReadString(&virtual_url_spec) ||
       !iterator->ReadString16(&title_) ||
-      !iterator->ReadString(&page_state_data) ||
+      !iterator->ReadString(&encoded_page_state_) ||
       !iterator->ReadInt(&transition_type_int))
     return false;
   virtual_url_ = GURL(virtual_url_spec);
-  page_state_ = content::PageState::CreateFromEncodedData(page_state_data);
   transition_type_ = ui::PageTransitionFromInt(transition_type_int);
 
   // type_mask did not always exist in the written stream. As such, we
   // don't fail if it can't be read.
   int type_mask = 0;
   bool has_type_mask = iterator->ReadInt(&type_mask);
 
   if (has_type_mask) {
     has_post_data_ = type_mask & HAS_POST_DATA;
     // the "referrer" property was added after type_mask to the written
     // stream. As such, we don't fail if it can't be read.
     std::string referrer_spec;
     if (!iterator->ReadString(&referrer_spec))
       referrer_spec = std::string();
+    referrer_url_ = GURL(referrer_spec);
+
     // The "referrer policy" property was added even later, so we fall back to
     // the default policy if the property is not present.
-    int policy_int;
-    blink::WebReferrerPolicy policy;
-    if (iterator->ReadInt(&policy_int))
-      policy = static_cast<blink::WebReferrerPolicy>(policy_int);
-    else
-      policy = blink::WebReferrerPolicyDefault;
-    referrer_ = content::Referrer(GURL(referrer_spec), policy);
+    if (!iterator->ReadInt(&referrer_policy_))
+      referrer_policy_ = GetDefaultReferrerPolicy();
 
     // If the original URL can't be found, leave it empty.
     std::string original_request_url_spec;
     if (!iterator->ReadString(&original_request_url_spec))
       original_request_url_spec = std::string();
     original_request_url_ = GURL(original_request_url_spec);
 
     // Default to not overriding the user agent if we don't have info.
     if (!iterator->ReadBool(&is_overriding_user_agent_))
       is_overriding_user_agent_ = false;
@@ skipping 19 matching lines @@
 
   return true;
 }
 
 scoped_ptr<NavigationEntry> SerializedNavigationEntry::ToNavigationEntry(
     int page_id,
     content::BrowserContext* browser_context) const {
   scoped_ptr<NavigationEntry> entry(
       content::NavigationController::CreateNavigationEntry(
           virtual_url_,
-          referrer_,
+          content::Referrer(
+              referrer_url_,
+              static_cast<blink::WebReferrerPolicy>(referrer_policy_)),
           // Use a transition type of reload so that we don't incorrectly
           // increase the typed count.
           ui::PAGE_TRANSITION_RELOAD,
           false,
           // The extra headers are not sync'ed across sessions.
           std::string(),
           browser_context));
 
   entry->SetTitle(title_);
-  entry->SetPageState(page_state_);
+  entry->SetPageState(
+      content::PageState::CreateFromEncodedData(encoded_page_state_));
   entry->SetPageID(page_id);
   entry->SetHasPostData(has_post_data_);
   entry->SetPostID(post_id_);
   entry->SetOriginalRequestURL(original_request_url_);
   entry->SetIsOverridingUserAgent(is_overriding_user_agent_);
   entry->SetTimestamp(timestamp_);
   entry->SetExtraData(kSearchTermsKey, search_terms_);
   entry->SetHttpStatusCode(http_status_code_);
   entry->SetRedirectChain(redirect_chain_);
 
   // These fields should have default values.
   DCHECK_EQ(STATE_INVALID, blocked_state_);
   DCHECK_EQ(0u, content_pack_categories_.size());
 
   return entry.Pass();
 }
 
 // TODO(zea): perhaps sync state (scroll position, form entries, etc.) as well?
 // See http://crbug.com/67068.
 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);
+  sync_data.set_referrer(referrer_url_.spec());
+  sync_data.set_referrer_policy(referrer_policy_);
   sync_data.set_title(base::UTF16ToUTF8(title_));
 
   // Page transition core.
   COMPILE_ASSERT(ui::PAGE_TRANSITION_LAST_CORE ==
                  ui::PAGE_TRANSITION_KEYWORD_GENERATED,
                  PageTransitionCoreBounds);
   switch (ui::PageTransitionStripQualifier(transition_type_)) {
     case ui::PAGE_TRANSITION_LINK:
       sync_data.set_page_transition(
           sync_pb::SyncEnums_PageTransition_LINK);
@@ skipping 115 matching lines @@
   std::vector<NavigationEntry*> entries;
   for (std::vector<SerializedNavigationEntry>::const_iterator
        it = navigations.begin(); it != navigations.end(); ++it) {
     entries.push_back(
         it->ToNavigationEntry(page_id, browser_context).release());
     ++page_id;
   }
   return entries;
 }
 
+// TODO(rohitrao): Move this content-specific code into a
+// SerializedNavigationEntryHelper class.
+int SerializedNavigationEntry::GetDefaultReferrerPolicy() const {

[rohitrao (ping after 24h), 2014/10/17 15:15:53]

I've tried to move the remaining content-specific logic into these three
functions here.  In a future CL, I will move these functions into a different
class.

+  return blink::WebReferrerPolicyDefault;
+}
+
+std::string SerializedNavigationEntry::GetSanitizedPageStateForPickle() const {

[blundell, 2014/10/17 15:19:28]

nit: Add a TODO here like you have in the other two methods.

[rohitrao (ping after 24h), 2014/10/20 13:24:49]

Done.

+  content::PageState page_state =
+      content::PageState::CreateFromEncodedData(encoded_page_state_);

[rohitrao (ping after 24h), 2014/10/17 15:15:53]

There will be some places where we now do an additional copy of the encoded
pagestate string.  Here, for example.  This is slightly balanced by some places
where the new code avoids a copy that was done previously.

+  if (has_post_data_)
+    page_state = page_state.RemovePasswordData();
+
+  return page_state.ToEncodedData();
+}
+
+// TODO(rohitrao): Move this content-specific code into a
+// SerializedNavigationEntryHelper class.
 void SerializedNavigationEntry::Sanitize() {
+  content::Referrer old_referrer(
+      referrer_url_,
+      static_cast<blink::WebReferrerPolicy>(referrer_policy_));
   content::Referrer new_referrer =
-      content::Referrer::SanitizeForRequest(virtual_url_, referrer_);
+      content::Referrer::SanitizeForRequest(virtual_url_, old_referrer);
 
   // No need to compare the policy, as it doesn't change during
   // sanitization. If there has been a change, the referrer needs to be
   // stripped from the page state as well.
-  if (referrer_.url != new_referrer.url) {
-    referrer_ = content::Referrer();
-    page_state_ = page_state_.RemoveReferrer();
+  if (referrer_url_ != new_referrer.url) {
+    referrer_url_ = GURL();
+    referrer_policy_ = GetDefaultReferrerPolicy();
+    encoded_page_state_ =
+        content::PageState::CreateFromEncodedData(encoded_page_state_)
+            .RemoveReferrer()
+            .ToEncodedData();
   }
 }
 
 }  // namespace sessions