Convert ExternalNavigationHandler to GURL

No functional changes intended, but GURL parsing could subtly differ
from the previous Uri and String parsing in unexpected ways.

Bug: 783819
Change-Id: I118fae14c47e28058f9735d0bc74b832d5790578
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2792241
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#870825}

chrome/android/java/src/org/chromium/chrome/browser/IntentHandler.java

@@ -63,6 +63,7 @@
 import org.chromium.net.HttpUtil;
 import org.chromium.network.mojom.ReferrerPolicy;
 import org.chromium.ui.base.PageTransition;
+import org.chromium.url.GURL;
 import org.chromium.url.Origin;
 
 import java.lang.annotation.Retention;
@@ -1383,10 +1384,10 @@ public static boolean isGoogleChromeScheme(String url) {
      * @param intent The intent to which we add a referrer.
      * @param url The referrer URL.
      */
-    public static void setPendingReferrer(Intent intent, String url) {
-        intent.putExtra(Intent.EXTRA_REFERRER, Uri.parse(url));
+    public static void setPendingReferrer(Intent intent, GURL url) {
+        intent.putExtra(Intent.EXTRA_REFERRER, Uri.parse(url.getSpec()));
         intent.putExtra(IntentHandler.EXTRA_REFERRER_ID, ++sReferrerId);
-        sPendingReferrer = new Pair<Integer, String>(sReferrerId, url);
+        sPendingReferrer = new Pair<Integer, String>(sReferrerId, url.getSpec());
     }
 
     /**

chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java

@@ -1129,8 +1129,7 @@ public boolean shouldInterceptNavigation(
                     mLastUserInteractionTimeSupplier.get(), RedirectHandler.INVALID_ENTRY_INDEX);
             ExternalNavigationParams params =
                     new ExternalNavigationParams
-                            .Builder(navigationParams.url.getSpec(), false,
-                                    navigationParams.referrer.getSpec(),
+                            .Builder(navigationParams.url, false, navigationParams.referrer,
                                     navigationParams.pageTransitionType,
                                     navigationParams.isRedirect)
                             .setApplicationMustBeInForeground(true)

chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabDelegateFactory.java

@@ -185,10 +185,11 @@ public boolean hasExternalActivityStarted() {
         }
 
         @Override
-        public boolean shouldDisableExternalIntentRequestsForUrl(String url) {
+        public boolean shouldDisableExternalIntentRequestsForUrl(GURL url) {
             // http://crbug.com/647569 : Do not forward URL requests to external intents for URLs
             // within the Webapp/TWA's scope.
-            return mVerifier != null && mVerifier.shouldIgnoreExternalIntentHandlers(url);
+            // TODO(https://crbug.com/783819): Migrate verifier hierarchy to GURL.
+            return mVerifier != null && mVerifier.shouldIgnoreExternalIntentHandlers(url.getSpec());
         }
     }
 

chrome/android/java/src/org/chromium/chrome/browser/externalnav/ExternalNavigationDelegateImpl.java

@@ -11,7 +11,6 @@
 import android.content.pm.ResolveInfo;
 import android.net.Uri;
 import android.provider.Browser;
-import android.text.TextUtils;
 
 import androidx.annotation.Nullable;
 
@@ -44,6 +43,7 @@
 import org.chromium.content_public.browser.LoadUrlParams;
 import org.chromium.content_public.browser.WebContents;
 import org.chromium.ui.base.WindowAndroid;
+import org.chromium.url.GURL;
 import org.chromium.url.Origin;
 
 import java.util.List;
@@ -118,7 +118,7 @@ public boolean willAppHandleIntent(Intent intent) {
     }
 
     @Override
-    public boolean shouldDisableExternalIntentRequestsForUrl(String url) {
+    public boolean shouldDisableExternalIntentRequestsForUrl(GURL url) {
         return false;
     }
 
@@ -154,10 +154,10 @@ public void didStartActivity(Intent intent) {}
 
     @Override
     public OverrideUrlLoadingResult handleIncognitoIntentTargetingSelf(
-            final Intent intent, final String referrerUrl, final String fallbackUrl) {
+            final Intent intent, final GURL referrerUrl, final GURL fallbackUrl) {
         String primaryUrl = intent.getDataString();
         boolean isUrlLoadedInTheSameTab = ExternalNavigationHandler.loadUrlFromIntent(
-                referrerUrl, primaryUrl, fallbackUrl, this, false, true);
+                referrerUrl, new GURL(primaryUrl), fallbackUrl.getSpec(), this, false, true);
         return (isUrlLoadedInTheSameTab) ? OverrideUrlLoadingResult.forClobberingTab()
                                          : OverrideUrlLoadingResult.forExternalIntent();
     }
@@ -168,8 +168,8 @@ public boolean supportsCreatingNewTabs() {
     }
 
     @Override
-    public void loadUrlInNewTab(final String url, final boolean launchIncognito) {
-        Intent intent = new Intent(Intent.ACTION_VIEW, Uri.parse(url));
+    public void loadUrlInNewTab(final GURL url, final boolean launchIncognito) {
+        Intent intent = new Intent(Intent.ACTION_VIEW, Uri.parse(url.getSpec()));
         String packageName = ContextUtils.getApplicationContext().getPackageName();
         intent.putExtra(Browser.EXTRA_APPLICATION_ID, packageName);
         if (launchIncognito) intent.putExtra(IntentHandler.EXTRA_OPEN_NEW_INCOGNITO_TAB, true);
@@ -241,7 +241,7 @@ public void maybeSetRequestMetadata(Intent intent, boolean hasUserGesture,
     }
 
     @Override
-    public void maybeSetPendingReferrer(Intent intent, String referrerUrl) {
+    public void maybeSetPendingReferrer(Intent intent, GURL referrerUrl) {
         IntentHandler.setPendingReferrer(intent, referrerUrl);
     }
 
@@ -252,7 +252,7 @@ public void maybeSetPendingIncognitoUrl(Intent intent) {
 
     @Override
     public boolean maybeLaunchInstantApp(
-            String url, String referrerUrl, boolean isIncomingRedirect, boolean isSerpReferrer) {
+            GURL url, GURL referrerUrl, boolean isIncomingRedirect, boolean isSerpReferrer) {
         if (!hasValidTab() || mTab.getWebContents() == null) return false;
 
         InstantAppsHandler handler = InstantAppsHandler.getInstance();
@@ -263,14 +263,13 @@ public boolean maybeLaunchInstantApp(
             // Set the URL the redirect was resolved to for checking the existence of the
             // instant app inside handleIncomingIntent().
             Intent resolvedIntent = new Intent(intent);
-            resolvedIntent.setData(Uri.parse(url));
+            resolvedIntent.setData(Uri.parse(url.getSpec()));
             return handler.handleIncomingIntent(getAvailableContext(), resolvedIntent,
                     LaunchIntentDispatcher.isCustomTabIntent(resolvedIntent), true);
         } else if (!isIncomingRedirect) {
             // Check if the navigation is coming from SERP and skip instant app handling.
             if (isSerpReferrer) return false;
-            return handler.handleNavigation(getAvailableContext(), url,
-                    TextUtils.isEmpty(referrerUrl) ? null : Uri.parse(referrerUrl), mTab);
+            return handler.handleNavigation(getAvailableContext(), url, referrerUrl, mTab);
         }
         return false;
     }
@@ -300,10 +299,9 @@ public void dispatchAuthenticatedIntent(Intent intent) {
      * Starts the autofill assistant with the given intent. Exists to allow tests to stub out this
      * functionality.
      */
-    protected void startAutofillAssistantWithIntent(
-            Intent targetIntent, String browserFallbackUrl) {
+    protected void startAutofillAssistantWithIntent(Intent targetIntent, GURL browserFallbackUrl) {
         AutofillAssistantFacade.start(
-                TabUtils.getActivity(mTab), targetIntent.getExtras(), browserFallbackUrl);
+                TabUtils.getActivity(mTab), targetIntent.getExtras(), browserFallbackUrl.getSpec());
     }
 
     /**
@@ -336,8 +334,8 @@ public boolean isIntentToAutofillAssistant(Intent intent) {
 
     @Override
     public boolean handleWithAutofillAssistant(ExternalNavigationParams params, Intent targetIntent,
-            String browserFallbackUrl, boolean isGoogleReferrer) {
-        if (browserFallbackUrl != null && !params.isIncognito()
+            GURL browserFallbackUrl, boolean isGoogleReferrer) {
+        if (!browserFallbackUrl.isEmpty() && !params.isIncognito()
                 && AutofillAssistantFacade.isAutofillAssistantByIntentTriggeringEnabled(
                         targetIntent)
                 && isGoogleReferrer) {

chrome/android/java/src/org/chromium/chrome/browser/instantapps/InstantAppsHandler.java

@@ -23,6 +23,7 @@
 import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.content_public.browser.WebContents;
+import org.chromium.url.GURL;
 
 /** A launcher for Instant Apps. */
 public class InstantAppsHandler {
@@ -231,13 +232,14 @@ protected boolean tryLaunchingInstantApp(
      * App banner.
      * @return Whether an Instant App intent was started.
      */
-    public boolean handleNavigation(Context context, String url, Uri referrer, Tab tab) {
+    public boolean handleNavigation(Context context, GURL url, GURL referrer, Tab tab) {
         boolean urlIsInstantAppDefault =
                 InstantAppsSettings.isInstantAppDefault(tab.getWebContents(), url);
+        Uri referrerUri = referrer.isEmpty() ? null : Uri.parse(referrer.getSpec());
         if (shouldLaunchInstantApp(tab.getWebContents(), url, referrer, urlIsInstantAppDefault)) {
-            return launchInstantAppForNavigation(context, url, referrer);
+            return launchInstantAppForNavigation(context, url.getSpec(), referrerUri);
         }
-        maybeShowInstantAppBanner(context, url, referrer, tab, urlIsInstantAppDefault);
+        maybeShowInstantAppBanner(context, url.getSpec(), referrerUri, tab, urlIsInstantAppDefault);
         return false;
     }
 
@@ -250,18 +252,16 @@ public boolean handleNavigation(Context context, String url, Uri referrer, Tab t
      * @return Whether we should launch the instant app.
      */
     private boolean shouldLaunchInstantApp(
-            WebContents webContents, String url, Uri referrer, boolean urlIsInstantAppDefault) {
+            WebContents webContents, GURL url, GURL referrer, boolean urlIsInstantAppDefault) {
         // Launch the instant app automatically on these conditions:
         // a) The host of the current URL and referrer are different, and the user has chosen to
         //    launch this instant app in the past.
         // b) The host of the current URL and referrer are the same, but the referrer URL isn't
         //    handled by an instant app and the current one is.
         if (!urlIsInstantAppDefault) return false;
 
-        String urlHost = Uri.parse(url).getHost();
-        boolean sameHosts =
-                referrer != null && urlHost != null && urlHost.equals(referrer.getHost());
-        return (sameHosts && getInstantAppIntentForUrl(referrer.toString()) == null) || !sameHosts;
+        boolean sameHosts = !referrer.isEmpty() && url.getHost().equals(referrer.getHost());
+        return (sameHosts && getInstantAppIntentForUrl(referrer.getSpec()) == null) || !sameHosts;
     }
 
     /**

chrome/android/java/src/org/chromium/chrome/browser/instantapps/InstantAppsSettings.java

@@ -6,6 +6,7 @@
 
 import org.chromium.base.annotations.NativeMethods;
 import org.chromium.content_public.browser.WebContents;
+import org.chromium.url.GURL;
 
 /**
  * A bridge class for retrieving Instant Apps-related settings.
@@ -15,7 +16,7 @@ public class InstantAppsSettings {
     /**
      * Check whether the instant app at the given url should be opened by default.
      */
-    public static boolean isInstantAppDefault(WebContents webContents, String url) {
+    public static boolean isInstantAppDefault(WebContents webContents, GURL url) {
         return InstantAppsSettingsJni.get().getInstantAppDefault(webContents, url);
     }
 
@@ -35,7 +36,7 @@ public static boolean shouldShowBanner(WebContents webContents, String url) {
 
     @NativeMethods
     interface Natives {
-        boolean getInstantAppDefault(WebContents webContents, String url);
+        boolean getInstantAppDefault(WebContents webContents, GURL url);
         void setInstantAppDefault(WebContents webContents, String url);
         boolean shouldShowBanner(WebContents webContents, String url);
     }

chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabExternalNavigationTest.java

@@ -33,6 +33,7 @@
 import org.chromium.components.external_intents.ExternalNavigationHandler.OverrideUrlLoadingResultType;
 import org.chromium.components.external_intents.ExternalNavigationParams;
 import org.chromium.net.test.EmbeddedTestServer;
+import org.chromium.url.GURL;
 
 import java.util.concurrent.TimeoutException;
 
@@ -107,7 +108,7 @@ private void launchTwa(String twaPackageName, String url) throws TimeoutExceptio
     @Test
     @SmallTest
     public void testExternalActivityStartedForDefaultUrl() {
-        final String testUrl = "customtab://customtabtest/intent";
+        final GURL testUrl = new GURL("customtab://customtabtest/intent");
         ExternalNavigationParams params = new ExternalNavigationParams.Builder(testUrl, false)
                 .build();
         OverrideUrlLoadingResult result = mUrlHandler.shouldOverrideUrlLoading(params);
@@ -127,7 +128,7 @@ public void testExternalActivityStartedForDefaultUrl() {
             sdk_is_less_than = VERSION_CODES.Q, message = "crbug.com/1188920")
     public void
     testIntentPickerNotShownForNormalUrl() {
-        final String testUrl = "http://customtabtest.com";
+        final GURL testUrl = new GURL("http://customtabtest.com");
         ExternalNavigationParams params = new ExternalNavigationParams.Builder(testUrl, false)
                 .build();
         OverrideUrlLoadingResult result = mUrlHandler.shouldOverrideUrlLoading(params);
@@ -149,9 +150,9 @@ public void testExternalActivityStartedForDefaultUrl() {
     @Test
     @SmallTest
     public void testShouldDisableExternalIntentRequestsForUrl() throws TimeoutException {
-        String insideVerifiedOriginUrl =
-                mTestServer.getURL("/chrome/test/data/android/simple.html");
-        String outsideVerifiedOriginUrl = "https://example.com/test.html";
+        GURL insideVerifiedOriginUrl =
+                new GURL(mTestServer.getURL("/chrome/test/data/android/simple.html"));
+        GURL outsideVerifiedOriginUrl = new GURL("https://example.com/test.html");
 
         TrustedWebActivityTestUtil.waitForCurrentPageVerifierToFinish(
                 mCustomTabActivityTestRule.getActivity());

chrome/android/javatests/src/org/chromium/chrome/browser/externalnav/ExternalNavigationDelegateImplTest.java

@@ -27,6 +27,7 @@
 import org.chromium.components.external_intents.ExternalNavigationHandler;
 import org.chromium.components.external_intents.ExternalNavigationParams;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
+import org.chromium.url.GURL;
 import org.chromium.url.Origin;
 
 /**
@@ -56,7 +57,7 @@ public ExternalNavigationDelegateImplForTesting(Tab activityTab) {
 
         @Override
         protected void startAutofillAssistantWithIntent(
-                Intent targetIntent, String browserFallbackUrl) {
+                Intent targetIntent, GURL browserFallbackUrl) {
             mWasAutofillAssistantStarted = true;
         }
 
@@ -76,7 +77,7 @@ public boolean handleWithAutofillAssistant(
                 return false;
             }
 
-            String fallbackUrl = "https://www.example.com";
+            GURL fallbackUrl = new GURL("https://www.example.com");
 
             return handleWithAutofillAssistant(params, intent, fallbackUrl, isGoogleReferrer);
         }
@@ -213,12 +214,12 @@ public void maybeSetRequestMetadata() {
     @Test
     @SmallTest
     public void testMaybeSetPendingReferrer() {
-        String url = "http://www.example.com";
+        String url = "http://www.example.com/";
         Intent intent = new Intent(Intent.ACTION_VIEW);
         intent.setData(Uri.parse(url));
 
-        String referrerUrl = "http://www.example-referrer.com";
-        mExternalNavigationDelegateImpl.maybeSetPendingReferrer(intent, referrerUrl);
+        String referrerUrl = "http://www.example-referrer.com/";
+        mExternalNavigationDelegateImpl.maybeSetPendingReferrer(intent, new GURL(referrerUrl));
 
         Assert.assertEquals(
                 Uri.parse(referrerUrl), intent.getParcelableExtra(Intent.EXTRA_REFERRER));
@@ -234,7 +235,7 @@ public void testMaybeSetPendingReferrer() {
     testHandleWithAutofillAssistant_TriggersFromSearch() {
         ExternalNavigationParams params =
                 new ExternalNavigationParams
-                        .Builder(AUTOFILL_ASSISTANT_INTENT_URL, /*isIncognito=*/false)
+                        .Builder(new GURL(AUTOFILL_ASSISTANT_INTENT_URL), /*isIncognito=*/false)
                         .build();
 
         Assert.assertTrue(mExternalNavigationDelegateImplForTesting.handleWithAutofillAssistant(
@@ -250,7 +251,7 @@ public void testMaybeSetPendingReferrer() {
     testHandleWithAutofillAssistant_DoesNotTriggerFromSearchInIncognito() {
         ExternalNavigationParams params =
                 new ExternalNavigationParams
-                        .Builder(AUTOFILL_ASSISTANT_INTENT_URL, /*isIncognito=*/true)
+                        .Builder(new GURL(AUTOFILL_ASSISTANT_INTENT_URL), /*isIncognito=*/true)
                         .build();
 
         Assert.assertFalse(mExternalNavigationDelegateImplForTesting.handleWithAutofillAssistant(
@@ -266,7 +267,7 @@ public void testMaybeSetPendingReferrer() {
     testHandleWithAutofillAssistant_DoesNotTriggerFromDifferentOrigin() {
         ExternalNavigationParams params =
                 new ExternalNavigationParams
-                        .Builder(AUTOFILL_ASSISTANT_INTENT_URL, /*isIncognito=*/false)
+                        .Builder(new GURL(AUTOFILL_ASSISTANT_INTENT_URL), /*isIncognito=*/false)
                         .build();
 
         Assert.assertFalse(mExternalNavigationDelegateImplForTesting.handleWithAutofillAssistant(
@@ -282,7 +283,7 @@ public void testMaybeSetPendingReferrer() {
     testHandleWithAutofillAssistant_DoesNotTriggerWhenFeatureDisabled() {
         ExternalNavigationParams params =
                 new ExternalNavigationParams
-                        .Builder(AUTOFILL_ASSISTANT_INTENT_URL, /*isIncognito=*/false)
+                        .Builder(new GURL(AUTOFILL_ASSISTANT_INTENT_URL), /*isIncognito=*/false)
                         .build();
 
         Assert.assertFalse(mExternalNavigationDelegateImplForTesting.handleWithAutofillAssistant(

chrome/android/javatests/src/org/chromium/chrome/browser/instantapps/InstantAppsHandlerTest.java

@@ -31,6 +31,7 @@
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
+import org.chromium.url.GURL;
 
 /**
  * Unit tests for {@link InstantAppsHandler}.
@@ -162,11 +163,12 @@ public void testNfcIntent() {
     @SmallTest
     public void testHandleNavigation_startAsyncCheck() {
         TestThreadUtils.runOnUiThreadBlocking(
-                () -> Assert.assertFalse(
-                                mHandler.handleNavigation(mContext, INSTANT_APP_URL, REFERRER_URI,
-                                        mActivityTestRule.getActivity()
-                                                .getTabModelSelector()
-                                                .getCurrentTab())));
+                ()
+                        -> Assert.assertFalse(mHandler.handleNavigation(mContext,
+                                new GURL(INSTANT_APP_URL), new GURL(REFERRER_URI.toString()),
+                                mActivityTestRule.getActivity()
+                                        .getTabModelSelector()
+                                        .getCurrentTab())));
         Assert.assertFalse(mHandler.mLaunchInstantApp);
         Assert.assertTrue(mHandler.mStartedAsyncCall);
     }
@@ -204,11 +206,12 @@ public void testLaunchFromBanner() {
         // After a banner launch, test that the next launch happens automatically
 
         TestThreadUtils.runOnUiThreadBlocking(
-                () -> Assert.assertTrue(
-                                mHandler.handleNavigation(mContext, INSTANT_APP_URL, REFERRER_URI,
-                                        mActivityTestRule.getActivity()
-                                                .getTabModelSelector()
-                                                .getCurrentTab())));
+                ()
+                        -> Assert.assertTrue(mHandler.handleNavigation(mContext,
+                                new GURL(INSTANT_APP_URL), new GURL(REFERRER_URI.toString()),
+                                mActivityTestRule.getActivity()
+                                        .getTabModelSelector()
+                                        .getCurrentTab())));
         Assert.assertFalse(mHandler.mStartedAsyncCall);
         Assert.assertTrue(mHandler.mLaunchInstantApp);
     }