[Offline Pages on the NTP] Bypass the network interstitial

chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java

Index: chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
index 857151e7928088458671d472985fb4cf00a8727d..6bb1171dce02734e87c312a029ad13fe75ed43e8 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
@@ -46,6 +46,8 @@ import org.chromium.chrome.browser.ntp.NewTabPageView.NewTabPageManager;
 import org.chromium.chrome.browser.ntp.interests.InterestsPage;
 import org.chromium.chrome.browser.ntp.interests.InterestsPage.InterestsClickListener;
 import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
+import org.chromium.chrome.browser.offlinepages.OfflinePageItem;
+import org.chromium.chrome.browser.offlinepages.OfflinePageUtils;
 import org.chromium.chrome.browser.preferences.DocumentModeManager;
 import org.chromium.chrome.browser.preferences.DocumentModePreference;
 import org.chromium.chrome.browser.preferences.PrefServiceBridge;
@@ -327,6 +329,17 @@ public class NewTabPage
 
         @Override
         public void open(String url) {
+            if (!OfflinePageUtils.isConnected(mNewTabPageView.getContext()) && !mIsDestroyed

[newt (away), 2016/01/12 20:58:55]

This seems like the wrong layer (too high) to be adding this logic in.
Redirecting to the offline version of a page should happen at a lower layer,
common to all navigations.

One problem with the current solution is that it doesn't handle the myriad other
ways that offline pages could be opened: from the bookmarks page, from the
recent tabs page, from an add-to-homescreen link, from an omnibox navigation,
etc.

Here are some options to consider:
 - BrowserURLHandler. See e.g.
ChromeContentBrowserClient::BrowserURLHandlerCreated() and
new_tab_page_url_handler.cc
 - InterceptNavigationDelegateImpl.java
 - ResourceThrottler/NavigationThrottler

Someone who knows the content/ layer well can probably point you in the right
direction.

[Marc Treib, 2016/01/13 09:38:19]

I don't think we want to do the automatic indirection in general. The usual flow
is that you follow the online link (from wherever) and get a network
interstitial, which tells you you're offline and offers to show the offline copy
instead.
The NTP is different because it will have special UI to indicate that you'll see
an offline copy, so there's no need to also show the interstitial.

[newt (away), 2016/01/13 22:05:12]

Ok, I'll buy that distinction between navigation from the NTP and navigation
from anywhere else in Chrome :)

In that case, could you encapsulate this logic for picking which URL to load
inside OfflinePageUtils? e.g. something like
OfflinePageUtils.getOfflineUrlIfAvailableAndOffline(String url). It makes more
sense for OfflinePageUtils to deal with offline-specific details than for
NewTabPage; and this method seems like something that might be reused elsewhere.

[Marc Treib, 2016/01/14 10:09:17]

Done.

+                    && OfflinePageBridge.isEnabled()) {
+                if (mOfflinePageBridge == null) {
+                    mOfflinePageBridge = new OfflinePageBridge(mProfile);
+                }
+                OfflinePageItem offlineItem = mOfflinePageBridge.getPageByOnlineURL(url);
+                if (offlineItem != null) {
+                    mOfflinePageBridge.markPageAccessed(offlineItem.getBookmarkId());
+                    url = offlineItem.getOfflineUrl();
+                }
+            }
             mTab.loadUrl(new LoadUrlParams(url, PageTransition.AUTO_BOOKMARK));
         }
 
@@ -446,7 +459,7 @@ public class NewTabPage
         public boolean isOfflineAvailable(String pageUrl) {
             if (mIsDestroyed || !OfflinePageBridge.isEnabled()) return false;
             if (mOfflinePageBridge == null) mOfflinePageBridge = new OfflinePageBridge(mProfile);
-            return mOfflinePageBridge.getPageByOnlineURL(pageUrl) != null;
+            return mOfflinePageBridge.getOfflineUrlForOnlineUrl(pageUrl) != null;
         }
 
         @Override