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

Issue 659021 link

Starred by 2 users
Status: Fixed
Owner:
ksakamoto@chromium.org
Closed: Aug 8
Cc:
█ l...@chromium.org
kouhei@chromium.org
█ nednguyen@chromium.org
mythria@chromium.org
tdres...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 632081



Sign in to add a comment

FirstMeaningfulPaint can be reported too soon on CPU-heavy pages

Project Member Reported by ksakamoto@chromium.org, Oct 25 2016 
WPT result: https://www.webpagetest.org/video/compare.php?tests=161025_TS_AW3-r:1-c:0
Trace file: https://www.webpagetest.org/getgzip.php?test=161025_TS_AW3&file=1_trace.json

FirstMeaningfulPaint is reported at 4.1sec, where only a spinner icon is painted. This is even before the FirstContentfulPaint (7.4sec).

WPT's network timeline shows that there is a large gap in the network activity, around 4sec ~ 6sec. CPU is busy executing JS during this period (look at the bottom of the timeline).

FirstMeaningfulPaintDetector thinks this 2 seconds network idle as load completion, and reports a spinner-only paint as first meaningful paint.
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 15 2016 

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

commit 2b891d60b92a6066fa1620794e244ef6e0ce65e2
Author: ksakamoto <ksakamoto@chromium.org>
Date: Thu Dec 15 09:25:57 2016

[FirstMeaningfulPaint] Do not stop observing layouts before FirstContentfulPaint

This should mitigate a bug that FirstMeaningfulPaint is reported too
soon (or not reported) on some CPU-heavy pages, because network idle is
detected while CPU is busy running scripts.

BUG= 659021 , 674228 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2581633002
Cr-Commit-Position: refs/heads/master@{#438791}

[modify] https://crrev.com/2b891d60b92a6066fa1620794e244ef6e0ce65e2/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.cpp
[modify] https://crrev.com/2b891d60b92a6066fa1620794e244ef6e0ce65e2/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetectorTest.cpp

Comment 2 by rmcilroy@chromium.org, Mar 17 2017

Cc: mythria@chromium.org
+Mythri who was seeing this in RCS benchmarks.
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 25 2017 

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

commit 333d8da3bd696fdcf4b7a2051ff7d586db8ee47c
Author: Pavel Feldman <pfeldman@chromium.org>
Date: Fri Aug 25 20:48:55 2017

Performance monitor: prepare to the new network idle computation that does not treat task time as idle.

Bug:  659021 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I6fbd91fe651212c5553462ecfd69835ede370ba6
Reviewed-on: https://chromium-review.googlesource.com/634328
Commit-Queue: Pavel Feldman <pfeldman@chromium.org>
Reviewed-by: lpy <lpy@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497529}
[modify] https://crrev.com/333d8da3bd696fdcf4b7a2051ff7d586db8ee47c/third_party/WebKit/LayoutTests/http/tests/inspector-protocol/page/page-lifecycleEvents.js
[modify] https://crrev.com/333d8da3bd696fdcf4b7a2051ff7d586db8ee47c/third_party/WebKit/Source/core/frame/PerformanceMonitor.cpp
[modify] https://crrev.com/333d8da3bd696fdcf4b7a2051ff7d586db8ee47c/third_party/WebKit/Source/core/frame/PerformanceMonitor.h
[modify] https://crrev.com/333d8da3bd696fdcf4b7a2051ff7d586db8ee47c/third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.cpp
[modify] https://crrev.com/333d8da3bd696fdcf4b7a2051ff7d586db8ee47c/third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.h
[modify] https://crrev.com/333d8da3bd696fdcf4b7a2051ff7d586db8ee47c/third_party/WebKit/Source/core/inspector/InspectorPageAgent.cpp
[modify] https://crrev.com/333d8da3bd696fdcf4b7a2051ff7d586db8ee47c/third_party/WebKit/Source/core/inspector/InspectorTraceEvents.cpp
[modify] https://crrev.com/333d8da3bd696fdcf4b7a2051ff7d586db8ee47c/third_party/WebKit/Source/core/inspector/InspectorTraceEvents.h
[modify] https://crrev.com/333d8da3bd696fdcf4b7a2051ff7d586db8ee47c/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/333d8da3bd696fdcf4b7a2051ff7d586db8ee47c/third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp
[modify] https://crrev.com/333d8da3bd696fdcf4b7a2051ff7d586db8ee47c/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.cpp
[modify] https://crrev.com/333d8da3bd696fdcf4b7a2051ff7d586db8ee47c/third_party/WebKit/Source/core/probe/CoreProbes.json5
[modify] https://crrev.com/333d8da3bd696fdcf4b7a2051ff7d586db8ee47c/third_party/WebKit/Source/core/probe/CoreProbes.pidl
[modify] https://crrev.com/333d8da3bd696fdcf4b7a2051ff7d586db8ee47c/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
[modify] https://crrev.com/333d8da3bd696fdcf4b7a2051ff7d586db8ee47c/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.h
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 1 2017 

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

