Commit 923fe126 authored by Colin Blundell's avatar Colin Blundell Committed by Commit Bot

[Android] Remove knowledge of Tab from ExternalNavigationParams

The only client of ExternalNavigationParams.getTab() is now
ExternalNavigationDelegateImpl, and the only client that sets the Tab of
the params is InterceptNavigationDelegateImpl. The latter sets the Tab
of the params to its own ivar, which is the same object that it has
passed to the ExternalNavigationDelegateImpl instance that it constructs
via TabDelegateFactory.createExternalNavigationHandler(). Thus,
ExternalNavigationDelegateImpl can simply use its own |mTab| ivar in
place of ExternalNavigationParams.getTab(), as they refer to the same
object. This CL also makes a minor change to the structure of
ExternalNavigationDelegateImplTest.java to adjust for the production
change.

This change aids in the componentiation of ExternalNavigationHandler for
sharing with WebLayer.

Bug: 1031465
Change-Id: I0b79258555c63171ab61459187ad6303822a467e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2087671
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: default avatarMichael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747155}
parent 7721fd2b
...@@ -630,6 +630,16 @@ public class ExternalNavigationDelegateImpl implements ExternalNavigationDelegat ...@@ -630,6 +630,16 @@ public class ExternalNavigationDelegateImpl implements ExternalNavigationDelegat
getAvailableContext().startActivity(proxyIntent); getAvailableContext().startActivity(proxyIntent);
} }
/**
* Starts the autofill assistant with the given intent. Exists to allow tests to stub out this
* functionality.
*/
protected void startAutofillAssistantWithIntent(
Intent targetIntent, String browserFallbackUrl) {
AutofillAssistantFacade.start(
((TabImpl) mTab).getActivity(), targetIntent.getExtras(), browserFallbackUrl);
}
/** /**
* @return Whether or not we have a valid {@link Tab} available. * @return Whether or not we have a valid {@link Tab} available.
*/ */
...@@ -654,9 +664,8 @@ public class ExternalNavigationDelegateImpl implements ExternalNavigationDelegat ...@@ -654,9 +664,8 @@ public class ExternalNavigationDelegateImpl implements ExternalNavigationDelegat
&& AutofillAssistantFacade.isAutofillAssistantByIntentTriggeringEnabled( && AutofillAssistantFacade.isAutofillAssistantByIntentTriggeringEnabled(
targetIntent) targetIntent)
&& isSerpReferrer()) { && isSerpReferrer()) {
if (params.getTab() != null) { if (mTab != null) {
AutofillAssistantFacade.start(((TabImpl) params.getTab()).getActivity(), startAutofillAssistantWithIntent(targetIntent, browserFallbackUrl);
targetIntent.getExtras(), browserFallbackUrl);
} }
return true; return true;
} }
......
...@@ -4,7 +4,6 @@ ...@@ -4,7 +4,6 @@
package org.chromium.chrome.browser.externalnav; package org.chromium.chrome.browser.externalnav;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabRedirectHandler; import org.chromium.chrome.browser.tab.TabRedirectHandler;
/** /**
...@@ -32,8 +31,6 @@ public class ExternalNavigationParams { ...@@ -32,8 +31,6 @@ public class ExternalNavigationParams {
/** A redirect handler. */ /** A redirect handler. */
private final TabRedirectHandler mRedirectHandler; private final TabRedirectHandler mRedirectHandler;
private final Tab mTab;
/** Whether the intent should force a new tab to open. */ /** Whether the intent should force a new tab to open. */
private final boolean mOpenInNewTab; private final boolean mOpenInNewTab;
...@@ -60,7 +57,7 @@ public class ExternalNavigationParams { ...@@ -60,7 +57,7 @@ public class ExternalNavigationParams {
private ExternalNavigationParams(String url, boolean isIncognito, String referrerUrl, private ExternalNavigationParams(String url, boolean isIncognito, String referrerUrl,
int pageTransition, boolean isRedirect, boolean appMustBeInForeground, int pageTransition, boolean isRedirect, boolean appMustBeInForeground,
TabRedirectHandler redirectHandler, Tab tab, boolean openInNewTab, TabRedirectHandler redirectHandler, boolean openInNewTab,
boolean isBackgroundTabNavigation, boolean isMainFrame, String nativeClientPackageName, boolean isBackgroundTabNavigation, boolean isMainFrame, String nativeClientPackageName,
boolean hasUserGesture, boolean hasUserGesture,
boolean shouldCloseContentsOnOverrideUrlLoadingAndLaunchIntent) { boolean shouldCloseContentsOnOverrideUrlLoadingAndLaunchIntent) {
...@@ -71,7 +68,6 @@ public class ExternalNavigationParams { ...@@ -71,7 +68,6 @@ public class ExternalNavigationParams {
mIsRedirect = isRedirect; mIsRedirect = isRedirect;
mApplicationMustBeInForeground = appMustBeInForeground; mApplicationMustBeInForeground = appMustBeInForeground;
mRedirectHandler = redirectHandler; mRedirectHandler = redirectHandler;
mTab = tab;
mOpenInNewTab = openInNewTab; mOpenInNewTab = openInNewTab;
mIsBackgroundTabNavigation = isBackgroundTabNavigation; mIsBackgroundTabNavigation = isBackgroundTabNavigation;
mIsMainFrame = isMainFrame; mIsMainFrame = isMainFrame;
...@@ -116,11 +112,6 @@ public class ExternalNavigationParams { ...@@ -116,11 +112,6 @@ public class ExternalNavigationParams {
return mRedirectHandler; return mRedirectHandler;
} }
/** @return The current tab. */
public Tab getTab() {
return mTab;
}
/** /**
* @return Whether the external navigation should be opened in a new tab if handled by Chrome * @return Whether the external navigation should be opened in a new tab if handled by Chrome
* through the intent picker. * through the intent picker.
...@@ -183,8 +174,6 @@ public class ExternalNavigationParams { ...@@ -183,8 +174,6 @@ public class ExternalNavigationParams {
/** A redirect handler. */ /** A redirect handler. */
private TabRedirectHandler mRedirectHandler; private TabRedirectHandler mRedirectHandler;
private Tab mTab;
/** Whether the intent should force a new tab to open. */ /** Whether the intent should force a new tab to open. */
private boolean mOpenInNewTab; private boolean mOpenInNewTab;
...@@ -235,12 +224,6 @@ public class ExternalNavigationParams { ...@@ -235,12 +224,6 @@ public class ExternalNavigationParams {
return this; return this;
} }
/** Sets the current tab. */
public Builder setTab(Tab tab) {
mTab = tab;
return this;
}
/** Sets whether we want to open the intent URL in new tab, if handled by Chrome. */ /** Sets whether we want to open the intent URL in new tab, if handled by Chrome. */
public Builder setOpenInNewTab(boolean v) { public Builder setOpenInNewTab(boolean v) {
mOpenInNewTab = v; mOpenInNewTab = v;
...@@ -282,10 +265,9 @@ public class ExternalNavigationParams { ...@@ -282,10 +265,9 @@ public class ExternalNavigationParams {
/** @return A fully constructed {@link ExternalNavigationParams} object. */ /** @return A fully constructed {@link ExternalNavigationParams} object. */
public ExternalNavigationParams build() { public ExternalNavigationParams build() {
return new ExternalNavigationParams(mUrl, mIsIncognito, mReferrerUrl, mPageTransition, return new ExternalNavigationParams(mUrl, mIsIncognito, mReferrerUrl, mPageTransition,
mIsRedirect, mApplicationMustBeInForeground, mRedirectHandler, mTab, mIsRedirect, mApplicationMustBeInForeground, mRedirectHandler, mOpenInNewTab,
mOpenInNewTab, mIsBackgroundTabNavigation, mIsMainFrame, mIsBackgroundTabNavigation, mIsMainFrame, mNativeClientPackageName,
mNativeClientPackageName, mHasUserGesture, mHasUserGesture, mShouldCloseContentsOnOverrideUrlLoadingAndLaunchIntent);
mShouldCloseContentsOnOverrideUrlLoadingAndLaunchIntent);
} }
} }
} }
...@@ -126,7 +126,6 @@ public class InterceptNavigationDelegateImpl implements InterceptNavigationDeleg ...@@ -126,7 +126,6 @@ public class InterceptNavigationDelegateImpl implements InterceptNavigationDeleg
} }
ExternalNavigationParams params = new ExternalNavigationParams.Builder(url, incognito) ExternalNavigationParams params = new ExternalNavigationParams.Builder(url, incognito)
.setTab(mTab)
.setOpenInNewTab(true) .setOpenInNewTab(true)
.build(); .build();
mLastOverrideUrlLoadingResult = mExternalNavHandler.shouldOverrideUrlLoading(params); mLastOverrideUrlLoadingResult = mExternalNavHandler.shouldOverrideUrlLoading(params);
...@@ -221,7 +220,6 @@ public class InterceptNavigationDelegateImpl implements InterceptNavigationDeleg ...@@ -221,7 +220,6 @@ public class InterceptNavigationDelegateImpl implements InterceptNavigationDeleg
return new ExternalNavigationParams return new ExternalNavigationParams
.Builder(navigationParams.url, mTab.isIncognito(), navigationParams.referrer, .Builder(navigationParams.url, mTab.isIncognito(), navigationParams.referrer,
navigationParams.pageTransitionType, navigationParams.isRedirect) navigationParams.pageTransitionType, navigationParams.isRedirect)
.setTab(mTab)
.setApplicationMustBeInForeground(true) .setApplicationMustBeInForeground(true)
.setRedirectHandler(tabRedirectHandler) .setRedirectHandler(tabRedirectHandler)
.setOpenInNewTab(shouldCloseTab) .setOpenInNewTab(shouldCloseTab)
......
...@@ -60,6 +60,16 @@ import java.util.List; ...@@ -60,6 +60,16 @@ import java.util.List;
mIsSerpReferrer = value; mIsSerpReferrer = value;
} }
@Override
protected void startAutofillAssistantWithIntent(
Intent targetIntent, String browserFallbackUrl) {
mWasAutofillAssistantStarted = true;
}
public boolean wasAutofillAssistantStarted() {
return mWasAutofillAssistantStarted;
}
// Convenience for testing that reduces boilerplate in constructing arguments to the // Convenience for testing that reduces boilerplate in constructing arguments to the
// production method that are common across tests. // production method that are common across tests.
public boolean handleWithAutofillAssistant(ExternalNavigationParams params) { public boolean handleWithAutofillAssistant(ExternalNavigationParams params) {
...@@ -77,6 +87,7 @@ import java.util.List; ...@@ -77,6 +87,7 @@ import java.util.List;
} }
private boolean mIsSerpReferrer; private boolean mIsSerpReferrer;
private boolean mWasAutofillAssistantStarted;
} }
@Rule @Rule
...@@ -251,15 +262,13 @@ import java.util.List; ...@@ -251,15 +262,13 @@ import java.util.List;
new ExternalNavigationDelegateImplForTesting(); new ExternalNavigationDelegateImplForTesting();
delegate.setIsSerpReferrer(true); 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 = ExternalNavigationParams params =
new ExternalNavigationParams new ExternalNavigationParams
.Builder(AUTOFILL_ASSISTANT_INTENT_URL, /*isIncognito=*/false) .Builder(AUTOFILL_ASSISTANT_INTENT_URL, /*isIncognito=*/false)
.build(); .build();
Assert.assertTrue(delegate.handleWithAutofillAssistant(params)); Assert.assertTrue(delegate.handleWithAutofillAssistant(params));
Assert.assertTrue(delegate.wasAutofillAssistantStarted());
} }
@Test @Test
...@@ -278,6 +287,7 @@ import java.util.List; ...@@ -278,6 +287,7 @@ import java.util.List;
.build(); .build();
Assert.assertFalse(delegate.handleWithAutofillAssistant(params)); Assert.assertFalse(delegate.handleWithAutofillAssistant(params));
Assert.assertFalse(delegate.wasAutofillAssistantStarted());
} }
@Test @Test
...@@ -296,6 +306,7 @@ import java.util.List; ...@@ -296,6 +306,7 @@ import java.util.List;
.build(); .build();
Assert.assertFalse(delegate.handleWithAutofillAssistant(params)); Assert.assertFalse(delegate.handleWithAutofillAssistant(params));
Assert.assertFalse(delegate.wasAutofillAssistantStarted());
} }
@Test @Test
...@@ -314,5 +325,6 @@ import java.util.List; ...@@ -314,5 +325,6 @@ import java.util.List;
.build(); .build();
Assert.assertFalse(delegate.handleWithAutofillAssistant(params)); Assert.assertFalse(delegate.handleWithAutofillAssistant(params));
Assert.assertFalse(delegate.wasAutofillAssistantStarted());
} }
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment