Wire up CaptureModeController to holding space.

Currently a screenshot is added to holding space when the user takes a
screenshot using the ChromeScreenshotGrabber. The same should occur if
the user takes a screenshot using the CaptureModeController.

This CL wires that up and adds test coverage for both code paths.

Bug: 1126643
Change-Id: I65f25392d4f3dc83b26156ba31827b76ded015a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2404420
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#806364}

ash/capture_mode/capture_mode_close_button.cc

@@ -25,6 +25,9 @@ CaptureModeCloseButton::CaptureModeCloseButton(views::ButtonListener* listener)
   SetImageVerticalAlignment(ALIGN_MIDDLE);
   GetViewAccessibility().OverrideIsLeaf(true);
 
+  // TODO(afakhry): Fix this.
+  GetViewAccessibility().OverrideName(GetClassName());
+
   SetInstallFocusRingOnFocus(true);
   SetFocusForPlatform();
   focus_ring()->SetColor(color_provider->GetControlsLayerColor(

ash/capture_mode/capture_mode_controller.cc

@@ -9,7 +9,10 @@
 #include <utility>
 
 #include "ash/capture_mode/capture_mode_session.h"
+#include "ash/public/cpp/ash_features.h"
 #include "ash/public/cpp/capture_mode_delegate.h"
+#include "ash/public/cpp/holding_space/holding_space_client.h"
+#include "ash/public/cpp/holding_space/holding_space_controller.h"
 #include "ash/public/cpp/notification_utils.h"
 #include "ash/resources/vector_icons/vector_icons.h"
 #include "ash/shell.h"
@@ -351,6 +354,9 @@ void CaptureModeController::OnImageFileSaved(
   const auto image = gfx::Image::CreateFrom1xPNGBytes(png_bytes);
   CopyImageToClipboard(image);
   ShowPreviewNotification(path, image);
+
+  if (features::IsTemporaryHoldingSpaceEnabled())
+    HoldingSpaceController::Get()->client()->AddScreenshot(path);
 }
 
 void CaptureModeController::ShowPreviewNotification(

ash/capture_mode/capture_mode_toggle_button.cc

@@ -5,6 +5,7 @@
 #include "ash/capture_mode/capture_mode_toggle_button.h"
 
 #include "ash/style/ash_color_provider.h"
+#include "base/strings/utf_string_conversions.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/views/accessibility/view_accessibility.h"
@@ -22,6 +23,9 @@ CaptureModeToggleButton::CaptureModeToggleButton(
   SetImageVerticalAlignment(ALIGN_MIDDLE);
   GetViewAccessibility().OverrideIsLeaf(true);
 
+  // TODO(afakhry): Fix this.
+  SetTooltipText(base::UTF8ToUTF16(GetClassName()));
+
   SetInstallFocusRingOnFocus(true);
   SetFocusForPlatform();
   const auto* color_provider = AshColorProvider::Get();

ash/public/cpp/holding_space/holding_space_client.h

@@ -8,6 +8,10 @@
 #include "ash/public/cpp/ash_public_export.h"
 #include "base/callback_forward.h"
 
+namespace base {
+class FilePath;
+}  // namespace base
+
 namespace ash {
 
 class HoldingSpaceItem;
@@ -17,6 +21,9 @@ class ASH_PUBLIC_EXPORT HoldingSpaceClient {
  public:
   using SuccessCallback = base::OnceCallback<void(bool)>;
 
+  // Adds a screenshot item backed by the provided `file_path`.
+  virtual void AddScreenshot(const base::FilePath& file_path) = 0;
+
   // Attempts to copy the specified holding space `item` to the clipboard.
   // Success is returned via the supplied `callback`.
   virtual void CopyToClipboard(const HoldingSpaceItem& item,

chrome/browser/ui/ash/holding_space/holding_space_client_impl.cc

@@ -57,6 +57,10 @@ HoldingSpaceClientImpl::HoldingSpaceClientImpl(Profile* profile)
 
 HoldingSpaceClientImpl::~HoldingSpaceClientImpl() = default;
 
+void HoldingSpaceClientImpl::AddScreenshot(const base::FilePath& file_path) {
+  GetHoldingSpaceKeyedService(profile_)->AddScreenshot(file_path);
+}
+
 // TODO(crbug/1126274): Implement.
 void HoldingSpaceClientImpl::CopyToClipboard(const HoldingSpaceItem& item,
                                              SuccessCallback callback) {

chrome/browser/ui/ash/holding_space/holding_space_client_impl.h

@@ -22,6 +22,7 @@ class HoldingSpaceClientImpl : public HoldingSpaceClient {
   ~HoldingSpaceClientImpl() override;
 
   // HoldingSpaceClient:
+  void AddScreenshot(const base::FilePath& file_path) override;
   void CopyToClipboard(const HoldingSpaceItem&, SuccessCallback) override;
   void OpenItem(const HoldingSpaceItem&, SuccessCallback) override;
   void OpenItemInFolder(const HoldingSpaceItem&, SuccessCallback) override;

chrome/browser/ui/ash/holding_space/holding_space_ui_browsertest.cc

@@ -4,9 +4,14 @@
 
 #include "chrome/browser/ui/ash/holding_space/holding_space_browsertest_base.h"
 
+#include "ash/public/cpp/ash_features.h"
+#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_model_observer.h"
 #include "base/test/bind_test_util.h"
 #include "content/public/test/browser_test.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "ui/aura/window.h"
 #include "ui/base/dragdrop/drag_drop_types.h"
 #include "ui/events/test/event_generator.h"
@@ -30,6 +35,28 @@ void MouseDrag(const views::View* from, const views::View* to) {
   event_generator.ReleaseLeftButton();
 }
 
+// Performs a press and release of the specified `key_code` with `flags`.
+void PressAndReleaseKey(ui::KeyboardCode key_code, int flags = ui::EF_NONE) {
+  auto* root_window = HoldingSpaceBrowserTestBase::GetRootWindowForNewWindows();
+  ui::test::EventGenerator event_generator(root_window);
+  event_generator.PressKey(key_code, flags);
+  event_generator.ReleaseKey(key_code, flags);
+}
+
+// Mocks -----------------------------------------------------------------------
+
+class MockHoldingSpaceModelObserver : public HoldingSpaceModelObserver {
+ public:
+  MOCK_METHOD(void,
+              OnHoldingSpaceItemAdded,
+              (const HoldingSpaceItem* item),
+              (override));
+  MOCK_METHOD(void,
+              OnHoldingSpaceItemRemoved,
+              (const HoldingSpaceItem* item),
+              (override));
+};
+
 // DropTargetView --------------------------------------------------------------
 
 class DropTargetView : public views::WidgetDelegateView {
@@ -122,4 +149,60 @@ IN_PROC_BROWSER_TEST_F(HoldingSpaceUiBrowserTest, DragAndDrop) {
   drop_target_view->GetWidget()->Close();
 }
 
+// Base class for holding space UI browser tests that take screenshots.
+// Parameterized by whether or not `features::CaptureMode` is enabled.
+class HoldingSpaceUiScreenshotBrowserTest
+    : public HoldingSpaceUiBrowserTest,
+      public testing::WithParamInterface<bool> {
+ public:
+  HoldingSpaceUiScreenshotBrowserTest() {
+    scoped_feature_list_.InitWithFeatureState(features::kCaptureMode,
+                                              GetParam());
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+// Verifies that taking a screenshot adds a screenshot holding space item.
+IN_PROC_BROWSER_TEST_P(HoldingSpaceUiScreenshotBrowserTest, AddScreenshot) {
+  // Verify that no screenshots exist in holding space UI.
+  Show();
+  ASSERT_TRUE(IsShowing());
+  EXPECT_TRUE(GetScreenshotViews().empty());
+
+  Close();
+  ASSERT_FALSE(IsShowing());
+
+  // Take a screenshot using the keyboard. If `features::kCaptureMode` is
+  // enabled, the screenshot will be taken using the `CaptureModeController`.
+  // Otherwise the screenshot will be taken using the `ChromeScreenshotGrabber`.
+  PressAndReleaseKey(ui::VKEY_MEDIA_LAUNCH_APP1,
+                     ui::EF_ALT_DOWN | ui::EF_CONTROL_DOWN);
+  PressAndReleaseKey(ui::VKEY_RETURN);
+
+  // Bind an observer to watch for updates to the holding space model.
+  testing::NiceMock<MockHoldingSpaceModelObserver> mock;
+  ScopedObserver<HoldingSpaceModel, HoldingSpaceModelObserver> observer{&mock};
+  observer.Add(HoldingSpaceController::Get()->model());
+
+  // Expect and wait for a screenshot item to be added to holding space.
+  base::RunLoop run_loop;
+  EXPECT_CALL(mock, OnHoldingSpaceItemAdded)
+      .WillOnce([&](const HoldingSpaceItem* item) {
+        if (item->type() == HoldingSpaceItem::Type::kScreenshot)
+          run_loop.Quit();
+      });
+  run_loop.Run();
+
+  // Verify that the screenshot appears in holding space UI.
+  Show();
+  ASSERT_TRUE(IsShowing());
+  EXPECT_EQ(1u, GetScreenshotViews().size());
+}
+
+INSTANTIATE_TEST_SUITE_P(All,
+                         HoldingSpaceUiScreenshotBrowserTest,
+                         testing::Bool());
+
 }  // namespace ash