[Paint Preview] Try to reduce memory use during capture

paint_preview::BuildResponse() shows up a fair amount in stack scans.
Often times these are OOMs I want to try to understand if this is a
material issue or if identical code folding is mangling BuildResponse
with other symbols.

This change does a few things

1) Try to reduce memory usage in FinishRecording() to reduce OOM risk.
   Mainly by freeing/reseting memory expensive objects earlier.
2) Log when paint_preview::BuildResponse() is legitimately called.
3) General refactoring/cleanup.

I hope to revert 2 once we know if the issue is actually
paint_preview::BuildResponse() at fault.

Change-Id: I14d99f1fb733565f984fc93fc131ee448fd642f9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2423408Reviewed-by: Mehran Mahmoudi <mahmoudi@chromium.org>
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809394}

components/paint_preview/renderer/paint_preview_recorder_impl.cc

@@ -64,28 +64,31 @@ FinishedRecording FinishRecording(
   TRACE_EVENT_BEGIN0("paint_preview", "ParseGlyphsAndLinks");
   ParseGlyphsAndLinks(recording.get(), tracker.get());
   TRACE_EVENT_END0("paint_preview", "ParseGlyphsAndLinks");
-  size_t serialized_size = 0;
 
   TRACE_EVENT0("paint_preview", "SerializeAsSkPicture");
 
-  auto skp = PaintRecordToSkPicture(recording, tracker.get(), bounds);
-  if (!skp) {
-    out.status = mojom::PaintPreviewStatus::kCaptureFailed;
-    return out;
-  }
-
   bool success = false;
-  switch (persistence) {
-    case RecordingPersistence::kFileSystem:
-      success = RecordToFile(std::move(skp_file), skp, tracker.get(),
-                             max_capture_size, &serialized_size);
-      break;
-    case RecordingPersistence::kMemoryBuffer:
-      base::Optional<mojo_base::BigBuffer> buffer = RecordToBuffer(
-          skp, tracker.get(), max_capture_size, &serialized_size);
-      success = buffer.has_value();
-      out.response->skp.emplace(std::move(buffer.value()));
-      break;
+  size_t serialized_size = 0;
+  {
+    auto skp = PaintRecordToSkPicture(recording, tracker.get(), bounds);
+    recording.reset();
+    if (!skp) {
+      out.status = mojom::PaintPreviewStatus::kCaptureFailed;
+      return out;
+    }
+
+    switch (persistence) {
+      case RecordingPersistence::kFileSystem:
+        success = RecordToFile(std::move(skp_file), skp, tracker.get(),
+                               max_capture_size, &serialized_size);
+        break;
+      case RecordingPersistence::kMemoryBuffer:
+        base::Optional<mojo_base::BigBuffer> buffer = RecordToBuffer(
+            skp, tracker.get(), max_capture_size, &serialized_size);
+        success = buffer.has_value();
+        out.response->skp.emplace(std::move(buffer.value()));
+        break;
+    }
   }
 
   if (!success) {
@@ -93,7 +96,7 @@ FinishedRecording FinishRecording(
     return out;
   }
 
-  BuildResponse(tracker.get(), out.response.get());
+  BuildResponse(tracker.get(), out.response.get(), /*log=*/true);
   out.response->serialized_size = serialized_size;
   return out;
 }

components/paint_preview/renderer/paint_preview_recorder_utils.cc

@@ -119,19 +119,29 @@ sk_sp<const SkPicture> PaintRecordToSkPicture(
 }
 
 void BuildResponse(PaintPreviewTracker* tracker,
-                   mojom::PaintPreviewCaptureResponse* response) {
+                   mojom::PaintPreviewCaptureResponse* response,
+                   bool log) {
+  // Ensure these always exist.
+  DCHECK(tracker);
+  DCHECK(response);
+
+  // paint_preview::BuildResponse has been showing in a large number of crashes
+  // under stack scans. In order to determine if these entries are "real" we
+  // should log the calls and check the log output.
+  if (log)
+    LOG(WARNING) << "paint_preview::BuildResponse() called";
+
   response->embedding_token = tracker->EmbeddingToken();
+  tracker->MoveLinks(&response->links);
 
   PictureSerializationContext* picture_context =
       tracker->GetPictureSerializationContext();
-  if (picture_context) {
-    for (const auto& id_pair : picture_context->content_id_to_embedding_token) {
-      response->content_id_to_embedding_token.insert(
-          {id_pair.first, id_pair.second});
-    }
-  }
+  if (!picture_context)
+    return;
 
-  tracker->MoveLinks(&response->links);
+  for (const auto& id_pair : picture_context->content_id_to_embedding_token) {
+    response->content_id_to_embedding_token.insert(id_pair);
+  }
 }
 
 }  // namespace paint_preview

components/paint_preview/renderer/paint_preview_recorder_utils.h

@@ -35,7 +35,8 @@ sk_sp<const SkPicture> PaintRecordToSkPicture(
 
 // NOTE: |tracker| is effectively const here despite being passed by pointer.
 void BuildResponse(PaintPreviewTracker* tracker,
-                   mojom::PaintPreviewCaptureResponse* response);
+                   mojom::PaintPreviewCaptureResponse* response,
+                   bool log = false);
 
 }  // namespace paint_preview