Commit 5cd59211 authored by Tommy Li's avatar Tommy Li Committed by Commit Bot

[omnibox] Basic implementation of on-clobber ZeroSuggest for Desktop

This CL is the most basic implementation possible of requesting
zero-prefix suggestions (ZeroSuggestProvider) when the user deletes the
permanent text in the omnibox.

In other words, when the flag is on, and the user enters
user_input_in_progress mode by deleting everything, this will request
ZeroSuggestions.

This doesn't implement the oft=2 mode yet, so there's no distinction
between on-clobber and on-focus requests.

Bug: 1106096
Change-Id: I2f0f3353605f82b378cc2d73499b8f03ceb83f32
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2335509Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#794691}
parent 2202afc8
......@@ -600,7 +600,7 @@ void OmniboxViewViews::SetFocus(bool is_user_initiated) {
// |is_user_initiated| is true for focus events from keyboard accelerators.
if (is_user_initiated)
model()->ShowOnFocusSuggestionsIfAutocompleteIdle();
model()->StartZeroSuggestRequest();
// Restore caret visibility if focus is explicitly requested. This is
// necessary because if we already have invisible focus, the RequestFocus()
......@@ -1364,7 +1364,7 @@ bool OmniboxViewViews::OnMousePressed(const ui::MouseEvent& event) {
// - The textfield doesn't already have focus.
// - Or if the textfield is empty, to cover the NTP ZeroSuggest case.
if (event.IsOnlyLeftMouseButton() && (!HasFocus() || GetText().empty()))
model()->ShowOnFocusSuggestionsIfAutocompleteIdle();
model()->StartZeroSuggestRequest();
bool handled = views::Textfield::OnMousePressed(event);
......@@ -1474,7 +1474,7 @@ void OmniboxViewViews::OnGestureEvent(ui::GestureEvent* event) {
// - The textfield is taking focus.
// - The textfield is focused but empty, to cover the NTP ZeroSuggest case.
if (gesture_should_take_focus || (HasFocus() && GetText().empty()))
model()->ShowOnFocusSuggestionsIfAutocompleteIdle();
model()->StartZeroSuggestRequest();
views::Textfield::OnGestureEvent(event);
......
......@@ -42,6 +42,7 @@
#include "components/omnibox/browser/omnibox_view.h"
#include "components/omnibox/browser/search_provider.h"
#include "components/omnibox/browser/suggestion_answer.h"
#include "components/omnibox/common/omnibox_features.h"
#include "components/search_engines/template_url.h"
#include "components/search_engines/template_url_prepopulate_data.h"
#include "components/search_engines/template_url_service.h"
......@@ -467,14 +468,25 @@ bool OmniboxEditModel::ShouldShowCurrentPageIcon() const {
void OmniboxEditModel::UpdateInput(bool has_selected_text,
bool prevent_inline_autocomplete) {
bool changed = SetInputInProgressNoNotify(true);
bool changed_to_user_input_in_progress = SetInputInProgressNoNotify(true);
if (!has_focus()) {
if (changed)
if (changed_to_user_input_in_progress)
NotifyObserversInputInProgress(true);
return;
}
StartAutocomplete(has_selected_text, prevent_inline_autocomplete);
if (changed)
if (changed_to_user_input_in_progress && user_text_.empty() &&
base::FeatureList::IsEnabled(omnibox::kClobberIsZeroSuggestEntrypoint)) {
// In the case the user enters user-input-in-progress mode by clearing
// everything (i.e. via Backspace), and the special flag is on, ask for
// ZeroSuggestions instead of the normal prefix (as-you-type) autocomplete.
StartZeroSuggestRequest(/*user_clobbered_permanent_text=*/true);
} else {
// Otherwise run the normal prefix (as-you-type) autocomplete.
StartAutocomplete(has_selected_text, prevent_inline_autocomplete);
}
if (changed_to_user_input_in_progress)
NotifyObserversInputInProgress(true);
}
......@@ -1072,7 +1084,8 @@ void OmniboxEditModel::OnSetFocus(bool control_down) {
client_->OnInputStateChanged();
}
void OmniboxEditModel::ShowOnFocusSuggestionsIfAutocompleteIdle() {
void OmniboxEditModel::StartZeroSuggestRequest(
bool user_clobbered_permanent_text) {
// Early exit if a query is already in progress or the popup is already open.
// This is what allows this method to be called multiple times in multiple
// code locations without harm.
......@@ -1084,7 +1097,7 @@ void OmniboxEditModel::ShowOnFocusSuggestionsIfAutocompleteIdle() {
return;
// Early exit if the user already has a navigation or search query in mind.
if (user_input_in_progress_)
if (user_input_in_progress_ && !user_clobbered_permanent_text)
return;
// Send the textfield contents exactly as-is, as otherwise the verbatim
......@@ -1093,6 +1106,8 @@ void OmniboxEditModel::ShowOnFocusSuggestionsIfAutocompleteIdle() {
client_->GetSchemeClassifier());
input_.set_current_url(client_->GetURL());
input_.set_current_title(client_->GetTitle());
// TODO(tommycli): Distinguish between on-focus and on-clobber ZeroSuggest
// requests.
input_.set_from_omnibox_focus(true);
autocomplete_controller()->Start(input_);
}
......
......@@ -299,10 +299,11 @@ class OmniboxEditModel {
// control key is down (at the time we're gaining focus).
void OnSetFocus(bool control_down);
// Shows On-Focus Suggestions (ZeroSuggest) if no query is currently running
// and the popup is closed. This can be called multiple times without harm,
// since it will early-exit if an earlier request is in progress (or done).
void ShowOnFocusSuggestionsIfAutocompleteIdle();
// Starts a request for zero-prefix suggestions if no query is currently
// running and the popup is closed. This can be called multiple times without
// harm, since it will early-exit if an earlier request is in progress or
// done.
void StartZeroSuggestRequest(bool user_clobbered_permanent_text = false);
// Sets the visibility of the caret in the omnibox, if it has focus. The
// visibility of the caret is reset to visible if either
......
......@@ -270,7 +270,7 @@ TEST_F(OmniboxEditModelTest, RespectUnelisionInZeroSuggest) {
// Test that we don't clobber the unelided text with inline autocomplete text.
EXPECT_EQ(base::string16(), view()->inline_autocomplete_text());
model()->ShowOnFocusSuggestionsIfAutocompleteIdle();
model()->StartZeroSuggestRequest();
model()->OnPopupDataChanged(base::string16(), /*is_temporary_text=*/false,
base::string16(), base::string16(), false,
base::string16());
......@@ -290,7 +290,7 @@ TEST_F(OmniboxEditModelTest, RevertZeroSuggestTemporaryText) {
// Simulate getting ZeroSuggestions and arrowing to a different match.
view()->SelectAll(true);
model()->ShowOnFocusSuggestionsIfAutocompleteIdle();
model()->StartZeroSuggestRequest();
model()->OnPopupDataChanged(base::ASCIIToUTF16("fake_temporary_text"),
/*is_temporary_text=*/true, base::string16(),
base::string16(), false, base::string16());
......
......@@ -615,10 +615,14 @@ bool ZeroSuggestProvider::AllowZeroSuggestSuggestions(
return false;
// When the omnibox is empty, only allow zero suggest for the ChromeOS
// Launcher and NTP.
// Launcher and NTP, unless the clobber flag is on.
//
// TODO(tommycli): Add more nuance here, likely with an omnibox_focus_type.
if (input_type == metrics::OmniboxInputType::EMPTY &&
!(page_class == metrics::OmniboxEventProto::CHROMEOS_APP_LIST ||
IsNTPPage(page_class))) {
IsNTPPage(page_class) ||
base::FeatureList::IsEnabled(
omnibox::kClobberIsZeroSuggestEntrypoint))) {
return false;
}
......
......@@ -290,7 +290,7 @@ void OmniboxViewIOS::OnDidBeginEditing() {
model()->set_focus_source(OmniboxFocusSource::OMNIBOX);
}
model()->ShowOnFocusSuggestionsIfAutocompleteIdle();
model()->StartZeroSuggestRequest();
model()->OnSetFocus(/*control_down=*/false);
}
......
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