Reland "Create separate class to handle creation of HoldingSpaceBubble"

This class will own pinned_files_container and recent_files_container.
It will also be responsible for setting the max size for the pinned
files container based on available screen space and update it when
screen space changes.

The reland fixes a bug caused by removing GetBubbleView override
from holding_space_tray that broke browser tests.

Bug: 1131268
Change-Id: Ia0bf21261bb8f3b96f37366b04e1519dbe85c7a0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2432367
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@{#811104}

ash/BUILD.gn

@@ -936,6 +936,8 @@ component("ash") {
     "system/holding_space/holding_space_item_views_container.h",
     "system/holding_space/holding_space_tray.cc",
     "system/holding_space/holding_space_tray.h",
+    "system/holding_space/holding_space_tray_bubble.cc",
+    "system/holding_space/holding_space_tray_bubble.h",
     "system/holding_space/pinned_files_container.cc",
     "system/holding_space/pinned_files_container.h",
     "system/holding_space/recent_files_container.cc",

ash/public/cpp/holding_space/holding_space_constants.h

@@ -12,6 +12,7 @@
 namespace ash {
 
 // Appearance.
+constexpr int kHoldingSpaceBubbleWidth = 360;
 constexpr gfx::Insets kHoldingSpaceContainerPadding(16);
 constexpr int kHoldingSpaceContainerChildSpacing = 16;
 constexpr int kHoldingSpaceContainerSpacing = 8;

ash/system/holding_space/holding_space_tray.cc

@@ -13,43 +13,20 @@
 #include "ash/shelf/shelf.h"
 #include "ash/shell.h"
 #include "ash/strings/grit/ash_strings.h"
-#include "ash/style/ash_color_provider.h"
-#include "ash/system/holding_space/pinned_files_container.h"
-#include "ash/system/holding_space/recent_files_container.h"
-#include "ash/system/tray/tray_bubble_wrapper.h"
-#include "ash/system/tray/tray_constants.h"
+#include "ash/system/holding_space/holding_space_tray_bubble.h"
 #include "ash/system/tray/tray_container.h"
-#include "ash/system/tray/tray_utils.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
-#include "ui/views/controls/separator.h"
 #include "ui/views/layout/fill_layout.h"
 #include "ui/views/vector_icons.h"
 
 namespace ash {
 
 namespace {
-
 // Padding for tray icon (dp; the button that shows the palette menu).
 constexpr int kTrayIconMainAxisInset = 6;
-
-// Width of the holding space bubble itself (dp).
-constexpr int kHoldingSpaceWidth = 360;
-
-void SetupChildLayer(views::View* child) {
-  child->SetPaintToLayer(ui::LAYER_SOLID_COLOR);
-
-  auto* layer = child->layer();
-  layer->SetRoundedCornerRadius(gfx::RoundedCornersF{kUnifiedTrayCornerRadius});
-  layer->SetColor(AshColorProvider::Get()->GetBaseLayerColor(
-      AshColorProvider::BaseLayerType::kTransparent80));
-  layer->SetBackgroundBlur(kUnifiedMenuBackgroundBlur);
-  layer->SetFillsBoundsOpaquely(false);
-  layer->SetIsFastRoundedCorner(true);
-}
-
 }  // namespace
 
 HoldingSpaceTray::HoldingSpaceTray(Shelf* shelf) : TrayBackgroundView(shelf) {
@@ -63,20 +40,7 @@ HoldingSpaceTray::HoldingSpaceTray(Shelf* shelf) : TrayBackgroundView(shelf) {
   tray_container()->SetMargin(kTrayIconMainAxisInset, 0);
 }
 
-HoldingSpaceTray::~HoldingSpaceTray() {
-  if (bubble_) {
-    // View hierarchy must not outlive `this`.
-    bubble_->bubble_view()->ResetDelegate();
-    bubble_->GetBubbleWidget()->CloseNow();
-  }
-}
-
-bool HoldingSpaceTray::ContainsPointInScreen(const gfx::Point& point) {
-  if (GetBoundsInScreen().Contains(point))
-    return true;
-
-  return bubble_ && bubble_->bubble_view()->GetBoundsInScreen().Contains(point);
-}
+HoldingSpaceTray::~HoldingSpaceTray() = default;
 
 void HoldingSpaceTray::ClickedOutsideBubble() {
   CloseBubble();
@@ -111,7 +75,7 @@ void HoldingSpaceTray::OnWidgetDestroying(views::Widget* widget) {
 
 void HoldingSpaceTray::AnchorUpdated() {
   if (bubble_)
-    bubble_->bubble_view()->UpdateBubble();
+    bubble_->AnchorUpdated();
 }
 
 bool HoldingSpaceTray::PerformAction(const ui::Event& event) {
@@ -158,43 +122,7 @@ void HoldingSpaceTray::ShowBubble(bool show_by_click) {
 
   DCHECK(tray_container());
 
-  TrayBubbleView::InitParams init_params;
-  init_params.delegate = this;
-  init_params.parent_window = GetBubbleWindowContainer();
-  init_params.anchor_view = GetBubbleAnchor();
-  init_params.shelf_alignment = shelf()->alignment();
-  init_params.preferred_width = kHoldingSpaceWidth;
-  init_params.close_on_deactivate = true;
-  init_params.show_by_click = show_by_click;
-  init_params.has_shadow = false;
-
-  // Create and customize bubble view.
-  TrayBubbleView* bubble_view = new TrayBubbleView(init_params);
-  bubble_view->set_anchor_view_insets(GetBubbleAnchorInsets());
-  bubble_view->set_margins(GetSecondaryBubbleInsets());
-
-  // Add pinned files container.
-  pinned_files_container_ = bubble_view->AddChildView(
-      std::make_unique<PinnedFilesContainer>(&delegate_));
-  SetupChildLayer(pinned_files_container_);
-
-  // Separator between the two containers, gives illusion of 2 separate bubbles.
-  auto* separator =
-      bubble_view->AddChildView(std::make_unique<views::Separator>());
-  separator->SetBorder(views::CreateEmptyBorder(
-      gfx::Insets(kHoldingSpaceContainerSpacing, 0, 0, 0)));
-
-  recent_files_container_ = bubble_view->AddChildView(
-      std::make_unique<RecentFilesContainer>(&delegate_));
-  SetupChildLayer(recent_files_container_);
-
-  // Show the bubble.
-  bubble_ = std::make_unique<TrayBubbleWrapper>(this, bubble_view,
-                                                false /* is_persistent */);
-
-  // Set bubble frame to be invisible.
-  bubble_->GetBubbleWidget()->non_client_view()->frame_view()->SetVisible(
-      false);
+  bubble_ = std::make_unique<HoldingSpaceTrayBubble>(this, show_by_click);
 
   // Observe the bubble widget so that we can do proper clean up when it is
   // being destroyed. If destruction is due to a call to `CloseBubble()` we will
@@ -206,7 +134,7 @@ void HoldingSpaceTray::ShowBubble(bool show_by_click) {
 }
 
 TrayBubbleView* HoldingSpaceTray::GetBubbleView() {
-  return bubble_ ? bubble_->bubble_view() : nullptr;
+  return bubble_ ? bubble_->GetBubbleView() : nullptr;
 }
 
 const char* HoldingSpaceTray::GetClassName() const {

ash/system/holding_space/holding_space_tray.h

@@ -8,25 +8,16 @@
 #include <memory>
 
 #include "ash/ash_export.h"
-#include "ash/system/holding_space/holding_space_item_view_delegate.h"
+#include "ash/system/holding_space/holding_space_tray_bubble.h"
 #include "ash/system/tray/tray_background_view.h"
 #include "base/memory/weak_ptr.h"
 #include "ui/views/widget/widget_observer.h"
 
-namespace gfx {
-class Point;
-}
-
 namespace views {
 class ImageView;
 }
 
 namespace ash {
-
-class PinnedFilesContainer;
-class RecentFilesContainer;
-class TrayBubbleWrapper;
-
 // The HoldingSpaceTray shows the tray button in the bottom area of the screen.
 // This class also controls the lifetime for all of the tools available in the
 // palette. HoldingSpaceTray has one instance per-display.
@@ -38,10 +29,6 @@ class ASH_EXPORT HoldingSpaceTray : public TrayBackgroundView,
   HoldingSpaceTray& operator=(const HoldingSpaceTray& other) = delete;
   ~HoldingSpaceTray() override;
 
-  // Returns true if the tray contains the given point. This is useful
-  // for determining if an event should be propagated through to the palette.
-  bool ContainsPointInScreen(const gfx::Point& point);
-
   // TrayBackgroundView:
   void ClickedOutsideBubble() override;
   base::string16 GetAccessibleNameForTray() override;
@@ -64,14 +51,7 @@ class ASH_EXPORT HoldingSpaceTray : public TrayBackgroundView,
   // views::WidgetObserver:
   void OnWidgetDestroying(views::Widget* widget) override;
 
-  // The singleton delegate for `HoldingSpaceItemView`s that implements support
-  // for context menu, drag-and-drop, and multiple selection.
-  HoldingSpaceItemViewDelegate delegate_;
-
-  std::unique_ptr<TrayBubbleWrapper> bubble_;
-
-  PinnedFilesContainer* pinned_files_container_ = nullptr;
-  RecentFilesContainer* recent_files_container_ = nullptr;
+  std::unique_ptr<HoldingSpaceTrayBubble> bubble_;
 
   // Weak pointer, will be parented by TrayContainer for its lifetime.
   views::ImageView* icon_ = nullptr;

ash/system/holding_space/holding_space_tray_bubble.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ash/system/holding_space/holding_space_tray_bubble.h"
+
+#include "ash/public/cpp/holding_space/holding_space_constants.h"
+#include "ash/public/cpp/shelf_config.h"
+#include "ash/shelf/shelf.h"
+#include "ash/system/holding_space/holding_space_tray.h"
+#include "ash/system/holding_space/pinned_files_container.h"
+#include "ash/system/holding_space/recent_files_container.h"
+#include "ash/system/tray/tray_bubble_wrapper.h"
+#include "ash/system/tray/tray_constants.h"
+#include "ash/system/tray/tray_utils.h"
+#include "ui/views/controls/separator.h"
+
+namespace ash {
+
+namespace {
+void SetupViewLayer(views::View* view) {
+  view->SetPaintToLayer(ui::LAYER_SOLID_COLOR);
+
+  auto* layer = view->layer();
+  layer->SetRoundedCornerRadius(gfx::RoundedCornersF{kUnifiedTrayCornerRadius});
+  layer->SetColor(AshColorProvider::Get()->GetBaseLayerColor(
+      AshColorProvider::BaseLayerType::kTransparent80));
+  layer->SetBackgroundBlur(kUnifiedMenuBackgroundBlur);
+  layer->SetFillsBoundsOpaquely(false);
+  layer->SetIsFastRoundedCorner(true);
+}
+}  // namespace
+
+HoldingSpaceTrayBubble::HoldingSpaceTrayBubble(
+    HoldingSpaceTray* holding_space_tray,
+    bool show_by_click) {
+  TrayBubbleView::InitParams init_params;
+  init_params.delegate = holding_space_tray;
+  init_params.parent_window = holding_space_tray->GetBubbleWindowContainer();
+  init_params.anchor_view = holding_space_tray->GetBubbleAnchor();
+  init_params.shelf_alignment = holding_space_tray->shelf()->alignment();
+  init_params.preferred_width = kHoldingSpaceBubbleWidth;
+  init_params.close_on_deactivate = true;
+  init_params.show_by_click = show_by_click;
+  init_params.has_shadow = false;
+
+  // Create and customize bubble view.
+  TrayBubbleView* bubble_view = new TrayBubbleView(init_params);
+  bubble_view->set_anchor_view_insets(
+      holding_space_tray->GetBubbleAnchorInsets());
+  bubble_view->set_margins(GetSecondaryBubbleInsets());
+
+  // Add pinned files container.
+  pinned_files_container_ = bubble_view->AddChildView(
+      std::make_unique<PinnedFilesContainer>(&delegate_));
+  SetupViewLayer(pinned_files_container_);
+
+  // Separator between the two containers, gives illusion of 2 separate bubbles.
+  auto* separator =
+      bubble_view->AddChildView(std::make_unique<views::Separator>());
+  separator->SetBorder(views::CreateEmptyBorder(
+      gfx::Insets(kHoldingSpaceContainerSpacing, 0, 0, 0)));
+
+  recent_files_container_ = bubble_view->AddChildView(
+      std::make_unique<RecentFilesContainer>(&delegate_));
+  SetupViewLayer(recent_files_container_);
+
+  // Show the bubble.
+  bubble_wrapper_ = std::make_unique<TrayBubbleWrapper>(
+      holding_space_tray, bubble_view, false /* is_persistent */);
+
+  // Set bubble frame to be invisible.
+  bubble_wrapper_->GetBubbleWidget()
+      ->non_client_view()
+      ->frame_view()
+      ->SetVisible(false);
+}
+
+HoldingSpaceTrayBubble::~HoldingSpaceTrayBubble() {
+  bubble_wrapper_->bubble_view()->ResetDelegate();
+  bubble_wrapper_->GetBubbleWidget()->CloseNow();
+}
+
+void HoldingSpaceTrayBubble::AnchorUpdated() {
+  bubble_wrapper_->bubble_view()->UpdateBubble();
+}
+
+TrayBubbleView* HoldingSpaceTrayBubble::GetBubbleView() {
+  return bubble_wrapper_->bubble_view();
+}
+
+views::Widget* HoldingSpaceTrayBubble::GetBubbleWidget() {
+  return bubble_wrapper_->GetBubbleWidget();
+}
+
+}  // namespace ash

ash/system/holding_space/holding_space_tray_bubble.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef ASH_SYSTEM_HOLDING_SPACE_HOLDING_SPACE_TRAY_BUBBLE_H_
+#define ASH_SYSTEM_HOLDING_SPACE_HOLDING_SPACE_TRAY_BUBBLE_H_
+
+#include <memory>
+
+#include "ash/ash_export.h"
+#include "ash/system/holding_space/holding_space_item_view_delegate.h"
+#include "ash/system/tray/tray_bubble_wrapper.h"
+
+namespace ash {
+
+class HoldingSpaceTray;
+class PinnedFilesContainer;
+class RecentFilesContainer;
+
+class ASH_EXPORT HoldingSpaceTrayBubble {
+ public:
+  HoldingSpaceTrayBubble(HoldingSpaceTray* holding_space_tray,
+                         bool show_by_click);
+  HoldingSpaceTrayBubble(const HoldingSpaceTrayBubble&) = delete;
+  HoldingSpaceTrayBubble& operator=(const HoldingSpaceTrayBubble&) = delete;
+  ~HoldingSpaceTrayBubble();
+
+  void AnchorUpdated();
+
+  TrayBubbleView* GetBubbleView();
+  views::Widget* GetBubbleWidget();
+
+ private:
+  // The singleton delegate for `HoldingSpaceItemView`s that implements support
+  // for context menu, drag-and-drop, and multiple selection.
+  HoldingSpaceItemViewDelegate delegate_;
+
+  PinnedFilesContainer* pinned_files_container_ = nullptr;
+  RecentFilesContainer* recent_files_container_ = nullptr;
+
+  std::unique_ptr<TrayBubbleWrapper> bubble_wrapper_;
+};
+
+}  // namespace ash
+
+#endif  // ASH_SYSTEM_HOLDING_SPACE_HOLDING_SPACE_TRAY_BUBBLE_H_