Views Bubbles set their bounds twice whenever they are created |
|||
Issue descriptionChrome Version : 64.0.3269.3 They should only set their bounds once... Otherwise it takes twice as long to create a bubble! The problem: - BubbleDialogDelegateView::CreateBubble() calls SizeToContents(), but - anon::CreateBubbleWidget(BubbleDialogDelegateView* bubble).. calls Widget::Init() without setting InitParams::bounds, which means - Widget::SetInitialBounds(..) will call NativeWidget::CenterWindow(non_client_view_->GetPreferredSize()); But centering on screen is never the right thing for anchored bubbles. Now _in theory_ this isn't terrible. SizeToContents() should only have to change the _origin_ of the window, and not the size. However, in practice, that isn't what happens. The size also changes between these calls (at least for the profile switcher bubble, which I'm looking at right now). That means a full layout of the bubble occurs twice, and the lag is quite noticeable. I think we should get rid of the `SizeToContents` call in BubbleDialogDelegateView::CreateBubble(), and set Widget::InitParams::bounds. One problem with that is that BubbleDialogDelegateView::GetBubbleBounds() depends on GetWidget() which doesn't exist yet. That should be fixable with some refactoring though. The tougher problem is that it's possible for bubbles' client views to change their mind about their size before and after being added to a Widget. Also, GetPreferredSize isn't guaranteed to be idempotent. But I think both of these constitute bugs. Maybe the first step is a DCHECK that the size doesn't actually change during bubble construction so we can smoke out specific dialogs that violate these requirements.
,
Nov 27 2017
Min sizes can let child bubbles shrink to fit better in small browser windows. I'm not sure that needs to be calculated during init though.
,
Dec 1 2017
I've made a doc with a bunch of bottlenecks I found while analysing the ProfileChooserView (context: Issue 789118 ). https://docs.google.com/document/d/1t7MdfWLdTFbVVIrxTZJxYuy9VzzTFFWZHgOE3OH-q5A/edit#heading=h.qb0azcbiko9q Most of these are generic to all views bubbles / layout. The doc is a bit rough, but I've been buried in data for 2 days and I needed to brain dump. For an idea of scope/magnitude the change at http://crbug.com/789118#c4 took the widget creation latency of the profile menu from a very perceptible ~240ms to a still-perceptible ~140ms (on a powerful MacPro, non-retina). Retina spends more time painting, but layout complexity should be comparable. Switching to RenderTextHarfBuzz takes it down to ~70ms, which is likely to be similar to what we'd see on a powerful Windows computer. I think that's still kinda bad. The doc links a bunch of CLs I'm exploring - comments welcome if you think any are dead-ends -- you might save me some time :).
,
Dec 14 2017
,
May 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/addb9888f3408ebbb6ad6709b01a412df8ccd4c8 commit addb9888f3408ebbb6ad6709b01a412df8ccd4c8 Author: Trent Apted <tapted@chromium.org> Date: Mon May 21 07:36:35 2018 Use unconstrained sizes in bubble *dialog* frames. Calling CalculatePreferredSize() can be expensive. The default BubbleFrameView implmementations of GetMinimumSize() and GetMaximumSize() invoke it, but popovers are not user-sizable, so there is no point calculating anything other than the preferred size. Popups also commonly change their preferred size without calling Widget::OnSizeConstraintsChanged() so the min/max sizes set will be wrong anyway. Note that BubbleFrameView is also used for (non-bubble) DialogDelegateView. These dialogs may be user-sizable, so that implementation is unchanged. Bug: 788597 Change-Id: I7473177eb3f0c9906c7a3191c776d2f36080cb26 Reviewed-on: https://chromium-review.googlesource.com/790175 Commit-Queue: Trent Apted <tapted@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Cr-Commit-Position: refs/heads/master@{#560240} [modify] https://crrev.com/addb9888f3408ebbb6ad6709b01a412df8ccd4c8/ui/views/bubble/bubble_dialog_delegate.cc [modify] https://crrev.com/addb9888f3408ebbb6ad6709b01a412df8ccd4c8/ui/views/bubble/bubble_dialog_delegate.h [modify] https://crrev.com/addb9888f3408ebbb6ad6709b01a412df8ccd4c8/ui/views/bubble/bubble_frame_view_unittest.cc [modify] https://crrev.com/addb9888f3408ebbb6ad6709b01a412df8ccd4c8/ui/views/bubble/tray_bubble_view.cc [modify] https://crrev.com/addb9888f3408ebbb6ad6709b01a412df8ccd4c8/ui/views/bubble/tray_bubble_view.h
,
Aug 21
Fixing invalid statuses (no owner but not marked as available). |
|||
►
Sign in to add a comment |
|||
Comment 1 by tapted@chromium.org
, Nov 27 2017Status: Started (was: Available)
2.5 KB
2.5 KB View Download
2.6 KB
2.6 KB View Download