[Paint Preview] Move file system related logic out of PaintPreviewBaseService

This is a refactoring effort to have in-memory and in-file capture support for PaintPreviewBaseService.

Change-Id: I57c796d689239dfe133a0927979555b3e8b72ea0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2548541Reviewed-by: Tanya Gupta <tgupta@chromium.org>
Reviewed-by: Calder Kitagawa <ckitagawa@chromium.org>
Commit-Queue: Yashar Dabiran <yashard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#831083}

chrome/browser/long_screenshots/long_screenshots_tab_service.cc

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/long_screenshots/long_screenshots_tab_service.h"
 
+#include <utility>
+
 #include "base/callback.h"
 #include "base/memory/memory_pressure_listener.h"
 #include "base/memory/memory_pressure_monitor.h"
@@ -20,12 +22,10 @@ constexpr size_t kMaxPerCaptureSizeBytes = 5 * 1000L * 1000L;  // 5 MB.
 }  // namespace
 
 LongScreenshotsTabService::LongScreenshotsTabService(
-    const base::FilePath& profile_dir,
-    base::StringPiece ascii_feature_name,
+    std::unique_ptr<paint_preview::PaintPreviewFileMixin> file_mixin,
     std::unique_ptr<paint_preview::PaintPreviewPolicy> policy,
     bool is_off_the_record)
