Log operator code histogram on new metric log

chrome/browser/metrics/metrics_service_unittest.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.
 
 #include "chrome/browser/metrics/metrics_service.h"
 
 #include <string>
 
 #include "base/command_line.h"
 #include "base/threading/platform_thread.h"
@@ skipping 117 matching lines @@
     return false;
   }
 
  private:
   content::TestBrowserThreadBundle thread_bundle_;
   ScopedTestingLocalState testing_local_state_;
 
   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) {
   EXPECT_TRUE(
       MetricsService::IsPluginProcess(content::PROCESS_TYPE_PLUGIN));
   EXPECT_TRUE(
       MetricsService::IsPluginProcess(content::PROCESS_TYPE_PPAPI_PLUGIN));
   EXPECT_FALSE(
       MetricsService::IsPluginProcess(content::PROCESS_TYPE_GPU));
 }
@@ skipping 149 matching lines @@
   GetLocalState()->SetBoolean(crash_pref, true);
   EXPECT_TRUE(MetricsServiceHelper::IsCrashReportingEnabled());
   GetLocalState()->ClearPref(crash_pref);
   EXPECT_FALSE(MetricsServiceHelper::IsCrashReportingEnabled());
 #endif  // !defined(OS_CHROMEOS)
 #else  // defined(GOOGLE_CHROME_BUILD)
   // Chromium branded browsers never have crash reporting enabled.
   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.