Refactors offline page tab helper to stash the offline page item on redirect.

chrome/browser/android/offline_pages/offline_page_tab_helper.cc

Index: chrome/browser/android/offline_pages/offline_page_tab_helper.cc
diff --git a/chrome/browser/android/offline_pages/offline_page_tab_helper.cc b/chrome/browser/android/offline_pages/offline_page_tab_helper.cc
index 9f1f868b35f9f2ba746c7b8026adb81ce07f3b86..1a8e389fe6ad3b965ee2094bdfc6a94e35686fcc 100644
--- a/chrome/browser/android/offline_pages/offline_page_tab_helper.cc
+++ b/chrome/browser/android/offline_pages/offline_page_tab_helper.cc
@@ -6,9 +6,12 @@
 
 #include "base/bind.h"
 #include "base/logging.h"
+#include "base/memory/ptr_util.h"
 #include "base/metrics/histogram.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "chrome/browser/android/offline_pages/offline_page_model_factory.h"
 #include "chrome/browser/android/offline_pages/offline_page_utils.h"
+#include "components/offline_pages/offline_page_model.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
@@ -52,6 +55,7 @@ void OfflinePageTabHelper::DidStartNavigation(
   // This is a new navigation so we can invalidate any previously scheduled
   // operations.
   weak_ptr_factory_.InvalidateWeakPtrs();
+  offline_page_.reset(nullptr);
 
   // Ignore navigations that are forward or back transitions in the nav stack
   // which are not at the head of the stack.
@@ -64,16 +68,24 @@ void OfflinePageTabHelper::DidStartNavigation(
   }
 
   content::BrowserContext* context = web_contents()->GetBrowserContext();
+  OfflinePageModel* offline_page_model =
+      OfflinePageModelFactory::GetForBrowserContext(context);
+  if (!offline_page_model)
+    return;
+
   GURL navigated_url = navigation_handle->GetURL();
-  auto redirect_url_callback =
-      base::Bind(&OfflinePageTabHelper::GotRedirectURLForStartedNavigation,
-                 weak_ptr_factory_.GetWeakPtr(), navigated_url);
+
   if (net::NetworkChangeNotifier::IsOffline()) {
-    OfflinePageUtils::GetOfflineURLForOnlineURL(context, navigated_url,
-                                                redirect_url_callback);
+    offline_page_model->GetBestPageForOnlineURL(
+        navigated_url,
+        base::Bind(&OfflinePageTabHelper::TryRedirectToOffline,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   RedirectReason::DISCONNECTED_NETWORK, navigated_url));
   } else {
-    OfflinePageUtils::GetOnlineURLForOfflineURL(context, navigated_url,
-                                                redirect_url_callback);
+    offline_page_model->GetPageByOfflineURL(
+        navigated_url,
+        base::Bind(&OfflinePageTabHelper::RedirectToOnline,
+                   weak_ptr_factory_.GetWeakPtr(), navigated_url));
   }
 }
 
@@ -110,41 +122,32 @@ void OfflinePageTabHelper::DidFinishNavigation(
     return;
   }
 
-  // Otherwise, get the offline URL for this url, and attempt a redirect if
-  // necessary.
-  OfflinePageUtils::GetOfflineURLForOnlineURL(
-      browser_context, navigated_url,
-      base::Bind(&OfflinePageTabHelper::GotRedirectURLForSupportedErrorCode,
-                 weak_ptr_factory_.GetWeakPtr(),
-                 navigation_handle->GetPageTransition(), navigated_url));
-}
-
-void OfflinePageTabHelper::GotRedirectURLForSupportedErrorCode(
-    ui::PageTransition transition,
-    const GURL& from_url,
-    const GURL& redirect_url) {
-  // If we didn't find an offline URL, or we are doing a forward/back
-  // transition, don't redirect.
-  bool do_redirect =
-      redirect_url.is_valid() && transition != ui::PAGE_TRANSITION_FORWARD_BACK;
-  UMA_HISTOGRAM_BOOLEAN("OfflinePages.ShowOfflinePageOnBadNetwork",
-                        do_redirect);
-  if (!do_redirect)
+  OfflinePageModel* offline_page_model =
+      OfflinePageModelFactory::GetForBrowserContext(browser_context);
+  if (!offline_page_model)
     return;
 
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::Bind(&OfflinePageTabHelper::Redirect,
-                 weak_ptr_factory_.GetWeakPtr(), from_url, redirect_url));
+  // Otherwise, get the offline URL for this url, and attempt a redirect if
+  // necessary.
+  RedirectReason reason =
+      navigation_handle->GetPageTransition() == ui::PAGE_TRANSITION_FORWARD_BACK
+          ? RedirectReason::FLAKY_NETWORK_FORWARD_BACK
+          : RedirectReason::FLAKY_NETWORK;
+  offline_page_model->GetBestPageForOnlineURL(
+      navigated_url,
+      base::Bind(&OfflinePageTabHelper::TryRedirectToOffline,
+                 weak_ptr_factory_.GetWeakPtr(), reason, navigated_url));
 }
 
