Reduce plugin_metrics_provider_ usage in MetricsService

chrome/browser/metrics/metrics_service.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 //------------------------------------------------------------------------------
 // Description of the life cycle of a instance of MetricsService.
 //
 //  OVERVIEW
 //
 // A MetricsService instance is typically created at application startup.  It is
@@ skipping 1373 matching lines @@
 
   // Redundant setting to assure that we always reset this value at shutdown
   // (and that we don't use some alternate path, and not call LogCleanShutdown).
   clean_shutdown_status_ = CLEANLY_SHUTDOWN;
 
   RecordBooleanPrefValue(prefs::kStabilityExitedCleanly, true);
   local_state_->SetInteger(prefs::kStabilityExecutionPhase,
                            MetricsService::SHUTDOWN_COMPLETE);
 }
 
-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
-}
-
 bool MetricsService::ShouldLogEvents() {
   // We simply don't log events to UMA if there is a single incognito
   // session visible. The problem is that we always notify using the orginal
   // profile in order to simplify notification processing.
   return !client_->IsOffTheRecordSessionActive();
 }
 
 void MetricsService::RecordBooleanPrefValue(const char* path, bool value) {
   DCHECK(IsSingleThreaded());
   local_state_->SetBoolean(path, value);
   RecordCurrentState(local_state_);
 }
 
 void MetricsService::RecordCurrentState(PrefService* pref) {
   pref->SetInt64(prefs::kStabilityLastTimestampSec, Time::Now().ToTimeT());
 
-#if defined(ENABLE_PLUGINS)
-  plugin_metrics_provider_->RecordPluginChanges();
-#endif
+  for (size_t i = 0; i < metrics_providers_.size(); ++i)
+    metrics_providers_[i]->RecordCurrentState();

[Alexei Svitkine (slow), 2014/05/29 20:47:44]

I know this is what the current code does, but I'm really not convinced this is
the right thing to do.

It's pretty dubious to have sync the plugins list at arbitrary times to the
local state. Instead, I'd rather go with something that makes more sense - such
as record periodically whenever plugins change through a timer trigger within
the provider itself.

However, obviously this would be a functional change. If you'd really prefer
that we don't do functional changes in this refactoring, let's stick a TODO to
remove this method with a corresponding crbug. Then we can follow-up on this
later. (If we go this way, I don't mind owning it - you can assign the bug to
me.)

[blundell, 2014/05/30 14:42:10]

I think you're better suited to make that change. Bug created, TODO filed.

 }