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/+/2436671Reviewed-by: Ahmed Mehfooz <amehfooz@chromium.org>
Commit-Queue: David Black <dmblack@google.com>
Cr-Commit-Position: refs/heads/master@{#812064}

ash/public/cpp/holding_space/holding_space_constants.h

@@ -24,6 +24,7 @@ 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 kHoldingSpacePinIconSize = 20;
 constexpr int kHoldingSpaceRowSpacing = 8;

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_delegate.cc

@@ -76,6 +76,12 @@ void HoldingSpaceItemViewDelegate::OnHoldingSpaceItemViewDestroyed(
 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 +182,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(

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

@@ -134,7 +134,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() {