Skip to content

Commit

Permalink
Remove textDeleted from LocationBarLayout
Browse files Browse the repository at this point in the history
shouldAutocomplete() already tracks whether the last selection was a deletion,
So the only logic change is in mRequestSuggestions.run() that it no longer
checks the old state, which makes sense in case you delete something
and then type immediately.

This also helps simplify the new model we are about to add.

BUG=735609, 539536

Change-Id: Id5346399e60f4c9e401cc1fe8088c3876fd46c02
Reviewed-on: https://chromium-review.googlesource.com/544516
Commit-Queue: Changwan Ryu <changwan@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481567}
  • Loading branch information
galmacky authored and Commit Bot committed Jun 22, 2017
1 parent cbbf1c7 commit d43235d
Show file tree
Hide file tree
Showing 8 changed files with 25 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ public void replaceAllTextFromAutocomplete(String text) {
}

@Override
public void onAutocompleteTextStateChanged(boolean textDeleted, boolean updateDisplay) {
public void onAutocompleteTextStateChanged(boolean updateDisplay) {
assert false; // make sure that this method is properly overridden.
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,13 @@ private void notifyAutocompleteTextStateChanged(boolean textDeleted, boolean upd
}
if (mIgnoreTextChangeFromAutocomplete) return;
mLastEditWasDelete = textDeleted;
mDelegate.onAutocompleteTextStateChanged(textDeleted, updateDisplay);
mDelegate.onAutocompleteTextStateChanged(updateDisplay);
// Occasionally, was seeing the selection in the URL not being cleared during
// very rapid editing. This is here to hopefully force a selection reset during
// deletes.
if (textDeleted) {
mDelegate.setSelection(mDelegate.getSelectionStart(), mDelegate.getSelectionStart());
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,9 @@ public interface Delegate {

/**
* This is called when autocomplete text state changes.
* @param textDeleted True if text is just deleted.
* @param updateDisplay True if string is changed.
*/
void onAutocompleteTextStateChanged(boolean textDeleted, boolean updateDisplay);
void onAutocompleteTextStateChanged(boolean updateDisplay);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1126,7 +1126,7 @@ private void startZeroSuggest() {
}

@Override
public void onTextChangedForAutocomplete(final boolean textDeleted) {
public void onTextChangedForAutocomplete() {
cancelPendingAutocompleteStart();

updateButtonVisibility();
Expand All @@ -1152,8 +1152,7 @@ public void onTextChangedForAutocomplete(final boolean textDeleted) {
@Override
public void run() {
String textWithoutAutocomplete = mUrlBar.getTextWithoutAutocomplete();

boolean preventAutocomplete = textDeleted || !mUrlBar.shouldAutocomplete();
boolean preventAutocomplete = !mUrlBar.shouldAutocomplete();
mRequestSuggestions = null;

if (getCurrentTab() == null
Expand All @@ -1174,11 +1173,6 @@ public void run() {
mDeferredNativeRunnables.add(mRequestSuggestions);
}
}

// Occasionally, was seeing the selection in the URL not being cleared during
// very rapid editing. This is here to hopefully force a selection reset during
// deletes.
if (textDeleted) mUrlBar.setSelection(mUrlBar.getSelectionStart());
}

@Override
Expand Down Expand Up @@ -2373,7 +2367,7 @@ public void onWindowFocusChanged(boolean hasWindowFocus) {
if (!hasWindowFocus && !mSuggestionModalShown) {
hideSuggestions();
} else if (hasWindowFocus && mUrlHasFocus && mNativeInitialized) {
onTextChangedForAutocomplete(false);
onTextChangedForAutocomplete();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,8 @@ public interface UrlBarDelegate {
/**
* Called when the text state has changed and the autocomplete suggestions should be
* refreshed.
*
* @param textDeleted Whether this change was as a result of text being deleted.
*/
void onTextChangedForAutocomplete(boolean textDeleted);
void onTextChangedForAutocomplete();

/**
* @return Whether the light security theme should be used.
Expand Down Expand Up @@ -784,15 +782,14 @@ public void replaceAllTextFromAutocomplete(String text) {
}

@Override
public void onAutocompleteTextStateChanged(boolean textDeleted, boolean updateDisplay) {
public void onAutocompleteTextStateChanged(boolean updateDisplay) {
if (DEBUG) {
Log.i(TAG, "onAutocompleteTextStateChanged: DEL[%b], DIS[%b]", textDeleted,
updateDisplay);
Log.i(TAG, "onAutocompleteTextStateChanged: DIS[%b]", updateDisplay);
}
if (mUrlBarDelegate == null) return;
if (updateDisplay) limitDisplayableLength();

mUrlBarDelegate.onTextChangedForAutocomplete(textDeleted);
mUrlBarDelegate.onTextChangedForAutocomplete();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public void onNativeLibraryReady() {
void onDeferredStartup(boolean isVoiceSearchIntent) {
SearchWidgetProvider.updateCachedVoiceSearchAvailability(isVoiceSearchEnabled());
if (isVoiceSearchIntent && mUrlBar.isFocused()) onUrlFocusChange(true);
if (!TextUtils.isEmpty(mUrlBar.getText())) onTextChangedForAutocomplete(false);
if (!TextUtils.isEmpty(mUrlBar.getText())) onTextChangedForAutocomplete();
}

/** Begins a new query. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,7 @@ private static String parsePublisherNameFromUrl(String url) {
// Toolbar and LocationBar calls that are not relevant here.

@Override
public void onTextChangedForAutocomplete(boolean canInlineAutocomplete) {}
public void onTextChangedForAutocomplete() {}

@Override
public void backKeyPressed() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public class AutocompleteEditTextTest {

// This is needed to limit the target of inOrder#verify.
private static class AutocompleteEmbedder {
public void onAutocompleteTextStateChanged(boolean textDeleted, boolean updateDisplay) {
public void onAutocompleteTextStateChanged(boolean updateDisplay) {
if (DEBUG) Log.i(TAG, "onAutocompleteTextStateChanged");
}
}
Expand All @@ -61,10 +61,10 @@ public TestAutocompleteEditText(
}

@Override
public void onAutocompleteTextStateChanged(boolean textDeleted, boolean updateDisplay) {
public void onAutocompleteTextStateChanged(boolean updateDisplay) {
// This function is called in super(), so mEmbedder may be null.
if (mEmbedder != null) {
mEmbedder.onAutocompleteTextStateChanged(textDeleted, updateDisplay);
mEmbedder.onAutocompleteTextStateChanged(updateDisplay);
}
}

Expand All @@ -84,7 +84,7 @@ public void setUp() throws Exception {
mAutocomplete = spy(new TestAutocompleteEditText(mEmbedder, mContext, null));
assertNotNull(mAutocomplete);
// Note: this cannot catch the first {@link
// AutocompleteEditText#onAutocompleteTextStateChanged(boolean, boolean)}, which is caused
// AutocompleteEditText#onAutocompleteTextStateChanged(boolean)}, which is caused
// by View constructor's call to setText().
mInOrder = inOrder(mEmbedder);
mAutocomplete.onCreateInputConnection(new EditorInfo());
Expand All @@ -107,10 +107,10 @@ public void testAppend_CommitText() {
mAutocomplete.setIgnoreTextChangesForAutocomplete(false);
// User types "h".
assertTrue(mInputConnection.commitText("h", 1));
mInOrder.verify(mEmbedder).onAutocompleteTextStateChanged(false, true);
mInOrder.verify(mEmbedder).onAutocompleteTextStateChanged(true);
// User types "hello".
assertTrue(mInputConnection.commitText("ello", 1));
mInOrder.verify(mEmbedder).onAutocompleteTextStateChanged(false, true);
mInOrder.verify(mEmbedder).onAutocompleteTextStateChanged(true);
mInOrder.verifyNoMoreInteractions();
// The controller kicks in.
mAutocomplete.setAutocompleteText("hello", " world");
Expand All @@ -120,7 +120,7 @@ public void testAppend_CommitText() {
assertTexts("hello ", "world");
mInOrder.verifyNoMoreInteractions();
assertTrue(mInputConnection.endBatchEdit());
mInOrder.verify(mEmbedder).onAutocompleteTextStateChanged(false, true);
mInOrder.verify(mEmbedder).onAutocompleteTextStateChanged(true);
mAutocomplete.setAutocompleteText("hello ", "world");
assertTexts("hello ", "world");
mInOrder.verifyNoMoreInteractions();
Expand Down

0 comments on commit d43235d

Please sign in to comment.