Loosen FirstMeaningfulPaint UMA network-idle heuristics

Loosen FirstMeaningfulPaint UMA network-idle heuristics

Currently, FirstMeaningfulPaint (FMP) UMA is logged when page's network
activity is "0-quiet" (no network requests) for 0.5 seconds. But about
23% of page loads are aborted before reaching that condition.

To improve the coverage of FMP UMA, this patch changes the network idle
heuristic to "2-quiet" (no more than 2 network requests) for 3 seconds
window.

To verify this change doesn't make FMP reported too soon,
FirstMeaningfulPaintDetector continues to observe layouts until network
0-quiet, and report whether the last FMP candidate was after network
2-quiet or not.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2711893002
Cr-Commit-Position: refs/heads/master@{#457017}
Committed: https://chromium.googlesource.com/chromium/src/+/4d059dcf25a256d591234eb2a7ffd1673f7c9709

[Kunihiko Sakamoto, 2017-02-23 07:37:32]

Description was changed from

==========
WIP
==========

to

==========
WIP
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Kunihiko Sakamoto, 2017-02-24 02:12:41]

The CQ bit was checked by ksakamoto@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-24 02:13:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-24 02:17:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-24 02:17:03]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Kunihiko Sakamoto, 2017-02-24 03:23:00]

The CQ bit was checked by ksakamoto@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-24 03:23:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-24 04:34:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-24 04:34:55]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Kunihiko Sakamoto, 2017-02-24 05:56:22]

The CQ bit was checked by ksakamoto@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-24 05:56:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Kunihiko Sakamoto, 2017-02-24 06:30:07]

Description was changed from

==========
WIP
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Report FirstMeaningfulPaint at network 2-quiet

Currently, FirstMeaningfulPaint (FMP) UMA is logged when page's network
activity is "0-quiet" (no network requests) for 0.5 seconds. But about
23% of page loads are aborted before reaching that condition.

To improve the coverage of FMP UMA, this patch changes the network idle
heuristic to "2-quiet" (no more than 2 network requests) for 3 seconds
window.

To verify this change doesn't make FMP reported too soon,
FirstMeaningfulPaintDetector continues to observe layouts until network
0-quiet, and report whether the last FMP candidate was after network
2-quiet or not.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Kunihiko Sakamoto, 2017-02-24 06:34:52]

Description was changed from

==========
Report FirstMeaningfulPaint at network 2-quiet

Currently, FirstMeaningfulPaint (FMP) UMA is logged when page's network
activity is "0-quiet" (no network requests) for 0.5 seconds. But about
23% of page loads are aborted before reaching that condition.

To improve the coverage of FMP UMA, this patch changes the network idle
heuristic to "2-quiet" (no more than 2 network requests) for 3 seconds
window.

To verify this change doesn't make FMP reported too soon,
FirstMeaningfulPaintDetector continues to observe layouts until network
0-quiet, and report whether the last FMP candidate was after network
2-quiet or not.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Loosen FirstMeaningfulPaint UMA network-idle heuristics

Currently, FirstMeaningfulPaint (FMP) UMA is logged when page's network
activity is "0-quiet" (no network requests) for 0.5 seconds. But about
23% of page loads are aborted before reaching that condition.

To improve the coverage of FMP UMA, this patch changes the network idle
heuristic to "2-quiet" (no more than 2 network requests) for 3 seconds
window.

To verify this change doesn't make FMP reported too soon,
FirstMeaningfulPaintDetector continues to observe layouts until network
0-quiet, and report whether the last FMP candidate was after network
2-quiet or not.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Kunihiko Sakamoto, 2017-02-24 07:29:19]

Patchset #1 (id:1) has been deleted

[Kunihiko Sakamoto, 2017-02-24 07:29:27]

Patchset #1 (id:20001) has been deleted

[Kunihiko Sakamoto, 2017-02-24 07:29:36]

Patchset #1 (id:40001) has been deleted

[Kunihiko Sakamoto, 2017-02-24 07:39:48]

