Split redirect delay metrics into separate histograms.

Split redirect delay metrics into separate histograms.

In https://codereview.chromium.org/2103153004/, a histogram was added
to track the time between a page painting, and initiating a client-side
redirect.

Additionally, a zero value was recorded in the following two cases:
* the page initiated a client-side redirect, then painted
* the page initiated a client-side redirect, but never painted

It turns out to be important to distinguish these cases. Distinguishing
these cases will let us understand how frequently a page may initiate a
redirect, paint, then have the redirect commit. In this case, we may want
to suppress the paint notification since it ends up being more of an
interstitial paint while navigating to the actual destination page, rather
than what we'd consider an actual first paint. In order to decide whether
we should add logic to suppress, this change separates the histogram into
two so we can understand how common this case is.

BUG=613634
Committed: https://crrev.com/09b1ee4df148f93c4a8f7f7f3d64a80a3479f0f1
Cr-Commit-Position: refs/heads/master@{#406192}

[Bryan McQuade, 2016-07-15 21:02:11]

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

[commit-bot: I haz the power, 2016-07-15 21:02:38]

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-15 21:02:57]

Description was changed from

==========
Split redirect delay metrics into separate histograms.

In https://codereview.chromium.org/2103153004/, a histogram was added
to track the time between a page painting, and initiating a client-side
redirect.

Additionally, a zero value was recorded in the following two cases:
* the page initiated a client-side redirect, then painted
* the page initiated a client-side redirect, but never painted

It turns out to be important to distinguish these cases. Distinguishing
these cases will let us understand how frequently a page may initiate the
redirect, paint, then the redirect commits. In this case, we may want to
suppress the paint notification since it ends up being more of an
interstitial paint while navigating to the actual destination page, rather
than what we'd consider an actual first paint. In order to decide whether
we should add logic to suppress, this change separates the histogram into
two so we can understand how common this case is.

BUG=613634
==========

to

==========
Split redirect delay metrics into separate histograms.

In https://codereview.chromium.org/2103153004/, a histogram was added
to track the time between a page painting, and initiating a client-side
redirect.

Additionally, a zero value was recorded in the following two cases:
* the page initiated a client-side redirect, then painted
* the page initiated a client-side redirect, but never painted

It turns out to be important to distinguish these cases. Distinguishing
these cases will let us understand how frequently a page may initiate a
redirect, paint, then have the redirect commit. In this case, we may want
to suppress the paint notification since it ends up being more of an
interstitial paint while navigating to the actual destination page, rather
than what we'd consider an actual first paint. In order to decide whether
we should add logic to suppress, this change separates the histogram into
two so we can understand how common this case is.

BUG=613634
==========

[Bryan McQuade, 2016-07-15 21:08:13]

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

[commit-bot: I haz the power, 2016-07-15 21:08:47]

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-15 21:59:06]

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

[Bryan McQuade, 2016-07-15 21:59:06]

PTAL

[commit-bot: I haz the power, 2016-07-15 22:17:04]

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

[commit-bot: I haz the power, 2016-07-15 22:17:05]

Dry run: This issue passed the CQ dry run.

[Ilya Sherman, 2016-07-15 23:05:38]

https://codereview.chromium.org/2151363003/diff/20001/components/page_load_me...
File components/page_load_metrics/browser/metrics_web_contents_observer.cc
(right):

https://codereview.chromium.org/2151363003/diff/20001/components/page_load_me...
components/page_load_metrics/browser/metrics_web_contents_observer.cc:401:
UMA_HISTOGRAM_COUNTS(internal::kClientRedirectWithoutPaint, 1);
This allocates 50 buckets, of which only one is used.  Please use
UMA_HISTOGRAM_BOOLEAN instead.

https://codereview.chromium.org/2151363003/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (left):

https://codereview.chromium.org/2151363003/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:37521: -<histogram
name="PageLoad.Internal.ClientRedirectDelayAfterPaint" units="ms">
Please mark this histogram as obsolete.

[Bryan McQuade, 2016-07-16 14:06:57]

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

[commit-bot: I haz the power, 2016-07-16 14:07:02]

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-16 15:00:12]

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

[commit-bot: I haz the power, 2016-07-16 15:00:13]

Dry run: This issue passed the CQ dry run.

[Bryan McQuade, 2016-07-17 13:18:06]

PTAL thanks!

https://codereview.chromium.org/2151363003/diff/20001/components/page_load_me...
File components/page_load_metrics/browser/metrics_web_contents_observer.cc
(right):

https://codereview.chromium.org/2151363003/diff/20001/components/page_load_me...
components/page_load_metrics/browser/metrics_web_contents_observer.cc:401:
UMA_HISTOGRAM_COUNTS(internal::kClientRedirectWithoutPaint, 1);
On 2016/07/15 at 23:05:38, Ilya Sherman wrote:
> This allocates 50 buckets, of which only one is used.  Please use
UMA_HISTOGRAM_BOOLEAN instead.

Ah, good to know, thanks! Switched.

https://codereview.chromium.org/2151363003/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (left):

https://codereview.chromium.org/2151363003/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:37521: -<histogram
name="PageLoad.Internal.ClientRedirectDelayAfterPaint" units="ms">
On 2016/07/15 at 23:05:38, Ilya Sherman wrote:
> Please mark this histogram as obsolete.

Done.

[Charlie Harrison, 2016-07-18 20:04:57]

lgtm

[Ilya Sherman, 2016-07-18 23:02:00]

LGTM, thanks.

[Bryan McQuade, 2016-07-18 23:06:09]

The CQ bit was checked by bmcquade@chromium.org

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

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 02:26:44]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-19 02:29:00]

Description was changed from

==========
Split redirect delay metrics into separate histograms.

In https://codereview.chromium.org/2103153004/, a histogram was added
to track the time between a page painting, and initiating a client-side
redirect.

Additionally, a zero value was recorded in the following two cases:
* the page initiated a client-side redirect, then painted
* the page initiated a client-side redirect, but never painted

It turns out to be important to distinguish these cases. Distinguishing
these cases will let us understand how frequently a page may initiate a
redirect, paint, then have the redirect commit. In this case, we may want
to suppress the paint notification since it ends up being more of an
interstitial paint while navigating to the actual destination page, rather
than what we'd consider an actual first paint. In order to decide whether
we should add logic to suppress, this change separates the histogram into
two so we can understand how common this case is.

BUG=613634
==========

to

==========
Split redirect delay metrics into separate histograms.

In https://codereview.chromium.org/2103153004/, a histogram was added
to track the time between a page painting, and initiating a client-side
redirect.

Additionally, a zero value was recorded in the following two cases:
* the page initiated a client-side redirect, then painted
* the page initiated a client-side redirect, but never painted

It turns out to be important to distinguish these cases. Distinguishing
these cases will let us understand how frequently a page may initiate a
redirect, paint, then have the redirect commit. In this case, we may want
to suppress the paint notification since it ends up being more of an
interstitial paint while navigating to the actual destination page, rather
than what we'd consider an actual first paint. In order to decide whether
we should add logic to suppress, this change separates the histogram into
two so we can understand how common this case is.

BUG=613634

Committed: https://crrev.com/09b1ee4df148f93c4a8f7f7f3d64a80a3479f0f1
Cr-Commit-Position: refs/heads/master@{#406192}
==========

[commit-bot: I haz the power, 2016-07-19 02:29:01]

Patchset 3 (id:??) landed as
https://crrev.com/09b1ee4df148f93c4a8f7f7f3d64a80a3479f0f1
Cr-Commit-Position: refs/heads/master@{#406192}