Add UMA for service worker navigation preload.

Add UMA for service worker navigation preload.

This will help understand how the experiment is going. Adds:
- ServiceWorker.ActivatedWorkerPreparationForMainFrame.Type
  This is the type of preparation that was needed for any navigation,
  regardless of if nav preload was needed. We already had the data
  based on the .Time suffixes, but it's more convenient to have
  a histogram over looking at the total counts for each suffix.

- ServiceWorker.ActivatedWorkerPreparationForMainFrame.Type_NavigationPreloadEnabled
  This is the type of preparation that was needed when navigation preload
  was enabled. It should not change much from the general histogram, but
  this will check that assumption.

- ServiceWorker.ActivatedWorkerPreparationForMainFrame.Time_NavigationPreloadEnabled
  The time taken to prepare the worker, when nav preload was enabled.

- ServiceWorker.ActivatedWorkerPreparationForMainFrame.Time_StartWorkerExistingProcess_NavigationPreloadEnabled
  The time taken for preparation when the worker had to start up (in the
  existing process common case), when nav preload was enabled.

BUG=661090
Review-Url: https://codereview.chromium.org/2641813002
Cr-Commit-Position: refs/heads/master@{#444639}
Committed: https://chromium.googlesource.com/chromium/src/+/3d62570c946d4dcfb0a562582daa74e804b15d1e

[falken, 2017-01-18 09:51:05]

falken@chromium.org changed reviewers:
+ horo@chromium.org

[falken, 2017-01-18 09:51:06]

This starts adding some UMA for nav preload.

[falken, 2017-01-18 09:56:47]

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

[commit-bot: I haz the power, 2017-01-18 09:57:06]

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-01-18 11:35:46]

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

[commit-bot: I haz the power, 2017-01-18 11:35:47]

Dry run: This issue passed the CQ dry run.

[horo, 2017-01-18 12:26:41]

https://codereview.chromium.org/2641813002/diff/1/content/browser/service_wor...
File content/browser/service_worker/service_worker_metrics_unittest.cc (right):

https://codereview.chromium.org/2641813002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_metrics_unittest.cc:21: }
}  // namespace

https://codereview.chromium.org/2641813002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_metrics_unittest.cc:69: }
Please check this:

    histogram_tester.ExpectTotalCount(kPreparationTime +
                                          kStartWorkerDuringStartupSuffix +
                                          kNavigationPreloadSuffix,
                                      0);

https://codereview.chromium.org/2641813002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_metrics_unittest.cc:98: }  //
namespace
}  // namespace content

[falken, 2017-01-18 15:02:33]

falken@chromium.org changed reviewers:
+ rkaplow@chromium.org

[falken, 2017-01-18 15:02:34]

rkaplow: Can you review histograms?
horo: Thanks.

https://codereview.chromium.org/2641813002/diff/1/content/browser/service_wor...
File content/browser/service_worker/service_worker_metrics_unittest.cc (right):

https://codereview.chromium.org/2641813002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_metrics_unittest.cc:21: }
On 2017/01/18 12:26:41, horo wrote:
> }  // namespace

Done.

https://codereview.chromium.org/2641813002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_metrics_unittest.cc:69: }
On 2017/01/18 12:26:41, horo wrote:
> Please check this:
> 
>     histogram_tester.ExpectTotalCount(kPreparationTime +
>                                           kStartWorkerDuringStartupSuffix +
>                                           kNavigationPreloadSuffix,
>                                       0);

Done.

https://codereview.chromium.org/2641813002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_metrics_unittest.cc:98: }  //
namespace
On 2017/01/18 12:26:41, horo wrote:
> }  // namespace content

Done.

[falken, 2017-01-18 15:03:08]

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

[commit-bot: I haz the power, 2017-01-18 15:03:35]

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

[horo, 2017-01-18 15:24:55]

lgtm

[commit-bot: I haz the power, 2017-01-18 16:32:55]

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

[commit-bot: I haz the power, 2017-01-18 16:32:56]

Dry run: This issue passed the CQ dry run.

[rkaplow, 2017-01-19 01:37:54]

https://codereview.chromium.org/2641813002/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_metrics.cc (right):

https://codereview.chromium.org/2641813002/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_metrics.cc:440: kTypeName +
"_NavigationPreloadEnabled", static_cast<int>(preparation),
this isn't quite right, we don't allow the MACRO use with a non-const input.

You need to call the function that the macro uses. It's a bit ugly.
here's a example:
https://codesearch.chromium.org/chromium/src/ui/gl/angle_platform_impl.cc?q=L...

[falken, 2017-01-19 01:43:03]

https://codereview.chromium.org/2641813002/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_metrics.cc (right):

https://codereview.chromium.org/2641813002/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_metrics.cc:440: kTypeName +
"_NavigationPreloadEnabled", static_cast<int>(preparation),
On 2017/01/19 01:37:53, rkaplow wrote:
> this isn't quite right, we don't allow the MACRO use with a non-const input.
> 
> You need to call the function that the macro uses. It's a bit ugly.
> here's a example:
>
https://codesearch.chromium.org/chromium/src/ui/gl/angle_platform_impl.cc?q=L...

Ah, I thought this was OK as long as the input is always the same, since the
macro caches the result after the first time. If that's not OK, could I just
change it to use const std::string kTypeNameForNavigationPreloadEnabled?

[rkaplow, 2017-01-19 01:52:10]

https://codereview.chromium.org/2641813002/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_metrics.cc (right):

