New layout manager for toolbar, tab handles, bookmark, and really all of Chrome. |
||||
Issue descriptionChrome Version: 72 OS: all desktop We would like a single layout manager that can handle existing custom layouts (listed above) and also most of the cases where BoxLayout and GridLayout are used in the existing code (especially when GridLayout is being used to lay out a single column or row of content and we don't actually need a grid). Design (internal only): https://docs.google.com/document/d/1sOcsB2smt4pBzPgwAAtlh5CM4kSSYDxW4IIjx1aO010
,
Oct 25
,
Oct 25
Unmarking R-V-G. While there is an internal only doc mentioned, it doesn't mean this issue need to be R-V-G.
,
Dec 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6f5f5a4da5ef596fdc83abc181090e7ce2b8f415 commit 6f5f5a4da5ef596fdc83abc181090e7ce2b8f415 Author: Dana Fried <dfried@chromium.org> Date: Fri Dec 07 21:59:11 2018 View::SetVisible() notifies layout manager. Explicit calls to View::SetVisible() outside of LayoutManager::Layout() will cause the view's parent's layout manager (if there is one) to be notified. This way, we can treat someone calling SetVisible() as a state change for the layout manager, perpendicular to any changes the layout manager itself makes to the visibility of child views (for example, if it decides to hide them because there is not enough space to display them). Also, reordering child views invalidates layout. It should always have. Bug: 898632 Change-Id: I47382e5efe348b63a7dda7b6e493f55ac5bd74fb Reviewed-on: https://chromium-review.googlesource.com/c/1368274 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Dana Fried <dfried@chromium.org> Cr-Commit-Position: refs/heads/master@{#614832} [modify] https://crrev.com/6f5f5a4da5ef596fdc83abc181090e7ce2b8f415/ui/views/layout/layout_manager.cc [modify] https://crrev.com/6f5f5a4da5ef596fdc83abc181090e7ce2b8f415/ui/views/layout/layout_manager.h [modify] https://crrev.com/6f5f5a4da5ef596fdc83abc181090e7ce2b8f415/ui/views/view.cc
,
Dec 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9367b837898b8e6960a6c916436094d0d9977b9b commit 9367b837898b8e6960a6c916436094d0d9977b9b Author: Dana Fried <dfried@chromium.org> Date: Mon Dec 10 19:59:23 2018 Base CL for new near-universal "flex layout". As per previously discussed design: https://docs.google.com/document/d/1sOcsB2smt4pBzPgwAAtlh5CM4kSSYDxW4IIjx1aO010 Has unit tests and example based on the existing BoxLayout example. Open questions: - Do we want to put the new layout stuff in its own namespace (views::layout)? Followup CLs in the pipe: - Use FlexLayout for toolbar layout, vastly simplifying toolbar_view.cc - Add concept of "overflow" child views that appear conditionally. - Use FlexLayout for tab handles, vastly simplifying tab.cc Future work: - Use layout manager for bookmarks bar instead of bespoke layout. - Replace existing uses of BoxLayout and GridLayout that lay out a single row or column with the new, unified layout manager. - Rename TOOLBAR_STANDARD_SPACING to something sensible. - Fix View::GetContentsBounds() to actually call GetInsets(). - Remove ToolbarButton::SetLayoutInsetDelta() and make logic around switching to/from touch UI sane. - Enhance Views properties system to allow for larger value types (insets, flex rules, etc.) without having to explicitly store pointers. Bug: 898632 Change-Id: I38698b01dc5afdd143ba350f2c53f1062baec3a3 Reviewed-on: https://chromium-review.googlesource.com/c/1331040 Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Allen Bauer <kylixrd@chromium.org> Commit-Queue: Dana Fried <dfried@chromium.org> Cr-Commit-Position: refs/heads/master@{#615207} [modify] https://crrev.com/9367b837898b8e6960a6c916436094d0d9977b9b/ui/views/BUILD.gn [modify] https://crrev.com/9367b837898b8e6960a6c916436094d0d9977b9b/ui/views/examples/BUILD.gn [modify] https://crrev.com/9367b837898b8e6960a6c916436094d0d9977b9b/ui/views/examples/examples_window.cc [add] https://crrev.com/9367b837898b8e6960a6c916436094d0d9977b9b/ui/views/examples/flex_layout_example.cc [add] https://crrev.com/9367b837898b8e6960a6c916436094d0d9977b9b/ui/views/examples/flex_layout_example.h [add] https://crrev.com/9367b837898b8e6960a6c916436094d0d9977b9b/ui/views/layout/flex_layout.cc [add] https://crrev.com/9367b837898b8e6960a6c916436094d0d9977b9b/ui/views/layout/flex_layout.h [add] https://crrev.com/9367b837898b8e6960a6c916436094d0d9977b9b/ui/views/layout/flex_layout_types.cc [add] https://crrev.com/9367b837898b8e6960a6c916436094d0d9977b9b/ui/views/layout/flex_layout_types.h [add] https://crrev.com/9367b837898b8e6960a6c916436094d0d9977b9b/ui/views/layout/flex_layout_types_internal.cc [add] https://crrev.com/9367b837898b8e6960a6c916436094d0d9977b9b/ui/views/layout/flex_layout_types_internal.h [add] https://crrev.com/9367b837898b8e6960a6c916436094d0d9977b9b/ui/views/layout/flex_layout_unittest.cc [modify] https://crrev.com/9367b837898b8e6960a6c916436094d0d9977b9b/ui/views/view_properties.cc [modify] https://crrev.com/9367b837898b8e6960a6c916436094d0d9977b9b/ui/views/view_properties.h
,
Dec 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a commit d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a Author: Dana Fried <dfried@chromium.org> Date: Tue Dec 11 01:45:33 2018 Move toolbar to new FlexLayout, demonstrating new layout manager. Vastly simplifies layout in the toolbar and makes it mostly declarative with a single init function, rather than complex, bespoke - and sometimes incorrect - Layout(), GetMinimumSize(), etc. methods. Because of a logical change in the way the UI works when an extension is uninstalled, we uncovered a possible read-after-free in the extension install UI (shows up in some ASAN tests). So we plumbed the scoped_refptr through from crx_installer to the UI so that the memory would be valid until the bubble is closed. See parent CL for FlexLayout details and future work. Bug: 898632 Change-Id: I2cfe5b6119dc2886f88ae12675f8184cf90d56fd Reviewed-on: https://chromium-review.googlesource.com/c/1331041 Commit-Queue: Dana Fried <dfried@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Reviewed-by: Bret Sepulveda <bsep@chromium.org> Cr-Commit-Position: refs/heads/master@{#615372} [modify] https://crrev.com/d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a/chrome/browser/extensions/crx_installer.cc [modify] https://crrev.com/d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a/chrome/browser/extensions/crx_installer_browsertest.cc [modify] https://crrev.com/d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a/chrome/browser/extensions/extension_install_prompt.cc [modify] https://crrev.com/d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a/chrome/browser/extensions/extension_install_prompt.h [modify] https://crrev.com/d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a/chrome/browser/ui/extensions/extension_install_ui_default.cc [modify] https://crrev.com/d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a/chrome/browser/ui/extensions/extension_install_ui_default.h [modify] https://crrev.com/d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a/chrome/browser/ui/extensions/extension_installed_bubble.cc [modify] https://crrev.com/d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a/chrome/browser/ui/extensions/extension_installed_bubble.h [modify] https://crrev.com/d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a/chrome/browser/ui/toolbar/toolbar_actions_bar.cc [modify] https://crrev.com/d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a/chrome/browser/ui/views/location_bar/location_bar_view.cc [modify] https://crrev.com/d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc [modify] https://crrev.com/d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a/chrome/browser/ui/views/toolbar/browser_actions_container.cc [modify] https://crrev.com/d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a/chrome/browser/ui/views/toolbar/browser_actions_container.h [modify] https://crrev.com/d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a/chrome/browser/ui/views/toolbar/browser_app_menu_button.cc [modify] https://crrev.com/d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a/chrome/browser/ui/views/toolbar/browser_app_menu_button.h [modify] https://crrev.com/d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a/chrome/browser/ui/views/toolbar/toolbar_button.cc [modify] https://crrev.com/d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a/chrome/browser/ui/views/toolbar/toolbar_button.h [modify] https://crrev.com/d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a/chrome/browser/ui/views/toolbar/toolbar_view.cc [modify] https://crrev.com/d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a/chrome/browser/ui/views/toolbar/toolbar_view.h [modify] https://crrev.com/d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a/chrome/test/data/extensions/api_test/window_update/sizing/test.js [modify] https://crrev.com/d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a/extensions/browser/install/extension_install_ui.h
,
Dec 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/043c95f211872d0739ed6d94e8c54ca2e29757cc commit 043c95f211872d0739ed6d94e8c54ca2e29757cc Author: Friedrich Horschig [CET] <fhorschig@chromium.org> Date: Tue Dec 11 08:49:32 2018 Revert "Move toolbar to new FlexLayout, demonstrating new layout manager." This reverts commit d080cfa6b2816045a60cc2d16d2bf67ff1c15d8a. Reason for revert: Suspected culprit for ExtensionApiTabTest.UpdateWindowResize failure on network_service_browser_tests (failing on chromium.mac/Mac10.12 Tests). Bug: 913848 Original change's description: > Move toolbar to new FlexLayout, demonstrating new layout manager. > > Vastly simplifies layout in the toolbar and makes it mostly declarative with a > single init function, rather than complex, bespoke - and sometimes incorrect - > Layout(), GetMinimumSize(), etc. methods. > > Because of a logical change in the way the UI works when an extension is > uninstalled, we uncovered a possible read-after-free in the extension install > UI (shows up in some ASAN tests). So we plumbed the scoped_refptr through from > crx_installer to the UI so that the memory would be valid until the bubble is > closed. > > See parent CL for FlexLayout details and future work. > > Bug: 898632 > Change-Id: I2cfe5b6119dc2886f88ae12675f8184cf90d56fd > Reviewed-on: https://chromium-review.googlesource.com/c/1331041 > Commit-Queue: Dana Fried <dfried@chromium.org> > Reviewed-by: Devlin <rdevlin.cronin@chromium.org> > Reviewed-by: Bret Sepulveda <bsep@chromium.org> > Cr-Commit-Position: refs/heads/master@{#615372} TBR=rdevlin.cronin@chromium.org,bsep@chromium.org,dfried@chromium.org Change-Id: Ie93f170c2c6048072820f2d6dc9950f75bc4aa94 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 898632 Reviewed-on: https://chromium-review.googlesource.com/c/1371384 Reviewed-by: Friedrich Horschig [CET] <fhorschig@chromium.org> Commit-Queue: Friedrich Horschig [CET] <fhorschig@chromium.org> Cr-Commit-Position: refs/heads/master@{#615465} [modify] https://crrev.com/043c95f211872d0739ed6d94e8c54ca2e29757cc/chrome/browser/extensions/crx_installer.cc [modify] https://crrev.com/043c95f211872d0739ed6d94e8c54ca2e29757cc/chrome/browser/extensions/crx_installer_browsertest.cc [modify] https://crrev.com/043c95f211872d0739ed6d94e8c54ca2e29757cc/chrome/browser/extensions/extension_install_prompt.cc [modify] https://crrev.com/043c95f211872d0739ed6d94e8c54ca2e29757cc/chrome/browser/extensions/extension_install_prompt.h [modify] https://crrev.com/043c95f211872d0739ed6d94e8c54ca2e29757cc/chrome/browser/ui/extensions/extension_install_ui_default.cc [modify] https://crrev.com/043c95f211872d0739ed6d94e8c54ca2e29757cc/chrome/browser/ui/extensions/extension_install_ui_default.h [modify] https://crrev.com/043c95f211872d0739ed6d94e8c54ca2e29757cc/chrome/browser/ui/extensions/extension_installed_bubble.cc [modify] https://crrev.com/043c95f211872d0739ed6d94e8c54ca2e29757cc/chrome/browser/ui/extensions/extension_installed_bubble.h [modify] https://crrev.com/043c95f211872d0739ed6d94e8c54ca2e29757cc/chrome/browser/ui/toolbar/toolbar_actions_bar.cc [modify] https://crrev.com/043c95f211872d0739ed6d94e8c54ca2e29757cc/chrome/browser/ui/views/location_bar/location_bar_view.cc [modify] https://crrev.com/043c95f211872d0739ed6d94e8c54ca2e29757cc/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc [modify] https://crrev.com/043c95f211872d0739ed6d94e8c54ca2e29757cc/chrome/browser/ui/views/toolbar/browser_actions_container.cc [modify] https://crrev.com/043c95f211872d0739ed6d94e8c54ca2e29757cc/chrome/browser/ui/views/toolbar/browser_actions_container.h [modify] https://crrev.com/043c95f211872d0739ed6d94e8c54ca2e29757cc/chrome/browser/ui/views/toolbar/browser_app_menu_button.cc [modify] https://crrev.com/043c95f211872d0739ed6d94e8c54ca2e29757cc/chrome/browser/ui/views/toolbar/browser_app_menu_button.h [modify] https://crrev.com/043c95f211872d0739ed6d94e8c54ca2e29757cc/chrome/browser/ui/views/toolbar/toolbar_button.cc [modify] https://crrev.com/043c95f211872d0739ed6d94e8c54ca2e29757cc/chrome/browser/ui/views/toolbar/toolbar_button.h [modify] https://crrev.com/043c95f211872d0739ed6d94e8c54ca2e29757cc/chrome/browser/ui/views/toolbar/toolbar_view.cc [modify] https://crrev.com/043c95f211872d0739ed6d94e8c54ca2e29757cc/chrome/browser/ui/views/toolbar/toolbar_view.h [modify] https://crrev.com/043c95f211872d0739ed6d94e8c54ca2e29757cc/chrome/test/data/extensions/api_test/window_update/sizing/test.js [modify] https://crrev.com/043c95f211872d0739ed6d94e8c54ca2e29757cc/extensions/browser/install/extension_install_ui.h
,
Dec 11
,
Dec 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/06eb57247bf2a8de3b5526bb86c879166abf9b19 commit 06eb57247bf2a8de3b5526bb86c879166abf9b19 Author: Dana Fried <dfried@chromium.org> Date: Tue Dec 11 22:23:39 2018 Fix overlapping windows in drag test. Does not currently affect the test, but the minimum size reported by the toolbar is incorrect (see issue #842239 ). If the display is small and the minimum window width grows at all, the target point for the drag operation ends up still in the first (and focused) window, so the test fails. This change forces the endpoint of the drag onto the second window, and raises an error if it cannot do so. Bug: 898632 Change-Id: If4ee828e3309de247b1940a6b7f8ea98cffd5063 Reviewed-on: https://chromium-review.googlesource.com/c/1372711 Reviewed-by: Bret Sepulveda <bsep@chromium.org> Commit-Queue: Dana Fried <dfried@chromium.org> Cr-Commit-Position: refs/heads/master@{#615690} [modify] https://crrev.com/06eb57247bf2a8de3b5526bb86c879166abf9b19/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
,
Dec 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c3c302252c649c7878074d9d131d2f249276de36 commit c3c302252c649c7878074d9d131d2f249276de36 Author: Dana Fried <dfried@chromium.org> Date: Tue Dec 11 23:34:56 2018 Pass refptr through to extension bubble to prevent RAF. Current code does not cause RAF in tests, but changes we want to make for this bug uncovers a potential order-of-operations issue that can cause read of uninitialized memory (specifically, the extension can be programmatically deleted while the "extension installed" bubble is visible). Bug: 898632 Change-Id: I4bdf9e5fbb8fac1bc1a30a0b2a629655a77de9e7 Reviewed-on: https://chromium-review.googlesource.com/c/1372276 Reviewed-by: Bret Sepulveda <bsep@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Commit-Queue: Dana Fried <dfried@chromium.org> Cr-Commit-Position: refs/heads/master@{#615722} [modify] https://crrev.com/c3c302252c649c7878074d9d131d2f249276de36/chrome/browser/extensions/crx_installer.cc [modify] https://crrev.com/c3c302252c649c7878074d9d131d2f249276de36/chrome/browser/extensions/crx_installer_browsertest.cc [modify] https://crrev.com/c3c302252c649c7878074d9d131d2f249276de36/chrome/browser/extensions/extension_install_prompt.cc [modify] https://crrev.com/c3c302252c649c7878074d9d131d2f249276de36/chrome/browser/extensions/extension_install_prompt.h [modify] https://crrev.com/c3c302252c649c7878074d9d131d2f249276de36/chrome/browser/ui/extensions/extension_install_ui_default.cc [modify] https://crrev.com/c3c302252c649c7878074d9d131d2f249276de36/chrome/browser/ui/extensions/extension_install_ui_default.h [modify] https://crrev.com/c3c302252c649c7878074d9d131d2f249276de36/chrome/browser/ui/extensions/extension_installed_bubble.cc [modify] https://crrev.com/c3c302252c649c7878074d9d131d2f249276de36/chrome/browser/ui/extensions/extension_installed_bubble.h [modify] https://crrev.com/c3c302252c649c7878074d9d131d2f249276de36/extensions/browser/install/extension_install_ui.h
,
Dec 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b692e77fcca4b5dd70c5362f34214cbebe5723bb commit b692e77fcca4b5dd70c5362f34214cbebe5723bb Author: Dana Fried <dfried@chromium.org> Date: Tue Dec 11 23:43:32 2018 Fix order of operations in undo action popout. Current code does not cause a problem with this order of operations, but changes we want to make for the attached bug uncovers a situation where the action is still popped out after its view is removed, which causes an error. Bug: 898632 Change-Id: I73d14a61cddd4b052c9dbf774864f68036a93b4c Reviewed-on: https://chromium-review.googlesource.com/c/1372710 Reviewed-by: Bret Sepulveda <bsep@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Commit-Queue: Dana Fried <dfried@chromium.org> Cr-Commit-Position: refs/heads/master@{#615726} [modify] https://crrev.com/b692e77fcca4b5dd70c5362f34214cbebe5723bb/chrome/browser/ui/toolbar/toolbar_actions_bar.cc
,
Dec 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9f1da68eb596d1eb1992c9cd910f2567375a69e1 commit 9f1da68eb596d1eb1992c9cd910f2567375a69e1 Author: Dana Fried <dfried@chromium.org> Date: Wed Dec 12 18:21:29 2018 Add PreferredSizeChanged() calls. When a view changes size, it's important to call PreferredSizeChanged() so that its parent knows it might need to re-layout or repaint. The current code either fails to call PreferredSizeChanged() or instead lays out the current view in a couple of cases relevant to the toolbar, preventing us from using a consistent layout manager. This CL is a prerequisite for actually switching to a layout manager for the toolbar. Bug: 898632 Change-Id: I7154eb6ac58a11eca9069b25655a6c2666a704e0 Reviewed-on: https://chromium-review.googlesource.com/c/1372715 Commit-Queue: Dana Fried <dfried@chromium.org> Reviewed-by: Bret Sepulveda <bsep@chromium.org> Cr-Commit-Position: refs/heads/master@{#615969} [modify] https://crrev.com/9f1da68eb596d1eb1992c9cd910f2567375a69e1/chrome/browser/ui/views/location_bar/location_bar_view.cc [modify] https://crrev.com/9f1da68eb596d1eb1992c9cd910f2567375a69e1/chrome/browser/ui/views/toolbar/browser_actions_container.cc
,
Dec 18
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c77b0235fe8e67c60160fadcf5ce4c40b0563343 commit c77b0235fe8e67c60160fadcf5ce4c40b0563343 Author: Dana Fried <dfried@chromium.org> Date: Mon Dec 17 23:58:56 2018 Move toolbar to flex layout. This demonstrates the new FlexLayout, which we eventually hope to replace most of the custom layout code in top chrome as well as many instances of BoxLayout and GridLayout (in the case of a single row or column). We expect a small number of tests to break due to changes in how minimum size is reported (this also fixes issue #842239 ), so expect some iteration - a previous attempt to submit this CL resulted in the regression described in issue #913848 . Bug: 898632 Change-Id: Ieda3ad3a02abe17c8890ffc65610481903cea4a0 Reviewed-on: https://chromium-review.googlesource.com/c/1372748 Commit-Queue: Dana Fried <dfried@chromium.org> Reviewed-by: Bret Sepulveda <bsep@chromium.org> Cr-Commit-Position: refs/heads/master@{#617299} [modify] https://crrev.com/c77b0235fe8e67c60160fadcf5ce4c40b0563343/chrome/browser/ui/views/toolbar/browser_actions_container.cc [modify] https://crrev.com/c77b0235fe8e67c60160fadcf5ce4c40b0563343/chrome/browser/ui/views/toolbar/browser_actions_container.h [modify] https://crrev.com/c77b0235fe8e67c60160fadcf5ce4c40b0563343/chrome/browser/ui/views/toolbar/browser_app_menu_button.cc [modify] https://crrev.com/c77b0235fe8e67c60160fadcf5ce4c40b0563343/chrome/browser/ui/views/toolbar/browser_app_menu_button.h [modify] https://crrev.com/c77b0235fe8e67c60160fadcf5ce4c40b0563343/chrome/browser/ui/views/toolbar/toolbar_button.cc [modify] https://crrev.com/c77b0235fe8e67c60160fadcf5ce4c40b0563343/chrome/browser/ui/views/toolbar/toolbar_button.h [modify] https://crrev.com/c77b0235fe8e67c60160fadcf5ce4c40b0563343/chrome/browser/ui/views/toolbar/toolbar_view.cc [modify] https://crrev.com/c77b0235fe8e67c60160fadcf5ce4c40b0563343/chrome/browser/ui/views/toolbar/toolbar_view.h
,
Dec 18
Looks like https://chromium-review.googlesource.com/c/1372748 broke PWA window title bar layout. Repro steps: 1. Visit watery-shingle.glitch.me. 2. Menu > Install Watery Shingle.
,
Dec 18
I'm having an unsuccessful non-committal go at converting HostedAppButtonContainer to use FlexLayout. I'm noticing that Layout() is calling itself recursively for the same HostedAppButtonContainer object which is concerning. This was not previously the case with BoxLayout. #0 0x7fec36cc864f base::debug::StackTrace::StackTrace() #1 0x7fec36cc8131 base::debug::(anonymous namespace)::StackDumpSignalHandler() #2 0x7fec2a3ff0c0 <unknown> #3 0x7fec312c8049 views::View::SetBoundsRect() #4 0x7fec312bae0d views::FlexLayout::Layout() #5 0x7fec312cade0 views::View::Layout() #6 0x7fec312c80b7 views::View::SetBoundsRect() #7 0x7fec312c800e views::View::SetBounds() #8 0x563bca0e5688 HostedAppButtonContainer::LayoutInContainer() #9 0x563bca1693fc OpaqueBrowserFrameViewLayout::LayoutTitleBar() #10 0x563bca169efd OpaqueBrowserFrameViewLayout::Layout() #11 0x7fec312cade0 views::View::Layout() #12 0x7fec312cedce views::View::PreferredSizeChanged() #13 0x7fec312c9d59 views::View::SetVisible() #14 0x7fec312c26e9 views::LayoutManager::SetViewVisibility() #15 0x7fec312badba views::FlexLayout::Layout() #16 0x7fec312cade0 views::View::Layout() #17 0x7fec312c80b7 views::View::SetBoundsRect() #18 0x7fec312c800e views::View::SetBounds() #19 0x563bca0e5688 HostedAppButtonContainer::LayoutInContainer() #20 0x563bca1693fc OpaqueBrowserFrameViewLayout::LayoutTitleBar() #21 0x563bca169efd OpaqueBrowserFrameViewLayout::Layout() #22 0x7fec312cade0 views::View::Layout() #23 0x7fec312cedce views::View::PreferredSizeChanged() #24 0x7fec312c9d59 views::View::SetVisible() #25 0x7fec312c26e9 views::LayoutManager::SetViewVisibility() #26 0x7fec312badba views::FlexLayout::Layout() #27 0x7fec312cade0 views::View::Layout() #28 0x7fec312c8dd2 views::View::BoundsChanged() #29 0x7fec312c8178 views::View::SetBoundsRect() #30 0x7fec312c800e views::View::SetBounds() #31 0x563bca0e5688 HostedAppButtonContainer::LayoutInContainer() #32 0x563bca1693fc OpaqueBrowserFrameViewLayout::LayoutTitleBar() #33 0x563bca169efd OpaqueBrowserFrameViewLayout::Layout() #34 0x7fec312cade0 views::View::Layout() #35 0x563bca166665 OpaqueBrowserFrameView::UpdateWindowTitle() #36 0x7fec312d8d46 views::Widget::Init() #37 0x563bc9ffc8c8 BrowserFrame::InitBrowserFrame() #38 0x563bca032bbb BrowserWindow::CreateBrowserWindow() #39 0x563bc9e84f61 Browser::Browser() #40 0x563bca17319b CreateApplicationWindow()
,
Dec 18
alancutter - this is a known regression; see issue #916106 I'd love to work with you to figure out why this loop is happening. There are definitely some cases in the code where views are laying out themselves, or changing their own preferred size or position on layout, which creates a problem. It would be awesome to get a stack where it printed out the GetClassName() of each view in that sequence so we'd know what was causing the loop (since we don't have RTTI). It's possible there's some self-interaction I just haven't seen with any of the views I've tried FlexLayout on, too, which might warrant changes in which events get triggered (or ignored) by the layout manager.
,
Dec 19
Opened https://bugs.chromium.org/p/chromium/issues/detail?id=916394 to track migrating HostedAppButtonContainer to FlexLayout. |
||||
►
Sign in to add a comment |
||||
Comment 1 by dfried@chromium.org
, Oct 24