Commit 7c2b5f35 authored by Maciej Pawlowski's avatar Maciej Pawlowski Committed by Chromium LUCI CQ

Ensure the omnibox's text isn't shorter than the cursor position

Sometimes cursorPosition computed based on mUrlBarEditingTextProvider
would be larger than the size of textWithoutAutocomplete given to
onTextChanged(). This happened when typing really fast on a slow device,
in Debug builds.

To ensure this situation doesn't trip a DCHECK within
AutocompleteControllerAndroid, use the up-to-date value of
mUrlBarEditingTextProvider's text when passing a computed cursor position.

Bug: 1142817
Change-Id: Id6ef48d561461fd711cbc9a4abab8851b8406a2a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2503693Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Reviewed-by: default avatarTomasz Wiszkowski <ender@google.com>
Commit-Queue: Ted Choc <tedchoc@chromium.org>
Auto-Submit: Maciej Pawlowski <m.pawlowski@eyeo.com>
Cr-Commit-Position: refs/heads/master@{#831920}
parent 82ab360b
......@@ -637,6 +637,7 @@ Luke Seunghoe Gu <gulukesh@gmail.com>
Luke Zarko <lukezarko@gmail.com>
Luoxi Pan <l.panpax@gmail.com>
Maarten Lankhorst <m.b.lankhorst@gmail.com>
Maciej Pawlowski <m.pawlowski@eyeo.com>
Magnus Danielsson <fuzzac@gmail.com>
Mahesh Kulkarni <mahesh.kk@samsung.com>
Mahesh Machavolu <mahesh.ma@samsung.com>
......
......@@ -691,38 +691,35 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi
startZeroSuggest();
} else {
assert mRequestSuggestions == null : "Multiple omnibox requests in flight.";
mRequestSuggestions = () -> {
if (mDataProvider.hasTab() || mDataProvider.isInOverviewAndShowingOmnibox()) {
boolean preventAutocomplete = !mUrlBarEditingTextProvider.shouldAutocomplete();
mRequestSuggestions = null;
// There may be no tabs when searching form omnibox in overview mode. In that case,
// LocationBarDataProvider.getCurrentUrl() returns NTP url.
if (!mDataProvider.hasTab() && !mDataProvider.isInOverviewAndShowingOmnibox()) {
// crbug.com/764749
Log.w(TAG, "onTextChangedForAutocomplete: no tab");
return;
}
Profile profile = mDataProvider.getProfile();
int cursorPosition = -1;
if (mUrlBarEditingTextProvider.getSelectionStart()
== mUrlBarEditingTextProvider.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 = mUrlBarEditingTextProvider.getSelectionStart();
}
int cursorPosition = mUrlBarEditingTextProvider.getSelectionStart()
== mUrlBarEditingTextProvider.getSelectionEnd()
? mUrlBarEditingTextProvider.getSelectionStart()
: -1;
int pageClassification =
mDataProvider.getPageClassification(mDelegate.didFocusUrlFromFakebox());
mAutocomplete.start(profile, mDataProvider.getCurrentUrl(), pageClassification,
textWithoutAutocomplete, cursorPosition, preventAutocomplete, null,
mDelegate.didFocusUrlFromQueryTiles()
|| mEditSessionState == EditSessionState.ACTIVATED_BY_QUERY_TILE);
};
if (mNativeInitialized) {
mHandler.postDelayed(mRequestSuggestions, OMNIBOX_SUGGESTION_START_DELAY_MS);
String currentUrl = mDataProvider.getCurrentUrl();
boolean isQueryStartedFromTiles = mDelegate.didFocusUrlFromQueryTiles()
|| mEditSessionState == EditSessionState.ACTIVATED_BY_QUERY_TILE;
mRequestSuggestions = () -> {
mRequestSuggestions = null;
mAutocomplete.start(profile, currentUrl, pageClassification,
textWithoutAutocomplete, cursorPosition, preventAutocomplete, null,
isQueryStartedFromTiles);
};
if (mNativeInitialized) {
mHandler.postDelayed(mRequestSuggestions, OMNIBOX_SUGGESTION_START_DELAY_MS);
} else {
mDeferredNativeRunnables.add(mRequestSuggestions);
}
} else {
mDeferredNativeRunnables.add(mRequestSuggestions);
// There may be no tabs when searching form omnibox in overview mode. In that case,
// LocationBarDataProvider.getCurrentUrl() returns NTP url.
// crbug.com/764749
Log.w(TAG, "onTextChangedForAutocomplete: no tab");
}
}
......
......@@ -41,6 +41,7 @@ import org.chromium.base.ContextUtils;
import org.chromium.base.test.BaseJUnit4ClassRunner;
import org.chromium.base.test.UiThreadTest;
import org.chromium.base.test.util.Batch;
import org.chromium.base.test.util.DisabledTest;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
import org.chromium.chrome.browser.omnibox.LocationBarDataProvider;
......@@ -441,6 +442,7 @@ public class AutocompleteMediatorUnitTest {
.start(profile, url, pageClassification, "test", 4, false, null, false);
}
@DisabledTest(message = "Does not test what it says: http://crbug.com/1147443")
@Test
@SmallTest
@UiThreadTest
......
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