Browser window spends ~50ms laying out while resizing on pages with text in the security chip |
|||||||||||
Issue descriptionChrome Version: 69.0.3451.0 OS: macOS What steps will reproduce the problem? (1) Run Chrome with --enable-features=ViewsBrowserWindows. (2) Visit about:blank. (3) Resize the window and watch the performance. (4) Visit "data:text/html" or any other simple web page. (5) Repeat step 3. What is the expected result? Smooth resizing. What happens instead? Smooth resizing on about:blank, sloooow resizing on any other page. It looks like about 50ms per frame is spent on layout in Views, when on a non-about:blank page.
,
Jun 6 2018
,
Jun 7 2018
I have an old hacky cl that adds more tracepoints to dive into the expensive Layout() calls - https://chromium-review.googlesource.com/c/chromium/src/+/799537 Hiding the bookmarks bar may have significant impact. Changing the way we elide bookmarks in the bookmarks bar from ELIDE to FADE might have a big impact too (but might upset some people). We should also focus on optimizing the layout provided by --top-chrome-md=MaterialRefresh
,
Jun 7 2018
(oh, and caching RenderText objects in the bookmarks bar might also have significant impact .. assuming we don't currently. But not doing that has traditionally been a big source of slowness.)
,
Jun 7 2018
FWIW: - Bookmark bar hidden/shown doesn't seem to make a difference. - The difference between being on about:blank and being on "data:text/html," (which show the same content/ui afaik) is huge. - The speed is about the same whether refresh is on or off.
,
Jun 8 2018
I added some logging in View::InvalidateLayout:
static int lcount = 0;
LOG(ERROR) << "InvalidateLayout " << GetClassName();
if (!strcmp(GetClassName(), "Label")) {
lcount++;
if (lcount % 50 == 0)
base::debug::StackTrace().Print();
}
This yielded:
[70326:775:0608/084318.370708:ERROR:view.cc(628)] InvalidateLayout Label
0 libbase.dylib 0x00000001156b5c5c base::debug::StackTrace::StackTrace(unsigned long) + 28
1 libviews.dylib 0x000000011f6d8894 views::View::InvalidateLayout() + 228
2 libviews.dylib 0x000000011f6dcaec views::View::PreferredSizeChanged() + 28
3 libviews.dylib 0x000000011f66f70a views::Label::ResetLayout() + 26
4 libviews.dylib 0x000000011f66f835 views::Label::SetText(std::__1::basic_string<unsigned short, base::string16_internals::string16_char_traits, std::__1::allocator<unsigned short> > const&) + 149
5 libviews.dylib 0x000000011f66ee99 views::Label::Init(std::__1::basic_string<unsigned short, base::string16_internals::string16_char_traits, std::__1::allocator<unsigned short> > const&, gfx::FontList const&) + 457
6 libchrome_dll.dylib 0x00000001111adc3a LocationIconView::GetMinimumSizeForLabelText(std::__1::basic_string<unsigned short, base::string16_internals::string16_char_traits, std::__1::allocator<unsigned short> > const&) const + 58
7 libchrome_dll.dylib 0x00000001111a94ed LocationBarView::CalculatePreferredSize() const + 221
8 libchrome_dll.dylib 0x0000000111209a9b ToolbarView::Layout() + 859
9 libviews.dylib 0x000000011f6d5b3e views::View::SetBounds(int, int, int, int) + 126
10 libchrome_dll.dylib 0x0000000111195b8f BrowserViewLayout::Layout(views::View*) + 335
11 libviews.dylib 0x000000011f6d85ee views::View::Layout() + 62
12 libchrome_dll.dylib 0x000000011119316e non-virtual thunk to BrowserView::Layout() + 46
13 libviews.dylib 0x000000011f6fb01d views::NonClientView::Layout() + 253
14 libviews.dylib 0x000000011f6d6694 views::View::BoundsChanged(gfx::Rect const&) + 2196
15 libviews.dylib 0x000000011f6d5c68 views::View::SetBoundsRect(gfx::Rect const&) + 280
16 libviews.dylib 0x000000011f6c8e98 views::FillLayout::Layout(views::View*) + 104
17 libviews.dylib 0x000000011f6d85ee views::View::Layout() + 62
18 libviews.dylib 0x000000011f6d6694 views::View::BoundsChanged(gfx::Rect const&) + 2196
19 libviews.dylib 0x000000011f6d5c68 views::View::SetBoundsRect(gfx::Rect const&) + 280
20 libviews.dylib 0x000000011f6d6789 views::View::SetSize(gfx::Size const&) + 137
21 libviews.dylib 0x000000011f648f50 -[BridgedContentView setFrameSize:] + 256
22 AppKit 0x00007fff3a021818 -[NSWindow _oldPlaceWindow:] + 1012
23 AppKit 0x00007fff3a020c79 -[NSWindow _setFrameCommon:display:stashSize:] + 3017
24 AppKit 0x00007fff3a020099 -[NSWindow _setFrame:display:allowImplicitAnimation:stashSize:] + 192
25 AppKit 0x00007fff3a01ffce -[NSWindow setFrame:display:] + 51
26 AppKit 0x00007fff3aa14751 -[NSWindow(NSWindowResizing) _resizeSetFrame:withEvent:] + 107
27 AppKit 0x00007fff3a268b51 -[NSWindow(NSWindowResizing) _resizeWithEvent:] + 3269
28 AppKit 0x00007fff3a78b788 -[NSTitledFrame attemptResizeWithEvent:] + 177
29 AppKit 0x00007fff3a17cb08 -[NSThemeFrame handleMouseDown:] + 297
30 AppKit 0x00007fff3a17ccfa -[NSThemeFrame mouseDown:] + 30
31 libchrome_dll.dylib 0x00000001110d1388 -[BrowserWindowFrame mouseDown:] + 184
32 AppKit 0x00007fff3a84fb39 -[NSWindow(NSEventRouting) _handleMouseDownEvent:isDelayedEvent:] + 5658
33 AppKit 0x00007fff3a84c91b -[NSWindow(NSEventRouting) _reallySendEvent:isDelayedEvent:] + 2319
34 AppKit 0x00007fff3a84bbdd -[NSWindow(NSEventRouting) sendEvent:] + 481
35 libviews.dylib 0x000000011f654a51 -[NativeWidgetMacNSWindow sendEvent:] + 161
36 AppKit 0x00007fff3a6b2650 -[NSApplication(NSEvent) sendEvent:] + 336
37 libchrome_dll.dylib 0x000000010fa7cabc __34-[BrowserCrApplication sendEvent:]_block_invoke + 172
38 libbase.dylib 0x00000001155d7f6a base::mac::CallWithEHFrame(void () block_pointer) + 10
39 libchrome_dll.dylib 0x000000010fa7c76d -[BrowserCrApplication sendEvent:] + 797
40 AppKit 0x00007fff39f6211d -[NSApplication run] + 755
41 libbase.dylib 0x00000001155f80bc base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate*) + 364
42 libbase.dylib 0x00000001155f630e base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*) + 110
43 libbase.dylib 0x00000001155f2124 base::MessageLoop::Run(bool) + 132
44 libbase.dylib 0x0000000115630709 base::RunLoop::Run() + 249
45 libchrome_dll.dylib 0x000000010fa8312b ChromeBrowserMainParts::MainMessageLoopRun(int*) + 267
46 libcontent.dylib 0x00000001197a1a34 content::BrowserMainLoop::RunMainMessageLoopParts() + 52
47 libcontent.dylib 0x00000001197a4a16 content::BrowserMainRunnerImpl::Run() + 102
48 libcontent.dylib 0x000000011979de64 content::BrowserMain(content::MainFunctionParams const&, std::__1::unique_ptr<content::BrowserProcessSubThread, std::__1::default_delete<content::BrowserProcessSubThread> >) + 276
49 libcontent.dylib 0x000000011a364443 content::RunBrowserProcessMain(content::MainFunctionParams const&, content::ContentMainDelegate*, std::__1::unique_ptr<content::BrowserProcessSubThread, std::__1::default_delete<content::BrowserProcessSubThread> >) + 195
50 libcontent.dylib 0x000000011a3653be content::ContentMainRunnerImpl::Run() + 686
51 libembedder.dylib 0x0000000115553de7 service_manager::Main(service_manager::MainParams const&) + 2855
52 libcontent.dylib 0x000000011a364354 content::ContentMain(content::ContentMainParams const&) + 68
53 libchrome_dll.dylib 0x000000010f3cb373 ChromeMain + 179
54 Chromium 0x000000010f352de5 main + 405
55 libdyld.dylib 0x00007fff6997fee1 start + 1
56 ??? 0x0000000000000001 0x0 + 1
,
Jun 8 2018
That one's probably a red herring (although having LocationIconView::GetMinimumSizeForLabelText create a new Label constantly is pretty bad). This stack trace looks more relevant: [70326:775:0608/084318.528557:ERROR:view.cc(628)] InvalidateLayout Label 0 libbase.dylib 0x00000001156b5c5c base::debug::StackTrace::StackTrace(unsigned long) + 28 1 libviews.dylib 0x000000011f6d8894 views::View::InvalidateLayout() + 228 2 libviews.dylib 0x000000011f6dcaec views::View::PreferredSizeChanged() + 28 3 libviews.dylib 0x000000011f66f70a views::Label::ResetLayout() + 26 4 libviews.dylib 0x000000011f66f835 views::Label::SetText(std::__1::basic_string<unsigned short, base::string16_internals::string16_char_traits, std::__1::allocator<unsigned short> > const&) + 149 5 libchrome_dll.dylib 0x00000001111a3f3a IconLabelBubbleView::SetLabel(std::__1::basic_string<unsigned short, base::string16_internals::string16_char_traits, std::__1::allocator<unsigned short> > const&) + 26 6 libchrome_dll.dylib 0x00000001111a9aff LocationBarView::Layout() + 303 7 libviews.dylib 0x000000011f6d5b3e views::View::SetBounds(int, int, int, int) + 126 8 libchrome_dll.dylib 0x0000000111209acc ToolbarView::Layout() + 908 9 libviews.dylib 0x000000011f6d8732 views::View::Layout() + 386 10 libchrome_dll.dylib 0x0000000111195ef4 BrowserViewLayout::UpdateTopContainerBounds() + 244 11 libchrome_dll.dylib 0x0000000111195c20 BrowserViewLayout::Layout(views::View*) + 480 12 libviews.dylib 0x000000011f6d85ee views::View::Layout() + 62 13 libchrome_dll.dylib 0x000000011119316e non-virtual thunk to BrowserView::Layout() + 46 14 libviews.dylib 0x000000011f6fafc9 views::NonClientView::Layout() + 169 15 libviews.dylib 0x000000011f6d8732 views::View::Layout() + 386 16 libviews.dylib 0x000000011f6d6789 views::View::SetSize(gfx::Size const&) + 137 17 libviews.dylib 0x000000011f6eea88 views::Widget::OnNativeWidgetSizeChanged(gfx::Size const&) + 40 18 libviews.dylib 0x000000011f64edcb views::BridgedNativeWidget::OnSizeChanged() + 171 19 CoreFoundation 0x00007fff3c9e4098 __CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__ + 12 20 CoreFoundation 0x00007fff3c9e401c ___CFXRegistrationPost_block_invoke + 63 21 CoreFoundation 0x00007fff3c9e3f92 _CFXRegistrationPost + 397 22 CoreFoundation 0x00007fff3c9e3d0e ___CFXNotificationPost_block_invoke + 87 23 CoreFoundation 0x00007fff3c9ad123 -[_CFXNotificationRegistrar find:object:observer:enumerator:] + 1804 24 CoreFoundation 0x00007fff3c9ac2ad _CFXNotificationPost + 524 25 Foundation 0x00007fff3edc0e1f -[NSNotificationCenter postNotificationName:object:userInfo:] + 66 26 AppKit 0x00007fff3a020f1c -[NSWindow _setFrameCommon:display:stashSize:] + 3692 27 AppKit 0x00007fff3a020099 -[NSWindow _setFrame:display:allowImplicitAnimation:stashSize:] + 192 28 AppKit 0x00007fff3a01ffce -[NSWindow setFrame:display:] + 51 29 AppKit 0x00007fff3aa14751 -[NSWindow(NSWindowResizing) _resizeSetFrame:withEvent:] + 107 30 AppKit 0x00007fff3a268b51 -[NSWindow(NSWindowResizing) _resizeWithEvent:] + 3269 31 AppKit 0x00007fff3a78b788 -[NSTitledFrame attemptResizeWithEvent:] + 177 32 AppKit 0x00007fff3a17cb08 -[NSThemeFrame handleMouseDown:] + 297 33 AppKit 0x00007fff3a17ccfa -[NSThemeFrame mouseDown:] + 30 34 libchrome_dll.dylib 0x00000001110d1388 -[BrowserWindowFrame mouseDown:] + 184 35 AppKit 0x00007fff3a84fb39 -[NSWindow(NSEventRouting) _handleMouseDownEvent:isDelayedEvent:] + 5658 36 AppKit 0x00007fff3a84c91b -[NSWindow(NSEventRouting) _reallySendEvent:isDelayedEvent:] + 2319 37 AppKit 0x00007fff3a84bbdd -[NSWindow(NSEventRouting) sendEvent:] + 481 38 libviews.dylib 0x000000011f654a51 -[NativeWidgetMacNSWindow sendEvent:] + 161 39 AppKit 0x00007fff3a6b2650 -[NSApplication(NSEvent) sendEvent:] + 336 40 libchrome_dll.dylib 0x000000010fa7cabc __34-[BrowserCrApplication sendEvent:]_block_invoke + 172 41 libbase.dylib 0x00000001155d7f6a base::mac::CallWithEHFrame(void () block_pointer) + 10 42 libchrome_dll.dylib 0x000000010fa7c76d -[BrowserCrApplication sendEvent:] + 797 43 AppKit 0x00007fff39f6211d -[NSApplication run] + 755 44 libbase.dylib 0x00000001155f80bc base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate*) + 364 45 libbase.dylib 0x00000001155f630e base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*) + 110 46 libbase.dylib 0x00000001155f2124 base::MessageLoop::Run(bool) + 132 47 libbase.dylib 0x0000000115630709 base::RunLoop::Run() + 249 48 libchrome_dll.dylib 0x000000010fa8312b ChromeBrowserMainParts::MainMessageLoopRun(int*) + 267 49 libcontent.dylib 0x00000001197a1a34 content::BrowserMainLoop::RunMainMessageLoopParts() + 52 50 libcontent.dylib 0x00000001197a4a16 content::BrowserMainRunnerImpl::Run() + 102 51 libcontent.dylib 0x000000011979de64 content::BrowserMain(content::MainFunctionParams const&, std::__1::unique_ptr<content::BrowserProcessSubThread, std::__1::default_delete<content::BrowserProcessSubThread> >) + 276 52 libcontent.dylib 0x000000011a364443 content::RunBrowserProcessMain(content::MainFunctionParams const&, content::ContentMainDelegate*, std::__1::unique_ptr<content::BrowserProcessSubThread, std::__1::default_delete<content::BrowserProcessSubThread> >) + 195 53 libcontent.dylib 0x000000011a3653be content::ContentMainRunnerImpl::Run() + 686 54 libembedder.dylib 0x0000000115553de7 service_manager::Main(service_manager::MainParams const&) + 2855 55 libcontent.dylib 0x000000011a364354 content::ContentMain(content::ContentMainParams const&) + 68 56 libchrome_dll.dylib 0x000000010f3cb373 ChromeMain + 179 57 Chromium 0x000000010f352de5 main + 405 58 libdyld.dylib 0x00007fff6997fee1 start + 1 59 ??? 0x0000000000000001 0x0 + 1
,
Jun 8 2018
My reading of the stack in #7 is:
LocationBarView::Layout() does:
LocationIconView::SetLabel("");
...
if (GetLocationIconText())
LocationIconView::SetLabel(GetLocationIconText());
Which explains why this doesn't show up on about:blank - that page has no icon text, but data:text/html, has icon text ("Not secure"). Each call to SetLabel() causes the LocationIconView's Label to ResetLayout() because the text is different, which causes an InvalidateLayout(), which propagates back up to the BrowserView :(
,
Jun 8 2018
Also suspect: 0 libbase.dylib 0x00000001172b0c5c base::debug::StackTrace::StackTrace(unsigned long) + 28 1 libviews.dylib 0x000000011f99387f views::View::InvalidateLayout() + 191 2 libviews.dylib 0x000000011f990b4e views::View::SetBounds(int, int, int, int) + 126 3 libchrome_dll.dylib 0x0000000111451b5f BrowserViewLayout::Layout(views::View*) + 335 4 libviews.dylib 0x000000011f9935fe views::View::Layout() + 62 5 libchrome_dll.dylib 0x000000011144f13e non-virtual thunk to BrowserView::Layout() + 46 6 libviews.dylib 0x000000011f9b5f99 views::NonClientView::Layout() + 169 7 libviews.dylib 0x000000011f983ea8 views::FillLayout::Layout(views::View*) + 104 8 libviews.dylib 0x000000011f9935fe views::View::Layout() + 62 9 libviews.dylib 0x000000011f991799 views::View::SetSize(gfx::Size const&) + 137 10 libviews.dylib 0x000000011f9a9a58 views::Widget::OnNativeWidgetSizeChanged(gfx::Size const&) + 40 11 libviews.dylib 0x000000011f909ddb views::BridgedNativeWidget::OnSizeChanged() + 171 Here BrowserView::Layout() is leading into things that will re-invalidate its own layout (!). I suspect there are a lot of loops of this form. :(
,
Jun 8 2018
Me and Leonard have attained a state of deep puzzlement:
1) View::Layout() begins by setting needs_layout_ to false
2) Often, overrides of View::Layout() call SetBounds() on child views
3) Child views sometimes do this:
void Label::OnBoundsChanged(const gfx::Rect& previous_bounds) {
if (previous_bounds.size() != size())
InvalidateLayout();
}
4) InvalidateLayout() invalidates the parent view's layout:
void View::InvalidateLayout() {
// Always invalidate up. This is needed to handle the case of us already being
// valid, but not our parent.
needs_layout_ = true;
if (parent_)
parent_->InvalidateLayout();
}
Result: after View::Layout() is done, the parent's layout is still marked invalid :(.
The same syndrome obtains with AppMenuButton:
void ToolbarView::Layout() {
...
app_menu_button_->SetTrailingMargin(maximized ? end_padding : 0);
}
void BrowserAppMenuButton::SetTrailingMargin(int margin) {
margin_trailing_ = margin;
UpdateThemedBorder();
InvalidateLayout();
}
In general we have this antipattern of child ::InvalidateLayout being called from parent ::Layout. Perhaps we can DCHECK for that? :(
,
Jun 8 2018
In the spirit of a horror movie, I'd like to see a CL that put that DCHECK in just to see how red the trybots got...
,
Jun 8 2018
https://chromium-review.googlesource.com/c/chromium/src/+/1094034 has the DCHECK for layout during layout. How red will it get?
,
Jun 9 2018
That CL shows that if you DCHECK if layout is invalid at the end of layout, just about every browsertest, component browsertest, constrained window unit test, content browsertest, interactive ui test, Views-using unit test, and views_unittest fails.
,
Jun 11 2018
,
Jun 11 2018
I made a doc a while ago when looking into the profile switcher layout performance - https://docs.google.com/document/d/1t7MdfWLdTFbVVIrxTZJxYuy9VzzTFFWZHgOE3OH-q5A/edit#heading=h.w6y07tc35a1t - Issue 788597 is the associated bug (which is bubble-specific). However, the doc covers some general issues.
,
Jun 19 2018
,
Jun 22 2018
,
Jun 25 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f70f003f8d4c89129c923238170e194f87dd0d16 commit f70f003f8d4c89129c923238170e194f87dd0d16 Author: Leonard Grey <lgrey@chromium.org> Date: Mon Jun 25 16:43:37 2018 Views: more accurate layout tracing Consider two views: A and its child view B. The "Views::Layout" trace event for B is currently emitted when A calls B's Layout as part of A's Layout. This is less intuitive than calling it in B's Layout, but is better because it can capture an event for a B which doesn't call through to the base class Layout implementation in its Layout. Unfortunately, this still leaves many gaps: 1) Layout managers 2) Views whose *parents* don't call the base class implementation 3) Layouts triggered by bounds changes 4) Probably some more I can't think of right now. This change adds extra trace events to address #3 from the list above. Bug: 850128 , 835983 Change-Id: I135ac0e8b2c6b9e277dce6da06e9d0aeae7dfbd1 Reviewed-on: https://chromium-review.googlesource.com/1112466 Commit-Queue: Leonard Grey <lgrey@chromium.org> Reviewed-by: Robert Liao <robliao@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Cr-Commit-Position: refs/heads/master@{#570065} [modify] https://crrev.com/f70f003f8d4c89129c923238170e194f87dd0d16/ui/views/view.cc
,
Jun 26 2018
Verified the fix on Mac 10.13.1 using Chrome version #69.0.3473.0 as per the comment#0. Attaching screen cast for reference. Observed the Smooth resizing of any web page. Hence, the fix is working as expected. Adding the verified labels. Note: Able to reproduce the issue on chrome version 69.0.3451.0 Thanks...!!
,
Jun 26 2018
Re: c#19, the change in c#18 was diagnostic, not a fix.
,
Jun 26 2018
Updated title to more accurately reflect current state. The experiment that shows only a lock icon in the security chip makes this better in the common case, but it's still bad for non-HTTPS and similar.
,
Jun 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d1d86a9dc603cc93c3a8de10ccbc31e2ff561ab0 commit d1d86a9dc603cc93c3a8de10ccbc31e2ff561ab0 Author: Leonard Grey <lgrey@chromium.org> Date: Wed Jun 27 21:21:35 2018 Views: don't unnecessarily create temp label when sizing LocationIconView This change is a 15% resize speedup in a release build on my machine. Bug: 850128 Change-Id: Icbcd79b7ea5a8a5f06fc20595f67092b8bd12c42 Reviewed-on: https://chromium-review.googlesource.com/1110700 Reviewed-by: Tommy Li <tommycli@chromium.org> Reviewed-by: Robert Liao <robliao@chromium.org> Commit-Queue: Leonard Grey <lgrey@chromium.org> Cr-Commit-Position: refs/heads/master@{#570903} [modify] https://crrev.com/d1d86a9dc603cc93c3a8de10ccbc31e2ff561ab0/chrome/browser/ui/views/location_bar/location_icon_view.cc
,
Jun 28 2018
Issue 665280 has been merged into this issue.
,
Jul 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5acef1d366b62aca067f52f88e7e8428b65bc60a commit 5acef1d366b62aca067f52f88e7e8428b65bc60a Author: Leonard Grey <lgrey@chromium.org> Date: Mon Jul 02 14:13:41 2018 Call View::Layout at the *end* of Location Bar layout This reduces layout time per tick while resizing (with text in the security chip) from 56ms to 16 ms in a local release build. It seems fine visually, and all tests pass, so... Bug: 850128 Change-Id: I13b3441a174958c15c9d72bf4462c47819de0040 Reviewed-on: https://chromium-review.googlesource.com/1119210 Reviewed-by: Robert Liao <robliao@chromium.org> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Commit-Queue: Leonard Grey <lgrey@chromium.org> Cr-Commit-Position: refs/heads/master@{#571886} [modify] https://crrev.com/5acef1d366b62aca067f52f88e7e8428b65bc60a/chrome/browser/ui/views/location_bar/location_bar_view.cc
,
Jul 2
,
Jul 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/05c382ec0bb0e3ffc82b95566d97624bdab832b8 commit 05c382ec0bb0e3ffc82b95566d97624bdab832b8 Author: Leonard Grey <lgrey@chromium.org> Date: Tue Jul 03 16:01:10 2018 Views: only install focus ring once in page action view Bug: 850128 Change-Id: Ifc009ffebaddac9037b7e72175ba571f980ff83a Reviewed-on: https://chromium-review.googlesource.com/1124759 Commit-Queue: Leonard Grey <lgrey@chromium.org> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Cr-Commit-Position: refs/heads/master@{#572241} [modify] https://crrev.com/05c382ec0bb0e3ffc82b95566d97624bdab832b8/chrome/browser/ui/views/page_action/page_action_icon_view.cc
,
Jul 6
Between the above changes and ccameron@'s https://chromium-review.googlesource.com/c/chromium/src/+/1123456, this now spends ~6ms in layout so I'm declaring victory on the layout front.
,
Jul 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bd83c7a7b4e1b157b99c79139b54fb6437fe5c17 commit bd83c7a7b4e1b157b99c79139b54fb6437fe5c17 Author: Leonard Grey <lgrey@chromium.org> Date: Fri Jul 06 16:19:21 2018 Views: reduce non-client view frame/client view relayouts Currently, the NonClient view forces layout on its frame and client views after setting their bounds. Since setting bounds triggers a layout, this causes us to do twice the work each time (and since this is essentially the root, this is a lot of work). The original intention appears to be to trigger a paint for any changes that weren't covered by the bounds change. The second layout was mitigated against by no-oping OnBoundsChanged, but this is no longer effective, since the layout is triggered directly in SetBoundsRect and OnBoundsChanged is now just a subclass hook. This change removes the extra layout calls in favor of scheduling a paint. Bug: 850128 , 835983 Change-Id: Id041373d14d9ba88affd81dfa1863aad8e4b5ba0 Reviewed-on: https://chromium-review.googlesource.com/1102479 Commit-Queue: Leonard Grey <lgrey@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Reviewed-by: Robert Liao <robliao@chromium.org> Cr-Commit-Position: refs/heads/master@{#572985} [modify] https://crrev.com/bd83c7a7b4e1b157b99c79139b54fb6437fe5c17/ui/views/BUILD.gn [modify] https://crrev.com/bd83c7a7b4e1b157b99c79139b54fb6437fe5c17/ui/views/window/non_client_view.cc [modify] https://crrev.com/bd83c7a7b4e1b157b99c79139b54fb6437fe5c17/ui/views/window/non_client_view.h [add] https://crrev.com/bd83c7a7b4e1b157b99c79139b54fb6437fe5c17/ui/views/window/non_client_view_unittest.cc
,
Jul 12
|
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by sdy@chromium.org
, Jun 6 20185.0 MB
5.0 MB Download
450 KB
450 KB View Download