Commit 9f3f4689 authored by ckitagawa's avatar ckitagawa Committed by Commit Bot

[Paint Preview] Add performance UKM

Adds UKM support for reporting the capture time of the main frame (in
ms) for a paint preview. Support for memory overhead and on-disk size
may be added to this dataset in future, but is out-of-scope for this CL
as getting reliable memory metrics is non-trivial and on-disk size is
handled on a per-implementation basis.

Bug: 1038390
Change-Id: Iaaf77b3722633773455babd3e7e51cd95f713577
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1988222
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Reviewed-by: default avatarMichael Thiessen <mthiesse@chromium.org>
Reviewed-by: default avatarKen Buchanan <kenrb@chromium.org>
Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738638}
parent 2570f733
...@@ -16,10 +16,12 @@ ...@@ -16,10 +16,12 @@
#include "components/paint_preview/browser/paint_preview_client.h" #include "components/paint_preview/browser/paint_preview_client.h"
#include "components/paint_preview/common/file_stream.h" #include "components/paint_preview/common/file_stream.h"
#include "components/paint_preview/common/serial_utils.h" #include "components/paint_preview/common/serial_utils.h"
#include "components/ukm/test_ukm_recorder.h"
#include "content/public/browser/notification_types.h" #include "content/public/browser/notification_types.h"
#include "content/public/common/content_switches.h" #include "content/public/common/content_switches.h"
#include "net/dns/mock_host_resolver.h" #include "net/dns/mock_host_resolver.h"
#include "net/test/embedded_test_server/embedded_test_server.h" #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/SkPicture.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -88,6 +90,7 @@ class PaintPreviewBrowserTest : public InProcessBrowserTest { ...@@ -88,6 +90,7 @@ class PaintPreviewBrowserTest : public InProcessBrowserTest {
IN_PROC_BROWSER_TEST_F(PaintPreviewBrowserTest, CaptureFrame) { IN_PROC_BROWSER_TEST_F(PaintPreviewBrowserTest, CaptureFrame) {
LoadPage(http_server_.GetURL("a.com", "/cross_site_iframe_factory.html?a")); LoadPage(http_server_.GetURL("a.com", "/cross_site_iframe_factory.html?a"));
ukm::TestAutoSetUkmRecorder ukm_recorder;
base::UnguessableToken guid = base::UnguessableToken::Create(); base::UnguessableToken guid = base::UnguessableToken::Create();
PaintPreviewClient::PaintPreviewParams params; PaintPreviewClient::PaintPreviewParams params;
...@@ -134,6 +137,10 @@ IN_PROC_BROWSER_TEST_F(PaintPreviewBrowserTest, CaptureFrame) { ...@@ -134,6 +137,10 @@ IN_PROC_BROWSER_TEST_F(PaintPreviewBrowserTest, CaptureFrame) {
}, },
loop.QuitClosure(), guid)); loop.QuitClosure(), guid));
loop.Run(); loop.Run();
auto entries = ukm_recorder.GetEntriesByName(
ukm::builders::PaintPreviewCapture::kEntryName);
EXPECT_EQ(1u, entries.size());
} }
IN_PROC_BROWSER_TEST_F(PaintPreviewBrowserTest, CaptureMainFrameWithSubframe) { IN_PROC_BROWSER_TEST_F(PaintPreviewBrowserTest, CaptureMainFrameWithSubframe) {
......
...@@ -31,9 +31,12 @@ source_set("browser") { ...@@ -31,9 +31,12 @@ source_set("browser") {
"//components/discardable_memory/service", "//components/discardable_memory/service",
"//components/keyed_service/core", "//components/keyed_service/core",
"//components/strings:components_strings_grit", "//components/strings:components_strings_grit",
"//components/ukm/content",
"//content/public/browser", "//content/public/browser",
"//mojo/public/cpp/base", "//mojo/public/cpp/base",
"//mojo/public/cpp/bindings", "//mojo/public/cpp/bindings",
"//services/metrics/public/cpp:metrics_cpp",
"//services/metrics/public/cpp:ukm_builders",
"//third_party/blink/public:blink_headers", "//third_party/blink/public:blink_headers",
"//third_party/blink/public/common", "//third_party/blink/public/common",
"//third_party/zlib/google:zip", "//third_party/zlib/google:zip",
......
include_rules = [ include_rules = [
"+cc/base", "+cc/base",
"+components/discardable_memory/service", "+components/discardable_memory/service",
"+components/keyed_service/core",
"+components/services/paint_preview_compositor/public/mojom", "+components/services/paint_preview_compositor/public/mojom",
"+components/strings/grit/components_strings.h", "+components/strings/grit/components_strings.h",
"+components/keyed_service/core", "+components/ukm/content",
"+content/public/browser", "+content/public/browser",
"+content/public/test", "+content/public/test",
"+services/metrics/public/cpp",
"+third_party/blink/public/common", "+third_party/blink/public/common",
"+third_party/zlib/google", "+third_party/zlib/google",
] ]
...@@ -11,10 +11,15 @@ ...@@ -11,10 +11,15 @@
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/task/post_task.h" #include "base/task/post_task.h"
#include "base/threading/scoped_blocking_call.h" #include "base/threading/scoped_blocking_call.h"
#include "components/ukm/content/source_url_recorder.h"
#include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_frame_host.h"
#include "content/public/browser/render_process_host.h" #include "content/public/browser/render_process_host.h"
#include "content/public/browser/web_contents.h"
#include "services/metrics/public/cpp/metrics_utils.h"
#include "services/metrics/public/cpp/ukm_builders.h"
#include "services/metrics/public/cpp/ukm_recorder.h"
#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h" #include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
namespace paint_preview { namespace paint_preview {
...@@ -69,16 +74,32 @@ PaintPreviewCaptureResponseToPaintPreviewFrameProto( ...@@ -69,16 +74,32 @@ PaintPreviewCaptureResponseToPaintPreviewFrameProto(
return frame_guids; return frame_guids;
} }
// Records UKM data for the capture.
// TODO(crbug/1038390): Add more metrics;
// - Peak memory during capture (bucketized).
// - Compressed on disk size (bucketized).
void RecordUkmCaptureData(ukm::SourceId source_id,
base::TimeDelta blink_recording_time) {
if (source_id == ukm::kInvalidSourceId)
return;
ukm::builders::PaintPreviewCapture(source_id)
.SetBlinkCaptureTime(blink_recording_time.InMilliseconds())
.Record(ukm::UkmRecorder::Get());
}
} // namespace } // namespace
PaintPreviewClient::PaintPreviewParams::PaintPreviewParams() = default; PaintPreviewClient::PaintPreviewParams::PaintPreviewParams() = default;
PaintPreviewClient::PaintPreviewParams::~PaintPreviewParams() = default; PaintPreviewClient::PaintPreviewParams::~PaintPreviewParams() = default;
PaintPreviewClient::PaintPreviewData::PaintPreviewData() = default; PaintPreviewClient::PaintPreviewData::PaintPreviewData() = default;
PaintPreviewClient::PaintPreviewData::~PaintPreviewData() = default; PaintPreviewClient::PaintPreviewData::~PaintPreviewData() = default;
PaintPreviewClient::PaintPreviewData& PaintPreviewClient::PaintPreviewData:: PaintPreviewClient::PaintPreviewData& PaintPreviewClient::PaintPreviewData::
operator=(PaintPreviewData&& rhs) noexcept = default; operator=(PaintPreviewData&& rhs) noexcept = default;
PaintPreviewClient::PaintPreviewData::PaintPreviewData( PaintPreviewClient::PaintPreviewData::PaintPreviewData(
PaintPreviewData&& other) noexcept = default; PaintPreviewData&& other) noexcept = default;
...@@ -89,11 +110,13 @@ PaintPreviewClient::CreateResult::CreateResult(base::File file, ...@@ -89,11 +110,13 @@ PaintPreviewClient::CreateResult::CreateResult(base::File file,
PaintPreviewClient::CreateResult::~CreateResult() = default; PaintPreviewClient::CreateResult::~CreateResult() = default;
PaintPreviewClient::CreateResult::CreateResult(CreateResult&& other) = default; PaintPreviewClient::CreateResult::CreateResult(CreateResult&& other) = default;
PaintPreviewClient::CreateResult& PaintPreviewClient::CreateResult::operator=( PaintPreviewClient::CreateResult& PaintPreviewClient::CreateResult::operator=(
CreateResult&& other) = default; CreateResult&& other) = default;
PaintPreviewClient::PaintPreviewClient(content::WebContents* web_contents) PaintPreviewClient::PaintPreviewClient(content::WebContents* web_contents)
: content::WebContentsObserver(web_contents) {} : content::WebContentsObserver(web_contents) {}
PaintPreviewClient::~PaintPreviewClient() = default; PaintPreviewClient::~PaintPreviewClient() = default;
void PaintPreviewClient::CapturePaintPreview( void PaintPreviewClient::CapturePaintPreview(
...@@ -105,11 +128,13 @@ void PaintPreviewClient::CapturePaintPreview( ...@@ -105,11 +128,13 @@ void PaintPreviewClient::CapturePaintPreview(
mojom::PaintPreviewStatus::kGuidCollision, nullptr); mojom::PaintPreviewStatus::kGuidCollision, nullptr);
return; return;
} }
all_document_data_.insert({params.document_guid, PaintPreviewData()}); PaintPreviewData document_data;
auto* document_data = &all_document_data_[params.document_guid]; document_data.root_dir = params.root_dir;
document_data->root_dir = params.root_dir; document_data.callback = std::move(callback);
document_data->callback = std::move(callback); document_data.root_url = render_frame_host->GetLastCommittedURL();
document_data->root_url = render_frame_host->GetLastCommittedURL(); document_data.source_id =
ukm::GetSourceIdForWebContentsDocument(web_contents());
all_document_data_.insert({params.document_guid, std::move(document_data)});
CapturePaintPreviewInternal(params, render_frame_host); CapturePaintPreviewInternal(params, render_frame_host);
} }
...@@ -309,16 +334,17 @@ mojom::PaintPreviewStatus PaintPreviewClient::RecordFrame( ...@@ -309,16 +334,17 @@ mojom::PaintPreviewStatus PaintPreviewClient::RecordFrame(
content::RenderFrameHost* render_frame_host, content::RenderFrameHost* render_frame_host,
mojom::PaintPreviewCaptureResponsePtr response) { mojom::PaintPreviewCaptureResponsePtr response) {
auto it = all_document_data_.find(guid); auto it = all_document_data_.find(guid);
DCHECK(it != all_document_data_.end());
if (!it->second.proto) { if (!it->second.proto) {
it->second.proto = std::make_unique<PaintPreviewProto>(); it->second.proto = std::make_unique<PaintPreviewProto>();
it->second.proto->mutable_metadata()->set_url( it->second.proto->mutable_metadata()->set_url(it->second.root_url.spec());
all_document_data_[guid].root_url.spec());
} }
PaintPreviewProto* proto_ptr = it->second.proto.get(); PaintPreviewProto* proto_ptr = it->second.proto.get();
PaintPreviewFrameProto* frame_proto; PaintPreviewFrameProto* frame_proto;
if (is_main_frame) { if (is_main_frame) {
it->second.main_frame_blink_recording_time = response->blink_recording_time;
frame_proto = proto_ptr->mutable_root_frame(); frame_proto = proto_ptr->mutable_root_frame();
frame_proto->set_is_main_frame(true); frame_proto->set_is_main_frame(true);
uint64_t old_style_id = MakeOldStyleId(render_frame_host); uint64_t old_style_id = MakeOldStyleId(render_frame_host);
...@@ -353,6 +379,9 @@ void PaintPreviewClient::OnFinished(base::UnguessableToken guid, ...@@ -353,6 +379,9 @@ void PaintPreviewClient::OnFinished(base::UnguessableToken guid,
"Browser.PaintPreview.Capture.NumberOfFramesCaptured", "Browser.PaintPreview.Capture.NumberOfFramesCaptured",
document_data->finished_subframes.size()); document_data->finished_subframes.size());
RecordUkmCaptureData(document_data->source_id,
document_data->main_frame_blink_recording_time);
// At a minimum one frame was captured successfully, it is up to the // At a minimum one frame was captured successfully, it is up to the
// caller to decide if a partial success is acceptable based on what is // caller to decide if a partial success is acceptable based on what is
// contained in the proto. // contained in the proto.
......
...@@ -96,6 +96,12 @@ class PaintPreviewClient ...@@ -96,6 +96,12 @@ class PaintPreviewClient
// URL of the root frame. // URL of the root frame.
GURL root_url; GURL root_url;
// UKM Source ID of the WebContent.
ukm::SourceId source_id;
// Main frame capture time.
base::TimeDelta main_frame_blink_recording_time;
// Callback that is invoked on completion of data. // Callback that is invoked on completion of data.
PaintPreviewCallback callback; PaintPreviewCallback callback;
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
module paint_preview.mojom; module paint_preview.mojom;
import "mojo/public/mojom/base/file.mojom"; import "mojo/public/mojom/base/file.mojom";
import "mojo/public/mojom/base/time.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"; import "url/mojom/url.mojom";
...@@ -68,6 +69,9 @@ struct PaintPreviewCaptureResponse { ...@@ -68,6 +69,9 @@ struct PaintPreviewCaptureResponse {
// A list of links within the frame. // A list of links within the frame.
array<LinkData> links; array<LinkData> links;
// The time spent capturing in Blink.
mojo_base.mojom.TimeDelta blink_recording_time;
}; };
// Service for capturing a paint preview of a RenderFrame's contents. This // Service for capturing a paint preview of a RenderFrame's contents. This
......
...@@ -122,6 +122,7 @@ void PaintPreviewRecorderImpl::CapturePaintPreviewInternal( ...@@ -122,6 +122,7 @@ void PaintPreviewRecorderImpl::CapturePaintPreviewInternal(
base::TimeTicks start_time = base::TimeTicks::Now(); base::TimeTicks start_time = base::TimeTicks::Now();
bool success = frame->CapturePaintPreview(bounds, canvas); bool success = frame->CapturePaintPreview(bounds, canvas);
base::TimeDelta capture_time = base::TimeTicks::Now() - start_time; base::TimeDelta capture_time = base::TimeTicks::Now() - start_time;
response->blink_recording_time = capture_time;
if (is_main_frame_) { if (is_main_frame_) {
base::UmaHistogramBoolean("Renderer.PaintPreview.Capture.MainFrameSuccess", base::UmaHistogramBoolean("Renderer.PaintPreview.Capture.MainFrameSuccess",
......
...@@ -7314,6 +7314,22 @@ be describing additional metrics about the same event. ...@@ -7314,6 +7314,22 @@ be describing additional metrics about the same event.
</metric> </metric>
</event> </event>
<event name="PaintPreviewCapture">
<owner>ckitagawa@chromium.org</owner>
<owner>fredmello@chromium.org</owner>
<owner>mahmoudi@chromium.org</owner>
<owner>vollick@chromium.org</owner>
<summary>
Metrics related to the performance of capturing Paint Previews.
</summary>
<metric name="BlinkCaptureTime">
<summary>
The amount of time spent painting in Blink to record a Paint Preview of
the main frame.
</summary>
</metric>
</event>
<event name="PasswordForm"> <event name="PasswordForm">
<owner>battre@chromium.org</owner> <owner>battre@chromium.org</owner>
<summary> <summary>
......
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