Move play icon to screen captures view.

The play icon is only applicable to the screen captures holding space
item views so should be implemented at that level rather than at the
base holding space item view class level.

Bug: 1154998
Change-Id: I6f7bfe287a50f2029beff43c13e01cccc2a2d680
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2578003Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Commit-Queue: David Black <dmblack@google.com>
Cr-Commit-Position: refs/heads/master@{#834480}

ash/public/cpp/holding_space/holding_space_constants.h

@@ -55,7 +55,8 @@ enum HoldingSpaceCommandId {
 // View IDs.
 constexpr int kHoldingSpacePinnedFilesContainerId = 1;
 constexpr int kHoldingSpaceRecentFilesContainerId = 2;
-constexpr int kHoldingSpaceTrayIconId = 3;
+constexpr int kHoldingSpaceScreenCapturePlayIconId = 3;
+constexpr int kHoldingSpaceTrayIconId = 4;
 
 // The maximum allowed age for files restored into the holding space model.
 // Note that this is not enforced for pinned items.

ash/system/holding_space/holding_space_item_screen_capture_view.cc

@@ -9,10 +9,14 @@
 #include "ash/public/cpp/holding_space/holding_space_image.h"
 #include "ash/public/cpp/holding_space/holding_space_item.h"
 #include "ash/public/cpp/rounded_image_view.h"
+#include "ash/style/ash_color_provider.h"
 #include "ash/system/tray/tray_constants.h"
+#include "components/vector_icons/vector_icons.h"
+#include "ui/gfx/paint_vector_icon.h"
 #include "ui/views/accessibility/view_accessibility.h"
 #include "ui/views/background.h"
 #include "ui/views/controls/button/image_button.h"
+#include "ui/views/controls/image_view.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/fill_layout.h"
 #include "ui/views/metadata/metadata_impl_macros.h"
@@ -69,24 +73,29 @@ void HoldingSpaceItemScreenCaptureView::UpdateImage() {
 }
 
 void HoldingSpaceItemScreenCaptureView::AddPlayIcon() {
-  views::View* play_icon_container =
-      AddChildView(std::make_unique<views::View>());
+  auto* play_icon_container = AddChildView(std::make_unique<views::View>());
+  play_icon_container->SetFocusBehavior(views::View::FocusBehavior::NEVER);
 
   auto* layout =
       play_icon_container->SetLayoutManager(std::make_unique<views::BoxLayout>(
-          views::BoxLayout::Orientation::kHorizontal,
-          kHoldingSpaceScreenCapturePadding));
+          views::BoxLayout::Orientation::kHorizontal));
   layout->set_main_axis_alignment(views::BoxLayout::MainAxisAlignment::kCenter);
   layout->set_cross_axis_alignment(
       views::BoxLayout::CrossAxisAlignment::kCenter);
 
-  views::View* play_icon = CreatePlayIcon(play_icon_container);
+  auto* play_icon =
+      play_icon_container->AddChildView(std::make_unique<views::ImageView>());
+  play_icon->SetID(kHoldingSpaceScreenCapturePlayIconId);
+  play_icon->SetImage(gfx::CreateVectorIcon(
+      vector_icons::kPlayArrowIcon, kHoldingSpaceIconSize,
+      AshColorProvider::Get()->GetContentLayerColor(
+          AshColorProvider::ContentLayerType::kButtonIconColor)));
+  play_icon->SetPreferredSize(kHoldingSpaceScreenCapturePlayIconSize);
 
   // Create contrasting background for the play icon.
   play_icon->SetBackground(views::CreateRoundedRectBackground(
       HoldingSpaceColorProvider::Get()->GetBackgroundColor(),
       kHoldingSpaceScreenCapturePlayIconSize.width() / 2));
-  play_icon->SetPreferredSize(kHoldingSpaceScreenCapturePlayIconSize);
 }
 
 BEGIN_METADATA(HoldingSpaceItemScreenCaptureView, HoldingSpaceItemView)

ash/system/holding_space/holding_space_item_screen_capture_view.h

@@ -32,7 +32,7 @@ class ASH_EXPORT HoldingSpaceItemScreenCaptureView
  private:
   void UpdateImage();
 
-  // Overlays a play icon over the Screen Capture preview.
+  // Overlays a play icon over `image_`.
   void AddPlayIcon();
 
   RoundedImageView* image_ = nullptr;

ash/system/holding_space/holding_space_item_view.cc

@@ -13,7 +13,6 @@
 #include "ash/style/ash_color_provider.h"
 #include "ash/system/holding_space/holding_space_item_view_delegate.h"
 #include "base/bind.h"
-#include "components/vector_icons/vector_icons.h"
 #include "ui/base/class_property.h"
 #include "ui/base/dragdrop/drag_drop_types.h"
 #include "ui/compositor/layer_animation_element.h"
@@ -308,23 +307,6 @@ views::ToggleImageButton* HoldingSpaceItemView::AddPin(views::View* parent) {
   return pin_;
 }
 
-views::View* HoldingSpaceItemView::CreatePlayIcon(views::View* parent) {
-  DCHECK(!play_icon_);
-  play_icon_ = parent->AddChildView(std::make_unique<views::ImageView>());
-  play_icon_->SetFocusBehavior(views::View::FocusBehavior::NEVER);
-  play_icon_->SetVisible(true);
-
-  const SkColor icon_color = AshColorProvider::Get()->GetContentLayerColor(
-      AshColorProvider::ContentLayerType::kButtonIconColor);
-
-  const gfx::ImageSkia play_icon_image = gfx::CreateVectorIcon(
-      vector_icons::kPlayArrowIcon, kHoldingSpaceIconSize, icon_color);
-
-  play_icon_->SetImage(&play_icon_image);
-
-  return play_icon_;
-}
-
 void HoldingSpaceItemView::OnPaintFocus(gfx::Canvas* canvas, gfx::Size size) {
   if (!HasFocus())
     return;

ash/system/holding_space/holding_space_item_view.h

@@ -9,7 +9,6 @@
 
 #include "ash/ash_export.h"
 #include "ui/views/animation/ink_drop_host_view.h"
-#include "ui/views/controls/image_view.h"
 #include "ui/views/metadata/metadata_header_macros.h"
 
 namespace ui {
@@ -74,13 +73,8 @@ class ASH_EXPORT HoldingSpaceItemView : public views::InkDropHostView {
   void SetSelected(bool selected);
   bool selected() const { return selected_; }
 
-  views::View* play_icon() { return play_icon_; }
-
  protected:
   views::ToggleImageButton* AddPin(views::View* parent);
-  // Creates a View consisting of a play icon.
-  views::View* CreatePlayIcon(views::View* parent);
-
   virtual void OnPinVisiblityChanged(bool pin_visible) {}
 
  private:
@@ -105,7 +99,6 @@ class ASH_EXPORT HoldingSpaceItemView : public views::InkDropHostView {
   const std::string item_id_;
 
   views::ToggleImageButton* pin_ = nullptr;  // Owned by view hierarchy.
-  views::ImageView* play_icon_ = nullptr;    // Owned by view hierarchy.
 
   // Owners for the layers used to paint focused and selected states.
   std::unique_ptr<ui::LayerOwner> selected_layer_owner_;

ash/system/holding_space/holding_space_tray_unittest.cc

@@ -1335,10 +1335,12 @@ TEST_P(HoldingSpaceTrayTest, PlayIconForScreenRecordings) {
 
   EXPECT_EQ(screenshot_item->id(),
             HoldingSpaceItemView::Cast(screen_capture_chips[1])->item()->id());
-  EXPECT_FALSE(HoldingSpaceItemView::Cast(screen_capture_chips[1])->play_icon());
+  EXPECT_FALSE(screen_capture_chips[1]->GetViewByID(
+      kHoldingSpaceScreenCapturePlayIconId));
   EXPECT_EQ(screen_recording_item->id(),
             HoldingSpaceItemView::Cast(screen_capture_chips[0])->item()->id());
-  EXPECT_TRUE(HoldingSpaceItemView::Cast(screen_capture_chips[0])->play_icon());
+  EXPECT_TRUE(screen_capture_chips[0]->GetViewByID(
+      kHoldingSpaceScreenCapturePlayIconId));
 }
 
 INSTANTIATE_TEST_SUITE_P(All, HoldingSpaceTrayTest, testing::Bool());