Desks: Make a desk name a property of `Desk` rather than its mini view

This is a preparation work for adding support for renaming desks.

BUG=1040980
TEST=Manual, no change in functionality.

Change-Id: Ic5f846a0ef1f8fa6c975ec06a22263ce99107846
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2026692Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736198}

ash/wm/desks/desk.cc

@@ -202,6 +202,13 @@ base::AutoReset<bool> Desk::GetScopedNotifyContentChangedDisabler() {
   return base::AutoReset<bool>(&should_notify_content_changed_, false);
 }
 
+void Desk::SetName(base::string16 new_name) {
+  name_ = std::move(new_name);
+
+  for (auto& observer : observers_)
+    observer.OnDeskNameChanged(name_);
+}
+
 void Desk::Activate(bool update_window_activation) {
   // Show the associated containers on all roots.
   for (aura::Window* root : Shell::GetAllRootWindows())

ash/wm/desks/desk.h

@@ -13,6 +13,7 @@
 #include "base/containers/flat_map.h"
 #include "base/macros.h"
 #include "base/observer_list.h"
+#include "base/strings/string16.h"
 #include "ui/aura/window_observer.h"
 
 namespace ash {
@@ -41,6 +42,9 @@ class ASH_EXPORT Desk {
     // Called when Desk is at the end of its destructor. Desk automatically
     // removes its Observers before calling this.
     virtual void OnDeskDestroyed(const Desk* desk) = 0;
+
+    // Called  when the desk's name changes.
+    virtual void OnDeskNameChanged(const base::string16& new_name) = 0;
   };
 
   explicit Desk(int associated_container_id);
@@ -50,6 +54,8 @@ class ASH_EXPORT Desk {
 
   const std::vector<aura::Window*>& windows() const { return windows_; }
 
+  const base::string16& name() const { return name_; }
+
   bool is_active() const { return is_active_; }
 
   bool should_notify_content_changed() const {
@@ -67,6 +73,9 @@ class ASH_EXPORT Desk {
 
   base::AutoReset<bool> GetScopedNotifyContentChangedDisabler();
 
+  // Sets the desk's name to |new_name| and updates the observers.
+  void SetName(base::string16 new_name);
+
   // Activates this desk. All windows on this desk (if any) will become visible
   // (by means of showing this desk's associated containers on all root
   // windows). If |update_window_activation| is true, the most recently
@@ -113,6 +122,9 @@ class ASH_EXPORT Desk {
   // TODO(afakhry): Change this to track MRU windows on this desk.
   std::vector<aura::Window*> windows_;
 
+  // The name given to this desk.
+  base::string16 name_;
+
   // Maps all root windows to observer objects observing the containers
   // associated with this desk on those root windows.
   base::flat_map<aura::Window*, std::unique_ptr<DeskContainerObserver>>

ash/wm/desks/desk_mini_view.cc

@@ -61,14 +61,13 @@ gfx::Rect GetDeskPreviewBounds(aura::Window* root_window, int preview_height) {
 
 DeskMiniView::DeskMiniView(DesksBarView* owner_bar,
                            aura::Window* root_window,
-                           Desk* desk,
-                           const base::string16& title)
+                           Desk* desk)
     : views::Button(owner_bar),
       owner_bar_(owner_bar),
       root_window_(root_window),
       desk_(desk),
       desk_preview_(CreateDeskPreviewView(this)),
-      label_(new views::Label(title)),
+      label_(new views::Label()),
       close_desk_button_(new CloseDeskButton(this)) {
   desk_->AddObserver(this);
 
@@ -83,7 +82,7 @@ DeskMiniView::DeskMiniView(DesksBarView* owner_bar,
 
   close_desk_button_->SetVisible(false);
 
-  // TODO(afakhry): Tooltips and accessible names.
+  // TODO(afakhry): Tooltips.
 
   AddChildView(desk_preview_.get());
   AddChildView(label_);
@@ -93,7 +92,7 @@ DeskMiniView::DeskMiniView(DesksBarView* owner_bar,
   SetInkDropMode(InkDropMode::OFF);
 
   UpdateBorderColor();
-  SetAccessibleName(title);
+  OnDeskNameChanged(desk_->name());
 }
 
 DeskMiniView::~DeskMiniView() {
@@ -103,11 +102,6 @@ DeskMiniView::~DeskMiniView() {
     desk_->RemoveObserver(this);
 }
 
-void DeskMiniView::SetTitle(const base::string16& title) {
-  label_->SetText(title);
-  SetAccessibleName(title);
-}
-
 aura::Window* DeskMiniView::GetDeskContainer() const {
   DCHECK(desk_);
   return desk_->GetDeskContainerForRoot(root_window_);
@@ -251,6 +245,11 @@ void DeskMiniView::OnDeskDestroyed(const Desk* desk) {
   // No need to remove `this` as an observer; it's done automatically.
 }
 
+void DeskMiniView::OnDeskNameChanged(const base::string16& new_name) {
+  label_->SetText(new_name);
+  SetAccessibleName(new_name);
+}
+
 views::View* DeskMiniView::GetView() {
   return this;
 }

ash/wm/desks/desk_mini_view.h

@@ -30,10 +30,7 @@ class ASH_EXPORT DeskMiniView
       public Desk::Observer,
       public OverviewHighlightController::OverviewHighlightableView {
  public:
-  DeskMiniView(DesksBarView* owner_bar,
-               aura::Window* root_window,
-               Desk* desk,
-               const base::string16& title);
+  DeskMiniView(DesksBarView* owner_bar, aura::Window* root_window, Desk* desk);
   ~DeskMiniView() override;
 
   aura::Window* root_window() { return root_window_; }
@@ -44,8 +41,6 @@ class ASH_EXPORT DeskMiniView
     return close_desk_button_;
   }
 
-  void SetTitle(const base::string16& title);
-
   // Returns the associated desk's container window on the display this
   // mini_view resides on.
   aura::Window* GetDeskContainer() const;
@@ -75,6 +70,7 @@ class ASH_EXPORT DeskMiniView
   // Desk::Observer:
   void OnContentChanged() override;
   void OnDeskDestroyed(const Desk* desk) override;
+  void OnDeskNameChanged(const base::string16& new_name) override;
 
   // OverviewHighlightController::OverviewHighlightableView:
   views::View* GetView() override;

ash/wm/desks/desks_bar_view.cc

@@ -10,7 +10,6 @@
 
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/shell.h"
-#include "ash/strings/grit/ash_strings.h"
 #include "ash/style/ash_color_provider.h"
 #include "ash/wm/desks/desk_mini_view.h"
 #include "ash/wm/desks/desk_mini_view_animations.h"
@@ -22,7 +21,6 @@
 #include "base/stl_util.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/aura/window.h"
-#include "ui/base/l10n/l10n_util.h"
 #include "ui/events/event_observer.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/geometry/insets.h"
@@ -46,17 +44,6 @@ constexpr int kIconAndTextVerticalPadding = 8;
 // Spacing between mini views.
 constexpr int kMiniViewsSpacing = 12;
 
-base::string16 GetMiniViewTitle(int mini_view_index) {
-  DCHECK_GE(mini_view_index, 0);
-  DCHECK_LT(mini_view_index, 4);
-  constexpr int kStringIds[] = {IDS_ASH_DESKS_DESK_1_MINI_VIEW_TITLE,
-                                IDS_ASH_DESKS_DESK_2_MINI_VIEW_TITLE,
-                                IDS_ASH_DESKS_DESK_3_MINI_VIEW_TITLE,
-                                IDS_ASH_DESKS_DESK_4_MINI_VIEW_TITLE};
-
-  return l10n_util::GetStringUTF16(kStringIds[mini_view_index]);
-}
-
 gfx::Rect GetGestureEventScreenRect(const ui::Event& event) {
   DCHECK(event.IsGestureEvent());
   return event.AsGestureEvent()->details().bounding_box();
@@ -308,8 +295,6 @@ void DesksBarView::OnDeskRemoved(const Desk* desk) {
 
   UpdateMinimumWidthToFitContents();
   overview_grid_->OnDesksChanged();
-
-  UpdateMiniViewsLabels();
   new_desk_button_->UpdateButtonState();
 
   std::vector<DeskMiniView*> mini_views_before;
@@ -368,8 +353,8 @@ void DesksBarView::UpdateNewMiniViews(bool animate) {
   DCHECK(root_window);
   for (const auto& desk : desks) {
     if (!FindMiniViewForDesk(desk.get())) {
-      mini_views_.emplace_back(std::make_unique<DeskMiniView>(
-          this, root_window, desk.get(), GetMiniViewTitle(mini_views_.size())));
+      mini_views_.push_back(
+          std::make_unique<DeskMiniView>(this, root_window, desk.get()));
       DeskMiniView* mini_view = mini_views_.back().get();
       mini_view->set_owned_by_client();
       new_mini_views.emplace_back(mini_view);
@@ -397,13 +382,6 @@ DeskMiniView* DesksBarView::FindMiniViewForDesk(const Desk* desk) const {
   return nullptr;
 }
 
-void DesksBarView::UpdateMiniViewsLabels() {
-  // TODO(afakhry): Don't do this for user-modified desk labels.
-  size_t i = 0;
-  for (auto& mini_view : mini_views_)
-    mini_view->SetTitle(GetMiniViewTitle(i++));
-}
-
 int DesksBarView::GetFirstMiniViewXOffset() const {
   return mini_views_.empty() ? bounds().CenterPoint().x()
                              : mini_views_[0]->bounds().x();

ash/wm/desks/desks_bar_view.h

@@ -105,10 +105,6 @@ class ASH_EXPORT DesksBarView : public views::View,
   // has been created for it yet.
   DeskMiniView* FindMiniViewForDesk(const Desk* desk) const;
 
-  // Updates the text labels of the existing mini_views. This is called after a
-  // mini_view has been removed.
-  void UpdateMiniViewsLabels();
-
   // Returns the X offset of the first mini_view on the left (if there's one),
   // or the X offset of this view's center point when there are no mini_views.
   // This offset is used to calculate the amount by which the mini_views should

ash/wm/desks/desks_controller.cc

@@ -103,6 +103,18 @@ ui::Compositor* GetSelectedCompositorForAnimationSmoothness() {
   return selected_root->layer()->GetCompositor();
 }
 
+base::string16 GetDeskDefaultName(size_t desk_index) {
+  DCHECK_LT(desk_index, desks_util::kMaxNumberOfDesks);
+  constexpr int kStringIds[] = {IDS_ASH_DESKS_DESK_1_MINI_VIEW_TITLE,
+                                IDS_ASH_DESKS_DESK_2_MINI_VIEW_TITLE,
+                                IDS_ASH_DESKS_DESK_3_MINI_VIEW_TITLE,
+                                IDS_ASH_DESKS_DESK_4_MINI_VIEW_TITLE};
+  static_assert(desks_util::kMaxNumberOfDesks == base::size(kStringIds),
+                "Wrong default desks' names.");
+
+  return l10n_util::GetStringUTF16(kStringIds[desk_index]);
+}
+
 }  // namespace
 
 // -----------------------------------------------------------------------------
@@ -453,8 +465,10 @@ void DesksController::NewDesk(DesksCreationRemovalSource source) {
 
   base::AutoReset<bool> in_progress(&are_desks_being_modified_, true);
 
-  desks_.emplace_back(std::make_unique<Desk>(available_container_ids_.front()));
+  desks_.push_back(std::make_unique<Desk>(available_container_ids_.front()));
   available_container_ids_.pop();
+  Desk* new_desk = desks_.back().get();
+  new_desk->SetName(GetDeskDefaultName(desks_.size() - 1));
 
   UMA_HISTOGRAM_ENUMERATION(kNewDeskHistogramName, source);
   ReportDesksCountHistogram();
@@ -465,7 +479,7 @@ void DesksController::NewDesk(DesksCreationRemovalSource source) {
           base::NumberToString16(desks_.size())));
 
   for (auto& observer : observers_)
-    observer.OnDeskAdded(desks_.back().get());
+    observer.OnDeskAdded(new_desk);
 }
 
 void DesksController::RemoveDesk(const Desk* desk,
@@ -821,6 +835,8 @@ void DesksController::RemoveDeskInternal(const Desk* desk,
   if (!removed_desk_windows.empty())
     active_desk_->NotifyContentChanged(/*force_update_backdrops=*/true);
 
+  UpdateDesksDefaultNames();
+
   for (auto& observer : observers_)
     observer.OnDeskRemoved(removed_desk.get());
 
@@ -903,4 +919,11 @@ void DesksController::ReportDesksCountHistogram() const {
                              desks_util::kMaxNumberOfDesks);
 }
 
+void DesksController::UpdateDesksDefaultNames() {
+  // TODO(afakhry): Don't do this for user-modified desk labels.
+  size_t i = 0;
+  for (auto& desk : desks_)
+    desk->SetName(GetDeskDefaultName(i++));
+}
+
 }  // namespace ash

ash/wm/desks/desks_controller.h

@@ -185,6 +185,11 @@ class ASH_EXPORT DesksController : public DesksHelper,
 
   void ReportDesksCountHistogram() const;
 
+  // Updates the default names (e.g. "Desk 1", "Desk 2", ... etc.) given to the
+  // desks. This is called when desks are added or removed to update the names
+  // based on the desks order.
+  void UpdateDesksDefaultNames();
+
   std::vector<std::unique_ptr<Desk>> desks_;
 
   Desk* active_desk_ = nullptr;

ash/wm/desks/desks_unittests.cc

@@ -267,6 +267,7 @@ class TestDeskObserver : public Desk::Observer {
   // Desk::Observer:
   void OnContentChanged() override { ++notify_counts_; }
   void OnDeskDestroyed(const Desk* desk) override {}
+  void OnDeskNameChanged(const base::string16& new_name) override {}
 
  private:
   int notify_counts_ = 0;