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

Create OmniboxResultViews lazily.

This reduces memory and CPU use during browser startup and more than eliminates
the perf regression from r706189.

Bug: 1021323
Change-Id: I10b4cf053d73fc0b38c89a35f7786635b06986ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1900403
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: default avatarKevin Bailey <krb@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713476}
parent 9e9ea001
...@@ -146,13 +146,6 @@ OmniboxPopupContentsView::OmniboxPopupContentsView( ...@@ -146,13 +146,6 @@ OmniboxPopupContentsView::OmniboxPopupContentsView(
SetLayoutManager(std::make_unique<views::BoxLayout>( SetLayoutManager(std::make_unique<views::BoxLayout>(
views::BoxLayout::Orientation::kVertical)); views::BoxLayout::Orientation::kVertical));
// TODO(krb): Remove this when we're sure that nothing accesses the
// matches between here and UpdatePopupAppearance().
for (size_t i = 0; i < AutocompleteResult::GetMaxMatches(); ++i) {
AddChildView(std::make_unique<OmniboxResultView>(this, i, theme_provider_))
->SetVisible(false);
}
} }
OmniboxPopupContentsView::~OmniboxPopupContentsView() { OmniboxPopupContentsView::~OmniboxPopupContentsView() {
...@@ -248,14 +241,14 @@ void OmniboxPopupContentsView::UpdatePopupAppearance() { ...@@ -248,14 +241,14 @@ void OmniboxPopupContentsView::UpdatePopupAppearance() {
// we have enough row views. // we have enough row views.
const size_t result_size = model_->result().size(); const size_t result_size = model_->result().size();
for (size_t i = 0; i < result_size; ++i) { for (size_t i = 0; i < result_size; ++i) {
// The model can send us more results than we expected when the user // Create child views lazily. Since especially the first result view may be
// enables loose-limit-on-submatches and has dedicated rows. Add rows to // expensive to create due to loading font data, this saves time and memory
// handle what they've sent. // during browser startup.
if (children().size() <= i) { if (children().size() <= i) {
AddChildView( AddChildView(
std::make_unique<OmniboxResultView>(this, i, theme_provider_)) std::make_unique<OmniboxResultView>(this, i, theme_provider_));
->SetVisible(false);
} }
OmniboxResultView* view = result_view_at(i); OmniboxResultView* view = result_view_at(i);
const AutocompleteMatch& match = GetMatchAtIndex(i); const AutocompleteMatch& match = GetMatchAtIndex(i);
view->SetMatch(match); view->SetMatch(match);
......
...@@ -93,6 +93,8 @@ class TestAXEventObserver : public views::AXEventObserver { ...@@ -93,6 +93,8 @@ class TestAXEventObserver : public views::AXEventObserver {
// views::AXEventObserver: // views::AXEventObserver:
void OnViewEvent(views::View* view, ax::mojom::Event event_type) override { void OnViewEvent(views::View* view, ax::mojom::Event event_type) override {
if (!view->GetWidget())
return;
ui::AXNodeData node_data; ui::AXNodeData node_data;
view->GetAccessibleNodeData(&node_data); view->GetAccessibleNodeData(&node_data);
if (event_type == ax::mojom::Event::kTextChanged && if (event_type == ax::mojom::Event::kTextChanged &&
...@@ -330,6 +332,12 @@ IN_PROC_BROWSER_TEST_F(OmniboxPopupContentsViewTest, MAYBE_ClickOmnibox) { ...@@ -330,6 +332,12 @@ IN_PROC_BROWSER_TEST_F(OmniboxPopupContentsViewTest, MAYBE_ClickOmnibox) {
// color. // color.
IN_PROC_BROWSER_TEST_F(OmniboxPopupContentsViewTest, IN_PROC_BROWSER_TEST_F(OmniboxPopupContentsViewTest,
PopupMatchesLocationBarBackground) { PopupMatchesLocationBarBackground) {
// In dark mode the omnibox focused and unfocused colors are the same, which
// makes this test fail; see comments below.
BrowserView::GetBrowserViewForBrowser(browser())
->GetNativeTheme()
->set_use_dark_colors(false);
// Start with the Omnibox unfocused. // Start with the Omnibox unfocused.
omnibox_view()->GetFocusManager()->ClearFocus(); omnibox_view()->GetFocusManager()->ClearFocus();
const SkColor color_before_focus = location_bar()->background()->get_color(); const SkColor color_before_focus = location_bar()->background()->get_color();
...@@ -420,6 +428,8 @@ IN_PROC_BROWSER_TEST_F(OmniboxPopupContentsViewTest, ...@@ -420,6 +428,8 @@ IN_PROC_BROWSER_TEST_F(OmniboxPopupContentsViewTest,
ACMatches matches; ACMatches matches;
matches.push_back(match); matches.push_back(match);
results.AppendMatches(input, matches); results.AppendMatches(input, matches);
results.SortAndCull(input, nullptr);
autocomplete_controller->NotifyChanged(true);
// Lets check that arrowing up and down emits the event. // Lets check that arrowing up and down emits the event.
TestAXEventObserver observer; TestAXEventObserver observer;
......
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