chrome power profiler chrome side changes

content/browser/power_profiler/power_profiler_host.h

+// Copyright 2013 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.
+
+#ifndef CONTENT_BROWSER_POWER_PROFILER_HOST_H_
+#define CONTENT_BROWSER_POWER_PROFILER_HOST_H_
+
+#include "base/basictypes.h"
+#include "base/memory/ref_counted.h"
+#include "content/browser/power_profiler/power_event.h"
+
+#if defined(OS_WIN)
+#define ENABLE_POWER_PROFILER 1

[qsr, 2014/01/17 11:50:14]

 Using pre-processor variable at this level seems brittle. Moreover, I do not
think the condition is the right one. Right now there is only one implementation
of the power profiler, but we want a generic API that will work on more
platform.

 As you do have a factory to create the service, you should have a boolean on
the service itself telling you if the power profiler is effective or not, and
use it instead of this compilation directive.

+#else
+#undef ENABLE_POWER_PROFILE
+#endif  // !OS_WIN
+
+namespace content {
+
+// A class used to monitor the power usage and tell profiler the power data
+class PowerProfilerHost : public base::RefCountedThreadSafe<PowerProfilerHost> {
+ public:
+  enum Resolution {
+    LOW,
+    NORMAL,
+    HIGH,
+    TYPE_COUNT // this should be always the last one
+  };
+
+  PowerProfilerHost();
+  ~PowerProfilerHost();
+
+  void UnRegister();

[qsr, 2014/01/17 11:50:14]

Can you add comments on all those methods. Moreover, use Unregister instead.

+  void SetResolution(Resolution);
+  Resolution GetResolution() const { return resolution_; }

[qsr, 2014/01/17 11:50:14]

Inlined getter should be resolution() { return resolution_; }

+
+  virtual bool Register();
+  virtual void Send(PowerEvent*, int count) {}
+
+ protected:
+  Resolution resolution_;

[qsr, 2014/01/17 11:50:14]

Why is this protected? You are setting this in the constructor of the sub class,
but you should be either using the setter, or have a parameter, and put this in
the private section.

+
+ private:
+  void RegisterWrapper();
+  void UnRegisterWrapper();
+  void SetResolutionWrapper();
+
+  DISALLOW_COPY_AND_ASSIGN(PowerProfilerHost);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_POWER_PROFILER_HOST_H_