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

Reland "Reland "[ToolbarMVC] Move loadUrl logic to LocationBarMediator""

This is a reland of d3688f7c

Original change's description:
> 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: Filip Gorski <fgorski@chromium.org>
> Reviewed-by: Ted Choc <tedchoc@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#832627}

Bug: 1147581
Change-Id: Ib4da1e94a9b1f12653a8194d76b3bd8a545a5a62
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2568884
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Reviewed-by: default avatarFilip Gorski <fgorski@chromium.org>
Reviewed-by: default avatarbttk <bttk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#834488}
parent 77e57218
...@@ -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;
...@@ -118,10 +119,12 @@ public final class LocationBarCoordinator ...@@ -118,10 +119,12 @@ public final class LocationBarCoordinator
mUrlBar = mLocationBarLayout.findViewById(R.id.url_bar); mUrlBar = mLocationBarLayout.findViewById(R.id.url_bar);
OneshotSupplierImpl<AssistantVoiceSearchService> assistantVoiceSearchSupplier = OneshotSupplierImpl<AssistantVoiceSearchService> assistantVoiceSearchSupplier =
new OneshotSupplierImpl(); new OneshotSupplierImpl();
// TODO(crbug.com/1151513): Inject LocaleManager instance to LocationBarCoordinator instead
// of using the singleton.
mLocationBarMediator = new LocationBarMediator(mLocationBarLayout, locationBarDataProvider, mLocationBarMediator = new LocationBarMediator(mLocationBarLayout, locationBarDataProvider,
assistantVoiceSearchSupplier, profileObservableSupplier, assistantVoiceSearchSupplier, profileObservableSupplier,
PrivacyPreferencesManagerImpl.getInstance()); PrivacyPreferencesManagerImpl.getInstance(), overrideUrlLoadingDelegate,
LocaleManager.getInstance());
mUrlCoordinator = mUrlCoordinator =
new UrlBarCoordinator((UrlBar) mUrlBar, windowDelegate, actionModeCallback, new UrlBarCoordinator((UrlBar) mUrlBar, windowDelegate, actionModeCallback,
mCallbackController.makeCancelable(mLocationBarMediator::onUrlFocusChange), mCallbackController.makeCancelable(mLocationBarMediator::onUrlFocusChange),
...@@ -149,7 +152,7 @@ public final class LocationBarCoordinator ...@@ -149,7 +152,7 @@ public final class LocationBarCoordinator
mLocationBarLayout.addUrlFocusChangeListener(mAutocompleteCoordinator); mLocationBarLayout.addUrlFocusChangeListener(mAutocompleteCoordinator);
mLocationBarLayout.initialize(mAutocompleteCoordinator, mUrlCoordinator, mStatusCoordinator, mLocationBarLayout.initialize(mAutocompleteCoordinator, mUrlCoordinator, mStatusCoordinator,
locationBarDataProvider, windowDelegate, windowAndroid, overrideUrlLoadingDelegate, locationBarDataProvider, windowDelegate, windowAndroid,
mLocationBarMediator.getVoiceRecognitionHandler(), assistantVoiceSearchSupplier); mLocationBarMediator.getVoiceRecognitionHandler(), assistantVoiceSearchSupplier);
} }
......
...@@ -36,8 +36,6 @@ import org.chromium.base.metrics.RecordUserAction; ...@@ -36,8 +36,6 @@ import org.chromium.base.metrics.RecordUserAction;
import org.chromium.base.supplier.OneshotSupplier; 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.locale.LocaleManager;
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;
...@@ -55,15 +53,11 @@ import org.chromium.components.embedder_support.util.UrlUtilities; ...@@ -55,15 +53,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;
/** /**
...@@ -114,7 +108,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener { ...@@ -114,7 +108,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener {
private Runnable mKeyboardResizeModeTask; private Runnable mKeyboardResizeModeTask;
private Runnable mKeyboardHideTask; private Runnable mKeyboardHideTask;
private TemplateUrlServiceObserver mTemplateUrlObserver; private TemplateUrlServiceObserver mTemplateUrlObserver;
private OverrideUrlLoadingDelegate mOverrideUrlLoadingDelegate;
public LocationBarLayout(Context context, AttributeSet attrs) { public LocationBarLayout(Context context, AttributeSet attrs) {
this(context, attrs, R.layout.location_bar); this(context, attrs, R.layout.location_bar);
...@@ -209,7 +202,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener { ...@@ -209,7 +202,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener {
@NonNull UrlBarCoordinator urlCoordinator, @NonNull StatusCoordinator statusCoordinator, @NonNull UrlBarCoordinator urlCoordinator, @NonNull StatusCoordinator statusCoordinator,
@NonNull LocationBarDataProvider locationBarDataProvider, @NonNull LocationBarDataProvider locationBarDataProvider,
@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;
...@@ -218,7 +210,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener { ...@@ -218,7 +210,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;
...@@ -330,18 +321,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener { ...@@ -330,18 +321,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.
...@@ -394,7 +373,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener { ...@@ -394,7 +373,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) {
...@@ -601,78 +579,6 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener { ...@@ -601,78 +579,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.
*/ */
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
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.KeyEvent; import android.view.KeyEvent;
import android.view.View; import android.view.View;
import android.view.View.OnKeyListener; import android.view.View.OnKeyListener;
...@@ -15,15 +16,19 @@ import androidx.annotation.Nullable; ...@@ -15,15 +16,19 @@ import androidx.annotation.Nullable;
import org.chromium.base.CallbackController; import org.chromium.base.CallbackController;
import org.chromium.base.CommandLine; import org.chromium.base.CommandLine;
import org.chromium.base.metrics.RecordUserAction;
import org.chromium.base.supplier.ObservableSupplier; import org.chromium.base.supplier.ObservableSupplier;
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.flags.ChromeSwitches; import org.chromium.chrome.browser.flags.ChromeSwitches;
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.native_page.NativePageFactory; import org.chromium.chrome.browser.native_page.NativePageFactory;
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.UrlBarCoordinator.SelectionState; import org.chromium.chrome.browser.omnibox.UrlBarCoordinator.SelectionState;
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;
...@@ -33,9 +38,15 @@ import org.chromium.chrome.browser.preferences.SharedPreferencesManager; ...@@ -33,9 +38,15 @@ import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
import org.chromium.chrome.browser.privacy.settings.PrivacyPreferencesManagerImpl; import org.chromium.chrome.browser.privacy.settings.PrivacyPreferencesManagerImpl;
import org.chromium.chrome.browser.profiles.Profile; import org.chromium.chrome.browser.profiles.Profile;
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.browser.util.KeyNavigationUtil; import org.chromium.chrome.browser.util.KeyNavigationUtil;
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;
/** /**
...@@ -58,6 +69,8 @@ class LocationBarMediator implements LocationBarDataProvider.Observer, Autocompl ...@@ -58,6 +69,8 @@ class LocationBarMediator implements LocationBarDataProvider.Observer, Autocompl
private ObservableSupplier<Profile> mProfileSupplier; private ObservableSupplier<Profile> mProfileSupplier;
private PrivacyPreferencesManagerImpl mPrivacyPreferencesManager; private PrivacyPreferencesManagerImpl mPrivacyPreferencesManager;
private CallbackController mCallbackController = new CallbackController(); private CallbackController mCallbackController = new CallbackController();
private final OverrideUrlLoadingDelegate mOverrideUrlLoadingDelegate;
private final LocaleManager mLocaleManager;
private boolean mNativeInitialized; private boolean mNativeInitialized;
...@@ -65,7 +78,9 @@ class LocationBarMediator implements LocationBarDataProvider.Observer, Autocompl ...@@ -65,7 +78,9 @@ class LocationBarMediator implements LocationBarDataProvider.Observer, Autocompl
@NonNull LocationBarDataProvider locationBarDataProvider, @NonNull LocationBarDataProvider locationBarDataProvider,
@NonNull OneshotSupplierImpl<AssistantVoiceSearchService> assistantVoiceSearchSupplier, @NonNull OneshotSupplierImpl<AssistantVoiceSearchService> assistantVoiceSearchSupplier,
@NonNull ObservableSupplier<Profile> profileSupplier, @NonNull ObservableSupplier<Profile> profileSupplier,
@NonNull PrivacyPreferencesManagerImpl privacyPreferencesManager) { @NonNull PrivacyPreferencesManagerImpl privacyPreferencesManager,
@NonNull OverrideUrlLoadingDelegate overrideUrlLoadingDelegate,
@NonNull LocaleManager localeManager) {
mLocationBarLayout = locationBarLayout; mLocationBarLayout = locationBarLayout;
mLocationBarDataProvider = locationBarDataProvider; mLocationBarDataProvider = locationBarDataProvider;
mLocationBarDataProvider.addObserver(this); mLocationBarDataProvider.addObserver(this);
...@@ -74,6 +89,8 @@ class LocationBarMediator implements LocationBarDataProvider.Observer, Autocompl ...@@ -74,6 +89,8 @@ class LocationBarMediator implements LocationBarDataProvider.Observer, Autocompl
mProfileSupplier = profileSupplier; mProfileSupplier = profileSupplier;
mProfileSupplier.addObserver(mCallbackController.makeCancelable(this::setProfile)); mProfileSupplier.addObserver(mCallbackController.makeCancelable(this::setProfile));
mPrivacyPreferencesManager = privacyPreferencesManager; mPrivacyPreferencesManager = privacyPreferencesManager;
mOverrideUrlLoadingDelegate = overrideUrlLoadingDelegate;
mLocaleManager = localeManager;
} }
/** /**
...@@ -143,6 +160,14 @@ class LocationBarMediator implements LocationBarDataProvider.Observer, Autocompl ...@@ -143,6 +160,14 @@ class LocationBarMediator implements LocationBarDataProvider.Observer, Autocompl
SearchEngineLogoUtils.shouldShowSearchEngineLogo(profile.isOffTheRecord())); SearchEngineLogoUtils.shouldShowSearchEngineLogo(profile.isOffTheRecord()));
} }
private void focusCurrentTab() {
assert mLocationBarDataProvider != null;
if (mLocationBarDataProvider.hasTab()) {
View view = mLocationBarDataProvider.getTab().getView();
if (view != null) view.requestFocus();
}
}
/*package */ void updateVisualsForState() { /*package */ void updateVisualsForState() {
mLocationBarLayout.onPrimaryColorChanged(); mLocationBarLayout.onPrimaryColorChanged();
} }
...@@ -216,7 +241,15 @@ class LocationBarMediator implements LocationBarDataProvider.Observer, Autocompl ...@@ -216,7 +241,15 @@ class LocationBarMediator implements LocationBarDataProvider.Observer, Autocompl
@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
...@@ -281,14 +314,70 @@ class LocationBarMediator implements LocationBarDataProvider.Observer, Autocompl ...@@ -281,14 +314,70 @@ class LocationBarMediator implements LocationBarDataProvider.Observer, Autocompl
} }
@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
...@@ -327,7 +416,7 @@ class LocationBarMediator implements LocationBarDataProvider.Observer, Autocompl ...@@ -327,7 +416,7 @@ class LocationBarMediator implements LocationBarDataProvider.Observer, Autocompl
@Override @Override
public void loadUrlFromVoice(String url) { public void loadUrlFromVoice(String url) {
mLocationBarLayout.loadUrlFromVoice(url); loadUrl(url, PageTransition.TYPED, 0);
} }
@Override @Override
...@@ -371,6 +460,7 @@ class LocationBarMediator implements LocationBarDataProvider.Observer, Autocompl ...@@ -371,6 +460,7 @@ class LocationBarMediator implements LocationBarDataProvider.Observer, Autocompl
@Override @Override
public void backKeyPressed() { public void backKeyPressed() {
mLocationBarLayout.backKeyPressed(); mLocationBarLayout.backKeyPressed();
focusCurrentTab();
} }
@Override @Override
......
...@@ -119,12 +119,11 @@ class LocationBarTablet extends LocationBarLayout { ...@@ -119,12 +119,11 @@ class LocationBarTablet extends LocationBarLayout {
@NonNull UrlBarCoordinator urlCoordinator, @NonNull StatusCoordinator statusCoordinator, @NonNull UrlBarCoordinator urlCoordinator, @NonNull StatusCoordinator statusCoordinator,
@NonNull LocationBarDataProvider locationBarDataProvider, @NonNull LocationBarDataProvider locationBarDataProvider,
@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, windowDelegate, windowAndroid, overrideUrlLoadingDelegate, locationBarDataProvider, windowDelegate, windowAndroid, voiceRecognitionHandler,
voiceRecognitionHandler, assistantVoiceSearchSupplier); 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();
} }
......
...@@ -18,7 +18,6 @@ import org.chromium.chrome.browser.WindowDelegate; ...@@ -18,7 +18,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;
...@@ -34,9 +33,6 @@ import org.chromium.ui.base.WindowAndroid; ...@@ -34,9 +33,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();
} }
...@@ -64,26 +60,18 @@ public class SearchActivityLocationBarLayout extends LocationBarLayout { ...@@ -64,26 +60,18 @@ public class SearchActivityLocationBarLayout extends LocationBarLayout {
@NonNull UrlBarCoordinator urlCoordinator, @NonNull StatusCoordinator statusCoordinator, @NonNull UrlBarCoordinator urlCoordinator, @NonNull StatusCoordinator statusCoordinator,
@NonNull LocationBarDataProvider locationBarDataProvider, @NonNull LocationBarDataProvider locationBarDataProvider,
@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, windowDelegate, windowAndroid, overrideUrlLoadingDelegate, locationBarDataProvider, windowDelegate, windowAndroid, 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();
......
...@@ -11,26 +11,31 @@ import static org.mockito.ArgumentMatchers.notNull; ...@@ -11,26 +11,31 @@ import static org.mockito.ArgumentMatchers.notNull;
import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never; import static org.mockito.Mockito.never;
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;
import org.junit.Assert;
import org.junit.Before; import org.junit.Before;
import org.junit.Rule; 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.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.ObservableSupplierImpl; import org.chromium.base.supplier.ObservableSupplierImpl;
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.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.UrlBarCoordinator.SelectionState; import org.chromium.chrome.browser.omnibox.UrlBarCoordinator.SelectionState;
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;
...@@ -43,12 +48,16 @@ import org.chromium.chrome.browser.tab.Tab; ...@@ -43,12 +48,16 @@ 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.embedder_support.util.UrlConstants; import org.chromium.components.embedder_support.util.UrlConstants;
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 @Rule
public MockitoRule mMockitoRule = MockitoJUnit.rule(); public MockitoRule mMockitoRule = MockitoJUnit.rule();
@Rule @Rule
...@@ -82,6 +91,13 @@ public class LocationBarMediatorTest { ...@@ -82,6 +91,13 @@ public class LocationBarMediatorTest {
private OmniboxPrerender.Natives mPrerenderJni; private OmniboxPrerender.Natives mPrerenderJni;
@Mock @Mock
private SearchEngineLogoUtils.Delegate mSearchEngineDelegate; private SearchEngineLogoUtils.Delegate mSearchEngineDelegate;
@Mock
private OverrideUrlLoadingDelegate mOverrideUrlLoadingDelegate;
@Mock
private LocaleManager mLocaleManager;
@Captor
private ArgumentCaptor<LoadUrlParams> mLoadUrlParamsCaptor;
private ObservableSupplierImpl<Profile> mProfileSupplier = new ObservableSupplierImpl<>(); private ObservableSupplierImpl<Profile> mProfileSupplier = new ObservableSupplierImpl<>();
private LocationBarMediator mMediator; private LocationBarMediator mMediator;
...@@ -93,7 +109,8 @@ public class LocationBarMediatorTest { ...@@ -93,7 +109,8 @@ public class LocationBarMediatorTest {
mJniMocker.mock(ProfileJni.TEST_HOOKS, mProfileNativesJniMock); mJniMocker.mock(ProfileJni.TEST_HOOKS, mProfileNativesJniMock);
mJniMocker.mock(OmniboxPrerenderJni.TEST_HOOKS, mPrerenderJni); mJniMocker.mock(OmniboxPrerenderJni.TEST_HOOKS, mPrerenderJni);
mMediator = new LocationBarMediator(mLocationBarLayout, mLocationBarDataProvider, mMediator = new LocationBarMediator(mLocationBarLayout, mLocationBarDataProvider,
mAssistantVoiceSearchSupplier, mProfileSupplier, mPrivacyPreferencesManager); mAssistantVoiceSearchSupplier, mProfileSupplier, mPrivacyPreferencesManager,
mOverrideUrlLoadingDelegate, mLocaleManager);
mMediator.setCoordinators(mUrlCoordinator, mAutocompleteCoordinator, mStatusCoordintor); mMediator.setCoordinators(mUrlCoordinator, mAutocompleteCoordinator, mStatusCoordintor);
SearchEngineLogoUtils.setDelegateForTesting(mSearchEngineDelegate); SearchEngineLogoUtils.setDelegateForTesting(mSearchEngineDelegate);
} }
...@@ -180,4 +197,44 @@ public class LocationBarMediatorTest { ...@@ -180,4 +197,44 @@ public class LocationBarMediatorTest {
456L, profile, mTab); 456L, profile, mTab);
verify(mUrlCoordinator).setAutocompleteText("text", "textWithAutocomplete"); verify(mUrlCoordinator).setAutocompleteText("text", "textWithAutocomplete");
} }
@Test
public void testLoadUrl() {
mMediator.onFinishNativeInitialization();
doReturn(mTab).when(mLocationBarDataProvider).getTab();
mMediator.loadUrl(TEST_URL, PageTransition.TYPED, 0);
verify(mTab).loadUrl(mLoadUrlParamsCaptor.capture());
Assert.assertEquals(TEST_URL, mLoadUrlParamsCaptor.getValue().getUrl());
Assert.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