Reduce plugin_metrics_provider_ usage in MetricsService

chrome/browser/browser_process.h

 // 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.
 
 // This interface is for managing the global services of the application. Each
 // service is lazily created when requested the first time. The service getters
 // will return NULL if the service is not available, so callers must check for
 // this condition.
 
 #ifndef CHROME_BROWSER_BROWSER_PROCESS_H_
@@ skipping 13 matching lines @@
 class DownloadStatusUpdater;
 class GLStringManager;
 class GpuModeManager;
 class IconManager;
 class IntranetRedirectDetector;
 class IOThread;
 class MediaFileSystemRegistry;
 class MetricsService;
 class NetworkTimeTracker;
 class NotificationUIManager;
+class PluginMetricsProvider;
 class PrefRegistrySimple;
 class PrefService;
 class Profile;
 class ProfileManager;
 class SafeBrowsingService;
 class StatusTray;
 class WatchDogThread;
 #if defined(ENABLE_WEBRTC)
 class WebRtcLogUploader;
 #endif
@@ skipping 53 matching lines @@
   virtual void ResourceDispatcherHostCreated() = 0;
 
   // Invoked when the user is logging out/shutting down. When logging off we may
   // not have enough time to do a normal shutdown. This method is invoked prior
   // to normal shutdown and saves any state that must be saved before we are
   // continue shutdown.
   virtual void EndSession() = 0;
 
   // Services: any of these getters may return NULL
   virtual MetricsService* metrics_service() = 0;
+  virtual PluginMetricsProvider* plugin_metrics_provider() = 0;

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

I'd really like to hear Ilya's thoughts on this, but I'm not a big fan of
exposing the provider on g_browser_process.

I'd rather we expose MetricsServicesManager and for that have a method
"OnPluginLoadingError()" that the plugins code can call.

The reason I'd prefer the above is that I think we'll need to expose
MetricsServicesManager eventually anyway and it's cleaner to just have that
exposed than a bunch of smaller sub-parts (i.e. individual providers).

Then, PluginMetricsProvider would just be an implementation detail of the
metrics system and things wouldn't need to know about it.

Ilya, WDYT?

[Alexei Svitkine (slow), 2014/05/29 20:49:03]

(Just to clarify, my suggestion is to expose MetricsServicesManager and have a
method on that be plumbed through to the client and service.)

[Ilya Sherman, 2014/05/29 21:04:13]

Honestly, either approach is fine with me.  On the one hand, exposing just the
PluginMetricsProvider exposes a smaller API surface.  On the other hand, if
we're going to need to expose the MetricsServicesManager later on down the line
anyway, then there's not much harm to exposing it sooner rather than later.

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

Done.

   virtual rappor::RapporService* rappor_service() = 0;
   virtual ProfileManager* profile_manager() = 0;
   virtual PrefService* local_state() = 0;
   virtual net::URLRequestContextGetter* system_request_context() = 0;
   virtual chrome_variations::VariationsService* variations_service() = 0;
 
   virtual BrowserProcessPlatformPart* platform_part() = 0;
 
   virtual extensions::EventRouterForwarder*
       extension_event_router_forwarder() = 0;
@@ skipping 105 matching lines @@
 
   virtual NetworkTimeTracker* network_time_tracker() = 0;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(BrowserProcess);
 };
 
 extern BrowserProcess* g_browser_process;
 
 #endif  // CHROME_BROWSER_BROWSER_PROCESS_H_