Commit 6f79cffc authored by Tommy Li's avatar Tommy Li Committed by Commit Bot

[omnibox] Prevent on-focus suggestions from popping up on startup

This CL is a followup to:
https://chromium-review.googlesource.com/c/chromium/src/+/1865616

It should prevent renderer-initiated or browser startup focus events
from triggering on-focus suggestions in the omnibox.

Bug: 1013287, 996516
Change-Id: I5bb000a6ce37a2adbd9dec9b1ef2f3a78da1ac42
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1867307Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Reviewed-by: default avatarLei Zhang <thestig@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707443}
parent 47d05071
......@@ -431,7 +431,10 @@ void OmniboxViewViews::SetFocus(bool is_user_initiated) {
->GetRevealedLock(ImmersiveModeController::ANIMATE_REVEAL_YES));
}
suppress_on_focus_suggestions_ = !is_user_initiated;
RequestFocus();
suppress_on_focus_suggestions_ = false;
// Restore caret visibility if focus is explicitly requested. This is
// necessary because if we already have invisible focus, the RequestFocus()
// call above will short-circuit, preventing us from reaching
......@@ -1306,7 +1309,7 @@ void OmniboxViewViews::OnFocus() {
// Investigate why it's needed and see if we can remove it.
model()->ResetDisplayTexts();
// TODO(oshima): Get control key state.
model()->OnSetFocus(false);
model()->OnSetFocus(false, suppress_on_focus_suggestions_);
// Don't call controller()->OnSetFocus, this view has already acquired focus.
// Restore the selection we saved in OnBlur() if it's still valid.
......
......@@ -341,6 +341,10 @@ class OmniboxViewViews : public OmniboxView,
// and gets a tap. So we use this variable to remember focus state before tap.
bool select_all_on_gesture_tap_ = false;
// True if we should suppress on-focus suggestions, because we are currently
// processing a focus ovent that we know the user didn't explicitly initiate.
bool suppress_on_focus_suggestions_ = false;
// The time of the first character insert operation that has not yet been
// painted. Used to measure omnibox responsiveness with a histogram.
base::TimeTicks insert_char_time_;
......
......@@ -481,7 +481,8 @@ void SendToOmniboxAndSubmit(Browser* browser,
base::TimeTicks match_selection_timestamp) {
LocationBar* location_bar = browser->window()->GetLocationBar();
OmniboxView* omnibox = location_bar->GetOmniboxView();
omnibox->model()->OnSetFocus(false);
omnibox->model()->OnSetFocus(/*control_down=*/false,
/*suppress_on_focus_suggestions=*/false);
omnibox->SetUserText(base::ASCIIToUTF16(input));
location_bar->AcceptInput(match_selection_timestamp);
......
......@@ -1055,7 +1055,8 @@ void OmniboxEditModel::ClearKeyword() {
}
}
void OmniboxEditModel::OnSetFocus(bool control_down) {
void OmniboxEditModel::OnSetFocus(bool control_down,
bool suppress_on_focus_suggestions) {
last_omnibox_focus_ = base::TimeTicks::Now();
user_input_since_focus_ = false;
......@@ -1069,7 +1070,8 @@ void OmniboxEditModel::OnSetFocus(bool control_down) {
// example, if the user presses ctrl-l to focus the omnibox.
control_key_state_ = control_down ? DOWN_AND_CONSUMED : UP;
ShowOnFocusSuggestionsIfAutocompleteIdle();
if (!suppress_on_focus_suggestions)
ShowOnFocusSuggestionsIfAutocompleteIdle();
if (user_input_in_progress_ || !in_revert_)
client_->OnInputStateChanged();
......
......@@ -291,7 +291,10 @@ class OmniboxEditModel {
// Called when the view is gaining focus. |control_down| is whether the
// control key is down (at the time we're gaining focus).
void OnSetFocus(bool control_down);
// |suppress_on_focus_suggestions| is set to true when on-focus suggestions
// should not appear for this focus event. For instance, for
// renderer-initiated focus events, it should be set to true.
void OnSetFocus(bool control_down, bool suppress_on_focus_suggestions);
// Shows On-Focus Suggestions (ZeroSuggest) if no query is currently running
// and the popup is closed. This can be called multiple times without harm,
......
......@@ -323,7 +323,7 @@ TEST_F(OmniboxEditModelTest, AlternateNavHasHTTP) {
AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED);
const GURL alternate_nav_url("http://abcd/");
model()->OnSetFocus(false); // Avoids DCHECK in OpenMatch().
model()->OnSetFocus(false, false); // Avoids DCHECK in OpenMatch().
model()->SetUserText(base::ASCIIToUTF16("http://abcd"));
model()->OpenMatch(match, WindowOpenDisposition::CURRENT_TAB,
alternate_nav_url, base::string16(), 0);
......@@ -457,7 +457,7 @@ TEST_F(OmniboxEditModelTest, IgnoreInvalidSavedFocusStates) {
ASSERT_EQ(OMNIBOX_FOCUS_NONE, state.focus_state);
// Simulate the tab-switching system focusing the Omnibox.
model()->OnSetFocus(false);
model()->OnSetFocus(false, true);
// Restoring the old saved state should not clobber the model's focus state.
model()->RestoreState(&state);
......@@ -483,9 +483,9 @@ TEST_F(OmniboxEditModelTest, ConsumeCtrlKey) {
// Tests ctrl_key_state_ is set consumed if the ctrl key is down on focus.
TEST_F(OmniboxEditModelTest, ConsumeCtrlKeyOnRequestFocus) {
model()->control_key_state_ = TestOmniboxEditModel::DOWN;
model()->OnSetFocus(false);
model()->OnSetFocus(false, false);
EXPECT_EQ(model()->control_key_state_, TestOmniboxEditModel::UP);
model()->OnSetFocus(true);
model()->OnSetFocus(true, false);
EXPECT_EQ(model()->control_key_state_,
TestOmniboxEditModel::DOWN_AND_CONSUMED);
}
......
......@@ -394,7 +394,8 @@ void OmniboxViewIOS::OnDidBeginEditing() {
model()->set_focus_source(OmniboxFocusSource::OMNIBOX);
}
model()->OnSetFocus(false);
model()->OnSetFocus(/*control_down=*/false,
/*suppress_on_focus_suggestions=*/false);
}
// If the omnibox is displaying a URL and the popup is not showing, set the
......
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