diff --git a/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/OmniboxSuggestionsDropdown.java b/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/OmniboxSuggestionsDropdown.java index 2b696a726395fa..0953252cd73839 100644 --- a/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/OmniboxSuggestionsDropdown.java +++ b/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/OmniboxSuggestionsDropdown.java @@ -137,8 +137,9 @@ public int scrollVerticallyBy( // visibility), or // 2. the list is too short, and almost entirely fits on the screen, leaving at most // just a few pixels of content hiding under the keyboard. - // There's no need to dismiss the keyboard in any of these cases. - if (resultingDeltaY < requestedDeltaY) return resultingDeltaY; + // Note that the list may extend below the keyboard and still be non-scrollable: + // http://crbug/1479437 + // Otherwise decide whether keyboard should be shown or not. // We want to call keyboard up only when we know we reached the top of the list. // Note: the condition below evaluates `true` only if the scroll direction is "up", diff --git a/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/OmniboxSuggestionsDropdownUnitTest.java b/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/OmniboxSuggestionsDropdownUnitTest.java index d869f22df20e91..efe7e56b1db944 100644 --- a/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/OmniboxSuggestionsDropdownUnitTest.java +++ b/chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/suggestions/OmniboxSuggestionsDropdownUnitTest.java @@ -221,36 +221,41 @@ public void testScrollListener_keyboardShouldShowOnScrollToTop() { @Test @SmallTest - public void testScrollListener_notDismissingKeyboardWhenScrollDoesNotHappen() { + public void testScrollListener_dismissingKeyboardWhenScrollDoesNotHappen() { + // In some cases the list may be long enough to stretch below the keyboard, but not long + // enough to be scrollable. We want to dismiss the keyboard in these cases, too. mDropdown.setSuggestionDropdownScrollListener(mDropdownScrollListener); + mDropdown.setSuggestionDropdownOverscrolledToTopListener(mDropdownScrollToTopListener); // Pretend we're scrolling down (delta=10) but there is no content to move to (scroll=0). assertEquals(0, mListener.updateKeyboardVisibilityAndScroll(0, 10)); - // Confirm that we're not hiding the keyboard. - verifyNoMoreInteractions(mDropdownScrollListener); + // Confirm that we're hiding the keyboard. + verify(mDropdownScrollListener).run(); // Pretend we're scrolling up now (delta=-10) but we're already on top and can't move. assertEquals(0, mListener.updateKeyboardVisibilityAndScroll(0, -10)); // Confirm that we're not trying to show the keyboard. - verifyNoMoreInteractions(mDropdownScrollListener); + verify(mDropdownScrollToTopListener).run(); + + verifyNoMoreInteractions(mDropdownScrollListener, mDropdownScrollToTopListener); } @Test @SmallTest - public void testScrollListener_notDismissingKeyboardWhenTheListIsOnlyBarelyUnderTheKeyboard() { + public void testScrollListener_dismissingKeyboardWhenTheListIsOnlyBarelyUnderTheKeyboard() { mDropdown.setSuggestionDropdownScrollListener(mDropdownScrollListener); + mDropdown.setSuggestionDropdownOverscrolledToTopListener(mDropdownScrollToTopListener); - // We want to scroll by 10px, but there's only 1px of content. Don't hide the keyboard. - assertEquals(1, mListener.updateKeyboardVisibilityAndScroll(1, 10)); - verifyNoMoreInteractions(mDropdownScrollListener); - - // We want to scroll by 10px, but there's only 9px of content. Don't hide the keyboard. - assertEquals(9, mListener.updateKeyboardVisibilityAndScroll(9, 10)); - verifyNoMoreInteractions(mDropdownScrollListener); + // We want to scroll by 10px, but there's only 1px of slack. This means the suggestions list + // spans entirely under the keyboard. Hide the keyboard. + assertEquals(0, mListener.updateKeyboardVisibilityAndScroll(1, 10)); + verify(mDropdownScrollListener).run(); - // But then, if we scroll back up, we likely should not ask for keyboard to show. + // Reset keyboard state. assertEquals(-9, mListener.updateKeyboardVisibilityAndScroll(-9, -10)); - verifyNoMoreInteractions(mDropdownScrollListener); + verify(mDropdownScrollToTopListener).run(); + + verifyNoMoreInteractions(mDropdownScrollListener, mDropdownScrollToTopListener); } @Test