Hide pinned files container once files have been pinned and removed

The empty prompt label should be only shown to users who have not
pinned any files.
Also fixed function order in holding_space_tray

Bug: 1134393
Change-Id: I198e8420f27e5e4fe556a461197c29d00d5a9c48
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2459689
Commit-Queue: Ahmed Mehfooz <amehfooz@chromium.org>
Reviewed-by: David Black <dmblack@google.com>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815796}

ash/system/holding_space/holding_space_tray.cc

@@ -52,38 +52,16 @@ void HoldingSpaceTray::HandleLocaleChange() {
 
 void HoldingSpaceTray::HideBubbleWithView(const TrayBubbleView* bubble_view) {}
 
-base::string16 HoldingSpaceTray::GetAccessibleNameForBubble() {
-  return GetAccessibleNameForTray();
-}
-
-bool HoldingSpaceTray::ShouldEnableExtraKeyboardAccessibility() {
-  return Shell::Get()->accessibility_controller()->spoken_feedback().enabled();
-}
-
-void HoldingSpaceTray::HideBubble(const TrayBubbleView* bubble_view) {
-  CloseBubble();
-}
-
-void HoldingSpaceTray::OnWidgetDragWillStart(views::Widget* widget) {
-  // The holding space bubble should be closed while dragging holding space
-  // items so as not to obstruct drop targets. Post the task to close the bubble
-  // so that we don't attempt to destroy the bubble widget before the associated
-  // drag event has been fully initialized.
-  base::SequencedTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::BindOnce(&HoldingSpaceTray::CloseBubble,
-                                weak_factory_.GetWeakPtr()));
-}
-
-void HoldingSpaceTray::OnWidgetDestroying(views::Widget* widget) {
-  widget->RemoveObserver(this);
-  CloseBubble();
-}
-
 void HoldingSpaceTray::AnchorUpdated() {
   if (bubble_)
     bubble_->AnchorUpdated();
 }
 
+void HoldingSpaceTray::UpdateAfterLoginStatusChange() {
+  SetVisiblePreferred(shelf()->GetStatusAreaWidget()->login_status() ==
+                      LoginStatus::USER);
+}
+
 bool HoldingSpaceTray::PerformAction(const ui::Event& event) {
   holding_space_metrics::RecordPodAction(
       holding_space_metrics::PodAction::kClick);
@@ -106,11 +84,6 @@ bool HoldingSpaceTray::PerformAction(const ui::Event& event) {
   return true;
 }
 
-void HoldingSpaceTray::UpdateAfterLoginStatusChange() {
-  SetVisiblePreferred(shelf()->GetStatusAreaWidget()->login_status() ==
-                      LoginStatus::USER);
-}
-
 void HoldingSpaceTray::CloseBubble() {
   if (!bubble_)
     return;
@@ -150,4 +123,31 @@ const char* HoldingSpaceTray::GetClassName() const {
   return "HoldingSpaceTray";
 }
 
+base::string16 HoldingSpaceTray::GetAccessibleNameForBubble() {
+  return GetAccessibleNameForTray();
+}
+
+bool HoldingSpaceTray::ShouldEnableExtraKeyboardAccessibility() {
+  return Shell::Get()->accessibility_controller()->spoken_feedback().enabled();
+}
+
+void HoldingSpaceTray::HideBubble(const TrayBubbleView* bubble_view) {
+  CloseBubble();
+}
+
+void HoldingSpaceTray::OnWidgetDragWillStart(views::Widget* widget) {
+  // The holding space bubble should be closed while dragging holding space
+  // items so as not to obstruct drop targets. Post the task to close the bubble
+  // so that we don't attempt to destroy the bubble widget before the associated
+  // drag event has been fully initialized.
+  base::SequencedTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::BindOnce(&HoldingSpaceTray::CloseBubble,
+                                weak_factory_.GetWeakPtr()));
+}
+
+void HoldingSpaceTray::OnWidgetDestroying(views::Widget* widget) {
+  widget->RemoveObserver(this);
+  CloseBubble();
+}
+
 }  // namespace ash

ash/system/holding_space/holding_space_tray_bubble.cc

