Chrome power profiler service

content/browser/power_profiler/power_profiler_service.cc

Index: content/browser/power_profiler/power_profiler_service.cc
diff --git a/content/browser/power_profiler/power_profiler_service.cc b/content/browser/power_profiler/power_profiler_service.cc
new file mode 100644
index 0000000000000000000000000000000000000000..62c8e4780c4f90bad9d335e4ad8a27a1506a715f
--- /dev/null
+++ b/content/browser/power_profiler/power_profiler_service.cc
@@ -0,0 +1,116 @@
+// 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 "content/browser/power_profiler/power_profiler_service.h"
+
+#include "base/bind.h"
+#include "base/message_loop/message_loop.h"
+#include "base/threading/sequenced_worker_pool.h"
+#include "content/browser/power_profiler/power_data_provider_factory.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace {
+
+// Default sampling period, as recommended by Intel Power Gadget.
+// http://software.intel.com/en-us/blogs/2013/10/03/using-the-intel-power-gadget-api-on-windows

[jeremy, 2014/02/18 11:35:33]

Excellent, thanks! I might even say that it's section 3.1 of that URL but your
call...

[Pan, 2014/02/18 12:58:39]

Done.

+const int kDefaultSamplePeriodMS = 50;

[jeremy, 2014/02/18 11:35:33]

Sorry, this should end with Ms not MS, my mistake.

[Pan, 2014/02/18 12:58:39]

Done.

+
+}  // namespace
+
+namespace content {
+
+PowerProfilerService::PowerProfilerService()
+    : status_(UNINITIALIZED),
+      sample_period_(base::TimeDelta::FromMilliseconds(kDefaultSamplePeriodMS)),
+      data_provider_(PowerDataProviderFactory::Create()) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  if (data_provider_.get()) {

[jeremy, 2014/02/18 11:35:33]

I don't think you need this if() , that would mean that make_scoped_ptr() failed
and it appears we don't check that in other places in the code.

Looking at this further, do you really need the UNINITIALIZED status?

[qsr, 2014/02/18 11:38:09]

 This is in fact needed. The factory is not guaranteed to return a provider, if
it cannot create one for the current platform.

[Pan, 2014/02/18 12:58:39]

right, needed here.

+      status_ = INITIALIZED;
+      task_runner_ = BrowserThread::GetBlockingPool()->GetSequencedTaskRunner(
+          BrowserThread::GetBlockingPool()->GetSequenceToken());
+  }
+}
+
+PowerProfilerService::PowerProfilerService(
+    scoped_ptr<PowerDataProvider> provider,
+    scoped_refptr<base::TaskRunner> task_runner,
+    const base::TimeDelta& sample_period)
+    : task_runner_(task_runner),
+      status_(UNINITIALIZED),
+      sample_period_(sample_period),
+      data_provider_(provider.Pass()) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  if (data_provider_.get())
+      status_ = INITIALIZED;
+}
+
+PowerProfilerService::~PowerProfilerService() {
+}
+
+bool PowerProfilerService::IsAvailable() {
+  return status_ !=  UNINITIALIZED;
+}
+
+PowerProfilerService* PowerProfilerService::GetInstance() {
+  return Singleton<PowerProfilerService>::get();
+}
+
+void PowerProfilerService::AddObserver(PowerProfilerObserver* observer) {
+  if (status_ == UNINITIALIZED)
+    return;

[jeremy, 2014/02/18 11:35:33]

Should these ifs be DCHECKS or removed? My concern is with masking programming
errors...

[Pan, 2014/02/18 12:58:39]

status_ should be UNINTIALIZED if no provider.

+
+  observers_.AddObserver(observer);
+  if (status_ != PROFILING)
+    Start();
+}
+
+void PowerProfilerService::RemoveObserver(PowerProfilerObserver* observer) {
+  observers_.RemoveObserver(observer);
+
+  if (!observers_.might_have_observers())
+    Stop();
+}
+
+void PowerProfilerService::Start() {
+  DCHECK(status_ == INITIALIZED);
+  status_ = PROFILING;
+
+  // Send out power events immediately.
+  QueryData();
+
+  query_power_timer_.Start(FROM_HERE,
+      sample_period_, this, &PowerProfilerService::QueryData);
+}
+
+void PowerProfilerService::Stop() {
+  DCHECK(status_ == PROFILING);
+
+  query_power_timer_.Stop();
+  status_ = INITIALIZED;
+}
+
+void PowerProfilerService::QueryData() {
+  task_runner_->PostTask(
+      FROM_HERE, base::Bind(&PowerProfilerService::QueryDataOnTaskRunner,
+                            base::Unretained(this)));
+}
+
+void PowerProfilerService::Notify(const PowerEventVector& events) {
+  FOR_EACH_OBSERVER(PowerProfilerObserver, observers_, OnPowerEvent(events));
+}
+
+void PowerProfilerService::QueryDataOnTaskRunner() {
+  DCHECK(task_runner_->RunsTasksOnCurrentThread() && status_ == PROFILING);

[jeremy, 2014/02/18 11:35:33]

Would recommend breaking into 2 DCHECK() statements so if this fails we know
exactly what went wrong.

[Pan, 2014/02/18 12:58:39]

Done.

+
+  // Get data and notify.
+  PowerEventVector events = data_provider_->GetData();
+  if (events.size() != 0) {
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, base::Bind(
+        &PowerProfilerService::Notify, base::Unretained(this), events));
+  }
+}
+
+}  // namespace content