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

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

Index: chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteController.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteController.java b/chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteController.java
index a212b0889d2dbf1750da8c26ffe46ddbb69eee3d..44f9e93c76d20839ac335f18e1e209af86a56fb9 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteController.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteController.java
@@ -33,6 +33,9 @@ public class AutocompleteController {
     private final VoiceSuggestionProvider mVoiceSuggestionProvider = new VoiceSuggestionProvider();
 
 
+    private boolean mUseCachedZeroSuggestResults;
+    private boolean mWaitingForSuggestionsToCache;
+
     /**
      * Listener for receiving OmniboxSuggestions.
      */
@@ -73,6 +76,17 @@ public class AutocompleteController {
     }
 
     /**
+     * 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
@@ -140,6 +154,7 @@ public class AutocompleteController {
         if (profile == null || TextUtils.isEmpty(url)) return;
         mNativeAutocompleteControllerAndroid = nativeInit(profile);
         if (mNativeAutocompleteControllerAndroid != 0) {
+            if (mUseCachedZeroSuggestResults) mWaitingForSuggestionsToCache = true;
             nativeOnOmniboxFocused(mNativeAutocompleteControllerAndroid, omniboxText, url,
                     focusedFromFakebox);
         }
@@ -159,6 +174,7 @@ public class AutocompleteController {
     public void stop(boolean clear) {
         if (clear) mVoiceSuggestionProvider.clearVoiceSearchResults();
         mCurrentNativeAutocompleteResult = 0;
+        mWaitingForSuggestionsToCache = false;
         if (mNativeAutocompleteControllerAndroid != 0) {
             nativeStop(mNativeAutocompleteControllerAndroid, clear);
         }
@@ -210,6 +226,10 @@ public class AutocompleteController {
 
         // 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
@@ -234,6 +254,7 @@ public class AutocompleteController {
     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,
@@ -307,6 +328,7 @@ public class AutocompleteController {
      */
     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);
     }