CrOS Shelf: Set right margin on status area based on tablet mode

With this change, the padding between the edge of the screen and the
status area widget no longer has anything to do with the hotseat
state. This edge padding will now only ever have two possible values,
one value in tablet mode and another value in clamshell mode.

This is a reland of crrev.com/c/1977025, which was reverted because
of a test failure when the hotseat flag was enabled. The addition of
UpdateIsDense() in OnAppListVisibilityWillChange() fixes the test
failure. UpdateIsDense() has been changed to help minimize the number
of times OnShelfConfigUpdated() will be called.

Bug: 1035716
Change-Id: Iddf1e13928d73f6783d33bddfd6cc1796d0abe31
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1990513Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: Manu Cornet <manucornet@chromium.org>
Commit-Queue: Matthew Mourgos <mmourgos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#730274}

ash/public/cpp/shelf_config.h

@@ -175,8 +175,8 @@ class ASH_EXPORT ShelfConfig : public TabletModeObserver,
   // all observers.
   void OnShelfConfigUpdated();
 
-  // Updates |is_dense_| and notifies all observers of the update.
-  void UpdateIsDense();
+  // Updates |is_dense_| and returns whether |is_dense_| was changed.
+  bool UpdateIsDense();
 
   // Gets the current shelf size.
   // |ignore_in_app_state| - Whether the returned shelf size should be

ash/shelf/shelf_config.cc

@@ -61,7 +61,9 @@ ShelfConfig::ShelfConfig()
       shelf_blur_radius_(30),
       mousewheel_scroll_offset_threshold_(20),
       in_app_control_button_height_inset_(4) {
-  UpdateIsDense();
+  // Ensure ShelfConfig observers are notified if |is_dense_| is updated.
+  if (UpdateIsDense())
+    OnShelfConfigUpdated();
 }
 
 ShelfConfig::~ShelfConfig() = default;
