For the record, I'm not much surprised by this.  I'd expect (in some order):
(1) keystroke; run all omnibox suggestion provider synchronous pass; repaint
(2) history URL provider asynchronous pass that touches disk returns; incorporate results; repaint
(3) search suggestion request to the server returns; incorporate results; repaint
(4) the expire-old-matches timer triggers to remove stale results that may have been kept in order to make the omnibox suggestion list look smooth; repaint

But the list should only get repainted if there is an actual change in the list content.

There usually are changes in the list content in all those cases.  Are you certain there aren't?  One way to check this would be to look at the "incomplete results" for your input in about:omnibox and see how many different things come in at different times.

I see some quick successive updates, and I also see individual updates. But in all cases, there appear to be more repaints than actual list updates for each keystroke.

Comments #4 through #10 on the old bug 503734 may also be useful in deciding how to speed up this area.  Or not.

Unassigned P-1?

It's P1 for MacViewsBrowser, but that's not shipping in the near future.

[Bulk Edit]
Applying M-68 milestone per email discussion with ellyjones@. Pls change it if milestone is incorrectly applied. 

MacViews triage: sdy, can you take a look at this? I don't know how / if we can speed this up. This is Pri-2 and let's aim it at M69.

Sending to lgrey@ for verification since he's already looking in this area.

Renaming to encompass layout issues. From traces, paint doesn't look that bad (though I'll check it out in QuartzDebug later). Layout is killing us though, which really shows in the histograms here:
https://uma.googleplex.com/p/chrome/variations/?sid=4569c95ee1ef3bcef532e67d9b5ff6f8

lgrey@,
Which histogram are you looking at?

The only omnibox-painting histogram at that link doesn't support the claim of getting killed.  Omnibox.CharTypedToRepaintLatency measures different things on Views versus Cocoa platforms (read the histogram description for details), so the fact that that histogram seems to have regressed shouldn't be concerning.

What's more useful would be to compare the MacViews numbers with the Windows numbers.
https://uma.googleplex.com/p/chrome/histograms/?endDate=20180626&dayCount=7&histograms=Omnibox.CharTypedToRepaintLatency&fixupData=true&showMax=true&filters=platform%2Ceq%2CW%2Cchannel%2Ceq%2C1%2Cisofficial%2Ceq%2CTrue&implicitFilters=isofficial
That's fraught with peril, as the quality of machines can differ a lot.  It appears MacViews is a bit faster at 95th and 99th percentiles and a bit slower at 50th and 75th percentiles.

Agree that it's not an apples to apples comparison but:
- I think the size of the difference is probably not explained by compositing which seems like the main difference per the description
- Subjectively, everyone on the Mac team agrees that it *feels* slow, especially for the first keystroke as the dropdown is being constructed.

> - Subjectively, everyone on the Mac team agrees that it *feels* slow, especially for the first keystroke as the dropdown is being constructed.

I can confirm it. It always happens in the first opened window after launching Chrome. 

It looks fine in QuartzDebug right now modulo c#2. It looks like 2 (maybe 3 sometimes) in a row, then it stops, which roughly matches what tracing says.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/45bf0faac15aab71a2e0ea6eb090ff2fdad01017

commit 45bf0faac15aab71a2e0ea6eb090ff2fdad01017
Author: Leonard Grey <lgrey@chromium.org>
Date: Mon Jul 02 15:23:27 2018

Views: reduce redundant layout/text-setting in in OmniboxTextView

- exit early from SetText when possible
- replace some uses of CalculatePreferredSize with GetPreferredSize, and
update preferred size when text changes


Bug: 712268
Change-Id: Iaf9440b75aa94ec09b67f207bc9f98e4ab0d9301
Reviewed-on: https://chromium-review.googlesource.com/1119209
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Leonard Grey <lgrey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571898}
[modify] https://crrev.com/45bf0faac15aab71a2e0ea6eb090ff2fdad01017/chrome/browser/ui/views/omnibox/omnibox_match_cell_view.cc
[modify] https://crrev.com/45bf0faac15aab71a2e0ea6eb090ff2fdad01017/chrome/browser/ui/views/omnibox/omnibox_text_view.cc
[modify] https://crrev.com/45bf0faac15aab71a2e0ea6eb090ff2fdad01017/chrome/browser/ui/views/omnibox/omnibox_text_view.h
[modify] https://crrev.com/45bf0faac15aab71a2e0ea6eb090ff2fdad01017/components/omnibox/browser/autocomplete_match.h
[modify] https://crrev.com/45bf0faac15aab71a2e0ea6eb090ff2fdad01017/components/omnibox/browser/titled_url_match_utils_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6aac6ccf30deb69d888247a853b9087610606bd4