ksakamoto@chromium.org changed reviewers:
+ kouhei@chromium.org

[commit-bot: I haz the power, 2017-02-24 08:54:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-24 08:54:25]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Charlie Harrison, 2017-02-24 13:07:59]

csharrison@chromium.org changed reviewers:
+ csharrison@chromium.org

[Charlie Harrison, 2017-02-24 13:07:59]

Drive-by, why the jump from 0 to 2 and not 1?

[dproy, 2017-02-24 15:25:25]

On 2017/02/24 13:07:59, Charlie Harrison wrote:
> Drive-by, why the jump from 0 to 2 and not 1?

This is to make it similar to the latest firstInteractive definition. We started
with 2 request forgiveness in firstInteractive, and so far this worked well in
our experiments. 

Having said that, we *could* try making it 1 request to see if that is superior,
but after looking at lots of page load traces over the last few weeks my guess
is this will push the time we log FMP further away without any significant gain
in accuracy. By the time a page has only two pending network requests, the page
has almost always already reached FMP.

[dproy, 2017-02-24 15:25:26]

On 2017/02/24 13:07:59, Charlie Harrison wrote:
> Drive-by, why the jump from 0 to 2 and not 1?

This is to make it similar to the latest firstInteractive definition. We started
with 2 request forgiveness in firstInteractive, and so far this worked well in
our experiments. 

Having said that, we *could* try making it 1 request to see if that is superior,
but after looking at lots of page load traces over the last few weeks my guess
is this will push the time we log FMP further away without any significant gain
in accuracy. By the time a page has only two pending network requests, the page
has almost always already reached FMP.

[Kunihiko Sakamoto, 2017-02-27 09:56:48]

On 2017/02/24 15:25:26, dproy wrote:
> On 2017/02/24 13:07:59, Charlie Harrison wrote:
> > Drive-by, why the jump from 0 to 2 and not 1?
> 
> This is to make it similar to the latest firstInteractive definition. We
started
> with 2 request forgiveness in firstInteractive, and so far this worked well in
> our experiments. 
> 
> Having said that, we *could* try making it 1 request to see if that is
superior,
> but after looking at lots of page load traces over the last few weeks my guess
> is this will push the time we log FMP further away without any significant
gain
> in accuracy. By the time a page has only two pending network requests, the
page
> has almost always already reached FMP.

Accuracy regression is tracked with NetworkStateAtFirstMeaningfulPaint UMA, so
we can tweak the number once we get initial UMA results.

[kouhei (in TOK), 2017-02-28 23:47:32]

As discussed offline yesterday. (ksakamoto@ can skip reading this)

I'm a bit worried that this can actually make coverage worse.
- This CL assumes that Quiet2 comes before Quiet0, but that is not necessarily
true.
- In the case where network activity quickly drops to 0 and the user quickly
navigated away (w/in 0.5-3sec), in previous mode we still have recorded FMP, but
the new heuristics wouldn't record it as we haven't reached 3sec window.

https://codereview.chromium.org/2711893002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.h
(right):

https://codereview.chromium.org/2711893002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.h:56: enum
NetworkState { NetworkActive, Network2Quiet, Network0Quiet };
Optional: enum class NetworkState { Active, Quiet2, Quiet0 };

[Kunihiko Sakamoto, 2017-03-01 03:50:36]

On 2017/02/28 23:47:32, kouhei wrote:
> As discussed offline yesterday. (ksakamoto@ can skip reading this)
> 
> I'm a bit worried that this can actually make coverage worse.
> - This CL assumes that Quiet2 comes before Quiet0, but that is not necessarily
> true.
> - In the case where network activity quickly drops to 0 and the user quickly
> navigated away (w/in 0.5-3sec), in previous mode we still have recorded FMP,
but
> the new heuristics wouldn't record it as we haven't reached 3sec window.
> 
>
https://codereview.chromium.org/2711893002/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.h
> (right):
> 
>
https://codereview.chromium.org/2711893002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.h:56: enum
> NetworkState { NetworkActive, Network2Quiet, Network0Quiet };
> Optional: enum class NetworkState { Active, Quiet2, Quiet0 };

