Only report exit funnels for canary and dev channels.

Only report exit funnels for canary and dev channels.

BUG=442526
Committed: https://crrev.com/7815ac1cf0548803189d73a53fe53622516e30b2
Cr-Commit-Position: refs/heads/master@{#312911}

[Sigurður Ásgeirsson, 2015-01-23 18:27:24]

siggi@chromium.org changed reviewers:
+ asvitkine@chromium.org, erikwright@chromium.org

[Sigurður Ásgeirsson, 2015-01-23 18:27:24]

Hey Erik,

PTAL, Alexei for owners please.

Siggi

[erikwright (departed), 2015-01-23 18:35:26]

LGTM

[Alexei Svitkine (slow), 2015-01-23 18:45:22]

https://codereview.chromium.org/868163002/diff/1/chrome/browser/metrics/chrom...
File chrome/browser/metrics/chrome_metrics_service_client.cc (right):

https://codereview.chromium.org/868163002/diff/1/chrome/browser/metrics/chrom...
chrome/browser/metrics/chrome_metrics_service_client.cc:304: report_exit_funnels
= true;
Why can't this code be part of WatcherMetricsProviderWin()?

If it's just for testing, you could have two constructors for that class, one
that takes the parameter and the other that computes it...

https://codereview.chromium.org/868163002/diff/1/components/browser_watcher/w...
File components/browser_watcher/watcher_metrics_provider_win_unittest.cc
(right):

https://codereview.chromium.org/868163002/diff/1/components/browser_watcher/w...
components/browser_watcher/watcher_metrics_provider_win_unittest.cc:169: // Test
that the metrics provider picks up the writes from
Incomplete sentence?

[Sigurður Ásgeirsson, 2015-01-23 18:48:44]

Thanks, PTAL?

https://codereview.chromium.org/868163002/diff/1/chrome/browser/metrics/chrom...
File chrome/browser/metrics/chrome_metrics_service_client.cc (right):

https://codereview.chromium.org/868163002/diff/1/chrome/browser/metrics/chrom...
chrome/browser/metrics/chrome_metrics_service_client.cc:304: report_exit_funnels
= true;
On 2015/01/23 18:45:22, Alexei Svitkine wrote:
> Why can't this code be part of WatcherMetricsProviderWin()?
> 
> If it's just for testing, you could have two constructors for that class, one
> that takes the parameter and the other that computes it...

The provider is a component - I didn't think it's kosher for those to take a dep
on chrome::?

https://codereview.chromium.org/868163002/diff/1/components/browser_watcher/w...
File components/browser_watcher/watcher_metrics_provider_win_unittest.cc
(right):

https://codereview.chromium.org/868163002/diff/1/components/browser_watcher/w...
components/browser_watcher/watcher_metrics_provider_win_unittest.cc:169: // Test
that the metrics provider picks up the writes from
On 2015/01/23 18:45:22, Alexei Svitkine wrote:
> Incomplete sentence?

Done.

[Alexei Svitkine (slow), 2015-01-23 18:50:53]

lgtm

https://codereview.chromium.org/868163002/diff/1/chrome/browser/metrics/chrom...
File chrome/browser/metrics/chrome_metrics_service_client.cc (right):

https://codereview.chromium.org/868163002/diff/1/chrome/browser/metrics/chrom...
chrome/browser/metrics/chrome_metrics_service_client.cc:304: report_exit_funnels
= true;
On 2015/01/23 18:48:43, Sigurður Ásgeirsson wrote:
> On 2015/01/23 18:45:22, Alexei Svitkine wrote:
> > Why can't this code be part of WatcherMetricsProviderWin()?
> > 
> > If it's just for testing, you could have two constructors for that class,
one
> > that takes the parameter and the other that computes it...
> 
> The provider is a component - I didn't think it's kosher for those to take a
dep
> on chrome::?

Ah, you're absolutely right!

[Sigurður Ásgeirsson, 2015-01-23 18:52:03]

The CQ bit was checked by siggi@chromium.org

[commit-bot: I haz the power, 2015-01-23 18:53:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/868163002/20001

[commit-bot: I haz the power, 2015-01-23 20:05:42]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-01-23 20:08:12]

Patchset 2 (id:??) landed as
https://crrev.com/7815ac1cf0548803189d73a53fe53622516e30b2
Cr-Commit-Position: refs/heads/master@{#312911}