capture_mode: End recording on low disk space

Let the VideoFileHandler detect a low disk space condition,
and notify the CaptureModeController with this. We then end
the video recording and tell the user about the reason why
recording ended.

BUG=1142994
TEST=Manually, Added a unittest.

Change-Id: I5e007693c90424491f8300d61fb3d424658c2b35
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2551938Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829814}

ash/ash_strings.grd

@@ -3112,6 +3112,9 @@ Here are some things you can try to get started.
       <message name="IDS_ASH_SCREEN_CAPTURE_MESSAGE" desc="The message shows in the screenshot or screen recording notification.">
         Show in folder
       </message>
+      <message name="IDS_ASH_SCREEN_CAPTURE_LOW_DISK_SPACE_MESSAGE" desc="The message shows in the screen recording notification when recording ends due to low disk space.">
+        Recording ended due to critically low disk space
+      </message>
       <message name="IDS_ASH_SCREEN_CAPTURE_BUTTON_EDIT" desc="The edit button label for the screen capture notification.">
         Edit
       </message>

ash/ash_strings_grd/IDS_ASH_SCREEN_CAPTURE_LOW_DISK_SPACE_MESSAGE.png.sha1

+7a2faef8ba50e589e93a386c6baebe85dcde4e62
\ No newline at end of file

ash/capture_mode/capture_mode_controller.cc

@@ -23,6 +23,7 @@
 #include "ash/strings/grit/ash_strings.h"
 #include "ash/system/status_area_widget.h"
 #include "base/bind.h"
+#include "base/bind_post_task.h"
 #include "base/check.h"
 #include "base/check_op.h"
 #include "base/files/file_path.h"