-void OfflinePageTabHelper::GotRedirectURLForStartedNavigation(
-    const GURL& from_url,
-    const GURL& redirect_url) {
+void OfflinePageTabHelper::RedirectToOnline(
+    const GURL& navigated_url,
+    const OfflinePageItem* offline_page) {
   // Bails out if no redirection is needed.
-  if (!redirect_url.is_valid())
+  if (!offline_page)
     return;
 
+  GURL redirect_url = offline_page->url;
+
   const content::NavigationController& controller =
       web_contents()->GetController();
 
@@ -155,14 +158,46 @@ void OfflinePageTabHelper::GotRedirectURLForStartedNavigation(
     return;
   }
 
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::Bind(&OfflinePageTabHelper::Redirect,
-                 weak_ptr_factory_.GetWeakPtr(), from_url, redirect_url));
+  Redirect(navigated_url, redirect_url, nullptr);
 }
 
-void OfflinePageTabHelper::Redirect(
-    const GURL& from_url, const GURL& to_url) {
+void OfflinePageTabHelper::TryRedirectToOffline(
+    RedirectReason redirect_reason,
+    const GURL& from_url,
+    const OfflinePageItem* offline_page) {
+  if (!offline_page)
+    return;
+
+  GURL redirect_url = offline_page->GetOfflineURL();
+
+  if (!redirect_url.is_valid())
+    return;
+
+  if (redirect_reason == RedirectReason::FLAKY_NETWORK ||
+      redirect_reason == RedirectReason::FLAKY_NETWORK_FORWARD_BACK) {
+    UMA_HISTOGRAM_BOOLEAN("OfflinePages.ShowOfflinePageOnBadNetwork",
+                          redirect_reason == RedirectReason::FLAKY_NETWORK);
+    // Don't actually want to redirect on a forward/back nav.
+    if (redirect_reason == RedirectReason::FLAKY_NETWORK_FORWARD_BACK)
+      return;
+  } else {
+    const content::NavigationController& controller =
+        web_contents()->GetController();
+
+    // Avoids looping between online and offline redirections.
+    content::NavigationEntry* entry = controller.GetPendingEntry();
+    if (entry && !entry->GetRedirectChain().empty() &&
+        entry->GetRedirectChain().back() == redirect_url) {
+      return;
+    }
+  }
+
+  Redirect(from_url, redirect_url, offline_page);
+}
+
+void OfflinePageTabHelper::Redirect(const GURL& from_url,
+                                    const GURL& to_url,
+                                    const OfflinePageItem* offline_page) {
   if (to_url.SchemeIsFile()) {
     UMA_HISTOGRAM_COUNTS("OfflinePages.RedirectToOfflineCount", 1);
   } else {
@@ -173,6 +208,11 @@ void OfflinePageTabHelper::Redirect(
   load_params.transition_type = ui::PAGE_TRANSITION_CLIENT_REDIRECT;
   load_params.redirect_chain.push_back(from_url);
   web_contents()->GetController().LoadURLWithParams(load_params);
+
+  // We will always set the offline page if we redirect.  Note that this will
+  // clear the offline page if we redirect to online.
+  offline_page_ =
+      offline_page ? base::MakeUnique<OfflinePageItem>(*offline_page) : nullptr;

[jianli, 2016/06/15 00:44:00]

I think whether redirecting to online should be checked by
"!to_url.SchemeIsFile()". Passing nullptr to offline_page which indeed was not
null in the caller was very confusing.

[dewittj, 2016/06/15 17:21:05]

PTAL, I removed the SchemeIsFile call altogether.

 }
 
 }  // namespace offline_pages