New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 742508 link

Starred by 1 user
Status: Untriaged
Owner: ----
Cc:
skare@chromium.org
isherman@chromium.org
asvitk...@chromium.org
█ rkaplow@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

MetricsServiceClient & MetricsProvider interfaces should be narrowed.

Project Member Reported by holte@chromium.org, Jul 13 2017 
MetricsServiceClient & MetricsProvider currently have interfaces that are more complex than they could be, because they are oriented around getting individual specific from the context layer.  Many of the functions could generalized/consolidated.

Comment 1 by holte@chromium.org, Jul 13 2017

Components: Internals>Metrics
Labels: -Type-Bug Type-Task
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 24 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/631803df7045e1883d7fd6fed53f892fd82987af

commit 631803df7045e1883d7fd6fed53f892fd82987af
Author: Steven Holte <holte@google.com>
Date: Mon Jul 24 23:40:34 2017

Clarify MetricsProvider's consumed interface.

Add ProvidePreviousSessionData and ProvideCurrentSessionData methods,
to replace ProvideInitialStabilityMetrics, ProvideStabilityMetrics, and
ProvideGeneralMetrics.

Change-Id: I9097c0f095da11accbf6c1927f0edc39125829e0
Bug: 742508
Reviewed-on: https://chromium-review.googlesource.com/569402
Commit-Queue: Steven Holte <holte@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489141}
[modify] https://crrev.com/631803df7045e1883d7fd6fed53f892fd82987af/components/metrics/metrics_log.cc
[modify] https://crrev.com/631803df7045e1883d7fd6fed53f892fd82987af/components/metrics/metrics_log.h
[modify] https://crrev.com/631803df7045e1883d7fd6fed53f892fd82987af/components/metrics/metrics_log_unittest.cc
[modify] https://crrev.com/631803df7045e1883d7fd6fed53f892fd82987af/components/metrics/metrics_provider.cc
[modify] https://crrev.com/631803df7045e1883d7fd6fed53f892fd82987af/components/metrics/metrics_provider.h
[modify] https://crrev.com/631803df7045e1883d7fd6fed53f892fd82987af/components/metrics/metrics_service.cc
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 26 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f

commit 141462acc1f4a9d4fd0208ae6c04b1ec5210d16f
Author: Steven Holte <holte@google.com>
Date: Wed Jul 26 01:35:07 2017

Migrate MetricsProviders to new Provide functions.

ProvideInitialStabilityMetrics and ProvideGeneralMetrics are migrated
to ProvidePreviousSessionData and ProvideCurrentSessionData.

HasInitialStabilityMetrics renamed to HasPreviousSessionData.

Providers which only use ProvideStabilityMetrics are left unchanged,
but ones which use it with the others are also updated.

TBR=olivierrobin,andrewhayden,vakh,mpearson,skym,msarda

Bug: 742508
Change-Id: I3885e508ced837d3ef71511361aad209fb72a904
Reviewed-on: https://chromium-review.googlesource.com/583566
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Commit-Queue: Steven Holte <holte@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489517}
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/chrome/browser/metrics/android_metrics_provider.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/chrome/browser/metrics/android_metrics_provider.h
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/chrome/browser/metrics/chromeos_metrics_provider.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/chrome/browser/metrics/chromeos_metrics_provider.h
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/chrome/browser/metrics/https_engagement_metrics_provider.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/chrome/browser/metrics/https_engagement_metrics_provider.h
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/chrome/browser/page_load_metrics/observers/https_engagement_metrics/https_engagement_page_load_metrics_observer_browsertest.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/chrome/browser/safe_browsing/certificate_reporting_metrics_provider.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/chrome/browser/safe_browsing/certificate_reporting_metrics_provider.h
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/chrome/browser/signin/chrome_signin_status_metrics_provider_delegate.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/chrome/browser/signin/signin_status_metrics_provider_chromeos.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/chrome/browser/signin/signin_status_metrics_provider_chromeos.h
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/chrome/browser/translate/translate_ranker_metrics_provider.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/chrome/browser/translate/translate_ranker_metrics_provider.h
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/metrics/call_stack_profile_metrics_provider.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/metrics/call_stack_profile_metrics_provider.h
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/metrics/call_stack_profile_metrics_provider_unittest.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/metrics/file_metrics_provider.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/metrics/file_metrics_provider.h
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/metrics/file_metrics_provider_unittest.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/metrics/metrics_provider.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/metrics/metrics_provider.h
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/metrics/metrics_service.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/metrics/metrics_service_unittest.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/metrics/net/network_metrics_provider.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/metrics/net/network_metrics_provider.h
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/metrics/profiler/profiler_metrics_provider.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/metrics/profiler/profiler_metrics_provider.h
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/metrics/profiler/profiler_metrics_provider_unittest.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/metrics/test_metrics_provider.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/metrics/test_metrics_provider.h
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/omnibox/browser/omnibox_metrics_provider.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/omnibox/browser/omnibox_metrics_provider.h
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/signin/core/browser/signin_status_metrics_provider.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/signin/core/browser/signin_status_metrics_provider.h
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/sync/device_info/device_count_metrics_provider.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/sync/device_info/device_count_metrics_provider.h
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/components/sync/device_info/device_count_metrics_provider_unittest.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/ios/chrome/browser/metrics/mobile_session_shutdown_metrics_provider.h
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/ios/chrome/browser/metrics/mobile_session_shutdown_metrics_provider.mm
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/ios/chrome/browser/metrics/mobile_session_shutdown_metrics_provider_unittest.mm
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/ios/chrome/browser/translate/translate_ranker_metrics_provider.cc
[modify] https://crrev.com/141462acc1f4a9d4fd0208ae6c04b1ec5210d16f/ios/chrome/browser/translate/translate_ranker_metrics_provider.h
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 28 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c35411dc4e5a49f3297cfdbbbb33021e9c9213fe

