[Offlining] Updates the tab helper to use the async OfflinePageModel API

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 94dcb0af30e83525939c8dd7a85fa6ca3ef32e19..8d4e1a8b6732f32eb06bc4dfa8c930c75de37705 100644
--- a/chrome/browser/android/offline_pages/offline_page_tab_helper.cc
+++ b/chrome/browser/android/offline_pages/offline_page_tab_helper.cc
@@ -37,6 +37,10 @@ void OfflinePageTabHelper::DidStartNavigation(
   if (!navigation_handle->IsInMainFrame())
     return;
 
+  // This is a new navigation so we can invalidate any previously scheduled
+  // operations.
+  weak_ptr_factory_.InvalidateWeakPtrs();
+
   // 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 =
@@ -47,35 +51,28 @@ void OfflinePageTabHelper::DidStartNavigation(
     return;
   }
 
-  GURL redirect_url;
+  content::BrowserContext* context = web_contents()->GetBrowserContext();
+  GURL navigated_url = navigation_handle->GetURL();
+  auto redirect_url_callback =
+      base::Bind(&OfflinePageTabHelper::GotRedirectURLForStartedNavigation,
+                 weak_ptr_factory_.GetWeakPtr(), navigated_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());
+    OfflinePageUtils::GetOfflineURLForOnlineURL(context, navigated_url,
+                                                redirect_url_callback);
   } 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());
+    OfflinePageUtils::GetOnlineURLForOfflineURL(context, navigated_url,
+                                                redirect_url_callback);
   }
+}
 
-  // 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() == redirect_url) {
-    return;
+void ReportAccessedOfflinePage(content::BrowserContext* browser_context,

[jianli, 2016/06/04 00:27:20]

Please move it to anonymous namespace.

[dewittj, 2016/06/06 16:53:15]

Done.

+                               const GURL& navigated_url,
+                               const GURL& online_url) {
+  // If there is a valid online URL for this navigated URL, then we are looking
+  // at an offline page.
+  if (online_url.is_valid()) {

[jianli, 2016/06/04 00:27:19]

nit: remove {}

[dewittj, 2016/06/06 16:53:15]

Done.

+    OfflinePageUtils::MarkPageAccessed(browser_context, navigated_url);
   }
-
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::Bind(&OfflinePageTabHelper::Redirect,
-                 weak_ptr_factory_.GetWeakPtr(),
-                 navigation_handle->GetURL(), redirect_url));
 }
 
 void OfflinePageTabHelper::DidFinishNavigation(
@@ -84,17 +81,26 @@ void OfflinePageTabHelper::DidFinishNavigation(
   if (!navigation_handle->IsInMainFrame())
     return;
 
-  // If the offline page is being loaded successfully, set the access record.
+  GURL navigated_url = navigation_handle->GetURL();
   net::Error error_code = navigation_handle->GetNetErrorCode();
-  if (error_code == net::OK &&
-      OfflinePageUtils::IsOfflinePage(
-          web_contents()->GetBrowserContext(), navigation_handle->GetURL())) {
-    UMA_HISTOGRAM_BOOLEAN("OfflinePages.ShowOfflinePageOnBadNetwork", true);
-    OfflinePageUtils::MarkPageAccessed(
-        web_contents()->GetBrowserContext(), navigation_handle->GetURL());
+  content::BrowserContext* browser_context =
+      web_contents()->GetBrowserContext();
+
+  // If the offline page is being loaded successfully, set the access record but
+  // no need to do anything else.
+  if (error_code == net::OK) {
+    OfflinePageUtils::GetOnlineURLForOfflineURL(
+        browser_context, navigated_url,
+        base::Bind(&ReportAccessedOfflinePage, browser_context, navigated_url));
+    return;
   }
 
-  // Skips load failure other than no network.
+  // When the navigation starts, we redirect immediately from online page to
+  // offline version on the case that there is no network connection. If there
+  // is still network connection but with no or poor network connectivity, the
+  // 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 &&
@@ -102,30 +108,57 @@ void OfflinePageTabHelper::DidFinishNavigation(
     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) {
+  // 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.
+  if (!redirect_url.is_valid() ||
+      transition == ui::PAGE_TRANSITION_FORWARD_BACK) {
     UMA_HISTOGRAM_BOOLEAN("OfflinePages.ShowOfflinePageOnBadNetwork", false);

[jianli, 2016/06/04 00:27:20]

Can we combine both UMA reporting lines, like:
  bool do_redirect = redirect_url.is_valid() && transition !=
ui::PAGE_TRANSITION_FORWARD_BACK;
  UMA_HISTOGRAM_BOOLEAN("OfflinePages.ShowOfflinePageOnBadNetwork",
do_redirect);
  if (!do_redirect)
    return;

  ...

[dewittj, 2016/06/06 16:53:15]

great idea.  Done.

     return;
   }
 
-  // When the navigation starts, we redirect immediately from online page to
-  // offline version on the case that there is no network connection. If there
-  // is still network connection but with no or poor network connectivity, the
-  // navigation will eventually fail and we want to redirect to offline copy
-  // in this case.
-  GURL offline_url = offline_pages::OfflinePageUtils::GetOfflineURLForOnlineURL(
-      web_contents()->GetBrowserContext(), navigation_handle->GetURL());
-  if (!offline_url.is_valid()) {
-    UMA_HISTOGRAM_BOOLEAN("OfflinePages.ShowOfflinePageOnBadNetwork", false);
+  UMA_HISTOGRAM_BOOLEAN("OfflinePages.ShowOfflinePageOnBadNetwork", true);
+
+  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()) {

[jianli, 2016/06/04 00:27:20]

nit: remove {}

[dewittj, 2016/06/06 16:53:15]

Done.

+    return;
+  }
+
+  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(),
-                 navigation_handle->GetURL(), offline_url));
+                 weak_ptr_factory_.GetWeakPtr(), from_url, redirect_url));
 }
 
 void OfflinePageTabHelper::Redirect(