commit 6aac6ccf30deb69d888247a853b9087610606bd4
Author: Leonard Grey <lgrey@chromium.org>
Date: Mon Jul 09 14:50:17 2018

Views: Use cached size for Omnibox match separator

Missed that the separator was an OmniboxTextView.

Bug: 860789, 712268
Change-Id: I670fa25754f23a5aaaa9201f548d9b735f92ecf3
Reviewed-on: https://chromium-review.googlesource.com/1128187
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Leonard Grey <lgrey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573304}
[modify] https://crrev.com/6aac6ccf30deb69d888247a853b9087610606bd4/chrome/browser/ui/views/omnibox/omnibox_match_cell_view.cc

Does this bug encompass issues where keypresses into the Omnibox are being lost? I frequently create new tabs and either start typing quickly into the Omnibox or paste a URL into it, and on 69.0.3486.0 (Official Build) canary (64-bit), I'm seeing these initial keypresses frequently being dropped. Not sure when they start being accepted; maybe when the associated renderer process starts up?

Unable to reproduce the issue on chrome reported version# 60.0.3072.0 using Mac 10.12.6 with steps mentioned below:
1) Launched chrome reported version and enabled Macviews(#secondary-ui-md) flag from Chrome://flags
2) Opened New Tab Page and started typing in omnibar
3) Didn't observed any difference in the behaviour

@Leonard Grey: Please find the attached screencast for your reference and let us know if we missed anything in verifying the fix and help us in verifying the fix.

Thanks!

Deleted: 712268.mp4 1.3 MB

	712268.mp4 1.3 MB View Download

kbr@: I don't think so (suspect it has to do with the various focus issues). I know a few other people have run into that, asking around to see if there's an existing bug filed.


viswa.karala@chromium.org: the version in the original report required a separate build for MacViews mode. What we're currently targeting can be triggered via the #upcoming-ui-features flag in Canary.

Adding some analysis from a local trace of the first key being pressed:
- it takes 80 msec from keypress to the key being seen on the screen
- the first 30 msec are spent in omnibox code
- the last 24 msec are spent waiting for the frame to arrive from the GPU process

The omnibox beard window creates a new widget, which creates a new ui::Compositor, GL context, etc, and has to compile new shaders for its display. We have to wait for this window's contents to appear before we can draw the updated key in the location bar. We also have to do this wait any time that the beard resizes.

Deleted: first-key-2.png 489 KB

	first-key-2.png 489 KB View Download

Reassigning this one to ccameron@ since lgrey@ has been pulled onto a11y work for the moment.

The fringe of EnsureLayoutRunList in paint in that particular window is puzzling me since it should be 100% cache hits from layout. Are we painting with a slightly different environment? If so, I wonder if we can collapse it somehow.

> The omnibox beard window creates a new widget, which creates a new ui::Compositor, GL context, etc, and has to compile new shaders for its display. We have to wait for this window's contents to appear before we can draw the updated key in the location bar. We also have to do this wait any time that the beard resizes.

Some random thoughts.

We might be able to interleave more of the GPU work with the omnibox logic. E.g -[NSWindow setAlphaValue:0.0], Widget()->Show(), AutocompletePopupWidget::SetPopupContentsView() <waves hands>wait until frame swap, setAlphaValue:1.0</waves hands>

Or perhaps simpler, just call OmniboxPopupContentsView::UpdatePopupAppearance() with zero results early on but skip the popup_->ShowAnimated() - I think that will kick off compositor stuff via Widget::Init(), and the UI thread won't be blocked until makeKeyAndOrderFront: (maybe).

That only affects the first show though.

For resizes, I toyed with a static widget size in https://chromium-review.googlesource.com/c/chromium/src/+/869699/15 (omnibox_popup_shadow_frame) it was actually the only way to get the MD-style drop shadow on Mac since the Widget must be extend its borders rather than adopt the shadow from the WindowServer process. Something like that might do less work overall or avoid blocking the UI thread on a resize. The frame still has to come in though.


In MD-refresh only Mac cares since other platforms are all using a non-native NativeWidgetAura which shares its ui::Compositor and native window handle with the browser window. (with the downside that the beard will be "trimmed" to the height of the browser window). Moving in that direction might be good long-term remedy.

> The fringe of EnsureLayoutRunList in paint in that particular window is puzzling me since it should be 100% cache hits from layout.

I'll try printf()ing what strings exactly we're re-running there when I get a chance.

I tried a couple of fixes for the resizing-slowness issues.

