CrOS systray: More changes to follow the shelf layout update model

Separate logic of calculating target bounds and actually updating
layout.

Bug: 1047012, 1042918, 1056514
Change-Id: Iffba0d8b691ed45a04447274a3e8ed174268c70c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2076421
Commit-Queue: Manu Cornet <manucornet@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745207}

ash/system/palette/palette_tray.cc

@@ -597,7 +597,7 @@ void PaletteTray::UpdateIconVisibility() {
                            palette_utils::IsInUserSession();
   SetVisiblePreferred(visible_preferred);
   if (visible_preferred)
-    UpdateAfterShelfChange();
+    UpdateLayout();
 }
 
 }  // namespace ash

ash/system/session/logout_button_tray.cc

@@ -56,11 +56,11 @@ void LogoutButtonTray::RegisterProfilePrefs(PrefRegistrySimple* registry) {
   registry->RegisterIntegerPref(prefs::kLogoutDialogDurationMs, 20000);
 }
 
-void LogoutButtonTray::UpdateAfterShelfChange() {
+void LogoutButtonTray::UpdateLayout() {
   // We must first update the button so that its container can lay it out
   // correctly.
   UpdateButtonTextAndImage();
-  tray_container()->UpdateAfterShelfChange();
+  tray_container()->UpdateLayout();
 }
 
 void LogoutButtonTray::UpdateBackground() {

ash/system/session/logout_button_tray.h

@@ -37,7 +37,7 @@ class ASH_EXPORT LogoutButtonTray : public TrayBackgroundView,
 
   // TrayBackgroundView:
   void UpdateAfterLoginStatusChange() override;
-  void UpdateAfterShelfChange() override;
+  void UpdateLayout() override;
   void UpdateBackground() override;
   void ClickedOutsideBubble() override;
   void HideBubbleWithView(const TrayBubbleView* bubble_view) override;

ash/system/status_area_widget.cc

@@ -154,7 +154,10 @@ void StatusAreaWidget::UpdateCollapseState() {
 }
 
 void StatusAreaWidget::CalculateTargetBounds() {
+  for (TrayBackgroundView* tray_button : tray_buttons_)
+    tray_button->CalculateTargetBounds();
   status_area_widget_delegate_->CalculateTargetBounds();
+
   gfx::Size status_size(status_area_widget_delegate_->GetTargetBounds().size());
   const gfx::Size shelf_size = shelf_->shelf_widget()->GetTargetBounds().size();
   const gfx::Point shelf_origin =
@@ -186,7 +189,7 @@ void StatusAreaWidget::UpdateLayout(bool animate) {
     return;
 
   for (TrayBackgroundView* tray_button : tray_buttons_)
-    tray_button->UpdateAfterShelfChange();
+    tray_button->UpdateLayout();
   status_area_widget_delegate_->UpdateLayout(animate);
 
   // Having a window which is visible but does not have an opacity is an
@@ -420,8 +423,6 @@ void StatusAreaWidget::OnScrollEvent(ui::ScrollEvent* event) {
 }
 
 void StatusAreaWidget::OnShelfConfigUpdated() {
-  for (TrayBackgroundView* tray_button : tray_buttons_)
-    tray_button->UpdateAfterShelfChange();
   UpdateCollapseState();
 }
 

ash/system/status_area_widget.h

@@ -143,6 +143,9 @@ class ASH_EXPORT StatusAreaWidget : public SessionObserver,
     // Children change visibility only one at a time, so keeping track of
     // how many are visible (as opposed to the visibility state of each) is
     // sufficient to make sure we don't miss necessary layout changes.
+    // TODO(manucornet): The assumption that children only change visibility
+    // one at a time may be too optimistic. One way to address this would be
+    // to have a child visibility change counter.
     unsigned int number_of_visible_children = 0;
 
     bool operator==(const LayoutInputs& other) const {

ash/system/status_area_widget_delegate.cc

@@ -231,16 +231,7 @@ void StatusAreaWidgetDelegate::ChildPreferredSizeChanged(View* child) {
   // Need to re-layout the shelf when trays or items are added/removed.
   StatusAreaWidgetDelegateAnimationSettings settings(layer());
 
-  // The shelf only needs a re-layout if this widget has changed size in
-  // the primary dimension. No re-layout is needed for changes in the cross
-  // dimension.
-  bool should_relayout_shelf = (shelf_->IsHorizontalAlignment() &&
-                                new_size.width() != current_size.width()) ||
-                               (!shelf_->IsHorizontalAlignment() &&
-                                new_size.height() != current_size.height());
-  should_relayout_shelf = false;
-  if (should_relayout_shelf)
-    shelf_->shelf_layout_manager()->LayoutShelf(/*animate=*/false);
+  shelf_->shelf_layout_manager()->LayoutShelf(/*animate=*/false);
 }
 
 void StatusAreaWidgetDelegate::ChildVisibilityChanged(View* child) {

ash/system/tray/tray_background_view.cc

@@ -340,9 +340,13 @@ void TrayBackgroundView::CloseBubble() {}
 
 void TrayBackgroundView::ShowBubble(bool show_by_click) {}
 
-void TrayBackgroundView::UpdateAfterShelfChange() {
+void TrayBackgroundView::CalculateTargetBounds() {
+  tray_container_->CalculateTargetBounds();
+}
+
+void TrayBackgroundView::UpdateLayout() {
   UpdateBackground();
-  tray_container_->UpdateAfterShelfChange();
+  tray_container_->UpdateLayout();
 }
 
 void TrayBackgroundView::UpdateAfterLoginStatusChange() {

ash/system/tray/tray_background_view.h

@@ -63,8 +63,12 @@ class ASH_EXPORT TrayBackgroundView : public ActionableView,
   // whether the showing operation is initiated by mouse or gesture click.
   virtual void ShowBubble(bool show_by_click);
 
-  // Called whenever the shelf alignment or configuration changes.
-  virtual void UpdateAfterShelfChange();
+  // Calculates the ideal bounds that this view should have depending on the
+  // constraints.
+  virtual void CalculateTargetBounds();
+
+  // Makes this view's bounds and layout match its calculated target bounds.
+  virtual void UpdateLayout();
 
   // Called to update the tray button after the login status changes.
   virtual void UpdateAfterLoginStatusChange();

ash/system/tray/tray_container.cc

@@ -27,8 +27,46 @@ TrayContainer::TrayContainer(Shelf* shelf) : shelf_(shelf) {
 TrayContainer::~TrayContainer() {
 }
 
-void TrayContainer::UpdateAfterShelfChange() {
-  UpdateLayout();
+void TrayContainer::CalculateTargetBounds() {
+  const LayoutInputs new_layout_inputs = GetLayoutInputs();
+  const bool is_horizontal = new_layout_inputs.shelf_alignment_is_horizontal;
+
+  // Adjust the size of status tray dark background by adding additional
+  // empty border.
+  views::BoxLayout::Orientation orientation =
+      is_horizontal ? views::BoxLayout::Orientation::kHorizontal
+                    : views::BoxLayout::Orientation::kVertical;
+
+  gfx::Insets insets(
+      is_horizontal
+          ? gfx::Insets(0, new_layout_inputs.status_area_hit_region_padding)
+          : gfx::Insets(new_layout_inputs.status_area_hit_region_padding, 0));
+  border_ = views::CreateEmptyBorder(insets);
+
+  int horizontal_margin = new_layout_inputs.main_axis_margin;
+  int vertical_margin = new_layout_inputs.cross_axis_margin;
+  if (!is_horizontal)
+    std::swap(horizontal_margin, vertical_margin);
+
+  layout_manager_ = std::make_unique<views::BoxLayout>(
+      orientation, gfx::Insets(vertical_margin, horizontal_margin),
+      kUnifiedTraySpacingBetweenIcons);
+  layout_manager_->set_minimum_cross_axis_size(kTrayItemSize);
+}
+
+void TrayContainer::UpdateLayout() {
+  const LayoutInputs new_layout_inputs = GetLayoutInputs();
+  if (layout_inputs_ == new_layout_inputs)
+    return;
+
+  if (border_)
+    SetBorder(std::move(border_));
+
+  if (layout_manager_)
+    views::View::SetLayoutManager(std::move(layout_manager_));
+
+  Layout();
+  layout_inputs_ = new_layout_inputs;
 }
 
 void TrayContainer::SetMargin(int main_axis_margin, int cross_axis_margin) {
@@ -74,37 +112,4 @@ TrayContainer::LayoutInputs TrayContainer::GetLayoutInputs() const {
           GetAnchorBoundsInScreen(), main_axis_margin_, cross_axis_margin_};
 }
 
-void TrayContainer::UpdateLayout() {
-  const LayoutInputs new_layout_inputs = GetLayoutInputs();
-  if (layout_inputs_.has_value() && *layout_inputs_ == new_layout_inputs)
-    return;
-  const bool is_horizontal = new_layout_inputs.shelf_alignment_is_horizontal;
-
-  // Adjust the size of status tray dark background by adding additional
-  // empty border.
-  views::BoxLayout::Orientation orientation =
-      is_horizontal ? views::BoxLayout::Orientation::kHorizontal
-                    : views::BoxLayout::Orientation::kVertical;
-
-  gfx::Insets insets(
-      is_horizontal
-          ? gfx::Insets(0, new_layout_inputs.status_area_hit_region_padding)
-          : gfx::Insets(new_layout_inputs.status_area_hit_region_padding, 0));
-  SetBorder(views::CreateEmptyBorder(insets));
-
-  int horizontal_margin = new_layout_inputs.main_axis_margin;
-  int vertical_margin = new_layout_inputs.cross_axis_margin;
-  if (!is_horizontal)
-    std::swap(horizontal_margin, vertical_margin);
-
-  auto layout = std::make_unique<views::BoxLayout>(
-      orientation, gfx::Insets(vertical_margin, horizontal_margin),
-      kUnifiedTraySpacingBetweenIcons);
-  layout->set_minimum_cross_axis_size(kTrayItemSize);
-  views::View::SetLayoutManager(std::move(layout));
-
-  PreferredSizeChanged();
-  layout_inputs_ = new_layout_inputs;
-}
-
 }  // namespace ash

ash/system/tray/tray_container.h

@@ -7,8 +7,13 @@
 
 #include "base/macros.h"
 #include "base/optional.h"
+#include "ui/views/layout/box_layout.h"
 #include "ui/views/view.h"
 
+namespace views {
+class Border;
+}  // namespace views
+
 namespace ash {
 class Shelf;
 
@@ -19,7 +24,12 @@ class TrayContainer : public views::View {
   explicit TrayContainer(Shelf* shelf);
   ~TrayContainer() override;
 
-  void UpdateAfterShelfChange();
+  // Calculates the ideal bounds that this view should have depending on the
+  // constraints.
+  void CalculateTargetBounds();
+
+  // Makes this view's bounds and layout match its calculated target bounds.
+  void UpdateLayout();
 
   void SetMargin(int main_axis_margin, int cross_axis_margin);
 
@@ -54,13 +64,19 @@ class TrayContainer : public views::View {
   // Collects the inputs for layout.
   LayoutInputs GetLayoutInputs() const;
 
-  void UpdateLayout();
-
   // The set of inputs that impact this widget's layout. The assumption is that
   // this widget needs a relayout if, and only if, one or more of these has
   // changed.
   base::Optional<LayoutInputs> layout_inputs_;
 
+  // The border that has been calculated in the target bounds calculation
+  // phase, and will be applied in the layout update phase.
+  std::unique_ptr<views::Border> border_;
+
+  // The layout manager that has been set up in the target bounds calculation
+  // phase, and will be applied in the layout update phase.
+  std::unique_ptr<views::BoxLayout> layout_manager_;
+
   Shelf* const shelf_;
 
   int main_axis_margin_ = 0;

ash/system/unified/unified_system_tray.cc

@@ -410,8 +410,8 @@ void UnifiedSystemTray::ClickedOutsideBubble() {
   CloseBubble();
 }
 
-void UnifiedSystemTray::UpdateAfterShelfChange() {
-  TrayBackgroundView::UpdateAfterShelfChange();
+void UnifiedSystemTray::UpdateLayout() {
+  TrayBackgroundView::UpdateLayout();
   time_view_->UpdateAlignmentForShelf(shelf());
 }
 

ash/system/unified/unified_system_tray.h

@@ -128,7 +128,7 @@ class ASH_EXPORT UnifiedSystemTray : public TrayBackgroundView,
   void HideBubble(const TrayBubbleView* bubble_view) override;
   void HideBubbleWithView(const TrayBubbleView* bubble_view) override;
   void ClickedOutsideBubble() override;
-  void UpdateAfterShelfChange() override;
+  void UpdateLayout() override;
   void UpdateAfterLoginStatusChange() override;
   bool ShouldEnableExtraKeyboardAccessibility() override;
   const char* GetClassName() const override;