Reland "Miscellaneous holding space fixes."

This is a reland of 9a224d53

Original change's description:
> Miscellaneous holding space fixes.
>
> Fix pin toggle bounds in HoldingSpaceItemScreenshotView:
> The bounds of the pin toggle button were being stretched to match the
> full height of its parent container.
>
> Fix context menu bounds:
> Context menu should be below the associated view (when space allows).
>
> Fix long press gesture:
> When long pressing, the pressed view should become the selection just
> prior to showing the context menu.
>
> Fix crash in HoldingSpaceTrayBubble:
> If closing bubble due to ESC, bubble widget should only be closed if it
> exists and hasn't already been marked closed.
>
> Fix drag behavior:
> The holding space bubble should be closed during drag events.
>
> Bug: 1131262, 1131267, 1129981
> Change-Id: I5a1f0536ae6badeed841e0a6106e9c423cbf3781
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2436671
> Reviewed-by: Ahmed Mehfooz <amehfooz@chromium.org>
> Commit-Queue: David Black <dmblack@google.com>
> Cr-Commit-Position: refs/heads/master@{#812064}

Bug: 1131262
Bug: 1131267
Bug: 1129981
Change-Id: I2e7438ac8b46c2bedf0896393e11a52cbf9f26ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2441251
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: Ahmed Mehfooz <amehfooz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812535}

ash/public/cpp/holding_space/holding_space_constants.h

@@ -24,9 +24,10 @@ constexpr int kHoldingSpaceChipWidth = 160;
 constexpr int kHoldingSpaceChipsPerRow = 2;
 constexpr int kHoldingSpaceColumnSpacing = 8;
 constexpr int kHoldingSpaceColumnWidth = 160;
+constexpr int kHoldingSpaceContextMenuMargin = 8;
 constexpr int kHoldingSpaceCornerRadius = 8;
 constexpr int kHoldingSpaceDownloadsChevronIconSize = 20;
-constexpr int kHoldingSpaceDownloadsHeaderSpacing = 8;
+constexpr int kHoldingSpaceDownloadsHeaderSpacing = 16;
 constexpr int kHoldingSpacePinIconSize = 20;
 constexpr int kHoldingSpaceRowSpacing = 8;
 constexpr gfx::Insets kHoldingSpaceScreenshotPadding(8);

ash/system/holding_space/holding_space_item_chip_view.cc

@@ -34,6 +34,7 @@ HoldingSpaceItemChipView::HoldingSpaceItemChipView(
 
   label_ = AddChildView(std::make_unique<views::Label>(item->text()));
   label_->SetElideBehavior(gfx::ELIDE_MIDDLE);
+  label_->SetHorizontalAlignment(gfx::HorizontalAlignment::ALIGN_LEFT);
   layout->SetFlexForView(label_, 1);
 
   TrayPopupItemStyle(TrayPopupItemStyle::FontStyle::DETAILED_VIEW_LABEL)

ash/system/holding_space/holding_space_item_screenshot_view.cc

@@ -41,6 +41,9 @@ HoldingSpaceItemScreenshotView::HoldingSpaceItemScreenshotView(
           views::BoxLayout::Orientation::kHorizontal,
           kHoldingSpaceScreenshotPadding));
   layout->set_main_axis_alignment(views::BoxLayout::MainAxisAlignment::kEnd);
+  layout->set_cross_axis_alignment(
+      views::BoxLayout::CrossAxisAlignment::kStart);
+
   AddPin(pin_button_container);
 }
 

ash/system/holding_space/holding_space_item_view.cc

@@ -120,9 +120,7 @@ HoldingSpaceItemView::HoldingSpaceItemView(
   delegate_->OnHoldingSpaceItemViewCreated(this);
 }
 
