Add FMP to DataReductionProxy UMA and to Pingback Metrics

Add FMP to DataReductionProxy UMA and to Pingback Metrics

Time to first meaningful paint may provide the best estimates of real
user paint experience. DRP should consume these even if they are still
experimental.

BUG=649123
Committed: https://crrev.com/86c46d40b7a9f67fba3a19a3a7bfff19920d6bc2
Cr-Commit-Position: refs/heads/master@{#422249}

[RyanSturm, 2016-09-23 19:16:31]

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

[commit-bot: I haz the power, 2016-09-23 19:17:21]

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

[RyanSturm, 2016-09-23 19:21:09]

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

[commit-bot: I haz the power, 2016-09-23 19:21: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, 2016-09-23 20:12:02]

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

[commit-bot: I haz the power, 2016-09-23 20:12:03]

Dry run: Try jobs failed on following builders:
  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_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[RyanSturm, 2016-09-23 20:33:48]

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

[commit-bot: I haz the power, 2016-09-23 20:34:24]

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, 2016-09-23 21:24:28]

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

[commit-bot: I haz the power, 2016-09-23 21:24:29]

Dry run: This issue passed the CQ dry run.

[RyanSturm, 2016-09-23 22:58:33]

ryansturm@chromium.org changed reviewers:
+ csharrison@chromium.org, tbansal@chromium.org

[RyanSturm, 2016-09-23 22:58:34]

tbansal: PTAL @ DRP
csharrison: PTAL @ PLM

[tbansal1, 2016-09-24 00:01:57]

lgtm

[Charlie Harrison, 2016-09-25 23:34:53]

Will you okay with this metric changing from under you unexpectedly? We are
continuing to refine when we log FMP which causes big shifts in how many data
points get logged.

[Bryan McQuade, 2016-09-25 23:37:31]

On 2016/09/25 at 23:34:53, csharrison wrote:
> Will you okay with this metric changing from under you unexpectedly? We are
continuing to refine when we log FMP which causes big shifts in how many data
points get logged.

Yeah, I'd advise against including this in the pingback, or, if you do, I'd give
it a name like 'experimental_first_meaniningful_paint' with a comment noting
that the implementation may change at any time, and that the histogram may also
be deleted. If/when we do promote this to non-experimental I'd be very
supportive of having it in pingback at that time.

[Bryan McQuade, 2016-09-25 23:39:16]

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

[Bryan McQuade, 2016-09-25 23:39:18]

https://codereview.chromium.org/2363913003/diff/40001/chrome/browser/page_loa...
File
chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.cc
(right):

https://codereview.chromium.org/2363913003/diff/40001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.cc:77:
"PaintTiming.NavigationToFirstMeaningfulPaint";
this should include 'Experimental' in the name.

https://codereview.chromium.org/2363913003/diff/40001/components/data_reducti...
File components/data_reduction_proxy/proto/pageload_metrics.proto (right):

https://codereview.chromium.org/2363913003/diff/40001/components/data_reducti...
components/data_reduction_proxy/proto/pageload_metrics.proto:73: optional
Duration time_to_first_meaningful_paint = 15;
I'd name this experimental_time_to_first_meaningful_paint if you decide you want
to log this while it is still experimental and subject to being changed.

[RyanSturm, 2016-09-26 20:12:52]

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

[commit-bot: I haz the power, 2016-09-26 20:13:39]

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

[RyanSturm, 2016-09-26 20:15:51]

The server team is aware of how this may change unexpectedly, and are willing to
use it anyway. The analysis should be able to get consistent data across a
milestone in stable when varying server params and they won't invest too much
analysis into this metric. That being said, I made it more explicitly
experimental.

https://codereview.chromium.org/2363913003/diff/40001/chrome/browser/page_loa...
File
chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.cc
(right):

https://codereview.chromium.org/2363913003/diff/40001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.cc:77:
"PaintTiming.NavigationToFirstMeaningfulPaint";
On 2016/09/25 23:39:17, Bryan McQuade wrote:
> this should include 'Experimental' in the name.

Done.

https://codereview.chromium.org/2363913003/diff/40001/components/data_reducti...
File components/data_reduction_proxy/proto/pageload_metrics.proto (right):

https://codereview.chromium.org/2363913003/diff/40001/components/data_reducti...
components/data_reduction_proxy/proto/pageload_metrics.proto:73: optional
Duration time_to_first_meaningful_paint = 15;
On 2016/09/25 23:39:18, Bryan McQuade wrote:
> I'd name this experimental_time_to_first_meaningful_paint if you decide you
want
> to log this while it is still experimental and subject to being changed.

Done.

[commit-bot: I haz the power, 2016-09-26 20:17:52]

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

[commit-bot: I haz the power, 2016-09-26 20:17:52]

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_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...)
  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_...)
  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_...)

[Bryan McQuade, 2016-09-26 20:19:03]

Thanks! LGTM.

https://codereview.chromium.org/2363913003/diff/60001/chrome/browser/page_loa...
File
chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.cc
(right):

https://codereview.chromium.org/2363913003/diff/60001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.cc:166:
experimental_first_meaningful_paint = timing.first_meaningful_paint;
it strikes me we should probably also change the name to include 'experimental'
in PageLoadTiming... :) - I'll look at making that change soon.

[RyanSturm, 2016-09-26 20:46:24]

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

[RyanSturm, 2016-09-26 20:47:06]

ryansturm@chromium.org changed reviewers:
+ asvitkine@chromium.org

[RyanSturm, 2016-09-26 20:47:07]

asvitkine: PTAL @ histograms.xml

[commit-bot: I haz the power, 2016-09-26 20:47:15]

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

[Alexei Svitkine (slow), 2016-09-26 20:55:24]

lgtm

[commit-bot: I haz the power, 2016-09-27 00:46:29]

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

[commit-bot: I haz the power, 2016-09-27 00:46:31]

Dry run: This issue passed the CQ dry run.

[RyanSturm, 2016-09-30 21:32:03]

The CQ bit was checked by ryansturm@chromium.org

[RyanSturm, 2016-09-30 21:32:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
tbansal@chromium.org, bmcquade@chromium.org
Link to the patchset: https://codereview.chromium.org/2363913003/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2016-09-30 21:32:30]

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, 2016-09-30 22:36:21]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-30 22:39:42]

Description was changed from

==========
Add FMP to DataReductionProxy UMA and to Pingback Metrics

Time to first meaningful paint may provide the best estimates of real
user paint experience. DRP should consume these even if they are still
experimental.

BUG=649123
==========

to

==========
Add FMP to DataReductionProxy UMA and to Pingback Metrics

Time to first meaningful paint may provide the best estimates of real
user paint experience. DRP should consume these even if they are still
experimental.

BUG=649123

Committed: https://crrev.com/86c46d40b7a9f67fba3a19a3a7bfff19920d6bc2
Cr-Commit-Position: refs/heads/master@{#422249}
==========

[commit-bot: I haz the power, 2016-09-30 22:39:44]

Patchset 5 (id:??) landed as
https://crrev.com/86c46d40b7a9f67fba3a19a3a7bfff19920d6bc2
Cr-Commit-Position: refs/heads/master@{#422249}