Isolate Processor registration in AutocompleteMediator.

This change updates the way SuggestionProcessors are built for
AutocompleteMediator to allow the Mediator to be unit-tested.

Bug: 1050813

Change-Id: Idb806c6ad108940add73ea7b16712376f8972561
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2076006
Commit-Queue: Ender <ender@google.com>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Patrick Noland <pnoland@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745723}

chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteCoordinatorImpl.java

@@ -70,6 +70,7 @@ public class AutocompleteCoordinatorImpl implements AutocompleteCoordinator {
         ModelList listItems = new ModelList();
         mMediator =
                 new AutocompleteMediator(context, delegate, urlBarEditingTextProvider, listModel);
+        mMediator.initDefaultProcessors();
 
         listModel.set(SuggestionListProperties.EMBEDDER, listEmbedder);
         listModel.set(SuggestionListProperties.VISIBLE, false);

chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteMediator.java

@@ -70,8 +70,8 @@ import java.util.List;
 /**
  * Handles updating the model state for the currently visible omnibox suggestions.
  */
-class AutocompleteMediator implements OnSuggestionsReceivedListener, SuggestionHost,
-                                      StartStopWithNativeObserver, SuggestionListObserver {
+class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWithNativeObserver,
+                                      SuggestionListObserver {
     /** A struct containing information about the suggestion and its view type. */
     private static class SuggestionViewInfo extends MVCListAdapter.ListItem {
         /** Processor managing the suggestion. */
@@ -104,11 +104,11 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, SuggestionH
     private final PropertyModel mListPropertyModel;
     private final List<Runnable> mDeferredNativeRunnables = new ArrayList<Runnable>();
     private final Handler mHandler;
-    private final BasicSuggestionProcessor mBasicSuggestionProcessor;
+    // TODO(crbug.com/982818): make EditUrlProcessor behave like all other processors and register
+    // it in the mSuggestionProcessors list. The processor currently cannot be combined with
+    // other processors because of its unique requirements.
     private @Nullable EditUrlSuggestionProcessor mEditUrlProcessor;
-    private AnswerSuggestionProcessor mAnswerSuggestionProcessor;
-    private final EntitySuggestionProcessor mEntitySuggestionProcessor;
-    private final TailSuggestionProcessor mTailSuggestionProcessor;
+    private final List<SuggestionProcessor> mSuggestionProcessors;
 
     private ToolbarDataProvider mDataProvider;
     private OverviewModeBehavior mOverviewModeBehavior;
@@ -170,19 +170,7 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, SuggestionH
         mListPropertyModel = listPropertyModel;
         mAutocomplete = new AutocompleteController(this);
         mHandler = new Handler();
-
-        final Supplier<ImageFetcher> imageFetcherSupplier = createImageFetcherSupplier();
-        final Supplier<LargeIconBridge> iconBridgeSupplier = createIconBridgeSupplier();
-
-        mBasicSuggestionProcessor =
-                new BasicSuggestionProcessor(mContext, this, textProvider, iconBridgeSupplier);
-        mAnswerSuggestionProcessor =
-                new AnswerSuggestionProcessor(mContext, this, textProvider, imageFetcherSupplier);
-        mEditUrlProcessor = new EditUrlSuggestionProcessor(mContext, this, delegate,
-                (suggestion) -> onSelection(suggestion, 0), iconBridgeSupplier);
-        mEntitySuggestionProcessor =
-                new EntitySuggestionProcessor(mContext, this, imageFetcherSupplier);
-        mTailSuggestionProcessor = new TailSuggestionProcessor(mContext, this);
+        mSuggestionProcessors = new ArrayList<>();
 
         mOverviewModeObserver = new EmptyOverviewModeObserver() {
             @Override
@@ -194,6 +182,36 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, SuggestionH
         };
     }
 
+    /**
+     * Initialize the Mediator with default set of suggestions processors.
+     */
+    void initDefaultProcessors() {
+        final Supplier<ImageFetcher> imageFetcherSupplier = createImageFetcherSupplier();
+        final Supplier<LargeIconBridge> iconBridgeSupplier = createIconBridgeSupplier();
+
+        SuggestionHost host = this::createSuggestionViewDelegate;
+
+        mEditUrlProcessor =
+                new EditUrlSuggestionProcessor(mContext, host, mDelegate, iconBridgeSupplier);
+        registerSuggestionProcessor(new AnswerSuggestionProcessor(
+                mContext, host, mUrlBarEditingTextProvider, imageFetcherSupplier));
+        registerSuggestionProcessor(
+                new EntitySuggestionProcessor(mContext, host, imageFetcherSupplier));
+        registerSuggestionProcessor(new TailSuggestionProcessor(mContext, host));
+        registerSuggestionProcessor(new BasicSuggestionProcessor(
+                mContext, host, mUrlBarEditingTextProvider, iconBridgeSupplier));
+    }
+
+    /**
+     * Register new processor to process OmniboxSuggestions.
+     * Processors will be tried in the same order as they were added.
+     *
+     * @param processor SuggestionProcessor that handles OmniboxSuggestions.
+     */
+    void registerSuggestionProcessor(SuggestionProcessor processor) {
+        mSuggestionProcessors.add(processor);
+    }
+
     public void destroy() {
         if (mTabObserver != null) {
             mTabObserver.destroy();
@@ -425,10 +443,10 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, SuggestionH
             mHandler.post(deferredRunnable);
         }
         mDeferredNativeRunnables.clear();
-        mAnswerSuggestionProcessor.onNativeInitialized();
-        mBasicSuggestionProcessor.onNativeInitialized();
-        mEntitySuggestionProcessor.onNativeInitialized();
-        mTailSuggestionProcessor.onNativeInitialized();
+
+        for (SuggestionProcessor processor : mSuggestionProcessors) {
+            processor.onNativeInitialized();
+        }
         if (mEditUrlProcessor != null) mEditUrlProcessor.onNativeInitialized();
     }
 
@@ -496,11 +514,11 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, SuggestionH
 
             if (mImageFetcher != null) mImageFetcher.clear();
         }
+
         if (mEditUrlProcessor != null) mEditUrlProcessor.onUrlFocusChange(hasFocus);
-        mAnswerSuggestionProcessor.onUrlFocusChange(hasFocus);
-        mBasicSuggestionProcessor.onUrlFocusChange(hasFocus);
-        mEntitySuggestionProcessor.onUrlFocusChange(hasFocus);
-        mTailSuggestionProcessor.onUrlFocusChange(hasFocus);
+        for (SuggestionProcessor processor : mSuggestionProcessors) {
+            processor.onUrlFocusChange(hasFocus);
+        }
     }
 
     /**
@@ -544,8 +562,6 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, SuggestionH
         return mAutocomplete.getCurrentNativeAutocompleteResult();
     }
 
-    // TODO(mdjones): This should only exist in the BasicSuggestionProcessor.
-    @Override
     public SuggestionViewDelegate createSuggestionViewDelegate(
             OmniboxSuggestion suggestion, int position) {
         return new SuggestionViewDelegate() {
@@ -833,17 +849,16 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, SuggestionH
      */
     private SuggestionProcessor getProcessorForSuggestion(
             OmniboxSuggestion suggestion, boolean isFirst) {
-        if (mAnswerSuggestionProcessor.doesProcessSuggestion(suggestion)) {
-            return mAnswerSuggestionProcessor;
-        } else if (mEntitySuggestionProcessor.doesProcessSuggestion(suggestion)) {
-            return mEntitySuggestionProcessor;
-        } else if (mTailSuggestionProcessor.doesProcessSuggestion(suggestion)) {
-            return mTailSuggestionProcessor;
-        } else if (isFirst && mEditUrlProcessor != null
+        if (isFirst && mEditUrlProcessor != null
                 && mEditUrlProcessor.doesProcessSuggestion(suggestion)) {
             return mEditUrlProcessor;
         }
-        return mBasicSuggestionProcessor;
+
+        for (SuggestionProcessor processor : mSuggestionProcessors) {
+            if (processor.doesProcessSuggestion(suggestion)) return processor;
+        }
+        assert false : "No default handler for suggestions";
+        return null;
     }
 
     @Override
@@ -886,9 +901,13 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, SuggestionH
         }
 
         // Show the suggestion list.
-        mTailSuggestionProcessor.reset();
         List<MVCListAdapter.ListItem> newSuggestionViewInfos =
                 new ArrayList<>(newSuggestions.size());
+
+        for (SuggestionProcessor processor : mSuggestionProcessors) {
+            processor.onSuggestionsReceived();
+        }
+        // Ensure the list is fully replaced before broadcasting any change notifications.
         for (int i = 0; i < newSuggestions.size(); i++) {
             OmniboxSuggestion suggestion = newSuggestions.get(i);
             SuggestionProcessor processor = getProcessorForSuggestion(suggestion, i == 0);

chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/SuggestionProcessor.java

@@ -63,4 +63,9 @@ public interface SuggestionProcessor {
      * Only the processor responsible for managing specific suggestion receives this call.
      */
     void recordSuggestionUsed(OmniboxSuggestion suggestion, PropertyModel model);
+
+    /**
+     * Signals that the new suggestion list has been received.
+     */
+    void onSuggestionsReceived();
 }

chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/base/BaseSuggestionViewProcessor.java

@@ -39,6 +39,9 @@ public abstract class BaseSuggestionViewProcessor implements SuggestionProcessor
     @Override
     public void onNativeInitialized() {}
 
+    @Override
+    public void onSuggestionsReceived() {}
+
     /**
      * @param host A handle to the object using the suggestions.
      */

chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/editurl/EditUrlSuggestionProcessor.java

@@ -25,6 +25,7 @@ import org.chromium.chrome.browser.omnibox.suggestions.OmniboxSuggestion;
 import org.chromium.chrome.browser.omnibox.suggestions.OmniboxSuggestionUiType;
 import org.chromium.chrome.browser.omnibox.suggestions.SuggestionProcessor;
 import org.chromium.chrome.browser.omnibox.suggestions.basic.SuggestionHost;
+import org.chromium.chrome.browser.omnibox.suggestions.basic.SuggestionViewDelegate;
 import org.chromium.chrome.browser.search_engines.TemplateUrlServiceFactory;
 import org.chromium.chrome.browser.tab.SadTab;
 import org.chromium.chrome.browser.tab.Tab;
@@ -41,15 +42,6 @@ import java.lang.annotation.RetentionPolicy;
  * the rest of Chrome.
  */
 public class EditUrlSuggestionProcessor implements OnClickListener, SuggestionProcessor {
-    /** An interface for handling taps on the suggestion rather than its buttons. */
-    public interface SuggestionSelectionHandler {
-        /**
-         * Handle the edit URL suggestion selection.
-         * @param suggestion The selected suggestion.
-         */
-        void onEditUrlSuggestionSelected(OmniboxSuggestion suggestion);
-    }
-
     /** An interface for modifying the location bar and it's contents. */
     public interface LocationBarDelegate {
         /** Remove focus from the omnibox. */
@@ -95,9 +87,6 @@ public class EditUrlSuggestionProcessor implements OnClickListener, SuggestionPr
     /** The last omnibox suggestion to be processed. */
     private OmniboxSuggestion mLastProcessedSuggestion;
 
-    /** A handler for suggestion selection. */
-    private SuggestionSelectionHandler mSelectionHandler;
-
     /** The original title of the page. */
     private String mOriginalTitle;
 
@@ -115,13 +104,10 @@ public class EditUrlSuggestionProcessor implements OnClickListener, SuggestionPr
 
     /**
      * @param locationBarDelegate A means of modifying the location bar.
-     * @param selectionHandler A mechanism for handling selection of the edit URL suggestion item.
      */
     public EditUrlSuggestionProcessor(Context context, SuggestionHost suggestionHost,
-            LocationBarDelegate locationBarDelegate, SuggestionSelectionHandler selectionHandler,
-            Supplier<LargeIconBridge> iconBridgeSupplier) {
+            LocationBarDelegate locationBarDelegate, Supplier<LargeIconBridge> iconBridgeSupplier) {
         mLocationBarDelegate = locationBarDelegate;
-        mSelectionHandler = selectionHandler;
         mDesiredFaviconWidthPx = context.getResources().getDimensionPixelSize(
                 R.dimen.omnibox_suggestion_favicon_size);
         mSuggestionHost = suggestionHost;
@@ -175,7 +161,11 @@ public class EditUrlSuggestionProcessor implements OnClickListener, SuggestionPr
 
     @Override
     public void populateModel(OmniboxSuggestion suggestion, PropertyModel model, int position) {
-        model.set(EditUrlSuggestionProperties.TEXT_CLICK_LISTENER, this);
+        SuggestionViewDelegate delegate =
+                mSuggestionHost.createSuggestionViewDelegate(suggestion, position);
+
+        model.set(EditUrlSuggestionProperties.TEXT_CLICK_LISTENER,
+                v -> onSuggestionSelected(delegate));
         model.set(EditUrlSuggestionProperties.BUTTON_CLICK_LISTENER, this);
         if (mOriginalTitle == null) mOriginalTitle = mTabProvider.get().getTitle();
         model.set(EditUrlSuggestionProperties.TITLE_TEXT, mOriginalTitle);
@@ -203,6 +193,9 @@ public class EditUrlSuggestionProcessor implements OnClickListener, SuggestionPr
     @Override
     public void recordSuggestionUsed(OmniboxSuggestion suggestion, PropertyModel model) {}
 
+    @Override
+    public void onSuggestionsReceived() {}
+
     /**
      * @param provider A means of accessing the activity's tab.
      */
@@ -215,13 +208,11 @@ public class EditUrlSuggestionProcessor implements OnClickListener, SuggestionPr
      */
     public void destroy() {
         mLastProcessedSuggestion = null;
-        mSelectionHandler = null;
     }
 
     @Override
     public void onUrlFocusChange(boolean hasFocus) {
         if (hasFocus) return;
-
         mOriginalTitle = null;
         mHasClearedOmniboxForFocus = false;
         mLastProcessedSuggestion = null;
@@ -252,17 +243,27 @@ public class EditUrlSuggestionProcessor implements OnClickListener, SuggestionPr
             recordSuggestionAction(SuggestionAction.EDIT);
             ACTION_EDIT_URL_SUGGESTION_EDIT.record();
             mLocationBarDelegate.setOmniboxEditingText(mLastProcessedSuggestion.getUrl());
-        } else {
-            recordSuggestionAction(SuggestionAction.TAP);
-            ACTION_EDIT_URL_SUGGESTION_TAP.record();
-            // If the event wasn't on any of the buttons, treat is as a tap on the general
-            // suggestion.
-            if (mSelectionHandler != null) {
-                mSelectionHandler.onEditUrlSuggestionSelected(mLastProcessedSuggestion);
-            }
         }
     }
 
+    /**
+     * Responds to suggestion selected events.
+     * Records metrics and propagates the call to SuggestionViewDelegate.
+     *
+     * @param delegate Delegate responding to suggestion events.
+     *
+     * TODO(crbug.com/982818): drop the metric recording and reduce this call. These metrics are
+     * already marked as obsolete.
+     */
+    private void onSuggestionSelected(SuggestionViewDelegate delegate) {
+        recordSuggestionAction(SuggestionAction.TAP);
+        ACTION_EDIT_URL_SUGGESTION_TAP.record();
+        // If the event wasn't on any of the buttons, treat is as a tap on the general
+        // suggestion.
+        assert delegate != null : "EditURL suggestion delegate not available";
+        delegate.onSelection();
+    }
+
     private void recordSuggestionAction(@SuggestionAction int action) {
         RecordHistogram.recordEnumeratedHistogram(
                 "Omnibox.EditUrlSuggestionAction", action, SuggestionAction.NUM_ENTRIES);

chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/tail/TailSuggestionProcessor.java

@@ -69,7 +69,8 @@ public class TailSuggestionProcessor extends BaseSuggestionViewProcessor {
                         .build());
     }
 
-    public void reset() {
+    @Override
+    public void onSuggestionsReceived() {
         mAlignmentManager = new AlignmentManager();
     }
 }

chrome/android/junit/src/org/chromium/chrome/browser/omnibox/suggestions/editurl/EditUrlSuggestionTest.java

@@ -4,6 +4,8 @@
 
 package org.chromium.chrome.browser.omnibox.suggestions.editurl;
 
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.anyInt;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -26,6 +28,8 @@ import org.chromium.chrome.browser.ActivityTabProvider;
 import org.chromium.chrome.browser.favicon.LargeIconBridge;
 import org.chromium.chrome.browser.omnibox.OmniboxSuggestionType;
 import org.chromium.chrome.browser.omnibox.suggestions.OmniboxSuggestion;
+import org.chromium.chrome.browser.omnibox.suggestions.basic.SuggestionHost;
+import org.chromium.chrome.browser.omnibox.suggestions.basic.SuggestionViewDelegate;
 import org.chromium.chrome.browser.search_engines.TemplateUrlServiceFactory;
 import org.chromium.chrome.browser.tab.TabImpl;
 import org.chromium.components.search_engines.TemplateUrlService;
@@ -73,9 +77,6 @@ public final class EditUrlSuggestionTest {
     @Mock
     private EditUrlSuggestionProcessor.LocationBarDelegate mLocationBarDelegate;
 
-    @Mock
-    private EditUrlSuggestionProcessor.SuggestionSelectionHandler mSelectionHandler;
-
     @Mock
     private View mEditButton;
 
@@ -88,6 +89,12 @@ public final class EditUrlSuggestionTest {
     @Mock
     private TemplateUrlService mTemplateUrlService;
 
+    @Mock
+    private SuggestionHost mSuggestionHost;
+
+    @Mock
+    private SuggestionViewDelegate mDelegate;
+
     @Before
     public void setUp() {
         MockitoAnnotations.initMocks(this);
@@ -112,10 +119,12 @@ public final class EditUrlSuggestionTest {
 
         when(mOtherSuggestion.getType()).thenReturn(OmniboxSuggestionType.SEARCH_HISTORY);
 
+        when(mSuggestionHost.createSuggestionViewDelegate(any(), anyInt())).thenReturn(mDelegate);
+
         mModel = new PropertyModel.Builder(EditUrlSuggestionProperties.ALL_KEYS).build();
 
         mProcessor = new EditUrlSuggestionProcessor(
-                mContext, null, mLocationBarDelegate, mSelectionHandler, () -> mIconBridge);
+                mContext, mSuggestionHost, mLocationBarDelegate, () -> mIconBridge);
         mProcessor.setActivityTabProvider(mTabProvider);
 
         when(mEditButton.getId()).thenReturn(R.id.url_edit_icon);
@@ -171,9 +180,9 @@ public final class EditUrlSuggestionTest {
         mProcessor.doesProcessSuggestion(mWhatYouTypedSuggestion);
         mProcessor.populateModel(mWhatYouTypedSuggestion, mModel, 0);
 
-        mModel.get(EditUrlSuggestionProperties.BUTTON_CLICK_LISTENER).onClick(mSuggestionView);
+        mModel.get(EditUrlSuggestionProperties.TEXT_CLICK_LISTENER).onClick(mSuggestionView);
 
-        verify(mSelectionHandler).onEditUrlSuggestionSelected(mWhatYouTypedSuggestion);
+        verify(mDelegate).onSelection();
     }
 
     @Test