Restoring the chrome variatioons client, with a fix for browser_test failures caused by it.

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 17 matching lines @@
 class IOThread;
 class MetricsService;
 class NotificationUIManager;
 class PrefService;
 class Profile;
 class ProfileManager;
 class SafeBrowsingService;
 class StatusTray;
 class TabCloseableStateWatcher;
 class ThumbnailGenerator;
+class VariationsService;
 class WatchDogThread;
 
 #if defined(OS_CHROMEOS)
 namespace chromeos {
 class OomPriorityManager;
 }
 #endif  // defined(OS_CHROMEOS)
 
 namespace net {
 class URLRequestContextGetter;
@@ skipping 37 matching lines @@
   // 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 ProfileManager* profile_manager() = 0;
   virtual PrefService* local_state() = 0;
   virtual ui::Clipboard* clipboard() = 0;
   virtual net::URLRequestContextGetter* system_request_context() = 0;
+  virtual VariationsService* variations_service() = 0;

[Ilya Sherman, 2012/05/15 22:11:17]

I still think this might be more appropriate as a member of the MetricsService
class.  I'm not sure if there's someone who'd qualify as an "owner" of the
BrowserProcess code, who could comment on this

[SteveT, 2012/05/16 14:33:49]

Does the functionality in VariationsService have much to do with the
functionality in MetricsService? In fact, I don't think MetricsService actually
uses the VS at all.

I agree with you that we should avoid hanging more stuff off the browser process
if possible, but is it worth introducing a new member to the class just for the
class to hold on to it, without actually utilizing it?

It seems like the VS has more to do with the FieldTrialList than MetricsService,
but I'm also reluctant to put it in there :)

Any thoughts on these questions?

[jwd, 2012/05/16 14:47:31]

FWIW putting it here was a recommendation from dominich@.

[Ilya Sherman, 2012/05/16 17:57:23]

Fair enough, I'm ok with keeping it here if that seems better than having it be
a member of the MetricsService -- I don't have any better suggestions on where
it might live.

[Ilya Sherman, 2012/05/16 17:57:23]

Agreed, the MetricsService does not use the VariationsService directly. 
However, it does have the same lifetime, and is generally what I think of as the
"central dispatcher" for all things related to metrics.  It's certainly not a
perfect fit; my main motivation was to avoid adding yet another dependency to
the core browser process files.
Yeah, if the FieldTrialList code wasn't in base/, that would almost certainly be
the right place to store this... but given that it *is* in base/, that wouldn't
really work.

 
 #if defined(OS_CHROMEOS)
   // Returns the out-of-memory priority manager.
   virtual chromeos::OomPriorityManager* oom_priority_manager() = 0;
 #endif  // defined(OS_CHROMEOS)
 
   virtual ExtensionEventRouterForwarder*
       extension_event_router_forwarder() = 0;
 
   // Returns the manager for desktop notifications.
@@ skipping 93 matching lines @@
 
   virtual CRLSetFetcher* crl_set_fetcher() = 0;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(BrowserProcess);
 };
 
 extern BrowserProcess* g_browser_process;
 
 #endif  // CHROME_BROWSER_BROWSER_PROCESS_H_