-HoldingSpaceItemView::~HoldingSpaceItemView() {
-  delegate_->OnHoldingSpaceItemViewDestroyed(this);
-}
+HoldingSpaceItemView::~HoldingSpaceItemView() = default;
 
 // static
 HoldingSpaceItemView* HoldingSpaceItemView::Cast(views::View* view) {

ash/system/holding_space/holding_space_item_view_delegate.cc

@@ -65,17 +65,19 @@ HoldingSpaceItemViewDelegate::~HoldingSpaceItemViewDelegate() {
 
 void HoldingSpaceItemViewDelegate::OnHoldingSpaceItemViewCreated(
     HoldingSpaceItemView* view) {
+  view_observer_.Add(view);
   views_.push_back(view);
 }
 
-void HoldingSpaceItemViewDelegate::OnHoldingSpaceItemViewDestroyed(
-    HoldingSpaceItemView* view) {
-  base::Erase(views_, view);
-}
-
 void HoldingSpaceItemViewDelegate::OnHoldingSpaceItemViewGestureEvent(
     HoldingSpaceItemView* view,
     const ui::GestureEvent& event) {
+  // When a long press gesture occurs we are going to show the context menu.
+  // Ensure that the pressed `view` is the only view selected.
+  if (event.type() == ui::ET_GESTURE_LONG_PRESS) {
+    SetSelection(view);
+    return;
+  }
   // When a tap gesture occurs, we select and open only the item corresponding
   // to the tapped `view`.
   if (event.type() == ui::ET_GESTURE_TAP) {
@@ -176,17 +178,19 @@ void HoldingSpaceItemViewDelegate::ShowContextMenuForViewImpl(
     views::View* source,
     const gfx::Point& point,
     ui::MenuSourceType source_type) {
-  int run_types = views::MenuRunner::USE_TOUCHABLE_LAYOUT |
-                  views::MenuRunner::CONTEXT_MENU |
-                  views::MenuRunner::FIXED_ANCHOR;
+  const int run_types = views::MenuRunner::USE_TOUCHABLE_LAYOUT |
+                        views::MenuRunner::CONTEXT_MENU |
+                        views::MenuRunner::FIXED_ANCHOR;
 
   context_menu_runner_ =
       std::make_unique<views::MenuRunner>(BuildMenuModel(), run_types);
 
+  gfx::Rect bounds = source->GetBoundsInScreen();
+  bounds.Inset(gfx::Insets(-kHoldingSpaceContextMenuMargin, 0));
+
   context_menu_runner_->RunMenuAt(
-      source->GetWidget(), nullptr /*button_controller*/,
-      source->GetBoundsInScreen(), views::MenuAnchorPosition::kBubbleRight,
-      source_type);
+      source->GetWidget(), nullptr /*button_controller*/, bounds,
+      views::MenuAnchorPosition::kTopLeft, source_type);
 }
 
 bool HoldingSpaceItemViewDelegate::CanStartDragForView(
@@ -221,6 +225,11 @@ void HoldingSpaceItemViewDelegate::WriteDragDataForView(
   data->SetFilenames(filenames);
 }
 
+void HoldingSpaceItemViewDelegate::OnViewIsDeleting(views::View* view) {
+  base::Erase(views_, HoldingSpaceItemView::Cast(view));
+  view_observer_.Remove(view);
+}
+
 void HoldingSpaceItemViewDelegate::ExecuteCommand(int command_id,
                                                   int event_flags) {
   std::vector<const HoldingSpaceItemView*> selection = GetSelection();

ash/system/holding_space/holding_space_item_view_delegate.h

@@ -10,9 +10,12 @@
 #include <vector>
 
 #include "ash/ash_export.h"
+#include "base/scoped_observer.h"
 #include "ui/base/models/simple_menu_model.h"
 #include "ui/views/context_menu_controller.h"
 #include "ui/views/drag_controller.h"
+#include "ui/views/view.h"
+#include "ui/views/view_observer.h"
 
 namespace ui {
 class GestureEvent;
@@ -35,6 +38,7 @@ class HoldingSpaceItemView;
 class ASH_EXPORT HoldingSpaceItemViewDelegate
     : public views::ContextMenuController,
       public views::DragController,
+      public views::ViewObserver,
       public ui::SimpleMenuModel::Delegate {
  public:
   HoldingSpaceItemViewDelegate();
@@ -46,9 +50,6 @@ class ASH_EXPORT HoldingSpaceItemViewDelegate
   // Invoked when `view` has been created.
   void OnHoldingSpaceItemViewCreated(HoldingSpaceItemView* view);
 
-  // Invoked when `view` has been destroyed.
-  void OnHoldingSpaceItemViewDestroyed(HoldingSpaceItemView* view);
-
   // Invoked when `view` receives the specified gesture `event`.
   void OnHoldingSpaceItemViewGestureEvent(HoldingSpaceItemView* view,
                                           const ui::GestureEvent& event);
@@ -81,6 +82,9 @@ class ASH_EXPORT HoldingSpaceItemViewDelegate
                             const gfx::Point& press_pt,
                             ui::OSExchangeData* data) override;
 
+  // views::ViewObserver:
+  void OnViewIsDeleting(views::View* view) override;
+
   // SimpleMenuModel::Delegate:
   void ExecuteCommand(int command_id, int event_flags) override;
 
@@ -102,6 +106,9 @@ class ASH_EXPORT HoldingSpaceItemViewDelegate
   // ignored. This is to prevent us from selecting a view on mouse pressed but
   // then unselecting that same view on mouse released.
   HoldingSpaceItemView* ignore_mouse_released_ = nullptr;
+
+  // We observe `views_` for their lifetime so we can track selected state.
+  ScopedObserver<views::View, views::ViewObserver> view_observer_{this};
 };
 
 }  // namespace ash

ash/system/holding_space/holding_space_tray.cc

@@ -64,6 +64,16 @@ 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();

ash/system/holding_space/holding_space_tray.h

@@ -49,6 +49,7 @@ class ASH_EXPORT HoldingSpaceTray : public TrayBackgroundView,
   void HideBubble(const TrayBubbleView* bubble_view) override;
 
   // views::WidgetObserver:
+  void OnWidgetDragWillStart(views::Widget* widget) override;
   void OnWidgetDestroying(views::Widget* widget) override;
 
   std::unique_ptr<HoldingSpaceTrayBubble> bubble_;

ash/system/holding_space/holding_space_tray_bubble.cc

@@ -160,7 +160,10 @@ HoldingSpaceTrayBubble::HoldingSpaceTrayBubble(
 
 HoldingSpaceTrayBubble::~HoldingSpaceTrayBubble() {
   bubble_wrapper_->bubble_view()->ResetDelegate();
-  bubble_wrapper_->GetBubbleWidget()->CloseNow();
+  if (bubble_wrapper_->GetBubbleWidget() &&
+      !bubble_wrapper_->GetBubbleWidget()->IsClosed()) {
+    bubble_wrapper_->GetBubbleWidget()->CloseNow();
+  }
 }
 
 void HoldingSpaceTrayBubble::AnchorUpdated() {