Yeah, PatchSet1 was seeking the 0-quiet window from the point when 2-quiet is
detected, which is different from the "network stable" heuristic current FMP
uses. In order to understand the change correctly, 0-quiet should be searched
independently from 2-quiet. Since window sizes are different, 0-quiet may happen
before 2-quiet.

There are 4 cases where current FMP (FMP-0-quiet) and network-2-quiet based FMP
(FMP-2-quiet) are different:

a. only FMP-0-quiet exists (user navigated away before network 2-quiet)
b. only FMP-2-quiet exists (user navigated away before network 0-quiet)
c. FMP-2-quiet < FMP-0-quiet
d. FMP-0-quiet < FMP-2-quiet

Using the two UMA histograms added in PatchSet3, we would be able to count the
occurrences of each case.
PTAL, thanks!

[tdresser, 2017-03-01 13:33:07]

tdresser@chromium.org changed reviewers:
+ tdresser@chromium.org

[tdresser, 2017-03-01 13:33:08]

Good point.

We could test this using cluster telemetry without waiting for the metric to hit
UMA. Which approach ends earlier could be strongly dependent on connection speed
or other environmental factors though, so going the UMA route SGTM.

[Kunihiko Sakamoto, 2017-03-06 09:32:18]

Kouhei, PTAL when you have time. Thanks!


On 2017/03/01 13:33:08, tdresser wrote:
> We could test this using cluster telemetry without waiting for the metric to
hit
> UMA. Which approach ends earlier could be strongly dependent on connection
speed
> or other environmental factors though, so going the UMA route SGTM.

Also the coverage depends on at which point user navigates away from the page,
so I think cluster telemetry would give little insights here. I would like to go
the UMA route directly.

[tdresser, 2017-03-06 13:07:19]

On 2017/03/06 09:32:18, Kunihiko Sakamoto wrote:
> Kouhei, PTAL when you have time. Thanks!
> 
> 
> On 2017/03/01 13:33:08, tdresser wrote:
> > We could test this using cluster telemetry without waiting for the metric to
> hit
> > UMA. Which approach ends earlier could be strongly dependent on connection
> speed
> > or other environmental factors though, so going the UMA route SGTM.
> 
> Also the coverage depends on at which point user navigates away from the page,
> so I think cluster telemetry would give little insights here. I would like to
go
> the UMA route directly.

Good point.
SGTM.

[kouhei (in TOK), 2017-03-07 16:33:49]

lgtm sorry for the delay

[tdresser, 2017-03-07 17:15:51]

LGTM with nit.

https://codereview.chromium.org/2711893002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetectorTest.cpp
(right):

https://codereview.chromium.org/2711893002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetectorTest.cpp:43:
detector().network2QuietTimerFired(nullptr);
Should we have a test where we hit the two network quiet conditions at different
times?

[Charlie Harrison, 2017-03-07 17:54:07]

page_load_metrics RS LGTM

[Kunihiko Sakamoto, 2017-03-08 04:32:17]

The CQ bit was checked by ksakamoto@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-08 04:32:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-08 04:33:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-08 04:33:59]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Kunihiko Sakamoto, 2017-03-08 04:34:50]

ksakamoto@chromium.org changed reviewers:
+ isherman@chromium.org, tyoshino@chromium.org

[Kunihiko Sakamoto, 2017-03-08 04:34:51]

+tyoshino for platform/loader
+isherman for histograms.xml

https://codereview.chromium.org/2711893002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetectorTest.cpp
(right):

https://codereview.chromium.org/2711893002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetectorTest.cpp:43:
detector().network2QuietTimerFired(nullptr);
On 2017/03/07 17:15:51, tdresser wrote:
> Should we have a test where we hit the two network quiet conditions at
different
> times?

Done.

[Ilya Sherman, 2017-03-08 09:19:46]

https://codereview.chromium.org/2711893002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.cpp
(right):

