Revert of Reduce plugin_metrics_provider_ usage in MetricsService...

Revert of Reduce plugin_metrics_provider_ usage in MetricsService (https://codereview.chromium.org/308433004/)

Reason for revert:
This CL seems to cause bot failure on CI:
http://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%20LSan%20Tests%20%281%29/builds/2745

Original issue's description:
> Reduce plugin_metrics_provider_ usage in MetricsService
> 
> This CL eliminates MetricsService's call to
> PluginMetricsProvider::RecordPluginChanges (replaced by a new API on
> MetricsProvider). It additionally adds a
> MetricsServicesManager::OnPluginLoadingError() API and has the plugin observer
> call that API rather than calling MetricsService directly. This change will
> enable easily moving MetricsService::LogPluginLoadingError() to
> ChromeMetricsServiceClient once the latter is the class that keeps a weak
> pointer to the plugin metrics provider.
> 
> BUG=375776
> 
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=274403

TBR=asvitkine@chromium.org,isherman@chromium.org,jochen@chromium.org,blundell@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=375776

Review URL: https://codereview.chromium.org/314583002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@274418 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/browser_process.h

@@ -29,7 +29,6 @@ class IntranetRedirectDetector;
 class IOThread;
 class MediaFileSystemRegistry;
 class MetricsService;
-class MetricsServicesManager;
 class NetworkTimeTracker;
 class NotificationUIManager;
 class PrefRegistrySimple;
@@ -103,10 +102,6 @@ class BrowserProcess {
   // continue shutdown.
   virtual void EndSession() = 0;
 
-  // Gets the manager for the various metrics-related services, constructing it
-  // if necessary.
-  virtual MetricsServicesManager* GetMetricsServicesManager() = 0;
-
   // Services: any of these getters may return NULL
   virtual MetricsService* metrics_service() = 0;
   virtual rappor::RapporService* rappor_service() = 0;

chrome/browser/browser_process_impl.cc

@@ -423,13 +423,6 @@ void BrowserProcessImpl::EndSession() {
 #endif
 }
 
-MetricsServicesManager* BrowserProcessImpl::GetMetricsServicesManager() {
-  DCHECK(CalledOnValidThread());
-  if (!metrics_services_manager_)
-    metrics_services_manager_.reset(new MetricsServicesManager(local_state()));
-  return metrics_services_manager_.get();
-}
-
 MetricsService* BrowserProcessImpl::metrics_service() {
   DCHECK(CalledOnValidThread());
   return GetMetricsServicesManager()->GetMetricsService();
@@ -993,6 +986,13 @@ void BrowserProcessImpl::CreateSafeBrowsingService() {
 #endif
 }
 
+MetricsServicesManager* BrowserProcessImpl::GetMetricsServicesManager() {
+  DCHECK(CalledOnValidThread());
+  if (!metrics_services_manager_)
+    metrics_services_manager_.reset(new MetricsServicesManager(local_state()));
+  return metrics_services_manager_.get();
+}
+
 void BrowserProcessImpl::ApplyDefaultBrowserPolicy() {
   if (local_state()->GetBoolean(prefs::kDefaultBrowserSettingEnabled)) {
     scoped_refptr<ShellIntegration::DefaultWebClientWorker>

chrome/browser/browser_process_impl.h

@@ -73,7 +73,6 @@ class BrowserProcessImpl : public BrowserProcess,
   // BrowserProcess implementation.
   virtual void ResourceDispatcherHostCreated() OVERRIDE;
   virtual void EndSession() OVERRIDE;
-  virtual MetricsServicesManager* GetMetricsServicesManager() OVERRIDE;
   virtual MetricsService* metrics_service() OVERRIDE;
   virtual rappor::RapporService* rappor_service() OVERRIDE;
   virtual IOThread* io_thread() OVERRIDE;
@@ -153,6 +152,8 @@ class BrowserProcessImpl : public BrowserProcess,
   void CreateStatusTray();
   void CreateBackgroundModeManager();
 
+  MetricsServicesManager* GetMetricsServicesManager();
+
   void ApplyAllowCrossOriginAuthPromptPolicy();
   void ApplyDefaultBrowserPolicy();
   void ApplyMetricsReportingPolicy();

chrome/browser/metrics/metrics_service.cc

@@ -1313,6 +1313,7 @@ void MetricsService::LogCleanShutdown() {
 
 void MetricsService::LogPluginLoadingError(const base::FilePath& plugin_path) {
 #if defined(ENABLE_PLUGINS)
+  // TODO(asvitkine): Move this out of here.
   plugin_metrics_provider_->LogPluginLoadingError(plugin_path);
 #endif
 }
@@ -1333,6 +1334,7 @@ void MetricsService::RecordBooleanPrefValue(const char* path, bool value) {
 void MetricsService::RecordCurrentState(PrefService* pref) {
   pref->SetInt64(prefs::kStabilityLastTimestampSec, Time::Now().ToTimeT());
 
-  for (size_t i = 0; i < metrics_providers_.size(); ++i)
-    metrics_providers_[i]->RecordCurrentState();
+#if defined(ENABLE_PLUGINS)
+  plugin_metrics_provider_->RecordPluginChanges();
+#endif
 }

chrome/browser/metrics/metrics_service.h

@@ -206,7 +206,6 @@ class MetricsService
   bool recording_active() const;
   bool reporting_active() const;
 
-  // TODO(blundell): Move this to ChromeMetricsServiceClient.
   void LogPluginLoadingError(const base::FilePath& plugin_path);
 
   // Redundant test to ensure that we are notified of a clean exit.

chrome/browser/metrics/metrics_services_manager.cc

@@ -58,11 +58,6 @@ MetricsServicesManager::GetVariationsService() {
   return variations_service_.get();
 }
 
-void MetricsServicesManager::OnPluginLoadingError(
-    const base::FilePath& plugin_path) {
-  GetMetricsService()->LogPluginLoadingError(plugin_path);
-}
-
 metrics::MetricsStateManager* MetricsServicesManager::GetMetricsStateManager() {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (!metrics_state_manager_) {

chrome/browser/metrics/metrics_services_manager.h

@@ -13,10 +13,6 @@ class ChromeMetricsServiceClient;
 class MetricsService;
 class PrefService;
 
-namespace base {
-class FilePath;
-}
-
 namespace metrics {
 class MetricsStateManager;
 }
@@ -47,9 +43,6 @@ class MetricsServicesManager {
   // Returns the VariationsService, creating it if it hasn't been created yet.
   chrome_variations::VariationsService* GetVariationsService();
 
-  // Should be called when a plugin loading error occurs.
-  void OnPluginLoadingError(const base::FilePath& plugin_path);
-
  private:
   metrics::MetricsStateManager* GetMetricsStateManager();
 

chrome/browser/metrics/plugin_metrics_provider.cc

@@ -191,9 +191,7 @@ void PluginMetricsProvider::ProvideStabilityMetrics(
   local_state_->ClearPref(prefs::kStabilityPluginStats);
 }
 
-// Saves plugin-related updates from the in-object buffer to Local State
-// for retrieval next time we send a Profile log (generally next launch).
-void PluginMetricsProvider::RecordCurrentState() {
+void PluginMetricsProvider::RecordPluginChanges() {
   ListPrefUpdate update(local_state_, prefs::kStabilityPluginStats);
   base::ListValue* plugins = update.Get();
   DCHECK(plugins);

chrome/browser/metrics/plugin_metrics_provider.h

@@ -42,7 +42,10 @@ class PluginMetricsProvider : public metrics::MetricsProvider,
       metrics::SystemProfileProto* system_profile_proto) OVERRIDE;
   virtual void ProvideStabilityMetrics(
       metrics::SystemProfileProto* system_profile_proto) OVERRIDE;
-  virtual void RecordCurrentState() OVERRIDE;
+
+  // Saves plugin-related updates from the in-object buffer to Local State
+  // for retrieval next time we send a Profile log (generally next launch).
+  void RecordPluginChanges();
 
   // Notifies the provider about an error loading the plugin at |plugin_path|.
   void LogPluginLoadingError(const base::FilePath& plugin_path);

chrome/browser/plugins/plugin_observer.cc

@@ -15,7 +15,7 @@
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/infobars/simple_alert_infobar_delegate.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
-#include "chrome/browser/metrics/metrics_services_manager.h"
+#include "chrome/browser/metrics/metrics_service.h"
 #include "chrome/browser/plugins/plugin_finder.h"
 #include "chrome/browser/plugins/plugin_infobar_delegates.h"
 #include "chrome/browser/profiles/profile.h"
@@ -458,8 +458,7 @@ void PluginObserver::OnOpenAboutPlugins() {
 }
 
 void PluginObserver::OnCouldNotLoadPlugin(const base::FilePath& plugin_path) {
-  g_browser_process->GetMetricsServicesManager()->OnPluginLoadingError(
-      plugin_path);
+  g_browser_process->metrics_service()->LogPluginLoadingError(plugin_path);
   base::string16 plugin_name =
       PluginService::GetInstance()->GetPluginDisplayNameByPath(plugin_path);
   SimpleAlertInfoBarDelegate::Create(

chrome/test/base/testing_browser_process.cc

@@ -96,10 +96,6 @@ void TestingBrowserProcess::ResourceDispatcherHostCreated() {
 void TestingBrowserProcess::EndSession() {
 }
 
-MetricsServicesManager* TestingBrowserProcess::GetMetricsServicesManager() {
-  return NULL;
-}
-
 MetricsService* TestingBrowserProcess::metrics_service() {
   return NULL;
 }

chrome/test/base/testing_browser_process.h

@@ -56,7 +56,6 @@ class TestingBrowserProcess : public BrowserProcess {
 
   virtual void ResourceDispatcherHostCreated() OVERRIDE;
   virtual void EndSession() OVERRIDE;
-  virtual MetricsServicesManager* GetMetricsServicesManager() OVERRIDE;
   virtual MetricsService* metrics_service() OVERRIDE;
   virtual rappor::RapporService* rappor_service() OVERRIDE;
   virtual IOThread* io_thread() OVERRIDE;

components/metrics/metrics_provider.h

@@ -44,9 +44,6 @@ class MetricsProvider {
   virtual void ProvideGeneralMetrics(
       ChromeUserMetricsExtension* uma_proto) {}
 
-  // TODO(asvitkine): Remove this method. http://crbug.com/379148
-  virtual void RecordCurrentState() {}
-
  private:
   DISALLOW_COPY_AND_ASSIGN(MetricsProvider);
 };