Remove the Timing2 histograms that are deprecated.

Remove the Timing2 histograms that are deprecated.

The document.write observer was still logging a few Timing2 histograms,
which prevented us from deprecating them. Now that the new versions of
these histograms have rolled out to stable, we can safely remove the old
ones and mark the base Timing2 histograms as deprecated.

BUG=648726
Committed: https://crrev.com/e915bc7618c1e48299060bf8a45d1c8c53c6ef25
Cr-Commit-Position: refs/heads/master@{#420985}

[Bryan McQuade, 2016-09-20 20:23:37]

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

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

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-09-20 20:25:16]

Description was changed from

==========
Remove deprecated Timing2 histograms for doc write observer.

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

to

==========
Remove deprecated Timing2 histograms for doc write observer.

BUG=648726
==========

[commit-bot: I haz the power, 2016-09-20 21:33:22]

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

[commit-bot: I haz the power, 2016-09-20 21:33:24]

Dry run: This issue passed the CQ dry run.

[Bryan McQuade, 2016-09-21 20:32:22]

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

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

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-09-21 20:33:20]

Description was changed from

==========
Remove deprecated Timing2 histograms for doc write observer.

BUG=648726
==========

to

==========
Remove deprecated Timing2 histograms.

The document.write observer was still logging a few Timing2 histograms,
which prevented us from deprecating them. Now that the new versions of
these histograms have rolled out to stable, we can safely remove the old
ones and mark the base Timing2 histograms as deprecated.

BUG=648726
==========

[Bryan McQuade, 2016-09-21 20:34:46]

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

[Bryan McQuade, 2016-09-21 20:34:55]

PTAL, thanks!

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

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

[commit-bot: I haz the power, 2016-09-21 21:26:10]

Dry run: This issue passed the CQ dry run.

[shivanisha, 2016-09-26 15:54:38]

https://codereview.chromium.org/2359523003/diff/40001/chrome/browser/page_loa...
File
chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc
(left):

https://codereview.chromium.org/2359523003/diff/40001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc:449:
if (WasParseInForeground(timing.parse_start, timing.parse_stop, info)) {
WasParseInForeground also considers incomplete parses and we will lose that data
if we remove this. Do we want to do that or keep those metrics?

[Bryan McQuade, 2016-09-26 16:02:55]

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

[Bryan McQuade, 2016-09-26 16:03:08]

Thanks! I replied to your comment. PTAL, thanks!

https://codereview.chromium.org/2359523003/diff/40001/chrome/browser/page_loa...
File
chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc
(left):

https://codereview.chromium.org/2359523003/diff/40001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc:449:
if (WasParseInForeground(timing.parse_start, timing.parse_stop, info)) {
On 2016/09/26 at 15:54:38, shivanisha wrote:
> WasParseInForeground also considers incomplete parses and we will lose that
data if we remove this. Do we want to do that or keep those metrics?

I thought about this for a bit and came to the conclusion that having the two
variations, one for incomplete parse and one for full parse, provided more data
than we needed. I think logging just once at parse complete should give us
enough insight that we don't need the partial parse info. I went ahead and
removed the WasParseInForeground function as well, as part of this change. Let
me know if you think we should log on incomplete parse - I can definitely add
that back if you think it's important.

[commit-bot: I haz the power, 2016-09-26 16:03:22]

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

[shivanisha, 2016-09-26 16:52:17]

On 2016/09/26 at 16:03:08, bmcquade wrote:
> Thanks! I replied to your comment. PTAL, thanks!
> 
>
https://codereview.chromium.org/2359523003/diff/40001/chrome/browser/page_loa...
> File
chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc
(left):
> 
>
https://codereview.chromium.org/2359523003/diff/40001/chrome/browser/page_loa...
>
chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc:449:
if (WasParseInForeground(timing.parse_start, timing.parse_stop, info)) {
> On 2016/09/26 at 15:54:38, shivanisha wrote:
> > WasParseInForeground also considers incomplete parses and we will lose that
data if we remove this. Do we want to do that or keep those metrics?
> 
> I thought about this for a bit and came to the conclusion that having the two
variations, one for incomplete parse and one for full parse, provided more data
than we needed. I think logging just once at parse complete should give us
enough insight that we don't need the partial parse info. I went ahead and
removed the WasParseInForeground function as well, as part of this change. Let
me know if you think we should log on incomplete parse - I can definitely add
that back if you think it's important.

LGTM
Thanks!

[shivanisha, 2016-09-26 16:52:35]

https://codereview.chromium.org/2359523003/diff/40001/chrome/browser/page_loa...
File
chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc
(left):

https://codereview.chromium.org/2359523003/diff/40001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc:449:
if (WasParseInForeground(timing.parse_start, timing.parse_stop, info)) {
Removing those metrics sound good to me as well as they don't seem very
significant.

[commit-bot: I haz the power, 2016-09-26 20:03:29]

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

[commit-bot: I haz the power, 2016-09-26 20:03:32]

Dry run: This issue passed the CQ dry run.

[Bryan McQuade, 2016-09-26 20:07:09]

The CQ bit was checked by bmcquade@chromium.org

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

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

[Bryan McQuade, 2016-09-26 20:14:21]

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

[Bryan McQuade, 2016-09-26 20:14:33]

gayane, PTAL for histograms.xml, thanks!

[commit-bot: I haz the power, 2016-09-26 20:19:30]

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

[commit-bot: I haz the power, 2016-09-26 20:19:32]

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

[gayane -on leave until 09-2017, 2016-09-26 20:36:53]

LGTM

I find the CL description confusing because it seems like all Timing2 histograms
got deprecated by this CL, but I see some Timing2 histograms still remaining and
being used by core_page_load_metrics_observer

[Bryan McQuade, 2016-09-26 20:37:47]

Description was changed from

==========
Remove deprecated Timing2 histograms.

The document.write observer was still logging a few Timing2 histograms,
which prevented us from deprecating them. Now that the new versions of
these histograms have rolled out to stable, we can safely remove the old
ones and mark the base Timing2 histograms as deprecated.

BUG=648726
==========

to

==========
Remove the Timing2 histograms that are deprecated.

The document.write observer was still logging a few Timing2 histograms,
which prevented us from deprecating them. Now that the new versions of
these histograms have rolled out to stable, we can safely remove the old
ones and mark the base Timing2 histograms as deprecated.

BUG=648726
==========

[Bryan McQuade, 2016-09-26 20:37:50]

The CQ bit was checked by bmcquade@chromium.org

[commit-bot: I haz the power, 2016-09-26 20:38:31]

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

Committed patchset #4 (id:60001)

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

Description was changed from

==========
Remove the Timing2 histograms that are deprecated.

The document.write observer was still logging a few Timing2 histograms,
which prevented us from deprecating them. Now that the new versions of
these histograms have rolled out to stable, we can safely remove the old
ones and mark the base Timing2 histograms as deprecated.

BUG=648726
==========

to

==========
Remove the Timing2 histograms that are deprecated.

The document.write observer was still logging a few Timing2 histograms,
which prevented us from deprecating them. Now that the new versions of
these histograms have rolled out to stable, we can safely remove the old
ones and mark the base Timing2 histograms as deprecated.

BUG=648726

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

[commit-bot: I haz the power, 2016-09-26 20:50:40]

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