Allow PaintPreviewCapture to be parameterized over where to store its intermediate artifacts.

Allows for subframe recordings to be saved to either the disk or memory buffers.

Change-Id: I0630babd421716ed9bf354e22a3b8eca8ce46932
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2274166Reviewed-by: Calder Kitagawa <ckitagawa@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791529}

chrome/browser/paint_preview/paint_preview_browsertest.cc

@@ -16,7 +16,10 @@
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/paint_preview/browser/paint_preview_client.h"
 #include "components/paint_preview/common/file_stream.h"
+#include "components/paint_preview/common/mojom/paint_preview_recorder.mojom-shared.h"
+#include "components/paint_preview/common/proto/paint_preview.pb.h"
 #include "components/paint_preview/common/serial_utils.h"
+#include "components/paint_preview/common/test_utils.h"
 #include "components/ukm/test_ukm_recorder.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/common/content_switches.h"
@@ -25,6 +28,7 @@
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "services/metrics/public/cpp/ukm_builders.h"
 #include "third_party/skia/include/core/SkPicture.h"
+#include "third_party/skia/include/core/SkStream.h"
 #include "url/gurl.h"
 
 namespace paint_preview {
@@ -42,16 +46,32 @@ base::FilePath ProtoPathToFilePath(const base::StringPiece& proto_path) {
 // Check that |path| points to a valid SkPicture. Don't bother checking the
 // contents as this is non-trivial and could change. Instead check that
 // the SkPicture can be read correctly and has a cull rect of at least |size|.
-void EnsureSkPictureIsValid(const base::FilePath& path,
-                            size_t expected_subframe_count,
-                            const gfx::Size& size = gfx::Size(1, 1)) {
+void EnsureSkPictureIsValid(
+    const PaintPreviewClient::PaintPreviewParams& params,
+    const CaptureResult* const result,
+    const PaintPreviewFrameProto& frame_proto,
+    size_t expected_subframe_count,
+    const gfx::Size& size = gfx::Size(1, 1)) {
   base::ScopedAllowBlockingForTesting scoped_blocking;
-  EXPECT_TRUE(base::PathExists(path));
-  FileRStream rstream(
-      base::File(path, base::File::FLAG_OPEN | base::File::FLAG_READ));
+
+  std::unique_ptr<SkStream> stream;
+  if (params.persistence == mojom::Persistence::kFileSystem) {
+    base::FilePath path = ProtoPathToFilePath(frame_proto.file_path());
+    EXPECT_TRUE(base::PathExists(path));
+    stream = std::make_unique<FileRStream>(
+        base::File(path, base::File::FLAG_OPEN | base::File::FLAG_READ));
+  } else {
+    base::UnguessableToken frame_guid = base::UnguessableToken::Deserialize(
+        frame_proto.embedding_token_high(), frame_proto.embedding_token_low());
+    const mojo_base::BigBuffer& buffer = result->serialized_skps.at(frame_guid);
+    EXPECT_TRUE(buffer.size() > 0);
+    stream = std::make_unique<SkMemoryStream>(buffer.data(), buffer.size(),
+                                              /*copy_data=*/false);
+  }
+
   DeserializationContext ctx;
   auto deserial_procs = MakeDeserialProcs(&ctx);
-  auto skp = SkPicture::MakeFromStream(&rstream, &deserial_procs);
+  auto skp = SkPicture::MakeFromStream(stream.get(), &deserial_procs);
   EXPECT_NE(skp, nullptr);
   EXPECT_GE(skp->cullRect().width(), 0);
   EXPECT_GE(skp->cullRect().height(), 0);
@@ -64,7 +84,9 @@ void EnsureSkPictureIsValid(const base::FilePath& path,
 // - Each RenderFrame has an instance of PaintPreviewRecorder attached.
 // - Each WebContents has an instance of PaintPreviewClient attached.
 // This permits end-to-end testing of the flow of paint previews.
-class PaintPreviewBrowserTest : public InProcessBrowserTest {
+class PaintPreviewBrowserTest
+    : public InProcessBrowserTest,
+      public testing::WithParamInterface<mojom::Persistence> {
  protected:
   PaintPreviewBrowserTest() = default;
   ~PaintPreviewBrowserTest() override = default;
@@ -107,11 +129,10 @@ class PaintPreviewBrowserTest : public InProcessBrowserTest {
   }
 
   PaintPreviewClient::PaintPreviewParams MakeParams() const {
-    PaintPreviewClient::PaintPreviewParams params;
-    params.document_guid = base::UnguessableToken::Create();
-    params.is_main_frame = true;
+    PaintPreviewClient::PaintPreviewParams params(GetParam());
+    params.inner.is_main_frame = true;
     params.root_dir = temp_dir_.GetPath();
-    params.max_per_capture_size = 0;
+    params.inner.max_per_capture_size = 0;
     return params;
   }
 
@@ -137,7 +158,7 @@ class PaintPreviewBrowserTest : public InProcessBrowserTest {
   PaintPreviewBrowserTest& operator=(const PaintPreviewBrowserTest&) = delete;
 };
 
-IN_PROC_BROWSER_TEST_F(PaintPreviewBrowserTest, CaptureFrame) {
+IN_PROC_BROWSER_TEST_P(PaintPreviewBrowserTest, CaptureFrame) {
   LoadPage(http_server_.GetURL("a.com", "/cross_site_iframe_factory.html?a"));
   ukm::TestAutoSetUkmRecorder ukm_recorder;
   auto params = MakeParams();
@@ -150,21 +171,23 @@ IN_PROC_BROWSER_TEST_F(PaintPreviewBrowserTest, CaptureFrame) {
   client->CapturePaintPreview(
       params, GetWebContents()->GetMainFrame(),
       base::BindOnce(
-          [](base::RepeatingClosure quit, base::UnguessableToken expected_guid,
+          [](base::RepeatingClosure quit,
+             const PaintPreviewClient::PaintPreviewParams& params,
              base::UnguessableToken guid, mojom::PaintPreviewStatus status,
-             std::unique_ptr<PaintPreviewProto> proto) {
-            EXPECT_EQ(guid, expected_guid);
+             std::unique_ptr<CaptureResult> result) {
+            EXPECT_EQ(guid, params.inner.document_guid);
             EXPECT_EQ(status, mojom::PaintPreviewStatus::kOk);
-            EXPECT_TRUE(proto->has_root_frame());
-            EXPECT_EQ(proto->subframes_size(), 0);
-            EXPECT_EQ(proto->root_frame().content_id_to_embedding_tokens_size(),
+            EXPECT_TRUE(result->proto.has_root_frame());
+            EXPECT_EQ(result->proto.subframes_size(), 0);
+            EXPECT_EQ(result->proto.root_frame()
+                          .content_id_to_embedding_tokens_size(),
                       0);
-            EXPECT_TRUE(proto->root_frame().is_main_frame());
-            EnsureSkPictureIsValid(
-                ProtoPathToFilePath(proto->root_frame().file_path()), 0);
+            EXPECT_TRUE(result->proto.root_frame().is_main_frame());
+            EnsureSkPictureIsValid(params, result.get(),
+                                   result->proto.root_frame(), 0);
             quit.Run();
           },
-          loop.QuitClosure(), params.document_guid));
+          loop.QuitClosure(), params));
   loop.Run();
 
   auto entries = ukm_recorder.GetEntriesByName(
@@ -172,7 +195,7 @@ IN_PROC_BROWSER_TEST_F(PaintPreviewBrowserTest, CaptureFrame) {
   EXPECT_EQ(1u, entries.size());
 }
 
-IN_PROC_BROWSER_TEST_F(PaintPreviewBrowserTest,
+IN_PROC_BROWSER_TEST_P(PaintPreviewBrowserTest,
                        CaptureMainFrameWithCrossProcessSubframe) {
   LoadPage(
       http_server_.GetURL("a.com", "/cross_site_iframe_factory.html?a(b)"));
@@ -186,30 +209,33 @@ IN_PROC_BROWSER_TEST_F(PaintPreviewBrowserTest,
   client->CapturePaintPreview(
       params, GetWebContents()->GetMainFrame(),
       base::BindOnce(
-          [](base::RepeatingClosure quit, base::UnguessableToken expected_guid,
+          [](base::RepeatingClosure quit,
+             const PaintPreviewClient::PaintPreviewParams& params,
              base::UnguessableToken guid, mojom::PaintPreviewStatus status,
-             std::unique_ptr<PaintPreviewProto> proto) {
-            EXPECT_EQ(guid, expected_guid);
+             std::unique_ptr<CaptureResult> result) {
+            EXPECT_EQ(guid, params.inner.document_guid);
             EXPECT_EQ(status, mojom::PaintPreviewStatus::kOk);
-            EXPECT_TRUE(proto->has_root_frame());
-            EXPECT_EQ(proto->subframes_size(), 1);
-            EXPECT_EQ(proto->root_frame().content_id_to_embedding_tokens_size(),
+            EXPECT_TRUE(result->proto.has_root_frame());
+            EXPECT_EQ(result->proto.subframes_size(), 1);
+            EXPECT_EQ(result->proto.root_frame()
+                          .content_id_to_embedding_tokens_size(),
                       1);
-            EXPECT_TRUE(proto->root_frame().is_main_frame());
-            EXPECT_EQ(proto->subframes(0).content_id_to_embedding_tokens_size(),
+            EXPECT_TRUE(result->proto.root_frame().is_main_frame());
+            EXPECT_EQ(result->proto.subframes(0)
+                          .content_id_to_embedding_tokens_size(),
                       0);
-            EXPECT_FALSE(proto->subframes(0).is_main_frame());
-            EnsureSkPictureIsValid(
-                ProtoPathToFilePath(proto->root_frame().file_path()), 1);
-            EnsureSkPictureIsValid(
-                ProtoPathToFilePath(proto->subframes(0).file_path()), 0);
+            EXPECT_FALSE(result->proto.subframes(0).is_main_frame());
+            EnsureSkPictureIsValid(params, result.get(),
+                                   result->proto.root_frame(), 1);
+            EnsureSkPictureIsValid(params, result.get(),
+                                   result->proto.subframes(0), 0);
             quit.Run();
           },
-          loop.QuitClosure(), params.document_guid));
+          loop.QuitClosure(), params));
   loop.Run();
 }
 
-IN_PROC_BROWSER_TEST_F(PaintPreviewBrowserTest,
+IN_PROC_BROWSER_TEST_P(PaintPreviewBrowserTest,
                        CaptureMainFrameWithScrollableSameProcessSubframe) {
   std::string html = R"(<html>
                           <iframe
@@ -234,31 +260,34 @@ IN_PROC_BROWSER_TEST_F(PaintPreviewBrowserTest,
   client->CapturePaintPreview(
       params, GetWebContents()->GetMainFrame(),
       base::BindOnce(
-          [](base::RepeatingClosure quit, base::UnguessableToken expected_guid,
+          [](base::RepeatingClosure quit,
+             const PaintPreviewClient::PaintPreviewParams& params,
              base::UnguessableToken guid, mojom::PaintPreviewStatus status,
-             std::unique_ptr<PaintPreviewProto> proto) {
-            EXPECT_EQ(guid, expected_guid);
+             std::unique_ptr<CaptureResult> result) {
+            EXPECT_EQ(guid, params.inner.document_guid);
             EXPECT_EQ(status, mojom::PaintPreviewStatus::kOk);
-            EXPECT_TRUE(proto->has_root_frame());
-            EXPECT_EQ(proto->subframes_size(), 1);
-            EXPECT_EQ(proto->root_frame().content_id_to_embedding_tokens_size(),
+            EXPECT_TRUE(result->proto.has_root_frame());
+            EXPECT_EQ(result->proto.subframes_size(), 1);
+            EXPECT_EQ(result->proto.root_frame()
+                          .content_id_to_embedding_tokens_size(),
                       1);
-            EXPECT_TRUE(proto->root_frame().is_main_frame());
-            EXPECT_EQ(proto->subframes(0).content_id_to_embedding_tokens_size(),
+            EXPECT_TRUE(result->proto.root_frame().is_main_frame());
+            EXPECT_EQ(result->proto.subframes(0)
+                          .content_id_to_embedding_tokens_size(),
                       0);
-            EXPECT_FALSE(proto->subframes(0).is_main_frame());
-            EnsureSkPictureIsValid(
-                ProtoPathToFilePath(proto->root_frame().file_path()), 1);
-            EnsureSkPictureIsValid(
-                ProtoPathToFilePath(proto->subframes(0).file_path()), 0,
-                gfx::Size(300, 300));
+            EXPECT_FALSE(result->proto.subframes(0).is_main_frame());
+            EnsureSkPictureIsValid(params, result.get(),
+                                   result->proto.root_frame(), 1);
+            EnsureSkPictureIsValid(params, result.get(),
+                                   result->proto.subframes(0), 0,
+                                   gfx::Size(300, 300));
             quit.Run();
           },
-          loop.QuitClosure(), params.document_guid));
+          loop.QuitClosure(), params));
   loop.Run();
 }
 
-IN_PROC_BROWSER_TEST_F(PaintPreviewBrowserTest,
+IN_PROC_BROWSER_TEST_P(PaintPreviewBrowserTest,
                        CaptureMainFrameWithNonScrollableSameProcessSubframe) {
   std::string html = R"(<html>
                           <iframe
@@ -283,22 +312,30 @@ IN_PROC_BROWSER_TEST_F(PaintPreviewBrowserTest,
   client->CapturePaintPreview(
       params, GetWebContents()->GetMainFrame(),
       base::BindOnce(
-          [](base::RepeatingClosure quit, base::UnguessableToken expected_guid,
+          [](base::RepeatingClosure quit,
+             const PaintPreviewClient::PaintPreviewParams& params,
              base::UnguessableToken guid, mojom::PaintPreviewStatus status,
-             std::unique_ptr<PaintPreviewProto> proto) {
-            EXPECT_EQ(guid, expected_guid);
+             std::unique_ptr<CaptureResult> result) {
+            EXPECT_EQ(guid, params.inner.document_guid);
             EXPECT_EQ(status, mojom::PaintPreviewStatus::kOk);
-            EXPECT_TRUE(proto->has_root_frame());
-            EXPECT_EQ(proto->subframes_size(), 0);
-            EXPECT_EQ(proto->root_frame().content_id_to_embedding_tokens_size(),
+            EXPECT_TRUE(result->proto.has_root_frame());
+            EXPECT_EQ(result->proto.subframes_size(), 0);
+            EXPECT_EQ(result->proto.root_frame()
+                          .content_id_to_embedding_tokens_size(),
                       0);
-            EXPECT_TRUE(proto->root_frame().is_main_frame());
-            EnsureSkPictureIsValid(
-                ProtoPathToFilePath(proto->root_frame().file_path()), 0);
+            EXPECT_TRUE(result->proto.root_frame().is_main_frame());
+            EnsureSkPictureIsValid(params, result.get(),
+                                   result->proto.root_frame(), 0);
             quit.Run();
           },
-          loop.QuitClosure(), params.document_guid));
+          loop.QuitClosure(), params));
   loop.Run();
 }
 
+INSTANTIATE_TEST_SUITE_P(All,
+                         PaintPreviewBrowserTest,
+                         testing::Values(mojom::Persistence::kFileSystem,
+                                         mojom::Persistence::kMemoryBuffer),
+                         PersistenceParamToString);
+
 }  // namespace paint_preview

chrome/browser/paint_preview/services/paint_preview_demo_service.cc

@@ -111,8 +111,9 @@ void PaintPreviewDemoService::OnCaptured(
     FinishedSuccessfullyCallback callback,
     const DirectoryKey& key,
     PaintPreviewBaseService::CaptureStatus status,
-    std::unique_ptr<PaintPreviewProto> proto) {
-  if (status != PaintPreviewBaseService::CaptureStatus::kOk || !proto) {
+    std::unique_ptr<CaptureResult> result) {
+  if (status != PaintPreviewBaseService::CaptureStatus::kOk ||
+      !result->capture_success) {
     std::move(callback).Run(false);
     return;
   }
@@ -120,7 +121,7 @@ void PaintPreviewDemoService::OnCaptured(
   GetTaskRunner()->PostTaskAndReplyWithResult(
       FROM_HERE,
       base::BindOnce(&FileManager::SerializePaintPreviewProto, GetFileManager(),
-                     key, *proto, true),
+                     key, result->proto, true),
       base::BindOnce(&PaintPreviewDemoService::OnSerializationFinished,
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }

chrome/browser/paint_preview/services/paint_preview_demo_service.h

@@ -60,7 +60,7 @@ class PaintPreviewDemoService : public PaintPreviewBaseService {
   void OnCaptured(FinishedSuccessfullyCallback callback,
                   const DirectoryKey& key,
                   PaintPreviewBaseService::CaptureStatus status,
-                  std::unique_ptr<PaintPreviewProto> proto);
+                  std::unique_ptr<CaptureResult> result);
 
   void OnSerializationFinished(FinishedSuccessfullyCallback callback,
                                bool success);

chrome/browser/paint_preview/services/paint_preview_tab_service.cc

@@ -239,14 +239,15 @@ void PaintPreviewTabService::OnCaptured(
     int frame_tree_node_id,
     FinishedCallback callback,
     PaintPreviewBaseService::CaptureStatus status,
-    std::unique_ptr<PaintPreviewProto> proto) {
+    std::unique_ptr<CaptureResult> result) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   auto* web_contents =
       content::WebContents::FromFrameTreeNodeId(frame_tree_node_id);
   if (web_contents)
     web_contents->DecrementCapturerCount(true);
 
-  if (status != PaintPreviewBaseService::CaptureStatus::kOk || !proto) {
+  if (status != PaintPreviewBaseService::CaptureStatus::kOk ||
+      !result->capture_success) {
     std::move(callback).Run(Status::kCaptureFailed);
     return;
   }
@@ -254,7 +255,7 @@ void PaintPreviewTabService::OnCaptured(
   GetTaskRunner()->PostTaskAndReplyWithResult(
       FROM_HERE,
       base::BindOnce(&FileManager::SerializePaintPreviewProto, GetFileManager(),
-                     key, *proto, true),
+                     key, result->proto, true),
       base::BindOnce(&PaintPreviewTabService::OnFinished,
                      weak_ptr_factory_.GetWeakPtr(), tab_id,
                      std::move(callback)));

chrome/browser/paint_preview/services/paint_preview_tab_service.h

@@ -113,7 +113,7 @@ class PaintPreviewTabService : public PaintPreviewBaseService {
                   int frame_tree_node_id,
                   FinishedCallback callback,
                   PaintPreviewBaseService::CaptureStatus status,
-                  std::unique_ptr<PaintPreviewProto>);
+                  std::unique_ptr<CaptureResult> result);
 
   void OnFinished(int tab_id, FinishedCallback callback, bool success);
 

chrome/test/BUILD.gn

@@ -2839,6 +2839,7 @@ if (!is_android) {
         "//components/paint_preview/browser",
         "//components/paint_preview/browser:test_support",
         "//components/paint_preview/common",
+        "//components/paint_preview/common:test_utils",
         "//components/services/paint_preview_compositor/public/mojom",
       ]
     }

components/paint_preview/browser/paint_preview_base_service.cc

@@ -78,23 +78,24 @@ void PaintPreviewBaseService::CapturePaintPreview(
     OnCapturedCallback callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   if (policy_ && !policy_->SupportedForContents(web_contents)) {
-    std::move(callback).Run(kContentUnsupported, nullptr);
+    std::move(callback).Run(kContentUnsupported, {});
     return;
   }
 
   PaintPreviewClient::CreateForWebContents(web_contents);  // Is a singleton.
   auto* client = PaintPreviewClient::FromWebContents(web_contents);
   if (!client) {
-    std::move(callback).Run(kClientCreationFailed, nullptr);
+    std::move(callback).Run(kClientCreationFailed, {});
     return;
   }
 
-  PaintPreviewClient::PaintPreviewParams params;
-  params.document_guid = base::UnguessableToken::Create();
-  params.clip_rect = clip_rect;
-  params.is_main_frame = (render_frame_host == web_contents->GetMainFrame());
+  PaintPreviewClient::PaintPreviewParams params(
+      mojom::Persistence::kFileSystem);
   params.root_dir = root_dir;
-  params.max_per_capture_size = max_per_capture_size;
+  params.inner.clip_rect = clip_rect;
+  params.inner.is_main_frame =
+      (render_frame_host == web_contents->GetMainFrame());
+  params.inner.max_per_capture_size = 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
@@ -142,7 +143,7 @@ void PaintPreviewBaseService::OnCaptured(
     OnCapturedCallback callback,
     base::UnguessableToken guid,
     mojom::PaintPreviewStatus status,
-    std::unique_ptr<PaintPreviewProto> proto) {
+    std::unique_ptr<CaptureResult> result) {
   auto* web_contents =
       content::WebContents::FromFrameTreeNodeId(frame_tree_node_id);
   if (web_contents)
@@ -150,15 +151,15 @@ void PaintPreviewBaseService::OnCaptured(
 
   if (!(status == mojom::PaintPreviewStatus::kOk ||
         status == mojom::PaintPreviewStatus::kPartialSuccess) ||
-      !proto) {
+      !result->capture_success) {
     DVLOG(1) << "ERROR: Paint Preview failed to capture for document "
              << guid.ToString() << " with error " << status;
-    std::move(callback).Run(kCaptureFailed, nullptr);
+    std::move(callback).Run(kCaptureFailed, {});
     return;
   }
   base::UmaHistogramTimes("Browser.PaintPreview.Capture.TotalCaptureDuration",
                           base::TimeTicks::Now() - start_time);
-  std::move(callback).Run(kOk, std::move(proto));
+  std::move(callback).Run(kOk, std::move(result));
 }
 
 }  // namespace paint_preview

components/paint_preview/browser/paint_preview_base_service.h

@@ -20,6 +20,7 @@
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/paint_preview/browser/file_manager.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"
@@ -49,8 +50,7 @@ class PaintPreviewBaseService : public KeyedService {
   };
 
   using OnCapturedCallback =
-      base::OnceCallback<void(CaptureStatus,
-                              std::unique_ptr<PaintPreviewProto>)>;
+      base::OnceCallback<void(CaptureStatus, std::unique_ptr<CaptureResult>)>;
 
   using OnReadProtoCallback =
       base::OnceCallback<void(std::unique_ptr<PaintPreviewProto>)>;
@@ -136,7 +136,7 @@ class PaintPreviewBaseService : public KeyedService {
                   OnCapturedCallback callback,
                   base::UnguessableToken guid,
                   mojom::PaintPreviewStatus status,
-                  std::unique_ptr<PaintPreviewProto> proto);
+                  std::unique_ptr<CaptureResult> result);
 
   std::unique_ptr<PaintPreviewPolicy> policy_ = nullptr;
   scoped_refptr<base::SequencedTaskRunner> task_runner_;

components/paint_preview/browser/paint_preview_base_service_unittest.cc

@@ -195,22 +195,22 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureMainFrame) {
              PaintPreviewBaseService::CaptureStatus expected_status,
              const base::FilePath& expected_path,
              PaintPreviewBaseService::CaptureStatus status,
-             std::unique_ptr<PaintPreviewProto> proto) {
+             std::unique_ptr<CaptureResult> result) {
             EXPECT_EQ(status, expected_status);
-            EXPECT_TRUE(proto->has_root_frame());
-            EXPECT_EQ(proto->subframes_size(), 0);
-            EXPECT_TRUE(proto->root_frame().is_main_frame());
+            EXPECT_TRUE(result->proto.has_root_frame());
+            EXPECT_EQ(result->proto.subframes_size(), 0);
+            EXPECT_TRUE(result->proto.root_frame().is_main_frame());
             auto token = base::UnguessableToken::Deserialize(
-                proto->root_frame().embedding_token_high(),
-                proto->root_frame().embedding_token_low());
+                result->proto.root_frame().embedding_token_high(),
+                result->proto.root_frame().embedding_token_low());
 #if defined(OS_WIN)
             base::FilePath path = base::FilePath(
-                base::UTF8ToUTF16(proto->root_frame().file_path()));
+                base::UTF8ToUTF16(result->proto.root_frame().file_path()));
             base::FilePath name(
                 base::UTF8ToUTF16(base::StrCat({token.ToString(), ".skp"})));
 #else
             base::FilePath path =
-                base::FilePath(proto->root_frame().file_path());
+                base::FilePath(result->proto.root_frame().file_path());
             base::FilePath name(base::StrCat({token.ToString(), ".skp"}));
 #endif
             EXPECT_EQ(path.DirName(), expected_path);
@@ -248,9 +248,9 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureFailed) {
           [](base::OnceClosure quit_closure,
              PaintPreviewBaseService::CaptureStatus expected_status,
              PaintPreviewBaseService::CaptureStatus status,
-             std::unique_ptr<PaintPreviewProto> proto) {
+             std::unique_ptr<CaptureResult> result) {
             EXPECT_EQ(status, expected_status);
-            EXPECT_EQ(proto, nullptr);
+            EXPECT_EQ(result, nullptr);
             std::move(quit_closure).Run();
           },
           loop.QuitClosure(),
@@ -283,9 +283,9 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureDisallowed) {
           [](base::OnceClosure quit_closure,
              PaintPreviewBaseService::CaptureStatus expected_status,
              PaintPreviewBaseService::CaptureStatus status,
-             std::unique_ptr<PaintPreviewProto> proto) {
+             std::unique_ptr<CaptureResult> result) {
             EXPECT_EQ(status, expected_status);
-            EXPECT_EQ(proto, nullptr);
+            EXPECT_EQ(result, nullptr);
             std::move(quit_closure).Run();
           },
           loop.QuitClosure(),

components/paint_preview/browser/paint_preview_client.cc

@@ -6,6 +6,8 @@
 
 #include <utility>
 
+#include "base/bind.h"
+#include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/strings/strcat.h"
@@ -13,6 +15,9 @@
 #include "base/task/post_task.h"
 #include "base/task/thread_pool.h"
 #include "base/threading/scoped_blocking_call.h"
+#include "base/unguessable_token.h"
+#include "components/paint_preview/common/capture_result.h"
+#include "components/paint_preview/common/mojom/paint_preview_recorder.mojom-forward.h"
 #include "components/ukm/content/source_url_recorder.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
@@ -95,34 +100,168 @@ base::flat_set<base::UnguessableToken> CreateAcceptedTokenList(
   return base::flat_set<base::UnguessableToken>(std::move(tokens));
 }
 
+mojom::PaintPreviewCaptureParamsPtr CreateRecordingRequestParams(
+    mojom::Persistence persistence,
+    const RecordingParams& capture_params,
+    base::File file) {
+  mojom::PaintPreviewCaptureParamsPtr mojo_params =
+      mojom::PaintPreviewCaptureParams::New();
+  mojo_params->persistence = persistence;
+  mojo_params->guid = capture_params.document_guid;
+  mojo_params->clip_rect = capture_params.clip_rect;
+  // For now treat all clip rects as hints only. This API should be exposed
+  // when clip_rects are used intentionally to limit capture time.
+  mojo_params->clip_rect_is_hint = true;
+  mojo_params->is_main_frame = capture_params.is_main_frame;
+  mojo_params->file = std::move(file);
+  mojo_params->max_capture_size = capture_params.max_per_capture_size;
+  return mojo_params;
+}
+
+// Unconditionally create or overwrite a file for writing.
+base::File CreateOrOverwriteFileForWriting(const base::FilePath& path) {
+  base::File file(path,
+                  base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
+  return file;
+}
+
+using RecordingRequestParamsReadyCallback =
+    base::OnceCallback<void(mojom::PaintPreviewStatus,
+                            mojom::PaintPreviewCaptureParamsPtr)>;
+
+void OnSerializedRecordingFileCreated(
+    const RecordingParams& capture_params,
+    const base::FilePath& filename,
+    RecordingRequestParamsReadyCallback callback,
+    base::File file) {
+  if (!file.IsValid()) {
+    DLOG(ERROR) << "File create failed: " << file.error_details();
+    std::move(callback).Run(mojom::PaintPreviewStatus::kFileCreationError, {});
+  } else {
+    std::move(callback).Run(
+        mojom::PaintPreviewStatus::kOk,
+        CreateRecordingRequestParams(mojom::Persistence::kFileSystem,
+                                     capture_params, std::move(file)));
+  }
+}
+
+// Prepare the PaintPreviewRecorder mojo params request object. If |persistence|
+// is |Persistence::kFileSystem|, this will create the file that will act as the
+// sink for the recording.
+void PrepareRecordingRequestParams(
+    mojom::Persistence persistence,
+    const base::FilePath& frame_filepath,
+    const RecordingParams& capture_params,
+    RecordingRequestParamsReadyCallback callback) {
+  if (persistence == mojom::Persistence::kFileSystem) {
+    base::ThreadPool::PostTaskAndReplyWithResult(
+        FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE},
+        base::BindOnce(&CreateOrOverwriteFileForWriting, frame_filepath),
+        base::BindOnce(&OnSerializedRecordingFileCreated, capture_params,
+                       frame_filepath, std::move(callback)));
+  } else {
+    std::move(callback).Run(
+        mojom::PaintPreviewStatus::kOk,
+        CreateRecordingRequestParams(persistence, capture_params, {}));
+  }
+}
+
 }  // namespace
 
-PaintPreviewClient::PaintPreviewParams::PaintPreviewParams()
-    : is_main_frame(false), max_per_capture_size(0) {}
+PaintPreviewClient::PaintPreviewParams::PaintPreviewParams(
+    mojom::Persistence persistence)
+    : persistence(persistence),
+      inner(RecordingParams(base::UnguessableToken::Create())) {}
 
 PaintPreviewClient::PaintPreviewParams::~PaintPreviewParams() = default;
 
-PaintPreviewClient::PaintPreviewData::PaintPreviewData() = default;
+PaintPreviewClient::InProgressDocumentCaptureState::
+    InProgressDocumentCaptureState() = default;
+
+PaintPreviewClient::InProgressDocumentCaptureState::
+    ~InProgressDocumentCaptureState() {
+  if (persistence == mojom::Persistence::kFileSystem && should_clean_up_files) {
+    for (const auto& subframe_guid : awaiting_subframes) {
+      base::ThreadPool::PostTask(
+          FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
+          base::BindOnce(base::GetDeleteFileCallback(),
+                         FilePathForFrame(subframe_guid)));
+    }
 
-PaintPreviewClient::PaintPreviewData::~PaintPreviewData() = default;
+    for (const auto& subframe_guid : finished_subframes) {
+      base::ThreadPool::PostTask(
+          FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
+          base::BindOnce(base::GetDeleteFileCallback(),
+                         FilePathForFrame(subframe_guid)));
+    }
+  }
+}
+
+PaintPreviewClient::InProgressDocumentCaptureState&
+PaintPreviewClient::InProgressDocumentCaptureState::operator=(
+    InProgressDocumentCaptureState&& rhs) noexcept = default;
+
+PaintPreviewClient::InProgressDocumentCaptureState::
+    InProgressDocumentCaptureState(
+        InProgressDocumentCaptureState&& other) noexcept = default;
+
+base::FilePath
+PaintPreviewClient::InProgressDocumentCaptureState::FilePathForFrame(
+    const base::UnguessableToken& frame_guid) {
+  return root_dir.AppendASCII(base::StrCat({frame_guid.ToString(), ".skp"}));
+}
 
-PaintPreviewClient::PaintPreviewData&
-PaintPreviewClient::PaintPreviewData::operator=(
-    PaintPreviewData&& rhs) noexcept = default;
+void PaintPreviewClient::InProgressDocumentCaptureState::RecordSuccessfulFrame(
+    const base::UnguessableToken& frame_guid,
+    bool is_main_frame,
+    mojom::PaintPreviewCaptureResponsePtr response) {
+  // Records the data from a processed frame if it was captured successfully.
+  had_success = true;
 
-PaintPreviewClient::PaintPreviewData::PaintPreviewData(
-    PaintPreviewData&& other) noexcept = default;
+  PaintPreviewFrameProto* frame_proto;
+  if (is_main_frame) {
+    main_frame_blink_recording_time = response->blink_recording_time;
+    frame_proto = proto.mutable_root_frame();
+    frame_proto->set_is_main_frame(true);
+  } else {
+    frame_proto = proto.add_subframes();
+    frame_proto->set_is_main_frame(false);
+  }
 
-PaintPreviewClient::CreateResult::CreateResult(base::File file,
-                                               base::File::Error error)
-    : file(std::move(file)), error(error) {}
+  if (persistence == mojom::Persistence::kFileSystem) {
+    // Safe since |filename| is always in the form: "{hexadecimal}.skp".
+    frame_proto->set_file_path(FilePathForFrame(frame_guid).AsUTF8Unsafe());
+  } else {
+    DCHECK(response->skp.has_value());
+    serialized_skps.insert({frame_guid, std::move(response->skp.value())});
+  }
 
-PaintPreviewClient::CreateResult::~CreateResult() = default;
+  std::vector<base::UnguessableToken> remote_frame_guids =
+      PaintPreviewCaptureResponseToPaintPreviewFrameProto(
+          std::move(response), frame_guid, frame_proto);
 
-PaintPreviewClient::CreateResult::CreateResult(CreateResult&& other) = default;
+  for (const auto& remote_frame_guid : remote_frame_guids) {
+    // Don't wait again for a frame that was already captured. Also don't wait
+    // on frames that navigated during capture and have new embedding tokens.
+    if (!base::Contains(finished_subframes, remote_frame_guid) &&
+        base::Contains(accepted_tokens, remote_frame_guid)) {
+      awaiting_subframes.insert(remote_frame_guid);
+    }
+  }
+}
 
-PaintPreviewClient::CreateResult& PaintPreviewClient::CreateResult::operator=(
-    CreateResult&& other) = default;
+std::unique_ptr<CaptureResult>
+PaintPreviewClient::InProgressDocumentCaptureState::IntoCaptureResult() && {
+  // Do not clean up files since we're about to return to the user.
+  should_clean_up_files = false;
+
+  std::unique_ptr<CaptureResult> result =
+      std::make_unique<CaptureResult>(persistence);
+  result->proto = std::move(proto);
+  result->serialized_skps = std::move(serialized_skps);
+  result->capture_success = had_success;
+  return result;
+}
 
 PaintPreviewClient::PaintPreviewClient(content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents) {}
@@ -133,31 +272,34 @@ void PaintPreviewClient::CapturePaintPreview(
     const PaintPreviewParams& params,
     content::RenderFrameHost* render_frame_host,
     PaintPreviewCallback callback) {
-  if (base::Contains(all_document_data_, params.document_guid)) {
-    std::move(callback).Run(params.document_guid,
-                            mojom::PaintPreviewStatus::kGuidCollision, nullptr);
+  if (base::Contains(all_document_data_, params.inner.document_guid)) {
+    std::move(callback).Run(params.inner.document_guid,
+                            mojom::PaintPreviewStatus::kGuidCollision, {});
     return;
   }
-  PaintPreviewData document_data;
+  InProgressDocumentCaptureState document_data;
+  document_data.should_clean_up_files = true;
+  document_data.persistence = params.persistence;
   document_data.root_dir = params.root_dir;
+  document_data.proto.mutable_metadata()->set_url(
+      render_frame_host->GetLastCommittedURL().spec());
   document_data.callback = std::move(callback);
-  document_data.root_url = render_frame_host->GetLastCommittedURL();
   document_data.source_id =
       ukm::GetSourceIdForWebContentsDocument(web_contents());
   document_data.accepted_tokens = CreateAcceptedTokenList(render_frame_host);
-  all_document_data_.insert({params.document_guid, std::move(document_data)});
+  all_document_data_.insert(
+      {params.inner.document_guid, std::move(document_data)});
   TRACE_EVENT_NESTABLE_ASYNC_BEGIN0(
       "paint_preview", "PaintPreviewClient::CapturePaintPreview",
-      TRACE_ID_LOCAL(&all_document_data_[params.document_guid]));
-  CapturePaintPreviewInternal(params, render_frame_host);
+      TRACE_ID_LOCAL(&all_document_data_[params.inner.document_guid]));
+  CapturePaintPreviewInternal(params.inner, render_frame_host);
 }
 
 void PaintPreviewClient::CaptureSubframePaintPreview(
     const base::UnguessableToken& guid,
     const gfx::Rect& rect,
     content::RenderFrameHost* render_subframe_host) {
-  PaintPreviewParams params;
-  params.document_guid = guid;
+  RecordingParams params(guid);
   params.clip_rect = rect;
   params.is_main_frame = false;
   CapturePaintPreviewInternal(params, render_subframe_host);
@@ -200,31 +342,8 @@ void PaintPreviewClient::RenderFrameDeleted(
   pending_previews_on_subframe_.erase(frame_guid);
 }
 
-PaintPreviewClient::CreateResult PaintPreviewClient::CreateFileHandle(
-    const base::FilePath& path) {
-  base::File file(path,
-                  base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
-  return CreateResult(std::move(file), file.error_details());
-}
-
-mojom::PaintPreviewCaptureParamsPtr PaintPreviewClient::CreateMojoParams(
-    const PaintPreviewParams& params,
-    base::File file) {
-  mojom::PaintPreviewCaptureParamsPtr mojo_params =
-      mojom::PaintPreviewCaptureParams::New();
-  mojo_params->guid = params.document_guid;
-  mojo_params->clip_rect = params.clip_rect;
-  // For now treat all clip rects as hints only. This API should be exposed
-  // when clip_rects are used intentionally to limit capture time.
-  mojo_params->clip_rect_is_hint = true;
-  mojo_params->is_main_frame = params.is_main_frame;
-  mojo_params->file = std::move(file);
-  mojo_params->max_capture_size = params.max_per_capture_size;
-  return mojo_params;
-}
-
 void PaintPreviewClient::CapturePaintPreviewInternal(
-    const PaintPreviewParams& params,
+    const RecordingParams& params,
     content::RenderFrameHost* render_frame_host) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   // Use a frame's embedding token as its GUID.
@@ -256,25 +375,23 @@ void PaintPreviewClient::CapturePaintPreviewInternal(
   if (base::Contains(document_data->awaiting_subframes, frame_guid) ||
       base::Contains(document_data->finished_subframes, frame_guid))
     return;
-  base::FilePath file_path = document_data->root_dir.AppendASCII(
-      base::StrCat({frame_guid.ToString(), ".skp"}));
-  base::ThreadPool::PostTaskAndReplyWithResult(
-      FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE},
-      base::BindOnce(&CreateFileHandle, file_path),
+
+  PrepareRecordingRequestParams(
+      document_data->persistence, document_data->FilePathForFrame(frame_guid),
+      params,
       base::BindOnce(&PaintPreviewClient::RequestCaptureOnUIThread,
-                     weak_ptr_factory_.GetWeakPtr(), params, frame_guid,
+                     weak_ptr_factory_.GetWeakPtr(), frame_guid, params,
                      content::GlobalFrameRoutingId(
                          render_frame_host->GetProcess()->GetID(),
-                         render_frame_host->GetRoutingID()),
-                     file_path));
+                         render_frame_host->GetRoutingID())));
 }
 
 void PaintPreviewClient::RequestCaptureOnUIThread(
-    const PaintPreviewParams& params,
     const base::UnguessableToken& frame_guid,
+    const RecordingParams& params,
     const content::GlobalFrameRoutingId& render_frame_id,
-    const base::FilePath& file_path,
-    CreateResult result) {
+    mojom::PaintPreviewStatus status,
+    mojom::PaintPreviewCaptureParamsPtr capture_params) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   auto it = all_document_data_.find(params.document_guid);
   if (it == all_document_data_.end())
@@ -283,10 +400,8 @@ void PaintPreviewClient::RequestCaptureOnUIThread(
   if (!document_data->callback)
     return;
 
-  if (result.error != base::File::FILE_OK) {
-    std::move(document_data->callback)
-        .Run(params.document_guid,
-             mojom::PaintPreviewStatus::kFileCreationError, nullptr);
+  if (status != mojom::PaintPreviewStatus::kOk) {
+    std::move(document_data->callback).Run(params.document_guid, status, {});
     return;
   }
 
@@ -297,7 +412,7 @@ void PaintPreviewClient::RequestCaptureOnUIThread(
                                 base::UnguessableToken::Null()) != frame_guid) {
     std::move(document_data->callback)
         .Run(params.document_guid, mojom::PaintPreviewStatus::kCaptureFailed,
-             nullptr);
+             {});
     return;
   }
 
@@ -318,52 +433,50 @@ void PaintPreviewClient::RequestCaptureOnUIThread(
         &interface_ptrs_[frame_guid]);
   }
   interface_ptrs_[frame_guid]->CapturePaintPreview(
-      CreateMojoParams(params, std::move(result.file)),
+      std::move(capture_params),
       base::BindOnce(&PaintPreviewClient::OnPaintPreviewCapturedCallback,
-                     weak_ptr_factory_.GetWeakPtr(), params.document_guid,
-                     frame_guid, params.is_main_frame, file_path,
+                     weak_ptr_factory_.GetWeakPtr(), frame_guid, params,
                      render_frame_id));
 }
 
 void PaintPreviewClient::OnPaintPreviewCapturedCallback(
-    const base::UnguessableToken& guid,
     const base::UnguessableToken& frame_guid,
-    bool is_main_frame,
-    const base::FilePath& filename,
+    const RecordingParams& params,
     const content::GlobalFrameRoutingId& render_frame_id,
     mojom::PaintPreviewStatus status,
     mojom::PaintPreviewCaptureResponsePtr response) {
   // There is no retry logic so always treat a frame as processed regardless of
   // |status|
-  MarkFrameAsProcessed(guid, frame_guid);
+  MarkFrameAsProcessed(params.document_guid, frame_guid);
 
-  if (status == mojom::PaintPreviewStatus::kOk) {
-    status = RecordFrame(guid, frame_guid, is_main_frame, filename,
-                         render_frame_id, std::move(response));
-  }
-  if (status != mojom::PaintPreviewStatus::kOk) {
-    // If the capture failed then cleanup the file.
-    base::ThreadPool::PostTask(
-        FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
-        base::BindOnce(base::GetDeleteFileCallback(), filename));
-
-    // If this is the main frame we should just abort the capture.
-    if (is_main_frame) {
-      auto it = all_document_data_.find(guid);
-      if (it != all_document_data_.end())
-        OnFinished(guid, &it->second);
-    }
+  // If the render frame host navigated or is no longer around treat this as a
+  // failure as a navigation occurring during capture is bad.
+  auto* render_frame_host = content::RenderFrameHost::FromID(render_frame_id);
+  if (!render_frame_host || render_frame_host->GetEmbeddingToken().value_or(
+                                base::UnguessableToken::Null()) != frame_guid) {
+    status = mojom::PaintPreviewStatus::kCaptureFailed;
   }
 
-  auto it = all_document_data_.find(guid);
+  auto it = all_document_data_.find(params.document_guid);
   if (it == all_document_data_.end())
     return;
   auto* document_data = &it->second;
-  if (status != mojom::PaintPreviewStatus::kOk)
+
+  if (status == mojom::PaintPreviewStatus::kOk) {
+    document_data->RecordSuccessfulFrame(frame_guid, params.is_main_frame,
+                                         std::move(response));
+  } else {
     document_data->had_error = true;
 
+    // If this is the main frame we should just abort the capture on failure.
+    if (params.is_main_frame) {
+      OnFinished(params.document_guid, document_data);
+      return;
+    }
+  }
+
   if (document_data->awaiting_subframes.empty())
-    OnFinished(guid, document_data);
+    OnFinished(params.document_guid, document_data);
 }
 
 void PaintPreviewClient::MarkFrameAsProcessed(
@@ -380,75 +493,20 @@ void PaintPreviewClient::MarkFrameAsProcessed(
   document_data->awaiting_subframes.erase(frame_guid);
 }
 
-mojom::PaintPreviewStatus PaintPreviewClient::RecordFrame(
-    const base::UnguessableToken& guid,
-    const base::UnguessableToken& frame_guid,
-    bool is_main_frame,
-    const base::FilePath& filename,
-    const content::GlobalFrameRoutingId& render_frame_id,
-    mojom::PaintPreviewCaptureResponsePtr response) {
-  // If the render frame host navigated or is no longer around treat this as a
-  // failure as a navigation occurring during capture is bad.
-  auto* render_frame_host = content::RenderFrameHost::FromID(render_frame_id);
-  if (!render_frame_host || render_frame_host->GetEmbeddingToken().value_or(
-                                base::UnguessableToken::Null()) != frame_guid) {
-    return mojom::PaintPreviewStatus::kCaptureFailed;
-  }
-
-  auto it = all_document_data_.find(guid);
-  if (it == all_document_data_.end())
-    return mojom::PaintPreviewStatus::kCaptureFailed;
-
-  auto* document_data = &it->second;
-  if (!document_data->proto) {
-    document_data->proto = std::make_unique<PaintPreviewProto>();
-    document_data->proto->mutable_metadata()->set_url(
-        document_data->root_url.spec());
-  }
-
-  PaintPreviewProto* proto_ptr = document_data->proto.get();
-
-  PaintPreviewFrameProto* frame_proto;
-  if (is_main_frame) {
-    document_data->main_frame_blink_recording_time =
-        response->blink_recording_time;
-    frame_proto = proto_ptr->mutable_root_frame();
-    frame_proto->set_is_main_frame(true);
-  } else {
-    frame_proto = proto_ptr->add_subframes();
-    frame_proto->set_is_main_frame(false);
-  }
-  // Safe since always HEX.skp.
-  frame_proto->set_file_path(filename.AsUTF8Unsafe());
-
-  std::vector<base::UnguessableToken> remote_frame_guids =
-      PaintPreviewCaptureResponseToPaintPreviewFrameProto(
-          std::move(response), frame_guid, frame_proto);
-
-  for (const auto& remote_frame_guid : remote_frame_guids) {
-    // Don't wait again for a frame that was already captured. Also don't wait
-    // on frames that navigated during capture and have new embedding tokens.
-    if (!base::Contains(document_data->finished_subframes, remote_frame_guid) &&
-        base::Contains(document_data->accepted_tokens, remote_frame_guid)) {
-      document_data->awaiting_subframes.insert(remote_frame_guid);
-    }
-  }
-  return mojom::PaintPreviewStatus::kOk;
-}
-
-void PaintPreviewClient::OnFinished(base::UnguessableToken guid,
-                                    PaintPreviewData* document_data) {
+void PaintPreviewClient::OnFinished(
+    base::UnguessableToken guid,
+    InProgressDocumentCaptureState* document_data) {
   if (!document_data || !document_data->callback)
     return;
 
   TRACE_EVENT_NESTABLE_ASYNC_END2(
       "paint_preview", "PaintPreviewClient::CapturePaintPreview",
-      TRACE_ID_LOCAL(document_data), "success", document_data->proto != nullptr,
+      TRACE_ID_LOCAL(document_data), "success", document_data->had_success,
       "subframes", document_data->finished_subframes.size());
 
   base::UmaHistogramBoolean("Browser.PaintPreview.Capture.Success",
-                            document_data->proto != nullptr);
-  if (document_data->proto) {
+                            document_data->had_success);
+  if (document_data->had_success) {
     base::UmaHistogramCounts100(
         "Browser.PaintPreview.Capture.NumberOfFramesCaptured",
         document_data->finished_subframes.size());
@@ -464,11 +522,11 @@ void PaintPreviewClient::OnFinished(base::UnguessableToken guid,
              document_data->had_error
                  ? mojom::PaintPreviewStatus::kPartialSuccess
                  : mojom::PaintPreviewStatus::kOk,
-             std::move(document_data->proto));
+             std::move(*document_data).IntoCaptureResult());
   } else {
     // A proto could not be created indicating all frames failed to capture.
     std::move(document_data->callback)
-        .Run(guid, mojom::PaintPreviewStatus::kFailed, nullptr);
+        .Run(guid, mojom::PaintPreviewStatus::kFailed, {});
   }
   all_document_data_.erase(guid);
 }

components/paint_preview/browser/paint_preview_client.h

@@ -14,10 +14,13 @@
 #include "base/containers/flat_set.h"
 #include "base/files/file_path.h"
 #include "base/unguessable_token.h"
+#include "components/paint_preview/common/capture_result.h"
+#include "components/paint_preview/common/mojom/paint_preview_recorder.mojom-shared.h"
 #include "components/paint_preview/common/mojom/paint_preview_recorder.mojom.h"
 #include "components/paint_preview/common/proto/paint_preview.pb.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
+#include "mojo/public/cpp/base/big_buffer.h"
 #include "mojo/public/cpp/bindings/associated_remote.h"
 #include "ui/gfx/geometry/rect.h"
 #include "url/gurl.h"
@@ -37,33 +40,22 @@ class PaintPreviewClient
   using PaintPreviewCallback =
       base::OnceCallback<void(base::UnguessableToken,
                               mojom::PaintPreviewStatus,
-                              std::unique_ptr<PaintPreviewProto>)>;
+                              std::unique_ptr<CaptureResult>)>;
 
   // Augmented version of mojom::PaintPreviewServiceParams.
   struct PaintPreviewParams {
-    PaintPreviewParams();
+    explicit PaintPreviewParams(mojom::Persistence persistence);
     ~PaintPreviewParams();
 
-    // The document GUID for this capture.
-    base::UnguessableToken document_guid;
+    // Indicates where the PaintPreviewRecorder should store its intermediate
+    // artifacts.
+    mojom::Persistence persistence;
 
     // The root directory in which to store paint_previews. This should be
     // a subdirectory inside the active user profile's directory.
     base::FilePath root_dir;
 
-    // The rect to which to clip the capture to.
-    gfx::Rect clip_rect;
-
-    // Whether the capture is for the main frame or an OOP subframe.
-    bool is_main_frame;
-
-    // The maximum capture size allowed per SkPicture captured. A size of 0 is
-    // unlimited.
-    // TODO(crbug/1071446): Ideally, this would cap the total size rather than
-    // being a per SkPicture limit. However, that is non-trivial due to the
-    // async ordering of captures from different frames making it hard to keep
-    // track of available headroom at the time of each capture triggering.
-    size_t max_per_capture_size;
+    RecordingParams inner;
   };
 
   ~PaintPreviewClient() override;
@@ -95,20 +87,21 @@ class PaintPreviewClient
 
   // Internal Storage Classes -------------------------------------------------
 
-  // Representation of data for capturing a paint preview.
-  struct PaintPreviewData {
+  // Ephemeral state for a document being captured. This will be accumulated to
+  // as the capture progresses and results in a |CaptureResult|.
+  struct InProgressDocumentCaptureState {
    public:
-    PaintPreviewData();
-    ~PaintPreviewData();
+    InProgressDocumentCaptureState();
+    ~InProgressDocumentCaptureState();
+
+    mojom::Persistence persistence;
 
-    // Root directory to store artifacts to.
+    // If |Persistence::kFileSystem|, the root directory to store artifacts to.
+    // Ignored if |Persistence::kMemoryBuffer|.
     base::FilePath root_dir;
 
     base::UnguessableToken root_frame_token;
 
-    // URL of the root frame.
-    GURL root_url;
-
     // UKM Source ID of the WebContent.
     ukm::SourceId source_id;
 
@@ -127,45 +120,51 @@ class PaintPreviewClient
     // All the render frames that are allowed to be captured.
     base::flat_set<base::UnguessableToken> accepted_tokens;
 
-    // Data proto that is returned via callback.
-    std::unique_ptr<PaintPreviewProto> proto;
+    // If |Persistence::kMemoryBuffer|, this will contain the successful
+    // recordings. Empty if |Persistence::FileSystem|
+    base::flat_map<base::UnguessableToken, mojo_base::BigBuffer>
+        serialized_skps;
 
-    bool had_error = false;
+    PaintPreviewProto proto;
 
-    PaintPreviewData& operator=(PaintPreviewData&& other) noexcept;
-    PaintPreviewData(PaintPreviewData&& other) noexcept;
+    // Indicates that at least one subframe finished unsuccessfully.
+    bool had_error = false;
 
-   private:
-    PaintPreviewData(const PaintPreviewData&) = delete;
-    PaintPreviewData& operator=(const PaintPreviewData&) = delete;
-  };
+    // Indicates that at least one subframe finished successfully.
+    bool had_success = false;
 
-  struct CreateResult {
-   public:
-    CreateResult(base::File file, base::File::Error error);
-    ~CreateResult();
-    CreateResult(CreateResult&& other);
-    CreateResult& operator=(CreateResult&& other);
+    // Indicates if we should clean up files associated with awaiting frames on
+    // destruction
+    bool should_clean_up_files = false;
 
-    base::File file;
-    base::File::Error error;
+    // Generates a file path based off |root_dir| and |frame_guid|. Will be in
+    // the form "{hexadecimal}.skp".
+    base::FilePath FilePathForFrame(const base::UnguessableToken& frame_guid);
 
-   private:
-    CreateResult(const CreateResult&) = delete;
-    CreateResult& operator=(const CreateResult&) = delete;
-  };
+    // Record a successful recording into this capture state.
+    void RecordSuccessfulFrame(const base::UnguessableToken& frame_guid,
+                               bool is_main_frame,
+                               mojom::PaintPreviewCaptureResponsePtr response);
 
-  // Helpers -------------------------------------------------------------------
+    // Convert this capture state into a form that can be returned to the
+    // original paint preview capture request.
+    std::unique_ptr<CaptureResult> IntoCaptureResult() &&;
 
-  static CreateResult CreateFileHandle(const base::FilePath& path);
+    InProgressDocumentCaptureState& operator=(
+        InProgressDocumentCaptureState&& other) noexcept;
+    InProgressDocumentCaptureState(
+        InProgressDocumentCaptureState&& other) noexcept;
 
-  mojom::PaintPreviewCaptureParamsPtr CreateMojoParams(
-      const PaintPreviewParams& params,
-      base::File file);
+   private:
+    InProgressDocumentCaptureState(const InProgressDocumentCaptureState&) =
+        delete;
+    InProgressDocumentCaptureState& operator=(
+        const InProgressDocumentCaptureState&) = delete;
+  };
 
   // Sets up for a capture of a frame on |render_frame_host| according to
   // |params|.
-  void CapturePaintPreviewInternal(const PaintPreviewParams& params,
+  void CapturePaintPreviewInternal(const RecordingParams& params,
                                    content::RenderFrameHost* render_frame_host);
 
   // Initiates capture via the PaintPreviewRecorder associated with
@@ -173,19 +172,17 @@ class PaintPreviewClient
   // is the GUID associated with the frame. |path| is file path associated with
   // the File stored in |result| (base::File isn't aware of its file path).
   void RequestCaptureOnUIThread(
-      const PaintPreviewParams& params,
       const base::UnguessableToken& frame_guid,
+      const RecordingParams& params,
       const content::GlobalFrameRoutingId& render_frame_id,
-      const base::FilePath& path,
-      CreateResult result);
+      mojom::PaintPreviewStatus status,
+      mojom::PaintPreviewCaptureParamsPtr capture_params);
 
   // Handles recording the frame and updating client state when capture is
   // complete.
   void OnPaintPreviewCapturedCallback(
-      const base::UnguessableToken& guid,
       const base::UnguessableToken& frame_guid,
-      bool is_main_frame,
-      const base::FilePath& filename,
+      const RecordingParams& params,
       const content::GlobalFrameRoutingId& render_frame_id,
       mojom::PaintPreviewStatus status,
       mojom::PaintPreviewCaptureResponsePtr response);
@@ -195,17 +192,9 @@ class PaintPreviewClient
   void MarkFrameAsProcessed(base::UnguessableToken guid,
                             const base::UnguessableToken& frame_guid);
 
-  // Records the data from a processed frame if it was captured successfully.
-  mojom::PaintPreviewStatus RecordFrame(
-      const base::UnguessableToken& guid,
-      const base::UnguessableToken& frame_guid,
-      bool is_main_frame,
-      const base::FilePath& filename,
-      const content::GlobalFrameRoutingId& render_frame_id,
-      mojom::PaintPreviewCaptureResponsePtr response);
-
   // Handles finishing the capture once all frames are received.
-  void OnFinished(base::UnguessableToken guid, PaintPreviewData* document_data);
+  void OnFinished(base::UnguessableToken guid,
+                  InProgressDocumentCaptureState* document_data);
 
   // Storage ------------------------------------------------------------------
 
@@ -218,8 +207,11 @@ class PaintPreviewClient
   base::flat_map<base::UnguessableToken, base::flat_set<base::UnguessableToken>>
       pending_previews_on_subframe_;
 
-  // Maps a document GUID to its data.
-  base::flat_map<base::UnguessableToken, PaintPreviewData> all_document_data_;
+  // Maps a document GUID to its capture state while it is in-progress. Entries
+  // in this map should be cleaned up when a capture completes (either
+  // successfully or not).
+  base::flat_map<base::UnguessableToken, InProgressDocumentCaptureState>
+      all_document_data_;
 
   base::WeakPtrFactory<PaintPreviewClient> weak_ptr_factory_{this};
 

components/paint_preview/browser/paint_preview_client_unittest.cc

@@ -7,10 +7,14 @@
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
+#include "base/notreached.h"
 #include "base/strings/strcat.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/unguessable_token.h"
 #include "build/build_config.h"
+#include "components/paint_preview/common/capture_result.h"
+#include "components/paint_preview/common/mojom/paint_preview_recorder.mojom-forward.h"
+#include "components/paint_preview/common/mojom/paint_preview_recorder.mojom-shared.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/test_utils.h"
@@ -20,7 +24,9 @@
 #include "content/public/test/navigation_simulator.h"
 #include "content/public/test/test_renderer_host.h"
 #include "content/public/test/test_utils.h"
+#include "mojo/public/cpp/base/big_buffer.h"
 #include "mojo/public/cpp/bindings/associated_receiver.h"
+#include "mojo/public/cpp/bindings/struct_ptr.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
 
@@ -90,16 +96,18 @@ mojom::PaintPreviewCaptureParamsPtr ToMojoParams(
     PaintPreviewClient::PaintPreviewParams params) {
   mojom::PaintPreviewCaptureParamsPtr params_ptr =
       mojom::PaintPreviewCaptureParams::New();
-  params_ptr->guid = params.document_guid;
-  params_ptr->is_main_frame = params.is_main_frame;
-  params_ptr->clip_rect = params.clip_rect;
+  params_ptr->persistence = params.persistence;
+  params_ptr->guid = params.inner.document_guid;
+  params_ptr->is_main_frame = params.inner.is_main_frame;
+  params_ptr->clip_rect = params.inner.clip_rect;
   return params_ptr;
 }
 
 }  // namespace
 
 class PaintPreviewClientRenderViewHostTest
-    : public content::RenderViewHostTestHarness {
+    : public content::RenderViewHostTestHarness,
+      public testing::WithParamInterface<mojom::Persistence> {
  public:
   PaintPreviewClientRenderViewHostTest() {}
 
@@ -113,6 +121,15 @@ class PaintPreviewClientRenderViewHostTest
         web_contents(), GURL("https://www.chromium.org"));
   }
 
+  mojo::StructPtr<mojom::PaintPreviewCaptureResponse>
+  NewMockPaintPreviewCaptureResponse() {
+    auto response = mojom::PaintPreviewCaptureResponse::New();
+    if (GetParam() == mojom::Persistence::kMemoryBuffer) {
+      response->skp = {mojo_base::BigBuffer()};
+    }
+    return response;
+  }
+
   void OverrideInterface(MockPaintPreviewRecorder* service) {
     blink::AssociatedInterfaceProvider* remote_interfaces =
         web_contents()->GetMainFrame()->GetRemoteAssociatedInterfaces();
@@ -131,16 +148,15 @@ class PaintPreviewClientRenderViewHostTest
       const PaintPreviewClientRenderViewHostTest&) = delete;
 };
 
-TEST_F(PaintPreviewClientRenderViewHostTest, CaptureMainFrameMock) {
-  PaintPreviewClient::PaintPreviewParams params;
-  params.document_guid = base::UnguessableToken::Create();
+TEST_P(PaintPreviewClientRenderViewHostTest, CaptureMainFrameMock) {
+  PaintPreviewClient::PaintPreviewParams params(GetParam());
   params.root_dir = temp_dir_.GetPath();
-  params.is_main_frame = true;
+  params.inner.is_main_frame = true;
 
   content::RenderFrameHost* rfh = main_rfh();
   GURL expected_url = rfh->GetLastCommittedURL();
 
-  auto response = mojom::PaintPreviewCaptureResponse::New();
+  auto response = NewMockPaintPreviewCaptureResponse();
   response->embedding_token = base::nullopt;
   response->scroll_offsets = gfx::Size(5, 10);
 
@@ -156,13 +172,13 @@ TEST_F(PaintPreviewClientRenderViewHostTest, CaptureMainFrameMock) {
       [](base::RepeatingClosure quit, base::UnguessableToken expected_guid,
          base::FilePath temp_dir, PaintPreviewProto expected_proto,
          base::UnguessableToken returned_guid, mojom::PaintPreviewStatus status,
-         std::unique_ptr<PaintPreviewProto> proto) {
+         std::unique_ptr<CaptureResult> result) {
         EXPECT_EQ(returned_guid, expected_guid);
         EXPECT_EQ(status, mojom::PaintPreviewStatus::kOk);
 
         auto token = base::UnguessableToken::Deserialize(
-            proto->root_frame().embedding_token_high(),
-            proto->root_frame().embedding_token_low());
+            result->proto.root_frame().embedding_token_high(),
+            result->proto.root_frame().embedding_token_low());
         EXPECT_NE(token, base::UnguessableToken::Null());
 
         // The token for the main frame is set internally since the render frame
@@ -172,24 +188,40 @@ TEST_F(PaintPreviewClientRenderViewHostTest, CaptureMainFrameMock) {
             expected_proto.mutable_root_frame();
         main_frame->set_embedding_token_low(token.GetLowForSerialization());
         main_frame->set_embedding_token_high(token.GetHighForSerialization());
-        main_frame->set_file_path(
-            temp_dir.AppendASCII(base::StrCat({token.ToString(), ".skp"}))
-                .AsUTF8Unsafe());
+        if (GetParam() == mojom::Persistence::kFileSystem) {
+          main_frame->set_file_path(
+              temp_dir.AppendASCII(base::StrCat({token.ToString(), ".skp"}))
+                  .AsUTF8Unsafe());
+        }
+
+        EXPECT_THAT(result->proto, EqualsProto(expected_proto));
 
-        EXPECT_THAT(*proto, EqualsProto(expected_proto));
-        {
-          base::ScopedAllowBlockingForTesting scope;
+        switch (GetParam()) {
+          case mojom::Persistence::kFileSystem: {
+            base::ScopedAllowBlockingForTesting scope;
 #if defined(OS_WIN)
-          base::FilePath path = base::FilePath(
-              base::UTF8ToUTF16(proto->root_frame().file_path()));
+            base::FilePath path = base::FilePath(
+                base::UTF8ToUTF16(result->proto.root_frame().file_path()));
 #else
-          base::FilePath path = base::FilePath(proto->root_frame().file_path());
+            base::FilePath path =
+                base::FilePath(result->proto.root_frame().file_path());
 #endif
-          EXPECT_TRUE(base::PathExists(path));
+            EXPECT_TRUE(base::PathExists(path));
+          } break;
+
+          case mojom::Persistence::kMemoryBuffer: {
+            EXPECT_EQ(result->serialized_skps.size(), 1u);
+            EXPECT_TRUE(result->serialized_skps.contains(token));
+          } break;
+
+          default:
+            NOTREACHED();
+            break;
         }
+
         quit.Run();
       },
-      loop.QuitClosure(), params.document_guid, temp_dir_.GetPath(),
+      loop.QuitClosure(), params.inner.document_guid, temp_dir_.GetPath(),
       expected_proto);
   MockPaintPreviewRecorder service;
   service.SetExpectedParams(ToMojoParams(params));
@@ -202,24 +234,24 @@ TEST_F(PaintPreviewClientRenderViewHostTest, CaptureMainFrameMock) {
   loop.Run();
 }
 
-TEST_F(PaintPreviewClientRenderViewHostTest, CaptureFailureMock) {
-  PaintPreviewClient::PaintPreviewParams params;
-  params.document_guid = base::UnguessableToken::Create();
+TEST_P(PaintPreviewClientRenderViewHostTest, CaptureFailureMock) {
+  PaintPreviewClient::PaintPreviewParams params(GetParam());
   params.root_dir = temp_dir_.GetPath();
-  params.is_main_frame = true;
+  params.inner.is_main_frame = true;
 
-  auto response = mojom::PaintPreviewCaptureResponse::New();
+  auto response = NewMockPaintPreviewCaptureResponse();
+  response->skp = {mojo_base::BigBuffer()};
 
   base::RunLoop loop;
   auto callback = base::BindOnce(
       [](base::RepeatingClosure quit, base::UnguessableToken expected_guid,
          base::UnguessableToken returned_guid, mojom::PaintPreviewStatus status,
-         std::unique_ptr<PaintPreviewProto> proto) {
+         std::unique_ptr<CaptureResult> result) {
         EXPECT_EQ(returned_guid, expected_guid);
         EXPECT_EQ(status, mojom::PaintPreviewStatus::kFailed);
         quit.Run();
       },
-      loop.QuitClosure(), params.document_guid);
+      loop.QuitClosure(), params.inner.document_guid);
   MockPaintPreviewRecorder recorder;
   recorder.SetExpectedParams(ToMojoParams(params));
   recorder.SetResponse(mojom::PaintPreviewStatus::kCaptureFailed,
@@ -234,7 +266,7 @@ TEST_F(PaintPreviewClientRenderViewHostTest, CaptureFailureMock) {
   EXPECT_TRUE(base::IsDirectoryEmpty(temp_dir_.GetPath()));
 }
 
-TEST_F(PaintPreviewClientRenderViewHostTest, RenderFrameDeletedNotCapturing) {
+TEST_P(PaintPreviewClientRenderViewHostTest, RenderFrameDeletedNotCapturing) {
   // Test that a deleting a render frame doesn't cause any problems if not
   // capturing.
   PaintPreviewClient::CreateForWebContents(web_contents());
@@ -243,24 +275,23 @@ TEST_F(PaintPreviewClientRenderViewHostTest, RenderFrameDeletedNotCapturing) {
   client->RenderFrameDeleted(main_rfh());
 }
 
-TEST_F(PaintPreviewClientRenderViewHostTest, RenderFrameDeletedDuringCapture) {
-  PaintPreviewClient::PaintPreviewParams params;
-  params.document_guid = base::UnguessableToken::Create();
+TEST_P(PaintPreviewClientRenderViewHostTest, RenderFrameDeletedDuringCapture) {
+  PaintPreviewClient::PaintPreviewParams params(GetParam());
   params.root_dir = temp_dir_.GetPath();
-  params.is_main_frame = true;
+  params.inner.is_main_frame = true;
 
   content::RenderFrameHost* rfh = main_rfh();
 
-  auto response = mojom::PaintPreviewCaptureResponse::New();
+  auto response = NewMockPaintPreviewCaptureResponse();
   response->embedding_token = base::nullopt;
 
   base::RunLoop loop;
   auto callback = base::BindOnce(
       [](base::RepeatingClosure quit, base::UnguessableToken returned_guid,
          mojom::PaintPreviewStatus status,
-         std::unique_ptr<PaintPreviewProto> proto) {
+         std::unique_ptr<CaptureResult> result) {
         EXPECT_EQ(status, mojom::PaintPreviewStatus::kFailed);
-        EXPECT_EQ(proto, nullptr);
+        EXPECT_EQ(result, nullptr);
         quit.Run();
       },
       loop.QuitClosure());
@@ -278,4 +309,10 @@ TEST_F(PaintPreviewClientRenderViewHostTest, RenderFrameDeletedDuringCapture) {
   loop.Run();
 }
 
+INSTANTIATE_TEST_SUITE_P(All,
+                         PaintPreviewClientRenderViewHostTest,
+                         testing::Values(mojom::Persistence::kFileSystem,
+                                         mojom::Persistence::kMemoryBuffer),
+                         PersistenceParamToString);
+
 }  // namespace paint_preview

components/paint_preview/common/BUILD.gn

@@ -7,6 +7,8 @@ import("//testing/test.gni")
 if (!is_ios) {
   static_library("common") {
     sources = [
+      "capture_result.cc",
+      "capture_result.h",
       "file_stream.cc",
       "file_stream.h",
       "file_utils.cc",
@@ -37,9 +39,13 @@ if (!is_ios) {
   source_set("test_utils") {
     testonly = true
 
-    sources = [ "test_utils.h" ]
+    sources = [
+      "test_utils.cc",
+      "test_utils.h",
+    ]
 
     deps = [
+      "//components/paint_preview/common/mojom",
       "//testing/gmock",
       "//testing/gtest",
     ]
@@ -58,6 +64,7 @@ if (!is_ios) {
 
     deps = [
       ":common",
+      ":test_utils",
       "//base",
       "//base/test:test_support",
       "//skia",

components/paint_preview/common/capture_result.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/common/capture_result.h"
+
+namespace paint_preview {
+
+RecordingParams::RecordingParams(const base::UnguessableToken& document_guid)
+    : document_guid(document_guid),
+      is_main_frame(false),
+      max_per_capture_size(0) {}
+
+CaptureResult::CaptureResult(mojom::Persistence persistence)
+    : persistence(persistence) {}
+
+CaptureResult::~CaptureResult() = default;
+
+CaptureResult::CaptureResult(CaptureResult&&) = default;
+
+CaptureResult& CaptureResult::operator=(CaptureResult&&) = default;
+
+}  // namespace paint_preview

components/paint_preview/common/capture_result.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_COMMON_CAPTURE_RESULT_H_
+#define COMPONENTS_PAINT_PREVIEW_COMMON_CAPTURE_RESULT_H_
+
+#include "base/containers/flat_map.h"
+#include "base/unguessable_token.h"
+#include "components/paint_preview/common/mojom/paint_preview_recorder.mojom-forward.h"
+#include "components/paint_preview/common/proto/paint_preview.pb.h"
+#include "mojo/public/cpp/base/big_buffer.h"
+#include "ui/gfx/geometry/rect.h"
+
+namespace paint_preview {
+
+// A subset of PaintPreviewCaptureParams that will be filled in by
+// PaintPreviewClient. This type mainly exists to aggregate related parameters.
+struct RecordingParams {
+  explicit RecordingParams(const base::UnguessableToken& document_guid);
+
+  // The document GUID for this capture.
+  const base::UnguessableToken document_guid;
+
+  // The rect to which to clip the capture to.
+  gfx::Rect clip_rect;
+
+  // Whether the capture is for the main frame or an OOP subframe.
+  bool is_main_frame;
+
+  // The maximum capture size allowed per SkPicture captured. A size of 0 is
+  // unlimited.
+  // TODO(crbug/1071446): Ideally, this would cap the total size rather than
+  // being a per SkPicture limit. However, that is non-trivial due to the
+  // async ordering of captures from different frames making it hard to keep
+  // track of available headroom at the time of each capture triggering.
+  size_t max_per_capture_size;
+};
+
+// The result of a capture of a WebContents, which may contain recordings of
+// multiple subframes.
+struct CaptureResult {
+ public:
+  explicit CaptureResult(mojom::Persistence persistence);
+  ~CaptureResult();
+
+  CaptureResult(CaptureResult&&);
+  CaptureResult& operator=(CaptureResult&&);
+
+  // Will match the |persistence| in the original capture request.
+  mojom::Persistence persistence;
+
+  PaintPreviewProto proto = {};
+
+  // Maps frame embedding tokens to buffers containing the serialized
+  // recordings. See |PaintPreviewCaptureResponse::skp| for information on how
+  // to intepret these buffers. Empty if |Persistence::FileSystem|.
+  base::flat_map<base::UnguessableToken, mojo_base::BigBuffer> serialized_skps =
+      {};
+
+  // Indicates that at least one subframe finished successfully.
+  bool capture_success = false;
+};
+
+}  // namespace paint_preview
+
+#endif

components/paint_preview/common/mojom/paint_preview_recorder.mojom

@@ -4,6 +4,7 @@
 
 module paint_preview.mojom;
 
+import "mojo/public/mojom/base/big_buffer.mojom";
 import "mojo/public/mojom/base/file.mojom";
 import "mojo/public/mojom/base/time.mojom";
 import "mojo/public/mojom/base/unguessable_token.mojom";
@@ -36,7 +37,24 @@ enum PaintPreviewStatus {
   kFailed,
 };
 
+// Enumation of strategies to store artifacts of an operation.
+enum Persistence {
+  // Store artifacts in the file system. This strategy can be robust to browser
+  // restarts, since the lifetime of the files can outlive the browser process.
+  kFileSystem,
+
+  // Store artifacts in memory buffers returned from or passed to capture or
+  // compositing methods. On low-memory devices, there is a higher risk of
+  // out-of-memory issues.
+  kMemoryBuffer,
+};
+
+// The collection of parameters needed for a recording of a render frame.
 struct PaintPreviewCaptureParams {
+  // The strategy for where to store the serialized SkPictures resulting from
+  // this capture.
+  Persistence persistence;
+
   // GUID for the Paint Preview (used to associate subframes to main frame).
   mojo_base.mojom.UnguessableToken guid;
 
@@ -52,7 +70,9 @@ struct PaintPreviewCaptureParams {
 
   // File to write the SkPicture to (write-only). A separate file should be
   // created for each RenderFrame.
-  mojo_base.mojom.File file;
+  //
+  // null if not |Persistence::FileSystem|.
+  mojo_base.mojom.File? file;
 
   // The maximum allowed size of a capture that can be produced. A value of
   // 0 means the size is unrestricted.
@@ -85,6 +105,15 @@ struct PaintPreviewCaptureResponse {
 
   // Scroll offsets of the frame at capture time.
   gfx.mojom.Size scroll_offsets;
+
+  // The serialized skia picture. It should be deserialized with the
+  // |SkDeserialProcs| that correspond with the |SkSerialProcs| it was
+  // serialized with.
+  //
+  // It is not safe to deserialize this in the browser process.
+  //
+  // null if not |Persistence::MemoryBuffer|.
+  mojo_base.mojom.BigBuffer? skp;
 };
 
 // Service for capturing a paint preview of a RenderFrame's contents. This

components/paint_preview/common/proto/paint_preview.proto

@@ -44,6 +44,8 @@ message PaintPreviewFrameProto {
   required bool is_main_frame = 3;
 
   // The file path to the serialized Skia Picture.
+  // null if the persistence type of the |PaintPreviewCaptureParams| is
+  // |Persistence::MemoryBuffer|.
   optional string file_path = 4;
 
   // A list of links within the frame.

components/paint_preview/common/test_utils.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/common/test_utils.h"
+
+std::string PersistenceParamToString(
+    const ::testing::TestParamInfo<paint_preview::mojom::Persistence>&
+        persistence) {
+  switch (persistence.param) {
+    case paint_preview::mojom::Persistence::kFileSystem:
+      return "FileSystem";
+    case paint_preview::mojom::Persistence::kMemoryBuffer:
+      return "MemoryBuffer";
+  }
+}

components/paint_preview/common/test_utils.h

@@ -5,6 +5,7 @@
 #ifndef COMPONENTS_PAINT_PREVIEW_COMMON_TEST_UTILS_H_
 #define COMPONENTS_PAINT_PREVIEW_COMMON_TEST_UTILS_H_
 
+#include "components/paint_preview/common/mojom/paint_preview_recorder.mojom-shared.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 MATCHER_P(EqualsProto, message, "") {
@@ -14,4 +15,9 @@ MATCHER_P(EqualsProto, message, "") {
   return expected_serialized == actual_serialized;
 }
 
+// Allow |mojom::Persistence| to be stringified in gtest.
+std::string PersistenceParamToString(
+    const ::testing::TestParamInfo<paint_preview::mojom::Persistence>&
+        persistence);
+
 #endif  // COMPONENTS_PAINT_PREVIEW_COMMON_TEST_UTILS_H_

components/paint_preview/renderer/BUILD.gn

@@ -38,6 +38,7 @@ if (!is_ios) {
       "//base",
       "//base/test:test_support",
       "//cc/paint",
+      "//components/paint_preview/common:test_utils",
       "//testing/gmock",
       "//testing/gtest",
     ]

components/paint_preview/renderer/paint_preview_recorder_impl.cc

@@ -18,7 +18,6 @@
 #include "cc/paint/paint_recorder.h"
 #include "components/paint_preview/renderer/paint_preview_recorder_utils.h"
 #include "content/public/renderer/render_frame.h"
-#include "mojo/public/cpp/base/shared_memory_utils.h"
 #include "third_party/blink/public/common/associated_interfaces/associated_interface_registry.h"
 #include "third_party/blink/public/web/web_local_frame.h"
 
@@ -46,6 +45,7 @@ FinishedRecording FinishRecording(
     sk_sp<const cc::PaintRecord> recording,
     const gfx::Rect& bounds,
     std::unique_ptr<PaintPreviewTracker> tracker,
+    mojom::Persistence persistence,
     base::File skp_file,
     size_t max_capture_size,
     mojom::PaintPreviewCaptureResponsePtr response) {
@@ -61,9 +61,24 @@ FinishedRecording FinishRecording(
   ParseGlyphs(recording.get(), tracker.get());
   TRACE_EVENT_END0("paint_preview", "ParseGlyphs");
   size_t serialized_size = 0;
-  if (!SerializeAsSkPicture(recording, tracker.get(), bounds,
-                            std::move(skp_file), max_capture_size,
-                            &serialized_size)) {
+
+  bool success = false;
+  switch (persistence) {
+    case mojom::Persistence::kFileSystem:
+      success = SerializeAsSkPictureToFile(recording, bounds, tracker.get(),
+                                           std::move(skp_file),
+                                           max_capture_size, &serialized_size);
+      break;
+    case mojom::Persistence::kMemoryBuffer:
+      mojo_base::BigBuffer buffer;
+      success = SerializeAsSkPictureToMemoryBuffer(
+          recording, bounds, tracker.get(), &buffer, max_capture_size,
+          &serialized_size);
+      out.response->skp.emplace(std::move(buffer));
+      break;
+  }
+
+  if (!success) {
     out.status = mojom::PaintPreviewStatus::kCaptureFailed;
     return out;
   }
@@ -215,7 +230,8 @@ void PaintPreviewRecorderImpl::CapturePaintPreviewInternal(
   // image.
   FinishedRecording recording = FinishRecording(
       recorder.finishRecordingAsPicture(), bounds, std::move(tracker),
-      std::move(params->file), params->max_capture_size, std::move(response));
+      params->persistence, std::move(params->file), params->max_capture_size,
+      std::move(response));
   std::move(callback).Run(recording.status, std::move(recording.response));
 }
 

components/paint_preview/renderer/paint_preview_recorder_utils.cc

@@ -33,12 +33,8 @@ void ParseGlyphs(const cc::PaintOpBuffer* buffer,
 bool SerializeAsSkPicture(sk_sp<const cc::PaintRecord> record,
                           PaintPreviewTracker* tracker,
                           const gfx::Rect& dimensions,
-                          base::File file,
-                          size_t max_size,
-                          size_t* serialized_size) {
+                          SkWStream* out_stream) {
   TRACE_EVENT0("paint_preview", "SerializeAsSkPicture");
-  if (!file.IsValid())
-    return false;
 
   // base::Unretained is safe as |tracker| outlives the usage of
   // |custom_callback|.
@@ -54,13 +50,10 @@ bool SerializeAsSkPicture(sk_sp<const cc::PaintRecord> record,
   TypefaceSerializationContext typeface_context(tracker->GetTypefaceUsageMap());
   auto serial_procs = MakeSerialProcs(tracker->GetPictureSerializationContext(),
                                       &typeface_context);
-  FileWStream stream(std::move(file), max_size);
-  skp->serialize(&stream, &serial_procs);
-  stream.flush();
-  stream.Close();
-  DCHECK(serialized_size);
-  *serialized_size = stream.ActualBytesWritten();
-  return !stream.DidWriteFail();
+
+  skp->serialize(out_stream, &serial_procs);
+  out_stream->flush();
+  return true;
 }
 
 void BuildResponse(PaintPreviewTracker* tracker,
@@ -79,4 +72,43 @@ void BuildResponse(PaintPreviewTracker* tracker,
   tracker->MoveLinks(&response->links);
 }
 
+bool SerializeAsSkPictureToFile(sk_sp<const cc::PaintRecord> recording,
+                                const gfx::Rect& bounds,
+                                PaintPreviewTracker* tracker,
+                                base::File file,
+                                size_t max_capture_size,
+                                size_t* serialized_size) {
+  if (!file.IsValid())
+    return false;
+
+  FileWStream file_stream(std::move(file), max_capture_size);
+  if (!SerializeAsSkPicture(recording, tracker, bounds, &file_stream))
+    return false;
+
+  file_stream.Close();
+  *serialized_size = file_stream.ActualBytesWritten();
+  return !file_stream.DidWriteFail();
+}
+
+bool SerializeAsSkPictureToMemoryBuffer(sk_sp<const cc::PaintRecord> recording,
+                                        const gfx::Rect& bounds,
+                                        PaintPreviewTracker* tracker,
+                                        mojo_base::BigBuffer* buffer,
+                                        size_t max_capture_size,
+                                        size_t* serialized_size) {
+  SkDynamicMemoryWStream memory_stream;
+  if (!SerializeAsSkPicture(recording, tracker, bounds, &memory_stream))
+    return false;
+
+  // |0| indicates "no size limit".
+  if (max_capture_size == 0)
+    max_capture_size = SIZE_MAX;
+
+  sk_sp<SkData> data = memory_stream.detachAsData();
+  *serialized_size = std::min(data->size(), max_capture_size);
+  *buffer = mojo_base::BigBuffer(
+      base::span<const uint8_t>(data->bytes(), *serialized_size));
+  return data->size() <= max_capture_size;
+}
+
 }  // namespace paint_preview

components/paint_preview/renderer/paint_preview_recorder_utils.h

@@ -8,7 +8,9 @@
 #include "base/files/file.h"
 #include "cc/paint/paint_record.h"
 #include "components/paint_preview/common/mojom/paint_preview_recorder.mojom-forward.h"
+#include "mojo/public/cpp/base/big_buffer.h"
 #include "third_party/skia/include/core/SkRefCnt.h"
+#include "third_party/skia/include/core/SkStream.h"
 #include "ui/gfx/geometry/rect.h"
 
 // These utilities are used by the PaintPreviewRecorderImpl. They are separate
@@ -23,17 +25,12 @@ class PaintPreviewTracker;
 // them to |tracker|.
 void ParseGlyphs(const cc::PaintOpBuffer* buffer, PaintPreviewTracker* tracker);
 
-// Serializes |record| to |file| as an SkPicture of size |dimensions|. |tracker|
-// supplies metadata required during serialization. |max_size| is a limit on the
-// total serialized size although 0 means the size is unrestricted. If
-// |max_size| is exceeded the serialization will fail. The size of the
-// serialized output is set as |serialized_size|.
+// Serializes |record| to |out_stream| as an SkPicture of size |dimensions|.
+// |tracker| supplies metadata required during serialization.
 bool SerializeAsSkPicture(sk_sp<const cc::PaintRecord> record,
                           PaintPreviewTracker* tracker,
                           const gfx::Rect& dimensions,
-                          base::File file,
-                          size_t max_size,
-                          size_t* serialized_size);
+                          SkWStream* out_stream);
 
 // Builds a mojom::PaintPreviewCaptureResponse |response| using the data
 // contained in |tracker|.
@@ -41,6 +38,36 @@ bool SerializeAsSkPicture(sk_sp<const cc::PaintRecord> record,
 void BuildResponse(PaintPreviewTracker* tracker,
                    mojom::PaintPreviewCaptureResponse* response);
 
+// Utility function that wraps |SerializeAsSkPicture| to serialize and write
+// |recording| to |file|.
+//
+// |max_size| is a limit on the total serialized size although 0 means the size
+// is unrestricted. If |max_size| is exceeded the serialization will fail.
+// |serialized_size| will contain the size of the serialized output.
+//
+// Returns |true| on success.
+bool SerializeAsSkPictureToFile(sk_sp<const cc::PaintRecord> recording,
+                                const gfx::Rect& bounds,
+                                PaintPreviewTracker* tracker,
+                                base::File file,
+                                size_t max_capture_size,
+                                size_t* serialized_size);
+
+// Utility function that wraps |SerializeAsSkPicture| to serialize and write
+// |recording| to |buffer|.
+//
+// |max_size| is a limit on the total serialized size although 0 means the size
+// is unrestricted. If |max_size| is exceeded the serialization will fail.
+// |serialized_size| will contain the size of the serialized output.
+//
+// Returns |true| on success.
+bool SerializeAsSkPictureToMemoryBuffer(sk_sp<const cc::PaintRecord> recording,
+                                        const gfx::Rect& bounds,
+                                        PaintPreviewTracker* tracker,
+                                        mojo_base::BigBuffer* buffer,
+                                        size_t max_capture_size,
+                                        size_t* serialized_size);
+
 }  // namespace paint_preview
 
 #endif  // COMPONENTS_PAINT_PREVIEW_RENDERER_PAINT_PREVIEW_RECORDER_UTILS_H_

components/paint_preview/renderer/paint_preview_recorder_utils_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "components/paint_preview/renderer/paint_preview_recorder_utils.h"
 
+#include <memory>
 #include <string>
 
 #include "base/containers/flat_map.h"
@@ -11,13 +12,18 @@
 #include "base/files/file.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/memory/read_only_shared_memory_region.h"
+#include "base/notreached.h"
+#include "base/optional.h"
 #include "base/unguessable_token.h"
 #include "cc/paint/paint_canvas.h"
 #include "cc/paint/paint_flags.h"
 #include "cc/paint/paint_recorder.h"
 #include "components/paint_preview/common/file_stream.h"
+#include "components/paint_preview/common/mojom/paint_preview_recorder.mojom-shared.h"
 #include "components/paint_preview/common/paint_preview_tracker.h"
 #include "components/paint_preview/common/serial_utils.h"
+#include "components/paint_preview/common/test_utils.h"
+#include "mojo/public/cpp/base/big_buffer.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkFont.h"
@@ -64,18 +70,80 @@ TEST(PaintPreviewRecorderUtilsTest, TestParseGlyphs) {
       (*usage_map)[typeface->uniqueID()]->IsSet(typeface->unicharToGlyph('g')));
 }
 
-TEST(PaintPreviewRecorderUtilsTest, TestSerializeAsSkPicture) {
-  PaintPreviewTracker tracker(base::UnguessableToken::Create(),
-                              base::UnguessableToken::Create(), true);
+class PaintPreviewRecorderUtilsSerializeAsSkPictureTest
+    : public testing::TestWithParam<mojom::Persistence> {
+ public:
+  PaintPreviewRecorderUtilsSerializeAsSkPictureTest()
+      : tracker(base::UnguessableToken::Create(),
+                base::UnguessableToken::Create(),
+                true),
+        dimensions(100, 100),
+        recorder() {}
+
+  ~PaintPreviewRecorderUtilsSerializeAsSkPictureTest() override = default;
+
+ protected:
+  void SetUp() override {
+    canvas = recorder.beginRecording(dimensions.width(), dimensions.width());
+    cc::PaintFlags flags;
+    canvas->drawRect(SkRect::MakeWH(dimensions.width(), dimensions.height()),
+                     flags);
+  }
+
+  base::Optional<std::unique_ptr<SkStream>> SerializeAsSkPicture(
+      base::Optional<size_t> max_capture_size,
+      size_t* serialized_size) {
+    auto record = recorder.finishRecordingAsPicture();
+    canvas = nullptr;
+
+    switch (GetParam()) {
+      case mojom::Persistence::kFileSystem: {
+        base::ScopedTempDir temp_dir;
+        if (!temp_dir.CreateUniqueTempDir())
+          return base::nullopt;
+
+        base::FilePath file_path = temp_dir.GetPath().AppendASCII("test_file");
+        base::File write_file(
+            file_path, base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
+        if (!SerializeAsSkPictureToFile(
+                record, dimensions, &tracker, std::move(write_file),
+                max_capture_size.value_or(0), serialized_size))
+          return base::nullopt;
+
+        base::File read_file(file_path, base::File::FLAG_OPEN |
+                                            base::File::FLAG_READ |
+                                            base::File::FLAG_EXCLUSIVE_READ);
+        return {std::make_unique<FileRStream>(std::move(read_file))};
+      } break;
+
+      case mojom::Persistence::kMemoryBuffer: {
+        mojo_base::BigBuffer memory_buffer;
+
+        if (!SerializeAsSkPictureToMemoryBuffer(
+                record, dimensions, &tracker, &memory_buffer,
+                max_capture_size.value_or(0), serialized_size))
+          return base::nullopt;
+
+        bool copy_data = true;
+        return {std::make_unique<SkMemoryStream>(
+            memory_buffer.data(), memory_buffer.size(), copy_data)};
+      } break;
+    }
+
+    NOTREACHED();
+    return base::nullopt;
+  }
 
-  gfx::Rect dimensions(100, 100);
+  PaintPreviewTracker tracker;
+
+  gfx::Rect dimensions;
   cc::PaintRecorder recorder;
-  cc::PaintCanvas* canvas =
-      recorder.beginRecording(dimensions.width(), dimensions.width());
-  cc::PaintFlags flags;
-  canvas->drawRect(SkRect::MakeWH(dimensions.width(), dimensions.height()),
-                   flags);
 
+  // Valid after SetUp() until SerializeAsSkPicture() is called.
+  cc::PaintCanvas* canvas{};
+};
+
+TEST_P(PaintPreviewRecorderUtilsSerializeAsSkPictureTest, Roundtrip) {
   base::flat_set<uint32_t> ctx;
   uint32_t content_id = tracker.CreateContentForRemoteFrame(
       gfx::Rect(10, 10), base::UnguessableToken::Create());
@@ -86,20 +154,10 @@ TEST(PaintPreviewRecorderUtilsTest, TestSerializeAsSkPicture) {
   canvas->recordCustomData(content_id);
   ctx.insert(content_id);
 
-  base::ScopedTempDir temp_dir;
-  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
-  base::FilePath file_path = temp_dir.GetPath().AppendASCII("test_file");
-  base::File write_file(
-      file_path, base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
-
-  auto record = recorder.finishRecordingAsPicture();
   size_t out_size = 0;
-  EXPECT_TRUE(SerializeAsSkPicture(record, &tracker, dimensions,
-                                   std::move(write_file), 0, &out_size));
-  base::File read_file(file_path, base::File::FLAG_OPEN |
-                                      base::File::FLAG_READ |
-                                      base::File::FLAG_EXCLUSIVE_READ);
-  FileRStream rstream(std::move(read_file));
+  auto rstream = SerializeAsSkPicture(base::nullopt, &out_size);
+  ASSERT_TRUE(rstream.has_value());
+
   SkDeserialProcs procs;
   procs.fPictureProc = [](const void* data, size_t length, void* ctx) {
     uint32_t content_id;
@@ -112,35 +170,23 @@ TEST(PaintPreviewRecorderUtilsTest, TestSerializeAsSkPicture) {
     return MakeEmptyPicture();
   };
   procs.fPictureCtx = &ctx;
-  SkPicture::MakeFromStream(&rstream, &procs);
+  SkPicture::MakeFromStream(rstream.value().get(), &procs);
   EXPECT_TRUE(ctx.empty());
 }
 
-TEST(PaintPreviewRecorderUtilsTest, TestSerializeAsSkPictureFail) {
-  PaintPreviewTracker tracker(base::UnguessableToken::Create(),
-                              base::UnguessableToken::Create(), true);
-
-  gfx::Rect dimensions(100, 100);
-  cc::PaintRecorder recorder;
-  cc::PaintCanvas* canvas =
-      recorder.beginRecording(dimensions.width(), dimensions.width());
-  cc::PaintFlags flags;
-  canvas->drawRect(SkRect::MakeWH(dimensions.width(), dimensions.height()),
-                   flags);
-
-  base::ScopedTempDir temp_dir;
-  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
-  base::FilePath file_path = temp_dir.GetPath().AppendASCII("test_file");
-  base::File write_file(
-      file_path, base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
-
-  auto record = recorder.finishRecordingAsPicture();
+TEST_P(PaintPreviewRecorderUtilsSerializeAsSkPictureTest, FailIfExceedMaxSize) {
   size_t out_size = 2;
-  EXPECT_FALSE(SerializeAsSkPicture(record, &tracker, dimensions,
-                                    std::move(write_file), 1, &out_size));
+  auto rstream = SerializeAsSkPicture({1}, &out_size);
+  EXPECT_FALSE(rstream.has_value());
   EXPECT_LE(out_size, 1U);
 }
 
+INSTANTIATE_TEST_SUITE_P(All,
+                         PaintPreviewRecorderUtilsSerializeAsSkPictureTest,
+                         testing::Values(mojom::Persistence::kFileSystem,
+                                         mojom::Persistence::kMemoryBuffer),
+                         PersistenceParamToString);
+
 TEST(PaintPreviewRecorderUtilsTest, TestBuildResponse) {
   auto token = base::UnguessableToken::Create();
   auto embedding_token = base::UnguessableToken::Create();

components/services/paint_preview_compositor/BUILD.gn

@@ -55,6 +55,7 @@ source_set("unit_tests") {
     "//base",
     "//base/test:test_support",
     "//components/paint_preview/common",
+    "//components/paint_preview/common:test_utils",
     "//components/paint_preview/common/proto",
     "//skia",
     "//testing/gmock",