https://codereview.chromium.org/2641813002/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_metrics.cc:440: kTypeName +
"_NavigationPreloadEnabled", static_cast<int>(preparation),
On 2017/01/19 01:43:03, falken wrote:
> On 2017/01/19 01:37:53, rkaplow wrote:
> > this isn't quite right, we don't allow the MACRO use with a non-const input.
> > 
> > You need to call the function that the macro uses. It's a bit ugly.
> > here's a example:
> >
>
https://codesearch.chromium.org/chromium/src/ui/gl/angle_platform_impl.cc?q=L...
> 
> Ah, I thought this was OK as long as the input is always the same, since the
> macro caches the result after the first time. If that's not OK, could I just
> change it to use const std::string kTypeNameForNavigationPreloadEnabled?

yeah - since you don't need it to be computed best to just inline it

[falken, 2017-01-19 02:01:45]

OK I just changed to use string literals. PTAL.

https://codereview.chromium.org/2641813002/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_metrics.cc (right):

https://codereview.chromium.org/2641813002/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_metrics.cc:422:
UMA_HISTOGRAM_MEDIUM_TIMES(name, time);
Oh, this was wrong!

https://codereview.chromium.org/2641813002/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_metrics.cc:440: kTypeName +
"_NavigationPreloadEnabled", static_cast<int>(preparation),
On 2017/01/19 01:52:10, rkaplow wrote:
> On 2017/01/19 01:43:03, falken wrote:
> > On 2017/01/19 01:37:53, rkaplow wrote:
> > > this isn't quite right, we don't allow the MACRO use with a non-const
input.
> > > 
> > > You need to call the function that the macro uses. It's a bit ugly.
> > > here's a example:
> > >
> >
>
https://codesearch.chromium.org/chromium/src/ui/gl/angle_platform_impl.cc?q=L...
> > 
> > Ah, I thought this was OK as long as the input is always the same, since the
> > macro caches the result after the first time. If that's not OK, could I just
> > change it to use const std::string kTypeNameForNavigationPreloadEnabled?
> 
> yeah - since you don't need it to be computed best to just inline it

Done.

[rkaplow, 2017-01-19 02:08:36]

lgtm

[falken, 2017-01-19 02:25:01]

Thank you! I checked chrome://histograms as well and things look good.
Committing this.

[falken, 2017-01-19 02:25:07]

The CQ bit was checked by falken@chromium.org

[falken, 2017-01-19 02:25:08]

The patchset sent to the CQ was uploaded after l-g-t-m from horo@chromium.org,
rkaplow@chromium.org
Link to the patchset: https://codereview.chromium.org/2641813002/#ps60001
(title: "patch for landing")

[commit-bot: I haz the power, 2017-01-19 02:25: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-01-19 04:20:34]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1484792707944920,
"parent_rev": "246883ff6add126d1f6482fe5f883a2d6a2b2525", "commit_rev":
"3d62570c946d4dcfb0a562582daa74e804b15d1e"}

[commit-bot: I haz the power, 2017-01-19 04:21:32]

Description was changed from

==========
Add UMA for service worker navigation preload.

This will help understand how the experiment is going. Adds:
- ServiceWorker.ActivatedWorkerPreparationForMainFrame.Type
  This is the type of preparation that was needed for any navigation,
  regardless of if nav preload was needed. We already had the data
  based on the .Time suffixes, but it's more convenient to have
  a histogram over looking at the total counts for each suffix.

-
ServiceWorker.ActivatedWorkerPreparationForMainFrame.Type_NavigationPreloadEnabled
  This is the type of preparation that was needed when navigation preload
  was enabled. It should not change much from the general histogram, but
  this will check that assumption.

-
ServiceWorker.ActivatedWorkerPreparationForMainFrame.Time_NavigationPreloadEnabled
  The time taken to prepare the worker, when nav preload was enabled.

-
ServiceWorker.ActivatedWorkerPreparationForMainFrame.Time_StartWorkerExistingProcess_NavigationPreloadEnabled
  The time taken for preparation when the worker had to start up (in the
  existing process common case), when nav preload was enabled.

BUG=661090
==========

to

==========
Add UMA for service worker navigation preload.

This will help understand how the experiment is going. Adds:
- ServiceWorker.ActivatedWorkerPreparationForMainFrame.Type
  This is the type of preparation that was needed for any navigation,
  regardless of if nav preload was needed. We already had the data
  based on the .Time suffixes, but it's more convenient to have
  a histogram over looking at the total counts for each suffix.

-
ServiceWorker.ActivatedWorkerPreparationForMainFrame.Type_NavigationPreloadEnabled
  This is the type of preparation that was needed when navigation preload
  was enabled. It should not change much from the general histogram, but
  this will check that assumption.

-
ServiceWorker.ActivatedWorkerPreparationForMainFrame.Time_NavigationPreloadEnabled
  The time taken to prepare the worker, when nav preload was enabled.

-
ServiceWorker.ActivatedWorkerPreparationForMainFrame.Time_StartWorkerExistingProcess_NavigationPreloadEnabled
  The time taken for preparation when the worker had to start up (in the
  existing process common case), when nav preload was enabled.

BUG=661090

Review-Url: https://codereview.chromium.org/2641813002
Cr-Commit-Position: refs/heads/master@{#444639}
Committed:
https://chromium.googlesource.com/chromium/src/+/3d62570c946d4dcfb0a562582daa...
==========

[commit-bot: I haz the power, 2017-01-19 04:21:34]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/3d62570c946d4dcfb0a562582daa...