Redirect immediately to offline copy on no network

chrome/browser/android/offline_pages/offline_page_tab_helper.cc

 // Copyright 2016 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 "chrome/browser/android/offline_pages/offline_page_tab_helper.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/thread_task_runner_handle.h"
@@ skipping 19 matching lines @@
 }
 
 OfflinePageTabHelper::~OfflinePageTabHelper() {}
 
 void OfflinePageTabHelper::DidStartNavigation(
     content::NavigationHandle* navigation_handle) {
   // Skips non-main frame.
   if (!navigation_handle->IsInMainFrame())
     return;
 
-  // Redirecting to online version will only take effect when there is network
-  // connection.
-  if (net::NetworkChangeNotifier::IsOffline())
-    return;
-
   // Ignore navigations that are forward or back transitions in the nav stack
   // which are not at the head of the stack.
   const content::NavigationController& controller =
       web_contents()->GetController();
   if (controller.GetEntryCount() > 0 &&
       controller.GetCurrentEntryIndex() != -1 &&
       controller.GetCurrentEntryIndex() < controller.GetEntryCount() - 1) {
     return;
   }
 
-  // Skips if not loading an offline copy of saved page.
-  GURL online_url = offline_pages::OfflinePageUtils::GetOnlineURLForOfflineURL(
+  GURL redirect_url;
+  if (net::NetworkChangeNotifier::IsOffline()) {
+    // When the network is disconnected, loading online page will result in
+    // immediate redirection to offline copy.
+    redirect_url = offline_pages::OfflinePageUtils::GetOfflineURLForOnlineURL(
       web_contents()->GetBrowserContext(), navigation_handle->GetURL());
-  if (!online_url.is_valid())
+  } else {
+    // When the network is connected, loading offline copy will result in
+    // immediate redirection to online page.
+    redirect_url = offline_pages::OfflinePageUtils::GetOnlineURLForOfflineURL(
+      web_contents()->GetBrowserContext(), navigation_handle->GetURL());
+  }
+
+  // Bails out if no redirection is needed.
+  if (!redirect_url.is_valid())
     return;
 
   // Avoids looping between online and offline redirections.
   content::NavigationEntry* entry = controller.GetPendingEntry();
   if (entry && !entry->GetRedirectChain().empty() &&
-      entry->GetRedirectChain().back() == online_url) {
+      entry->GetRedirectChain().back() == redirect_url) {
     return;
   }
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
       base::Bind(&OfflinePageTabHelper::Redirect,
                  weak_ptr_factory_.GetWeakPtr(),
-                 navigation_handle->GetURL(), online_url));
+                 navigation_handle->GetURL(), redirect_url));
 }
 
 void OfflinePageTabHelper::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
   // Skips non-main frame.
   if (!navigation_handle->IsInMainFrame())
     return;
 
   // Skips load failure other than no network.
   net::Error error_code = navigation_handle->GetNetErrorCode();
   if (error_code != net::ERR_INTERNET_DISCONNECTED &&
       error_code != net::ERR_NAME_NOT_RESOLVED &&
       error_code != net::ERR_ADDRESS_UNREACHABLE &&
       error_code != net::ERR_PROXY_CONNECTION_FAILED) {
     return;
   }
 
   // On a forward or back transition, don't affect the order of the nav stack.
   if (navigation_handle->GetPageTransition() ==
       ui::PAGE_TRANSITION_FORWARD_BACK) {
     return;
   }
 
   // Skips if not loading an online version of saved page.
   GURL offline_url = offline_pages::OfflinePageUtils::GetOfflineURLForOnlineURL(

[fgorski, 2016/05/04 21:45:12]

no change here?

[jianli, 2016/05/05 00:23:13]

The plan is that we still access the sync API for this on M52.

[fgorski, 2016/05/05 04:19:30]

OK. What I think I didn't get was, the fact that in DidStartNavigation we
redirect because of connection state, while in DidFinishNavigation, it is
because we hit the error page.

Perhaps we could have a high level comment somewhere for that?

       web_contents()->GetBrowserContext(), navigation_handle->GetURL());
   if (!offline_url.is_valid())
     return;
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
       base::Bind(&OfflinePageTabHelper::Redirect,
                  weak_ptr_factory_.GetWeakPtr(),
                  navigation_handle->GetURL(), offline_url));
 }
 
 void OfflinePageTabHelper::Redirect(
     const GURL& from_url, const GURL& to_url) {
-  if (to_url.SchemeIsFile())
+  if (to_url.SchemeIsFile()) {
     UMA_HISTOGRAM_COUNTS("OfflinePages.RedirectToOfflineCount", 1);
-  else
+    OfflinePageUtils::MarkPageAccessed(

[fgorski, 2016/05/04 21:45:12]

should we mark page access when it is loaded without being redirected?

[jianli, 2016/05/05 00:23:13]

Done.

+        web_contents()->GetBrowserContext(), to_url);
+  } else {
     UMA_HISTOGRAM_COUNTS("OfflinePages.RedirectToOnlineCount", 1);
+  }
 
   content::NavigationController::LoadURLParams load_params(to_url);
   load_params.transition_type = ui::PAGE_TRANSITION_CLIENT_REDIRECT;
   load_params.redirect_chain.push_back(from_url);
   web_contents()->GetController().LoadURLWithParams(load_params);
 }
 
 }  // namespace offline_pages