Add Screen Recording files into holding space

We want to give users a quick shortcut to access recent screen recording
files within Tote. As the capture mode controller ends screen video
recording and a file is generated, we should be able to show it in the
holding space tray.

Bug: 1144927
Change-Id: Ia0b33e0d739705e3ac7bf1117adcff1369dac26e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2527855
Commit-Queue: Ana Salazar <anasalazar@chromium.org>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829048}

ash/capture_mode/capture_mode_controller.cc

@@ -674,6 +674,11 @@ void CaptureModeController::OnVideoFileSaved(bool success) {
     DCHECK(!recording_start_time_.is_null());
     RecordCaptureModeRecordTime(
         (base::TimeTicks::Now() - recording_start_time_).InSeconds());
+
+    if (features::IsTemporaryHoldingSpaceEnabled()) {
+      HoldingSpaceController::Get()->client()->AddScreenRecording(
+          current_video_file_path_);
+    }
   }
 
   if (!on_file_saved_callback_.is_null())

ash/public/cpp/holding_space/holding_space_client.h

@@ -26,6 +26,9 @@ class ASH_PUBLIC_EXPORT HoldingSpaceClient {
   // Adds a screenshot item backed by the provided `file_path`.
   virtual void AddScreenshot(const base::FilePath& file_path) = 0;
 
+  // Adds a screen recording item backed by the provided `file_path`.
+  virtual void AddScreenRecording(const base::FilePath& file_path) = 0;
+
   // Attempts to copy the contents of the image file backing the specified
   // holding space `item` to the clipboard. If the backing file is not suspected
   // to contain image data, this method will abort early. Success is returned

ash/system/holding_space/recent_files_container.cc

@@ -54,11 +54,20 @@ bool BelongsToDownloadsSection(HoldingSpaceItem::Type type) {
          type == HoldingSpaceItem::Type::kNearbyShare;
 }
 
+// Returns if an item of the specified `type` belongs in the screen captures
+// section.
+bool BelongsToScreenCaptureSection(HoldingSpaceItem::Type type) {
+  return type == HoldingSpaceItem::Type::kScreenshot ||
+         type == HoldingSpaceItem::Type::kScreenRecording;
+}
+
 // Returns if items of the specified types belong to the same section.
 bool BelongToSameSection(HoldingSpaceItem::Type type,
                          HoldingSpaceItem::Type other_type) {
-  return type == other_type || (BelongsToDownloadsSection(type) &&
-                                BelongsToDownloadsSection(other_type));
+  return (BelongsToScreenCaptureSection(type) &&
+          BelongsToScreenCaptureSection(other_type)) ||
+         (BelongsToDownloadsSection(type) &&
+          BelongsToDownloadsSection(other_type));
 }
 
 // DownloadsHeader--------------------------------------------------------------
@@ -165,7 +174,7 @@ void RecentFilesContainer::AddHoldingSpaceItemView(const HoldingSpaceItem* item,
                                                    bool due_to_finalization) {
   DCHECK(item->IsFinalized());
 
-  if (item->type() != HoldingSpaceItem::Type::kScreenshot &&
+  if (!BelongsToScreenCaptureSection(item->type()) &&
       !BelongsToDownloadsSection(item->type())) {
     return;
   }
@@ -185,7 +194,7 @@ void RecentFilesContainer::AddHoldingSpaceItemView(const HoldingSpaceItem* item,
     }
   }
 
-  if (item->type() == HoldingSpaceItem::Type::kScreenshot)
+  if (BelongsToScreenCaptureSection(item->type()))
     AddHoldingSpaceScreenCaptureView(item, index);
   else if (BelongsToDownloadsSection(item->type()))
     AddHoldingSpaceDownloadView(item, index);
@@ -199,7 +208,7 @@ void RecentFilesContainer::RemoveAllHoldingSpaceItemViews() {
 
 void RecentFilesContainer::RemoveHoldingSpaceItemView(
     const HoldingSpaceItem* item) {
-  if (item->type() == HoldingSpaceItem::Type::kScreenshot)
+  if (BelongsToScreenCaptureSection(item->type()))
     RemoveHoldingSpaceScreenCaptureView(item);
   else if (BelongsToDownloadsSection(item->type()))
     RemoveHoldingSpaceDownloadView(item);
@@ -208,7 +217,7 @@ void RecentFilesContainer::RemoveHoldingSpaceItemView(
 void RecentFilesContainer::AddHoldingSpaceScreenCaptureView(
     const HoldingSpaceItem* item,
     size_t index) {
-  DCHECK_EQ(item->type(), HoldingSpaceItem::Type::kScreenshot);
+  DCHECK(BelongsToScreenCaptureSection(item->type()));
   DCHECK(!base::Contains(views_by_item_id_, item->id()));
 
   if (index >= kMaxScreenCaptures)
@@ -231,7 +240,7 @@ void RecentFilesContainer::AddHoldingSpaceScreenCaptureView(
 
 void RecentFilesContainer::RemoveHoldingSpaceScreenCaptureView(
     const HoldingSpaceItem* item) {
-  DCHECK_EQ(item->type(), HoldingSpaceItem::Type::kScreenshot);
+  DCHECK(BelongsToScreenCaptureSection(item->type()));
 
   auto it = views_by_item_id_.find(item->id());
   if (it == views_by_item_id_.end())
@@ -250,7 +259,7 @@ void RecentFilesContainer::RemoveHoldingSpaceScreenCaptureView(
   for (const auto& candidate :
        base::Reversed(HoldingSpaceController::Get()->model()->items())) {
     if (candidate->IsFinalized() &&
-        candidate->type() == HoldingSpaceItem::Type::kScreenshot &&
+        BelongsToScreenCaptureSection(item->type()) &&
         !base::Contains(views_by_item_id_, candidate->id())) {
       views_by_item_id_[candidate->id()] =
           screen_captures_container_->AddChildView(

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

@@ -71,6 +71,11 @@ void HoldingSpaceClientImpl::AddScreenshot(const base::FilePath& file_path) {
   GetHoldingSpaceKeyedService(profile_)->AddScreenshot(file_path);
 }
 
+void HoldingSpaceClientImpl::AddScreenRecording(
+    const base::FilePath& file_path) {
+  GetHoldingSpaceKeyedService(profile_)->AddScreenRecording(file_path);
+}
+
 void HoldingSpaceClientImpl::CopyImageToClipboard(const HoldingSpaceItem& item,
                                                   SuccessCallback callback) {
   holding_space_metrics::RecordItemAction(

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

@@ -25,6 +25,7 @@ class HoldingSpaceClientImpl : public HoldingSpaceClient {
   ~HoldingSpaceClientImpl() override;
 
   // HoldingSpaceClient:
+  void AddScreenRecording(const base::FilePath& file_path) override;
   void AddScreenshot(const base::FilePath& file_path) override;
   void CopyImageToClipboard(const HoldingSpaceItem&, SuccessCallback) override;
   void OpenDownloads(SuccessCallback callback) override;

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

@@ -5,14 +5,17 @@
 #include "chrome/browser/ui/ash/holding_space/holding_space_browsertest_base.h"
 
 #include "ash/public/cpp/ash_features.h"
+#include "ash/public/cpp/capture_mode_test_api.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 "ash/public/cpp/holding_space/holding_space_prefs.h"
+#include "ash/public/cpp/holding_space/holding_space_test_api.h"
 #include "base/scoped_observer.h"
 #include "base/test/bind.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "content/public/test/browser_test.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -510,6 +513,60 @@ IN_PROC_BROWSER_TEST_P(HoldingSpaceUiScreenshotBrowserTest, AddScreenshot) {
   EXPECT_EQ(1u, GetScreenCaptureViews().size());
 }
 
+// Base class for holding space UI browser tests that take screen recordings.
+class HoldingSpaceUiScreenCaptureBrowserTest
+    : public HoldingSpaceUiBrowserTest {
+ public:
+  HoldingSpaceUiScreenCaptureBrowserTest() {
+    scoped_feature_list_.InitAndEnableFeature(features::kCaptureMode);
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+// Verifies that taking a screen recording adds a screen recording holding space
+// item.
+IN_PROC_BROWSER_TEST_F(HoldingSpaceUiScreenCaptureBrowserTest,
+                       AddScreenRecording) {
+  // Verify that no screen recordings exist in holding space UI.
+  Show();
+  ASSERT_TRUE(IsShowing());
+  EXPECT_TRUE(GetScreenCaptureViews().empty());
+
+  Close();
+  ASSERT_FALSE(IsShowing());
+  ash::CaptureModeTestApi capture_mode_test_api;
+  capture_mode_test_api.StartForFullscreen(/*for_video=*/true);
+  capture_mode_test_api.PerformCapture();
+  // Record a 100 ms long video.
+  base::RunLoop video_recording_time;
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE, video_recording_time.QuitClosure(),
+      base::TimeDelta::FromMilliseconds(100));
+  video_recording_time.Run();
+  capture_mode_test_api.StopVideoRecording();
+
+  // 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());
+
+  base::RunLoop wait_for_item;
+  // Expect and wait for a screen recording item to be added to holding space.
+  EXPECT_CALL(mock, OnHoldingSpaceItemAdded)
+      .WillOnce([&](const HoldingSpaceItem* item) {
+        if (item->type() == HoldingSpaceItem::Type::kScreenRecording)
+          wait_for_item.Quit();
+      });
+  wait_for_item.Run();
+
+  // Verify that the screen recording appears in holding space UI.
+  Show();
+  ASSERT_TRUE(IsShowing());
+  EXPECT_EQ(1u, GetScreenCaptureViews().size());
+}
+
 INSTANTIATE_TEST_SUITE_P(All,
                          HoldingSpaceUiScreenshotBrowserTest,
                          testing::Bool());