[Android] Move autofill assistant knowledge to external nav delegate

This CL moves knowledge of the //chrome-level AutofillAssistant out of
ExternalNavigationHandler.java and into
ExternalNavigationDelegateImpl.java via the ExternalNavigationDelegate
interface. This change is a step toward componentization of
ExternalNavigationHandler.java for sharing with WebLayer. Via this
change, we also eliminate knowledge of //chrome's Tab object from
ExternalNavigationHandler.

The bulk of this change is in the tests, as
ExternalNavigationHandlerTest had several tests of the functionality
that this CL moves. We split those tests across the test of the handler
and the test of the delegate impl: the tests in the handler now test
that the handler behaves as expected when the intent is/is not handled
by the the delegate in this case, while the delegate impl tests test
that the behavior of the production method is as expected (i.e., these
tests contain the bulk of the logic that was previously in the
handler-level tests of the production code).

Change-Id: If950a3fac701744af4610173e5c221bdb8a8a7c1
Bug: 1031465
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2081418
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746865}

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

@@ -8,6 +8,7 @@ import org.chromium.base.supplier.Supplier;
 import org.chromium.chrome.browser.contextmenu.ChromeContextMenuPopulator;
 import org.chromium.chrome.browser.contextmenu.ContextMenuPopulator;
 import org.chromium.chrome.browser.externalauth.ExternalAuthUtils;
+import org.chromium.chrome.browser.externalnav.ExternalNavigationDelegateImpl;
 import org.chromium.chrome.browser.externalnav.ExternalNavigationHandler;
 import org.chromium.chrome.browser.share.ShareDelegate;
 import org.chromium.chrome.browser.tab.Tab;