commit c35411dc4e5a49f3297cfdbbbb33021e9c9213fe
Author: Steven Holte <holte@google.com>
Date: Fri Jul 28 04:22:14 2017

Create DelegatingProvider to wrap multi MetricsProvider logic.

TBR=olivierrobin

Bug: 742508
Change-Id: I6afde50f9a7802a5d41aa267739271af16e4a5d5
Reviewed-on: https://chromium-review.googlesource.com/583943
Commit-Queue: Steven Holte <holte@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490259}
[modify] https://crrev.com/c35411dc4e5a49f3297cfdbbbb33021e9c9213fe/components/metrics/BUILD.gn
[add] https://crrev.com/c35411dc4e5a49f3297cfdbbbb33021e9c9213fe/components/metrics/delegating_provider.cc
[add] https://crrev.com/c35411dc4e5a49f3297cfdbbbb33021e9c9213fe/components/metrics/delegating_provider.h
[modify] https://crrev.com/c35411dc4e5a49f3297cfdbbbb33021e9c9213fe/components/metrics/metrics_log.cc
[modify] https://crrev.com/c35411dc4e5a49f3297cfdbbbb33021e9c9213fe/components/metrics/metrics_log.h
[modify] https://crrev.com/c35411dc4e5a49f3297cfdbbbb33021e9c9213fe/components/metrics/metrics_log_unittest.cc
[modify] https://crrev.com/c35411dc4e5a49f3297cfdbbbb33021e9c9213fe/components/metrics/metrics_service.cc
[modify] https://crrev.com/c35411dc4e5a49f3297cfdbbbb33021e9c9213fe/components/metrics/metrics_service.h
[modify] https://crrev.com/c35411dc4e5a49f3297cfdbbbb33021e9c9213fe/components/metrics/metrics_service_unittest.cc
[modify] https://crrev.com/c35411dc4e5a49f3297cfdbbbb33021e9c9213fe/components/ukm/ukm_service.cc
[modify] https://crrev.com/c35411dc4e5a49f3297cfdbbbb33021e9c9213fe/components/ukm/ukm_service.h
[modify] https://crrev.com/c35411dc4e5a49f3297cfdbbbb33021e9c9213fe/ios/chrome/today_extension/today_metrics_logger.mm

Comment 5 by mpear...@chromium.org, Dec 13 2017 

Is there anything left to do here?

Comment 6 by holte@chromium.org, Dec 13 2017 

Yes, for MetricsServiceClient, I think at least some of these can be replaced with a few metrics MetricsProviders, since they are mostly just used for populating SystemProfile

  virtual int32_t GetProduct() = 0;
  virtual std::string GetApplicationLocale() = 0;
  virtual bool GetBrand(std::string* brand_code) = 0;
  virtual SystemProfileProto::Channel GetChannel() = 0;
  virtual std::string GetVersionString() = 0;

  virtual void OnPluginLoadingError(const base::FilePath& plugin_path) {}
  virtual void OnRendererProcessCrash() {}
  virtual bool IsReportingPolicyManaged();
  virtual EnableMetricsDefault GetMetricsReportingDefaultState();
►

Sign in to add a comment