https://codereview.chromium.org/2711893002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.cpp:172: enum
HadNetworkQuiet {
Please document that this enum should be treated as append-only.

https://codereview.chromium.org/2711893002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.cpp:181: enum
FMPOrderingEnum {
And this one too =)

https://codereview.chromium.org/2711893002/diff/120001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2711893002/diff/120001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:20286: +<histogram
name="FirstMeaningfulPaintDetector.FirstMeaningfulPaintOrdering"
Would it be appropriate to group these new histograms under the
"PageLoad.Experimental.PaintTiming.*" category?

https://codereview.chromium.org/2711893002/diff/120001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:20290: +    Whether the two variants of
First Meaningful Paint reported different value,
nit: s/value/values

https://codereview.chromium.org/2711893002/diff/120001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:20301: +    network connections for 2
seconds).
Would it be useful to also record when a page never reached either of these
states, as a baseline?

[Kunihiko Sakamoto, 2017-03-08 10:16:10]

https://codereview.chromium.org/2711893002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.cpp
(right):

https://codereview.chromium.org/2711893002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.cpp:172: enum
HadNetworkQuiet {
On 2017/03/08 09:19:46, Ilya Sherman wrote:
> Please document that this enum should be treated as append-only.

Done.

https://codereview.chromium.org/2711893002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.cpp:181: enum
FMPOrderingEnum {
On 2017/03/08 09:19:46, Ilya Sherman wrote:
> And this one too =)

Done.

https://codereview.chromium.org/2711893002/diff/120001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2711893002/diff/120001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:20286: +<histogram
name="FirstMeaningfulPaintDetector.FirstMeaningfulPaintOrdering"
On 2017/03/08 09:19:46, Ilya Sherman wrote:
> Would it be appropriate to group these new histograms under the
> "PageLoad.Experimental.PaintTiming.*" category?

I wanted to separate these from PageLoad.* metrics, that are aggregated and
filtered (excluding background tabs etc.) in browser process. These
FirstMeaningfulPaintDetector.* are logged from renderer process.

https://codereview.chromium.org/2711893002/diff/120001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:20290: +    Whether the two variants of
First Meaningful Paint reported different value,
On 2017/03/08 09:19:46, Ilya Sherman wrote:
> nit: s/value/values

Done.

https://codereview.chromium.org/2711893002/diff/120001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:20301: +    network connections for 2
seconds).
On 2017/03/08 09:19:46, Ilya Sherman wrote:
> Would it be useful to also record when a page never reached either of these
> states, as a baseline?

Unfortunately it's hard (if not impossible) to record it reliably in renderer,
since renderer can be killed any time. We record (# page loads that had 2-quiet
/ total page loads) in browser side PageLoad metrics, so not recording baseline
here would be okay.

[Ilya Sherman, 2017-03-08 22:46:46]

https://codereview.chromium.org/2711893002/diff/120001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2711893002/diff/120001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:20286: +<histogram
name="FirstMeaningfulPaintDetector.FirstMeaningfulPaintOrdering"
On 2017/03/08 10:16:10, Kunihiko Sakamoto wrote:
> On 2017/03/08 09:19:46, Ilya Sherman wrote:
> > Would it be appropriate to group these new histograms under the
> > "PageLoad.Experimental.PaintTiming.*" category?
> 
> I wanted to separate these from PageLoad.* metrics, that are aggregated and
> filtered (excluding background tabs etc.) in browser process. These
> FirstMeaningfulPaintDetector.* are logged from renderer process.

What about something like
"PageLoad.Experimental.Renderer.FirstMeaningfulPaintDetector.*"?  These are
still metrics related to page load timing, correct?  The top-level category
should reflect a fairly general concept -- "FirstMeaningfulPaintDetector" seems
to me to be overly specific.

