New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 915700 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 17
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

[FCP++] Send size info to the browser

Project Member Reported by maxlg@chromium.org, Dec 17

Issue description

According to discussion in https://docs.google.com/document/d/1yEm8YrFsWpl9R4jWTpWDz24VRnk9RuLRadUWnSgJJLM/edit?ts=5c13e9a0#, we need to pass image/text size to PageLoadMetrics. The is in addition to the timestamps we have already passed. UMA and UKM need to use size information to tell whether a metric result fires at a time when large image is not loaded yet.
 
Status: Started (was: Available)
Description: Show this description
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d21c49585f262e851e2984f96f52905782706325

commit d21c49585f262e851e2984f96f52905782706325
Author: Liquan(Max) Gu <maxlg@chromium.org>
Date: Mon Jan 07 22:04:32 2019

[FCP++] TextPaint: report size to Page Load Metrics

Currently we are only reporting time of Largest/Last Text Paint to the Page
Load Metrics. We need to report size as well. This is mainly because of Largest
Content Paint. LCP requires size to decide the largest content, before
reporting the paint time of the largest content. As well, the size info
will make it possible to merge FCP++ based on the size of contents from
iframes.

This CL plumbs the size from TextPaintTimingDetector to PageLoadMetrics.

The change for the image part lives in here:
https://chromium-review.googlesource.com/c/chromium/src/+/1380193

Bug:  915700 
Change-Id: I377326c67f909176bca3c2db6881eabf4f614412
Reviewed-on: https://chromium-review.googlesource.com/c/1396597
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620496}
[modify] https://crrev.com/d21c49585f262e851e2984f96f52905782706325/chrome/common/page_load_metrics/page_load_metrics.mojom
[modify] https://crrev.com/d21c49585f262e851e2984f96f52905782706325/chrome/renderer/page_load_metrics/metrics_render_frame_observer.cc
[modify] https://crrev.com/d21c49585f262e851e2984f96f52905782706325/third_party/blink/public/web/web_performance.h
[modify] https://crrev.com/d21c49585f262e851e2984f96f52905782706325/third_party/blink/renderer/core/exported/web_performance.cc
[modify] https://crrev.com/d21c49585f262e851e2984f96f52905782706325/third_party/blink/renderer/core/paint/text_paint_timing_detector.cc
[modify] https://crrev.com/d21c49585f262e851e2984f96f52905782706325/third_party/blink/renderer/core/paint/text_paint_timing_detector.h
[modify] https://crrev.com/d21c49585f262e851e2984f96f52905782706325/third_party/blink/renderer/core/timing/performance_timing.cc
[modify] https://crrev.com/d21c49585f262e851e2984f96f52905782706325/third_party/blink/renderer/core/timing/performance_timing.h

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/58d5515ec3ca1a34ce284844e927199c40eadc10

commit 58d5515ec3ca1a34ce284844e927199c40eadc10
Author: Liquan(Max) Gu <maxlg@chromium.org>
Date: Tue Jan 08 03:09:08 2019

[FCP++] ImagePaint: report size to Page Load Metrics

FCP++ currently only sends timestamps to PageLoadMetrics in the browser process.
We need to pass size information along with timestamps. Doing this, UMA and UKM
can figure out whether FCP++ results point to a large or small image.

This CL is about:
1. Refactor ImagePaintTimingDetector to buffer image record other than time as
the metric result.
2. Plumb size information from the detector to PageLoadMetrics.

This CL starts the works from Largest & Last ImagePaint. Following CLs will do
the same change for Largest & Last TextPaint (https://chromium-review.googlesource.com/c/chromium/src/+/1396597)

Bug:  915700 
Change-Id: Ie88a71c932eec6c6c85ab93c9e0566e0e1643be4
Reviewed-on: https://chromium-review.googlesource.com/c/1380193
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620595}
[modify] https://crrev.com/58d5515ec3ca1a34ce284844e927199c40eadc10/chrome/common/page_load_metrics/page_load_metrics.mojom
[modify] https://crrev.com/58d5515ec3ca1a34ce284844e927199c40eadc10/chrome/renderer/page_load_metrics/metrics_render_frame_observer.cc
[modify] https://crrev.com/58d5515ec3ca1a34ce284844e927199c40eadc10/third_party/blink/public/web/web_performance.h
[modify] https://crrev.com/58d5515ec3ca1a34ce284844e927199c40eadc10/third_party/blink/renderer/core/exported/web_performance.cc
[modify] https://crrev.com/58d5515ec3ca1a34ce284844e927199c40eadc10/third_party/blink/renderer/core/paint/image_paint_timing_detector.cc
[modify] https://crrev.com/58d5515ec3ca1a34ce284844e927199c40eadc10/third_party/blink/renderer/core/paint/image_paint_timing_detector.h
[modify] https://crrev.com/58d5515ec3ca1a34ce284844e927199c40eadc10/third_party/blink/renderer/core/paint/image_paint_timing_detector_test.cc
[modify] https://crrev.com/58d5515ec3ca1a34ce284844e927199c40eadc10/third_party/blink/renderer/core/timing/performance_timing.cc
[modify] https://crrev.com/58d5515ec3ca1a34ce284844e927199c40eadc10/third_party/blink/renderer/core/timing/performance_timing.h

Comment 5 by maxlg@chromium.org, Jan 17 (5 days ago)

Status: Fixed (was: Started)

Sign in to add a comment