@@ -100,16 +102,22 @@ void ShelfConfig::Shutdown() {
 }
 
 void ShelfConfig::OnTabletModeStarted() {
-  UpdateIsDense();
+  // Ensure ShelfConfig observers are notified if |is_dense_| is updated.
+  if (UpdateIsDense())
+    OnShelfConfigUpdated();
 }
 
 void ShelfConfig::OnTabletModeEnded() {
-  UpdateIsDense();
+  // Ensure ShelfConfig observers are notified if |is_dense_| is updated.
+  if (UpdateIsDense())
+    OnShelfConfigUpdated();
 }
 
 void ShelfConfig::OnDisplayMetricsChanged(const display::Display& display,
                                           uint32_t changed_metrics) {
-  UpdateIsDense();
+  // Ensure ShelfConfig observers are notified if |is_dense_| is updated.
+  if (UpdateIsDense())
+    OnShelfConfigUpdated();
 }
 
 void ShelfConfig::OnAppListVisibilityWillChange(bool shown,
@@ -119,6 +127,13 @@ void ShelfConfig::OnAppListVisibilityWillChange(bool shown,
   DCHECK_NE(is_app_list_visible_, shown);
 
   is_app_list_visible_ = shown;
+
+  // Ensure |is_dense_| is updated since this code path can be triggered
+  // by a tablet mode change before the tablet mode change has propagated to
+  // ShelfConfig. Updating |is_dense_| here will minimize the number of times
+  // OnShelfConfigUpdated() is called during a tablet mode change.
+  UpdateIsDense();
+
   OnShelfConfigUpdated();
 }
 
@@ -208,7 +223,7 @@ bool ShelfConfig::is_in_app() const {
          !is_app_list_visible_;
 }
 
-void ShelfConfig::UpdateIsDense() {
+bool ShelfConfig::UpdateIsDense() {
   const gfx::Rect screen_size =
       display::Screen::GetScreen()->GetPrimaryDisplay().bounds();
 
@@ -218,10 +233,10 @@ void ShelfConfig::UpdateIsDense() {
        (screen_size.width() <= kDenseShelfScreenSizeThreshold ||
         screen_size.height() <= kDenseShelfScreenSizeThreshold));
   if (new_is_dense == is_dense_)
-    return;
+    return false;
 
   is_dense_ = new_is_dense;
-  OnShelfConfigUpdated();
+  return true;
 }
 
 int ShelfConfig::GetShelfSize(bool ignore_in_app_state) const {

ash/system/status_area_widget_delegate.cc

@@ -7,11 +7,11 @@
 #include "ash/focus_cycler.h"
 #include "ash/root_window_controller.h"
 #include "ash/shelf/shelf.h"
-#include "ash/shelf/shelf_layout_manager.h"
 #include "ash/shelf/shelf_widget.h"
 #include "ash/shell.h"
 #include "ash/system/status_area_widget.h"
 #include "ash/system/tray/tray_constants.h"
+#include "ash/wm/tablet_mode/tablet_mode_controller.h"
 #include "chromeos/constants/chromeos_switches.h"
 #include "ui/compositor/layer.h"
 #include "ui/compositor/scoped_layer_animation_settings.h"
@@ -86,7 +86,6 @@ StatusAreaWidgetDelegate::StatusAreaWidgetDelegate(Shelf* shelf)
   set_owned_by_client();  // Deleted by DeleteDelegate().
 
   ShelfConfig::Get()->AddObserver(this);
-  shelf_->shelf_layout_manager()->AddObserver(this);
 
   // Allow the launcher to surrender the focus to another window upon
   // navigation completion by the user.
@@ -97,7 +96,6 @@ StatusAreaWidgetDelegate::StatusAreaWidgetDelegate(Shelf* shelf)
 
 StatusAreaWidgetDelegate::~StatusAreaWidgetDelegate() {
   ShelfConfig::Get()->RemoveObserver(this);
-  shelf_->shelf_layout_manager()->RemoveObserver(this);
 }
 
 void StatusAreaWidgetDelegate::SetFocusCyclerForTesting(
@@ -180,21 +178,6 @@ void StatusAreaWidgetDelegate::OnShelfConfigUpdated() {
   UpdateLayout();
 }
 
-void StatusAreaWidgetDelegate::OnHotseatStateChanged(HotseatState old_state,
-                                                     HotseatState new_state) {
-  // Update the border of the last visible child so it has the right
-  // padding depending of the state of the shelf (See
-  // https://crbug.com/1025270). Don't layout as it will cause the whole
-  // transition to snap instead of animate (See https://crbug.com/1032770).
-  auto it = std::find_if(children().crbegin(), children().crend(),
-                         [](const View* v) { return v->GetVisible(); });
-  if (it == children().crend())
-    return;
-
-  View* last_visible_child = *it;
-  SetBorderOnChild(last_visible_child, /*is_child_on_edge=*/true);
-}
-
 void StatusAreaWidgetDelegate::UpdateLayout() {
   // Use a grid layout so that the trays can be centered in each cell, and
   // so that the widget gets laid out correctly when tray sizes change.
@@ -274,11 +257,13 @@ void StatusAreaWidgetDelegate::SetBorderOnChild(views::View* child,
   // items also takes care of padding at the edge of the shelf.
   int right_edge = kPaddingBetweenWidgetsNewUi;
 
-  if (is_child_on_edge && chromeos::switches::ShouldShowShelfHotseat()) {
-    right_edge =
-        shelf_->shelf_layout_manager()->hotseat_state() == HotseatState::kShown
-            ? kPaddingBetweenWidgetAndRightScreenEdge
-            : 0;
+  const bool tablet_mode =
+      Shell::Get()->tablet_mode_controller() &&
+      Shell::Get()->tablet_mode_controller()->InTabletMode();
+
+  if (is_child_on_edge && chromeos::switches::ShouldShowShelfHotseat() &&
+      !tablet_mode) {
+    right_edge = kPaddingBetweenWidgetAndRightScreenEdge;
   }
 
   // Swap edges if alignment is not horizontal (bottom-to-top).

ash/system/status_area_widget_delegate.h

@@ -8,7 +8,6 @@
 #include "ash/ash_export.h"
 #include "ash/public/cpp/shelf_config.h"
 #include "ash/public/cpp/shelf_types.h"
-#include "ash/shelf/shelf_layout_manager_observer.h"
 #include "ash/system/status_area_widget.h"
 #include "base/macros.h"
 #include "ui/gfx/image/image_skia.h"
@@ -22,8 +21,7 @@ class Shelf;
 // The View for the status area widget.
 class ASH_EXPORT StatusAreaWidgetDelegate : public views::AccessiblePaneView,
                                             public views::WidgetDelegate,
-                                            public ShelfConfig::Observer,
-                                            public ShelfLayoutManagerObserver {
+                                            public ShelfConfig::Observer {
  public:
   explicit StatusAreaWidgetDelegate(Shelf* shelf);
   ~StatusAreaWidgetDelegate() override;
@@ -61,10 +59,6 @@ class ASH_EXPORT StatusAreaWidgetDelegate : public views::AccessiblePaneView,
   // Overridden from ShelfConfig::Observer:
   void OnShelfConfigUpdated() override;
 
-  // ShelfLayoutManagerObserver:
-  void OnHotseatStateChanged(HotseatState old_state,
-                             HotseatState new_state) override;
-
   void set_default_last_focusable_child(bool default_last_focusable_child) {
     default_last_focusable_child_ = default_last_focusable_child;
   }