Use the final URL in the intent to open an auto-fetch page

This prevents a bug which can occurr for URLs that redirect. The bug is a bit
subtle, so see the crbug.com/937581 for more details.

We're almost guaranteed that Chrome hasn't seen a redirect for the final URL,
so this will fix the problem of redirecting before hitting the offline url
loader request interceptor. We will intercept requests for either the original
URL or the final URL.

Note that because we're using a different URL in the intent, when a URL is a
redirect, we will always open the page in a new tab. This is not ideal, but
is preferable to failing to open the page.

Bug: 937581
Change-Id: I04f02556e93eba1ca0b2380313f1052c7f8f7cd3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1505895
Commit-Queue: Dan H <harringtond@google.com>
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638312}

chrome/android/java/src/org/chromium/chrome/browser/offlinepages/AutoFetchNotifier.java

@@ -262,28 +262,31 @@ public class AutoFetchNotifier {
      * If the notification is tapped, it opens the offline page in Chrome.
      *
      * @param pageTitle     The title of the page. This is displayed on the notification.
-     * @param originalUrl   The originally requested URL (before any redirection).
+     * @param finalUrl      The requested URL (after any redirection).
      * @param tabId         ID of the tab where the auto-fetch occurred. This tab is used, if
      *                      available, to open the offline page when the notification is tapped.
      * @param offlineId     The offlineID for the offline page that was just saved.
      */
     @CalledByNative
     private static void showCompleteNotification(
-            String pageTitle, String originalUrl, int tabId, long offlineId) {
+            String pageTitle, String finalUrl, int tabId, long offlineId) {
         Context context = ContextUtils.getApplicationContext();
         OfflinePageUtils.getLoadUrlParamsForOpeningOfflineVersion(
-                originalUrl, offlineId, LaunchLocation.NOTIFICATION, (params) -> {
+                finalUrl, offlineId, LaunchLocation.NOTIFICATION, (params) -> {
                     showCompleteNotificationWithParams(
-                            pageTitle, tabId, offlineId, originalUrl, params);
+                            pageTitle, tabId, offlineId, finalUrl, params);
                 });
     }
 
     private static void showCompleteNotificationWithParams(
-            String pageTitle, int tabId, long offlineId, String originalUrl, LoadUrlParams params) {
+            String pageTitle, int tabId, long offlineId, String finalUrl, LoadUrlParams params) {
         Context context = ContextUtils.getApplicationContext();
         // Create an intent to handle tapping the notification.
         Intent clickIntent = new Intent(context, CompleteNotificationReceiver.class);
-        clickIntent.putExtra(EXTRA_URL, originalUrl);
+        // TODO(crbug.com/937581): We're using the final URL here so that redirects can't break
+        // the page load. This will result in opening a new tab if there was a redirect (because
+        // the URL doesn't match the old dino page), which is not ideal.
+        clickIntent.putExtra(EXTRA_URL, finalUrl);
         IntentHandler.setIntentExtraHeaders(params.getExtraHeaders(), clickIntent);
         clickIntent.putExtra(TabOpenType.REUSE_TAB_MATCHING_ID_STRING, tabId);
         clickIntent.putExtra(Browser.EXTRA_APPLICATION_ID, context.getPackageName());

chrome/browser/offline_pages/android/auto_fetch_notifier.cc

@@ -58,14 +58,14 @@ void AutoFetchCancellationComplete() {
 }
 
 void ShowAutoFetchCompleteNotification(const base::string16& pageTitle,
-                                       const std::string& url,
+                                       const std::string& final_url,
                                        int android_tab_id,
                                        int64_t offline_id) {
   JNIEnv* env = base::android::AttachCurrentThread();
   Java_AutoFetchNotifier_showCompleteNotification(
       env,
       base::android::ConvertUTF8ToJavaString(env, base::UTF16ToUTF8(pageTitle)),
-      base::android::ConvertUTF8ToJavaString(env, url), android_tab_id,
+      base::android::ConvertUTF8ToJavaString(env, final_url), android_tab_id,
       offline_id);
 }
 

chrome/browser/offline_pages/android/auto_fetch_notifier.h

@@ -25,8 +25,8 @@ bool AutoFetchInProgressNotificationCanceled();
 void AutoFetchCancellationComplete();
 
 // Triggers the auto-fetch complete notification. See AutoFetchNotifier.java.
-void ShowAutoFetchCompleteNotification(const base::string16& pageTitle,
-                                       const std::string& originalUrl,
+void ShowAutoFetchCompleteNotification(const base::string16& page_title,
+                                       const std::string& final_url,
                                        int android_tab_id,
                                        int64_t offline_id);
 

chrome/browser/offline_pages/android/offline_page_auto_fetcher_service.cc

@@ -173,10 +173,7 @@ void OfflinePageAutoFetcherService::AutoFetchComplete(
   if (!metadata)
     return;
 
-  const GURL& url =
-      page->original_url.is_empty() ? page->url : page->original_url;
-
-  delegate_->ShowAutoFetchCompleteNotification(page->title, url.spec(),
+  delegate_->ShowAutoFetchCompleteNotification(page->title, page->url.spec(),
                                                metadata.value().android_tab_id,
                                                page->offline_id);
 }