Serve offline page for online URL on disconnected or bad networks

chrome/browser/android/offline_pages/offline_page_request_handler.cc

Index: chrome/browser/android/offline_pages/offline_page_request_handler.cc
diff --git a/chrome/browser/android/offline_pages/offline_page_request_handler.cc b/chrome/browser/android/offline_pages/offline_page_request_handler.cc
index 0e9056c5c7865f3bcc91d783838862c1968bf5ed..0227ab83839776987dc95062ceb1ee1e00d07d21 100644
--- a/chrome/browser/android/offline_pages/offline_page_request_handler.cc
+++ b/chrome/browser/android/offline_pages/offline_page_request_handler.cc
@@ -4,15 +4,46 @@
 
 #include "chrome/browser/android/offline_pages/offline_page_request_handler.h"
 
+#include "base/metrics/histogram_macros.h"
+#include "base/strings/string_tokenizer.h"
+#include "base/strings/string_util.h"
+#include "chrome/browser/android/offline_pages/offline_page_model_factory.h"
+#include "chrome/browser/android/offline_pages/offline_page_tab_helper.h"
+#include "chrome/browser/android/offline_pages/offline_page_utils.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
 #include "components/offline_pages/offline_page_feature.h"
+#include "components/offline_pages/offline_page_model.h"
+#include "components/previews/previews_experiments.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/web_contents.h"
+#include "net/base/filename_util.h"
+#include "net/base/network_change_notifier.h"
+#include "net/nqe/network_quality_estimator.h"
 #include "net/url_request/url_request.h"