-    : PaintPreviewBaseService(profile_dir,
-                              ascii_feature_name,
+    : PaintPreviewBaseService(std::move(file_mixin),
                               std::move(policy),
                               is_off_the_record) {
   // TODO(tgupta): Populate this.
@@ -52,12 +52,12 @@ void LongScreenshotsTabService::CaptureTab(int tab_id,
   // to ensure the renderer doesn't go away while that happens.
   contents->IncrementCapturerCount(gfx::Size(), true);
 
-  auto file_manager = GetFileManager();
+  auto file_manager = GetFileMixin()->GetFileManager();
   auto key = file_manager->CreateKey(tab_id);
-  GetTaskRunner()->PostTaskAndReplyWithResult(
+  GetFileMixin()->GetTaskRunner()->PostTaskAndReplyWithResult(
       FROM_HERE,
       base::BindOnce(&paint_preview::FileManager::CreateOrGetDirectory,
-                     GetFileManager(), key, true),
+                     GetFileMixin()->GetFileManager(), key, true),
       // TODO(tgupta): Check for AMP pages here and get the right node id.
       base::BindOnce(&LongScreenshotsTabService::CaptureTabInternal,
                      weak_ptr_factory_.GetWeakPtr(), tab_id, key,
@@ -93,8 +93,15 @@ void LongScreenshotsTabService::CaptureTabInternal(
   }
   // TODO(tgupta): Modify this call to specify the size of the capture rather
   // than the whole area.
+  CaptureParams capture_params;
+  capture_params.web_contents = contents;
+  capture_params.root_dir = &file_path.value();
+  capture_params.persistence = paint_preview::RecordingPersistence::kFileSystem;
+  capture_params.clip_rect = gfx::Rect();
+  capture_params.capture_links = true;
+  capture_params.max_per_capture_size = kMaxPerCaptureSizeBytes;
   CapturePaintPreview(
-      contents, file_path.value(), gfx::Rect(), true, kMaxPerCaptureSizeBytes,
+      capture_params,
       base::BindOnce(&LongScreenshotsTabService::OnCaptured,
                      weak_ptr_factory_.GetWeakPtr(), tab_id, key,
                      frame_tree_node_id, std::move(callback)));
@@ -118,11 +125,12 @@ void LongScreenshotsTabService::OnCaptured(
     return;
   }
 
-  auto file_manager = GetFileManager();
-  GetTaskRunner()->PostTaskAndReplyWithResult(
+  auto file_manager = GetFileMixin()->GetFileManager();
+  GetFileMixin()->GetTaskRunner()->PostTaskAndReplyWithResult(
       FROM_HERE,
-      base::BindOnce(&FileManager::SerializePaintPreviewProto, GetFileManager(),
-                     key, result->proto, true),
+      base::BindOnce(&FileManager::SerializePaintPreviewProto,
+                     GetFileMixin()->GetFileManager(), key, result->proto,
+                     true),
       base::BindOnce(&LongScreenshotsTabService::OnFinished,
                      weak_ptr_factory_.GetWeakPtr(), tab_id,
                      std::move(callback)));
@@ -136,8 +144,9 @@ void LongScreenshotsTabService::OnFinished(int tab_id,
 }
 
 void LongScreenshotsTabService::DeleteAllLongScreenshotFiles() {
-  GetTaskRunner()->PostTask(
-      FROM_HERE, base::BindOnce(&FileManager::DeleteAll, GetFileManager()));
+  GetFileMixin()->GetTaskRunner()->PostTask(
+      FROM_HERE, base::BindOnce(&FileManager::DeleteAll,
+                                GetFileMixin()->GetFileManager()));
 }
 
 }  // namespace long_screenshots

chrome/browser/long_screenshots/long_screenshots_tab_service.h

@@ -33,8 +33,7 @@ class LongScreenshotsTabService
     : public paint_preview::PaintPreviewBaseService {
  public:
   LongScreenshotsTabService(
-      const base::FilePath& profile_dir,
-      base::StringPiece ascii_feature_name,
+      std::unique_ptr<paint_preview::PaintPreviewFileMixin> file_mixin,
       std::unique_ptr<paint_preview::PaintPreviewPolicy> policy,
       bool is_off_the_record);
   ~LongScreenshotsTabService() override;

chrome/browser/long_screenshots/long_screenshots_tab_service_factory.cc

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/long_screenshots/long_screenshots_tab_service_factory.h"
 
+#include <utility>
+
 #include "build/build_config.h"
 #include "chrome/browser/long_screenshots/long_screenshots_tab_service.h"
 #include "components/keyed_service/core/simple_dependency_manager.h"
@@ -44,7 +46,9 @@ LongScreenshotsTabServiceFactory::BuildServiceInstanceFor(
     return nullptr;
 
   return std::make_unique<LongScreenshotsTabService>(
-      key->GetPath(), kFeatureDirname, nullptr, key->IsOffTheRecord());
+      std::make_unique<paint_preview::PaintPreviewFileMixin>(key->GetPath(),
+                                                             kFeatureDirname),
+      nullptr, key->IsOffTheRecord());
 }
 
 SimpleFactoryKey* LongScreenshotsTabServiceFactory::GetKeyToUse(

chrome/browser/long_screenshots/long_screenshots_tab_service_unittest.cc

@@ -86,7 +86,9 @@ class LongScreenshotsTabServiceTest : public ChromeRenderViewHostTestHarness {
     task_environment()->RunUntilIdle();
     EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
     service_ = std::make_unique<LongScreenshotsTabService>(
-        temp_dir_.GetPath(), kFeatureName, nullptr, false);
+        std::make_unique<paint_preview::PaintPreviewFileMixin>(
+            temp_dir_.GetPath(), kFeatureName),
+        nullptr, false);
     task_environment()->RunUntilIdle();
   }
 
@@ -126,9 +128,9 @@ TEST_F(LongScreenshotsTabServiceTest, CaptureTab) {
       }));
   task_environment()->RunUntilIdle();
 
-  auto file_manager = service->GetFileManager();
+  auto file_manager = service->GetFileMixin()->GetFileManager();
   auto key = file_manager->CreateKey(kTabId);
-  service->GetTaskRunner()->PostTaskAndReplyWithResult(
+  service->GetFileMixin()->GetTaskRunner()->PostTaskAndReplyWithResult(
       FROM_HERE,
       base::BindOnce(&paint_preview::FileManager::DirectoryExists, file_manager,
                      key),
@@ -137,7 +139,7 @@ TEST_F(LongScreenshotsTabServiceTest, CaptureTab) {
 
   service->DeleteAllLongScreenshotFiles();
   task_environment()->RunUntilIdle();
-  service->GetTaskRunner()->PostTaskAndReplyWithResult(
+  service->GetFileMixin()->GetTaskRunner()->PostTaskAndReplyWithResult(
       FROM_HERE,
       base::BindOnce(&FileManager::DirectoryExists, file_manager, key),
       base::BindOnce([](bool exists) { EXPECT_FALSE(exists); }));
@@ -160,9 +162,9 @@ TEST_F(LongScreenshotsTabServiceTest, CaptureTabFailed) {
       }));
   task_environment()->RunUntilIdle();
 
-  auto file_manager = service->GetFileManager();
+  auto file_manager = service->GetFileMixin()->GetFileManager();
   auto key = file_manager->CreateKey(kTabId);
-  service->GetTaskRunner()->PostTaskAndReplyWithResult(
+  service->GetFileMixin()->GetTaskRunner()->PostTaskAndReplyWithResult(
       FROM_HERE,
       base::BindOnce(&FileManager::DirectoryExists, file_manager, key),
       base::BindOnce([](bool exists) { EXPECT_TRUE(exists); }));
@@ -170,7 +172,7 @@ TEST_F(LongScreenshotsTabServiceTest, CaptureTabFailed) {
 
   service->DeleteAllLongScreenshotFiles();
   task_environment()->RunUntilIdle();
-  service->GetTaskRunner()->PostTaskAndReplyWithResult(
+  service->GetFileMixin()->GetTaskRunner()->PostTaskAndReplyWithResult(
       FROM_HERE,
       base::BindOnce(&FileManager::DirectoryExists, file_manager, key),
       base::BindOnce([](bool exists) { EXPECT_FALSE(exists); }));

chrome/browser/paint_preview/BUILD.gn

@@ -8,6 +8,8 @@ source_set("services") {
     "services/paint_preview_tab_service.h",
     "services/paint_preview_tab_service_factory.cc",
     "services/paint_preview_tab_service_factory.h",
+    "services/paint_preview_tab_service_file_mixin.cc",
+    "services/paint_preview_tab_service_file_mixin.h",
   ]
 
   deps = [

chrome/browser/paint_preview/services/paint_preview_tab_service.cc

@@ -14,6 +14,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/task/post_task.h"
 #include "base/task/task_traits.h"
+#include "chrome/browser/paint_preview/services/paint_preview_tab_service_file_mixin.h"
 #include "components/paint_preview/browser/file_manager.h"
 #include "components/paint_preview/browser/warm_compositor.h"
 #include "content/public/browser/render_process_host.h"
@@ -34,8 +35,6 @@ namespace {
 
 constexpr size_t kMaxPerCaptureSizeBytes = 5 * 1000L * 1000L;    // 5 MB.
 constexpr size_t kMaximumTotalCaptureSize = 25 * 1000L * 1000L;  // 25 MB.
-// The time horizon after which unused paint previews will be deleted.
-constexpr int kExpiryHorizonHrs = 72;
 
 #if defined(OS_ANDROID)
 void JavaBooleanCallbackAdapter(base::OnceCallback<void(bool)> callback,
@@ -56,22 +55,23 @@ int TabIdFromDirectoryKey(const DirectoryKey& key) {
 }  // namespace
 
 PaintPreviewTabService::PaintPreviewTabService(
-    const base::FilePath& profile_dir,
-    base::StringPiece ascii_feature_name,
+    std::unique_ptr<PaintPreviewFileMixin> file_mixin,
     std::unique_ptr<PaintPreviewPolicy> policy,
     bool is_off_the_record)
-    : PaintPreviewBaseService(profile_dir,
-                              ascii_feature_name,
+    : PaintPreviewBaseService(std::move(file_mixin),
                               std::move(policy),
                               is_off_the_record),
       cache_ready_(false) {
-  GetTaskRunner()->PostTaskAndReplyWithResult(
-      FROM_HERE, base::BindOnce(&FileManager::ListUsedKeys, GetFileManager()),
+  GetFileMixin()->GetTaskRunner()->PostTaskAndReplyWithResult(
+      FROM_HERE,
+      base::BindOnce(&FileManager::ListUsedKeys,
+                     GetFileMixin()->GetFileManager()),
       base::BindOnce(&PaintPreviewTabService::InitializeCache,
                      weak_ptr_factory_.GetWeakPtr()));
-  GetTaskRunner()->PostTaskAndReplyWithResult(
+  GetFileMixin()->GetTaskRunner()->PostTaskAndReplyWithResult(
       FROM_HERE,
-      base::BindOnce(&FileManager::GetTotalDiskUsage, GetFileManager()),
+      base::BindOnce(&FileManager::GetTotalDiskUsage,
+                     GetFileMixin()->GetFileManager()),
       base::BindOnce([](size_t size_bytes) {
         base::UmaHistogramMemoryKB(
             "Browser.PaintPreview.TabService.DiskUsageAtStartup",
@@ -111,12 +111,12 @@ void PaintPreviewTabService::CaptureTab(int tab_id,
   // to ensure the renderer doesn't go away while that happens.
   contents->IncrementCapturerCount(gfx::Size(), true);
 
-  auto file_manager = GetFileManager();
+  auto file_manager = GetFileMixin()->GetFileManager();
   auto key = file_manager->CreateKey(tab_id);
-  GetTaskRunner()->PostTaskAndReplyWithResult(
+  GetFileMixin()->GetTaskRunner()->PostTaskAndReplyWithResult(
       FROM_HERE,
-      base::BindOnce(&FileManager::CreateOrGetDirectory, GetFileManager(), key,
-                     true),
+      base::BindOnce(&FileManager::CreateOrGetDirectory,
+                     GetFileMixin()->GetFileManager(), key, true),
       base::BindOnce(&PaintPreviewTabService::CaptureTabInternal,
                      weak_ptr_factory_.GetWeakPtr(), tab_id, key,
                      contents->GetMainFrame()->GetFrameTreeNodeId(),
@@ -136,9 +136,9 @@ void PaintPreviewTabService::TabClosed(int tab_id) {
     return;
   }
 
-  auto file_manager = GetFileManager();
+  auto file_manager = GetFileMixin()->GetFileManager();
   captured_tab_ids_.erase(tab_id);
-  GetTaskRunner()->PostTask(
+  GetFileMixin()->GetTaskRunner()->PostTask(
       FROM_HERE, base::BindOnce(&FileManager::DeleteArtifactSet, file_manager,
                                 file_manager->CreateKey(tab_id)));
 }
@@ -162,24 +162,14 @@ void PaintPreviewTabService::AuditArtifacts(
     return;
   }
 
-  GetTaskRunner()->PostTaskAndReplyWithResult(
-      FROM_HERE, base::BindOnce(&FileManager::ListUsedKeys, GetFileManager()),
+  GetFileMixin()->GetTaskRunner()->PostTaskAndReplyWithResult(
+      FROM_HERE,
+      base::BindOnce(&FileManager::ListUsedKeys,
+                     GetFileMixin()->GetFileManager()),
       base::BindOnce(&PaintPreviewTabService::RunAudit,
                      weak_ptr_factory_.GetWeakPtr(), active_tab_ids));
 }
 
-void PaintPreviewTabService::GetCapturedPaintPreviewProto(
-    const DirectoryKey& key,
-    base::Optional<base::TimeDelta> expiry_horizon,
-    PaintPreviewBaseService::OnReadProtoCallback on_read_proto_callback) {
-  PaintPreviewBaseService::GetCapturedPaintPreviewProto(
-      key,
-      expiry_horizon.has_value()
-          ? expiry_horizon.value()
-          : base::TimeDelta::FromHours(kExpiryHorizonHrs),
-      std::move(on_read_proto_callback));
-}
-
 #if defined(OS_ANDROID)
 void PaintPreviewTabService::CaptureTabAndroid(
     JNIEnv* env,
@@ -220,7 +210,7 @@ jboolean PaintPreviewTabService::IsCacheInitializedAndroid(JNIEnv* env) {
 base::android::ScopedJavaLocalRef<jstring>
 PaintPreviewTabService::GetPathAndroid(JNIEnv* env) {
   return base::android::ConvertUTF8ToJavaString(
-      env, GetFileManager()->GetPath().AsUTF8Unsafe());
+      env, GetFileMixin()->GetFileManager()->GetPath().AsUTF8Unsafe());
 }
 #endif  // defined(OS_ANDROID)
 
@@ -257,9 +247,16 @@ void PaintPreviewTabService::CaptureTabInternal(
     std::move(callback).Run(Status::kWebContentsGone);
     return;
   }
+  CaptureParams capture_params;
+  capture_params.web_contents = contents;
+  capture_params.render_frame_host = rfh;
+  capture_params.root_dir = &file_path.value();
+  capture_params.persistence = RecordingPersistence::kFileSystem;
+  capture_params.clip_rect = gfx::Rect();
+  capture_params.capture_links = true;
+  capture_params.max_per_capture_size = kMaxPerCaptureSizeBytes;
   CapturePaintPreview(
-      contents, rfh, file_path.value(), gfx::Rect(), true,
-      kMaxPerCaptureSizeBytes,
+      capture_params,
       base::BindOnce(&PaintPreviewTabService::OnCaptured,
                      weak_ptr_factory_.GetWeakPtr(), tab_id, key,
                      frame_tree_node_id, std::move(callback)));
@@ -283,11 +280,12 @@ void PaintPreviewTabService::OnCaptured(
     std::move(callback).Run(Status::kCaptureFailed);
     return;
   }
-  auto file_manager = GetFileManager();
-  GetTaskRunner()->PostTaskAndReplyWithResult(
+  auto file_manager = GetFileMixin()->GetFileManager();
+  GetFileMixin()->GetTaskRunner()->PostTaskAndReplyWithResult(
       FROM_HERE,
-      base::BindOnce(&FileManager::SerializePaintPreviewProto, GetFileManager(),
-                     key, result->proto, true),
+      base::BindOnce(&FileManager::SerializePaintPreviewProto,
+                     GetFileMixin()->GetFileManager(), key, result->proto,
+                     true),
       base::BindOnce(&PaintPreviewTabService::OnFinished,
                      weak_ptr_factory_.GetWeakPtr(), tab_id,
                      std::move(callback)));
@@ -301,12 +299,13 @@ void PaintPreviewTabService::OnFinished(int tab_id,
     captured_tab_ids_.insert(tab_id);
   std::move(callback).Run(success ? Status::kOk
                                   : Status::kProtoSerializationFailed);
-  auto file_manager = GetFileManager();
-  GetTaskRunner()->PostTaskAndReplyWithResult(
+  auto file_manager = GetFileMixin()->GetFileManager();
+  GetFileMixin()->GetTaskRunner()->PostTaskAndReplyWithResult(
       FROM_HERE,
       base::BindOnce(&FileManager::GetOldestArtifactsForCleanup, file_manager,
                      kMaximumTotalCaptureSize,
-                     base::TimeDelta::FromHours(kExpiryHorizonHrs)),
+                     base::TimeDelta::FromHours(
+                         PaintPreviewTabServiceFileMixin::kExpiryHorizonHrs)),
       base::BindOnce(&PaintPreviewTabService::CleanupOldestFiles,
                      weak_ptr_factory_.GetWeakPtr(), tab_id));
 }
@@ -326,16 +325,17 @@ void PaintPreviewTabService::CleanupOldestFiles(
     keys_to_delete.push_back(key);
   }
 
-  GetTaskRunner()->PostTask(FROM_HERE,
-                            base::BindOnce(&FileManager::DeleteArtifactSets,
-                                           GetFileManager(), keys_to_delete));
+  GetFileMixin()->GetTaskRunner()->PostTask(
+      FROM_HERE,
+      base::BindOnce(&FileManager::DeleteArtifactSets,
+                     GetFileMixin()->GetFileManager(), keys_to_delete));
 }
 
 void PaintPreviewTabService::RunAudit(
     const std::vector<int>& active_tab_ids,
     const base::flat_set<DirectoryKey>& in_use_keys) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  auto file_manager = GetFileManager();
+  auto file_manager = GetFileMixin()->GetFileManager();
   std::vector<DirectoryKey> keys;
   keys.reserve(active_tab_ids.size());
   for (const auto& tab_id : active_tab_ids)
@@ -355,9 +355,10 @@ void PaintPreviewTabService::RunAudit(
   for (const auto& key : keys_to_delete)
     captured_tab_ids_.erase(TabIdFromDirectoryKey(key));
 
-  GetTaskRunner()->PostTask(FROM_HERE,
-                            base::BindOnce(&FileManager::DeleteArtifactSets,
-                                           GetFileManager(), keys_to_delete));
+  GetFileMixin()->GetTaskRunner()->PostTask(
+      FROM_HERE,
+      base::BindOnce(&FileManager::DeleteArtifactSets,
+                     GetFileMixin()->GetFileManager(), keys_to_delete));
 }
 
 }  // namespace paint_preview

chrome/browser/paint_preview/services/paint_preview_tab_service.h

@@ -38,8 +38,7 @@ namespace paint_preview {
 // browser is restarted.
 class PaintPreviewTabService : public PaintPreviewBaseService {
  public:
-  PaintPreviewTabService(const base::FilePath& profile_dir,
-                         base::StringPiece ascii_feature_name,
+  PaintPreviewTabService(std::unique_ptr<PaintPreviewFileMixin> file_mixin,
                          std::unique_ptr<PaintPreviewPolicy> policy,
                          bool is_off_the_record);
   ~PaintPreviewTabService() override;
@@ -78,14 +77,6 @@ class PaintPreviewTabService : public PaintPreviewBaseService {
   // occurred.
   void AuditArtifacts(const std::vector<int>& active_tab_ids);
 
-  // Override for GetCapturedPaintPreviewProto. Defaults expiry horizon to 72
-  // hrs if not specified.
-  void GetCapturedPaintPreviewProto(
-      const DirectoryKey& key,
-      base::Optional<base::TimeDelta> expiry_horizon,
-      PaintPreviewBaseService::OnReadProtoCallback on_read_proto_callback)
-      override;
-
 #if defined(OS_ANDROID)
   // JNI wrapped versions of the above methods
   void CaptureTabAndroid(

chrome/browser/paint_preview/services/paint_preview_tab_service_factory.cc

@@ -4,8 +4,11 @@
 
 #include "chrome/browser/paint_preview/services/paint_preview_tab_service_factory.h"
 
+#include <utility>
+
 #include "build/build_config.h"
 #include "chrome/browser/paint_preview/services/paint_preview_tab_service.h"
+#include "chrome/browser/paint_preview/services/paint_preview_tab_service_file_mixin.h"
 #include "components/keyed_service/core/simple_dependency_manager.h"
 #include "components/keyed_service/core/simple_factory_key.h"
 
@@ -52,7 +55,9 @@ PaintPreviewTabServiceFactory::BuildServiceInstanceFor(
 
   // TODO(crbug/1060556): Inject a useful policy.
   return std::make_unique<paint_preview::PaintPreviewTabService>(
-      key->GetPath(), kFeatureDirname, nullptr, key->IsOffTheRecord());
+      std::make_unique<PaintPreviewTabServiceFileMixin>(key->GetPath(),
+                                                        kFeatureDirname),
+      nullptr, key->IsOffTheRecord());
 }
 
 SimpleFactoryKey* PaintPreviewTabServiceFactory::GetKeyToUse(

chrome/browser/paint_preview/services/paint_preview_tab_service_file_mixin.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 "chrome/browser/paint_preview/services/paint_preview_tab_service_file_mixin.h"
+#include "components/paint_preview/browser/paint_preview_file_mixin.h"
+
+namespace paint_preview {
+
+PaintPreviewTabServiceFileMixin::PaintPreviewTabServiceFileMixin(
+    const base::FilePath& path,
+    base::StringPiece ascii_feature_name)
+    : PaintPreviewFileMixin(path, ascii_feature_name) {}
+
+PaintPreviewTabServiceFileMixin::~PaintPreviewTabServiceFileMixin() = default;
+
+void PaintPreviewTabServiceFileMixin::GetCapturedPaintPreviewProto(
+    const DirectoryKey& key,
+    base::Optional<base::TimeDelta> expiry_horizon,
+    OnReadProtoCallback on_read_proto_callback) {
+  PaintPreviewFileMixin::GetCapturedPaintPreviewProto(
+      key,
+      expiry_horizon.has_value()
+          ? expiry_horizon.value()
+          : base::TimeDelta::FromHours(kExpiryHorizonHrs),
+      std::move(on_read_proto_callback));
+}
+
+}  // namespace paint_preview

chrome/browser/paint_preview/services/paint_preview_tab_service_file_mixin.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 CHROME_BROWSER_PAINT_PREVIEW_SERVICES_PAINT_PREVIEW_TAB_SERVICE_FILE_MIXIN_H_
+#define CHROME_BROWSER_PAINT_PREVIEW_SERVICES_PAINT_PREVIEW_TAB_SERVICE_FILE_MIXIN_H_
+
+#include "components/paint_preview/browser/paint_preview_file_mixin.h"
+
+namespace paint_preview {
+
+class PaintPreviewTabServiceFileMixin : public PaintPreviewFileMixin {
+ public:
+  PaintPreviewTabServiceFileMixin(const base::FilePath& profile_dir,
+                                  base::StringPiece ascii_feature_name);
+  PaintPreviewTabServiceFileMixin(const PaintPreviewTabServiceFileMixin&) =
+      delete;
+  PaintPreviewTabServiceFileMixin& operator=(
+      const PaintPreviewTabServiceFileMixin&) = delete;
+  ~PaintPreviewTabServiceFileMixin() override;
+
+  // Override for GetCapturedPaintPreviewProto. Defaults expiry horizon to 72
+  // hrs if not specified.
+  void GetCapturedPaintPreviewProto(
+      const DirectoryKey& key,
+      base::Optional<base::TimeDelta> expiry_horizon,
+      OnReadProtoCallback on_read_proto_callback) override;
+
+  // The time horizon after which unused paint previews will be deleted.
+  static constexpr int kExpiryHorizonHrs = 72;
+};
+
+}  // namespace paint_preview
+
+#endif  // CHROME_BROWSER_PAINT_PREVIEW_SERVICES_PAINT_PREVIEW_TAB_SERVICE_FILE_MIXIN_H_

components/paint_preview/browser/BUILD.gn

@@ -24,6 +24,8 @@ source_set("browser") {
     "paint_preview_compositor_client_impl.h",
     "paint_preview_compositor_service_impl.cc",
     "paint_preview_compositor_service_impl.h",
+    "paint_preview_file_mixin.cc",
+    "paint_preview_file_mixin.h",
     "paint_preview_policy.h",
     "service_sandbox_type.h",
     "warm_compositor.cc",

components/paint_preview/browser/paint_preview_base_service.cc

@@ -12,10 +12,7 @@
 #include "base/files/file_path.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_functions.h"
-#include "base/task/task_traits.h"
-#include "base/task/thread_pool.h"
 #include "build/build_config.h"
-#include "components/keyed_service/core/keyed_service.h"
 #include "components/paint_preview/browser/compositor_utils.h"
 #include "components/paint_preview/browser/paint_preview_client.h"
 #include "components/paint_preview/common/mojom/paint_preview_recorder.mojom.h"
@@ -25,99 +22,23 @@
 
 namespace paint_preview {
 
-namespace {
-
-const char kPaintPreviewDir[] = "paint_preview";
-
-}  // namespace
-
 PaintPreviewBaseService::PaintPreviewBaseService(
-    const base::FilePath& path,
-    base::StringPiece ascii_feature_name,
+    std::unique_ptr<PaintPreviewFileMixin> file_mixin,
     std::unique_ptr<PaintPreviewPolicy> policy,
     bool is_off_the_record)
-    : policy_(std::move(policy)),
-      task_runner_(base::ThreadPool::CreateSequencedTaskRunner(
-          {base::MayBlock(), base::TaskPriority::USER_VISIBLE,
-           base::TaskShutdownBehavior::BLOCK_SHUTDOWN,
-           base::ThreadPolicy::MUST_USE_FOREGROUND})),
-      file_manager_(base::MakeRefCounted<FileManager>(
-          path.AppendASCII(kPaintPreviewDir).AppendASCII(ascii_feature_name),
-          task_runner_)),
+    : file_mixin_(std::move(file_mixin)),
+      policy_(std::move(policy)),
       is_off_the_record_(is_off_the_record) {}
 
 PaintPreviewBaseService::~PaintPreviewBaseService() = default;
 
-void PaintPreviewBaseService::GetCapturedPaintPreviewProto(
-    const DirectoryKey& key,
-    base::Optional<base::TimeDelta> expiry_horizon,
-    OnReadProtoCallback on_read_proto_callback) {
-  task_runner_->PostTaskAndReplyWithResult(
-      FROM_HERE,
-      base::BindOnce(
-          [](scoped_refptr<FileManager> file_manager, const DirectoryKey& key,
-             base::Optional<base::TimeDelta> expiry_horizon)
-              -> std::pair<PaintPreviewBaseService::ProtoReadStatus,
-                           std::unique_ptr<PaintPreviewProto>> {
-            if (expiry_horizon.has_value()) {
-              auto file_info = file_manager->GetInfo(key);
-              if (!file_info.has_value())
-                return std::make_pair(ProtoReadStatus::kNoProto, nullptr);
-
-              if (file_info->last_modified + expiry_horizon.value() <
-                  base::Time::NowFromSystemTime()) {
-                return std::make_pair(ProtoReadStatus::kExpired, nullptr);
-              }
-            }
-            auto result = file_manager->DeserializePaintPreviewProto(key);
-            PaintPreviewBaseService::ProtoReadStatus status =
-                ProtoReadStatus::kNoProto;
-            switch (result.first) {
-              case FileManager::ProtoReadStatus::kOk:
-                status = ProtoReadStatus::kOk;
-                break;
-              case FileManager::ProtoReadStatus::kNoProto:
-                status = ProtoReadStatus::kNoProto;
-                break;
-              case FileManager::ProtoReadStatus::kDeserializationError:
-                status = ProtoReadStatus::kDeserializationError;
-                break;
-              default:
-                NOTREACHED();
-            }
-            return std::make_pair(status, std::move(result.second));
-          },
-          file_manager_, key, expiry_horizon),
-      base::BindOnce(
-          [](OnReadProtoCallback callback,
-             std::pair<PaintPreviewBaseService::ProtoReadStatus,
-                       std::unique_ptr<PaintPreviewProto>> result) {
-            std::move(callback).Run(result.first, std::move(result.second));
-          },
-          std::move(on_read_proto_callback)));
-}
-
-void PaintPreviewBaseService::CapturePaintPreview(
-    content::WebContents* web_contents,
-    const base::FilePath& root_dir,
-    gfx::Rect clip_rect,
-    bool capture_links,
-    size_t max_per_capture_size,
-    OnCapturedCallback callback) {
-  CapturePaintPreview(web_contents, web_contents->GetMainFrame(), root_dir,
-                      clip_rect, capture_links, max_per_capture_size,
-                      std::move(callback));
-}
-
-void PaintPreviewBaseService::CapturePaintPreview(
-    content::WebContents* web_contents,
-    content::RenderFrameHost* render_frame_host,
-    const base::FilePath& root_dir,
-    gfx::Rect clip_rect,
-    bool capture_links,
-    size_t max_per_capture_size,
-    OnCapturedCallback callback) {
+void PaintPreviewBaseService::CapturePaintPreview(CaptureParams capture_params,
+                                                  OnCapturedCallback callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  content::WebContents* web_contents = capture_params.web_contents;
+  content::RenderFrameHost* render_frame_host =
+      capture_params.render_frame_host ? capture_params.render_frame_host
+                                       : web_contents->GetMainFrame();
   if (policy_ && !policy_->SupportedForContents(web_contents)) {
     std::move(callback).Run(CaptureStatus::kContentUnsupported, {});
     return;
@@ -130,14 +51,15 @@ void PaintPreviewBaseService::CapturePaintPreview(
     return;
   }
 
-  PaintPreviewClient::PaintPreviewParams params(
-      RecordingPersistence::kFileSystem);
-  params.root_dir = root_dir;
-  params.inner.clip_rect = clip_rect;
+  PaintPreviewClient::PaintPreviewParams params(capture_params.persistence);
+  if (capture_params.root_dir) {
+    params.root_dir = *capture_params.root_dir;
+  }
+  params.inner.clip_rect = capture_params.clip_rect;
   params.inner.is_main_frame =
       (render_frame_host == web_contents->GetMainFrame());
-  params.inner.capture_links = capture_links;
-  params.inner.max_capture_size = max_per_capture_size;
+  params.inner.capture_links = capture_params.capture_links;
+  params.inner.max_capture_size = capture_params.max_per_capture_size;
 
   // TODO(crbug/1064253): Consider moving to client so that this always happens.
   // Although, it is harder to get this right in the client due to its

components/paint_preview/browser/paint_preview_base_service.h

@@ -9,7 +9,6 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
-#include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
@@ -17,28 +16,32 @@
 #include "base/time/time.h"
 #include "base/unguessable_token.h"
 #include "build/build_config.h"
-#include "components/keyed_service/core/keyed_service.h"
-#include "components/paint_preview/browser/file_manager.h"
+#include "components/paint_preview/browser/paint_preview_file_mixin.h"
 #include "components/paint_preview/browser/paint_preview_policy.h"
 #include "components/paint_preview/common/capture_result.h"
 #include "components/paint_preview/common/file_utils.h"
 #include "components/paint_preview/common/mojom/paint_preview_recorder.mojom.h"
 #include "components/paint_preview/common/proto/paint_preview.pb.h"
+#include "components/paint_preview/common/serialized_recording.h"
 #include "content/public/browser/web_contents.h"
 
 namespace paint_preview {
 
-// A base KeyedService that serves as the Public API for Paint Previews.
+// A base class that serves as the Public API for Paint Previews.
 // Features that want to use Paint Previews should extend this class.
+// This service supports both in-memery and in-file captures.
+//
 // The KeyedService provides a 1:1 mapping between the service and a key or
 // profile allowing each feature built on Paint Previews to reliably store
 // necessary data to the right directory on disk.
 //
-// Implementations of this service should be created by implementing a factory
-// that extends one of:
-// - BrowserContextKeyedServiceFactory
-// OR preferably the
-// - SimpleKeyedServiceFactory
+// [NOTE] for file system captures:
+// - PaintPreviewFileMixin object needs to be supplied.
+// - Implementations of the service should be created by implementing a factory
+//   that extends one of:
+//   - BrowserContextKeyedServiceFactory
+//   OR preferably the
+//   - SimpleKeyedServiceFactory
 class PaintPreviewBaseService : public KeyedService {
  public:
   enum class CaptureStatus : int {
@@ -48,96 +51,77 @@ class PaintPreviewBaseService : public KeyedService {
     kCaptureFailed,
   };
 
-  enum class ProtoReadStatus : int {
-    kOk = 0,
-    kNoProto,
-    kDeserializationError,
-    kExpired,
+  struct CaptureParams {
+    content::WebContents* web_contents = nullptr;
+
+    // In case of specifying, an individual |render_frame_host| and its
+    // descendents will be captured. In case of nullptr, full page contents will
+    // be captured.
+    //
+    // Generally, leaving this as nullptr is what you should be doing for most
+    // features. Specifying a |render_frame_host| is intended for capturing
+    // individual subframes and should be used for only a few use cases.
+    content::RenderFrameHost* render_frame_host = nullptr;
+
+    // Store artifacts in the file system or in memory buffers.
+    RecordingPersistence persistence;
+
+    // |root_dir| should be created using
+    // GetFileManager()->CreateOrGetDirectoryFor(). However, to provide
+    // flexibility in managing the lifetime of created objects and ease cleanup
+    // if a capture fails the service implementation is responsible for
+    // implementing this management and tracking the directories in existence.
+    // Data in a directory will contain:
+    // - a number of SKPs listed as <guid>.skp (one per frame)
+    //
+    // Will be ignored if persistence = kMemoryBuffer
+    const base::FilePath* root_dir = nullptr;
+
+    // The captured area is clipped to |clip_rect| if it is non-zero.
+    gfx::Rect clip_rect;
+
+    // Whether to record links.
+    bool capture_links;
+
+    // Cap the perframe SkPicture size to |max_per_capture_size| if non-zero.
+    size_t max_per_capture_size;
   };
 
   using OnCapturedCallback =
       base::OnceCallback<void(CaptureStatus, std::unique_ptr<CaptureResult>)>;
 
-  using OnReadProtoCallback =
-      base::OnceCallback<void(ProtoReadStatus,
-                              std::unique_ptr<PaintPreviewProto>)>;
-
   // Creates a service instance for a feature. Artifacts produced will live in
   // |profile_dir|/paint_preview/|ascii_feature_name|. Implementers of the
   // factory can also elect their factory to not construct services in the event
   // a profile |is_off_the_record|. The |policy| object is responsible for
   // determining whether or not a given WebContents is amenable to paint
   // preview. If nullptr is passed as |policy| all content is deemed amenable.
-  PaintPreviewBaseService(const base::FilePath& profile_dir,
-                          base::StringPiece ascii_feature_name,
+  //
+  // NOTE: Pass nullptr as |file_mixin| if you're planning to use the service
+  // for only in-memory captures.
+  PaintPreviewBaseService(std::unique_ptr<PaintPreviewFileMixin> file_mixin,
                           std::unique_ptr<PaintPreviewPolicy> policy,
                           bool is_off_the_record);
   ~PaintPreviewBaseService() override;
 
-  // Returns the file manager for the directory associated with the service.
-  scoped_refptr<FileManager> GetFileManager() { return file_manager_; }
-
-  // Returns the task runner that IO tasks should be scheduled on.
-  scoped_refptr<base::SequencedTaskRunner> GetTaskRunner() {
-    return task_runner_;
-  }
+  PaintPreviewFileMixin* GetFileMixin() { return file_mixin_.get(); }
 
   // Returns whether the created service is off the record.
   bool IsOffTheRecord() const { return is_off_the_record_; }
 
-  // Acquires the PaintPreviewProto that is associated with |key| and sends it
-  // to |on_read_proto_callback|. The default implementation attempts to invoke
-  // GetFileManager()->DeserializePaintPreviewProto(). If |expiry_horizon| is
-  // provided a proto that was last modified earlier than |now - expiry_horizon|
-  // will return the kExpired status.
-  //
-  // Derived classes may override this function; for example, the proto is
-  // cached in memory.
-  virtual void GetCapturedPaintPreviewProto(
-      const DirectoryKey& key,
-      base::Optional<base::TimeDelta> expiry_horizon,
-      OnReadProtoCallback on_read_proto_callback);
-
   // Captures need to run on the Browser UI thread! Captures may involve child
   // frames so the PaintPreviewClient (WebContentsObserver) must be stored as
   // WebContentsUserData which is not thread safe and must only be accessible
   // from a specific sequence i.e. the UI thread.
   //
-  // The following methods both capture a Paint Preview; however, their behavior
-  // and intended use is different. The first method is intended for capturing
-  // full page contents. Generally, this is what you should be using for most
-  // features. The second method is intended for capturing just an individual
-  // RenderFrameHost and its descendents. This is intended for capturing
-  // individual subframes and should be used for only a few use cases.
+  // Captures the main frame of |capture_params.web_contents| (an observer for
+  // capturing Paint Previews is created for web contents if it does not exist).
+  // The capture is attributed to the URL of the main frame. On completion the
+  // status of the capture is provided via |callback|.
   //
-  // NOTE: |root_dir| in the following methods should be created using
-  // GetFileManager()->CreateOrGetDirectoryFor(). However, to provide
-  // flexibility in managing the lifetime of created objects and ease cleanup
-  // if a capture fails the service implementation is responsible for
-  // implementing this management and tracking the directories in existence.
-  // Data in a directory will contain:
-  // - a number of SKPs listed as <guid>.skp (one per frame)
-  //
-  // Captures the main frame of |web_contents| (an observer for capturing Paint
-  // Previews is created for web contents if it does not exist). The capture is
-  // attributed to the URL of the main frame and is stored in |root_dir|. The
-  // captured area is clipped to |clip_rect| if it is non-zero. Caps the per
-  // frame SkPicture size to |max_per_capture_size| if non-zero. On completion
-  // the status of the capture is provided via |callback|.
-  void CapturePaintPreview(content::WebContents* web_contents,
-                           const base::FilePath& root_dir,
-                           gfx::Rect clip_rect,
-                           bool capture_links,
-                           size_t max_per_capture_size,
-                           OnCapturedCallback callback);
-  // Same as above except |render_frame_host| is directly captured rather than
-  // the main frame.
-  void CapturePaintPreview(content::WebContents* web_contents,
-                           content::RenderFrameHost* render_frame_host,
-                           const base::FilePath& root_dir,
-                           gfx::Rect clip_rect,
-                           bool capture_links,
-                           size_t max_per_capture_size,
+  // See |PaintPreviewBaseService::CaptureParams| for more info about the
+  // capture parameters.
+  void CapturePaintPreview(CaptureParams capture_params,
                            OnCapturedCallback callback);
 
  private:
@@ -148,9 +132,8 @@ class PaintPreviewBaseService : public KeyedService {
                   mojom::PaintPreviewStatus status,
                   std::unique_ptr<CaptureResult> result);
 
+  std::unique_ptr<PaintPreviewFileMixin> file_mixin_ = nullptr;
   std::unique_ptr<PaintPreviewPolicy> policy_ = nullptr;
-  scoped_refptr<base::SequencedTaskRunner> task_runner_;
-  scoped_refptr<FileManager> file_manager_;
   bool is_off_the_record_;
 
   base::WeakPtrFactory<PaintPreviewBaseService> weak_ptr_factory_{this};

components/paint_preview/browser/paint_preview_base_service_test_factory.cc

@@ -40,7 +40,8 @@ void PaintPreviewBaseServiceTestFactory::Destroy(SimpleFactoryKey* key) {
 std::unique_ptr<KeyedService> PaintPreviewBaseServiceTestFactory::Build(
     SimpleFactoryKey* key) {
   return std::make_unique<PaintPreviewBaseService>(
-      key->GetPath(), kTestFeatureDir, nullptr, key->IsOffTheRecord());
+      std::make_unique<PaintPreviewFileMixin>(key->GetPath(), kTestFeatureDir),
+      nullptr, key->IsOffTheRecord());
 }
 
 PaintPreviewBaseServiceTestFactory::~PaintPreviewBaseServiceTestFactory() =

components/paint_preview/browser/paint_preview_base_service_unittest.cc

@@ -4,6 +4,9 @@
 
 #include "components/paint_preview/browser/paint_preview_base_service.h"
 
+#include <memory>
+#include <utility>
+
 #include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
 #include "base/no_destructor.h"
@@ -11,6 +14,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "components/paint_preview/browser/paint_preview_base_service_test_factory.h"
+#include "components/paint_preview/browser/paint_preview_file_mixin.h"
 #include "components/paint_preview/common/mojom/paint_preview_recorder.mojom.h"
 #include "components/paint_preview/common/test_utils.h"
 #include "content/public/browser/render_process_host.h"
@@ -46,7 +50,7 @@ class RejectionPaintPreviewPolicy : public PaintPreviewPolicy {
 std::unique_ptr<KeyedService> BuildServiceWithRejectionPolicy(
     SimpleFactoryKey* key) {
   return std::make_unique<PaintPreviewBaseService>(
-      key->GetPath(), kTestFeatureDir,
+      std::make_unique<PaintPreviewFileMixin>(key->GetPath(), kTestFeatureDir),
       std::make_unique<RejectionPaintPreviewPolicy>(), key->IsOffTheRecord());
 }
 
@@ -165,6 +169,23 @@ class PaintPreviewBaseServiceTest : public content::RenderViewHostTestHarness {
         rejection_policy_key_.get());
   }
 
+  PaintPreviewBaseService::CaptureParams CreateCaptureParams(
+      content::WebContents* web_contents,
+      base::FilePath* root_dir,
+      RecordingPersistence persistence,
+      gfx::Rect clip_rect,
+      bool capture_links,
+      size_t max_per_capture_size) {
+    PaintPreviewBaseService::CaptureParams capture_params;
+    capture_params.web_contents = web_contents;
+    capture_params.root_dir = root_dir;
+    capture_params.persistence = persistence;
+    capture_params.clip_rect = clip_rect;
+    capture_params.capture_links = capture_links;
+    capture_params.max_per_capture_size = max_per_capture_size;
+    return capture_params;
+  }
+
  private:
   std::unique_ptr<SimpleFactoryKey> key_ = nullptr;
   std::unique_ptr<SimpleFactoryKey> rejection_policy_key_ = nullptr;
@@ -184,13 +205,15 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureMainFrame) {
 
   auto* service = GetService();
   EXPECT_FALSE(service->IsOffTheRecord());
-  auto manager = service->GetFileManager();
+  auto manager = service->GetFileMixin()->GetFileManager();
   base::FilePath path = CreateDir(
       manager, manager->CreateKey(web_contents()->GetLastCommittedURL()));
 
   base::RunLoop loop;
   service->CapturePaintPreview(
-      web_contents(), path, gfx::Rect(0, 0, 0, 0), true, 50,
+      CreateCaptureParams(web_contents(), &path,
+                          RecordingPersistence::kFileSystem,
+                          gfx::Rect(0, 0, 0, 0), true, 50),
       base::BindOnce(
           [](base::OnceClosure quit_closure,
              PaintPreviewBaseService::CaptureStatus expected_status,
@@ -224,6 +247,8 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureMainFrame) {
   loop.Run();
 }
 
+// TODO(crbug/1152163): Add tests for in-memory capture.
+
 TEST_F(PaintPreviewBaseServiceTest, CaptureFailed) {
   MockPaintPreviewRecorder recorder;
   auto params = mojom::PaintPreviewCaptureParams::New();
@@ -238,13 +263,15 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureFailed) {
 
   auto* service = GetService();
   EXPECT_FALSE(service->IsOffTheRecord());
-  auto manager = service->GetFileManager();
+  auto manager = service->GetFileMixin()->GetFileManager();
   base::FilePath path = CreateDir(
       manager, manager->CreateKey(web_contents()->GetLastCommittedURL()));
 
   base::RunLoop loop;
   service->CapturePaintPreview(
-      web_contents(), path, gfx::Rect(0, 0, 0, 0), true, 0,
+      CreateCaptureParams(web_contents(), &path,
+                          RecordingPersistence::kFileSystem,
+                          gfx::Rect(0, 0, 0, 0), true, 0),
       base::BindOnce(
           [](base::OnceClosure quit_closure,
              PaintPreviewBaseService::CaptureStatus expected_status,
@@ -273,13 +300,15 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureDisallowed) {
 
   auto* service = GetServiceWithRejectionPolicy();
   EXPECT_FALSE(service->IsOffTheRecord());
-  auto manager = service->GetFileManager();
+  auto manager = service->GetFileMixin()->GetFileManager();
   base::FilePath path = CreateDir(
       manager, manager->CreateKey(web_contents()->GetLastCommittedURL()));
 
   base::RunLoop loop;
   service->CapturePaintPreview(
-      web_contents(), path, gfx::Rect(0, 0, 0, 0), true, 0,
+      CreateCaptureParams(web_contents(), &path,
+                          RecordingPersistence::kFileSystem,
+                          gfx::Rect(0, 0, 0, 0), true, 0),
       base::BindOnce(
           [](base::OnceClosure quit_closure,
              PaintPreviewBaseService::CaptureStatus expected_status,

components/paint_preview/browser/paint_preview_file_mixin.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 "components/paint_preview/browser/paint_preview_file_mixin.h"
+
+#include <utility>
+
+#include "base/task/task_traits.h"
+#include "base/task/thread_pool.h"
+
+namespace paint_preview {
+
+namespace {
+
+const char kPaintPreviewDir[] = "paint_preview";
+
+}  // namespace
+
+PaintPreviewFileMixin::PaintPreviewFileMixin(
+    const base::FilePath& path,
+    base::StringPiece ascii_feature_name)
+    : task_runner_(base::ThreadPool::CreateSequencedTaskRunner(
+          {base::MayBlock(), base::TaskPriority::USER_VISIBLE,
+           base::TaskShutdownBehavior::BLOCK_SHUTDOWN,
+           base::ThreadPolicy::MUST_USE_FOREGROUND})),
+      file_manager_(base::MakeRefCounted<FileManager>(
+          path.AppendASCII(kPaintPreviewDir).AppendASCII(ascii_feature_name),
+          task_runner_)) {}
+
+PaintPreviewFileMixin::~PaintPreviewFileMixin() = default;
+
+void PaintPreviewFileMixin::GetCapturedPaintPreviewProto(
+    const DirectoryKey& key,
+    base::Optional<base::TimeDelta> expiry_horizon,
+    OnReadProtoCallback on_read_proto_callback) {
+  task_runner_->PostTaskAndReplyWithResult(
+      FROM_HERE,
+      base::BindOnce(
+          [](scoped_refptr<FileManager> file_manager, const DirectoryKey& key,
+             base::Optional<base::TimeDelta> expiry_horizon)
+              -> std::pair<PaintPreviewFileMixin::ProtoReadStatus,
+                           std::unique_ptr<PaintPreviewProto>> {
+            if (expiry_horizon.has_value()) {
+              auto file_info = file_manager->GetInfo(key);
+              if (!file_info.has_value())
+                return std::make_pair(ProtoReadStatus::kNoProto, nullptr);
+
+              if (file_info->last_modified + expiry_horizon.value() <
+                  base::Time::NowFromSystemTime()) {
+                return std::make_pair(ProtoReadStatus::kExpired, nullptr);
+              }
+            }
+            auto result = file_manager->DeserializePaintPreviewProto(key);
+            PaintPreviewFileMixin::ProtoReadStatus status =
+                ProtoReadStatus::kNoProto;
+            switch (result.first) {
+              case FileManager::ProtoReadStatus::kOk:
+                status = ProtoReadStatus::kOk;
+                break;
+              case FileManager::ProtoReadStatus::kNoProto:
+                status = ProtoReadStatus::kNoProto;
+                break;
+              case FileManager::ProtoReadStatus::kDeserializationError:
+                status = ProtoReadStatus::kDeserializationError;
+                break;
+              default:
+                NOTREACHED();
+            }
+            return std::make_pair(status, std::move(result.second));
+          },
+          file_manager_, key, expiry_horizon),
+      base::BindOnce(
+          [](OnReadProtoCallback callback,
+             std::pair<PaintPreviewFileMixin::ProtoReadStatus,
+                       std::unique_ptr<PaintPreviewProto>> result) {
+            std::move(callback).Run(result.first, std::move(result.second));
+          },
+          std::move(on_read_proto_callback)));
+}
+
+}  // namespace paint_preview

components/paint_preview/browser/paint_preview_file_mixin.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 COMPONENTS_PAINT_PREVIEW_BROWSER_PAINT_PREVIEW_FILE_MIXIN_H_
+#define COMPONENTS_PAINT_PREVIEW_BROWSER_PAINT_PREVIEW_FILE_MIXIN_H_
+
+#include "base/files/file_path.h"
+#include "components/keyed_service/core/keyed_service.h"
+#include "components/paint_preview/browser/file_manager.h"
+
+namespace paint_preview {
+
+class PaintPreviewFileMixin {
+ public:
+  enum class ProtoReadStatus : int {
+    kOk = 0,
+    kNoProto,
+    kDeserializationError,
+    kExpired,
+  };
+
+  using OnReadProtoCallback =
+      base::OnceCallback<void(ProtoReadStatus,
+                              std::unique_ptr<PaintPreviewProto>)>;
+
+  // Creates an instance for a profile. FileManager's root directory will be set
+  // to |profile_dir|/paint_preview/|ascii_feature_name|.
+  PaintPreviewFileMixin(const base::FilePath& profile_dir,
+                        base::StringPiece ascii_feature_name);
+  PaintPreviewFileMixin(const PaintPreviewFileMixin&) = delete;
+  PaintPreviewFileMixin& operator=(const PaintPreviewFileMixin&) = delete;
+  virtual ~PaintPreviewFileMixin();
+
+  // Returns the file manager for the directory associated with the profile.
+  scoped_refptr<FileManager> GetFileManager() { return file_manager_; }
+
+  // Returns the task runner that IO tasks should be scheduled on.
+  scoped_refptr<base::SequencedTaskRunner> GetTaskRunner() {
+    return task_runner_;
+  }
+
+  // Acquires the PaintPreviewProto that is associated with |key| and sends it
+  // to |on_read_proto_callback|. The default implementation attempts to invoke
+  // GetFileManager()->DeserializePaintPreviewProto(). If |expiry_horizon| is
+  // provided a proto that was last modified earlier than 'now - expiry_horizon'
+  // will return the kExpired status.
+  virtual void GetCapturedPaintPreviewProto(
+      const DirectoryKey& key,
+      base::Optional<base::TimeDelta> expiry_horizon,
+      OnReadProtoCallback on_read_proto_callback);
+
+ private:
+  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+  scoped_refptr<FileManager> file_manager_;
+};
+
+}  // namespace paint_preview
+
+#endif  // COMPONENTS_PAINT_PREVIEW_BROWSER_PAINT_PREVIEW_FILE_HELPER_H_

components/paint_preview/player/android/javatests/paint_preview_test_service.cc

@@ -5,6 +5,7 @@
 #include "components/paint_preview/player/android/javatests/paint_preview_test_service.h"
 
 #include <memory>
+#include <utility>
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
@@ -107,10 +108,10 @@ jlong JNI_PaintPreviewTestService_GetInstance(
 }
 
 PaintPreviewTestService::PaintPreviewTestService(const base::FilePath& path)
-    : PaintPreviewBaseService(path,
-                              kTestDirName,
-                              std::make_unique<TestPaintPreviewPolicy>(),
-                              false),
+    : PaintPreviewBaseService(
+          std::make_unique<PaintPreviewFileMixin>(path, kTestDirName),
+          std::make_unique<TestPaintPreviewPolicy>(),
+          false),
       test_data_dir_(
           path.AppendASCII(kPaintPreviewDir).AppendASCII(kTestDirName)) {}
 

components/paint_preview/player/player_compositor_delegate.cc

@@ -105,10 +105,11 @@ PlayerCompositorDelegate::PlayerCompositorDelegate()
 
 PlayerCompositorDelegate::~PlayerCompositorDelegate() {
   if (compress_on_close_ && paint_preview_service_) {
-    paint_preview_service_->GetTaskRunner()->PostTask(
+    paint_preview_service_->GetFileMixin()->GetTaskRunner()->PostTask(
         FROM_HERE,
         base::BindOnce(base::IgnoreResult(&FileManager::CompressDirectory),
-                       paint_preview_service_->GetFileManager(), key_));
+                       paint_preview_service_->GetFileMixin()->GetFileManager(),
+                       key_));
   }
 }
 
@@ -313,7 +314,7 @@ void PlayerCompositorDelegate::OnCompositorClientCreated(
   TRACE_EVENT_NESTABLE_ASYNC_END0("paint_preview",
                                   "PlayerCompositorDelegate CreateCompositor",
                                   TRACE_ID_LOCAL(this));
-  paint_preview_service_->GetCapturedPaintPreviewProto(
+  paint_preview_service_->GetFileMixin()->GetCapturedPaintPreviewProto(
       key, base::nullopt,
       base::BindOnce(&PlayerCompositorDelegate::OnProtoAvailable,
                      weak_factory_.GetWeakPtr(), expected_url));
@@ -321,20 +322,20 @@ void PlayerCompositorDelegate::OnCompositorClientCreated(
 
 void PlayerCompositorDelegate::OnProtoAvailable(
     const GURL& expected_url,
-    PaintPreviewBaseService::ProtoReadStatus proto_status,
+    PaintPreviewFileMixin::ProtoReadStatus proto_status,
     std::unique_ptr<PaintPreviewProto> proto) {
-  if (proto_status == PaintPreviewBaseService::ProtoReadStatus::kExpired) {
+  if (proto_status == PaintPreviewFileMixin::ProtoReadStatus::kExpired) {
     OnCompositorReady(CompositorStatus::CAPTURE_EXPIRED, nullptr);
     return;
   }
 
-  if (proto_status == PaintPreviewBaseService::ProtoReadStatus::kNoProto) {
+  if (proto_status == PaintPreviewFileMixin::ProtoReadStatus::kNoProto) {
     OnCompositorReady(CompositorStatus::NO_CAPTURE, nullptr);
     return;
   }
 
   if (proto_status ==
-          PaintPreviewBaseService::ProtoReadStatus::kDeserializationError ||
+          PaintPreviewFileMixin::ProtoReadStatus::kDeserializationError ||
       !proto || !proto->IsInitialized()) {
     OnCompositorReady(CompositorStatus::PROTOBUF_DESERIALIZATION_ERROR,
                       nullptr);

components/paint_preview/player/player_compositor_delegate.h

@@ -141,7 +141,7 @@ class PlayerCompositorDelegate {
   void OnCompositorTimeout();
 
   void OnProtoAvailable(const GURL& expected_url,
-                        PaintPreviewBaseService::ProtoReadStatus proto_status,
+                        PaintPreviewFileMixin::ProtoReadStatus proto_status,
                         std::unique_ptr<PaintPreviewProto> proto);
 
   void SendCompositeRequest(

components/paint_preview/player/player_compositor_delegate_unittest.cc

@@ -220,7 +220,8 @@ class PlayerCompositorDelegateTest : public testing::Test {
   void SetUp() override {
     ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
     service_ = std::make_unique<PaintPreviewBaseService>(
-        temp_dir.GetPath(), "test", nullptr, false);
+        std::make_unique<PaintPreviewFileMixin>(temp_dir.GetPath(), "test"),
+        nullptr, false);
   }
 
   PaintPreviewBaseService* GetBaseService() { return service_.get(); }
@@ -251,7 +252,7 @@ class PlayerCompositorDelegateTest : public testing::Test {
 
   void SerializeProtoAndCreateRootSkp(PaintPreviewProto* proto,
                                       const DirectoryKey& key) {
-    auto file_manager = GetBaseService()->GetFileManager();
+    auto file_manager = GetBaseService()->GetFileMixin()->GetFileManager();
     base::RunLoop loop;
     file_manager->GetTaskRunner()->PostTask(
         FROM_HERE,
@@ -283,7 +284,7 @@ class PlayerCompositorDelegateTest : public testing::Test {
 
 TEST_F(PlayerCompositorDelegateTest, OnClick) {
   auto* service = GetBaseService();
-  auto file_manager = service->GetFileManager();
+  auto file_manager = service->GetFileMixin()->GetFileManager();
   auto key = file_manager->CreateKey(1U);
 
   GURL url("www.example.com");
@@ -371,7 +372,7 @@ TEST_F(PlayerCompositorDelegateTest, OnClick) {
 
 TEST_F(PlayerCompositorDelegateTest, BadProto) {
   auto* service = GetBaseService();
-  auto file_manager = service->GetFileManager();
+  auto file_manager = service->GetFileMixin()->GetFileManager();
   auto key = file_manager->CreateKey(1U);
   base::RunLoop loop;
   file_manager->GetTaskRunner()->PostTask(
@@ -401,7 +402,7 @@ TEST_F(PlayerCompositorDelegateTest, BadProto) {
 
 TEST_F(PlayerCompositorDelegateTest, OldVersion) {
   auto* service = GetBaseService();
-  auto file_manager = service->GetFileManager();
+  auto file_manager = service->GetFileMixin()->GetFileManager();
   auto key = file_manager->CreateKey(1U);
   GURL url("https://www.chromium.org/");
   auto proto = CreateValidProto(url);
@@ -422,7 +423,7 @@ TEST_F(PlayerCompositorDelegateTest, OldVersion) {
 
 TEST_F(PlayerCompositorDelegateTest, URLMismatch) {
   auto* service = GetBaseService();
-  auto file_manager = service->GetFileManager();
+  auto file_manager = service->GetFileMixin()->GetFileManager();
   auto key = file_manager->CreateKey(1U);
   GURL url("https://www.chromium.org/");
   auto proto = CreateValidProto(url);
@@ -442,7 +443,7 @@ TEST_F(PlayerCompositorDelegateTest, URLMismatch) {
 
 TEST_F(PlayerCompositorDelegateTest, ServiceDisconnect) {
   auto* service = GetBaseService();
-  auto file_manager = service->GetFileManager();
+  auto file_manager = service->GetFileMixin()->GetFileManager();
   auto key = file_manager->CreateKey(1U);
   GURL url("https://www.chromium.org/");
   auto proto = CreateValidProto(url);
@@ -474,7 +475,7 @@ TEST_F(PlayerCompositorDelegateTest, ServiceDisconnect) {
 
 TEST_F(PlayerCompositorDelegateTest, ClientDisconnect) {
   auto* service = GetBaseService();
-  auto file_manager = service->GetFileManager();
+  auto file_manager = service->GetFileMixin()->GetFileManager();
   auto key = file_manager->CreateKey(1U);
   GURL url("https://www.chromium.org/");
   auto proto = CreateValidProto(url);
@@ -505,7 +506,7 @@ TEST_F(PlayerCompositorDelegateTest, ClientDisconnect) {
 
 TEST_F(PlayerCompositorDelegateTest, InvalidCompositeRequest) {
   auto* service = GetBaseService();
-  auto file_manager = service->GetFileManager();
+  auto file_manager = service->GetFileMixin()->GetFileManager();
   auto key = file_manager->CreateKey(1U);
   GURL url("https://www.chromium.org/");
   auto proto = CreateValidProto(url);
@@ -536,7 +537,7 @@ TEST_F(PlayerCompositorDelegateTest, InvalidCompositeRequest) {
 
 TEST_F(PlayerCompositorDelegateTest, CompositorDeserializationError) {
   auto* service = GetBaseService();
-  auto file_manager = service->GetFileManager();
+  auto file_manager = service->GetFileMixin()->GetFileManager();
   auto key = file_manager->CreateKey(1U);
   GURL url("https://www.chromium.org/");
   auto proto = CreateValidProto(url);
@@ -560,7 +561,7 @@ TEST_F(PlayerCompositorDelegateTest, CompositorDeserializationError) {
 
 TEST_F(PlayerCompositorDelegateTest, InvalidRootSkp) {
   auto* service = GetBaseService();
-  auto file_manager = service->GetFileManager();
+  auto file_manager = service->GetFileMixin()->GetFileManager();
   auto key = file_manager->CreateKey(1U);
   GURL url("https://www.chromium.org/");
   auto proto = CreateValidProto(url);
@@ -584,7 +585,7 @@ TEST_F(PlayerCompositorDelegateTest, InvalidRootSkp) {
 
 TEST_F(PlayerCompositorDelegateTest, CompressOnClose) {
   auto* service = GetBaseService();
-  auto file_manager = service->GetFileManager();
+  auto file_manager = service->GetFileMixin()->GetFileManager();
   auto key = file_manager->CreateKey(1U);
   base::FilePath dir;
   file_manager->GetTaskRunner()->PostTaskAndReplyWithResult(
@@ -616,7 +617,7 @@ TEST_F(PlayerCompositorDelegateTest, CompressOnClose) {
 
 TEST_F(PlayerCompositorDelegateTest, RequestBitmapWithCancel) {
   auto* service = GetBaseService();
-  auto file_manager = service->GetFileManager();
+  auto file_manager = service->GetFileMixin()->GetFileManager();
   auto key = file_manager->CreateKey(1U);
   {
     // This test skips setting up files as the fakes don't use them. In normal
@@ -687,7 +688,7 @@ TEST_F(PlayerCompositorDelegateTest, RequestBitmapWithCancel) {
 
 TEST_F(PlayerCompositorDelegateTest, RequestBitmapWithCancelAll) {
   auto* service = GetBaseService();
-  auto file_manager = service->GetFileManager();
+  auto file_manager = service->GetFileMixin()->GetFileManager();
   auto key = file_manager->CreateKey(1U);
   {
     // This test skips setting up files as the fakes don't use them. In normal
@@ -739,7 +740,7 @@ TEST_F(PlayerCompositorDelegateTest, RequestBitmapWithCancelAll) {
 
 TEST_F(PlayerCompositorDelegateTest, RequestBitmapSuccessQueued) {
   auto* service = GetBaseService();
-  auto file_manager = service->GetFileManager();
+  auto file_manager = service->GetFileMixin()->GetFileManager();
   auto key = file_manager->CreateKey(1U);
   {
     // This test skips setting up files as the fakes don't use them. In normal
@@ -772,7 +773,7 @@ TEST_F(PlayerCompositorDelegateTest, RequestBitmapSuccessQueued) {
 
 TEST_F(PlayerCompositorDelegateTest, Timeout) {
   auto* service = GetBaseService();
-  auto file_manager = service->GetFileManager();
+  auto file_manager = service->GetFileMixin()->GetFileManager();
   auto key = file_manager->CreateKey(1U);
   {
     PlayerCompositorDelegateImpl player_compositor_delegate;
@@ -798,7 +799,7 @@ TEST_F(PlayerCompositorDelegateTest, Timeout) {
 
 TEST_F(PlayerCompositorDelegateTest, CriticalMemoryPressure) {
   auto* service = GetBaseService();
-  auto file_manager = service->GetFileManager();
+  auto file_manager = service->GetFileMixin()->GetFileManager();
   auto key = file_manager->CreateKey(1U);
   {
     // This test skips setting up files as the fakes don't use them. In normal
@@ -831,7 +832,7 @@ TEST_F(PlayerCompositorDelegateTest, CriticalMemoryPressure) {
 
 TEST_F(PlayerCompositorDelegateTest, CriticalMemoryPressureBeforeStart) {
   auto* service = GetBaseService();
-  auto file_manager = service->GetFileManager();
+  auto file_manager = service->GetFileMixin()->GetFileManager();
   auto key = file_manager->CreateKey(1U);
   {
     // This test skips setting up files as the fakes don't use them. In normal