Refactor HashedExtensionMetrics into ExtensionsMetricsProvider.

Refactor HashedExtensionMetrics into ExtensionsMetricsProvider.

Transforms HashedExtensionMetrics into a metrics::MetricsProvider, eliminating
direct knowledge of this class from MetricsLog. Renames the class to
ExtensionsMetricsProvider. Notably, changes the class to take in the
MetricsStateManager and later obtain the client ID from the manager rather than
directly taking in the ID; this change is necessary because the provider
instance is now constructed before the ID is determined.

BUG=374225
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=272633

[blundell, 2014-05-21 11:02:36]

https://codereview.chromium.org/297483008/diff/1/chrome/browser/metrics/exten...
File chrome/browser/metrics/extensions_metrics_provider.cc (right):

https://codereview.chromium.org/297483008/diff/1/chrome/browser/metrics/exten...
chrome/browser/metrics/extensions_metrics_provider.cc:92: return
metrics::MetricsLogBase::Hash(metrics_state_manager_->client_id());
Do you think it's worth having MetricsLogBase expose a method GetClientIDAsInt()
to avoid duplicating the implementation?

[Alexei Svitkine (slow), 2014-05-21 12:23:21]

https://codereview.chromium.org/297483008/diff/1/chrome/browser/metrics/exten...
File chrome/browser/metrics/extensions_metrics_provider.cc (right):

https://codereview.chromium.org/297483008/diff/1/chrome/browser/metrics/exten...
chrome/browser/metrics/extensions_metrics_provider.cc:92: return
metrics::MetricsLogBase::Hash(metrics_state_manager_->client_id());
On 2014/05/21 11:02:37, blundell wrote:
> Do you think it's worth having MetricsLogBase expose a method
GetClientIDAsInt()
> to avoid duplicating the implementation?

I'm OK with punting this for now and putting a TODO about it here.

https://codereview.chromium.org/297483008/diff/1/chrome/browser/metrics/exten...
chrome/browser/metrics/extensions_metrics_provider.cc:101: const int kClientKey
= GetClientID() % kExtensionListClientKeys;
Nit: kNamingConvention actually shouldn't be used for things that are dynamic.

https://codereview.chromium.org/297483008/diff/40001/chrome/browser/metrics/e...
File chrome/browser/metrics/extensions_metrics_provider.cc (right):

https://codereview.chromium.org/297483008/diff/40001/chrome/browser/metrics/e...
chrome/browser/metrics/extensions_metrics_provider.cc:37: }
Nit: Add DCHECK(metrics_state_manager_);

https://codereview.chromium.org/297483008/diff/40001/chrome/browser/metrics/e...
File chrome/browser/metrics/extensions_metrics_provider.h (right):

https://codereview.chromium.org/297483008/diff/40001/chrome/browser/metrics/e...
chrome/browser/metrics/extensions_metrics_provider.h:30: explicit
ExtensionsMetricsProvider(
Nit: Add a comment about |metrics_state_manager| ownership.

[blundell, 2014-05-21 13:03:29]

https://codereview.chromium.org/297483008/diff/1/chrome/browser/metrics/exten...
File chrome/browser/metrics/extensions_metrics_provider.cc (right):

https://codereview.chromium.org/297483008/diff/1/chrome/browser/metrics/exten...
chrome/browser/metrics/extensions_metrics_provider.cc:92: return
metrics::MetricsLogBase::Hash(metrics_state_manager_->client_id());
On 2014/05/21 12:23:21, Alexei Svitkine wrote:
> On 2014/05/21 11:02:37, blundell wrote:
> > Do you think it's worth having MetricsLogBase expose a method
> GetClientIDAsInt()
> > to avoid duplicating the implementation?
> 
> I'm OK with punting this for now and putting a TODO about it here.

Done.

https://codereview.chromium.org/297483008/diff/1/chrome/browser/metrics/exten...
chrome/browser/metrics/extensions_metrics_provider.cc:101: const int kClientKey
= GetClientID() % kExtensionListClientKeys;
On 2014/05/21 12:23:21, Alexei Svitkine wrote:
> Nit: kNamingConvention actually shouldn't be used for things that are dynamic.

Done.

https://codereview.chromium.org/297483008/diff/40001/chrome/browser/metrics/e...
File chrome/browser/metrics/extensions_metrics_provider.cc (right):

https://codereview.chromium.org/297483008/diff/40001/chrome/browser/metrics/e...
chrome/browser/metrics/extensions_metrics_provider.cc:37: }
On 2014/05/21 12:23:22, Alexei Svitkine wrote:
> Nit: Add DCHECK(metrics_state_manager_);

Done.

https://codereview.chromium.org/297483008/diff/40001/chrome/browser/metrics/e...
File chrome/browser/metrics/extensions_metrics_provider.h (right):

https://codereview.chromium.org/297483008/diff/40001/chrome/browser/metrics/e...
chrome/browser/metrics/extensions_metrics_provider.h:30: explicit
ExtensionsMetricsProvider(
On 2014/05/21 12:23:22, Alexei Svitkine wrote:
> Nit: Add a comment about |metrics_state_manager| ownership.

Done.

[Alexei Svitkine (slow), 2014-05-21 13:06:14]

LGTM!

[blundell, 2014-05-23 12:22:09]

The CQ bit was checked by blundell@chromium.org

[commit-bot: I haz the power, 2014-05-23 12:22:47]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/blundell@chromium.org/297483008/80001

[commit-bot: I haz the power, 2014-05-23 17:12:10]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-23 17:50:49]

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

[commit-bot: I haz the power, 2014-05-23 17:50:49]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[blundell, 2014-05-23 20:11:49]

The CQ bit was checked by blundell@chromium.org

[commit-bot: I haz the power, 2014-05-23 20:13:34]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/blundell@chromium.org/297483008/80001

[commit-bot: I haz the power, 2014-05-23 23:28:15]

Change committed as 272633