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}

chrome/browser/ui/views/frame/glass_browser_frame_view_browsertest_win.cc

@@ -110,6 +110,10 @@ 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);

chrome/browser/ui/views/page_action/page_action_icon_container_view.cc

@@ -263,10 +263,6 @@ void PageActionIconContainerView::ChildPreferredSizeChanged(
   PreferredSizeChanged();
 }
 
-void PageActionIconContainerView::ChildVisibilityChanged(views::View* child) {
-  PreferredSizeChanged();
-}
-
 void PageActionIconContainerView::OnDefaultZoomLevelChanged() {
   ZoomChangedForActiveTab(false);
 }

chrome/browser/ui/views/page_action/page_action_icon_container_view.h

@@ -99,7 +99,6 @@ 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.

chrome/browser/ui/views/web_apps/web_app_frame_toolbar_view.cc

@@ -158,11 +158,6 @@ 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_;
 
@@ -568,11 +563,6 @@ 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();

chrome/browser/ui/views/web_apps/web_app_frame_toolbar_view.h

@@ -151,7 +151,6 @@ 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;

ui/views/layout/animating_layout_manager.cc

@@ -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_ && is_animating_)
+  if (should_animate_bounds_)
     minimum_size.SetToMin(current_layout_.host_size);
   return minimum_size;
 }

ui/views/layout/layout_manager.cc

@@ -39,7 +39,14 @@ void LayoutManager::ViewAdded(View* host, View* view) {
 void LayoutManager::ViewRemoved(View* host, View* view) {
 }
 
-void LayoutManager::ViewVisibilitySet(View* host, View* view, bool visible) {}
+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::SetViewVisibility(View* view, bool visible) {
   DCHECK_EQ(view->parent()->GetLayoutManager(), this);