@@ -83,11 +83,13 @@ class HoldingSpaceBubbleContainer : public views::View {
  private:
   // views::View:
   void ChildPreferredSizeChanged(views::View* child) override {
-    PreferredSizeChanged();
+    if (GetWidget())
+      PreferredSizeChanged();
   }
 
   void ChildVisibilityChanged(views::View* child) override {
-    PreferredSizeChanged();
+    if (GetWidget())
+      PreferredSizeChanged();
   }
 
   views::BoxLayout* layout_ = nullptr;
@@ -133,6 +135,13 @@ HoldingSpaceTrayBubble::HoldingSpaceTrayBubble(
       std::make_unique<RecentFilesContainer>(&delegate_));
   SetupViewLayer(recent_files_container_);
 
+  // Populate both containers if holding space model has already been attached.
+  HoldingSpaceModel* model = HoldingSpaceController::Get()->model();
+  if (model) {
+    pinned_files_container_->OnHoldingSpaceModelAttached(model);
+    recent_files_container_->OnHoldingSpaceModelAttached(model);
+  }
+
   // Show the bubble.
   bubble_wrapper_ = std::make_unique<TrayBubbleWrapper>(
       holding_space_tray, bubble_view, false /* is_persistent */);

ash/system/holding_space/pinned_files_container.cc

@@ -10,6 +10,8 @@
 #include "ash/public/cpp/holding_space/holding_space_controller.h"
 #include "ash/public/cpp/holding_space/holding_space_item.h"
 #include "ash/public/cpp/holding_space/holding_space_model.h"
+#include "ash/public/cpp/holding_space/holding_space_prefs.h"
+#include "ash/session/session_controller_impl.h"
 #include "ash/shell.h"
 #include "ash/strings/grit/ash_strings.h"
 #include "ash/system/holding_space/holding_space_item_chip_view.h"
@@ -17,6 +19,7 @@
 #include "ash/system/tray/tray_constants.h"
 #include "ash/system/tray/tray_popup_item_style.h"
 #include "base/optional.h"
+#include "components/prefs/pref_service.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/label.h"
@@ -29,6 +32,15 @@ namespace ash {
 
 namespace {
 
+bool HasEverPinnedHoldingSpaceItem() {
+  PrefService* active_pref_service =
+      Shell::Get()->session_controller()->GetActivePrefService();
+  return active_pref_service
+             ? holding_space_prefs::GetTimeOfFirstPin(active_pref_service)
+                   .has_value()
+             : false;
+}
+
 // HoldingSpaceScrollView ------------------------------------------------------
 
 class HoldingSpaceScrollView : public views::ScrollView,
@@ -84,16 +96,18 @@ PinnedFilesContainer::PinnedFilesContainer(
   TrayPopupItemStyle(TrayPopupItemStyle::FontStyle::SUB_HEADER)
       .SetupLabel(title_label);
 
-  empty_prompt_label_ = AddChildView(std::make_unique<views::Label>(
-      l10n_util::GetStringUTF16(IDS_ASH_HOLDING_SPACE_PINNED_EMPTY_PROMPT)));
-  empty_prompt_label_->SetHorizontalAlignment(
-      gfx::HorizontalAlignment::ALIGN_LEFT);
-  empty_prompt_label_->SetMultiLine(true);
-  empty_prompt_label_->SetPaintToLayer();
-  empty_prompt_label_->layer()->SetFillsBoundsOpaquely(false);
-
-  TrayPopupItemStyle(TrayPopupItemStyle::FontStyle::DETAILED_VIEW_LABEL)
-      .SetupLabel(empty_prompt_label_);
+  if (!HasEverPinnedHoldingSpaceItem()) {
+    empty_prompt_label_ = AddChildView(std::make_unique<views::Label>(
+        l10n_util::GetStringUTF16(IDS_ASH_HOLDING_SPACE_PINNED_EMPTY_PROMPT)));
+    empty_prompt_label_->SetHorizontalAlignment(
+        gfx::HorizontalAlignment::ALIGN_LEFT);
+    empty_prompt_label_->SetMultiLine(true);
+    empty_prompt_label_->SetPaintToLayer();
+    empty_prompt_label_->layer()->SetFillsBoundsOpaquely(false);
+
+    TrayPopupItemStyle(TrayPopupItemStyle::FontStyle::DETAILED_VIEW_LABEL)
+        .SetupLabel(empty_prompt_label_);
+  }
 
   auto* scroll_view = AddChildView(std::make_unique<HoldingSpaceScrollView>());
   scroll_view->SetBackgroundColor(base::nullopt);
@@ -105,10 +119,11 @@ PinnedFilesContainer::PinnedFilesContainer(
 
   item_chips_container_ = scroll_view->SetContents(
       std::make_unique<HoldingSpaceItemChipsContainer>());
-  item_chips_container_->SetVisible(false);
 
-  if (HoldingSpaceController::Get()->model())
-    OnHoldingSpaceModelAttached(HoldingSpaceController::Get()->model());
+  if (HasEverPinnedHoldingSpaceItem())
+    SetVisible(false);
+  else
+    item_chips_container_->SetVisible(false);
 }
 
 PinnedFilesContainer::~PinnedFilesContainer() = default;
@@ -120,15 +135,15 @@ void PinnedFilesContainer::ViewHierarchyChanged(
       item_chips_container_->children().size() != 1u) {
     return;
   }
-  if (details.is_add) {
-    // `item_chips_container_` is becoming non-empty.
+
+  if (details.is_add && empty_prompt_label_ &&
+      empty_prompt_label_->GetVisible()) {
+    // An item has been added to `item_chips_container_` for the first time.
     empty_prompt_label_->SetVisible(false);
     item_chips_container_->SetVisible(true);
-  } else {
-    // `item_chips_container_` is becoming empty.
-    item_chips_container_->SetVisible(false);
-    empty_prompt_label_->SetVisible(true);
   }
+
+  SetVisible(details.is_add);
 }
 
 void PinnedFilesContainer::AddHoldingSpaceItemView(

ash/system/holding_space/recent_files_container.cc

@@ -122,9 +122,6 @@ RecentFilesContainer::RecentFilesContainer(
   downloads_container_ =
       AddChildView(std::make_unique<HoldingSpaceItemChipsContainer>());
   downloads_container_->SetVisible(false);
-
-  if (HoldingSpaceController::Get()->model())
-    OnHoldingSpaceModelAttached(HoldingSpaceController::Get()->model());
 }
 
 RecentFilesContainer::~RecentFilesContainer() = default;