Add DeviceSettingProvider unit tests.

chrome/browser/metrics/metrics_service.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 file defines a service that collects information about the user
 // experience in order to help improve future versions of the app.
 
 #ifndef CHROME_BROWSER_METRICS_METRICS_SERVICE_H_
 #define CHROME_BROWSER_METRICS_METRICS_SERVICE_H_
 #pragma once
@@ skipping 25 matching lines @@
 class MetricsReportingScheduler;
 class PrefService;
 class Profile;
 class TemplateURLService;
 
 namespace base {
 class DictionaryValue;
 class MessageLoopProxy;
 }
 
+namespace chromeos {
+class DeviceSettingsProviderTest;
+}
+
 namespace content {
 class RenderProcessHost;
 }
 
 namespace prerender {
 bool IsOmniboxEnabled(Profile* profile);
 }
 
 namespace webkit {
 struct WebPluginInfo;
@@ skipping 70 matching lines @@
   bool recording_active() const;
   bool reporting_active() const;
 
   // Redundant test to ensure that we are notified of a clean exit.
   // This value should be true when process has completed shutdown.
   static bool UmaMetricsProperlyShutdown();
 
   // Set the dirty flag, which will require a later call to LogCleanShutdown().
   static void LogNeedForCleanShutdown();
 
+ protected:
+  // Constructor used in tests.
+  explicit MetricsService(bool active);
+
+  friend class chromeos::DeviceSettingsProviderTest;

[jar (doing other things), 2012/03/16 16:29:16]

I'm always leery about adding friendship to classes. It makes the class harder
to read, as there is always a fear that distant code will manipulate innards in
unknown ways.  When possible, it is always nicer to have a limited interface
that server the needs of this would-be new-friend. 

It is helpful that the class has the word "Test" in it, which generally assures
me that the tinkering won't happen in production, but it also means that if a
person modifies the MetricsService class, they must be aware of any/all accesses
made by a second class through undocumented interfaces :-(.

If you can clarify the limited set of interfaces and mutations you'd need for
the other class, that would be IMO much cleaner (more readable, maintainable,
etc.)

[pastarmovj, 2012/03/21 15:23:27]

I decided to split this CL and get all MetricsService related changes in another
one so this is reverted. 
I was anyhow planning to do some refactoring there like adding a virtual base
class to server as a base for mocking the MetricsService in tests. Also the
utility functions that are now used to turn on and off the metrics reporting
seem to be a little chaotic and I remember there were users directly accessing
the MetricsService while other user the OptionsUtil class. So I will consolidate
those as well if possible in this other CL. Might take a while till I get to
this task though.

+
  private:
   // The MetricsService has a lifecycle that is stored as a state.
   // See metrics_service.cc for description of this lifecycle.
   enum State {
     INITIALIZED,            // Constructor was called.
     INIT_TASK_SCHEDULED,    // Waiting for deferred init tasks to complete.
     INIT_TASK_DONE,         // Waiting for timer to send initial log.
     INITIAL_LOG_READY,      // Initial log generated, and waiting for reply.
     SENDING_OLD_LOGS,       // Sending unsent logs from previous session.
     SENDING_CURRENT_LOGS,   // Sending standard current logs as they acrue.
@@ skipping 290 matching lines @@
   friend bool prerender::IsOmniboxEnabled(Profile* profile);
   friend class ManifestFetchesBuilder;
 
   // Returns true if prefs::kMetricsReportingEnabled is set.
   static bool IsMetricsReportingEnabled();
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(MetricsServiceHelper);
 };
 
 #endif  // CHROME_BROWSER_METRICS_METRICS_SERVICE_H_