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

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/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"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "net/base/net_errors.h"
 #include "net/base/network_change_notifier.h"
 #include "ui/base/page_transition_types.h"
 
@@ skipping 23 matching lines @@
 
 void OfflinePageTabHelper::DidStartNavigation(
     content::NavigationHandle* navigation_handle) {
   // Skips non-main frame.
   if (!navigation_handle->IsInMainFrame())
     return;
 
   // 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.
   const content::NavigationController& controller =
       web_contents()->GetController();
   if (controller.GetEntryCount() > 0 &&
       controller.GetCurrentEntryIndex() != -1 &&
       controller.GetCurrentEntryIndex() < controller.GetEntryCount() - 1) {
     return;
   }
 
   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));
   }
 }
 
 void OfflinePageTabHelper::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
   // Skips non-main frame.
   if (!navigation_handle->IsInMainFrame())
     return;
 
   GURL navigated_url = navigation_handle->GetURL();
@@ skipping 16 matching lines @@
   // navigation will eventually fail and we want to redirect to offline copy
   // in this case. If error code doesn't match this list, then we still show
   // the error page and not an offline page, so do nothing.
   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;
   }
 
+  OfflinePageModel* offline_page_model =
+      OfflinePageModelFactory::GetForBrowserContext(browser_context);
+  if (!offline_page_model)
+    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));
+  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::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)
+void OfflinePageTabHelper::RedirectToOnline(
+    const GURL& navigated_url,
+    const OfflinePageItem* offline_page) {
+  // Bails out if no redirection is needed.
+  if (!offline_page)
     return;
 
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::Bind(&OfflinePageTabHelper::Redirect,
-                 weak_ptr_factory_.GetWeakPtr(), from_url, redirect_url));
-}
-
-void OfflinePageTabHelper::GotRedirectURLForStartedNavigation(
-    const GURL& from_url,
-    const GURL& redirect_url) {
-  // Bails out if no redirection is needed.
-  if (!redirect_url.is_valid())
-    return;
+  GURL redirect_url = offline_page->url;
 
   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;
   }
 
-  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 {
     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);
+
+  // 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