+#include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_interceptor.h"
+#include "net/url_request/url_request_job.h"
 
 namespace offline_pages {
 
+const char kLoadingOfflinePageHeader[] = "X-chromium-offline";
+const char kLoadingOfflinePageReason[] = "reason=";
+const char kLoadingOfflinePageDueToNetError[] = "error";
+
 namespace {
 
+// This enum is used to tell all possible outcomes of handling network requests
+// that might serve offline contents.
+enum class RequestResult {
+  OFFLINE_PAGE_SERVED,
+  NO_TAB_ID,
+  NO_WEB_CONTENTS,
+  PAGE_NOT_FRESH,
+  OFFLINE_PAGE_NOT_FOUND
+};
+
 int kUserDataKey;  // Only address is used.
 
 // An interceptor to hijack requests and potentially service them based on
@@ -43,6 +74,186 @@ class OfflinePageRequestInterceptor : public net::URLRequestInterceptor {
   DISALLOW_COPY_AND_ASSIGN(OfflinePageRequestInterceptor);
 };
 
+NetworkState GetNetworkState(net::URLRequest* request) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  // If extra header kLoadingOfflinePageHeader is present, offline page should
+  // be loaded if available.
+  std::string value;
+  if (request->extra_request_headers().GetHeader(kLoadingOfflinePageHeader,
+                                                 &value)) {
+    std::string reason;
+    base::StringTokenizer tokenizer(value, ", ");
+    while (tokenizer.GetNext()) {
+      if (base::StartsWith(tokenizer.token(),
+                           kLoadingOfflinePageReason,
+                           base::CompareCase::INSENSITIVE_ASCII)) {
+        reason = tokenizer.token().substr(
+            arraysize(kLoadingOfflinePageReason) - 1);
+        break;
+      }
+    }
+    return base::EqualsCaseInsensitiveASCII(
+                reason, kLoadingOfflinePageDueToNetError) ?
+        NetworkState::FLAKY_NETWORK : NetworkState::SKIPPED_NETWORK_CHECK;

[fgorski, 2016/08/15 21:38:32]

would it make sense to continue network checks if != error instead of skipping?

[jianli, 2016/08/15 23:15:43]

Done.

+  }
+
+  if (net::NetworkChangeNotifier::IsOffline())
+    return NetworkState::DISCONNECTED_NETWORK;
+
+  // NetworkQualityEstimator only works when it is enabled.
+  if (!previews::IsOfflinePreviewsEnabled())
+    return NetworkState::CONNECTED_NETWORK;
+
+  net::NetworkQualityEstimator* network_quality_estimator =
+      request->context() ? request->context()->network_quality_estimator()
+                         : nullptr;
+  if (!network_quality_estimator)
+    return NetworkState::CONNECTED_NETWORK;
+
+  net::EffectiveConnectionType effective_connection_type =
+      network_quality_estimator->GetEffectiveConnectionType();
+  return effective_connection_type >= net::EFFECTIVE_CONNECTION_TYPE_OFFLINE &&

[fgorski, 2016/08/15 21:38:32]

Is the effective connection type enum guaranteed to stay in order of network
quality?

[jianli, 2016/08/15 23:15:43]

I think so.

+         effective_connection_type <= net::EFFECTIVE_CONNECTION_TYPE_SLOW_2G ?
+             NetworkState::PROHIBITIVELY_SLOW_NETWORK :
+             NetworkState::CONNECTED_NETWORK;
+}
+
+void ReportRequestResult(RequestResult request_result,

[fgorski, 2016/08/15 21:38:32]

Consider splitting it in 2 methods, please. (But up to you)

One returns AggregatedRequestResult from RequestResult and Network state.

the other calls it and reports
OfflinePageRequestHandler::ReportAggregatedRequestResult(result);

I think it will be cleaner and you will simply have return statements.

[jianli, 2016/08/15 23:15:43]

Done.

+                         NetworkState network_state) {
+  AggregatedRequestResult result;
+  if (request_result == RequestResult::NO_TAB_ID) {
+    result = AggregatedRequestResult::NO_TAB_ID;
+  } else if (request_result == RequestResult::NO_WEB_CONTENTS) {
+    result = AggregatedRequestResult::NO_WEB_CONTENTS;
+  } else if (request_result == RequestResult::PAGE_NOT_FRESH) {
+    DCHECK_EQ(NetworkState::PROHIBITIVELY_SLOW_NETWORK, network_state);
+    result =
+        AggregatedRequestResult::PAGE_NOT_FRESH_ON_PROHIBITIVELY_SLOW_NETWORK;
+  } else if (request_result == RequestResult::OFFLINE_PAGE_NOT_FOUND) {
+    switch (network_state) {
+      case NetworkState::DISCONNECTED_NETWORK:
+        result =
+            AggregatedRequestResult::PAGE_NOT_FOUND_ON_DISCONNECTED_NETWORK;
+        break;
+      case NetworkState::PROHIBITIVELY_SLOW_NETWORK:
+        result =
+            AggregatedRequestResult::
+                PAGE_NOT_FOUND_ON_PROHIBITIVELY_SLOW_NETWORK;
+        break;
+      case NetworkState::FLAKY_NETWORK:
+        result =
+            AggregatedRequestResult::PAGE_NOT_FOUND_ON_DISCONNECTED_NETWORK;

[fgorski, 2016/08/15 21:38:32]

ON_FLAKY_NETWORK

[jianli, 2016/08/15 23:15:43]

Done.

+        break;
+      case NetworkState::SKIPPED_NETWORK_CHECK:
+        result = net::NetworkChangeNotifier::IsOffline() ?
+            AggregatedRequestResult::PAGE_NOT_FOUND_ON_DISCONNECTED_NETWORK :
+            AggregatedRequestResult::PAGE_NOT_FOUND_ON_CONNECTED_NETWORK;
+      default:
+        NOTREACHED();
+    }
+  } else {
+    DCHECK_EQ(RequestResult::OFFLINE_PAGE_SERVED, request_result);
+    switch (network_state) {
+      case NetworkState::DISCONNECTED_NETWORK:
+        result = AggregatedRequestResult::SHOW_OFFLINE_ON_DISCONNECTED_NETWORK;
+        break;
+      case NetworkState::PROHIBITIVELY_SLOW_NETWORK:
+        result =
+            AggregatedRequestResult::SHOW_OFFLINE_ON_PROHIBITIVELY_SLOW_NETWORK;
+        break;
+      case NetworkState::FLAKY_NETWORK:
+        result = AggregatedRequestResult::SHOW_OFFLINE_ON_FLAKY_NETWORK;
+        break;
+      case NetworkState::CONNECTED_NETWORK:
+        result = AggregatedRequestResult::SHOW_OFFLINE_ON_CONNECTED_NETWORK;
+        break;
+      case NetworkState::SKIPPED_NETWORK_CHECK:
+        result = net::NetworkChangeNotifier::IsOffline() ?
+            AggregatedRequestResult::SHOW_OFFLINE_ON_DISCONNECTED_NETWORK :
+            AggregatedRequestResult::SHOW_OFFLINE_ON_CONNECTED_NETWORK;
+      default:
+        NOTREACHED();
+     }
+  }
+
+  OfflinePageRequestHandler::ReportAggregatedRequestResult(result);
+}
+
+// Called on IO thread after an offline file path is obtained.

[fgorski, 2016/08/15 21:38:32]

nit: tell the reader what it does, reather than when it is called (name sort of
indicates that already), please.

[jianli, 2016/08/15 23:15:43]

Done.

+void DidGetOfflineFilePathOnIO(base::WeakPtr<OfflinePageRequestJob> job,
+                               const base::FilePath& offline_file_path) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  if (!job)
+    return;
+
+  if (offline_file_path.empty())
+    job->FallbackToDefault();
+  else
+    job->SetOfflineFilePath(offline_file_path);
+}
+
+// Called on UI thread after an offline file path is obtained.
+void DidGetOfflineFilePathOnUI(base::WeakPtr<OfflinePageRequestJob> job,
+                               const base::FilePath& offline_file_path) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&DidGetOfflineFilePathOnIO, job, offline_file_path));
+}
+
+// Called on UI thread after an offline page is found.