commit aef4a308e4c9a9f783519bb34e9ae09bd983c01c
Author: Alex Moshchuk <alexmos@chromium.org>
Date: Fri Sep 01 21:15:18 2017

Fix flakiness issues with http/tests/inspector-protocol/page/page-lifecycleEvents.js

Bug:  659021 
Change-Id: Iaf0f891c3b05042cb122b3a75a97b991c34a5931
Reviewed-on: https://chromium-review.googlesource.com/647387
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499298}
[modify] https://crrev.com/aef4a308e4c9a9f783519bb34e9ae09bd983c01c/third_party/WebKit/LayoutTests/http/tests/inspector-protocol/page/page-lifecycleEvents.js

Comment 5 by tdres...@chromium.org, Oct 3 2017

Components: Speed>Metrics
Is this still a problem?

Comment 6 by npm@chromium.org, Jan 12 2018 

Ping - kasamoto@ is FMP still being reported too soon in your experiment?

Comment 7 by ksakamoto@chromium.org, Jan 15 2018

Cc: l...@chromium.org
Yes FirstMeaningfulPaintDetector still has this problem.

IIUC, IdlenessDetector (WebKit/Source/core/loader/IdlenessDetector.h) excludes task processing time from its "network almost idle" signal calculation, so we could plumb that signal to FirstMeaningfulPaintDetector to fix this plobem.

lpy@, does that sound correct?

Comment 8 by l...@chromium.org, Jan 15 2018 

yes, we exclude task processing time.

Comment 9 by dproy@chromium.org, May 9 2018 

Any update on this?

Comment 10 by ksakamoto@chromium.org, Aug 2

Status: Started (was: Assigned)
Created a CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1159934
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 7 

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

commit 208565993af870c86cff06c27dbc67066694bf61
Author: Kunihiko Sakamoto <ksakamoto@chromium.org>
Date: Tue Aug 07 03:15:17 2018

FMPDetector: Use network quiet signal from IdlenessDetector

This patch removes the network-{0,2}-quiet detection logic from
FirstMeaningfulPaintDetector. After this patch, these signals are
notified from IdlenessDetector.

IdlenessDetector excludes task processing time from the network-quiet
time window, so this would fix the issue that FirstMeaningfulPaint is
reported too soon on CPU-heavy pages.

This patch also renames "fmp-network-quiet-timeout" command-line switch
to "network-quiet-timeout". It now controls the network-quiet window of
IdlenessDetector.

