Omnibox - Use Cursor Position When Generating Suggestions on Android

Both the local suggestion providers and the server can provide better
suggestions with this context.

Remove a now redundant / unnecessary function here.

Bug: 738559
Change-Id: I4780c99a1347b2aa80bc52cb57a1f772709b3ad8
Reviewed-on: https://chromium-review.googlesource.com/665514
Commit-Queue: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504184}

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

@@ -97,22 +97,8 @@ public class AutocompleteController {
      * @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.
-     * @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 start(Profile profile, String url, String text, boolean preventInlineAutocomplete,
-            boolean focusedFromFakebox) {
-        start(profile, url, text, -1, preventInlineAutocomplete, focusedFromFakebox);
-    }
-
-    /**
-     * 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 cursorPosition The position of the cursor within the text.
+     * @param cursorPosition The position of the cursor within the text.  Set to -1 if the cursor is
+     *                     not focussed on the text.
      * @param preventInlineAutocomplete Whether autocomplete suggestions should be prevented.
      * @param focusedFromFakebox Whether the user entered the omnibox by tapping the fakebox on the
      *                           native NTP. This should be false on all other pages.

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

@@ -1171,8 +1171,15 @@ public class LocationBarLayout extends FrameLayout
                     String url = mToolbarDataProvider.hasTab()
                             ? mToolbarDataProvider.getCurrentUrl()
                             : UrlConstants.NTP_URL;
-                    mAutocomplete.start(profile, url, textWithoutAutocomplete, preventAutocomplete,
-                            mUrlFocusedFromFakebox);
+                    int cursorPosition = -1;
+                    if (mUrlBar.getSelectionStart() == mUrlBar.getSelectionEnd()) {
+                        // Conveniently, if there is no selection, those two functions return -1,
+                        // exactly the same value needed to pass to start() to indicate no cursor
+                        // position.  Hence, there's no need to check for -1 here explicitly.
+                        cursorPosition = mUrlBar.getSelectionStart();
+                    }
+                    mAutocomplete.start(profile, url, textWithoutAutocomplete, cursorPosition,
+                            preventAutocomplete, mUrlFocusedFromFakebox);
                 }
             };
             if (mNativeInitialized) {
@@ -1903,10 +1910,10 @@ public class LocationBarLayout extends FrameLayout
         stopAutocomplete(false);
         if (mToolbarDataProvider.hasTab()) {
             mAutocomplete.start(mToolbarDataProvider.getProfile(),
-                    mToolbarDataProvider.getCurrentUrl(), query, false, false);
+                    mToolbarDataProvider.getCurrentUrl(), query, -1, false, false);
         } else if (mBottomSheet != null) {
-            mAutocomplete.start(
-                    mToolbarDataProvider.getProfile(), UrlConstants.NTP_URL, query, false, false);
+            mAutocomplete.start(mToolbarDataProvider.getProfile(), UrlConstants.NTP_URL, query, -1,
+                    false, false);
         }
         post(new Runnable() {
             @Override

chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/UrlBarTest.java

@@ -49,6 +49,7 @@ import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 
 /**
@@ -400,6 +401,94 @@ public class UrlBarTest {
         }
     }
 
+    /**
+     * Ensure that we send cursor position with autocomplete requests.
+     *
+     * When reading this test, it helps to remember that autocomplete requests are not sent
+     * with the user simply moves the cursor.  They're only sent on text modifications.
+     */
+    @Test
+    @SmallTest
+    @Feature({"Omnibox"})
+    @RetryOnFailure
+    public void testSendCursorPosition() throws InterruptedException, TimeoutException {
+        mActivityTestRule.startMainActivityOnBlankPage();
+
+        final CallbackHelper autocompleteHelper = new CallbackHelper();
+        final AtomicInteger cursorPositionUsed = new AtomicInteger();
+        final StubAutocompleteController controller = new StubAutocompleteController() {
+            @Override
+            public void start(Profile profile, String url, String text, int cursorPosition,
+                    boolean preventInlineAutocomplete, boolean focusedFromFakebox) {
+                cursorPositionUsed.set(cursorPosition);
+                autocompleteHelper.notifyCalled();
+            }
+        };
+        setAutocompleteController(controller);
+
+        final UrlBar urlBar = getUrlBar();
+        toggleFocusAndIgnoreImeOperations(urlBar, true);
+
+        // Add "a" to the omnibox and leave the cursor at the end of the new
+        // text.
+        ThreadUtils.runOnUiThreadBlocking(
+                () -> {
+                    urlBar.getInputConnection().commitText("a", 1);
+                });
+        autocompleteHelper.waitForCallback(0);
+        // omnmibox text: a|
+        Assert.assertEquals(1, cursorPositionUsed.get());
+
+        // Append "cd" to the omnibox and leave the cursor at the end of the new
+        // text.
+        ThreadUtils.runOnUiThreadBlocking(
+                () -> {
+                    urlBar.getInputConnection().commitText("cd", 1);
+                });
+        autocompleteHelper.waitForCallback(1);
+        // omnmibox text: acd|
+        Assert.assertEquals(3, cursorPositionUsed.get());
+
+        // Move the cursor.
+        ThreadUtils.runOnUiThreadBlocking(
+                () -> {
+                    urlBar.getInputConnection().setSelection(1, 1);
+                });
+        // omnmibox text: a|cd
+        // Moving the cursor shouldn't have caused a new call.
+        Assert.assertEquals(2, autocompleteHelper.getCallCount());
+        // The cursor position used on the last call should be the old position.
+        Assert.assertEquals(3, cursorPositionUsed.get());
+
+        // Insert "b" at the current cursor position and leave the cursor at
+        // the end of the new text.
+        ThreadUtils.runOnUiThreadBlocking(
+                () -> {
+                    urlBar.getInputConnection().commitText("b", 1);
+                });
+        autocompleteHelper.waitForCallback(2);
+        // omnmibox text: ab|cd
+        Assert.assertEquals(2, cursorPositionUsed.get());
+
+        // Delete the character before the cursor.
+        ThreadUtils.runOnUiThreadBlocking(
+                () -> {
+                    urlBar.getInputConnection().deleteSurroundingText(1, 0);
+                });
+        autocompleteHelper.waitForCallback(3);
+        // omnmibox text: a|cd
+        Assert.assertEquals(1, cursorPositionUsed.get());
+
+        // Delete the character before the cursor (again).
+        ThreadUtils.runOnUiThreadBlocking(
+                () -> {
+                    urlBar.getInputConnection().deleteSurroundingText(1, 0);
+                });
+        autocompleteHelper.waitForCallback(4);
+        // omnmibox text: |cd
+        Assert.assertEquals(0, cursorPositionUsed.get());
+    }
+
     /**
      * Ensure that we allow inline autocomplete when the text gets shorter but is not an explicit
      * delete action by the user.

chrome/test/android/javatests/src/org/chromium/chrome/test/util/OmniboxTestUtils.java

@@ -143,7 +143,7 @@ public class OmniboxTestUtils {
         }
 
         @Override
-        public void start(Profile profile, String url, final String text,
+        public void start(Profile profile, String url, final String text, int cursorPosition,
                 boolean preventInlineAutocomplete, boolean focusedFromFakebox) {
             mStartAutocompleteCalled = true;
             mSuggestionsDispatcher = new Runnable() {
@@ -211,12 +211,6 @@ public class OmniboxTestUtils {
         public void start(Profile profile, String url, String text, int cursorPosition,
                 boolean preventInlineAutocomplete, boolean focusedFromFakebox) {}
 
-        @Override
-        public final void start(Profile profile, String url, final String text,
-                boolean preventInlineAutocomplete, boolean focusedFromFakebox) {
-            start(profile, url, text, -1, preventInlineAutocomplete, focusedFromFakebox);
-        }
-
         @Override
         public void startZeroSuggest(Profile profile, String omniboxText, String url,
                 boolean focusedFromFakebox) {