[fgorski, 2016/08/15 21:38:32]

ditto

[jianli, 2016/08/15 23:15:43]

Done.

+void SelectPageForOnlineURLDone(
+    NetworkState network_state,
+    base::WeakPtr<OfflinePageRequestJob> job,
+    content::ResourceRequestInfo::WebContentsGetter web_contents_getter,
+    const OfflinePageItem* offline_page){
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  RequestResult request_result;
+  base::FilePath offline_file_path;
+
+  // If offline page is not found, fall through with empty file path such that
+  // OfflinePageRequestJob will be notified on failure.
+  if (offline_page) {
+    // If the page is being loaded on a slow network, only use the offline page
+    // if it was created within the past day.
+    if (network_state == NetworkState::PROHIBITIVELY_SLOW_NETWORK &&
+        base::Time::Now() - offline_page->creation_time >

[fgorski, 2016/08/15 21:38:32]

Add TODO: this should be policy driven I think:
1. 1 day is very arbitrary
2. Some pages survive longer. E.g. Bookmark pages will be kept for 7 days. No
point doing that if we never show pages that are available that long.
3. Even when this is specific to previews, they probably want to control that
time better than just in code. Probably worth raising this with Ben

[jianli, 2016/08/15 23:15:43]

This is ported from existing logic in OfflinePageTabHelper. Added a TODO to make
this policy driven.

+            base::TimeDelta::FromDays(1)) {
+      request_result = RequestResult::PAGE_NOT_FRESH;
+    } else {
+      net::FileURLToFilePath(offline_page->GetOfflineURL(), &offline_file_path);

[fgorski, 2016/08/15 21:38:32]

move to if web_contents

[jianli, 2016/08/15 23:15:43]

Done.

+
+      content::WebContents* web_contents = web_contents_getter.Run();
+      if (web_contents) {
+        request_result = RequestResult::OFFLINE_PAGE_SERVED;
+
+        OfflinePageModel* offline_page_model =
+            OfflinePageModelFactory::GetForBrowserContext(
+                web_contents->GetBrowserContext());
+        DCHECK(offline_page_model);
+        offline_page_model->MarkPageAccessed(offline_page->offline_id);
+
+        OfflinePageTabHelper* tab_helper =
+            OfflinePageTabHelper::FromWebContents(web_contents);
+        DCHECK(tab_helper);
+        tab_helper->SetOfflinePage(*offline_page);
+      } else {
+        request_result = RequestResult::NO_WEB_CONTENTS;
+      }
+    }
+  } else {
+    request_result = RequestResult::OFFLINE_PAGE_NOT_FOUND;
+  }
+
+  ReportRequestResult(request_result, network_state);
+  DidGetOfflineFilePathOnUI(job, offline_file_path);
+}
+
 }  // namespace
 
 // static