https://codereview.chromium.org/2711893002/diff/120001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:20301: +    network connections for 2
seconds).
On 2017/03/08 10:16:10, Kunihiko Sakamoto wrote:
> On 2017/03/08 09:19:46, Ilya Sherman wrote:
> > Would it be useful to also record when a page never reached either of these
> > states, as a baseline?
> 
> Unfortunately it's hard (if not impossible) to record it reliably in renderer,
> since renderer can be killed any time. We record (# page loads that had
2-quiet
> / total page loads) in browser side PageLoad metrics, so not recording
baseline
> here would be okay.

Okay.  In that case, could you please include a reference, in this histogram
description, to the metric that you think is an appropriate baseline?

[Kunihiko Sakamoto, 2017-03-10 04:37:10]

https://codereview.chromium.org/2711893002/diff/120001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2711893002/diff/120001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:20286: +<histogram
name="FirstMeaningfulPaintDetector.FirstMeaningfulPaintOrdering"
On 2017/03/08 22:46:45, Ilya Sherman wrote:
> On 2017/03/08 10:16:10, Kunihiko Sakamoto wrote:
> > On 2017/03/08 09:19:46, Ilya Sherman wrote:
> > > Would it be appropriate to group these new histograms under the
> > > "PageLoad.Experimental.PaintTiming.*" category?
> > 
> > I wanted to separate these from PageLoad.* metrics, that are aggregated and
> > filtered (excluding background tabs etc.) in browser process. These
> > FirstMeaningfulPaintDetector.* are logged from renderer process.
> 
> What about something like
> "PageLoad.Experimental.Renderer.FirstMeaningfulPaintDetector.*"?  These are
> still metrics related to page load timing, correct?  The top-level category
> should reflect a fairly general concept -- "FirstMeaningfulPaintDetector"
seems
> to me to be overly specific.

Renamed to "PageLoad.Experimental.Renderer.FirstMeaningfulPaintDetector.*".
Thanks for the naming suggestion.

https://codereview.chromium.org/2711893002/diff/120001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:20301: +    network connections for 2
seconds).
On 2017/03/08 22:46:45, Ilya Sherman wrote:
> On 2017/03/08 10:16:10, Kunihiko Sakamoto wrote:
> > On 2017/03/08 09:19:46, Ilya Sherman wrote:
> > > Would it be useful to also record when a page never reached either of
these
> > > states, as a baseline?
> > 
> > Unfortunately it's hard (if not impossible) to record it reliably in
renderer,
> > since renderer can be killed any time. We record (# page loads that had
> 2-quiet
> > / total page loads) in browser side PageLoad metrics, so not recording
> baseline
> > here would be okay.
> 
> Okay.  In that case, could you please include a reference, in this histogram
> description, to the metric that you think is an appropriate baseline?

Done.

[Ilya Sherman, 2017-03-10 20:45:24]

Metrics LGTM, thanks

[Kunihiko Sakamoto, 2017-03-13 07:13:35]

The CQ bit was checked by ksakamoto@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-13 07:13:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-13 08:14:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-13 08:14:12]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Kunihiko Sakamoto, 2017-03-14 03:18:32]

The CQ bit was checked by ksakamoto@chromium.org

[Kunihiko Sakamoto, 2017-03-14 03:18:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdresser@chromium.org, kouhei@chromium.org, csharrison@chromium.org,
isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/2711893002/#ps180001
(title: "rebase")

[commit-bot: I haz the power, 2017-03-14 03:18:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-14 03:27:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-14 03:27:15]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Kunihiko Sakamoto, 2017-03-14 03:30:47]

ksakamoto@chromium.org changed reviewers:
+ yhirano@chromium.org
- tyoshino@chromium.org

[Kunihiko Sakamoto, 2017-03-14 03:30:48]

Hirano-san, would you have a look, for platform/loader?

[yhirano, 2017-03-14 07:28:58]

platform/loader lgtm

[Kunihiko Sakamoto, 2017-03-14 08:24:36]

Patchset #8 (id:200001) has been deleted

[Kunihiko Sakamoto, 2017-03-14 08:25:49]

The CQ bit was checked by ksakamoto@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-14 08:26:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Kunihiko Sakamoto, 2017-03-14 08:57:33]

