Commit e11adef4 authored by Peter Kasting's avatar Peter Kasting Committed by Commit Bot

[omnibox] Cleanup: Hoist ProvideButtonHint() call to OmniboxPopupModel.

We want this to happen every time a row is BUTTON_FOCUSED.  Hoisting it to the
location where that's actually handled guarantees this and results in shorter
code.  It also makes more sense for the popup model to be instructing the result
views of this than it does OmniboxViewViews, which ideally wouldn't know much
about the popup views.

Bug: none
TBR: stkhapugin
Change-Id: I0b1da7d8d459f79f25dc7246dc7a1c4c81e9f25e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1900254Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Reviewed-by: default avatarKevin Bailey <krb@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713553}
parent 47c9a8ca
......@@ -194,11 +194,6 @@ void OmniboxPopupContentsView::UnselectButton() {
model_->SetSelectedLineState(OmniboxPopupModel::NORMAL);
}
void OmniboxPopupContentsView::ProvideButtonFocusHint(size_t line) {
OmniboxResultView* result = result_view_at(line);
result->ProvideButtonFocusHint();
}
bool OmniboxPopupContentsView::InExplicitExperimentalKeywordMode() {
return model_->edit_model()->InExplicitExperimentalKeywordMode();
}
......@@ -300,6 +295,10 @@ void OmniboxPopupContentsView::UpdatePopupAppearance() {
InvalidateLayout();
}
void OmniboxPopupContentsView::ProvideButtonFocusHint(size_t line) {
result_view_at(line)->ProvideButtonFocusHint();
}
void OmniboxPopupContentsView::OnMatchIconUpdated(size_t match_index) {
result_view_at(match_index)->OnMatchIconUpdated();
}
......
......@@ -60,9 +60,6 @@ class OmniboxPopupContentsView : public views::View, public OmniboxPopupView {
// Called by the active result view to inform model (due to mouse event).
void UnselectButton();
// Called to inform result view of button focus.
void ProvideButtonFocusHint(size_t line);
// Returns whether we're in experimental keyword mode and the input gives
// sufficient confidence that the user wants keyword mode.
bool InExplicitExperimentalKeywordMode();
......@@ -72,6 +69,7 @@ class OmniboxPopupContentsView : public views::View, public OmniboxPopupView {
void InvalidateLine(size_t line) override;
void OnLineSelected(size_t line) override;
void UpdatePopupAppearance() override;
void ProvideButtonFocusHint(size_t line) override;
void OnMatchIconUpdated(size_t match_index) override;
void OnDragCanceled() override;
......
......@@ -671,8 +671,6 @@ bool OmniboxViewViews::HandleEarlyTabActions(const ui::KeyEvent& event) {
model()->popup_model()->SelectedLineHasTabMatch()) {
model()->popup_model()->SetSelectedLineState(
OmniboxPopupModel::BUTTON_FOCUSED);
popup_view_->ProvideButtonFocusHint(
model()->popup_model()->selected_line());
}
return true;
......@@ -715,8 +713,6 @@ bool OmniboxViewViews::MaybeFocusTabButton() {
OmniboxPopupModel::NORMAL) {
model()->popup_model()->SetSelectedLineState(
OmniboxPopupModel::BUTTON_FOCUSED);
popup_view_->ProvideButtonFocusHint(
model()->popup_model()->selected_line());
return true;
}
return false;
......
......@@ -180,16 +180,9 @@ void OmniboxPopupModel::SetSelectedLineState(LineState state) {
DCHECK(!result().empty());
DCHECK_NE(kNoMatch, selected_line_);
const AutocompleteResult& result = this->result();
if (result.empty())
return;
const AutocompleteMatch& match = result.match_at(selected_line_);
const AutocompleteMatch& match = result().match_at(selected_line_);
GURL current_destination(match.destination_url);
if (state == KEYWORD) {
DCHECK(match.associated_keyword.get());
}
DCHECK((state != KEYWORD) || match.associated_keyword.get());
if (state == BUTTON_FOCUSED) {
// TODO(orinj): If in-suggestion Pedals are kept, refactor a bit
......@@ -201,10 +194,10 @@ void OmniboxPopupModel::SetSelectedLineState(LineState state) {
selected_line_state_ = state;
view_->InvalidateLine(selected_line_);
// Ensures update of accessibility data for button text.
if (state == BUTTON_FOCUSED) {
edit_model_->view()->SetAccessibilityLabel(edit_model_->view()->GetText(),
match);
view_->ProvideButtonFocusHint(selected_line_);
}
}
......
......@@ -33,6 +33,7 @@ class TestOmniboxPopupView : public OmniboxPopupView {
void InvalidateLine(size_t line) override {}
void OnLineSelected(size_t line) override {}
void UpdatePopupAppearance() override {}
void ProvideButtonFocusHint(size_t line) override {}
void OnMatchIconUpdated(size_t match_index) override {}
void OnDragCanceled() override {}
};
......
......@@ -32,6 +32,9 @@ class OmniboxPopupView {
// mean opening or closing the window.
virtual void UpdatePopupAppearance() = 0;
// Called to inform result view of button focus.
virtual void ProvideButtonFocusHint(size_t line) = 0;
// Notification that the icon used for the given match has been updated.
virtual void OnMatchIconUpdated(size_t match_index) = 0;
......
......@@ -38,6 +38,7 @@ class OmniboxPopupViewIOS : public OmniboxPopupView,
void InvalidateLine(size_t line) override {}
void OnLineSelected(size_t line) override {}
void UpdatePopupAppearance() override;
void ProvideButtonFocusHint(size_t line) override {}
void OnMatchIconUpdated(size_t match_index) override {}
void OnDragCanceled() override {}
......
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