Commit 8e3cae91 authored by Dana Fried's avatar Dana Fried Committed by Commit Bot

Reland "Reland "Make PWA titlebar and icons flex and animate properly.""

This reverts commit ef8dbf91.

Reason for revert: Fixed tests that are still failing/flaky.

Some of the logic around the layout of the PWA window in the original
patch was not as deterministic as it should have been, which caused test
flakiness. This addresses the problem along with several related bugs.

One of the fixes in this CL is to return the final resize of the failing
test back to something equivalent to the logic prior to the original
version of the patch, which was clearly done in a particular way to
handle different layout pathways on different platforms.

Original change's description:
> Revert "Reland "Make PWA titlebar and icons flex and animate properly.""
>
> This reverts commit e490a00a.
>
> Reason for revert: Sheriff here:
> Consistent failure of Linux MSan Tests:
> https://ci.chromium.org/p/chromium/builders/ci/Linux%20MSan%20Tests
> First failed run:
> https://ci.chromium.org/p/chromium/builders/ci/Linux%20MSan%20Tests/22357
>
> Original change's description:
> > Reland "Make PWA titlebar and icons flex and animate properly."
> >
> > This reverts commit de7cd485.
> >
> > Reason for revert: Fixing issue with Mac tests.
> >
> > Original change's description:
> > > Revert "Make PWA titlebar and icons flex and animate properly."
> > >
> > > This reverts commit 8f2f4243.
> > >
> > > Reason for revert: speculative revert for test failures on Mac
> > > for WebAppFrameToolbarBrowserTest.SpaceConstrained, see e.g.
> > > https://ci.chromium.org/p/chromium/builders/ci/Mac10.12%20Tests/31700
> > >
> > > Original change's description:
> > > > Make PWA titlebar and icons flex and animate properly.
> > > >
> > > > Nested layouts now allocate available space properly to their
> > > > descendents. Also, we consistently use flex layout in the PWA titlebar.
> > > >
> > > > Removed a couple of invalid assumptions that were encoded as DCHECKs.
> > > >
> > > > There are still some inkdrop issues but they are purely cosmetic, see
> > > > crbug.com/1006162 for more detail.
> > > >
> > > > Bug: 1048061
> > > > Change-Id: I403b71b5e2bbcb2d461e1ed6c543c76f91d4c652
> > > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2036264
> > > > Commit-Queue: Dana Fried <dfried@chromium.org>
> > > > Reviewed-by: Peter Boström <pbos@chromium.org>
> > > > Cr-Commit-Position: refs/heads/master@{#738874}
> > >
> > > TBR=pbos@chromium.org,dfried@chromium.org
> > >
> > > Change-Id: I731bad118043652a6dbd082c6099ebbd0c697835
> > > No-Presubmit: true
> > > No-Tree-Checks: true
> > > No-Try: true
> > > Bug: 1048061
> > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2041615
> > > Reviewed-by: Mikel Astiz <mastiz@chromium.org>
> > > Commit-Queue: Mikel Astiz <mastiz@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#738907}
> >
> > TBR=pbos@chromium.org,mastiz@chromium.org,dfried@chromium.org
> >
> > Change-Id: Idb9712d1ffe45bf374af846278e26d61983a44f0
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Bug: 1048061
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2042326
> > Reviewed-by: Dana Fried <dfried@chromium.org>
> > Commit-Queue: Dana Fried <dfried@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#742203}
>
> TBR=pbos@chromium.org,mastiz@chromium.org,dfried@chromium.org
>
> Change-Id: If65099915ea0efa01de8cc9b0df0a7e9bf03a60b
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 1048061
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2062996
> Reviewed-by: Sergey Poromov <poromov@chromium.org>
> Commit-Queue: Sergey Poromov <poromov@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#742549}

TBR=pbos@chromium.org,poromov@chromium.org,mastiz@chromium.org,dfried@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1048061, 1050957, 1053899
Change-Id: I5e709a60ae2bd1331bb48687fa9af68d7833b9a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2067639Reviewed-by: default avatarDana Fried <dfried@chromium.org>
Commit-Queue: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743824}
parent 37d57f2e
...@@ -36,29 +36,44 @@ ExtensionsToolbarContainer::DropInfo::DropInfo( ...@@ -36,29 +36,44 @@ ExtensionsToolbarContainer::DropInfo::DropInfo(
size_t index) size_t index)
: action_id(action_id), index(index) {} : action_id(action_id), index(index) {}
ExtensionsToolbarContainer::ExtensionsToolbarContainer(Browser* browser) ExtensionsToolbarContainer::ExtensionsToolbarContainer(Browser* browser,
DisplayMode display_mode)
: ToolbarIconContainerView(/*uses_highlight=*/true), : ToolbarIconContainerView(/*uses_highlight=*/true),
browser_(browser), browser_(browser),
model_(ToolbarActionsModel::Get(browser_->profile())), model_(ToolbarActionsModel::Get(browser_->profile())),
model_observer_(this), model_observer_(this),
extensions_button_(new ExtensionsToolbarButton(browser_, this)) { extensions_button_(new ExtensionsToolbarButton(browser_, this)),
display_mode_(display_mode) {
// The container shouldn't show unless / until we have extensions available. // The container shouldn't show unless / until we have extensions available.
SetVisible(false); SetVisible(false);
model_observer_.Add(model_); model_observer_.Add(model_);
// Do not flip the Extensions icon in RTL. // Do not flip the Extensions icon in RTL.
extensions_button_->EnableCanvasFlippingForRTLUI(false); extensions_button_->EnableCanvasFlippingForRTLUI(false);
const views::FlexSpecification hide_icon_flex_specification =
views::FlexSpecification(views::MinimumFlexSizeRule::kPreferredSnapToZero,
views::MaximumFlexSizeRule::kPreferred)
.WithWeight(0);
switch (display_mode) {
case DisplayMode::kNormal:
// In normal mode, the menu icon is always shown.
extensions_button_->SetProperty(views::kFlexBehaviorKey, extensions_button_->SetProperty(views::kFlexBehaviorKey,
views::FlexSpecification()); views::FlexSpecification());
break;
case DisplayMode::kCompact:
// In compact mode, the menu icon can be hidden but has the highest
// priority.
extensions_button_->SetProperty(
views::kFlexBehaviorKey, hide_icon_flex_specification.WithOrder(1));
break;
}
extensions_button_->SetID(VIEW_ID_EXTENSIONS_MENU_BUTTON); extensions_button_->SetID(VIEW_ID_EXTENSIONS_MENU_BUTTON);
AddMainButton(extensions_button_); AddMainButton(extensions_button_);
target_layout_manager() target_layout_manager()
->SetFlexAllocationOrder(views::FlexAllocationOrder::kReverse) ->SetFlexAllocationOrder(views::FlexAllocationOrder::kReverse)
.SetDefault(views::kFlexBehaviorKey, .SetDefault(views::kFlexBehaviorKey,
views::FlexSpecification( hide_icon_flex_specification.WithOrder(3));
views::MinimumFlexSizeRule::kPreferredSnapToZero,
views::MaximumFlexSizeRule::kPreferred)
.WithWeight(0));
CreateActions(); CreateActions();
// TODO(pbos): Consider splitting out tab-strip observing into another class. // TODO(pbos): Consider splitting out tab-strip observing into another class.
...@@ -139,11 +154,28 @@ void ExtensionsToolbarContainer::UpdateIconVisibility( ...@@ -139,11 +154,28 @@ void ExtensionsToolbarContainer::UpdateIconVisibility(
// Popped out action uses a flex rule that causes it to always be visible // Popped out action uses a flex rule that causes it to always be visible
// regardless of space; default for actions is to drop out when there is // regardless of space; default for actions is to drop out when there is
// insufficient space. So if an action is being forced visible, it should have // insufficient space. So if an action is being forced visible, it should have
// the always-show flex rule, and if it not, it should not. // a rule that gives it higher priority, and if it does not, it should use the
// default.
const bool must_show = ShouldForceVisibility(extension_id); const bool must_show = ShouldForceVisibility(extension_id);
if (must_show) { if (must_show) {
switch (display_mode_) {
case DisplayMode::kNormal:
// In normal mode, the icon's visibility is forced.
action_view->SetProperty(views::kFlexBehaviorKey, action_view->SetProperty(views::kFlexBehaviorKey,
views::FlexSpecification()); views::FlexSpecification());
break;
case DisplayMode::kCompact:
// In compact mode, the icon can still drop out, but receives precedence
// over other actions.
action_view->SetProperty(
views::kFlexBehaviorKey,
views::FlexSpecification(
views::MinimumFlexSizeRule::kPreferredSnapToZero,
views::MaximumFlexSizeRule::kPreferred)
.WithWeight(0)
.WithOrder(2));
break;
}
} else { } else {
action_view->ClearProperty(views::kFlexBehaviorKey); action_view->ClearProperty(views::kFlexBehaviorKey);
} }
......
...@@ -38,7 +38,22 @@ class ExtensionsToolbarContainer : public ToolbarIconContainerView, ...@@ -38,7 +38,22 @@ class ExtensionsToolbarContainer : public ToolbarIconContainerView,
using ToolbarIconMap = std::map<ToolbarActionsModel::ActionId, using ToolbarIconMap = std::map<ToolbarActionsModel::ActionId,
std::unique_ptr<ToolbarActionView>>; std::unique_ptr<ToolbarActionView>>;
explicit ExtensionsToolbarContainer(Browser* browser); // Determines how the container displays - specifically whether the menu and
// popped out action can be hidden.
enum class DisplayMode {
// In normal mode, the menu icon and popped-out action is always visible.
// Normal mode is used for the main toolbar and in windows where there is
// always enough space to show at least two icons.
kNormal,
// In compact mode, one or both of the menu icon and popped-out action may
// be hidden. Compact mode is used in smaller windows (e.g. webapps) where
// there may not be enough space to display the buttons.
kCompact,
};
explicit ExtensionsToolbarContainer(
Browser* browser,
DisplayMode display_mode = DisplayMode::kNormal);
~ExtensionsToolbarContainer() override; ~ExtensionsToolbarContainer() override;
ExtensionsToolbarButton* extensions_button() const { ExtensionsToolbarButton* extensions_button() const {
...@@ -189,6 +204,7 @@ class ExtensionsToolbarContainer : public ToolbarIconContainerView, ...@@ -189,6 +204,7 @@ class ExtensionsToolbarContainer : public ToolbarIconContainerView,
ScopedObserver<ToolbarActionsModel, ToolbarActionsModel::Observer> ScopedObserver<ToolbarActionsModel, ToolbarActionsModel::Observer>
model_observer_; model_observer_;
ExtensionsToolbarButton* const extensions_button_; ExtensionsToolbarButton* const extensions_button_;
DisplayMode display_mode_;
// TODO(pbos): Create actions and icons only for pinned pinned / popped out // TODO(pbos): Create actions and icons only for pinned pinned / popped out
// actions (lazily). Currently code expects GetActionForId() to return // actions (lazily). Currently code expects GetActionForId() to return
......
...@@ -841,8 +841,8 @@ class WebAppNonClientFrameViewAshTest ...@@ -841,8 +841,8 @@ class WebAppNonClientFrameViewAshTest
} }
PageActionIconView* GetPageActionIcon(PageActionIconType type) { PageActionIconView* GetPageActionIcon(PageActionIconType type) {
return browser_view_->toolbar_button_provider() return browser_view_->toolbar_button_provider()->GetPageActionIconView(
->GetPageActionIconView(type); type);
} }
ContentSettingImageView* GrantGeolocationPermission() { ContentSettingImageView* GrantGeolocationPermission() {
...@@ -1044,6 +1044,10 @@ IN_PROC_BROWSER_TEST_P(WebAppNonClientFrameViewAshTest, ...@@ -1044,6 +1044,10 @@ IN_PROC_BROWSER_TEST_P(WebAppNonClientFrameViewAshTest,
SetUpWebApp(); SetUpWebApp();
ContentSettingImageView* geolocation_icon = GrantGeolocationPermission(); ContentSettingImageView* geolocation_icon = GrantGeolocationPermission();
// In order to receive focus, the geo icon must be laid out (and be both
// visible and nonzero size).
web_app_frame_toolbar_->Layout();
EXPECT_FALSE(web_app_menu_button_->HasFocus()); EXPECT_FALSE(web_app_menu_button_->HasFocus());
EXPECT_FALSE(geolocation_icon->HasFocus()); EXPECT_FALSE(geolocation_icon->HasFocus());
......
...@@ -437,7 +437,6 @@ int BrowserActionsContainer::GetHeightForWidth(int width) const { ...@@ -437,7 +437,6 @@ int BrowserActionsContainer::GetHeightForWidth(int width) const {
} }
gfx::Size BrowserActionsContainer::GetMinimumSize() const { gfx::Size BrowserActionsContainer::GetMinimumSize() const {
DCHECK(interactive_);
return gfx::Size(GetResizeAreaWidth(), return gfx::Size(GetResizeAreaWidth(),
toolbar_actions_bar_->GetViewSize().height()); toolbar_actions_bar_->GetViewSize().height());
} }
......
...@@ -127,14 +127,14 @@ IN_PROC_BROWSER_TEST_F(WebAppFrameToolbarBrowserTest, SpaceConstrained) { ...@@ -127,14 +127,14 @@ IN_PROC_BROWSER_TEST_F(WebAppFrameToolbarBrowserTest, SpaceConstrained) {
// Resize the WebAppFrameToolbarView just enough to clip out the page action // Resize the WebAppFrameToolbarView just enough to clip out the page action
// icons (and toolbar contents left of them). // icons (and toolbar contents left of them).
const int original_toolbar_width = web_app_frame_toolbar()->width(); const int original_toolbar_width = web_app_frame_toolbar()->width();
const int new_toolbar_width = toolbar_right_container->width() -
page_action_icon_container->bounds().right();
const int new_frame_width =
frame_view()->width() - original_toolbar_width + new_toolbar_width;
web_app_frame_toolbar()->SetSize( web_app_frame_toolbar()->SetSize(
gfx::Size(toolbar_right_container->width() - {new_toolbar_width, web_app_frame_toolbar()->height()});
page_action_icon_container->bounds().right(), frame_view()->SetSize({new_frame_width, frame_view()->height()});
web_app_frame_toolbar()->height()));
frame_view()->SetSize(gfx::Size(frame_view()->width() -
original_toolbar_width +
web_app_frame_toolbar()->width(),
frame_view()->height()));
// The left container (containing Back and Reload) should be hidden. // The left container (containing Back and Reload) should be hidden.
EXPECT_FALSE(toolbar_left_container->GetVisible()); EXPECT_FALSE(toolbar_left_container->GetVisible());
...@@ -144,9 +144,9 @@ IN_PROC_BROWSER_TEST_F(WebAppFrameToolbarBrowserTest, SpaceConstrained) { ...@@ -144,9 +144,9 @@ IN_PROC_BROWSER_TEST_F(WebAppFrameToolbarBrowserTest, SpaceConstrained) {
EXPECT_EQ(window_title->width(), 0); EXPECT_EQ(window_title->width(), 0);
#endif #endif
// The page action icons should be clipped to 0 width while the app menu // The page action icons should be hidden while the app menu button retains
// button retains its full width. // its full width.
EXPECT_EQ(page_action_icon_container->width(), 0); EXPECT_FALSE(page_action_icon_container->GetVisible());
EXPECT_EQ(menu_button->width(), original_menu_button_width); EXPECT_EQ(menu_button->width(), original_menu_button_width);
} }
......
...@@ -454,13 +454,10 @@ class WebAppFrameToolbarView::ToolbarButtonContainer ...@@ -454,13 +454,10 @@ class WebAppFrameToolbarView::ToolbarButtonContainer
return gfx::Size(views::View::CalculatePreferredSize().width(), return gfx::Size(views::View::CalculatePreferredSize().width(),
web_app_menu_button_->GetPreferredSize().height()); web_app_menu_button_->GetPreferredSize().height());
} }
void ChildPreferredSizeChanged(views::View* child) override { void ChildPreferredSizeChanged(views::View* child) override {
PreferredSizeChanged(); PreferredSizeChanged();
} }
void ChildVisibilityChanged(views::View* child) override {
// Changes to layout need to be taken into account by the toolbar view.
PreferredSizeChanged();
}
// BrowserActionsContainer::Delegate: // BrowserActionsContainer::Delegate:
views::LabelButton* GetOverflowReferenceView() override { views::LabelButton* GetOverflowReferenceView() override {
...@@ -548,15 +545,23 @@ WebAppFrameToolbarView::ToolbarButtonContainer::ToolbarButtonContainer( ...@@ -548,15 +545,23 @@ WebAppFrameToolbarView::ToolbarButtonContainer::ToolbarButtonContainer(
views::Widget* widget, views::Widget* widget,
BrowserView* browser_view) BrowserView* browser_view)
: browser_view_(browser_view) { : browser_view_(browser_view) {
views::BoxLayout& layout = views::FlexLayout* const layout =
*SetLayoutManager(std::make_unique<views::BoxLayout>( SetLayoutManager(std::make_unique<views::FlexLayout>());
views::BoxLayout::Orientation::kHorizontal, layout->SetOrientation(views::LayoutOrientation::kHorizontal)
gfx::Insets(0, WebAppFrameRightMargin()), .SetInteriorMargin(gfx::Insets(0, WebAppFrameRightMargin()))
HorizontalPaddingBetweenPageActionsAndAppMenuButtons())); .SetDefault(
// Right align to clip the leftmost items first when not enough space. views::kMarginsKey,
layout.set_main_axis_alignment(views::BoxLayout::MainAxisAlignment::kEnd); gfx::Insets(0,
layout.set_cross_axis_alignment( HorizontalPaddingBetweenPageActionsAndAppMenuButtons()))
views::BoxLayout::CrossAxisAlignment::kCenter); .SetCollapseMargins(true)
.SetIgnoreDefaultMainAxisMargins(true)
.SetCrossAxisAlignment(views::LayoutAlignment::kCenter)
.SetDefault(views::kFlexBehaviorKey,
views::FlexSpecification(
views::MinimumFlexSizeRule::kPreferredSnapToZero,
views::MaximumFlexSizeRule::kPreferred)
.WithWeight(0))
.SetFlexAllocationOrder(views::FlexAllocationOrder::kReverse);
const auto* app_controller = browser_view_->browser()->app_controller(); const auto* app_controller = browser_view_->browser()->app_controller();
...@@ -595,15 +600,31 @@ WebAppFrameToolbarView::ToolbarButtonContainer::ToolbarButtonContainer( ...@@ -595,15 +600,31 @@ WebAppFrameToolbarView::ToolbarButtonContainer::ToolbarButtonContainer(
views::SetHitTestComponent(page_action_icon_container_view_, views::SetHitTestComponent(page_action_icon_container_view_,
static_cast<int>(HTCLIENT)); static_cast<int>(HTCLIENT));
// Extensions toolbar area with pinned extensions is lower priority than, for
// example, the menu button or other toolbar buttons, and pinned extensions
// should hide before other toolbar buttons.
constexpr int kLowPriorityFlexOrder = 2;
if (base::FeatureList::IsEnabled(features::kExtensionsToolbarMenu)) { if (base::FeatureList::IsEnabled(features::kExtensionsToolbarMenu)) {
extensions_container_ = AddChildView( extensions_container_ =
std::make_unique<ExtensionsToolbarContainer>(browser_view_->browser())); AddChildView(std::make_unique<ExtensionsToolbarContainer>(
browser_view_->browser(),
ExtensionsToolbarContainer::DisplayMode::kCompact));
extensions_container_->SetProperty(
views::kFlexBehaviorKey,
views::FlexSpecification(
extensions_container_->animating_layout_manager()
->GetDefaultFlexRule())
.WithOrder(kLowPriorityFlexOrder));
views::SetHitTestComponent(extensions_container_, views::SetHitTestComponent(extensions_container_,
static_cast<int>(HTCLIENT)); static_cast<int>(HTCLIENT));
} else { } else {
browser_actions_container_ = browser_actions_container_ =
AddChildView(std::make_unique<BrowserActionsContainer>( AddChildView(std::make_unique<BrowserActionsContainer>(
browser_view_->browser(), nullptr, this, false /* interactive */)); browser_view_->browser(), nullptr, this, false /* interactive */));
browser_actions_container_->SetProperty(
views::kFlexBehaviorKey,
views::FlexSpecification(browser_actions_container_->GetFlexRule())
.WithOrder(kLowPriorityFlexOrder));
views::SetHitTestComponent(browser_actions_container_, views::SetHitTestComponent(browser_actions_container_,
static_cast<int>(HTCLIENT)); static_cast<int>(HTCLIENT));
} }
...@@ -624,6 +645,8 @@ WebAppFrameToolbarView::ToolbarButtonContainer::ToolbarButtonContainer( ...@@ -624,6 +645,8 @@ WebAppFrameToolbarView::ToolbarButtonContainer::ToolbarButtonContainer(
web_app_menu_button_->SetID(VIEW_ID_APP_MENU); web_app_menu_button_->SetID(VIEW_ID_APP_MENU);
const bool is_browser_focus_mode = browser_view_->browser()->is_focus_mode(); const bool is_browser_focus_mode = browser_view_->browser()->is_focus_mode();
SetInsetsForWebAppToolbarButton(web_app_menu_button_, is_browser_focus_mode); SetInsetsForWebAppToolbarButton(web_app_menu_button_, is_browser_focus_mode);
web_app_menu_button_->SetProperty(views::kFlexBehaviorKey,
views::FlexSpecification());
browser_view_->immersive_mode_controller()->AddObserver(this); browser_view_->immersive_mode_controller()->AddObserver(this);
scoped_widget_observer_.Add(widget); scoped_widget_observer_.Add(widget);
...@@ -694,7 +717,7 @@ WebAppFrameToolbarView::WebAppFrameToolbarView(views::Widget* widget, ...@@ -694,7 +717,7 @@ WebAppFrameToolbarView::WebAppFrameToolbarView(views::Widget* widget,
std::make_unique<ToolbarButtonContainer>(widget, browser_view)); std::make_unique<ToolbarButtonContainer>(widget, browser_view));
right_container_->SetProperty( right_container_->SetProperty(
views::kFlexBehaviorKey, views::kFlexBehaviorKey,
views::FlexSpecification(views::MinimumFlexSizeRule::kScaleToZero, views::FlexSpecification(views::MinimumFlexSizeRule::kScaleToMinimum,
views::MaximumFlexSizeRule::kPreferred) views::MaximumFlexSizeRule::kPreferred)
.WithOrder(1)); .WithOrder(1));
......
...@@ -468,7 +468,6 @@ void AnimatingLayoutManager::LayoutImpl() { ...@@ -468,7 +468,6 @@ void AnimatingLayoutManager::LayoutImpl() {
GetMainAxis(orientation(), current_layout_.host_size); GetMainAxis(orientation(), current_layout_.host_size);
if (current_main > host_main || if (current_main > host_main ||
(bounds_main && current_main > *bounds_main)) { (bounds_main && current_main > *bounds_main)) {
DCHECK(!bounds_main || *bounds_main >= host_main);
last_available_host_size_ = available_size; last_available_host_size_ = available_size;
ResetLayoutToSize(host_size); ResetLayoutToSize(host_size);
} else if (available_size != last_available_host_size_) { } else if (available_size != last_available_host_size_) {
......
...@@ -42,7 +42,8 @@ ProposedLayout FillLayout::CalculateProposedLayout( ...@@ -42,7 +42,8 @@ ProposedLayout FillLayout::CalculateProposedLayout(
for (View* child : host_view()->children()) { for (View* child : host_view()->children()) {
if (ShouldIncludeChild(child)) { if (ShouldIncludeChild(child)) {
layout.child_layouts.push_back( layout.child_layouts.push_back(
{child, child->GetVisible(), contents_bounds}); ChildLayout{child, child->GetVisible(), contents_bounds,
SizeBounds(contents_bounds.size())});
} }
} }
......
...@@ -12,6 +12,29 @@ ...@@ -12,6 +12,29 @@
namespace views { namespace views {
namespace {
// Adjusts |child_available_size| by adding the difference between the host
// view's size and the size available to it.
SizeBounds AdjustAvailableSizeForParentAvailableSize(
const View* host,
const SizeBounds& child_available_size) {
if (!host || !host->parent() || child_available_size == SizeBounds())
return child_available_size;
SizeBounds host_additional_size = host->parent()->GetAvailableSize(host);
host_additional_size.Enlarge(-host->width(), -host->height());
return SizeBounds(
child_available_size.width() && host_additional_size.width()
? *child_available_size.width() + *host_additional_size.width()
: child_available_size.width(),
child_available_size.height() && host_additional_size.height()
? *child_available_size.height() + *host_additional_size.height()
: child_available_size.height());
}
} // anonymous namespace
LayoutManagerBase::~LayoutManagerBase() = default; LayoutManagerBase::~LayoutManagerBase() = default;
gfx::Size LayoutManagerBase::GetPreferredSize(const View* host) const { gfx::Size LayoutManagerBase::GetPreferredSize(const View* host) const {
...@@ -44,8 +67,10 @@ SizeBounds LayoutManagerBase::GetAvailableSize(const View* host, ...@@ -44,8 +67,10 @@ SizeBounds LayoutManagerBase::GetAvailableSize(const View* host,
DCHECK_EQ(host_view_, host); DCHECK_EQ(host_view_, host);
if (cached_layout_size_) { if (cached_layout_size_) {
for (const auto& child_layout : cached_layout_.child_layouts) for (const auto& child_layout : cached_layout_.child_layouts)
if (child_layout.child_view == view) if (child_layout.child_view == view) {
return child_layout.available_size; return AdjustAvailableSizeForParentAvailableSize(
host, child_layout.available_size);
}
} }
return SizeBounds(); return SizeBounds();
} }
......
...@@ -36,9 +36,17 @@ class TestLayoutManagerBase : public LayoutManagerBase { ...@@ -36,9 +36,17 @@ class TestLayoutManagerBase : public LayoutManagerBase {
return included; return included;
} }
void OverrideProposedLayout(const ProposedLayout& forced_layout) {
forced_layout_ = forced_layout;
InvalidateHost(true);
}
// LayoutManagerBase: // LayoutManagerBase:
ProposedLayout CalculateProposedLayout( ProposedLayout CalculateProposedLayout(
const SizeBounds& size_bounds) const override { const SizeBounds& size_bounds) const override {
if (forced_layout_)
return *forced_layout_;
ProposedLayout layout; ProposedLayout layout;
layout.host_size.set_width( layout.host_size.set_width(
std::max(kMinimumSize.width(), std::max(kMinimumSize.width(),
...@@ -48,6 +56,10 @@ class TestLayoutManagerBase : public LayoutManagerBase { ...@@ -48,6 +56,10 @@ class TestLayoutManagerBase : public LayoutManagerBase {
size_bounds.height().value_or(kPreferredSize.height()))); size_bounds.height().value_or(kPreferredSize.height())));
return layout; return layout;
} }
private:
// If specified, will always return this layout.
base::Optional<ProposedLayout> forced_layout_;
}; };
// This layout layout lays out included child views in the upper-left of the // This layout layout lays out included child views in the upper-left of the
...@@ -632,17 +644,12 @@ class LayoutManagerBaseAvailableSizeTest : public testing::Test { ...@@ -632,17 +644,12 @@ class LayoutManagerBaseAvailableSizeTest : public testing::Test {
view_->SetLayoutManager(std::make_unique<TestLayoutManagerBase>()); view_->SetLayoutManager(std::make_unique<TestLayoutManagerBase>());
} }
void SetCachedLayout(const ProposedLayout& layout) {
layout_->set_cached_layout_size(layout.host_size);
layout_->set_cached_layout(layout);
}
View* view() { return view_.get(); } View* view() { return view_.get(); }
LayoutManagerBase* layout() { return layout_; } TestLayoutManagerBase* layout() { return layout_; }
private: private:
std::unique_ptr<View> view_; std::unique_ptr<View> view_;
LayoutManagerBase* layout_; TestLayoutManagerBase* layout_;
}; };
TEST_F(LayoutManagerBaseAvailableSizeTest, ReturnsCorrectValues) { TEST_F(LayoutManagerBaseAvailableSizeTest, ReturnsCorrectValues) {
...@@ -652,13 +659,98 @@ TEST_F(LayoutManagerBaseAvailableSizeTest, ReturnsCorrectValues) { ...@@ -652,13 +659,98 @@ TEST_F(LayoutManagerBaseAvailableSizeTest, ReturnsCorrectValues) {
View* const child2 = view()->AddChildView(std::make_unique<View>()); View* const child2 = view()->AddChildView(std::make_unique<View>());
View not_a_child; View not_a_child;
SetCachedLayout({{10, 10}, layout()->OverrideProposedLayout(
{{10, 10},
{{child1, true, {1, 1, 1, 1}, kChild1Bounds}, {{child1, true, {1, 1, 1, 1}, kChild1Bounds},
{child2, true, {2, 2, 2, 2}, kChild2Bounds}}}); {child2, true, {2, 2, 2, 2}, kChild2Bounds}}});
view()->SizeToPreferredSize();
EXPECT_EQ(kChild1Bounds, view()->GetAvailableSize(child1)); EXPECT_EQ(kChild1Bounds, view()->GetAvailableSize(child1));
EXPECT_EQ(kChild2Bounds, view()->GetAvailableSize(child2)); EXPECT_EQ(kChild2Bounds, view()->GetAvailableSize(child2));
EXPECT_EQ(SizeBounds(), view()->GetAvailableSize(&not_a_child)); EXPECT_EQ(SizeBounds(), view()->GetAvailableSize(&not_a_child));
} }
TEST_F(LayoutManagerBaseAvailableSizeTest, AvailableSizesInNestedValuesAdd) {
View* const child = view()->AddChildView(std::make_unique<View>());
View* const grandchild = child->AddChildView(std::make_unique<View>());
auto* const child_layout =
child->SetLayoutManager(std::make_unique<TestLayoutManagerBase>());
constexpr gfx::Size kViewSize(18, 17);
constexpr SizeBounds kChildAvailableSize(16, 15);
constexpr gfx::Size kChildSize(13, 12);
constexpr SizeBounds kGrandchildAvailableSize(10, 9);
constexpr gfx::Size kGrandchildSize(3, 2);
layout()->OverrideProposedLayout(
{kViewSize, {{child, true, {{3, 3}, kChildSize}, kChildAvailableSize}}});
child_layout->OverrideProposedLayout({kChildSize,
{{grandchild,
true,
{{2, 2}, kGrandchildSize},
kGrandchildAvailableSize}}});
view()->SizeToPreferredSize();
EXPECT_EQ(kChildAvailableSize, view()->GetAvailableSize(child));
SizeBounds expected;
expected.set_width(*kGrandchildAvailableSize.width() +
*kChildAvailableSize.width() - kChildSize.width());
expected.set_height(*kGrandchildAvailableSize.height() +
*kChildAvailableSize.height() - kChildSize.height());
EXPECT_EQ(expected, child->GetAvailableSize(grandchild));
}
TEST_F(LayoutManagerBaseAvailableSizeTest,
PartiallySpecifiedAvailableSizesInNestedLayoutsAddPartially) {
View* const child = view()->AddChildView(std::make_unique<View>());
View* const grandchild = child->AddChildView(std::make_unique<View>());
auto* const child_layout =
child->SetLayoutManager(std::make_unique<TestLayoutManagerBase>());
constexpr gfx::Size kViewSize(18, 17);
constexpr SizeBounds kChildAvailableSize(16, base::nullopt);
constexpr gfx::Size kChildSize(13, 12);
constexpr SizeBounds kGrandchildAvailableSize(10, 9);
constexpr gfx::Size kGrandchildSize(3, 2);
layout()->OverrideProposedLayout(
{kViewSize, {{child, true, {{3, 3}, kChildSize}, kChildAvailableSize}}});
child_layout->OverrideProposedLayout({kChildSize,
{{grandchild,
true,
{{2, 2}, kGrandchildSize},
kGrandchildAvailableSize}}});
view()->SizeToPreferredSize();
EXPECT_EQ(kChildAvailableSize, view()->GetAvailableSize(child));
SizeBounds expected;
expected.set_width(*kGrandchildAvailableSize.width() +
*kChildAvailableSize.width() - kChildSize.width());
expected.set_height(*kGrandchildAvailableSize.height());
EXPECT_EQ(expected, child->GetAvailableSize(grandchild));
}
TEST_F(LayoutManagerBaseAvailableSizeTest,
MismatchedAvailableSizesInNestedLayoutsDoNotAdd) {
View* const child = view()->AddChildView(std::make_unique<View>());
View* const grandchild = child->AddChildView(std::make_unique<View>());
auto* const child_layout =
child->SetLayoutManager(std::make_unique<TestLayoutManagerBase>());
constexpr gfx::Size kViewSize(18, 17);
constexpr SizeBounds kChildAvailableSize(16, base::nullopt);
constexpr gfx::Size kChildSize(13, 12);
constexpr SizeBounds kGrandchildAvailableSize(base::nullopt, 9);
constexpr gfx::Size kGrandchildSize(3, 2);
layout()->OverrideProposedLayout(
{kViewSize, {{child, true, {{3, 3}, kChildSize}, kChildAvailableSize}}});
child_layout->OverrideProposedLayout({kChildSize,
{{grandchild,
true,
{{2, 2}, kGrandchildSize},
kGrandchildAvailableSize}}});
view()->SizeToPreferredSize();
EXPECT_EQ(kChildAvailableSize, view()->GetAvailableSize(child));
EXPECT_EQ(kGrandchildAvailableSize, child->GetAvailableSize(grandchild));
}
} // namespace views } // namespace views
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