Hirano-san pointed out a bug (thanks!!) that 2-quiet timer must not restart when
request count is changed from 2 to 1, or 1 to 0.

Kouhei, please take another look at FirstMeaningfulPaintDetector in patch set 8.
Thanks!

[commit-bot: I haz the power, 2017-03-14 10:19:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-14 10:19:32]

Dry run: This issue passed the CQ dry run.

[tdresser, 2017-03-14 12:49:31]

On 2017/03/14 08:57:33, Kunihiko Sakamoto wrote:
> Hirano-san pointed out a bug (thanks!!) that 2-quiet timer must not restart
when
> request count is changed from 2 to 1, or 1 to 0.
> 
> Kouhei, please take another look at FirstMeaningfulPaintDetector in patch set
8.
> Thanks!

Did we add test coverage for this bug?

[kouhei (in TOK), 2017-03-14 22:58:33]

still lgtm

[Kunihiko Sakamoto, 2017-03-15 03:24:27]

The CQ bit was checked by ksakamoto@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-15 03:25:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Kunihiko Sakamoto, 2017-03-15 03:26:48]

Test case added, please take a final look.
Thanks!

[kouhei (in TOK), 2017-03-15 05:27:50]

On 2017/03/15 03:26:48, Kunihiko Sakamoto wrote:
> Test case added, please take a final look.
> Thanks!

lgtm

[Kunihiko Sakamoto, 2017-03-15 05:29:01]

The CQ bit was checked by ksakamoto@chromium.org

[Kunihiko Sakamoto, 2017-03-15 05:29:02]

The patchset sent to the CQ was uploaded after l-g-t-m from
isherman@chromium.org, tdresser@chromium.org, yhirano@chromium.org,
csharrison@chromium.org
Link to the patchset: https://codereview.chromium.org/2711893002/#ps260001
(title: "less hacky test")

[commit-bot: I haz the power, 2017-03-15 05:29:25]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-15 06:57:13]

CQ is committing da patch.

Bot data: {"patchset_id": 260001, "attempt_start_ts": 1489555741795020,
"parent_rev": "93bb5a29073d2cb41b20d3cc6cf59459a3906043", "commit_rev":
"4d059dcf25a256d591234eb2a7ffd1673f7c9709"}

[commit-bot: I haz the power, 2017-03-15 06:58:10]

Description was changed from

==========
Loosen FirstMeaningfulPaint UMA network-idle heuristics

Currently, FirstMeaningfulPaint (FMP) UMA is logged when page's network
activity is "0-quiet" (no network requests) for 0.5 seconds. But about
23% of page loads are aborted before reaching that condition.

To improve the coverage of FMP UMA, this patch changes the network idle
heuristic to "2-quiet" (no more than 2 network requests) for 3 seconds
window.

To verify this change doesn't make FMP reported too soon,
FirstMeaningfulPaintDetector continues to observe layouts until network
0-quiet, and report whether the last FMP candidate was after network
2-quiet or not.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Loosen FirstMeaningfulPaint UMA network-idle heuristics

Currently, FirstMeaningfulPaint (FMP) UMA is logged when page's network
activity is "0-quiet" (no network requests) for 0.5 seconds. But about
23% of page loads are aborted before reaching that condition.

To improve the coverage of FMP UMA, this patch changes the network idle
heuristic to "2-quiet" (no more than 2 network requests) for 3 seconds
window.

To verify this change doesn't make FMP reported too soon,
FirstMeaningfulPaintDetector continues to observe layouts until network
0-quiet, and report whether the last FMP candidate was after network
2-quiet or not.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2711893002
Cr-Commit-Position: refs/heads/master@{#457017}
Committed:
https://chromium.googlesource.com/chromium/src/+/4d059dcf25a256d591234eb2a7ff...
==========

[commit-bot: I haz the power, 2017-03-15 06:58:12]

Committed patchset #10 (id:260001) as
https://chromium.googlesource.com/chromium/src/+/4d059dcf25a256d591234eb2a7ff...