Log operator code histogram on new metric log

chrome/browser/metrics/metrics_service_unittest.cc

Index: chrome/browser/metrics/metrics_service_unittest.cc
diff --git a/chrome/browser/metrics/metrics_service_unittest.cc b/chrome/browser/metrics/metrics_service_unittest.cc
index b42d789e29cf3cdf5092ba29b71b8f43257e68ce..583619c77c5dcfcce3f6c8ac134ab679fc9cadf9 100644
--- a/chrome/browser/metrics/metrics_service_unittest.cc
+++ b/chrome/browser/metrics/metrics_service_unittest.cc
@@ -135,6 +135,20 @@ class MetricsServiceTest : public testing::Test {
   DISALLOW_COPY_AND_ASSIGN(MetricsServiceTest);
 };
 
+class TestMetricsLogObserver : public MetricsLogObserver {
+ public:
+  TestMetricsLogObserver(): observed_(0) {}
+  virtual ~TestMetricsLogObserver() {}
+
+  virtual void OnNewMetricsLog() OVERRIDE {
+    observed_++;

[Ilya Sherman, 2014/05/07 01:03:16]

nit: Pre-increment.

[bolian, 2014/05/07 02:20:26]

Done.

+  }
+
+  int observed_;

[Ilya Sherman, 2014/05/07 01:03:16]

nit: Please leave a space after this.

[Ilya Sherman, 2014/05/07 01:03:16]

nit: Please make the variable private, and add a getter.

[bolian, 2014/05/07 02:20:26]

Done.

[bolian, 2014/05/07 02:20:26]

Done.

+ private:
+  DISALLOW_COPY_AND_ASSIGN(TestMetricsLogObserver);
+};
+
 }  // namespace
 
 TEST_F(MetricsServiceTest, IsPluginProcess) {
@@ -304,3 +318,25 @@ TEST_F(MetricsServiceTest, CrashReportingEnabled) {
   EXPECT_FALSE(MetricsServiceHelper::IsCrashReportingEnabled());
 #endif  // defined(GOOGLE_CHROME_BUILD)
 }
+
+TEST_F(MetricsServiceTest, MetricsLogObserver) {
+  MetricsService service;
+  TestMetricsLogObserver observer;
+
+  service.AddMetricsLogObserver(&observer);
+  EXPECT_EQ(0, observer.observed_);
+
+  service.OpenNewLog();
+  EXPECT_EQ(1, observer.observed_);
+  service.log_manager_.FinishCurrentLog();
+
+  service.OpenNewLog();
+  EXPECT_EQ(2, observer.observed_);
+  service.log_manager_.FinishCurrentLog();
+
+  service.RemoveMetricsLogObserver(&observer);
+
+  service.OpenNewLog();
+  EXPECT_EQ(2, observer.observed_);
+  service.log_manager_.FinishCurrentLog();
+}

[Ilya Sherman, 2014/05/07 01:03:16]

Could you please also add test coverage for the other two places where the
observer gets notified?

[bolian, 2014/05/07 02:20:26]

The other two places require non-trivial amount of plumbing to prepare the
data/environment so that they don't crash, seems an overkill for this test.