Bug:  659021 , 850403 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I008762a128f2d6243ed50af316a97350bccbea5a
Reviewed-on: https://chromium-review.googlesource.com/1159934
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Reviewed-by: Gyuyoung Kim <gyuyoung.kim@lge.com>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581115}
[modify] https://crrev.com/208565993af870c86cff06c27dbc67066694bf61/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/208565993af870c86cff06c27dbc67066694bf61/content/public/common/content_switches.cc
[modify] https://crrev.com/208565993af870c86cff06c27dbc67066694bf61/content/public/common/content_switches.h
[modify] https://crrev.com/208565993af870c86cff06c27dbc67066694bf61/content/renderer/render_view_impl.cc
[modify] https://crrev.com/208565993af870c86cff06c27dbc67066694bf61/headless/test/data/protocol/emulation/compositor-basic-raf-expected.txt
[modify] https://crrev.com/208565993af870c86cff06c27dbc67066694bf61/third_party/blink/public/web/web_settings.h
[modify] https://crrev.com/208565993af870c86cff06c27dbc67066694bf61/third_party/blink/renderer/core/dom/document.cc
[modify] https://crrev.com/208565993af870c86cff06c27dbc67066694bf61/third_party/blink/renderer/core/exported/web_settings_impl.cc
[modify] https://crrev.com/208565993af870c86cff06c27dbc67066694bf61/third_party/blink/renderer/core/exported/web_settings_impl.h
[modify] https://crrev.com/208565993af870c86cff06c27dbc67066694bf61/third_party/blink/renderer/core/frame/settings.json5
[modify] https://crrev.com/208565993af870c86cff06c27dbc67066694bf61/third_party/blink/renderer/core/loader/frame_fetch_context.cc
[modify] https://crrev.com/208565993af870c86cff06c27dbc67066694bf61/third_party/blink/renderer/core/loader/idleness_detector.cc
[modify] https://crrev.com/208565993af870c86cff06c27dbc67066694bf61/third_party/blink/renderer/core/loader/idleness_detector.h
[modify] https://crrev.com/208565993af870c86cff06c27dbc67066694bf61/third_party/blink/renderer/core/paint/first_meaningful_paint_detector.cc
[modify] https://crrev.com/208565993af870c86cff06c27dbc67066694bf61/third_party/blink/renderer/core/paint/first_meaningful_paint_detector.h
[modify] https://crrev.com/208565993af870c86cff06c27dbc67066694bf61/third_party/blink/renderer/core/paint/first_meaningful_paint_detector_test.cc
[modify] https://crrev.com/208565993af870c86cff06c27dbc67066694bf61/third_party/blink/renderer/core/paint/paint_timing.cc

Comment 12 by ksakamoto@chromium.org, Aug 8

Status: Fixed (was: Started)
Project Member

Comment 13 by bugdroid1@chromium.org, Sep 26 

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

commit 28b16872181066d84bf26e0610a8c8f2bfa5c9c5
Author: Gyuyoung Kim <gyuyoung.kim@lge.com>
Date: Wed Sep 26 00:14:20 2018

FMPDetector: Remove unused timout constants

kNetwork0QuietWindowTimeout and kNetwork2QuietWindowTimeout constants have not used
anywhere since "FMPDetector: Use network quiet signal from IdlenessDetector" commit.

This CL removes them and all uses in FirstMeaningfulPaintDetector.

Bug:  659021 , 850403 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I48c3349d208818ceb8d3cde18fdd7177e310b788
Reviewed-on: https://chromium-review.googlesource.com/1237896
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Gyuyoung Kim <gyuyoung.kim@lge.com>
Cr-Commit-Position: refs/heads/master@{#594154}
[modify] https://crrev.com/28b16872181066d84bf26e0610a8c8f2bfa5c9c5/third_party/blink/renderer/core/paint/first_meaningful_paint_detector.cc
[modify] https://crrev.com/28b16872181066d84bf26e0610a8c8f2bfa5c9c5/third_party/blink/renderer/core/paint/first_meaningful_paint_detector.h
[modify] https://crrev.com/28b16872181066d84bf26e0610a8c8f2bfa5c9c5/third_party/blink/renderer/core/paint/first_meaningful_paint_detector_test.cc
►

Sign in to add a comment