Persist offline page info in a navigation entry if needed

components/sessions/content/content_serialized_navigation_builder.cc

Index: components/sessions/content/content_serialized_navigation_builder.cc
diff --git a/components/sessions/content/content_serialized_navigation_builder.cc b/components/sessions/content/content_serialized_navigation_builder.cc
index 59df5cc2d016730fa5a64ff1c9875b12ad3cd696..77e38dfff9ba2faa489b834fc67ad666ba97d120 100644
--- a/components/sessions/content/content_serialized_navigation_builder.cc
+++ b/components/sessions/content/content_serialized_navigation_builder.cc
@@ -4,6 +4,9 @@
 
 #include "components/sessions/content/content_serialized_navigation_builder.h"
 
+#include "base/macros.h"
+#include "base/strings/string_tokenizer.h"
+#include "base/strings/string_util.h"
 #include "components/sessions/content/content_record_password_state.h"
 #include "components/sessions/core/serialized_navigation_entry.h"
 #include "content/public/browser/browser_context.h"
@@ -15,6 +18,34 @@
 
 namespace sessions {
 
+namespace {
+
+const char kOfflinePageHeader[] = "X-Chrome-offline: ";

[sky, 2016/09/12 15:04:21]

Is X-Chrome-offline and persist=1 defined somewhere that this code should be
using?

[jianli, 2016/09/12 21:17:46]

"X-Chrome-offline" is defined in offline_page_request_job.cc and "persist=1" is
first introduced in this patch. I have another patch which adds one more field
to "X-Chrome-offline" header and that patch contains all the definitions plus
header parsing. The reason for using "X-Chrome-offline" and "persist=1" defined
in this patch is to allow easier merge to M54 branch. Per TODO at line 27, I am
going to move all the header definition and parsing into a common directory
components/offline_pages/request after all these 2 patches get landed. The
reason for not doing the move right now is that I need to reorganize those files
under components/offline_pages.

+
+// Returns the offline page info that needs to be persisted, based on extra
+// headers.
+// TODO(jianli): Move this logic to components/offline_pages/request.
+std::string GetOfflinePageInfo(const content::NavigationEntry& entry) {
+  std::string extra_headers = entry.GetExtraHeaders();
+  if (extra_headers.empty())
+    return std::string();
+  // The offline header will be the only extra header if it is present.

[sky, 2016/09/12 15:04:21]

How do you know that this is true?

[jianli, 2016/09/12 21:17:46]

Most of the time extra request header is set in some custom fetcher requests,
that do not correspond to a navigation. For offline pages, it is set when we try
to open a downloaded page in which case no other extra header will be set.

[sky, 2016/09/13 01:53:31]

What's to stop a page from using these headers?

[jianli, 2016/09/13 20:29:18]

We want to keep these headers to allow loading the same offline page in order to
provide consistent experience to the user if these headers are set. Note that
these headers are only set in a few special scenarios. For example, when the
user opens a downloaded page from the Download home.

Normally these headers are not set when the user navigates to a page.

[sky, 2016/09/13 23:30:42]

Perhaps I wasn't clear. Let me rephrase my question. If I'm a website author and
I serve up a page with X-Chrome-offline: persist=1 then won't we automatically
persist the page? Shouldn't we only persist pages that Chrome chooses to
persist?

[jianli, 2016/09/13 23:44:16]

No, it is not possible for a web to do something like this since it only comes
from extra request header which could only be set in the client code.

+  if (!base::StartsWith(extra_headers,
+                        kOfflinePageHeader,
+                        base::CompareCase::INSENSITIVE_ASCII)) {
+    return std::string();
+  }
+  std::string value = extra_headers.substr(arraysize(kOfflinePageHeader) - 1);
+  base::StringTokenizer tokenizer(value, ", ");
+  while (tokenizer.GetNext()) {
+    if (tokenizer.token() == "persist=1")
+      return value;
+  }
+  return std::string();
+}
+
+}  // namespace
+
 // static
 SerializedNavigationEntry
 ContentSerializedNavigationBuilder::FromNavigationEntry(
@@ -41,6 +72,7 @@ ContentSerializedNavigationBuilder::FromNavigationEntry(
   navigation.http_status_code_ = entry.GetHttpStatusCode();
   navigation.redirect_chain_ = entry.GetRedirectChain();
   navigation.password_state_ = GetPasswordStateFromNavigation(entry);
+  navigation.offline_page_info_ = GetOfflinePageInfo(entry);
 
   return navigation;
 }
@@ -53,6 +85,11 @@ ContentSerializedNavigationBuilder::ToNavigationEntry(
     content::BrowserContext* browser_context) {
   blink::WebReferrerPolicy policy =
       static_cast<blink::WebReferrerPolicy>(navigation->referrer_policy_);
+  // The extra headers, except the one used for offline page support, are not
+  // sync'ed across sessions.
+  std::string extra_headers;
+  if (!navigation->offline_page_info_.empty())
+    extra_headers = kOfflinePageHeader + navigation->offline_page_info_;
   std::unique_ptr<content::NavigationEntry> entry(
       content::NavigationController::CreateNavigationEntry(
           navigation->virtual_url_,
@@ -62,8 +99,7 @@ ContentSerializedNavigationBuilder::ToNavigationEntry(
           // 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));
+          extra_headers, browser_context));
 
   entry->SetTitle(navigation->title_);
   entry->SetPageState(content::PageState::CreateFromEncodedData(