[PowerProfiler] Power Profiler service should detect the sampling rate from the data provider.

[PowerProfiler] Power Profiler service should detect the sampling rate from the data provider.

Present PowerProfilerService collects the power data from the PowerDataProvider at a fixed
sampling rate of 50ms.

This could be overwhelming for other software methods such as android power profiling,
estimation based methods (PowerTutor, AppScope) as running them at 50ms would drain more
power than being calculated.

Data provider should inform the PowerProfilerService about the sampling rate at which
it can operate optimally.

BUG=379702
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=275040

[vivekg, 2014-06-02 10:25:19]

PTAL, thanks!

[vivekg, 2014-06-03 05:07:15]

ping?

[qsr, 2014-06-03 09:08:15]

lgtm

[vivekg, 2014-06-03 09:27:36]

The CQ bit was checked by vivekg@chromium.org

[commit-bot: I haz the power, 2014-06-03 09:29:31]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vivek.vg@samsung.com/309803002/20001

[commit-bot: I haz the power, 2014-06-03 11:06:24]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/12170)

[commit-bot: I haz the power, 2014-06-03 11:09:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-03 11:09:23]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[vivekg, 2014-06-03 11:44:14]

+sky for OWNER review.

[sky, 2014-06-03 15:41:01]

https://codereview.chromium.org/309803002/diff/20001/content/browser/power_pr...
File content/browser/power_profiler/power_data_provider.h (right):

https://codereview.chromium.org/309803002/diff/20001/content/browser/power_pr...
content/browser/power_profiler/power_data_provider.h:29: virtual int64
SamplingRate() = 0;
How about:
virtual base::TimeDelta GetSamplingRate() const = 0;

This way you don't have to document it's in milliseconds as return value is self
documenting. And GetFoo is consistent with similar other code (say on line 26
above).

[vivekg, 2014-06-04 04:35:45]

Uploaded new patch with the review comments addressed.

https://chromiumcodereview.appspot.com/309803002/diff/20001/content/browser/p...
File content/browser/power_profiler/power_data_provider.h (right):

https://chromiumcodereview.appspot.com/309803002/diff/20001/content/browser/p...
content/browser/power_profiler/power_data_provider.h:29: virtual int64
SamplingRate() = 0;
On 2014/06/03 15:41:01, sky wrote:
> How about:
> virtual base::TimeDelta GetSamplingRate() const = 0;
> 
> This way you don't have to document it's in milliseconds as return value is
self
> documenting. And GetFoo is consistent with similar other code (say on line 26
> above).

Done. Yes it makes sense to return the TimeDelta instead of a number. Thanks for
the review.

[sky, 2014-06-04 15:31:47]

LGTM

https://chromiumcodereview.appspot.com/309803002/diff/60001/content/browser/p...
File content/browser/power_profiler/power_data_provider.h (right):

https://chromiumcodereview.appspot.com/309803002/diff/60001/content/browser/p...
content/browser/power_profiler/power_data_provider.h:10: #include
"base/basictypes.h"
Why do you need to add this? If it's to get base::TimeDelta you should forward
declare it.

[vivekg, 2014-06-04 16:49:29]

Thank you!

https://chromiumcodereview.appspot.com/309803002/diff/60001/content/browser/p...
File content/browser/power_profiler/power_data_provider.h (right):

https://chromiumcodereview.appspot.com/309803002/diff/60001/content/browser/p...
content/browser/power_profiler/power_data_provider.h:10: #include
"base/basictypes.h"
On 2014/06/04 15:31:48, sky wrote:
> Why do you need to add this? If it's to get base::TimeDelta you should forward
> declare it.

Done.

[vivekg, 2014-06-05 00:33:38]

The CQ bit was checked by vivekg@chromium.org

[commit-bot: I haz the power, 2014-06-05 00:35:51]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vivek.vg@samsung.com/309803002/80001

[commit-bot: I haz the power, 2014-06-05 05:35:03]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)

[commit-bot: I haz the power, 2014-06-05 05:39:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-05 05:39:19]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)

[vivekg, 2014-06-05 05:40:31]

The CQ bit was checked by vivekg@chromium.org

[commit-bot: I haz the power, 2014-06-05 05:44:12]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vivek.vg@samsung.com/309803002/80001

[commit-bot: I haz the power, 2014-06-05 05:52:28]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)

[commit-bot: I haz the power, 2014-06-05 05:57:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-05 05:57:31]

Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)

[vivekg, 2014-06-05 05:59:22]

The CQ bit was checked by vivekg@chromium.org

[commit-bot: I haz the power, 2014-06-05 06:00:26]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vivek.vg@samsung.com/309803002/80001

[commit-bot: I haz the power, 2014-06-05 06:05:34]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)

[commit-bot: I haz the power, 2014-06-05 06:12:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-05 06:12:21]

Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)

[vivekg, 2014-06-05 06:35:45]

The CQ bit was checked by vivekg@chromium.org

[commit-bot: I haz the power, 2014-06-05 06:37:14]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vivek.vg@samsung.com/309803002/80001

[commit-bot: I haz the power, 2014-06-05 06:43:17]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)

[commit-bot: I haz the power, 2014-06-05 06:46:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-05 06:46:45]

Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)

[vivekg, 2014-06-05 06:59:05]

The CQ bit was checked by vivekg@chromium.org

[commit-bot: I haz the power, 2014-06-05 07:00:25]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vivek.vg@samsung.com/309803002/80001

[commit-bot: I haz the power, 2014-06-05 07:29:28]

Change committed as 275040