@@ -64,9 +275,15 @@ void OfflinePageRequestHandler::InitializeHandler(
   if (request->method() != "GET")
     return;
 
+  // Bail out if no need to load offline page.
+  NetworkState network_state = GetNetworkState(request);
+  if (network_state == NetworkState::CONNECTED_NETWORK)
+    return;
+
   // Any more precise checks to see if the request should be intercepted are
   // asynchronous, so just create our handler in all cases.
-  request->SetUserData(&kUserDataKey, new OfflinePageRequestHandler());
+  request->SetUserData(&kUserDataKey,
+                       new OfflinePageRequestHandler(network_state));
 }
 
 // static
@@ -86,20 +303,94 @@ OfflinePageRequestHandler::CreateInterceptor(void* profile_id) {
       new OfflinePageRequestInterceptor(profile_id));
 }
 
-OfflinePageRequestHandler::OfflinePageRequestHandler() {
+// static
+void OfflinePageRequestHandler::ReportAggregatedRequestResult(
+    AggregatedRequestResult result) {
+  UMA_HISTOGRAM_ENUMERATION("OfflinePages.AggregatedRequestResult",
+      static_cast<int>(result),
+      static_cast<int>(AggregatedRequestResult::AGGREGATED_REQUEST_RESULT_MAX));
+}
+
+OfflinePageRequestHandler::OfflinePageRequestHandler(NetworkState network_state)
+    : network_state_(network_state),
+      weak_ptr_factory_(this) {
+  DCHECK_NE(NetworkState::CONNECTED_NETWORK, network_state);
 }
 
 OfflinePageRequestHandler::~OfflinePageRequestHandler() {
 }
 
+void OfflinePageRequestHandler::OnPrepareToRestart() {
+  use_default_ = true;
+  offline_job_.reset();
+}
+
 net::URLRequestJob* OfflinePageRequestHandler::MaybeCreateJob(
     net::URLRequest* request,
     net::NetworkDelegate* network_delegate,
     void* profile_id) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
-  // TODO(jianli): to be implemented.
-  return nullptr;
+  // Fall back to default which is set when offline page is not found.
+  if (use_default_)
+    return nullptr;

[fgorski, 2016/08/15 21:38:32]

is default selected if this returns nullptr?

[jianli, 2016/08/15 23:15:43]

Yes.

+
+  OfflinePageRequestJob* offline_job =
+      new OfflinePageRequestJob(request, network_delegate, this);
+  offline_job_ = offline_job->GetWeakPtr();
+
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI,
+      FROM_HERE,
+      base::Bind(&OfflinePageRequestHandler::GetOfflineFileURL,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 request->url(),
+                 profile_id,
+                 content::ResourceRequestInfo::ForRequest(request)->
+                     GetWebContentsGetterForRequest()));
+
+  return offline_job_.get();
+}
+
+void OfflinePageRequestHandler::GetOfflineFileURL(
+    const GURL& online_url,
+    void* profile_id,
+    content::ResourceRequestInfo::WebContentsGetter web_contents_getter){
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  // |profile_id| needs to be checked with ProfileManager::IsValidProfile
+  // before using it.
+  if (!g_browser_process->profile_manager()->IsValidProfile(profile_id)) {
+    FailedToGetOfflineFileURL();
+    return;
+  }
+  Profile* profile = reinterpret_cast<Profile*>(profile_id);
+
+  content::WebContents* web_contents = web_contents_getter.Run();
+  int tab_id;
+  if (!OfflinePageUtils::GetTabId(web_contents, &tab_id)) {
+    ReportRequestResult(RequestResult::NO_TAB_ID, network_state_);
+    FailedToGetOfflineFileURL();
+    return;
+  }
+
+  OfflinePageUtils::SelectPageForOnlineURL(
+      profile,
+      online_url,
+      tab_id,
+      base::Bind(&SelectPageForOnlineURLDone,
+                 network_state_,
+                 offline_job_,
+                 web_contents_getter));
+}
+
+void OfflinePageRequestHandler::FailedToGetOfflineFileURL() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  // Proceed with empty file path in order to notify the OfflinePageRequestJob
+  // about the failure.
+  base::FilePath empty_file_path;
+  DidGetOfflineFilePathOnUI(offline_job_, empty_file_path);
 }
 
 }  // namespace offline_pages