Add a way to cache and show zero suggest results before native

chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteController.java

 // Copyright 2014 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;
 
 import android.os.Bundle;
 import android.text.TextUtils;
 
 import org.chromium.base.VisibleForTesting;
@@ skipping 15 matching lines @@
 
     // Maximum number of voice suggestions to show.
     private static final int MAX_VOICE_SUGGESTION_COUNT = 3;
 
     private long mNativeAutocompleteControllerAndroid;
     private long mCurrentNativeAutocompleteResult;
     private final OnSuggestionsReceivedListener mListener;
     private final VoiceSuggestionProvider mVoiceSuggestionProvider = new VoiceSuggestionProvider();
 
 
+    private boolean mUseCachedZeroSuggestResults;
+    private boolean mWaitingForSuggestionsToCache;
+
     /**
      * Listener for receiving OmniboxSuggestions.
      */
     public static interface OnSuggestionsReceivedListener {
         void onSuggestionsReceived(List<OmniboxSuggestion> suggestions,
                 String inlineAutocompleteText);
     }
 
     public AutocompleteController(OnSuggestionsReceivedListener listener) {
         this(null, listener);
@@ skipping 20 matching lines @@
         stop(true);
         if (profile == null) {
             mNativeAutocompleteControllerAndroid = 0;
             return;
         }
 
         mNativeAutocompleteControllerAndroid = nativeInit(profile);
     }
 
     /**
+     * Sets to use any cached zero suggest results if there are any available and start caching them
+     * for all zero suggest updates.
+     */
+    public void setUseCachedZeroSuggestResults() {

[Maria, 2017/03/08 06:48:08]

nit: I don't know why but I have a lot of trouble parsing the name of this
function -- something about the combination of words makes it hard to grasp what
it's doing.

How about startCachedZeroSuggest? That goes together with the rest of method
naming.

[Yusuf, 2017/03/08 20:10:46]

Done.

+        mUseCachedZeroSuggestResults = true;
+        List<OmniboxSuggestion> suggestions =
+                OmniboxSuggestion.getCachedOmniboxSuggestionsForZeroSuggest();
+        if (suggestions != null) mListener.onSuggestionsReceived(suggestions, "");

[Maria, 2017/03/08 06:48:08]

I wonder if we can hit some subtle assumptions here by being synchronous whereas
we are usually async. But if it works, I guess then it's fine.

[Yusuf, 2017/03/08 20:10:46]

Acknowledged.

+    }
+
+    /**
      * Starts querying for omnibox suggestions for a given text.
      *
      * @param profile The profile to use for starting the AutocompleteController
      * @param url The URL of the current tab, used to suggest query refinements.
      * @param text The text to query autocomplete suggestions for.
      * @param preventInlineAutocomplete Whether autocomplete suggestions should be prevented.
      */
     public void start(Profile profile, String url, String text,
             boolean preventInlineAutocomplete) {
         start(profile, url, text, -1, preventInlineAutocomplete);
@@ skipping 47 matching lines @@
      * @param omniboxText The text displayed in the omnibox.
      * @param url The url of the currently loaded web page.
      * @param focusedFromFakebox Whether the user entered the omnibox by tapping the fakebox on the
      *                           native NTP. This should be false on all other pages.
      */
     public void startZeroSuggest(Profile profile, String omniboxText, String url,
             boolean focusedFromFakebox) {
         if (profile == null || TextUtils.isEmpty(url)) return;
         mNativeAutocompleteControllerAndroid = nativeInit(profile);
         if (mNativeAutocompleteControllerAndroid != 0) {
+            if (mUseCachedZeroSuggestResults) mWaitingForSuggestionsToCache = true;
             nativeOnOmniboxFocused(mNativeAutocompleteControllerAndroid, omniboxText, url,
                     focusedFromFakebox);
         }
     }
 
     /**
      * Stops generating autocomplete suggestions for the currently specified text from
      * {@link #start(Profile,String, String, boolean)}.
      *
      * <p>
      * Calling this method with {@code false}, will result in
      * {@link #onSuggestionsReceived(List, String, long)} being called with an empty
      * result set.
      *
      * @param clear Whether to clear the most recent autocomplete results.
      */
     public void stop(boolean clear) {
         if (clear) mVoiceSuggestionProvider.clearVoiceSearchResults();
         mCurrentNativeAutocompleteResult = 0;
+        mWaitingForSuggestionsToCache = false;
         if (mNativeAutocompleteControllerAndroid != 0) {
             nativeStop(mNativeAutocompleteControllerAndroid, clear);
         }
     }
 
     /**
      * Resets session for autocomplete controller. This happens every time we start typing
      * new input into the omnibox.
      */
     public void resetSession() {
@@ skipping 31 matching lines @@
         }
 
         // Run through new providers to get an updated list of suggestions.
         suggestions = mVoiceSuggestionProvider.addVoiceSuggestions(
                 suggestions, MAX_VOICE_SUGGESTION_COUNT);
 
         mCurrentNativeAutocompleteResult = currentNativeAutocompleteResult;
 
         // Notify callbacks of suggestions.
         mListener.onSuggestionsReceived(suggestions, inlineAutocompleteText);
+        if (mWaitingForSuggestionsToCache) {

[Maria, 2017/03/08 06:48:08]

This is a little fragile because it relies on zero suggest returning everything
in the first suggestions response (in general, this method can get called many
times with updated suggestions as they come in over the wire). I think today we
get zero suggest all at once, but with some new providers (that perhaps return
their suggestions async later) in this area you might end up with an incomplete
list here.

[Yusuf, 2017/03/08 20:10:46]

On the omnibox side, we never combine these calls though, right? So should I
maybe have a lower limit for # of suggestions? > 1, > 2?

[Maria, 2017/03/09 17:55:09]

Correct. Each onSuggestionReceived set is newly ranked set including top
suggestions we have up to date.

To give an example, let's say we have zero suggest providers A, B, and C.

A returns suggestions A1 with score 10, A2 with score 9, and A3 with score 2.
B returns suggestions B1 with score 5 and B2 with score 4
C returns suggestions C1 with score 11 and C2 with score 8

Now B and C are async providers that will message their suggestions as they get
them and C is slower than B. We may see something like the following:

onSuggestionsReceived call 1:
A1
A2
A3

onSuggestionReceived call 2:
A1
A2
B1
B2
A3

onSuggestionReceived call 3:
C1
A1
A2
C2
B1


Ideally, we would show the user the results of call 3 immediately. So I think it
may make sense to reset mWaitingForSuggestionsToCache in stop() call instead of
after caching the first set of suggestions and allow
cacheOmniboxSuggestionListForZeroSuggest to keep getting updated.

[Yusuf, 2017/03/09 19:29:38]

Done.

+            OmniboxSuggestion.cacheOmniboxSuggestionListForZeroSuggest(suggestions);
+            mWaitingForSuggestionsToCache = false;
+        }
     }
 
     @CalledByNative
     private void notifyNativeDestroyed() {
         mNativeAutocompleteControllerAndroid = 0;
     }
 
     /**
      * Called whenever a navigation happens from the omnibox to record metrics about the user's
      * interaction with the omnibox.
      *
      * @param selectedIndex The index of the suggestion that was selected.
      * @param type The type of the selected suggestion.
      * @param currentPageUrl The URL of the current page.
      * @param focusedFromFakebox Whether the user entered the omnibox by tapping the fakebox on the
      *                           native NTP. This should be false on all other pages.
      * @param elapsedTimeSinceModified The number of ms that passed between the user first
      *                                 modifying text in the omnibox and selecting a suggestion.
      * @param completedLength The length of the default match's inline autocompletion if any.
      * @param webContents The web contents for the tab where the selected suggestion will be shown.
      */
     public void onSuggestionSelected(int selectedIndex, int type,
             String currentPageUrl, boolean focusedFromFakebox, long elapsedTimeSinceModified,
             int completedLength, WebContents webContents) {
+        assert mNativeAutocompleteControllerAndroid != 0;
         // Don't natively log voice suggestion results as we add them in Java.
         if (type == OmniboxSuggestionType.VOICE_SUGGEST) return;
         nativeOnSuggestionSelected(mNativeAutocompleteControllerAndroid, selectedIndex,
                 currentPageUrl, focusedFromFakebox, elapsedTimeSinceModified,
                 completedLength, webContents);
     }
 
     /**
      * Pass the autocomplete controller a {@link Bundle} representing the results of a voice
      * recognition.
@@ skipping 53 matching lines @@
      *
      * @param selectedIndex The index of the autocomplete entry selected.
      * @param elapsedTimeSinceInputChange The number of ms between the time the user started
      *                                    typing in the omnibox and the time the user has selected
      *                                    a suggestion.
      * @return The url to navigate to for this match with aqs parameter updated, if we are
      *         making a Google search query.
      */
     public String updateMatchDestinationUrlWithQueryFormulationTime(int selectedIndex,
             long elapsedTimeSinceInputChange) {
+        if (mNativeAutocompleteControllerAndroid == 0) return null;

[Maria, 2017/03/08 06:48:08]

I thought this couldn't get called until native is loaded?

[Yusuf, 2017/03/08 20:10:46]

Done. Yeah this seems to be a leftover from initial code.

         return nativeUpdateMatchDestinationURLWithQueryFormulationTime(
                 mNativeAutocompleteControllerAndroid, selectedIndex, elapsedTimeSinceInputChange);
     }
 
     @VisibleForTesting
     protected native long nativeInit(Profile profile);
     private native void nativeStart(long nativeAutocompleteControllerAndroid, String text,
             int cursorPosition, String desiredTld, String currentUrl,
             boolean preventInlineAutocomplete, boolean preferKeyword,
             boolean allowExactKeywordMatch, boolean wantAsynchronousMatches);
@@ skipping 22 matching lines @@
      * @param query The query to be expanded into a fully qualified URL if appropriate.
      * @return The fully qualified URL or null.
      */
     public static native String nativeQualifyPartialURLQuery(String query);
 
     /**
      * Sends a zero suggest request to the server in order to pre-populate the result cache.
      */
     public static native void nativePrefetchZeroSuggestResults();
 }