Commit d3688f7c authored by Brandon Wylie's avatar Brandon Wylie Committed by Chromium LUCI CQ

Reland "[ToolbarMVC] Move loadUrl logic to LocationBarMediator"

This is a reland of 068a70b1

Internal change to remove the use of
LocationBarLayout#performSearchQuery:
https://chrome-internal-review.googlesource.com/c/clank/internal/apps/+/3428545

Original change's description:
> [ToolbarMVC] Move loadUrl logic to LocationBarMediator
>
> Bug: 1147581
> Change-Id: Ic19a6f597138d6d9a769ac1fb186a317d8e3f3fe
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2551466
> Commit-Queue: Brandon Wylie <wylieb@chromium.org>
> Reviewed-by: Ted Choc <tedchoc@chromium.org>
> Reviewed-by: Filip Gorski <fgorski@chromium.org>
> Reviewed-by: Patrick Noland <pnoland@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#830389}

Bug: 1147581
Change-Id: I0c523ff90079294db6e38da24ba764386c6abd0e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2558955
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Reviewed-by: default avatarFilip Gorski <fgorski@chromium.org>
Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#832627}
parent 07a5adb2
...@@ -21,6 +21,7 @@ import org.chromium.chrome.browser.WindowDelegate; ...@@ -21,6 +21,7 @@ import org.chromium.chrome.browser.WindowDelegate;
import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher; import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
import org.chromium.chrome.browser.lifecycle.Destroyable; import org.chromium.chrome.browser.lifecycle.Destroyable;
import org.chromium.chrome.browser.lifecycle.NativeInitObserver; import org.chromium.chrome.browser.lifecycle.NativeInitObserver;
import org.chromium.chrome.browser.locale.LocaleManager;
import org.chromium.chrome.browser.ntp.FakeboxDelegate; import org.chromium.chrome.browser.ntp.FakeboxDelegate;
import org.chromium.chrome.browser.omnibox.status.StatusCoordinator; import org.chromium.chrome.browser.omnibox.status.StatusCoordinator;
import org.chromium.chrome.browser.omnibox.status.StatusView; import org.chromium.chrome.browser.omnibox.status.StatusView;
...@@ -116,8 +117,11 @@ public final class LocationBarCoordinator ...@@ -116,8 +117,11 @@ public final class LocationBarCoordinator
View urlBar = mLocationBarLayout.findViewById(R.id.url_bar); View urlBar = mLocationBarLayout.findViewById(R.id.url_bar);
OneshotSupplierImpl<AssistantVoiceSearchService> assistantVoiceSearchSupplier = OneshotSupplierImpl<AssistantVoiceSearchService> assistantVoiceSearchSupplier =
new OneshotSupplierImpl(); new OneshotSupplierImpl();
mLocationBarMediator = new LocationBarMediator( // TODO(crbug.com/1151513): Inject LocaleManager instance to LocationBarCoordinator instead
mLocationBarLayout, locationBarDataProvider, assistantVoiceSearchSupplier); // of using the singleton.
mLocationBarMediator = new LocationBarMediator(mLocationBarLayout, locationBarDataProvider,
assistantVoiceSearchSupplier, overrideUrlLoadingDelegate,
LocaleManager.getInstance());
mUrlCoordinator = new UrlBarCoordinator((UrlBar) urlBar, windowDelegate, actionModeCallback, mUrlCoordinator = new UrlBarCoordinator((UrlBar) urlBar, windowDelegate, actionModeCallback,
mCallbackController.makeCancelable(mLocationBarMediator::onUrlFocusChange), mCallbackController.makeCancelable(mLocationBarMediator::onUrlFocusChange),
mLocationBarMediator); mLocationBarMediator);
...@@ -143,8 +147,7 @@ public final class LocationBarCoordinator ...@@ -143,8 +147,7 @@ public final class LocationBarCoordinator
mLocationBarLayout.addUrlFocusChangeListener(mAutocompleteCoordinator); mLocationBarLayout.addUrlFocusChangeListener(mAutocompleteCoordinator);
mLocationBarLayout.initialize(mAutocompleteCoordinator, mUrlCoordinator, mStatusCoordinator, mLocationBarLayout.initialize(mAutocompleteCoordinator, mUrlCoordinator, mStatusCoordinator,
locationBarDataProvider, profileObservableSupplier, windowDelegate, windowAndroid, locationBarDataProvider, profileObservableSupplier, windowDelegate, windowAndroid,
overrideUrlLoadingDelegate, mLocationBarMediator.getVoiceRecognitionHandler(), mLocationBarMediator.getVoiceRecognitionHandler(), assistantVoiceSearchSupplier);
assistantVoiceSearchSupplier);
} }
@Override @Override
......
...@@ -42,9 +42,7 @@ import org.chromium.base.supplier.OneshotSupplier; ...@@ -42,9 +42,7 @@ import org.chromium.base.supplier.OneshotSupplier;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.WindowDelegate; import org.chromium.chrome.browser.WindowDelegate;
import org.chromium.chrome.browser.flags.ChromeSwitches; import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.locale.LocaleManager;
import org.chromium.chrome.browser.native_page.NativePageFactory; import org.chromium.chrome.browser.native_page.NativePageFactory;
import org.chromium.chrome.browser.ntp.NewTabPageUma;
import org.chromium.chrome.browser.omnibox.UrlBar.ScrollType; import org.chromium.chrome.browser.omnibox.UrlBar.ScrollType;
import org.chromium.chrome.browser.omnibox.UrlBarCoordinator.SelectionState; import org.chromium.chrome.browser.omnibox.UrlBarCoordinator.SelectionState;
import org.chromium.chrome.browser.omnibox.geo.GeolocationHeader; import org.chromium.chrome.browser.omnibox.geo.GeolocationHeader;
...@@ -65,15 +63,11 @@ import org.chromium.components.embedder_support.util.UrlUtilities; ...@@ -65,15 +63,11 @@ import org.chromium.components.embedder_support.util.UrlUtilities;
import org.chromium.components.search_engines.TemplateUrl; import org.chromium.components.search_engines.TemplateUrl;
import org.chromium.components.search_engines.TemplateUrlService; import org.chromium.components.search_engines.TemplateUrlService;
import org.chromium.components.search_engines.TemplateUrlService.TemplateUrlServiceObserver; import org.chromium.components.search_engines.TemplateUrlService.TemplateUrlServiceObserver;
import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.content_public.common.ResourceRequestBody;
import org.chromium.ui.KeyboardVisibilityDelegate; import org.chromium.ui.KeyboardVisibilityDelegate;
import org.chromium.ui.base.PageTransition;
import org.chromium.ui.base.WindowAndroid; import org.chromium.ui.base.WindowAndroid;
import org.chromium.ui.util.ColorUtils; import org.chromium.ui.util.ColorUtils;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.HashMap;
import java.util.List; import java.util.List;
/** /**
...@@ -129,7 +123,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener { ...@@ -129,7 +123,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener {
private Callback<Profile> mProfileSupplierObserver; private Callback<Profile> mProfileSupplierObserver;
private CallbackController mCallbackController = new CallbackController(); private CallbackController mCallbackController = new CallbackController();
private TemplateUrlServiceObserver mTemplateUrlObserver; private TemplateUrlServiceObserver mTemplateUrlObserver;
private OverrideUrlLoadingDelegate mOverrideUrlLoadingDelegate;
/** /**
* Class to handle input from a hardware keyboard when the focus is on the URL bar. In * Class to handle input from a hardware keyboard when the focus is on the URL bar. In
...@@ -278,7 +271,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener { ...@@ -278,7 +271,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener {
@NonNull LocationBarDataProvider locationBarDataProvider, @NonNull LocationBarDataProvider locationBarDataProvider,
@NonNull ObservableSupplier<Profile> profileSupplier, @NonNull ObservableSupplier<Profile> profileSupplier,
@NonNull WindowDelegate windowDelegate, @NonNull WindowAndroid windowAndroid, @NonNull WindowDelegate windowDelegate, @NonNull WindowAndroid windowAndroid,
@NonNull OverrideUrlLoadingDelegate overrideUrlLoadingDelegate,
@NonNull VoiceRecognitionHandler voiceRecognitionHandler, @NonNull VoiceRecognitionHandler voiceRecognitionHandler,
@NonNull OneshotSupplier<AssistantVoiceSearchService> assistantVoiceSearchSupplier) { @NonNull OneshotSupplier<AssistantVoiceSearchService> assistantVoiceSearchSupplier) {
mAutocompleteCoordinator = autocompleteCoordinator; mAutocompleteCoordinator = autocompleteCoordinator;
...@@ -287,7 +279,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener { ...@@ -287,7 +279,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener {
mWindowDelegate = windowDelegate; mWindowDelegate = windowDelegate;
mWindowAndroid = windowAndroid; mWindowAndroid = windowAndroid;
mLocationBarDataProvider = locationBarDataProvider; mLocationBarDataProvider = locationBarDataProvider;
mOverrideUrlLoadingDelegate = overrideUrlLoadingDelegate;
mVoiceRecognitionHandler = voiceRecognitionHandler; mVoiceRecognitionHandler = voiceRecognitionHandler;
mAssistantVoiceSearchServiceSupplier = assistantVoiceSearchSupplier; mAssistantVoiceSearchServiceSupplier = assistantVoiceSearchSupplier;
...@@ -444,18 +435,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener { ...@@ -444,18 +435,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener {
// When we restore tabs, we focus the selected tab so the URL of the page shows. // When we restore tabs, we focus the selected tab so the URL of the page shows.
} }
public void performSearchQuery(String query, List<String> searchParams) {
if (TextUtils.isEmpty(query)) return;
String queryUrl = TemplateUrlServiceFactory.get().getUrlForSearchQuery(query, searchParams);
if (!TextUtils.isEmpty(queryUrl)) {
loadUrl(queryUrl, PageTransition.GENERATED, 0);
} else {
setSearchQuery(query);
}
}
/** /**
* Sets the query string in the omnibox (ensuring the URL bar has focus and triggering * Sets the query string in the omnibox (ensuring the URL bar has focus and triggering
* autocomplete for the specified query) as if the user typed it. * autocomplete for the specified query) as if the user typed it.
...@@ -508,7 +487,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener { ...@@ -508,7 +487,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener {
setUrlBarFocus(false, null, OmniboxFocusReason.UNFOCUS); setUrlBarFocus(false, null, OmniboxFocusReason.UNFOCUS);
// Revert the URL to match the current page. // Revert the URL to match the current page.
setUrl(mLocationBarDataProvider.getCurrentUrl()); setUrl(mLocationBarDataProvider.getCurrentUrl());
focusCurrentTab();
} }
public void gestureDetected(boolean isLongPress) { public void gestureDetected(boolean isLongPress) {
...@@ -713,78 +691,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener { ...@@ -713,78 +691,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener {
updateButtonVisibility(); updateButtonVisibility();
} }
/* package */ void loadUrlFromVoice(String url) {
loadUrl(url, PageTransition.TYPED, 0);
}
/**
* Load the url given with the given transition. Exposed for child classes to overwrite as
* necessary.
*/
/* package */ void loadUrl(String url, @PageTransition int transition, long inputStart) {
loadUrlWithPostData(url, transition, inputStart, null, null);
}
protected void loadUrlWithPostData(String url, @PageTransition int transition, long inputStart,
@Nullable String postDataType, @Nullable byte[] postData) {
Tab currentTab = getCurrentTab();
// The code of the rest of this class ensures that this can't be called until the native
// side is initialized
assert mNativeInitialized : "Loading URL before native side initialized";
// TODO(crbug.com/1085812): Should be taking a full loaded LoadUrlParams.
if (mOverrideUrlLoadingDelegate.willHandleLoadUrlWithPostData(url, transition, postDataType,
postData, mLocationBarDataProvider.isIncognito())) {
return;
}
if (currentTab != null
&& (currentTab.isNativePage()
|| UrlUtilities.isNTPUrl(currentTab.getUrlString()))) {
NewTabPageUma.recordOmniboxNavigation(url, transition);
// Passing in an empty string should not do anything unless the user is at the NTP.
// Since the NTP has no url, pressing enter while clicking on the URL bar should refresh
// the page as it does when you click and press enter on any other site.
if (url.isEmpty()) url = currentTab.getUrlString();
}
// Loads the |url| in a new tab or the current ContentView and gives focus to the
// ContentView.
if (currentTab != null && !url.isEmpty()) {
LoadUrlParams loadUrlParams = new LoadUrlParams(url);
loadUrlParams.setVerbatimHeaders(GeolocationHeader.getGeoHeader(url, currentTab));
loadUrlParams.setTransitionType(transition | PageTransition.FROM_ADDRESS_BAR);
if (inputStart != 0) {
loadUrlParams.setInputStartTimestamp(inputStart);
}
if (!TextUtils.isEmpty(postDataType)) {
StringBuilder headers = new StringBuilder();
String prevHeader = loadUrlParams.getVerbatimHeaders();
if (prevHeader != null && !prevHeader.isEmpty()) {
headers.append(prevHeader);
headers.append("\r\n");
}
loadUrlParams.setExtraHeaders(new HashMap<String, String>() {
{ put("Content-Type", postDataType); }
});
headers.append(loadUrlParams.getExtraHttpRequestHeadersString());
loadUrlParams.setVerbatimHeaders(headers.toString());
}
if (postData != null && postData.length != 0) {
loadUrlParams.setPostData(ResourceRequestBody.createFromBytes(postData));
}
currentTab.loadUrl(loadUrlParams);
RecordUserAction.record("MobileOmniboxUse");
}
LocaleManager.getInstance().recordLocaleBasedSearchMetrics(false, url, transition);
focusCurrentTab();
}
/** /**
* @param focusable Whether the url bar should be focusable. * @param focusable Whether the url bar should be focusable.
*/ */
...@@ -861,14 +767,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener { ...@@ -861,14 +767,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener {
SearchEngineLogoUtils.shouldShowSearchEngineLogo(profile.isOffTheRecord())); SearchEngineLogoUtils.shouldShowSearchEngineLogo(profile.isOffTheRecord()));
} }
/** Focuses the current page. */
private void focusCurrentTab() {
if (mLocationBarDataProvider.hasTab()) {
View view = getCurrentTab().getView();
if (view != null) view.requestFocus();
}
}
/** /**
* @return Whether the URL focus change is taking place, e.g. a focus animation is running on * @return Whether the URL focus change is taking place, e.g. a focus animation is running on
* a phone device. * a phone device.
......
...@@ -5,16 +5,21 @@ ...@@ -5,16 +5,21 @@
package org.chromium.chrome.browser.omnibox; package org.chromium.chrome.browser.omnibox;
import android.content.Context; import android.content.Context;
import android.text.TextUtils;
import android.view.View; import android.view.View;
import androidx.annotation.NonNull; import androidx.annotation.NonNull;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import org.chromium.base.metrics.RecordUserAction;
import org.chromium.base.supplier.OneshotSupplierImpl; import org.chromium.base.supplier.OneshotSupplierImpl;
import org.chromium.chrome.browser.AppHooks; import org.chromium.chrome.browser.AppHooks;
import org.chromium.chrome.browser.gsa.GSAState; import org.chromium.chrome.browser.gsa.GSAState;
import org.chromium.chrome.browser.locale.LocaleManager;
import org.chromium.chrome.browser.ntp.FakeboxDelegate; import org.chromium.chrome.browser.ntp.FakeboxDelegate;
import org.chromium.chrome.browser.ntp.NewTabPageUma;
import org.chromium.chrome.browser.omnibox.UrlBar.UrlBarDelegate; import org.chromium.chrome.browser.omnibox.UrlBar.UrlBarDelegate;
import org.chromium.chrome.browser.omnibox.geo.GeolocationHeader;
import org.chromium.chrome.browser.omnibox.status.StatusCoordinator; import org.chromium.chrome.browser.omnibox.status.StatusCoordinator;
import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteCoordinator; import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteCoordinator;
import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteDelegate; import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteDelegate;
...@@ -22,8 +27,14 @@ import org.chromium.chrome.browser.omnibox.voice.AssistantVoiceSearchService; ...@@ -22,8 +27,14 @@ import org.chromium.chrome.browser.omnibox.voice.AssistantVoiceSearchService;
import org.chromium.chrome.browser.omnibox.voice.VoiceRecognitionHandler; import org.chromium.chrome.browser.omnibox.voice.VoiceRecognitionHandler;
import org.chromium.chrome.browser.preferences.SharedPreferencesManager; import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
import org.chromium.chrome.browser.search_engines.TemplateUrlServiceFactory; import org.chromium.chrome.browser.search_engines.TemplateUrlServiceFactory;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.components.embedder_support.util.UrlUtilities;
import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.content_public.common.ResourceRequestBody;
import org.chromium.ui.base.PageTransition;
import org.chromium.ui.base.WindowAndroid; import org.chromium.ui.base.WindowAndroid;
import java.util.HashMap;
import java.util.List; import java.util.List;
/** /**
...@@ -34,20 +45,30 @@ class LocationBarMediator implements LocationBar, LocationBarDataProvider.Observ ...@@ -34,20 +45,30 @@ class LocationBarMediator implements LocationBar, LocationBarDataProvider.Observ
AutocompleteDelegate, FakeboxDelegate, AutocompleteDelegate, FakeboxDelegate,
VoiceRecognitionHandler.Delegate, VoiceRecognitionHandler.Delegate,
AssistantVoiceSearchService.Observer, UrlBarDelegate { AssistantVoiceSearchService.Observer, UrlBarDelegate {
private final OverrideUrlLoadingDelegate mOverrideUrlLoadingDelegate;
private final LocaleManager mLocaleManager;
private LocationBarLayout mLocationBarLayout; private LocationBarLayout mLocationBarLayout;
private VoiceRecognitionHandler mVoiceRecognitionHandler; private VoiceRecognitionHandler mVoiceRecognitionHandler;
private LocationBarDataProvider mLocationBarDataProvider; private LocationBarDataProvider mLocationBarDataProvider;
private AssistantVoiceSearchService mAssistantVoiceSearchService; private AssistantVoiceSearchService mAssistantVoiceSearchService;
private OneshotSupplierImpl<AssistantVoiceSearchService> mAssistantVoiceSearchSupplier; private OneshotSupplierImpl<AssistantVoiceSearchService> mAssistantVoiceSearchSupplier;
private StatusCoordinator mStatusCoordinator; private StatusCoordinator mStatusCoordinator;
private boolean mNativeInitialized;
/*package */ LocationBarMediator(@NonNull LocationBarLayout locationBarLayout, /*package */ LocationBarMediator(@NonNull LocationBarLayout locationBarLayout,
@NonNull LocationBarDataProvider locationBarDataProvider, @NonNull LocationBarDataProvider locationBarDataProvider,
@NonNull OneshotSupplierImpl<AssistantVoiceSearchService> @NonNull OneshotSupplierImpl<AssistantVoiceSearchService> assistantVoiceSearchSupplier,
assistantVoiceSearchSupplier) { @NonNull OverrideUrlLoadingDelegate overrideUrlLoadingDelegate,
@NonNull LocaleManager localeManager) {
mLocationBarLayout = locationBarLayout; mLocationBarLayout = locationBarLayout;
mOverrideUrlLoadingDelegate = overrideUrlLoadingDelegate;
mLocaleManager = localeManager;
mNativeInitialized = false;
mLocationBarDataProvider = locationBarDataProvider; mLocationBarDataProvider = locationBarDataProvider;
mLocationBarDataProvider.addObserver(this); mLocationBarDataProvider.addObserver(this);
mAssistantVoiceSearchSupplier = assistantVoiceSearchSupplier; mAssistantVoiceSearchSupplier = assistantVoiceSearchSupplier;
mVoiceRecognitionHandler = new VoiceRecognitionHandler(this, mAssistantVoiceSearchSupplier); mVoiceRecognitionHandler = new VoiceRecognitionHandler(this, mAssistantVoiceSearchSupplier);
} }
...@@ -57,6 +78,7 @@ class LocationBarMediator implements LocationBar, LocationBarDataProvider.Observ ...@@ -57,6 +78,7 @@ class LocationBarMediator implements LocationBar, LocationBarDataProvider.Observ
} }
/*package */ void onFinishNativeInitialization() { /*package */ void onFinishNativeInitialization() {
mNativeInitialized = true;
Context context = mLocationBarLayout.getContext(); Context context = mLocationBarLayout.getContext();
mAssistantVoiceSearchService = new AssistantVoiceSearchService(context, mAssistantVoiceSearchService = new AssistantVoiceSearchService(context,
AppHooks.get().getExternalAuthUtils(), TemplateUrlServiceFactory.get(), AppHooks.get().getExternalAuthUtils(), TemplateUrlServiceFactory.get(),
...@@ -89,6 +111,16 @@ class LocationBarMediator implements LocationBar, LocationBarDataProvider.Observ ...@@ -89,6 +111,16 @@ class LocationBarMediator implements LocationBar, LocationBarDataProvider.Observ
mStatusCoordinator = statusCoordinator; mStatusCoordinator = statusCoordinator;
} }
// Private methods
private void focusCurrentTab() {
assert mLocationBarDataProvider != null;
if (mLocationBarDataProvider.hasTab()) {
View view = mLocationBarDataProvider.getTab().getView();
if (view != null) view.requestFocus();
}
}
// LocationBarData.Observer implementation // LocationBarData.Observer implementation
@Override @Override
...@@ -178,7 +210,15 @@ class LocationBarMediator implements LocationBar, LocationBarDataProvider.Observ ...@@ -178,7 +210,15 @@ class LocationBarMediator implements LocationBar, LocationBarDataProvider.Observ
@Override @Override
public void performSearchQuery(String query, List<String> searchParams) { public void performSearchQuery(String query, List<String> searchParams) {
mLocationBarLayout.performSearchQuery(query, searchParams); if (TextUtils.isEmpty(query)) return;
String queryUrl = TemplateUrlServiceFactory.get().getUrlForSearchQuery(query, searchParams);
if (!TextUtils.isEmpty(queryUrl)) {
loadUrl(queryUrl, PageTransition.GENERATED, 0);
} else {
mLocationBarLayout.setSearchQuery(query);
}
} }
@Override @Override
...@@ -232,14 +272,70 @@ class LocationBarMediator implements LocationBar, LocationBarDataProvider.Observ ...@@ -232,14 +272,70 @@ class LocationBarMediator implements LocationBar, LocationBarDataProvider.Observ
} }
@Override @Override
public void loadUrl(String url, int transition, long inputStart) { public void loadUrl(String url, @PageTransition int transition, long inputStart) {
mLocationBarLayout.loadUrl(url, transition, inputStart); loadUrlWithPostData(url, transition, inputStart, null, null);
} }
@Override @Override
public void loadUrlWithPostData( public void loadUrlWithPostData(
String url, int transition, long inputStart, String postDataType, byte[] postData) { String url, int transition, long inputStart, String postDataType, byte[] postData) {
mLocationBarLayout.loadUrlWithPostData(url, transition, inputStart, postDataType, postData); assert mLocationBarDataProvider != null;
Tab currentTab = mLocationBarDataProvider.getTab();
// The code of the rest of this class ensures that this can't be called until the native
// side is initialized
assert mNativeInitialized : "Loading URL before native side initialized";
// TODO(crbug.com/1085812): Should be taking a full loaded LoadUrlParams.
if (mOverrideUrlLoadingDelegate.willHandleLoadUrlWithPostData(url, transition, postDataType,
postData, mLocationBarDataProvider.isIncognito())) {
return;
}
if (currentTab != null
&& (currentTab.isNativePage()
|| UrlUtilities.isNTPUrl(currentTab.getUrlString()))) {
NewTabPageUma.recordOmniboxNavigation(url, transition);
// Passing in an empty string should not do anything unless the user is at the NTP.
// Since the NTP has no url, pressing enter while clicking on the URL bar should refresh
// the page as it does when you click and press enter on any other site.
if (url.isEmpty()) url = currentTab.getUrlString();
}
// Loads the |url| in a new tab or the current ContentView and gives focus to the
// ContentView.
if (currentTab != null && !url.isEmpty()) {
LoadUrlParams loadUrlParams = new LoadUrlParams(url);
loadUrlParams.setVerbatimHeaders(GeolocationHeader.getGeoHeader(url, currentTab));
loadUrlParams.setTransitionType(transition | PageTransition.FROM_ADDRESS_BAR);
if (inputStart != 0) {
loadUrlParams.setInputStartTimestamp(inputStart);
}
if (!TextUtils.isEmpty(postDataType)) {
StringBuilder headers = new StringBuilder();
String prevHeader = loadUrlParams.getVerbatimHeaders();
if (prevHeader != null && !prevHeader.isEmpty()) {
headers.append(prevHeader);
headers.append("\r\n");
}
loadUrlParams.setExtraHeaders(new HashMap<String, String>() {
{ put("Content-Type", postDataType); }
});
headers.append(loadUrlParams.getExtraHttpRequestHeadersString());
loadUrlParams.setVerbatimHeaders(headers.toString());
}
if (postData != null && postData.length != 0) {
loadUrlParams.setPostData(ResourceRequestBody.createFromBytes(postData));
}
currentTab.loadUrl(loadUrlParams);
RecordUserAction.record("MobileOmniboxUse");
}
mLocaleManager.recordLocaleBasedSearchMetrics(false, url, transition);
focusCurrentTab();
} }
@Override @Override
...@@ -278,7 +374,7 @@ class LocationBarMediator implements LocationBar, LocationBarDataProvider.Observ ...@@ -278,7 +374,7 @@ class LocationBarMediator implements LocationBar, LocationBarDataProvider.Observ
@Override @Override
public void loadUrlFromVoice(String url) { public void loadUrlFromVoice(String url) {
mLocationBarLayout.loadUrlFromVoice(url); loadUrl(url, PageTransition.TYPED, 0);
} }
@Override @Override
...@@ -328,6 +424,7 @@ class LocationBarMediator implements LocationBar, LocationBarDataProvider.Observ ...@@ -328,6 +424,7 @@ class LocationBarMediator implements LocationBar, LocationBarDataProvider.Observ
@Override @Override
public void backKeyPressed() { public void backKeyPressed() {
mLocationBarLayout.backKeyPressed(); mLocationBarLayout.backKeyPressed();
focusCurrentTab();
} }
@Override @Override
......
...@@ -122,12 +122,11 @@ class LocationBarTablet extends LocationBarLayout { ...@@ -122,12 +122,11 @@ class LocationBarTablet extends LocationBarLayout {
@NonNull LocationBarDataProvider locationBarDataProvider, @NonNull LocationBarDataProvider locationBarDataProvider,
@NonNull ObservableSupplier<Profile> profileSupplier, @NonNull ObservableSupplier<Profile> profileSupplier,
@NonNull WindowDelegate windowDelegate, @NonNull WindowAndroid windowAndroid, @NonNull WindowDelegate windowDelegate, @NonNull WindowAndroid windowAndroid,
@NonNull OverrideUrlLoadingDelegate overrideUrlLoadingDelegate,
@NonNull VoiceRecognitionHandler voiceRecognitionHandler, @NonNull VoiceRecognitionHandler voiceRecognitionHandler,
@NonNull OneshotSupplier<AssistantVoiceSearchService> assistantVoiceSearchSupplier) { @NonNull OneshotSupplier<AssistantVoiceSearchService> assistantVoiceSearchSupplier) {
super.initialize(autocompleteCoordinator, urlCoordinator, statusCoordinator, super.initialize(autocompleteCoordinator, urlCoordinator, statusCoordinator,
locationBarDataProvider, profileSupplier, windowDelegate, windowAndroid, locationBarDataProvider, profileSupplier, windowDelegate, windowAndroid,
overrideUrlLoadingDelegate, voiceRecognitionHandler, assistantVoiceSearchSupplier); voiceRecognitionHandler, assistantVoiceSearchSupplier);
mStatusCoordinator.setShowIconsWhenUrlFocused(true); mStatusCoordinator.setShowIconsWhenUrlFocused(true);
if (SearchEngineLogoUtils.shouldShowSearchEngineLogo( if (SearchEngineLogoUtils.shouldShowSearchEngineLogo(
mLocationBarDataProvider.isIncognito())) { mLocationBarDataProvider.isIncognito())) {
......
...@@ -107,6 +107,7 @@ public class SearchActivity extends AsyncInitializationActivity ...@@ -107,6 +107,7 @@ public class SearchActivity extends AsyncInitializationActivity
/** Input submitted before before the native library was loaded. */ /** Input submitted before before the native library was loaded. */
private String mQueuedUrl; private String mQueuedUrl;
private @PageTransition int mQueuedTransition;
private String mQueuedPostDataType; private String mQueuedPostDataType;
private byte[] mQueuedPostData; private byte[] mQueuedPostData;
...@@ -176,7 +177,10 @@ public class SearchActivity extends AsyncInitializationActivity ...@@ -176,7 +177,10 @@ public class SearchActivity extends AsyncInitializationActivity
/*shareDelegateSupplier=*/null, /*incognitoStateProvider=*/null, /*shareDelegateSupplier=*/null, /*incognitoStateProvider=*/null,
getLifecycleDispatcher(), /*overrideUrlLoadingDelegate=*/ getLifecycleDispatcher(), /*overrideUrlLoadingDelegate=*/
(String url, @PageTransition int transition, String postDataType, byte[] postData, (String url, @PageTransition int transition, String postDataType, byte[] postData,
boolean incognito) -> false); boolean incognito) -> {
loadUrl(url, transition, postDataType, postData);
return true;
});
// Kick off everything needed for the user to type into the box. // Kick off everything needed for the user to type into the box.
beginQuery(); beginQuery();
...@@ -292,7 +296,9 @@ public class SearchActivity extends AsyncInitializationActivity ...@@ -292,7 +296,9 @@ public class SearchActivity extends AsyncInitializationActivity
assert !mIsActivityUsable assert !mIsActivityUsable
: "finishDeferredInitialization() incorrectly called multiple times"; : "finishDeferredInitialization() incorrectly called multiple times";
mIsActivityUsable = true; mIsActivityUsable = true;
if (mQueuedUrl != null) loadUrl(mQueuedUrl, mQueuedPostDataType, mQueuedPostData); if (mQueuedUrl != null) {
loadUrl(mQueuedUrl, mQueuedTransition, mQueuedPostDataType, mQueuedPostData);
}
// TODO(tedchoc): Warmup triggers the CustomTab layout to be inflated, but this widget // TODO(tedchoc): Warmup triggers the CustomTab layout to be inflated, but this widget
// will navigate to Tabbed mode. Investigate whether this can inflate // will navigate to Tabbed mode. Investigate whether this can inflate
...@@ -345,11 +351,12 @@ public class SearchActivity extends AsyncInitializationActivity ...@@ -345,11 +351,12 @@ public class SearchActivity extends AsyncInitializationActivity
return true; return true;
} }
@Override /* package */ void loadUrl(String url, @PageTransition int transition,
public void loadUrl(String url, @Nullable String postDataType, @Nullable byte[] postData) { @Nullable String postDataType, @Nullable byte[] postData) {
// Wait until native has loaded. // Wait until native has loaded.
if (!mIsActivityUsable) { if (!mIsActivityUsable) {
mQueuedUrl = url; mQueuedUrl = url;
mQueuedTransition = transition;
mQueuedPostDataType = postDataType; mQueuedPostDataType = postDataType;
mQueuedPostData = postData; mQueuedPostData = postData;
return; return;
...@@ -363,6 +370,7 @@ public class SearchActivity extends AsyncInitializationActivity ...@@ -363,6 +370,7 @@ public class SearchActivity extends AsyncInitializationActivity
.makeCustomAnimation(this, android.R.anim.fade_in, android.R.anim.fade_out) .makeCustomAnimation(this, android.R.anim.fade_in, android.R.anim.fade_out)
.toBundle()); .toBundle());
RecordUserAction.record("SearchWidget.SearchMade"); RecordUserAction.record("SearchWidget.SearchMade");
LocaleManager.getInstance().recordLocaleBasedSearchMetrics(true, url, transition);
finish(); finish();
} }
......
...@@ -21,7 +21,6 @@ import org.chromium.chrome.browser.WindowDelegate; ...@@ -21,7 +21,6 @@ import org.chromium.chrome.browser.WindowDelegate;
import org.chromium.chrome.browser.locale.LocaleManager; import org.chromium.chrome.browser.locale.LocaleManager;
import org.chromium.chrome.browser.omnibox.LocationBarDataProvider; import org.chromium.chrome.browser.omnibox.LocationBarDataProvider;
import org.chromium.chrome.browser.omnibox.LocationBarLayout; import org.chromium.chrome.browser.omnibox.LocationBarLayout;
import org.chromium.chrome.browser.omnibox.OverrideUrlLoadingDelegate;
import org.chromium.chrome.browser.omnibox.UrlBar; import org.chromium.chrome.browser.omnibox.UrlBar;
import org.chromium.chrome.browser.omnibox.UrlBarCoordinator; import org.chromium.chrome.browser.omnibox.UrlBarCoordinator;
import org.chromium.chrome.browser.omnibox.UrlBarCoordinator.SelectionState; import org.chromium.chrome.browser.omnibox.UrlBarCoordinator.SelectionState;
...@@ -38,9 +37,6 @@ import org.chromium.ui.base.WindowAndroid; ...@@ -38,9 +37,6 @@ import org.chromium.ui.base.WindowAndroid;
public class SearchActivityLocationBarLayout extends LocationBarLayout { public class SearchActivityLocationBarLayout extends LocationBarLayout {
/** Delegates calls out to the containing Activity. */ /** Delegates calls out to the containing Activity. */
public static interface Delegate { public static interface Delegate {
/** Load a URL in the associated tab. */
void loadUrl(String url, @Nullable String postDataType, @Nullable byte[] postData);
/** The user hit the back button. */ /** The user hit the back button. */
void backKeyPressed(); void backKeyPressed();
} }
...@@ -69,27 +65,18 @@ public class SearchActivityLocationBarLayout extends LocationBarLayout { ...@@ -69,27 +65,18 @@ public class SearchActivityLocationBarLayout extends LocationBarLayout {
@NonNull LocationBarDataProvider locationBarDataProvider, @NonNull LocationBarDataProvider locationBarDataProvider,
@NonNull ObservableSupplier<Profile> profileSupplier, @NonNull ObservableSupplier<Profile> profileSupplier,
@NonNull WindowDelegate windowDelegate, @NonNull WindowAndroid windowAndroid, @NonNull WindowDelegate windowDelegate, @NonNull WindowAndroid windowAndroid,
@NonNull OverrideUrlLoadingDelegate overrideUrlLoadingDelegate,
@NonNull VoiceRecognitionHandler voiceRecognitionHandler, @NonNull VoiceRecognitionHandler voiceRecognitionHandler,
@NonNull OneshotSupplier<AssistantVoiceSearchService> @NonNull OneshotSupplier<AssistantVoiceSearchService>
assistantVoiceSearchServiceSupplier) { assistantVoiceSearchServiceSupplier) {
super.initialize(autocompleteCoordinator, urlCoordinator, statusCoordinator, super.initialize(autocompleteCoordinator, urlCoordinator, statusCoordinator,
locationBarDataProvider, profileSupplier, windowDelegate, windowAndroid, locationBarDataProvider, profileSupplier, windowDelegate, windowAndroid,
overrideUrlLoadingDelegate, voiceRecognitionHandler, voiceRecognitionHandler, assistantVoiceSearchServiceSupplier);
assistantVoiceSearchServiceSupplier);
setUrlBarFocusable(true); setUrlBarFocusable(true);
mPendingSearchPromoDecision = LocaleManager.getInstance().needToCheckForSearchEnginePromo(); mPendingSearchPromoDecision = LocaleManager.getInstance().needToCheckForSearchEnginePromo();
getAutocompleteCoordinator().setShouldPreventOmniboxAutocomplete( getAutocompleteCoordinator().setShouldPreventOmniboxAutocomplete(
mPendingSearchPromoDecision); mPendingSearchPromoDecision);
} }
@Override
protected void loadUrlWithPostData(String url, int transition, long inputStart,
@Nullable String postDataType, @Nullable byte[] postData) {
mDelegate.loadUrl(url, postDataType, postData);
LocaleManager.getInstance().recordLocaleBasedSearchMetrics(true, url, transition);
}
@Override @Override
public void backKeyPressed() { public void backKeyPressed() {
mDelegate.backKeyPressed(); mDelegate.backKeyPressed();
......
...@@ -4,10 +4,13 @@ ...@@ -4,10 +4,13 @@
package org.chromium.chrome.browser.omnibox; package org.chromium.chrome.browser.omnibox;
import static junit.framework.Assert.assertEquals;
import static junit.framework.Assert.assertNull; import static junit.framework.Assert.assertNull;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.notNull; import static org.mockito.ArgumentMatchers.notNull;
import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
import android.content.Context; import android.content.Context;
...@@ -17,24 +20,38 @@ import org.junit.Rule; ...@@ -17,24 +20,38 @@ import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
import org.junit.rules.TestRule; import org.junit.rules.TestRule;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.Captor;
import org.mockito.Mock; import org.mockito.Mock;
import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule; import org.mockito.junit.MockitoRule;
import org.robolectric.annotation.Config; import org.robolectric.annotation.Config;
import org.chromium.base.BuildConfig;
import org.chromium.base.supplier.OneshotSupplierImpl; import org.chromium.base.supplier.OneshotSupplierImpl;
import org.chromium.base.test.BaseRobolectricTestRunner; import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.base.test.util.JniMocker;
import org.chromium.chrome.browser.flags.ChromeFeatureList; import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.locale.LocaleManager;
import org.chromium.chrome.browser.omnibox.voice.AssistantVoiceSearchService; import org.chromium.chrome.browser.omnibox.voice.AssistantVoiceSearchService;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.profiles.ProfileJni;
import org.chromium.chrome.browser.search_engines.TemplateUrlServiceFactory; import org.chromium.chrome.browser.search_engines.TemplateUrlServiceFactory;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.test.util.browser.Features; import org.chromium.chrome.test.util.browser.Features;
import org.chromium.components.search_engines.TemplateUrlService; import org.chromium.components.search_engines.TemplateUrlService;
import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.ui.base.PageTransition;
/** Unit tests for LocationBarMediator. */ /** Unit tests for LocationBarMediator. */
@RunWith(BaseRobolectricTestRunner.class) @RunWith(BaseRobolectricTestRunner.class)
@Config(manifest = Config.NONE) @Config(manifest = Config.NONE)
@Features.EnableFeatures(ChromeFeatureList.OMNIBOX_ASSISTANT_VOICE_SEARCH) @Features.EnableFeatures(ChromeFeatureList.OMNIBOX_ASSISTANT_VOICE_SEARCH)
public class LocationBarMediatorTest { public class LocationBarMediatorTest {
private static final String TEST_URL = "http://testurl.com";
@Rule
public JniMocker mJniMocker = new JniMocker();
@Rule @Rule
public MockitoRule mMockitoRule = MockitoJUnit.rule(); public MockitoRule mMockitoRule = MockitoJUnit.rule();
@Rule @Rule
...@@ -50,6 +67,16 @@ public class LocationBarMediatorTest { ...@@ -50,6 +67,16 @@ public class LocationBarMediatorTest {
private LocationBarDataProvider mLocationBarDataProvider; private LocationBarDataProvider mLocationBarDataProvider;
@Mock @Mock
private OneshotSupplierImpl<AssistantVoiceSearchService> mAssistantVoiceSearchSupplier; private OneshotSupplierImpl<AssistantVoiceSearchService> mAssistantVoiceSearchSupplier;
@Mock
private OverrideUrlLoadingDelegate mOverrideUrlLoadingDelegate;
@Mock
private LocaleManager mLocaleManager;
@Mock
private Profile.Natives mProfileNativesJniMock;
@Mock
private Tab mTab;
@Captor
private ArgumentCaptor<LoadUrlParams> mLoadUrlParamsCaptor;
private LocationBarMediator mMediator; private LocationBarMediator mMediator;
...@@ -57,8 +84,9 @@ public class LocationBarMediatorTest { ...@@ -57,8 +84,9 @@ public class LocationBarMediatorTest {
public void setUp() { public void setUp() {
doReturn(mContext).when(mLocationBarLayout).getContext(); doReturn(mContext).when(mLocationBarLayout).getContext();
TemplateUrlServiceFactory.setInstanceForTesting(mTemplateUrlService); TemplateUrlServiceFactory.setInstanceForTesting(mTemplateUrlService);
mMediator = new LocationBarMediator( mJniMocker.mock(ProfileJni.TEST_HOOKS, mProfileNativesJniMock);
mLocationBarLayout, mLocationBarDataProvider, mAssistantVoiceSearchSupplier); mMediator = new LocationBarMediator(mLocationBarLayout, mLocationBarDataProvider,
mAssistantVoiceSearchSupplier, mOverrideUrlLoadingDelegate, mLocaleManager);
} }
@Test @Test
...@@ -86,4 +114,44 @@ public class LocationBarMediatorTest { ...@@ -86,4 +114,44 @@ public class LocationBarMediatorTest {
mMediator.onNtpStartedLoading(); mMediator.onNtpStartedLoading();
verify(mLocationBarLayout).onNtpStartedLoading(); verify(mLocationBarLayout).onNtpStartedLoading();
} }
@Test
public void testLoadUrl() {
mMediator.onFinishNativeInitialization();
doReturn(mTab).when(mLocationBarDataProvider).getTab();
mMediator.loadUrl(TEST_URL, PageTransition.TYPED, 0);
verify(mTab).loadUrl(mLoadUrlParamsCaptor.capture());
assertEquals(TEST_URL, mLoadUrlParamsCaptor.getValue().getUrl());
assertEquals(PageTransition.TYPED | PageTransition.FROM_ADDRESS_BAR,
mLoadUrlParamsCaptor.getValue().getTransitionType());
}
@Test
public void testLoadUrl_NativeNotInitialized() {
if (BuildConfig.DCHECK_IS_ON) {
// clang-format off
try {
mMediator.loadUrl(TEST_URL, PageTransition.TYPED, 0);
throw new Error("Expected an assert to be triggered.");
} catch (AssertionError e) {}
// clang-format on
}
}
@Test
public void testLoadUrl_OverrideLoadingDelegate() {
mMediator.onFinishNativeInitialization();
doReturn(mTab).when(mLocationBarDataProvider).getTab();
doReturn(true)
.when(mOverrideUrlLoadingDelegate)
.willHandleLoadUrlWithPostData(TEST_URL, PageTransition.TYPED, null, null, false);
mMediator.loadUrl(TEST_URL, PageTransition.TYPED, 0);
verify(mOverrideUrlLoadingDelegate)
.willHandleLoadUrlWithPostData(TEST_URL, PageTransition.TYPED, null, null, false);
verify(mTab, times(0)).loadUrl(any());
}
} }
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