>  NativeWidgetAura which shares its ui::Compositor and native window handle with the browser window

For the initial show, recycling ui::Compositors makes a really big difference. If we don't create a new ui::Compositor, my local runs drop from ~50msec to ~35msec for the initial key press (with all shape runs cached in both tests).

And if we were okay with "trimming" the beard, then this problem would go away.

> We might be able to interleave more of the GPU work with the omnibox logic.

For resize, I tried disabling the UI thread wait for windows that are translucent and non-resize-able. It removes a lot of the jank, because the location bar updates before the beard -- the effect is that it's allowing more work-interleaving.

The hard part about that is that things look terrible when the beard changes size (solvable, but harder).

And again, "trimming" would make this issue go away, too.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1b6d6cec852a5721005bdc07883a2c97aaa74f77

commit 1b6d6cec852a5721005bdc07883a2c97aaa74f77
Author: Christopher Cameron <ccameron@chromium.org>
Date: Thu Jul 12 21:51:03 2018

MacViews: Enable compositor recycling

We create a new ui::Compositor at the first character typed in the
omnibox. This is very expensive, and is compounded by the fact that the
shaders used by rasterization and display must be re-created in this
new context.

There already exists ui::Compositor recycling in content:: for
RenderWidgetHostViewMac. Move this code to ui::, and share it with
BridgedNativeWidget.

Bug: 712268
Change-Id: I969174e2c38b181e18a10e78dd49d3e9be9690e5
Reviewed-on: https://chromium-review.googlesource.com/1132710
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Leonard Grey <lgrey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574738}
[modify] https://crrev.com/1b6d6cec852a5721005bdc07883a2c97aaa74f77/content/browser/renderer_host/browser_compositor_view_mac.h
[modify] https://crrev.com/1b6d6cec852a5721005bdc07883a2c97aaa74f77/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/1b6d6cec852a5721005bdc07883a2c97aaa74f77/ui/compositor/BUILD.gn
[modify] https://crrev.com/1b6d6cec852a5721005bdc07883a2c97aaa74f77/ui/compositor/DEPS
[add] https://crrev.com/1b6d6cec852a5721005bdc07883a2c97aaa74f77/ui/compositor/recyclable_compositor_mac.cc
[add] https://crrev.com/1b6d6cec852a5721005bdc07883a2c97aaa74f77/ui/compositor/recyclable_compositor_mac.h
[modify] https://crrev.com/1b6d6cec852a5721005bdc07883a2c97aaa74f77/ui/views/cocoa/bridged_native_widget.h
[modify] https://crrev.com/1b6d6cec852a5721005bdc07883a2c97aaa74f77/ui/views/cocoa/bridged_native_widget.mm
[modify] https://crrev.com/1b6d6cec852a5721005bdc07883a2c97aaa74f77/ui/views/widget/native_widget_mac_unittest.mm

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7017a3200682254332299d01783be507f8960258

commit 7017a3200682254332299d01783be507f8960258
Author: Christopher Cameron <ccameron@chromium.org>
Date: Thu Jul 12 23:28:52 2018

MacViews: Do not block UI thread waiting for translucent windows

For translucent windows, don't do any CATransaction synchronization
until after the first frame has been received.

This allows us to avoid blocking the UI thread when entering the first
keystrokes in the omnibox.

While in the neighborhood, delete the dead code for using alpha to
make windows appear atomically.

Bug: 712268
Change-Id: I71cab1c1c598adc34542b6990d8f9112eda8be4f
Reviewed-on: https://chromium-review.googlesource.com/1135891
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574771}
[modify] https://crrev.com/7017a3200682254332299d01783be507f8960258/ui/views/cocoa/bridged_native_widget.h
[modify] https://crrev.com/7017a3200682254332299d01783be507f8960258/ui/views/cocoa/bridged_native_widget.mm

Issue 862025 has been merged into this issue.

See attached image for another reason why recycling compositors might be good. This is after closing the omnibox popup, then refocusing the empty omnibox and typing 

Deleted: Screen Shot 2018-07-13 at 11.13.51 AM.png 47.5 KB

	Screen Shot 2018-07-13 at 11.13.51 AM.png 47.5 KB View Download

Yeah, we currently post a task for the destructor ... would be nice to post a task "to run when you're idle"

(all of this goes away next year with OOP-D and OOP-R)

It picks an interesting time to fire! (There was a few seconds between closing the old popup and this)

Current perf doesn't appear to be release blocking anymore. Cheers!

Let's create new bugs blocking this one for fututre work.

It appears this isn't actually waiting for any feedback.