Commit 5ef34b5a authored by ckitagawa's avatar ckitagawa Committed by Commit Bot

[Paint Preview] Pass all data via Mojo

Because it is potentially unsafe to handle a proto generated in the
renderer process in the browser process. The data should to be passed
as a mojo struct instead.

Bug: 1017350
Change-Id: I83c02eb65bdf7b0dc90412470c763bbfbe052462
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1875283Reviewed-by: default avatarKen Buchanan <kenrb@chromium.org>
Reviewed-by: default avatarIan Vollick <vollick@chromium.org>
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709164}
parent 0a7676ab
...@@ -24,10 +24,11 @@ if (!is_ios) { ...@@ -24,10 +24,11 @@ if (!is_ios) {
"//skia", "//skia",
"//third_party:freetype_harfbuzz", "//third_party:freetype_harfbuzz",
"//ui/gfx/geometry", "//ui/gfx/geometry",
"//url",
] ]
public_deps = [ public_deps = [
"//components/paint_preview/common/proto", "//components/paint_preview/common/mojom",
] ]
} }
......
...@@ -10,8 +10,8 @@ mojom("mojom") { ...@@ -10,8 +10,8 @@ mojom("mojom") {
] ]
public_deps = [ public_deps = [
"//components/discardable_memory/public/mojom",
"//mojo/public/mojom/base", "//mojo/public/mojom/base",
"//ui/gfx/geometry/mojom", "//ui/gfx/geometry/mojom",
"//url/mojom:url_mojom_gurl",
] ]
} }
...@@ -4,11 +4,10 @@ ...@@ -4,11 +4,10 @@
module paint_preview.mojom; module paint_preview.mojom;
import "components/discardable_memory/public/mojom/discardable_shared_memory_manager.mojom";
import "mojo/public/mojom/base/file.mojom"; import "mojo/public/mojom/base/file.mojom";
import "mojo/public/mojom/base/shared_memory.mojom";
import "mojo/public/mojom/base/unguessable_token.mojom"; import "mojo/public/mojom/base/unguessable_token.mojom";
import "ui/gfx/geometry/mojom/geometry.mojom"; import "ui/gfx/geometry/mojom/geometry.mojom";
import "url/mojom/url.mojom";
// Status codes for the PaintPreviewRecorder. // Status codes for the PaintPreviewRecorder.
enum PaintPreviewStatus { enum PaintPreviewStatus {
...@@ -42,6 +41,25 @@ struct PaintPreviewCaptureParams { ...@@ -42,6 +41,25 @@ struct PaintPreviewCaptureParams {
mojo_base.mojom.File file; mojo_base.mojom.File file;
}; };
struct LinkData {
// URL of the link.
url.mojom.Url url;
// Rect for the link.
gfx.mojom.Rect rect;
};
struct PaintPreviewCaptureResponse {
// Routing ID of the frame.
uint64 id;
// Map between subframe content IDs and the RenderFrameProxy Routing ID.
map<uint32, uint64> content_id_proxy_id_map;
// A list of links within the frame.
array<LinkData> links;
};
// Service for capturing a paint preview of a RenderFrame's contents. This // Service for capturing a paint preview of a RenderFrame's contents. This
// includes both the visual contents (as an SkPicture) and hyperlinks // includes both the visual contents (as an SkPicture) and hyperlinks
// for the frame. // for the frame.
...@@ -53,9 +71,9 @@ interface PaintPreviewRecorder { ...@@ -53,9 +71,9 @@ interface PaintPreviewRecorder {
// via the RenderFrameProxy. The browser handles dispatching these requests to // via the RenderFrameProxy. The browser handles dispatching these requests to
// the correct RenderFrame and aggregating all the outputs. // the correct RenderFrame and aggregating all the outputs.
// //
// Returns a status. If |status| == kOk then proto contains a serialized // Returns a status, and if |status| == kOK a valid instance of
// PaintPreviewFrameProto. // PaintPreviewCaptureResponse.
CapturePaintPreview(PaintPreviewCaptureParams params) => CapturePaintPreview(PaintPreviewCaptureParams params) =>
(PaintPreviewStatus status, (PaintPreviewStatus status,
mojo_base.mojom.ReadOnlySharedMemoryRegion? proto); PaintPreviewCaptureResponse response);
}; };
...@@ -12,12 +12,13 @@ ...@@ -12,12 +12,13 @@
#include "base/logging.h" #include "base/logging.h"
#include "base/stl_util.h" #include "base/stl_util.h"
#include "components/paint_preview/common/glyph_usage.h" #include "components/paint_preview/common/glyph_usage.h"
#include "components/paint_preview/common/proto/paint_preview.pb.h" #include "components/paint_preview/common/mojom/paint_preview_recorder.mojom.h"
#include "third_party/skia/include/core/SkCanvas.h" #include "third_party/skia/include/core/SkCanvas.h"
#include "third_party/skia/include/core/SkMatrix.h" #include "third_party/skia/include/core/SkMatrix.h"
#include "third_party/skia/include/core/SkTextBlob.h" #include "third_party/skia/include/core/SkTextBlob.h"
#include "third_party/skia/include/core/SkTypeface.h" #include "third_party/skia/include/core/SkTypeface.h"
#include "ui/gfx/geometry/rect.h" #include "ui/gfx/geometry/rect.h"
#include "url/gurl.h"
namespace paint_preview { namespace paint_preview {
...@@ -35,13 +36,6 @@ bool ShouldUseDenseGlyphUsage(SkTypeface* typeface) { ...@@ -35,13 +36,6 @@ bool ShouldUseDenseGlyphUsage(SkTypeface* typeface) {
return typeface->countGlyphs() < kMaxGlyphsForDenseGlyphUsage; return typeface->countGlyphs() < kMaxGlyphsForDenseGlyphUsage;
} }
void RectToRectProto(RectProto* rect_proto, const gfx::Rect& rect) {
rect_proto->set_x(rect.x());
rect_proto->set_y(rect.y());
rect_proto->set_width(rect.width());
rect_proto->set_height(rect.height());
}
} // namespace } // namespace
PaintPreviewTracker::PaintPreviewTracker(const base::UnguessableToken& guid, PaintPreviewTracker::PaintPreviewTracker(const base::UnguessableToken& guid,
...@@ -91,12 +85,8 @@ void PaintPreviewTracker::AddGlyphs(const SkTextBlob* blob) { ...@@ -91,12 +85,8 @@ void PaintPreviewTracker::AddGlyphs(const SkTextBlob* blob) {
} }
} }
void PaintPreviewTracker::AnnotateLink(const std::string& url, void PaintPreviewTracker::AnnotateLink(const GURL& url, const gfx::Rect& rect) {
const gfx::Rect& rect) { links_.push_back(mojom::LinkData(url, rect));
LinkDataProto link_data;
RectToRectProto(link_data.mutable_rect(), rect);
link_data.set_url(url);
links_.push_back(link_data);
} }
void PaintPreviewTracker::CustomDataToSkPictureCallback(SkCanvas* canvas, void PaintPreviewTracker::CustomDataToSkPictureCallback(SkCanvas* canvas,
......
...@@ -14,12 +14,13 @@ ...@@ -14,12 +14,13 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/unguessable_token.h" #include "base/unguessable_token.h"
#include "components/paint_preview/common/glyph_usage.h" #include "components/paint_preview/common/glyph_usage.h"
#include "components/paint_preview/common/proto/paint_preview.pb.h" #include "components/paint_preview/common/mojom/paint_preview_recorder.mojom.h"
#include "components/paint_preview/common/serial_utils.h" #include "components/paint_preview/common/serial_utils.h"
#include "third_party/skia/include/core/SkPicture.h" #include "third_party/skia/include/core/SkPicture.h"
#include "third_party/skia/include/core/SkRefCnt.h" #include "third_party/skia/include/core/SkRefCnt.h"
#include "third_party/skia/include/core/SkTextBlob.h" #include "third_party/skia/include/core/SkTextBlob.h"
#include "ui/gfx/geometry/rect.h" #include "ui/gfx/geometry/rect.h"
#include "url/gurl.h"
namespace paint_preview { namespace paint_preview {
...@@ -50,7 +51,7 @@ class PaintPreviewTracker { ...@@ -50,7 +51,7 @@ class PaintPreviewTracker {
void AddGlyphs(const SkTextBlob* blob); void AddGlyphs(const SkTextBlob* blob);
// Adds |link| with bounding box |rect| to the list of links. // Adds |link| with bounding box |rect| to the list of links.
void AnnotateLink(const std::string& link, const gfx::Rect& rect); void AnnotateLink(const GURL& link, const gfx::Rect& rect);
// Data Serialization ------------------------------------------------------- // Data Serialization -------------------------------------------------------
// NOTE: once any of these methods are called the PaintPreviewTracker should // NOTE: once any of these methods are called the PaintPreviewTracker should
...@@ -68,14 +69,14 @@ class PaintPreviewTracker { ...@@ -68,14 +69,14 @@ class PaintPreviewTracker {
TypefaceUsageMap* GetTypefaceUsageMap() { return &typeface_glyph_usage_; } TypefaceUsageMap* GetTypefaceUsageMap() { return &typeface_glyph_usage_; }
// Expose links for serialization to a PaintPreviewFrameProto. // Expose links for serialization to a PaintPreviewFrameProto.
const std::vector<LinkDataProto>& GetLinks() const { return links_; } const std::vector<mojom::LinkData>& GetLinks() const { return links_; }
private: private:
const base::UnguessableToken guid_; const base::UnguessableToken guid_;
const int routing_id_; const int routing_id_;
const bool is_main_frame_; const bool is_main_frame_;
std::vector<LinkDataProto> links_; std::vector<mojom::LinkData> links_;
PictureSerializationContext content_id_to_proxy_id_; PictureSerializationContext content_id_to_proxy_id_;
TypefaceUsageMap typeface_glyph_usage_; TypefaceUsageMap typeface_glyph_usage_;
base::flat_map<uint32_t, sk_sp<SkPicture>> subframe_pics_; base::flat_map<uint32_t, sk_sp<SkPicture>> subframe_pics_;
......
...@@ -44,7 +44,7 @@ TEST(PaintPreviewTrackerTest, TestRemoteFramePlaceholderPicture) { ...@@ -44,7 +44,7 @@ TEST(PaintPreviewTrackerTest, TestRemoteFramePlaceholderPicture) {
PictureSerializationContext* context = PictureSerializationContext* context =
tracker.GetPictureSerializationContext(); tracker.GetPictureSerializationContext();
EXPECT_TRUE(context->count(content_id)); EXPECT_TRUE(context->count(content_id));
EXPECT_EQ((*context)[content_id], kRoutingId); EXPECT_EQ((*context)[content_id], static_cast<uint32_t>(kRoutingId));
SkPictureRecorder recorder; SkPictureRecorder recorder;
SkCanvas* canvas = recorder.beginRecording(100, 100); SkCanvas* canvas = recorder.beginRecording(100, 100);
...@@ -77,25 +77,25 @@ TEST(PaintPreviewTrackerTest, TestGlyphRunList) { ...@@ -77,25 +77,25 @@ TEST(PaintPreviewTrackerTest, TestGlyphRunList) {
TEST(PaintPreviewTrackerTest, TestAnnotateLinks) { TEST(PaintPreviewTrackerTest, TestAnnotateLinks) {
PaintPreviewTracker tracker(base::UnguessableToken::Create(), kRoutingId, PaintPreviewTracker tracker(base::UnguessableToken::Create(), kRoutingId,
true); true);
const std::string url_1 = "https://www.chromium.org"; const GURL url_1("https://www.chromium.org");
const gfx::Rect rect_1(10, 20, 30, 40); const gfx::Rect rect_1(10, 20, 30, 40);
tracker.AnnotateLink(url_1, rect_1); tracker.AnnotateLink(url_1, rect_1);
const std::string url_2 = "https://www.w3.org"; const GURL url_2("https://www.w3.org");
const gfx::Rect rect_2(15, 25, 35, 45); const gfx::Rect rect_2(15, 25, 35, 45);
tracker.AnnotateLink(url_2, rect_2); tracker.AnnotateLink(url_2, rect_2);
auto links = tracker.GetLinks(); auto links = tracker.GetLinks();
EXPECT_EQ(links[0].url(), url_1); EXPECT_EQ(links[0].url, url_1);
EXPECT_EQ(links[0].rect().width(), rect_1.width()); EXPECT_EQ(links[0].rect.width(), rect_1.width());
EXPECT_EQ(links[0].rect().height(), rect_1.height()); EXPECT_EQ(links[0].rect.height(), rect_1.height());
EXPECT_EQ(links[0].rect().x(), rect_1.x()); EXPECT_EQ(links[0].rect.x(), rect_1.x());
EXPECT_EQ(links[0].rect().y(), rect_1.y()); EXPECT_EQ(links[0].rect.y(), rect_1.y());
EXPECT_EQ(links[1].url(), url_2); EXPECT_EQ(links[1].url, url_2);
EXPECT_EQ(links[1].rect().width(), rect_2.width()); EXPECT_EQ(links[1].rect.width(), rect_2.width());
EXPECT_EQ(links[1].rect().height(), rect_2.height()); EXPECT_EQ(links[1].rect.height(), rect_2.height());
EXPECT_EQ(links[1].rect().x(), rect_2.x()); EXPECT_EQ(links[1].rect.x(), rect_2.x());
EXPECT_EQ(links[1].rect().y(), rect_2.y()); EXPECT_EQ(links[1].rect.y(), rect_2.y());
} }
} // namespace paint_preview } // namespace paint_preview
...@@ -25,27 +25,23 @@ message LinkDataProto { ...@@ -25,27 +25,23 @@ message LinkDataProto {
} }
// A paint preview of a single frame. // A paint preview of a single frame.
// NEXT_TAG = 8 // NEXT_TAG = 6
message PaintPreviewFrameProto { message PaintPreviewFrameProto {
// Serialized base::UnguessableToken.
required uint64 unguessable_token_high = 1;
required uint64 unguessable_token_low = 2;
// Originates in renderer as Routing ID. // Originates in renderer as Routing ID.
// Converted to (Process ID || Routing ID) once processed in browser. // Converted to (Process ID || Routing ID) once processed in browser.
required uint64 id = 3; required uint64 id = 1;
// Boolean indicating if the frame is the main frame. // Boolean indicating if the frame is the main frame.
required bool is_main_frame = 4; required bool is_main_frame = 2;
// The file path to the serialized Skia Picture. // The file path to the serialized Skia Picture.
optional string file_path = 5; optional string file_path = 3;
// A list of links within the frame. // A list of links within the frame.
repeated LinkDataProto links = 6; repeated LinkDataProto links = 4;
// A map between the content IDs of subframes and the |id| field. // A map between the content IDs of subframes and the |id| field.
map<uint32, int64> content_id_proxy_id_map = 7; map<uint32, int64> content_id_proxy_id_map = 5;
} }
// A paint preview of the entire page. // A paint preview of the entire page.
......
...@@ -19,7 +19,7 @@ ...@@ -19,7 +19,7 @@
namespace paint_preview { namespace paint_preview {
// Maps a content ID to a frame ID (Process ID || Routing ID). // Maps a content ID to a frame ID (Process ID || Routing ID).
using PictureSerializationContext = base::flat_map<uint32_t, int64_t>; using PictureSerializationContext = base::flat_map<uint32_t, uint64_t>;
// Maps a typeface ID to a glyph usage tracker. // Maps a typeface ID to a glyph usage tracker.
using TypefaceUsageMap = base::flat_map<SkFontID, std::unique_ptr<GlyphUsage>>; using TypefaceUsageMap = base::flat_map<SkFontID, std::unique_ptr<GlyphUsage>>;
......
...@@ -25,7 +25,6 @@ if (!is_ios) { ...@@ -25,7 +25,6 @@ if (!is_ios) {
public_deps = [ public_deps = [
"//components/paint_preview/common", "//components/paint_preview/common",
"//components/paint_preview/common/mojom", "//components/paint_preview/common/mojom",
"//components/paint_preview/common/proto",
] ]
} }
...@@ -41,7 +40,6 @@ if (!is_ios) { ...@@ -41,7 +40,6 @@ if (!is_ios) {
"//base", "//base",
"//base/test:test_support", "//base/test:test_support",
"//cc/paint", "//cc/paint",
"//components/paint_preview/common:test_utils",
"//testing/gmock", "//testing/gmock",
"//testing/gtest", "//testing/gtest",
] ]
......
...@@ -8,7 +8,6 @@ ...@@ -8,7 +8,6 @@
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/files/scoped_temp_dir.h" #include "base/files/scoped_temp_dir.h"
#include "components/paint_preview/common/mojom/paint_preview_recorder.mojom.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/renderer/render_frame.h" #include "content/public/renderer/render_frame.h"
#include "content/public/renderer/render_view.h" #include "content/public/renderer/render_view.h"
#include "content/public/test/render_view_test.h" #include "content/public/test/render_view_test.h"
...@@ -18,19 +17,16 @@ namespace paint_preview { ...@@ -18,19 +17,16 @@ namespace paint_preview {
namespace { namespace {
// Checks that |status| == |expected_status| and loads |region| into |proto| if // Checks that |status| == |expected_status| and loads |response| into
// |expected_status| == kOk. If |expected_status| != kOk |proto| can safely be // |out_response| if |expected_status| == kOk. If |expected_status| != kOk
// nullptr. // |out_response| can safely be nullptr.
void OnCaptureFinished(mojom::PaintPreviewStatus expected_status, void OnCaptureFinished(mojom::PaintPreviewStatus expected_status,
PaintPreviewFrameProto* proto, mojom::PaintPreviewCaptureResponsePtr* out_response,
mojom::PaintPreviewStatus status, mojom::PaintPreviewStatus status,
base::ReadOnlySharedMemoryRegion region) { mojom::PaintPreviewCaptureResponsePtr response) {
EXPECT_EQ(status, expected_status); EXPECT_EQ(status, expected_status);
if (expected_status == mojom::PaintPreviewStatus::kOk) { if (expected_status == mojom::PaintPreviewStatus::kOk)
EXPECT_TRUE(region.IsValid()); *out_response = std::move(response);
auto mapping = region.Map();
EXPECT_TRUE(proto->ParseFromArray(mapping.memory(), mapping.size()));
}
} }
} // namespace } // namespace
...@@ -74,24 +70,20 @@ TEST_F(PaintPreviewRecorderRenderViewTest, TestCaptureMainFrame) { ...@@ -74,24 +70,20 @@ TEST_F(PaintPreviewRecorderRenderViewTest, TestCaptureMainFrame) {
base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE); base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
params->file = std::move(skp_file); params->file = std::move(skp_file);
PaintPreviewFrameProto out_proto; auto out_response = mojom::PaintPreviewCaptureResponse::New();
content::RenderFrame* frame = GetFrame(); content::RenderFrame* frame = GetFrame();
int routing_id = frame->GetRoutingID(); int routing_id = frame->GetRoutingID();
PaintPreviewRecorderImpl paint_preview_recorder(frame); PaintPreviewRecorderImpl paint_preview_recorder(frame);
paint_preview_recorder.CapturePaintPreview( paint_preview_recorder.CapturePaintPreview(
std::move(params), std::move(params),
base::BindOnce(&OnCaptureFinished, mojom::PaintPreviewStatus::kOk, base::BindOnce(&OnCaptureFinished, mojom::PaintPreviewStatus::kOk,
&out_proto)); base::Unretained(&out_response)));
// Here id() is just the routing ID. // Here id() is just the routing ID.
EXPECT_EQ(static_cast<int>(out_proto.id()), routing_id); EXPECT_EQ(static_cast<int>(out_response->id), routing_id);
EXPECT_TRUE(out_proto.is_main_frame()); EXPECT_EQ(out_response->content_id_proxy_id_map.size(), 0U);
EXPECT_EQ(out_proto.unguessable_token_low(), token.GetLowForSerialization());
EXPECT_EQ(out_proto.unguessable_token_high(),
token.GetHighForSerialization());
EXPECT_EQ(out_proto.content_id_proxy_id_map_size(), 0);
// NOTE: should be non-zero once the Blink implementation is hooked up. // NOTE: should be non-zero once the Blink implementation is hooked up.
EXPECT_EQ(out_proto.links_size(), 0); EXPECT_EQ(out_response->links.size(), 0U);
} }
TEST_F(PaintPreviewRecorderRenderViewTest, TestCaptureInvalidFile) { TEST_F(PaintPreviewRecorderRenderViewTest, TestCaptureInvalidFile) {
......
...@@ -7,7 +7,6 @@ ...@@ -7,7 +7,6 @@
#include <utility> #include <utility>
#include "base/auto_reset.h" #include "base/auto_reset.h"
#include "base/memory/shared_memory.h"
#include "base/task_runner.h" #include "base/task_runner.h"
#include "cc/paint/paint_record.h" #include "cc/paint/paint_record.h"
#include "cc/paint/paint_recorder.h" #include "cc/paint/paint_recorder.h"
...@@ -26,13 +25,12 @@ mojom::PaintPreviewStatus FinishRecording( ...@@ -26,13 +25,12 @@ mojom::PaintPreviewStatus FinishRecording(
const gfx::Rect& bounds, const gfx::Rect& bounds,
PaintPreviewTracker* tracker, PaintPreviewTracker* tracker,
base::File skp_file, base::File skp_file,
base::ReadOnlySharedMemoryRegion* region) { mojom::PaintPreviewCaptureResponse* response) {
ParseGlyphs(recording.get(), tracker); ParseGlyphs(recording.get(), tracker);
if (!SerializeAsSkPicture(recording, tracker, bounds, std::move(skp_file))) if (!SerializeAsSkPicture(recording, tracker, bounds, std::move(skp_file)))
return mojom::PaintPreviewStatus::kCaptureFailed; return mojom::PaintPreviewStatus::kCaptureFailed;
if (!BuildAndSerializeProto(tracker, region)) BuildResponse(tracker, response);
return mojom::PaintPreviewStatus::kProtoSerializationFailed;
return mojom::PaintPreviewStatus::kOk; return mojom::PaintPreviewStatus::kOk;
} }
...@@ -63,15 +61,16 @@ void PaintPreviewRecorderImpl::CapturePaintPreview( ...@@ -63,15 +61,16 @@ void PaintPreviewRecorderImpl::CapturePaintPreview(
// might happen due to it being tied to a RenderFrame rather than // might happen due to it being tied to a RenderFrame rather than
// RenderWidget and we don't want to crash the renderer as this is // RenderWidget and we don't want to crash the renderer as this is
// recoverable. // recoverable.
auto response = mojom::PaintPreviewCaptureResponse::New();
if (is_painting_preview_) { if (is_painting_preview_) {
status = mojom::PaintPreviewStatus::kAlreadyCapturing; status = mojom::PaintPreviewStatus::kAlreadyCapturing;
std::move(callback).Run(status, std::move(region)); std::move(callback).Run(status, std::move(response));
return; return;
} }
base::AutoReset<bool>(&is_painting_preview_, true); base::AutoReset<bool>(&is_painting_preview_, true);
CapturePaintPreviewInternal(params, &region, &status); CapturePaintPreviewInternal(params, response.get(), &status);
std::move(callback).Run(status, std::move(region)); std::move(callback).Run(status, std::move(response));
} }
void PaintPreviewRecorderImpl::OnDestruct() { void PaintPreviewRecorderImpl::OnDestruct() {
...@@ -86,7 +85,7 @@ void PaintPreviewRecorderImpl::BindPaintPreviewRecorder( ...@@ -86,7 +85,7 @@ void PaintPreviewRecorderImpl::BindPaintPreviewRecorder(
void PaintPreviewRecorderImpl::CapturePaintPreviewInternal( void PaintPreviewRecorderImpl::CapturePaintPreviewInternal(
const mojom::PaintPreviewCaptureParamsPtr& params, const mojom::PaintPreviewCaptureParamsPtr& params,
base::ReadOnlySharedMemoryRegion* region, mojom::PaintPreviewCaptureResponse* response,
mojom::PaintPreviewStatus* status) { mojom::PaintPreviewStatus* status) {
blink::WebLocalFrame* frame = render_frame()->GetWebFrame(); blink::WebLocalFrame* frame = render_frame()->GetWebFrame();
// Warm up paint for an out-of-lifecycle paint phase. // Warm up paint for an out-of-lifecycle paint phase.
...@@ -114,7 +113,7 @@ void PaintPreviewRecorderImpl::CapturePaintPreviewInternal( ...@@ -114,7 +113,7 @@ void PaintPreviewRecorderImpl::CapturePaintPreviewInternal(
// TODO(crbug/1011896): Determine if making this async would be beneficial. // TODO(crbug/1011896): Determine if making this async would be beneficial.
*status = FinishRecording(recorder.finishRecordingAsPicture(), bounds, *status = FinishRecording(recorder.finishRecordingAsPicture(), bounds,
&tracker, std::move(params->file), region); &tracker, std::move(params->file), response);
} }
} // namespace paint_preview } // namespace paint_preview
...@@ -15,10 +15,6 @@ ...@@ -15,10 +15,6 @@
#include "mojo/public/cpp/bindings/associated_receiver.h" #include "mojo/public/cpp/bindings/associated_receiver.h"
#include "mojo/public/cpp/bindings/pending_associated_receiver.h" #include "mojo/public/cpp/bindings/pending_associated_receiver.h"
namespace base {
class ReadOnlySharedMemoryRegion;
} // namespace base
namespace content { namespace content {
class RenderFrame; class RenderFrame;
} // namespace content } // namespace content
...@@ -49,7 +45,7 @@ class PaintPreviewRecorderImpl : public content::RenderFrameObserver, ...@@ -49,7 +45,7 @@ class PaintPreviewRecorderImpl : public content::RenderFrameObserver,
// Handles the bulk of the capture. // Handles the bulk of the capture.
void CapturePaintPreviewInternal( void CapturePaintPreviewInternal(
const mojom::PaintPreviewCaptureParamsPtr& params, const mojom::PaintPreviewCaptureParamsPtr& params,
base::ReadOnlySharedMemoryRegion* region, mojom::PaintPreviewCaptureResponse* region,
mojom::PaintPreviewStatus* status); mojom::PaintPreviewStatus* status);
bool is_painting_preview_; bool is_painting_preview_;
......
...@@ -7,10 +7,8 @@ ...@@ -7,10 +7,8 @@
#include <utility> #include <utility>
#include "base/bind.h" #include "base/bind.h"
#include "base/memory/shared_memory.h"
#include "components/paint_preview/common/file_stream.h" #include "components/paint_preview/common/file_stream.h"
#include "components/paint_preview/common/paint_preview_tracker.h" #include "components/paint_preview/common/paint_preview_tracker.h"
#include "components/paint_preview/common/proto/paint_preview.pb.h"
#include "mojo/public/cpp/base/shared_memory_utils.h" #include "mojo/public/cpp/base/shared_memory_utils.h"
namespace paint_preview { namespace paint_preview {
...@@ -59,31 +57,14 @@ bool SerializeAsSkPicture(sk_sp<const cc::PaintRecord> record, ...@@ -59,31 +57,14 @@ bool SerializeAsSkPicture(sk_sp<const cc::PaintRecord> record,
return true; return true;
} }
bool BuildAndSerializeProto(PaintPreviewTracker* tracker, void BuildResponse(PaintPreviewTracker* tracker,
base::ReadOnlySharedMemoryRegion* region) { mojom::PaintPreviewCaptureResponse* response) {
PaintPreviewFrameProto proto; response->id = tracker->RoutingId();
proto.set_unguessable_token_high(tracker->Guid().GetHighForSerialization()); for (const auto& id_pair : *(tracker->GetPictureSerializationContext())) {
proto.set_unguessable_token_low(tracker->Guid().GetLowForSerialization()); response->content_id_proxy_id_map.insert({id_pair.first, id_pair.second});
proto.set_id(tracker->RoutingId()); }
proto.set_is_main_frame(tracker->IsMainFrame());
auto* proto_content_proxy_map = proto.mutable_content_id_proxy_id_map();
for (const auto& id_pair : *(tracker->GetPictureSerializationContext()))
proto_content_proxy_map->insert(
google::protobuf::MapPair<uint32_t, int64_t>(id_pair.first,
id_pair.second));
for (const auto& link : tracker->GetLinks()) for (const auto& link : tracker->GetLinks())
*proto.add_links() = link; response->links.push_back(link.Clone());
base::MappedReadOnlyRegion region_mapping =
mojo::CreateReadOnlySharedMemoryRegion(proto.ByteSizeLong());
if (!region_mapping.IsValid())
return false;
bool success = proto.SerializeToArray(region_mapping.mapping.memory(),
proto.ByteSizeLong());
if (success)
*region = std::move(region_mapping.region);
return success;
} }
} // namespace paint_preview } // namespace paint_preview
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "base/files/file.h" #include "base/files/file.h"
#include "cc/paint/paint_record.h" #include "cc/paint/paint_record.h"
#include "components/paint_preview/common/mojom/paint_preview_recorder.mojom-forward.h"
#include "third_party/skia/include/core/SkRefCnt.h" #include "third_party/skia/include/core/SkRefCnt.h"
#include "ui/gfx/geometry/rect.h" #include "ui/gfx/geometry/rect.h"
...@@ -14,10 +15,6 @@ ...@@ -14,10 +15,6 @@
// for testing purposes and to enforce restrictions caused by the lifetime of // for testing purposes and to enforce restrictions caused by the lifetime of
// PaintPreviewServiceImpl being tied to it's associated RenderFrame. // PaintPreviewServiceImpl being tied to it's associated RenderFrame.
namespace base {
class ReadOnlySharedMemoryRegion;
} // namespace base
namespace paint_preview { namespace paint_preview {
class PaintPreviewTracker; class PaintPreviewTracker;
...@@ -33,11 +30,11 @@ bool SerializeAsSkPicture(sk_sp<const cc::PaintRecord> record, ...@@ -33,11 +30,11 @@ bool SerializeAsSkPicture(sk_sp<const cc::PaintRecord> record,
const gfx::Rect& dimensions, const gfx::Rect& dimensions,
base::File file); base::File file);
// Builds and serializes a proto to |region| using the data contained in // Builds a mojom::PaintPreviewCaptureResponse |response| using the data
// |tracker|. Returns true on success. // contained in |tracker|. Returns true on success.
// NOTE: |tracker| is effectively const here despite being passed by pointer. // NOTE: |tracker| is effectively const here despite being passed by pointer.
bool BuildAndSerializeProto(PaintPreviewTracker* tracker, void BuildResponse(PaintPreviewTracker* tracker,
base::ReadOnlySharedMemoryRegion* region); mojom::PaintPreviewCaptureResponse* response);
} // namespace paint_preview } // namespace paint_preview
......
...@@ -17,9 +17,7 @@ ...@@ -17,9 +17,7 @@
#include "cc/paint/paint_recorder.h" #include "cc/paint/paint_recorder.h"
#include "components/paint_preview/common/file_stream.h" #include "components/paint_preview/common/file_stream.h"
#include "components/paint_preview/common/paint_preview_tracker.h" #include "components/paint_preview/common/paint_preview_tracker.h"
#include "components/paint_preview/common/proto/paint_preview.pb.h"
#include "components/paint_preview/common/serial_utils.h" #include "components/paint_preview/common/serial_utils.h"
#include "components/paint_preview/common/test_utils.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "third_party/skia/include/core/SkFont.h" #include "third_party/skia/include/core/SkFont.h"
...@@ -126,31 +124,23 @@ TEST(PaintPreviewServiceUtilsTest, TestSerializeAsSkPicture) { ...@@ -126,31 +124,23 @@ TEST(PaintPreviewServiceUtilsTest, TestSerializeAsSkPicture) {
TEST(PaintPreviewServiceUtilsTest, TestBuildAndSerializeProto) { TEST(PaintPreviewServiceUtilsTest, TestBuildAndSerializeProto) {
auto token = base::UnguessableToken::Create(); auto token = base::UnguessableToken::Create();
PaintPreviewTracker tracker(token, kRoutingId, true); PaintPreviewTracker tracker(token, kRoutingId, true);
tracker.AnnotateLink("www.google.com", gfx::Rect(1, 2, 3, 4)); tracker.AnnotateLink(GURL("www.google.com"), gfx::Rect(1, 2, 3, 4));
tracker.AnnotateLink("www.chromium.org", gfx::Rect(10, 20, 10, 20)); tracker.AnnotateLink(GURL("www.chromium.org"), gfx::Rect(10, 20, 10, 20));
tracker.CreateContentForRemoteFrame(gfx::Rect(1, 1, 1, 1), kRoutingId + 1); tracker.CreateContentForRemoteFrame(gfx::Rect(1, 1, 1, 1), kRoutingId + 1);
tracker.CreateContentForRemoteFrame(gfx::Rect(1, 2, 4, 8), kRoutingId + 2); tracker.CreateContentForRemoteFrame(gfx::Rect(1, 2, 4, 8), kRoutingId + 2);
base::ReadOnlySharedMemoryRegion region; auto response = mojom::PaintPreviewCaptureResponse::New();
EXPECT_TRUE(BuildAndSerializeProto(&tracker, &region)); BuildResponse(&tracker, response.get());
PaintPreviewFrameProto proto;
EXPECT_TRUE(region.IsValid()); EXPECT_EQ(static_cast<int>(response->id), kRoutingId);
auto mapping = region.Map(); EXPECT_EQ(response->links.size(), 2U);
EXPECT_TRUE(mapping.IsValid()); EXPECT_EQ(response->links.size(), tracker.GetLinks().size());
EXPECT_TRUE(proto.ParseFromArray(mapping.memory(), mapping.size())); for (size_t i = 0; i < response->links.size(); ++i) {
EXPECT_THAT(response->links[i]->url, tracker.GetLinks()[i].url);
EXPECT_TRUE(proto.is_main_frame()); EXPECT_THAT(response->links[i]->rect, tracker.GetLinks()[i].rect);
EXPECT_EQ(static_cast<int>(proto.id()), kRoutingId); }
EXPECT_EQ(proto.unguessable_token_low(),
tracker.Guid().GetLowForSerialization());
EXPECT_EQ(proto.unguessable_token_high(),
tracker.Guid().GetHighForSerialization());
EXPECT_EQ(proto.links_size(), 2);
EXPECT_EQ(static_cast<size_t>(proto.links_size()), tracker.GetLinks().size());
for (int i = 0; i < proto.links_size(); ++i)
EXPECT_THAT(proto.links(i), EqualsProto(tracker.GetLinks()[i]));
auto* content_map = tracker.GetPictureSerializationContext(); auto* content_map = tracker.GetPictureSerializationContext();
for (const auto& id_pair : proto.content_id_proxy_id_map()) { for (const auto& id_pair : response->content_id_proxy_id_map) {
auto it = content_map->find(id_pair.first); auto it = content_map->find(id_pair.first);
EXPECT_NE(it, content_map->end()); EXPECT_NE(it, content_map->end());
EXPECT_EQ(id_pair.first, it->first); EXPECT_EQ(id_pair.first, it->first);
......
...@@ -1374,7 +1374,7 @@ void GraphicsContext::SetURLForRect(const KURL& link, ...@@ -1374,7 +1374,7 @@ void GraphicsContext::SetURLForRect(const KURL& link,
// Intercept URL rects when painting previews. // Intercept URL rects when painting previews.
if (IsPaintingPreview() && tracker_) { if (IsPaintingPreview() && tracker_) {
tracker_->AnnotateLink(link.GetString().Utf8(), dest_rect); tracker_->AnnotateLink(GURL(link), dest_rect);
return; return;
} }
...@@ -1391,7 +1391,7 @@ void GraphicsContext::SetURLFragmentForRect(const String& dest_name, ...@@ -1391,7 +1391,7 @@ void GraphicsContext::SetURLFragmentForRect(const String& dest_name,
// Intercept URL rects when painting previews. // Intercept URL rects when painting previews.
if (IsPaintingPreview() && tracker_) { if (IsPaintingPreview() && tracker_) {
tracker_->AnnotateLink(dest_name.Utf8(), rect); tracker_->AnnotateLink(GURL(dest_name.Utf8()), rect);
return; return;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment