Commit 662c6ad2 authored by Tomasz Wiszkowski's avatar Tomasz Wiszkowski

Drop the AutocompleteCoordinator interface (2/2)

This interface was conceived to support modularization efforts of the
Autocomplete system for Chrome.
This effort has been postponed indefinitely in 2019, and the
interface is not very helpful, specifically with ongoing
LocationBar modernization efforts.

We anticipate the interface to be different as a result of the above
efforts and to prevent additional methods from creeping in to the
interface, as well as to make room to correctly address other
dependency issues when the right time comes, i would like to propose
we drop this for now.

Omnibox and Autocomplete modularization will follow the completion of
refactoring efforts and modularization of the LocationBar.

Bug: 966424
Change-Id: Ia568830f53a3872394bfce025ad30bc93372c2f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2508326Reviewed-by: default avatarFilip Gorski <fgorski@chromium.org>
Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823351}
parent 17bcd6db
......@@ -1117,8 +1117,6 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/omnibox/styles/OmniboxTheme.java",
"java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteController.java",
"java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteCoordinator.java",
"java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteCoordinatorFactory.java",
"java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteCoordinatorImpl.java",
"java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteDelegate.java",
"java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteMediator.java",
"java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteResult.java",
......
......@@ -43,7 +43,7 @@ import org.chromium.chrome.browser.externalnav.ExternalNavigationDelegateImpl;
import org.chromium.chrome.browser.externalnav.IntentWithRequestMetadataHandler;
import org.chromium.chrome.browser.externalnav.IntentWithRequestMetadataHandler.RequestMetadata;
import org.chromium.chrome.browser.offlinepages.OfflinePageUtils;
import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteCoordinatorFactory;
import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteCoordinator;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.search_engines.TemplateUrlServiceFactory;
import org.chromium.chrome.browser.tab.Tab;
......@@ -669,7 +669,7 @@ public class IntentHandler {
return null;
}
String query = results.get(0);
String url = AutocompleteCoordinatorFactory.qualifyPartialURLQuery(query);
String url = AutocompleteCoordinator.qualifyPartialURLQuery(query);
if (url == null) {
List<String> urls = IntentUtils.safeGetStringArrayListExtra(
intent, RecognizerResultsIntent.EXTRA_VOICE_SEARCH_RESULT_URLS);
......
......@@ -58,7 +58,6 @@ import org.chromium.chrome.browser.omnibox.geo.GeolocationHeader;
import org.chromium.chrome.browser.omnibox.status.StatusCoordinator;
import org.chromium.chrome.browser.omnibox.status.StatusView;
import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteCoordinator;
import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteCoordinatorFactory;
import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteDelegate;
import org.chromium.chrome.browser.omnibox.suggestions.OmniboxSuggestionsDropdown;
import org.chromium.chrome.browser.omnibox.voice.AssistantVoiceSearchService;
......@@ -231,8 +230,8 @@ public class LocationBarLayout
return mIsTablet ? LocationBarLayout.this : null;
}
};
mAutocompleteCoordinator = AutocompleteCoordinatorFactory.createAutocompleteCoordinator(
this, this, embedder, mUrlCoordinator);
mAutocompleteCoordinator =
new AutocompleteCoordinator(this, this, embedder, mUrlCoordinator);
addUrlFocusChangeListener(mAutocompleteCoordinator);
mUrlCoordinator.addUrlTextChangeListener(mAutocompleteCoordinator);
......
......@@ -28,7 +28,7 @@ import org.chromium.chrome.browser.omnibox.UrlBar.UrlTextChangeListener;
import org.chromium.chrome.browser.omnibox.UrlBarCoordinator.SelectionState;
import org.chromium.chrome.browser.omnibox.UrlBarProperties.AutocompleteText;
import org.chromium.chrome.browser.omnibox.UrlBarProperties.UrlBarTextState;
import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteCoordinatorFactory;
import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteCoordinator;
import org.chromium.components.omnibox.OmniboxUrlEmphasizer.UrlEmphasisSpan;
import org.chromium.content_public.browser.BrowserStartupController;
import org.chromium.ui.base.Clipboard;
......@@ -399,7 +399,7 @@ class UrlBarMediator
private void recordPasteMetrics(String text) {
boolean isUrl = BrowserStartupController.getInstance().isFullBrowserStarted()
&& AutocompleteCoordinatorFactory.qualifyPartialURLQuery(text) != null;
&& AutocompleteCoordinator.qualifyPartialURLQuery(text) != null;
long age = System.currentTimeMillis() - Clipboard.getInstance().getLastModifiedTimeMs();
RecordHistogram.recordCustomTimesHistogram("MobileOmnibox.LongPressPasteAge", age,
......
......@@ -24,7 +24,7 @@ import org.chromium.chrome.browser.omnibox.LocationBarDataProvider;
import org.chromium.chrome.browser.omnibox.SearchEngineLogoUtils;
import org.chromium.chrome.browser.omnibox.UrlBarEditingTextStateProvider;
import org.chromium.chrome.browser.omnibox.status.StatusProperties.StatusIconResource;
import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteCoordinatorFactory;
import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteCoordinator;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.tabmodel.IncognitoStateProvider;
import org.chromium.chrome.browser.toolbar.ToolbarColors;
......@@ -39,12 +39,12 @@ class StatusMediator implements IncognitoStateProvider.IncognitoStateObserver {
@VisibleForTesting
@MockedInTests
class StatusMediatorDelegate {
/** @see {@link AutocompleteCoordinatorFactory#qualifyPartialURLQuery} */
/** @see {@link AutocompleteCoordinator#qualifyPartialURLQuery} */
boolean isUrlValid(String partialUrl) {
if (TextUtils.isEmpty(partialUrl)) return false;
return BrowserStartupController.getInstance().isFullBrowserStarted()
&& AutocompleteCoordinatorFactory.qualifyPartialURLQuery(partialUrl) != null;
&& AutocompleteCoordinator.qualifyPartialURLQuery(partialUrl) != null;
}
/** @see {@link SearchEngineLogoUtils#getSearchEngineLogoFavicon} */
......
......@@ -22,7 +22,9 @@ import org.chromium.chrome.R;
import org.chromium.chrome.browser.ActivityTabProvider;
import org.chromium.chrome.browser.layouts.LayoutStateProvider;
import org.chromium.chrome.browser.omnibox.LocationBarDataProvider;
import org.chromium.chrome.browser.omnibox.UrlBar.UrlTextChangeListener;
import org.chromium.chrome.browser.omnibox.UrlBarEditingTextStateProvider;
import org.chromium.chrome.browser.omnibox.UrlFocusChangeListener;
import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteController.OnSuggestionsReceivedListener;
import org.chromium.chrome.browser.omnibox.suggestions.SuggestionListViewBinder.SuggestionListViewHolder;
import org.chromium.chrome.browser.omnibox.suggestions.answer.AnswerSuggestionViewBinder;
......@@ -56,20 +58,13 @@ import java.util.List;
/**
* Coordinator that handles the interactions with the autocomplete system.
*/
public class AutocompleteCoordinatorImpl implements AutocompleteCoordinator {
public class AutocompleteCoordinator implements UrlFocusChangeListener, UrlTextChangeListener {
private final ViewGroup mParent;
private OmniboxQueryTileCoordinator mQueryTileCoordinator;
private AutocompleteMediator mMediator;
private OmniboxSuggestionsDropdown mDropdown;
/**
* See {@link AutocompleteCoordinatorFactory#createAutocompleteCoordinator}.
*
* Keep this constructor protected so clients use the factory instead.
*/
@VisibleForTesting
protected AutocompleteCoordinatorImpl(ViewGroup parent, AutocompleteDelegate delegate,
public AutocompleteCoordinator(ViewGroup parent, AutocompleteDelegate delegate,
OmniboxSuggestionsDropdown.Embedder dropdownEmbedder,
UrlBarEditingTextStateProvider urlBarEditingTextProvider) {
mParent = parent;
......@@ -99,7 +94,9 @@ public class AutocompleteCoordinatorImpl implements AutocompleteCoordinator {
updateSuggestionListLayoutDirection();
}
@Override
/**
* Clean up resources used by this class.
*/
public void destroy() {
mQueryTileCoordinator.destroy();
mQueryTileCoordinator = null;
......@@ -214,83 +211,133 @@ public class AutocompleteCoordinatorImpl implements AutocompleteCoordinator {
mMediator.onUrlAnimationFinished(hasFocus);
}
@Override
/**
* Provides data and state for the toolbar component.
* @param locationBarDataProvider The data provider.
*/
public void setLocationBarDataProvider(LocationBarDataProvider locationBarDataProvider) {
mMediator.setLocationBarDataProvider(locationBarDataProvider);
}
@Override
/**
* @param layoutStateProvider A means of accessing the current Layout state and a way to
* listen to state changes.
*/
public void setLayoutStateProvider(LayoutStateProvider layoutStateProvider) {
mMediator.setLayoutStateProvider(layoutStateProvider);
}
@Override
/**
* Updates the profile used for generating autocomplete suggestions.
* @param profile The profile to be used.
*/
public void setAutocompleteProfile(Profile profile) {
mMediator.setAutocompleteProfile(profile);
mQueryTileCoordinator.setProfile(profile);
}
@Override
/**
* Set the WindowAndroid instance associated with the containing Activity.
*/
public void setWindowAndroid(WindowAndroid windowAndroid) {
mMediator.setWindowAndroid(windowAndroid);
}
@Override
/**
* @param provider A means of accessing the activity's tab.
*/
public void setActivityTabProvider(ActivityTabProvider provider) {
mMediator.setActivityTabProvider(provider);
}
@Override
/**
* @param shareDelegateSupplier A means of accessing the sharing feature.
*/
public void setShareDelegateSupplier(Supplier<ShareDelegate> shareDelegateSupplier) {
mMediator.setShareDelegateSupplier(shareDelegateSupplier);
}
@Override
/**
* Whether omnibox autocomplete should currently be prevented from generating suggestions.
*/
public void setShouldPreventOmniboxAutocomplete(boolean prevent) {
mMediator.setShouldPreventOmniboxAutocomplete(prevent);
}
@Override
/**
* @return The number of current autocomplete suggestions.
*/
public int getSuggestionCount() {
return mMediator.getSuggestionCount();
}
@Override
/**
* Retrieve the omnibox suggestion at the specified index. The index represents the ordering
* in the underlying model. The index does not represent visibility due to the current scroll
* position of the list.
*
* @param index The index of the suggestion to fetch.
* @return The suggestion at the given index.
*/
public OmniboxSuggestion getSuggestionAt(int index) {
return mMediator.getSuggestionAt(index);
}
@Override
/**
* Signals that native initialization has completed.
*/
public void onNativeInitialized() {
mMediator.onNativeInitialized();
}
@Override
/**
* @see AutocompleteController#onVoiceResults(List)
*/
public void onVoiceResults(@Nullable List<VoiceRecognitionHandler.VoiceResult> results) {
mMediator.onVoiceResults(results);
}
@Override
/**
* @return The current native pointer to the autocomplete results.
* TODO(ender): Figure out how to remove this.
*/
public long getCurrentNativeAutocompleteResult() {
return mMediator.getCurrentNativeAutocompleteResult();
}
@Override
/**
* Update the layout direction of the suggestion list based on the parent layout direction.
*/
public void updateSuggestionListLayoutDirection() {
mMediator.setLayoutDirection(ViewCompat.getLayoutDirection(mParent));
}
@Override
/**
* Update the visuals of the autocomplete UI.
* @param useDarkColors Whether dark colors should be applied to the UI.
* @param isIncognito Whether the UI is for incognito mode or not.
*/
public void updateVisualsForState(boolean useDarkColors, boolean isIncognito) {
mMediator.updateVisualsForState(useDarkColors, isIncognito);
}
@Override
/**
* Sets to show cached zero suggest results. This will start both caching zero suggest results
* in shared preferences and also attempt to show them when appropriate without needing native
* initialization.
* @param showCachedZeroSuggestResults Whether cached zero suggest should be shown.
*/
public void setShowCachedZeroSuggestResults(boolean showCachedZeroSuggestResults) {
mMediator.setShowCachedZeroSuggestResults(showCachedZeroSuggestResults);
}
@Override
/**
* Handle the key events associated with the suggestion list.
*
* @param keyCode The keycode representing what key was interacted with.
* @param event The key event containing all meta-data associated with the event.
* @return Whether the key event was handled.
*/
public boolean handleKeyEvent(int keyCode, KeyEvent event) {
boolean isShowingList = mDropdown != null && mDropdown.getViewGroup().isShown();
......@@ -314,37 +361,56 @@ public class AutocompleteCoordinatorImpl implements AutocompleteCoordinator {
mMediator.onTextChanged(textWithoutAutocomplete, textWithAutocomplete);
}
@Override
/**
* Trigger autocomplete for the given query.
*/
public void startAutocompleteForQuery(String query) {
mMediator.startAutocompleteForQuery(query);
}
@Override
public String qualifyPartialURLQuery(String query) {
/**
* Given a search query, this will attempt to see if the query appears to be portion of a
* properly formed URL. If it appears to be a URL, this will return the fully qualified
* version (i.e. including the scheme, etc...). If the query does not appear to be a URL,
* this will return null.
*
* TODO(crbug.com/966424): Fix the dependency issue and remove this method.
*
* @param query The query to be expanded into a fully qualified URL if appropriate.
* @return The fully qualified URL or null.
*/
@Deprecated
public static String qualifyPartialURLQuery(String query) {
return AutocompleteControllerJni.get().qualifyPartialURLQuery(query);
}
@Override
/**
* Sends a zero suggest request to the server in order to pre-populate the result cache.
*/
public void prefetchZeroSuggestResults() {
AutocompleteControllerJni.get().prefetchZeroSuggestResults();
}
@Override
/** @return Suggestions Dropdown view, showing the list of suggestions. */
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
public OmniboxSuggestionsDropdown getSuggestionsDropdownForTest() {
return mDropdown;
}
@Override
/** @param controller The instance of AutocompleteController to be used. */
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
public void setAutocompleteControllerForTest(AutocompleteController controller) {
mMediator.setAutocompleteControllerForTest(controller);
}
@Override
/** @return The current receiving OnSuggestionsReceived events. */
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
public OnSuggestionsReceivedListener getSuggestionsReceivedListenerForTest() {
return mMediator;
}
@Override
/** @return The ModelList for the currently shown suggestions. */
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
public ModelList getSuggestionModelListForTest() {
return mMediator.getSuggestionModelListForTest();
}
......
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.omnibox.suggestions;
import android.view.ViewGroup;
import org.chromium.chrome.browser.omnibox.UrlBarEditingTextStateProvider;
/**
* Factory to create AutocompleteCoordinator instances.
*/
public class AutocompleteCoordinatorFactory {
private AutocompleteCoordinatorFactory() {}
/**
* Constructs a coordinator for the autocomplete system.
*
* @param parent The UI parent component for the autocomplete UI.
* @param delegate The delegate to fulfill additional autocomplete requirements.
* @param dropdownEmbedder The embedder for controlling the display constraints of the
* suggestions dropdown.
* @param urlBarEditingTextProvider Provider of editing text state from the UrlBar.
*/
public static AutocompleteCoordinator createAutocompleteCoordinator(ViewGroup parent,
AutocompleteDelegate delegate, OmniboxSuggestionsDropdown.Embedder dropdownEmbedder,
UrlBarEditingTextStateProvider urlBarEditingTextProvider) {
return new AutocompleteCoordinatorImpl(
parent, delegate, dropdownEmbedder, urlBarEditingTextProvider);
}
/**
* Temporary shortcut for {@link org.chromium.chrome.browser.IntentHandler} to access
* {@link AutocompleteControllerJni.get().qualifyPartialURLQuery(String)} without having an
* instance of {@link AutocompleteCoordinator}.
*
* TODO(crbug.com/966424): Fix the dependency issue and remove this method.
*
* @param query The query to be expanded into a fully qualified URL if appropriate.
* @return The fully qualified URL or null.
*/
@Deprecated
public static String qualifyPartialURLQuery(String query) {
return AutocompleteControllerJni.get().qualifyPartialURLQuery(query);
}
}
......@@ -269,7 +269,7 @@ public class VoiceRecognitionHandler {
return;
}
String url = autocompleteCoordinator.qualifyPartialURLQuery(topResultQuery);
String url = AutocompleteCoordinator.qualifyPartialURLQuery(topResultQuery);
if (url == null) {
url = TemplateUrlServiceFactory.get()
.getUrlForVoiceSearchQuery(topResultQuery)
......@@ -319,9 +319,6 @@ public class VoiceRecognitionHandler {
// Langues is optional, so only check the size when it's non-null.
if (languages != null && languages.size() != strings.size()) return null;
AutocompleteCoordinator autocompleteCoordinator = mDelegate.getAutocompleteCoordinator();
assert autocompleteCoordinator != null;
List<VoiceResult> results = new ArrayList<>();
for (int i = 0; i < strings.size(); ++i) {
// Remove any spaces in the voice search match when determining whether it
......@@ -331,7 +328,7 @@ public class VoiceRecognitionHandler {
// If the string appears to be a URL, then use it instead of the string returned from
// the voice engine.
String culledString = strings.get(i).replaceAll(" ", "");
String url = autocompleteCoordinator.qualifyPartialURLQuery(culledString);
String url = AutocompleteCoordinator.qualifyPartialURLQuery(culledString);
String language = languages == null ? null : languages.get(i);
results.add(new VoiceResult(
url == null ? strings.get(i) : culledString, confidences[i], language));
......
......@@ -27,7 +27,7 @@ import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.omnibox.LocationBarLayout;
import org.chromium.chrome.browser.omnibox.OmniboxSuggestionType;
import org.chromium.chrome.browser.omnibox.UrlBar;
import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteCoordinatorImpl;
import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteCoordinator;
import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteResult;
import org.chromium.chrome.browser.omnibox.suggestions.OmniboxSuggestion.NavsuggestTile;
import org.chromium.chrome.browser.omnibox.suggestions.OmniboxSuggestionBuilderForTest;
......@@ -85,7 +85,7 @@ public class MostVisitedTilesTest {
private UrlBar mUrlBar;
private LocationBarLayout mLocationBarLayout;
private TestAutocompleteController mController;
private AutocompleteCoordinatorImpl mAutocomplete;
private AutocompleteCoordinator mAutocomplete;
private EmbeddedTestServer mTestServer;
private Tab mTab;
private BaseCarouselSuggestionView mCarousel;
......@@ -100,8 +100,7 @@ public class MostVisitedTilesTest {
mActivity = sActivityTestRule.getActivity();
mLocationBarLayout = mActivity.findViewById(R.id.location_bar);
mUrlBar = mActivity.findViewById(R.id.url_bar);
mAutocomplete =
(AutocompleteCoordinatorImpl) mLocationBarLayout.getAutocompleteCoordinator();
mAutocomplete = mLocationBarLayout.getAutocompleteCoordinator();
mTab = mActivity.getActivityTab();
ChromeTabUtils.waitForInteractable(mTab);
......
......@@ -38,7 +38,6 @@ import org.chromium.chrome.browser.omnibox.UrlBarData;
import org.chromium.chrome.browser.omnibox.UrlBarEditingTextStateProvider;
import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteController.OnSuggestionsReceivedListener;
import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteCoordinator;
import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteCoordinatorImpl;
import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteDelegate;
import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteResult;
import org.chromium.chrome.browser.omnibox.suggestions.OmniboxSuggestionsDropdown;
......@@ -298,10 +297,10 @@ public class VoiceRecognitionHandlerTest {
}
/**
* TODO(crbug.com/962527): Remove this dependency on {@link AutocompleteCoordinatorImpl}.
* TODO(crbug.com/962527): Remove this dependency on {@link AutocompleteCoordinator}.
*/
private class TestAutocompleteCoordinatorImpl extends AutocompleteCoordinatorImpl {
public TestAutocompleteCoordinatorImpl(ViewGroup parent, AutocompleteDelegate delegate,
private class TestAutocompleteCoordinator extends AutocompleteCoordinator {
public TestAutocompleteCoordinator(ViewGroup parent, AutocompleteDelegate delegate,
OmniboxSuggestionsDropdown.Embedder dropdownEmbedder,
UrlBarEditingTextStateProvider urlBarEditingTextProvider) {
super(parent, delegate, dropdownEmbedder, urlBarEditingTextProvider);
......@@ -325,8 +324,7 @@ public class VoiceRecognitionHandlerTest {
ViewGroup parent =
(ViewGroup) mActivityTestRule.getActivity().findViewById(android.R.id.content);
Assert.assertNotNull(parent);
mAutocompleteCoordinator =
new TestAutocompleteCoordinatorImpl(parent, null, null, null);
mAutocompleteCoordinator = new TestAutocompleteCoordinator(parent, null, null, null);
}
@Override
......
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