@@ -242,7 +243,7 @@ void CopyImageToClipboard(const gfx::Image& image) {
 CaptureModeController::CaptureModeController(
     std::unique_ptr<CaptureModeDelegate> delegate)
     : delegate_(std::move(delegate)),
-      task_runner_(base::ThreadPool::CreateSequencedTaskRunner(
+      blocking_task_runner_(base::ThreadPool::CreateSequencedTaskRunner(
           // A task priority of BEST_EFFORT is good enough for this runner,
           // since it's used for blocking file IO such as saving the screenshots
           // or the successive webm video chunks received from the recording
@@ -627,7 +628,7 @@ void CaptureModeController::OnImageCaptured(
   }
 
   const base::FilePath path = BuildImagePath(timestamp);
-  task_runner_->PostTaskAndReplyWithResult(
+  blocking_task_runner_->PostTaskAndReplyWithResult(
       FROM_HERE, base::BindOnce(&SaveFile, png_bytes, path),
       base::BindOnce(&CaptureModeController::OnImageFileSaved,
                      weak_ptr_factory_.GetWeakPtr(), png_bytes, path));
@@ -684,6 +685,7 @@ void CaptureModeController::OnVideoFileSaved(bool success) {
   if (!on_file_saved_callback_.is_null())
     std::move(on_file_saved_callback_).Run(current_video_file_path_);
 
+  low_disk_space_threshold_reached_ = false;
   recording_start_time_ = base::TimeTicks();
   current_video_file_path_.clear();
   video_file_handler_.Reset();
@@ -693,16 +695,20 @@ void CaptureModeController::ShowPreviewNotification(
     const base::FilePath& screen_capture_path,
     const gfx::Image& preview_image,
     const CaptureModeType type) {
+  const bool for_video = type == CaptureModeType::kVideo;
   const base::string16 title = l10n_util::GetStringUTF16(
-      type == CaptureModeType::kImage ? IDS_ASH_SCREEN_CAPTURE_SCREENSHOT_TITLE
-                                      : IDS_ASH_SCREEN_CAPTURE_RECORDING_TITLE);
+      for_video ? IDS_ASH_SCREEN_CAPTURE_RECORDING_TITLE
+                : IDS_ASH_SCREEN_CAPTURE_SCREENSHOT_TITLE);
   const base::string16 message =
-      l10n_util::GetStringUTF16(IDS_ASH_SCREEN_CAPTURE_MESSAGE);
+      for_video && low_disk_space_threshold_reached_
+          ? l10n_util::GetStringUTF16(
+                IDS_ASH_SCREEN_CAPTURE_LOW_DISK_SPACE_MESSAGE)
+          : l10n_util::GetStringUTF16(IDS_ASH_SCREEN_CAPTURE_MESSAGE);
 
   message_center::RichNotificationData optional_fields;
   message_center::ButtonInfo edit_button(
       l10n_util::GetStringUTF16(IDS_ASH_SCREEN_CAPTURE_BUTTON_EDIT));
-  if (type == CaptureModeType::kImage)
+  if (!for_video)
     optional_fields.buttons.push_back(edit_button);
   message_center::ButtonInfo delete_button(
       l10n_util::GetStringUTF16(IDS_ASH_SCREEN_CAPTURE_BUTTON_DELETE));
@@ -737,7 +743,7 @@ void CaptureModeController::HandleNotificationClicked(
   if (type == CaptureModeType::kVideo) {
     DCHECK_EQ(button_index_value,
               VideoNotificationButtonIndex::BUTTON_DELETE_VIDEO);
-    DeleteFileAsync(task_runner_, screen_capture_path);
+    DeleteFileAsync(blocking_task_runner_, screen_capture_path);
     return;
   }
 
@@ -748,7 +754,7 @@ void CaptureModeController::HandleNotificationClicked(
       delegate_->OpenScreenshotInImageEditor(screen_capture_path);
       break;
     case ScreenshotNotificationButtonIndex::BUTTON_DELETE:
-      DeleteFileAsync(task_runner_, screen_capture_path);
+      DeleteFileAsync(blocking_task_runner_, screen_capture_path);
       break;
     default:
       NOTREACHED();
@@ -813,14 +819,28 @@ void CaptureModeController::OnVideoRecordCountDownFinished() {
   video_recording_watcher_ =
       std::make_unique<VideoRecordingWatcher>(this, capture_params->window);
 
-  // TODO(afakhry): Choose a real buffer capacity when the recording service is
-  // in.
   constexpr size_t kVideoBufferCapacityBytes = 512 * 1024;
+
+  // We use a threshold of 512 MB to end the video recording due to low disk
+  // space, which is the same threshold as that used by the low disk space
+  // notification (See low_disk_notification.cc).
+  constexpr size_t kLowDiskSpaceThresholdInBytes = 512 * 1024 * 1024;
+
+  // The |video_file_handler_| performs all its tasks on the
+  // |blocking_task_runner_|. However, we want the low disk space callback to be
+  // run on the UI thread.
+  base::OnceClosure on_low_disk_space_callback =
+      base::BindPostTask(base::ThreadTaskRunnerHandle::Get(),
+                         base::BindOnce(&CaptureModeController::OnLowDiskSpace,
+                                        weak_ptr_factory_.GetWeakPtr()));
+
   DCHECK(current_video_file_path_.empty());
   recording_start_time_ = base::TimeTicks::Now();
   current_video_file_path_ = BuildVideoPath(base::Time::Now());
   video_file_handler_ = VideoFileHandler::Create(
-      task_runner_, current_video_file_path_, kVideoBufferCapacityBytes);
+      blocking_task_runner_, current_video_file_path_,
+      kVideoBufferCapacityBytes, kLowDiskSpaceThresholdInBytes,
+      std::move(on_low_disk_space_callback));
   video_file_handler_.AsyncCall(&VideoFileHandler::Initialize)
       .Then(on_video_file_status_);
 
@@ -840,4 +860,16 @@ void CaptureModeController::InterruptVideoRecording() {
   EndVideoRecording();
 }
 
+void CaptureModeController::OnLowDiskSpace() {
+  DCHECK(base::CurrentUIThread::IsSet());
+
+  low_disk_space_threshold_reached_ = true;
+  // We end the video recording normally (i.e. we don't consider this to be a
+  // failure). The low disk space threashold was chosen to be big enough to
+  // allow the remaining chunks to be saved normally. However,
+  // |low_disk_space_threshold_reached_| will be used to display a different
+  // message in the notification.
+  EndVideoRecording();
+}
+
 }  // namespace ash

ash/capture_mode/capture_mode_controller.h

@@ -214,13 +214,19 @@ class ASH_EXPORT CaptureModeController
   // allowed to be captured.
   void InterruptVideoRecording();
 
+  // Called back by |video_file_handler_| when it detects a low disk space
+  // condition. In this case we end the video recording to avoid consuming too
+  // much space, and we make sure the video preview notification shows a message
+  // explaining why the recording ended.
+  void OnLowDiskSpace();
+
   std::unique_ptr<CaptureModeDelegate> delegate_;
 
   CaptureModeType type_ = CaptureModeType::kImage;
   CaptureModeSource source_ = CaptureModeSource::kRegion;
 
   // A blocking task runner for file IO operations.
-  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+  scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_;
 
   mojo::Remote<recording::mojom::RecordingService> recording_service_remote_;
   mojo::Receiver<recording::mojom::RecordingServiceClient>
@@ -236,7 +242,7 @@ class ASH_EXPORT CaptureModeController
   base::FilePath current_video_file_path_;
 
   // Handles the file IO operations of the video file. This enforces doing all
-  // video file related operations on the blocking |task_runner_|.
+  // video file related operations on the |blocking_task_runner_|.
   base::SequenceBound<VideoFileHandler> video_file_handler_;
 
   // We remember the user selected capture region when the source is |kRegion|
@@ -253,6 +259,12 @@ class ASH_EXPORT CaptureModeController
   // will start immediately.
   bool skip_count_down_ui_ = false;
 
+  // True if while writing the video chunks by |video_file_handler_| we detected
+  // a low disk space. This value is used only to determine the message shown to
+  // the user in the video preview notification to explain why the recording was
+  // ended, and is then reset back to false.
+  bool low_disk_space_threshold_reached_ = false;
+
   // Watches events that lead to ending video recording.
   std::unique_ptr<VideoRecordingWatcher> video_recording_watcher_;
 

ash/capture_mode/video_file_handler.cc

@@ -6,6 +6,7 @@
 
 #include "base/containers/span.h"
 #include "base/logging.h"
+#include "base/system/sys_info.h"
 #include "base/task/current_thread.h"
 
 namespace ash {
@@ -23,11 +24,14 @@ constexpr size_t kReservedNumberOfChunks = 100;
 base::SequenceBound<VideoFileHandler> VideoFileHandler::Create(
     scoped_refptr<base::SequencedTaskRunner> blocking_task_runner,
     const base::FilePath& path,
-    size_t max_buffered_bytes) {
+    size_t max_buffered_bytes,
+    size_t low_disk_space_threshold_bytes,
+    base::OnceClosure on_low_disk_space_callback) {
   DCHECK(base::CurrentUIThread::IsSet());
 
-  return base::SequenceBound<VideoFileHandler>(blocking_task_runner, path,
-                                               max_buffered_bytes);
+  return base::SequenceBound<VideoFileHandler>(
+      blocking_task_runner, path, max_buffered_bytes,
+      low_disk_space_threshold_bytes, std::move(on_low_disk_space_callback));
 }
 
 bool VideoFileHandler::Initialize() {
@@ -94,8 +98,13 @@ bool VideoFileHandler::FlushBufferedChunks() {
 }
 
 VideoFileHandler::VideoFileHandler(const base::FilePath& path,
-                                   size_t max_buffered_bytes)
-    : path_(path), max_buffered_bytes_(max_buffered_bytes) {
+                                   size_t max_buffered_bytes,
+                                   size_t low_disk_space_threshold_bytes,
+                                   base::OnceClosure on_low_disk_space_callback)
+    : path_(path),
+      max_buffered_bytes_(max_buffered_bytes),
+      low_disk_space_threshold_bytes_(low_disk_space_threshold_bytes),
+      on_low_disk_space_callback_(std::move(on_low_disk_space_callback)) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(max_buffered_bytes_);
 
@@ -113,6 +122,21 @@ void VideoFileHandler::AppendToFile(const std::string& chunk) {
 
   success_ &=
       file_.WriteAtCurrentPosAndCheck(base::as_bytes(base::make_span(chunk)));
+
+  if (!on_low_disk_space_callback_)
+    return;
+
+  const int64_t remaining_disk_bytes =
+      base::SysInfo::AmountOfFreeDiskSpace(path_);
+  if (remaining_disk_bytes >= 0 &&
+      size_t{remaining_disk_bytes} < low_disk_space_threshold_bytes_) {
+    // Note that a low disk space is not considered an IO failure, and should
+    // not affect |success_|. It simply means that the available disk space is
+    // now below the threshold set by the owner of this object, and should not
+    // affect the actual writing of the chunks until the file system actually
+    // fails to write.
+    std::move(on_low_disk_space_callback_).Run();
+  }
 }
 
 }  // namespace ash

ash/capture_mode/video_file_handler.h

@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "ash/ash_export.h"
+#include "base/callback_forward.h"
 #include "base/files/file.h"
 #include "base/files/file_path.h"
 #include "base/sequence_checker.h"
@@ -58,7 +59,9 @@ class ASH_EXPORT VideoFileHandler {
   static base::SequenceBound<VideoFileHandler> Create(
       scoped_refptr<base::SequencedTaskRunner> blocking_task_runner,
       const base::FilePath& path,
-      size_t max_buffered_bytes);
+      size_t max_buffered_bytes,
+      size_t low_disk_space_threshold_bytes,
+      base::OnceClosure on_low_disk_space_callback);
 
   // Must be done only once before all other operations to open/create the file
   // at |path_| via |file_| in preparation for future writes. The file remains
@@ -85,7 +88,10 @@ class ASH_EXPORT VideoFileHandler {
  private:
   friend class base::SequenceBound<VideoFileHandler>;
 
-  VideoFileHandler(const base::FilePath& path, size_t max_buffered_bytes);
+  VideoFileHandler(const base::FilePath& path,
+                   size_t max_buffered_bytes,
+                   size_t low_disk_space_threshold_bytes,
+                   base::OnceClosure on_low_disk_space_callback);
   ~VideoFileHandler();
 
   // Appends the given |chunk| to |file_| at its current position, and updates
@@ -101,6 +107,11 @@ class ASH_EXPORT VideoFileHandler {
   // OOM'in the system.
   const size_t max_buffered_bytes_;
 
+  // The number of bytes if the free disk space goes below which, we will notify
+  // the owner of this object of this condition via the
+  // |on_low_disk_space_callback_|.
+  const size_t low_disk_space_threshold_bytes_;
+
   // The opened file that exists at |path_|. All I/O operations on the file will
   // be done through this object, which must first be initialized by calling
   // |Initialize()|.
@@ -120,6 +131,11 @@ class ASH_EXPORT VideoFileHandler {
   // repeatedly updated by each write operation.
   bool success_ GUARDED_BY_CONTEXT(sequence_checker_) = false;
 
+  // Called to notify the owner of this object that the low disk space threshold
+  // has been reached.
+  base::OnceClosure on_low_disk_space_callback_
+      GUARDED_BY_CONTEXT(sequence_checker_);
+
   // Checker to guarantee certain operations are run on the
   // |blocking_task_runner| given to |Create()|.
   SEQUENCE_CHECKER(sequence_checker_);

ash/capture_mode/video_file_handler_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "ash/capture_mode/video_file_handler.h"
 
+#include <limits>
 #include <string>
 
 #include "base/bind.h"
@@ -15,6 +16,7 @@
 #include "base/run_loop.h"
 #include "base/task/task_traits.h"
 #include "base/task/thread_pool.h"
+#include "base/test/bind.h"
 #include "base/test/task_environment.h"
 #include "base/threading/sequence_bound.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -45,9 +47,13 @@ class VideoFileHandlerTest : public ::testing::Test {
   }
 
   // Creates and returns an initialized VideoFileHandler instance.
-  base::SequenceBound<VideoFileHandler> CreateAndInitHandler(size_t capacity) {
-    base::SequenceBound<VideoFileHandler> handler =
-        VideoFileHandler::Create(task_runner(), temp_file(), capacity);
+  base::SequenceBound<VideoFileHandler> CreateAndInitHandler(
+      size_t capacity,
+      size_t low_disk_space_threshold_bytes = 1024,
+      base::OnceClosure on_low_disk_space_callback = base::DoNothing()) {
+    base::SequenceBound<VideoFileHandler> handler = VideoFileHandler::Create(
+        task_runner(), temp_file(), capacity, low_disk_space_threshold_bytes,
+        std::move(on_low_disk_space_callback));
     const bool success =
         RunOnHandlerAndWait(&handler, &VideoFileHandler::Initialize);
     EXPECT_TRUE(success);
@@ -61,12 +67,11 @@ class VideoFileHandlerTest : public ::testing::Test {
     base::RunLoop run_loop;
     task_runner_->PostTaskAndReplyWithResult(
         FROM_HERE, std::move(task),
-        base::BindOnce(
-            [](base::RunLoop* loop, bool* result, bool success) {
-              *result = success;
-              loop->Quit();
-            },
-            &run_loop, &result));
+        base::OnceCallback<void(bool)>(
+            base::BindLambdaForTesting([&](bool success) {
+              result = success;
+              run_loop.Quit();
+            })));
     run_loop.Run();
     return result;
   }
@@ -86,12 +91,11 @@ class VideoFileHandlerTest : public ::testing::Test {
                            Method method) const {
     base::RunLoop run_loop;
     bool result = false;
-    handler->AsyncCall(method).Then(base::BindOnce(
-        [](base::RunLoop* loop, bool* result, bool success) {
-          *result = success;
-          loop->Quit();
-        },
-        &run_loop, &result));
+    handler->AsyncCall(method).Then(
+        base::BindLambdaForTesting([&](bool success) {
+          result = success;
+          run_loop.Quit();
+        }));
     run_loop.Run();
     return result;
   }
@@ -218,16 +222,55 @@ TEST_F(VideoFileHandlerTest, ManualFlush) {
   // It's possible to flush the buffer manually.
   base::RunLoop run_loop;
   handler.AsyncCall(&VideoFileHandler::FlushBufferedChunks)
-      .Then(base::BindOnce(
-          [](base::RunLoop* loop, bool success) {
-            EXPECT_TRUE(success);
-            loop->Quit();
-          },
-          &run_loop));
+      .Then(base::BindLambdaForTesting([&](bool success) {
+        EXPECT_TRUE(success);
+        run_loop.Quit();
+      }));
   run_loop.Run();
   file_content = ReadTempFileContent();
   EXPECT_EQ(file_content, chunk_1);
   EXPECT_TRUE(GetSuccessStatusOnUi(&handler));
 }
 
+TEST_F(VideoFileHandlerTest, LowDiskSpace) {
+  constexpr size_t kCapacity = 10;
+  // Simulate 0 disk space remaining.
+  constexpr size_t kLowDiskSpaceThreshold = std::numeric_limits<size_t>::max();
+  bool low_disk_space_threshold_reached = false;
+  base::SequenceBound<VideoFileHandler> handler = CreateAndInitHandler(
+      kCapacity, kLowDiskSpaceThreshold, base::BindLambdaForTesting([&]() {
+        low_disk_space_threshold_reached = true;
+      }));
+  ASSERT_TRUE(handler);
+
+  // Append a chunk smaller than the capacity. Nothing will be written to the
+  // file yet, and the low disk space notification won't be trigged, not until
+  // the buffer is actually written on the next append.
+  std::string chunk_1 = "12345";
+  handler.AsyncCall(&VideoFileHandler::AppendChunk)
+      .WithArgs(chunk_1)
+      .Then(GetIgnoreResultCallback());
+  std::string file_content = ReadTempFileContent();
+  EXPECT_TRUE(file_content.empty());
+  EXPECT_TRUE(GetSuccessStatusOnUi(&handler));
+  EXPECT_FALSE(low_disk_space_threshold_reached);
+
+  std::string chunk_2 = "1234567";
+  handler.AsyncCall(&VideoFileHandler::AppendChunk)
+      .WithArgs(chunk_2)
+      .Then(GetIgnoreResultCallback());
+  file_content = ReadTempFileContent();
+  // Only the buffered chunk will be written, and the low disk space callback
+  // will be triggered.
+  EXPECT_EQ(file_content, chunk_1);
+  EXPECT_TRUE(GetSuccessStatusOnUi(&handler));
+  EXPECT_TRUE(low_disk_space_threshold_reached);
+
+  // Reaching low disk space threshold is not a failure, and it's still possible
+  // to flush the remaining buffered chunks.
+  handler.Reset();
+  file_content = ReadTempFileContent();
+  EXPECT_EQ(file_content, chunk_1 + chunk_2);
+}
+
 }  // namespace ash