Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(596)

Unified Diff: chrome/browser/android/offline_pages/offline_page_tab_helper.cc

Issue 2040573002: [Offlining] Updates the tab helper to use the async OfflinePageModel API (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: address jianli's nits. Created 4 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..9f1f868b35f9f2ba746c7b8026adb81ce07f3b86 100644
--- a/chrome/browser/android/offline_pages/offline_page_tab_helper.cc
+++ b/chrome/browser/android/offline_pages/offline_page_tab_helper.cc
@@ -22,6 +22,18 @@
DEFINE_WEB_CONTENTS_USER_DATA_KEY(offline_pages::OfflinePageTabHelper);
namespace offline_pages {
+namespace {
+
+void ReportAccessedOfflinePage(content::BrowserContext* browser_context,
+ 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())
+ OfflinePageUtils::MarkPageAccessed(browser_context, navigated_url);
+}
+
+} // namespace
OfflinePageTabHelper::OfflinePageTabHelper(content::WebContents* web_contents)
: content::WebContentsObserver(web_contents),
@@ -37,6 +49,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 +63,18 @@ 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;
- }
-
- base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE,
- base::Bind(&OfflinePageTabHelper::Redirect,
- weak_ptr_factory_.GetWeakPtr(),
- navigation_handle->GetURL(), redirect_url));
}
void OfflinePageTabHelper::DidFinishNavigation(
@@ -84,17 +83,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 +110,55 @@ 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) {
- UMA_HISTOGRAM_BOOLEAN("OfflinePages.ShowOfflinePageOnBadNetwork", false);
+ // 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)
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);
+ 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;
+
+ 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(

Powered by Google App Engine
This is Rietveld 408576698