Commit a078d9ff authored by Dana Fried's avatar Dana Fried Committed by Commit Bot

Revert "Layout invalidates when child view's visibility changes.

"

This reverts commit 9932d106.

Reason for revert: Broke a test with insufficient testing (see crbug.com/1021332) - will require additional investigation.

Original change's description:
> Layout invalidates when child view's visibility changes.
> 
> 
> 
> Exactly what it says on the tin.
> 
> 
> 
> This is a fix for legacy layouts (fill, box, grid) which aren't derived
> 
> from LayoutManagerBase and which do not benefit from the logic in that
> 
> class which handles the invalidation. LayoutManagerBase still overrides
> 
> this logic with something more sophisticated.
> 
> 
> 
> If in the future a legacy layout overrides ViewVisibilitySet(), it
> 
> should either call the base implementation or handle the invalidation on
> 
> its own.
> 
> 
> 
> 
> Bug: 1003500
> 
> Change-Id: I9eccef21784175820816852e1b9c05d0713ef2eb
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1894260
> Commit-Queue: Dana Fried <dfried@chromium.org>
> Reviewed-by: Siyu An <siyua@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#711966}

TBR=siyua@chromium.org,dfried@chromium.org

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

Bug: 1003500
Change-Id: I9f82556592e9e8db02900620085250e696ab30c6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1899387Reviewed-by: default avatarDana Fried <dfried@chromium.org>
Commit-Queue: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712497}
parent 421a7198
......@@ -110,10 +110,6 @@ IN_PROC_BROWSER_TEST_F(WebAppGlassBrowserFrameViewTest, SpaceConstrained) {
// Cause the zoom page action icon to be visible.
chrome::Zoom(app_browser_, content::PAGE_ZOOM_IN);
// The layout should be invalidated, but since we don't have the benefit of
// the compositor to immediately kick a layout off, we have to do it manually.
web_app_frame_toolbar_->Layout();
// The page action icons should now take up width.
EXPECT_GT(page_action_icon_container->width(), 0);
EXPECT_EQ(menu_button->width(), original_menu_button_width);
......
......@@ -263,6 +263,10 @@ void PageActionIconContainerView::ChildPreferredSizeChanged(
PreferredSizeChanged();
}
void PageActionIconContainerView::ChildVisibilityChanged(views::View* child) {
PreferredSizeChanged();
}
void PageActionIconContainerView::OnDefaultZoomLevelChanged() {
ZoomChangedForActiveTab(false);
}
......@@ -99,6 +99,7 @@ class PageActionIconContainerView : public views::View,
private:
// views::View:
void ChildPreferredSizeChanged(views::View* child) override;
void ChildVisibilityChanged(views::View* child) override;
// ZoomEventManagerObserver:
// Updates the view for the zoom icon when default zoom levels change.
......
......@@ -158,6 +158,11 @@ class WebAppFrameToolbarView::ContentSettingsContainer : public views::View {
}
private:
// views::View:
void ChildVisibilityChanged(views::View* child) override {
PreferredSizeChanged();
}
// Owned by the views hierarchy.
std::vector<ContentSettingImageView*> content_setting_views_;
......@@ -563,6 +568,11 @@ void WebAppFrameToolbarView::ChildPreferredSizeChanged(views::View* child) {
PreferredSizeChanged();
}
void WebAppFrameToolbarView::ChildVisibilityChanged(views::View* child) {
// Changes to layout need to be taken into account by the frame view.
PreferredSizeChanged();
}
bool WebAppFrameToolbarView::ShouldAnimate() const {
return !g_animation_disabled_for_testing &&
!browser_view_->immersive_mode_controller()->IsEnabled();
......
......@@ -151,6 +151,7 @@ class WebAppFrameToolbarView : public views::AccessiblePaneView,
// views::AccessiblePaneView:
gfx::Size CalculatePreferredSize() const override;
void ChildPreferredSizeChanged(views::View* child) override;
void ChildVisibilityChanged(views::View* child) override;
private:
friend class WebAppNonClientFrameViewAshTest;
......
......@@ -319,7 +319,7 @@ gfx::Size AnimatingLayoutManager::GetMinimumSize(const View* host) const {
// TODO(dfried): consider cases where the minimum size might not be just the
// minimum size of the embedded layout.
gfx::Size minimum_size = target_layout_manager()->GetMinimumSize(host);
if (should_animate_bounds_)
if (should_animate_bounds_ && is_animating_)
minimum_size.SetToMin(current_layout_.host_size);
return minimum_size;
}
......
......@@ -39,14 +39,7 @@ void LayoutManager::ViewAdded(View* host, View* view) {
void LayoutManager::ViewRemoved(View* host, View* view) {
}
void LayoutManager::ViewVisibilitySet(View* host, View* view, bool visible) {
// Changing the visibility of a child view should force a re-layout. There is
// more sophisticated logic in LayoutManagerBase but this should be adequate
// for most legacy layouts (none of which override this method).
// TODO(dfried): Remove this if/when LayoutManager and LayoutManagerBase can
// be merged.
host->InvalidateLayout();
}
void LayoutManager::ViewVisibilitySet(View* host, View* view, bool visible) {}
void LayoutManager::SetViewVisibility(View* view, bool visible) {
DCHECK_EQ(view->parent()->GetLayoutManager(), this);
......
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