Chrome power profiler service

content/browser/power_profiler/power_profiler_service_unittest.cc

+// Copyright 2014 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 "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "content/browser/browser_thread_impl.h"
+#include "content/browser/power_profiler/power_profiler_service.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace content {
+
+namespace {
+
+const int kEvents = 3;

[jeremy, 2014/02/16 14:09:19]

kNumEvents, kNumObservers ?

[Pan, 2014/02/17 03:17:15]

Done.

+const int kObservers = 3;
+
+class TestPowerDataProvider : public PowerDataProvider {
+ public:
+  TestPowerDataProvider(int count) : event_number_(count) {}
+  ~TestPowerDataProvider() {}
+
+  virtual PowerEventVector GetData() OVERRIDE {
+    PowerEventVector events;
+    if (event_number_ == 0)
+      return events;
+
+    PowerEvent event;
+    event.type = PowerEvent::SOC_PACKAGE;
+    event.time = base::TimeTicks::Now();
+    event.value = 1.0;
+    events.push_back(event);
+
+    event_number_--;
+    return events;
+  }
+
+ private:
+  int event_number_;
+  DISALLOW_COPY_AND_ASSIGN(TestPowerDataProvider);
+};
+
+class TestPowerProfilerObserver : public PowerProfilerObserver {
+ public:
+  TestPowerProfilerObserver() : valid_event_count_(0) {}
+  ~TestPowerProfilerObserver() {}
+
+  virtual void OnPowerEvent(const PowerEventVector& events) OVERRIDE {
+    if (IsValidEvent(events[0]))
+      ++valid_event_count_;
+  }
+
+  int valid_event_count() const { return valid_event_count_; }
+
+ private:
+  bool IsValidEvent(const PowerEvent& event) {
+    return event.type == PowerEvent::SOC_PACKAGE &&
+           !event.time.is_null() &&
+           event.value > 0;
+  }
+
+  int valid_event_count_;
+  DISALLOW_COPY_AND_ASSIGN(TestPowerProfilerObserver);
+};
+
+} // namespace
+
+class PowerProfilerServiceTest : public testing::Test {
+ public:
+  void ServiceStartTest() {
+    service_ = new PowerProfilerService(
+        make_scoped_ptr<PowerDataProvider>(new TestPowerDataProvider(kEvents)),
+        message_loop_.message_loop_proxy(),
+        base::TimeDelta::FromMilliseconds(5));
+    EXPECT_TRUE(service_->IsAvailable());
+  }
+
+  void AddObserverTest() {
+    for (int index = 0; index < kObservers; ++index)
+      service_->AddObserver(&observers_[index]);
+
+    // No one received PowerEvent now.

[jeremy, 2014/02/16 14:09:19]

// No PowerEvents received.

[Pan, 2014/02/17 03:17:15]

Done.

+    for (int index = 0; index < kObservers; ++index)
+      EXPECT_EQ(observers_[0].valid_event_count(), 0);
+  }
+
+  void RemoveObserverTest() {
+    for (int index = 0; index < kObservers; ++index)
+      service_->RemoveObserver(&observers_[index]);
+
+    // Everyone received |kEvents| events.
+    for (int index = 0; index < kObservers; ++index)
+      EXPECT_EQ(observers_[index].valid_event_count(), kEvents);
+  }
+
+  void RunLoopForDelay(base::TimeDelta delay) {
+    message_loop_.PostDelayedTask(
+        FROM_HERE, base::MessageLoop::QuitClosure(), delay);
+    base::RunLoop().Run();
+  }
+
+ protected:
+  PowerProfilerServiceTest() : ui_thread_(BrowserThread::UI, &message_loop_) {}
+  virtual ~PowerProfilerServiceTest() {}
+
+ private:
+  PowerProfilerService* service_;
+  TestPowerProfilerObserver observers_[kObservers];
+
+  // UI thread
+  base::MessageLoopForUI message_loop_;
+  BrowserThreadImpl ui_thread_;
+
+  DISALLOW_COPY_AND_ASSIGN(PowerProfilerServiceTest);
+};
+
+TEST_F(PowerProfilerServiceTest, AvailableService) {

[jeremy, 2014/02/16 14:09:19]

Could you add a comment on what this is supposed to test.

[Pan, 2014/02/17 03:17:15]

Done.

+  ServiceStartTest();
+  AddObserverTest();
+  RunLoopForDelay(base::TimeDelta::FromMilliseconds(20));

[jeremy, 2014/02/16 14:09:19]

Is there a better way to wait here?  Using a timeout can be flaky?...

[Pan, 2014/02/17 03:17:15]

All run in a single thread, I don't see a better way here,
we can make the wait time long enough for all events dispatched, and add a
comment here.

[jeremy, 2014/02/17 06:31:13]

Flaky tests are a major problem, the pattern of waiting for a certain timeout is
not a good one.

We know how many events the observer is expected to see so can you just spin the
message loop until it receives that number of events and then terminate the
current loop.

if that doesn't work please reach out to phajdan.jr@chromium.org who knows a lot
about writing stable tests, he can point you in the right direction.

Whatever he thinks is best I'm fine with too...

+  RemoveObserverTest();
+}
+
+}  // namespace content