Remove non-immediate core page load metrics

Remove non-immediate core page load metrics

This change removes the PageLoad.Timing2.* metrics, as they have
been replaced with
PageLoad.(Paint|Document|Parse)Timing.* equivalents.

Additionally, histograms that use dom loading are removed, as it
has been pointed out that dom loading is not well spec'd and has
been marked as deprecated
(https://github.com/w3c/navigation-timing/issues/13).

This change does not obsolete these histograms in histograms.xml,
as some experiments continue to depend on them and removing
them from histograms.xml makes them inaccessible in the Finch
dashboard. We'll remove these histograms from histograms.xml in
a follow up change in a few releases, when everyone has migrated
to the new metrics.

A few PageLoad.Timing2.* metrics are not removed in this change.
Those metrics will eventually need to be migrated to new names,
but for the time being they do not have new equivalents, so we
have not migrated them yet.

BUG=611740
Committed: https://crrev.com/ea728cb128c672e9cb9c99c16c00206575584d0a
Cr-Commit-Position: refs/heads/master@{#406530}

[Bryan McQuade, 2016-07-18 11:34:54]

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

[commit-bot: I haz the power, 2016-07-18 11:35:08]

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

[Bryan McQuade, 2016-07-18 12:04:24]

Description was changed from

==========
remove non-immediate metrics

BUG=
==========

to

==========
Remove non-immediate core page load metrics

This change removes the PageLoad.Timing2.* metrics with
PageLoad.(Paint|Document|Parse)Timing.* equivalents.

Additionally, histograms that use dom loading are removed, as it
has been pointed out that dom loading is not well spec'd and has
been marked as deprecated
(https://github.com/w3c/navigation-timing/issues/13).

BUG=611740
==========

[commit-bot: I haz the power, 2016-07-18 12:06:22]

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

[commit-bot: I haz the power, 2016-07-18 12:06:23]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Bryan McQuade, 2016-07-18 12:20:20]

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

[commit-bot: I haz the power, 2016-07-18 12:20:30]

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

[Bryan McQuade, 2016-07-18 12:22:46]

Description was changed from

==========
Remove non-immediate core page load metrics

This change removes the PageLoad.Timing2.* metrics with
PageLoad.(Paint|Document|Parse)Timing.* equivalents.

Additionally, histograms that use dom loading are removed, as it
has been pointed out that dom loading is not well spec'd and has
been marked as deprecated
(https://github.com/w3c/navigation-timing/issues/13).

BUG=611740
==========

to

==========
Remove non-immediate core page load metrics

This change removes the PageLoad.Timing2.* metrics with
PageLoad.(Paint|Document|Parse)Timing.* equivalents.

Additionally, histograms that use dom loading are removed, as it
has been pointed out that dom loading is not well spec'd and has
been marked as deprecated
(https://github.com/w3c/navigation-timing/issues/13).

This change does not remove these histograms from histograms.xml,
as some experiments continue to depend on them and removing
them from histograms.xml makes them inaccessible in the Finch
dashboard. We'll remove these histograms from histograms.xml in
a follow up change in a few releases, when everyone has migrated
to the new metrics.

BUG=611740
==========

[Bryan McQuade, 2016-07-18 12:23:07]

Description was changed from

==========
Remove non-immediate core page load metrics

This change removes the PageLoad.Timing2.* metrics with
PageLoad.(Paint|Document|Parse)Timing.* equivalents.

Additionally, histograms that use dom loading are removed, as it
has been pointed out that dom loading is not well spec'd and has
been marked as deprecated
(https://github.com/w3c/navigation-timing/issues/13).

This change does not remove these histograms from histograms.xml,
as some experiments continue to depend on them and removing
them from histograms.xml makes them inaccessible in the Finch
dashboard. We'll remove these histograms from histograms.xml in
a follow up change in a few releases, when everyone has migrated
to the new metrics.

BUG=611740
==========

to

==========
Remove non-immediate core page load metrics

This change removes the PageLoad.Timing2.* metrics, as they have
been replaced with
PageLoad.(Paint|Document|Parse)Timing.* equivalents.

Additionally, histograms that use dom loading are removed, as it
has been pointed out that dom loading is not well spec'd and has
been marked as deprecated
(https://github.com/w3c/navigation-timing/issues/13).

This change does not remove these histograms from histograms.xml,
as some experiments continue to depend on them and removing
them from histograms.xml makes them inaccessible in the Finch
dashboard. We'll remove these histograms from histograms.xml in
a follow up change in a few releases, when everyone has migrated
to the new metrics.

BUG=611740
==========

[Bryan McQuade, 2016-07-18 12:24:36]

Description was changed from

==========
Remove non-immediate core page load metrics

This change removes the PageLoad.Timing2.* metrics, as they have
been replaced with
PageLoad.(Paint|Document|Parse)Timing.* equivalents.

Additionally, histograms that use dom loading are removed, as it
has been pointed out that dom loading is not well spec'd and has
been marked as deprecated
(https://github.com/w3c/navigation-timing/issues/13).

This change does not remove these histograms from histograms.xml,
as some experiments continue to depend on them and removing
them from histograms.xml makes them inaccessible in the Finch
dashboard. We'll remove these histograms from histograms.xml in
a follow up change in a few releases, when everyone has migrated
to the new metrics.

BUG=611740
==========

to

==========
Remove non-immediate core page load metrics

This change removes the PageLoad.Timing2.* metrics, as they have
been replaced with
PageLoad.(Paint|Document|Parse)Timing.* equivalents.

Additionally, histograms that use dom loading are removed, as it
has been pointed out that dom loading is not well spec'd and has
been marked as deprecated
(https://github.com/w3c/navigation-timing/issues/13).

This change does not remove these histograms from histograms.xml,
as some experiments continue to depend on them and removing
them from histograms.xml makes them inaccessible in the Finch
dashboard. We'll remove these histograms from histograms.xml in
a follow up change in a few releases, when everyone has migrated
to the new metrics.

A few PageLoad.Timing2.* metrics are not removed in this change.
Those metrics will eventually need to be migrated to new names,
but for the time being they do not have new equivalents, so we
have not migrated them yet.

BUG=611740
==========

[Bryan McQuade, 2016-07-18 12:24:47]

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

[Bryan McQuade, 2016-07-18 12:24:54]

PTAL

[Bryan McQuade, 2016-07-18 12:28:08]

Description was changed from

==========
Remove non-immediate core page load metrics

This change removes the PageLoad.Timing2.* metrics, as they have
been replaced with
PageLoad.(Paint|Document|Parse)Timing.* equivalents.

Additionally, histograms that use dom loading are removed, as it
has been pointed out that dom loading is not well spec'd and has
been marked as deprecated
(https://github.com/w3c/navigation-timing/issues/13).

This change does not remove these histograms from histograms.xml,
as some experiments continue to depend on them and removing
them from histograms.xml makes them inaccessible in the Finch
dashboard. We'll remove these histograms from histograms.xml in
a follow up change in a few releases, when everyone has migrated
to the new metrics.

A few PageLoad.Timing2.* metrics are not removed in this change.
Those metrics will eventually need to be migrated to new names,
but for the time being they do not have new equivalents, so we
have not migrated them yet.

BUG=611740
==========

to

==========
Remove non-immediate core page load metrics

This change removes the PageLoad.Timing2.* metrics, as they have
been replaced with
PageLoad.(Paint|Document|Parse)Timing.* equivalents.

Additionally, histograms that use dom loading are removed, as it
has been pointed out that dom loading is not well spec'd and has
been marked as deprecated
(https://github.com/w3c/navigation-timing/issues/13).

This change does not obsolete these histograms in histograms.xml,
as some experiments continue to depend on them and removing
them from histograms.xml makes them inaccessible in the Finch
dashboard. We'll remove these histograms from histograms.xml in
a follow up change in a few releases, when everyone has migrated
to the new metrics.

A few PageLoad.Timing2.* metrics are not removed in this change.
Those metrics will eventually need to be migrated to new names,
but for the time being they do not have new equivalents, so we
have not migrated them yet.

BUG=611740
==========

[commit-bot: I haz the power, 2016-07-18 13:26:28]

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

[commit-bot: I haz the power, 2016-07-18 13:26:29]

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

[Bryan McQuade, 2016-07-18 13:48:25]

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

[commit-bot: I haz the power, 2016-07-18 13:48:46]

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-07-18 14:36:08]

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

[commit-bot: I haz the power, 2016-07-18 14:36:09]

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

[shivanisha, 2016-07-18 14:50:34]

Thanks for this change!

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

https://codereview.chromium.org/2156093002/diff/40001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc:239:
timing.first_paint > info.first_foreground_time.value() &&
Should be timing.first_paint.value()

https://codereview.chromium.org/2156093002/diff/40001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc:241:
timing.first_paint < info.first_background_time.value())) {
Same : should be timing.first_paint.value()

Also can we include equality here to be consistent with the condition in
WasStartedInForegroundOptionalEventInForeground(), so 
timing.first_paint <= info.first_background_time.value() ?

[Bryan McQuade, 2016-07-18 15:06:50]

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

[commit-bot: I haz the power, 2016-07-18 15:07:30]

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

[Bryan McQuade, 2016-07-18 15:10:40]

Thanks! PTAL. Note that I'm also removing the low res/high res clock histograms
in https://codereview.chromium.org/2155143003 and I've updated this change to
match. I'll wait for that change to land and merge it before landing this
change.

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

https://codereview.chromium.org/2156093002/diff/40001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc:239:
timing.first_paint > info.first_foreground_time.value() &&
On 2016/07/18 at 14:50:34, shivanisha wrote:
> Should be timing.first_paint.value()

Fixed.

https://codereview.chromium.org/2156093002/diff/40001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc:241:
timing.first_paint < info.first_background_time.value())) {
On 2016/07/18 at 14:50:34, shivanisha wrote:
> Same : should be timing.first_paint.value()
> 
> Also can we include equality here to be consistent with the condition in
WasStartedInForegroundOptionalEventInForeground(), so 
> timing.first_paint <= info.first_background_time.value() ?

Sure, I switched the ordering around and used <= to be consistent. Optional<>
operators like < are overridden, so code compiles ok without the .value(), but I
agree it's better to be explicit so I've fixed these to include .value().
Thanks!

[shivanisha, 2016-07-18 15:27:43]

lgtm

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

https://codereview.chromium.org/2156093002/diff/40001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc:241:
timing.first_paint < info.first_background_time.value())) {
On 2016/07/18 at 15:10:40, Bryan McQuade wrote:
> On 2016/07/18 at 14:50:34, shivanisha wrote:
> > Same : should be timing.first_paint.value()
> > 
> > Also can we include equality here to be consistent with the condition in
WasStartedInForegroundOptionalEventInForeground(), so 
> > timing.first_paint <= info.first_background_time.value() ?
> 
> Sure, I switched the ordering around and used <= to be consistent. Optional<>
operators like < are overridden, so code compiles ok without the .value(), but I
agree it's better to be explicit so I've fixed these to include .value().
Thanks!

Good to know about the overriding in Optional.

[commit-bot: I haz the power, 2016-07-18 16:06:43]

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

[commit-bot: I haz the power, 2016-07-18 16:06:44]

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

[Bryan McQuade, 2016-07-19 12:02:37]

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

[commit-bot: I haz the power, 2016-07-19 12:02:51]

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-07-19 12:04:47]

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

[commit-bot: I haz the power, 2016-07-19 12:04:47]

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-gn on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-gn on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)

[Bryan McQuade, 2016-07-19 12:44:29]

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

[commit-bot: I haz the power, 2016-07-19 12:44:52]

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

[Bryan McQuade, 2016-07-19 13:10:38]

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

[commit-bot: I haz the power, 2016-07-19 13:38:06]

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

[commit-bot: I haz the power, 2016-07-19 13:38:06]

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

[Bryan McQuade, 2016-07-19 14:27:55]

isherman, PTAL for histograms.xml, thanks!

[Bryan McQuade, 2016-07-19 16:25:30]

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

[commit-bot: I haz the power, 2016-07-19 16:25:54]

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

[Bryan McQuade, 2016-07-19 16:40:20]

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

[commit-bot: I haz the power, 2016-07-19 16:41:01]

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-07-19 17:22:46]

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

[commit-bot: I haz the power, 2016-07-19 17:22:47]

Dry run: This issue passed the CQ dry run.

[Ilya Sherman, 2016-07-20 01:48:07]

histograms.xml lgtm.  Thanks for the cleanup!

[Ilya Sherman, 2016-07-20 01:48:14]

The CQ bit was checked by isherman@chromium.org

[Ilya Sherman, 2016-07-20 01:48:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
shivanisha@chromium.org
Link to the patchset: https://codereview.chromium.org/2156093002/#ps140001
(title: "address comments")

[commit-bot: I haz the power, 2016-07-20 01:49:24]

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

[commit-bot: I haz the power, 2016-07-20 01:49:25]

This CL has an open dependency (Issue 2152683004 Patch 310001). Please resolve
the dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[Bryan McQuade, 2016-07-20 02:35:31]

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

[commit-bot: I haz the power, 2016-07-20 02:36:04]

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-07-20 03:47:26]

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

[commit-bot: I haz the power, 2016-07-20 03:47:28]

Dry run: This issue passed the CQ dry run.

[Bryan McQuade, 2016-07-20 11:58:13]

The CQ bit was checked by bmcquade@chromium.org

[Bryan McQuade, 2016-07-20 11:58:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
shivanisha@chromium.org, isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/2156093002/#ps160001
(title: "address comments")

[commit-bot: I haz the power, 2016-07-20 11:58:40]

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-07-20 12:56:05]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-07-20 12:59:22]

Description was changed from

==========
Remove non-immediate core page load metrics

This change removes the PageLoad.Timing2.* metrics, as they have
been replaced with
PageLoad.(Paint|Document|Parse)Timing.* equivalents.

Additionally, histograms that use dom loading are removed, as it
has been pointed out that dom loading is not well spec'd and has
been marked as deprecated
(https://github.com/w3c/navigation-timing/issues/13).

This change does not obsolete these histograms in histograms.xml,
as some experiments continue to depend on them and removing
them from histograms.xml makes them inaccessible in the Finch
dashboard. We'll remove these histograms from histograms.xml in
a follow up change in a few releases, when everyone has migrated
to the new metrics.

A few PageLoad.Timing2.* metrics are not removed in this change.
Those metrics will eventually need to be migrated to new names,
but for the time being they do not have new equivalents, so we
have not migrated them yet.

BUG=611740
==========

to

==========
Remove non-immediate core page load metrics

This change removes the PageLoad.Timing2.* metrics, as they have
been replaced with
PageLoad.(Paint|Document|Parse)Timing.* equivalents.

Additionally, histograms that use dom loading are removed, as it
has been pointed out that dom loading is not well spec'd and has
been marked as deprecated
(https://github.com/w3c/navigation-timing/issues/13).

This change does not obsolete these histograms in histograms.xml,
as some experiments continue to depend on them and removing
them from histograms.xml makes them inaccessible in the Finch
dashboard. We'll remove these histograms from histograms.xml in
a follow up change in a few releases, when everyone has migrated
to the new metrics.

A few PageLoad.Timing2.* metrics are not removed in this change.
Those metrics will eventually need to be migrated to new names,
but for the time being they do not have new equivalents, so we
have not migrated them yet.

BUG=611740

Committed: https://crrev.com/ea728cb128c672e9cb9c99c16c00206575584d0a
Cr-Commit-Position: refs/heads/master@{#406530}
==========

[commit-bot: I haz the power, 2016-07-20 12:59:24]

Patchset 9 (id:??) landed as
https://crrev.com/ea728cb128c672e9cb9c99c16c00206575584d0a
Cr-Commit-Position: refs/heads/master@{#406530}