@@ -43,7 +44,7 @@ public class TabbedModeTabDelegateFactory implements TabDelegateFactory {
 
     @Override
     public ExternalNavigationHandler createExternalNavigationHandler(Tab tab) {
-        return new ExternalNavigationHandler(tab);
+        return new ExternalNavigationHandler(new ExternalNavigationDelegateImpl(tab));
     }
 
     @Override

chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelContent.java

@@ -19,6 +19,7 @@ import org.chromium.chrome.browser.ChromeVersionInfo;
 import org.chromium.chrome.browser.WebContentsFactory;
 import org.chromium.chrome.browser.content.ContentUtils;
 import org.chromium.chrome.browser.contextualsearch.ContextualSearchManager;
+import org.chromium.chrome.browser.externalnav.ExternalNavigationDelegateImpl;
 import org.chromium.chrome.browser.externalnav.ExternalNavigationHandler;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.components.embedder_support.delegate.WebContentsDelegateAndroid;
@@ -142,7 +143,7 @@ public class OverlayPanelContent {
         public InterceptNavigationDelegateImpl() {
             Tab tab = mActivity.getActivityTab();
             mExternalNavHandler = (tab != null && tab.getWebContents() != null)
-                    ? new ExternalNavigationHandler(tab)
+                    ? new ExternalNavigationHandler(new ExternalNavigationDelegateImpl(tab))
                     : null;
         }
 

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

@@ -160,4 +160,10 @@ interface ExternalNavigationDelegate {
      * @return Whether the package is a valid WebAPK package.
      */
     boolean isValidWebApk(String packageName);
+
+    /**
+     * Gives the embedder a chance to handle the intent via the autofill assistant.
+     */
+    boolean handleWithAutofillAssistant(
+            ExternalNavigationParams params, Intent targetIntent, String browserFallbackUrl);
 }

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

@@ -39,12 +39,14 @@ import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.ChromeTabbedActivity2;
 import org.chromium.chrome.browser.IntentHandler;
 import org.chromium.chrome.browser.LaunchIntentDispatcher;
+import org.chromium.chrome.browser.autofill_assistant.AutofillAssistantFacade;
 import org.chromium.chrome.browser.document.ChromeLauncherActivity;
 import org.chromium.chrome.browser.externalnav.ExternalNavigationHandler.OverrideUrlLoadingResult;
 import org.chromium.chrome.browser.instantapps.AuthenticatedProxyActivity;
 import org.chromium.chrome.browser.instantapps.InstantAppsHandler;
 import org.chromium.chrome.browser.tab.EmptyTabObserver;
 import org.chromium.chrome.browser.tab.Tab;
+import org.chromium.chrome.browser.tab.TabImpl;
 import org.chromium.chrome.browser.tab.TabObserver;
 import org.chromium.chrome.browser.tab.TabRedirectHandler;
 import org.chromium.chrome.browser.webapps.WebappActivity;
@@ -644,4 +646,20 @@ public class ExternalNavigationDelegateImpl implements ExternalNavigationDelegat
     public boolean isValidWebApk(String packageName) {
         return WebApkValidator.isValidWebApk(ContextUtils.getApplicationContext(), packageName);
     }
+
+    @Override
+    public boolean handleWithAutofillAssistant(
+            ExternalNavigationParams params, Intent targetIntent, String browserFallbackUrl) {
+        if (browserFallbackUrl != null && !params.isIncognito()
+                && AutofillAssistantFacade.isAutofillAssistantByIntentTriggeringEnabled(
+                        targetIntent)
+                && isSerpReferrer()) {
+            if (params.getTab() != null) {
+                AutofillAssistantFacade.start(((TabImpl) params.getTab()).getActivity(),
+                        targetIntent.getExtras(), browserFallbackUrl);
+            }
+            return true;
+        }
+        return false;
+    }
 }

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

@@ -27,11 +27,8 @@ import org.chromium.base.Log;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.chrome.browser.IntentHandler;
-import org.chromium.chrome.browser.autofill_assistant.AutofillAssistantFacade;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.instantapps.InstantAppsHandler;
-import org.chromium.chrome.browser.tab.Tab;
-import org.chromium.chrome.browser.tab.TabImpl;
 import org.chromium.chrome.browser.tab.TabRedirectHandler;
 import org.chromium.chrome.browser.webapps.WebappScopePolicy;
 import org.chromium.components.embedder_support.util.UrlConstants;
@@ -153,15 +150,6 @@ public class ExternalNavigationHandler {
         int NUM_ENTRIES = 4;
     }
 
-    /**
-     * A constructor for UrlHandler.
-     *
-     * @param tab The tab that initiated the external intent.
-     */
-    public ExternalNavigationHandler(Tab tab) {
-        this(new ExternalNavigationDelegateImpl(tab));
-    }
-
     /**
      * Constructs a new instance of {@link ExternalNavigationHandler}, using the injected
      * {@link ExternalNavigationDelegate}.
@@ -765,14 +753,7 @@ public class ExternalNavigationHandler {
 
     private boolean handleWithAutofillAssistant(
             ExternalNavigationParams params, Intent targetIntent, String browserFallbackUrl) {
-        if (browserFallbackUrl != null && !params.isIncognito()
-                && AutofillAssistantFacade.isAutofillAssistantByIntentTriggeringEnabled(
-                        targetIntent)
-                && mDelegate.isSerpReferrer()) {
-            if (params.getTab() != null) {
-                AutofillAssistantFacade.start(((TabImpl) params.getTab()).getActivity(),
-                        targetIntent.getExtras(), browserFallbackUrl);
-            }
+        if (mDelegate.handleWithAutofillAssistant(params, targetIntent, browserFallbackUrl)) {
             if (DEBUG) Log.i(TAG, "Handling with Assistant");
             return true;
         }

chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManagerTest.java

@@ -66,6 +66,7 @@ import org.chromium.chrome.browser.contextualsearch.ContextualSearchFakeServer.F
 import org.chromium.chrome.browser.contextualsearch.ContextualSearchFakeServer.MutableResolvedSearchTerm;
 import org.chromium.chrome.browser.contextualsearch.ContextualSearchInternalStateController.InternalState;
 import org.chromium.chrome.browser.contextualsearch.ResolvedSearchTerm.CardTag;
+import org.chromium.chrome.browser.externalnav.ExternalNavigationDelegateImpl;
 import org.chromium.chrome.browser.externalnav.ExternalNavigationHandler;
 import org.chromium.chrome.browser.findinpage.FindToolbar;
 import org.chromium.chrome.browser.firstrun.FirstRunStatus;
@@ -2217,7 +2218,8 @@ public class ContextualSearchManagerTest {
     @Feature({"ContextualSearch"})
     public void testExternalNavigationWithUserGesture() {
         final ExternalNavigationHandler externalNavHandler =
-                new ExternalNavigationHandler(mActivityTestRule.getActivity().getActivityTab());
+                new ExternalNavigationHandler(new ExternalNavigationDelegateImpl(
+                        mActivityTestRule.getActivity().getActivityTab()));
         final NavigationParams navigationParams = new NavigationParams(
                 "intent://test/#Intent;scheme=test;package=com.chrome.test;end", "",
                 false /* isPost */, true /* hasUserGesture */, PageTransition.LINK,
@@ -2242,7 +2244,8 @@ public class ContextualSearchManagerTest {
     @Feature({"ContextualSearch"})
     public void testRedirectedExternalNavigationWithUserGesture() {
         final ExternalNavigationHandler externalNavHandler =
-                new ExternalNavigationHandler(mActivityTestRule.getActivity().getActivityTab());
+                new ExternalNavigationHandler(new ExternalNavigationDelegateImpl(
+                        mActivityTestRule.getActivity().getActivityTab()));
 
         final NavigationParams initialNavigationParams = new NavigationParams("http://test.com", "",
                 false /* isPost */, true /* hasUserGesture */, PageTransition.LINK,
@@ -2276,7 +2279,8 @@ public class ContextualSearchManagerTest {
     @Feature({"ContextualSearch"})
     public void testExternalNavigationWithoutUserGesture() {
         final ExternalNavigationHandler externalNavHandler =
-                new ExternalNavigationHandler(mActivityTestRule.getActivity().getActivityTab());
+                new ExternalNavigationHandler(new ExternalNavigationDelegateImpl(
+                        mActivityTestRule.getActivity().getActivityTab()));
         final NavigationParams navigationParams = new NavigationParams(
                 "intent://test/#Intent;scheme=test;package=com.chrome.test;end", "",
                 false /* isPost */, false /* hasUserGesture */, PageTransition.LINK,

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

@@ -4,9 +4,11 @@
 
 package org.chromium.chrome.browser.externalnav;
 
+import android.content.Intent;
 import android.content.IntentFilter;
 import android.content.pm.ActivityInfo;
 import android.content.pm.ResolveInfo;
+import android.net.Uri;
 import android.os.Build;
 import android.support.test.filters.SmallTest;
 
@@ -19,9 +21,11 @@ import org.junit.runner.RunWith;
 import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.ChromeSwitches;
+import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.instantapps.InstantAppsHandler;
 import org.chromium.chrome.test.ChromeActivityTestRule;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
+import org.chromium.chrome.test.util.browser.Features;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -31,8 +35,50 @@ import java.util.List;
  * Instrumentation tests for {@link ExternalNavigationHandler}.
  */
 @RunWith(ChromeJUnit4ClassRunner.class)
-@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
-public class ExternalNavigationDelegateImplTest {
+@CommandLineFlags
+        .Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
+        @Features.DisableFeatures({ChromeFeatureList.AUTOFILL_ASSISTANT,
+                ChromeFeatureList.AUTOFILL_ASSISTANT_CHROME_ENTRY})
+        public class ExternalNavigationDelegateImplTest {
+    private static final String AUTOFILL_ASSISTANT_INTENT_URL =
+            "intent://www.example.com#Intent;scheme=https;"
+            + "B.org.chromium.chrome.browser.autofill_assistant.ENABLED=true;"
+            + "S." + ExternalNavigationHandler.EXTRA_BROWSER_FALLBACK_URL + "="
+            + Uri.encode("https://www.example.com") + ";end";
+
+    class ExternalNavigationDelegateImplForTesting extends ExternalNavigationDelegateImpl {
+        public ExternalNavigationDelegateImplForTesting() {
+            super(mActivityTestRule.getActivity().getActivityTab());
+        }
+
+        @Override
+        public boolean isSerpReferrer() {
+            return mIsSerpReferrer;
+        }
+
+        public void setIsSerpReferrer(boolean value) {
+            mIsSerpReferrer = value;
+        }
+
+        // Convenience for testing that reduces boilerplate in constructing arguments to the
+        // production method that are common across tests.
+        public boolean handleWithAutofillAssistant(ExternalNavigationParams params) {
+            Intent intent;
+            try {
+                intent = Intent.parseUri(AUTOFILL_ASSISTANT_INTENT_URL, Intent.URI_INTENT_SCHEME);
+            } catch (Exception ex) {
+                Assert.assertTrue(false);
+                return false;
+            }
+
+            String fallbackUrl = "https://www.example.com";
+
+            return handleWithAutofillAssistant(params, intent, fallbackUrl);
+        }
+
+        private boolean mIsSerpReferrer;
+    }
+
     @Rule
     public ChromeActivityTestRule<ChromeActivity> mActivityTestRule =
             new ChromeActivityTestRule<>(ChromeActivity.class);
@@ -194,4 +240,79 @@ public class ExternalNavigationDelegateImplTest {
     public void setUp() throws InterruptedException {
         mActivityTestRule.startMainActivityOnBlankPage();
     }
+
+    @Test
+    @SmallTest
+    @Features.EnableFeatures({ChromeFeatureList.AUTOFILL_ASSISTANT,
+            ChromeFeatureList.AUTOFILL_ASSISTANT_CHROME_ENTRY})
+    public void
+    testHandleWithAutofillAssistant_TriggersFromSearch() {
+        ExternalNavigationDelegateImplForTesting delegate =
+                new ExternalNavigationDelegateImplForTesting();
+        delegate.setIsSerpReferrer(true);
+
+        // Note: Leave the tab of |params| null to ensure that the delegate doesn't ask
+        // AutofillAssistantFacade to actually start the activity, which this test is not set up
+        // for.
+        ExternalNavigationParams params =
+                new ExternalNavigationParams
+                        .Builder(AUTOFILL_ASSISTANT_INTENT_URL, /*isIncognito=*/false)
+                        .build();
+
+        Assert.assertTrue(delegate.handleWithAutofillAssistant(params));
+    }
+
+    @Test
+    @SmallTest
+    @Features.EnableFeatures({ChromeFeatureList.AUTOFILL_ASSISTANT,
+            ChromeFeatureList.AUTOFILL_ASSISTANT_CHROME_ENTRY})
+    public void
+    testHandleWithAutofillAssistant_DoesNotTriggerFromSearchInIncognito() {
+        ExternalNavigationDelegateImplForTesting delegate =
+                new ExternalNavigationDelegateImplForTesting();
+        delegate.setIsSerpReferrer(true);
+
+        ExternalNavigationParams params =
+                new ExternalNavigationParams
+                        .Builder(AUTOFILL_ASSISTANT_INTENT_URL, /*isIncognito=*/true)
+                        .build();
+
+        Assert.assertFalse(delegate.handleWithAutofillAssistant(params));
+    }
+
+    @Test
+    @SmallTest
+    @Features.EnableFeatures({ChromeFeatureList.AUTOFILL_ASSISTANT,
+            ChromeFeatureList.AUTOFILL_ASSISTANT_CHROME_ENTRY})
+    public void
+    testHandleWithAutofillAssistant_DoesNotTriggerFromDifferentOrigin() {
+        ExternalNavigationDelegateImplForTesting delegate =
+                new ExternalNavigationDelegateImplForTesting();
+        delegate.setIsSerpReferrer(false);
+
+        ExternalNavigationParams params =
+                new ExternalNavigationParams
+                        .Builder(AUTOFILL_ASSISTANT_INTENT_URL, /*isIncognito=*/false)
+                        .build();
+
+        Assert.assertFalse(delegate.handleWithAutofillAssistant(params));
+    }
+
+    @Test
+    @SmallTest
+    @Features.DisableFeatures({ChromeFeatureList.AUTOFILL_ASSISTANT,
+            ChromeFeatureList.AUTOFILL_ASSISTANT_CHROME_ENTRY})
+    public void
+    testHandleWithAutofillAssistant_DoesNotTriggerWhenFeatureDisabled() {
+        ExternalNavigationDelegateImplForTesting delegate =
+                new ExternalNavigationDelegateImplForTesting();
+        delegate.setIsSerpReferrer(true);
+
+        ExternalNavigationParams params =
+                new ExternalNavigationParams
+                        .Builder(AUTOFILL_ASSISTANT_INTENT_URL, /*isIncognito=*/false)
+                        .build();
+
+        Assert.assertFalse(delegate.handleWithAutofillAssistant(params));
+    }
 }

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

@@ -62,8 +62,6 @@ import java.util.regex.Pattern;
         sdk_is_less_than = Build.VERSION_CODES.LOLLIPOP)
 @Features.EnableFeatures({ChromeFeatureList.CCT_EXTERNAL_LINK_HANDLING,
         ChromeFeatureList.INTENT_BLOCK_EXTERNAL_FORM_REDIRECT_NO_GESTURE})
-@Features.DisableFeatures({ChromeFeatureList.AUTOFILL_ASSISTANT,
-        ChromeFeatureList.AUTOFILL_ASSISTANT_CHROME_ENTRY})
 public class ExternalNavigationHandlerTest {
     // clang-format on
     @Rule
@@ -1594,11 +1592,8 @@ public class ExternalNavigationHandlerTest {
 
     @Test
     @SmallTest
-    @Features.EnableFeatures({ChromeFeatureList.AUTOFILL_ASSISTANT,
-            ChromeFeatureList.AUTOFILL_ASSISTANT_CHROME_ENTRY})
-    public void
-    testAssistantAutofillIntent_catchNavigationFromGoogleSearch() {
-        mDelegate.setIsSerpReferrer(true);
+    public void testAutofillAssistantIntent_handledByDelegate() {
+        mDelegate.setHandleIntentWithAutofillAssistant(true);
 
         checkUrl(AUTOFILL_ASSISTANT_INTENT_URL)
                 .expecting(OverrideUrlLoadingResult.OVERRIDE_WITH_CLOBBERING_TAB, IGNORE);
@@ -1608,11 +1603,8 @@ public class ExternalNavigationHandlerTest {
 
     @Test
     @SmallTest
-    @Features.EnableFeatures({ChromeFeatureList.AUTOFILL_ASSISTANT,
-            ChromeFeatureList.AUTOFILL_ASSISTANT_CHROME_ENTRY})
-    public void
-    testAssistantAutofillIntent_doNotCatchNavigationInIncognito() {
-        mDelegate.setIsSerpReferrer(true);
+    public void testAutofillAssistantIntent_notHandledByDelegate() {
+        mDelegate.setHandleIntentWithAutofillAssistant(false);
 
         checkUrl(AUTOFILL_ASSISTANT_INTENT_URL)
                 .withIsIncognito(true)
@@ -1623,38 +1615,6 @@ public class ExternalNavigationHandlerTest {
         Assert.assertTrue(mDelegate.startActivityIntent.getScheme().startsWith("https"));
     }
 
-    @Test
-    @SmallTest
-    @Features.EnableFeatures({ChromeFeatureList.AUTOFILL_ASSISTANT,
-            ChromeFeatureList.AUTOFILL_ASSISTANT_CHROME_ENTRY})
-    public void
-    testAssistantAutofillIntent_doNotCatchNavigationFromDifferentOrigin() {
-        mDelegate.setIsSerpReferrer(false);
-
-        checkUrl(AUTOFILL_ASSISTANT_INTENT_URL)
-                .expecting(OverrideUrlLoadingResult.OVERRIDE_WITH_EXTERNAL_INTENT,
-                        START_OTHER_ACTIVITY);
-
-        Assert.assertNotNull(mDelegate.startActivityIntent);
-        Assert.assertTrue(mDelegate.startActivityIntent.getScheme().startsWith("https"));
-    }
-
-    @Test
-    @SmallTest
-    @Features.DisableFeatures({ChromeFeatureList.AUTOFILL_ASSISTANT,
-            ChromeFeatureList.AUTOFILL_ASSISTANT_CHROME_ENTRY})
-    public void
-    testAssistantAutofillIntent_doNotCatchNavigationForNonEnabledFeature() {
-        mDelegate.setIsSerpReferrer(true);
-
-        checkUrl(AUTOFILL_ASSISTANT_INTENT_URL)
-                .expecting(OverrideUrlLoadingResult.OVERRIDE_WITH_EXTERNAL_INTENT,
-                        START_OTHER_ACTIVITY);
-
-        Assert.assertNotNull(mDelegate.startActivityIntent);
-        Assert.assertTrue(mDelegate.startActivityIntent.getScheme().startsWith("https"));
-    }
-
     @Test
     @SmallTest
     public void testIntentActionMetrics() {
@@ -1950,6 +1910,12 @@ public class ExternalNavigationHandlerTest {
             return false;
         }
 
+        @Override
+        public boolean handleWithAutofillAssistant(
+                ExternalNavigationParams params, Intent targetIntent, String browserFallbackUrl) {
+            return mHandleWithAutofillAssistant;
+        }
+
         public void reset() {
             startActivityIntent = null;
             startIncognitoIntentCalled = false;
@@ -1993,6 +1959,10 @@ public class ExternalNavigationHandlerTest {
             mCanHandleWithInstantApp = value;
         }
 
+        public void setHandleIntentWithAutofillAssistant(boolean value) {
+            mHandleWithAutofillAssistant = value;
+        }
+
         public void setIsSerpReferrer(boolean value) {
             mIsSerpReferrer = value;
         }
@@ -2018,6 +1988,7 @@ public class ExternalNavigationHandlerTest {
         private String mNewUrlAfterClobbering;
         private String mReferrerUrlForClobbering;
         private boolean mCanHandleWithInstantApp;
+        private boolean mHandleWithAutofillAssistant;
         private boolean mIsSerpReferrer;
         private String mPreviousUrl;
         public boolean mCalledWithProxy;

chrome/android/javatests/src/org/chromium/chrome/browser/tab/InterceptNavigationDelegateTest.java

@@ -19,6 +19,7 @@ import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.base.test.util.RetryOnFailure;
 import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.ChromeSwitches;
+import org.chromium.chrome.browser.externalnav.ExternalNavigationDelegateImpl;
 import org.chromium.chrome.browser.externalnav.ExternalNavigationHandler;
 import org.chromium.chrome.browser.externalnav.ExternalNavigationParams;
 import org.chromium.chrome.test.ChromeActivityTestRule;
@@ -72,7 +73,7 @@ public class InterceptNavigationDelegateTest {
 
     class TestExternalNavigationHandler extends ExternalNavigationHandler {
         public TestExternalNavigationHandler() {
-            super(mActivity.getActivityTab());
+            super(new ExternalNavigationDelegateImpl(mActivity.getActivityTab()));
         }
 
         @Override