Decouple PowerUsageMonitor{Impl} from //content/browser

Decouple PowerUsageMonitor{Impl} from //content/browser

PowerUsageMonitor is currently only used by Chrome and has significant //content
dependencies, to decouple it we just move it to //chrome/browser at the current
time.
The power_usage_monitor.h is deleted as there is no need to export the
StartPowerUsageMonitor interface from chrome any more. We add the
PowerUsageMonitor as a member into the ChromeBrowserMain.
Unittest are also changed accordingly.

BUG=647247
Committed: https://crrev.com/bf45805b20e15b30b0cecad0b9056136fb6d2849
Cr-Commit-Position: refs/heads/master@{#430233}

[ke.he, 2016-10-27 06:32:14]

The CQ bit was checked by ke.he@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-27 06:32:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-27 06:32:33]

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

[commit-bot: I haz the power, 2016-10-27 06:32:33]

Dry run: No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[ke.he, 2016-10-27 06:43:49]

ke.he@intel.com changed reviewers:
+ blundell@chromium.org, leon.han@intel.com

[ke.he, 2016-10-27 06:43:49]

Could you PTAL on this? Thanks!

[leonhsl(Using Gerrit), 2016-10-27 08:02:51]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-27 08:03:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-27 08:06:55]

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

[commit-bot: I haz the power, 2016-10-27 08:06:56]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[leonhsl(Using Gerrit), 2016-10-27 08:28:07]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-27 08:28:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-27 08:35:41]

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

[commit-bot: I haz the power, 2016-10-27 08:35:41]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[leonhsl(Using Gerrit), 2016-10-27 10:50:58]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-27 10:51:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-27 11:14:40]

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

[commit-bot: I haz the power, 2016-10-27 11:14:41]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[blundell, 2016-10-27 21:21:55]

Looks good, thanks! In addition to the comments that I've left, it looks you
have some compile problems to fix.

https://codereview.chromium.org/2456783002/diff/40001/chrome/browser/BUILD.gn
File chrome/browser/BUILD.gn (right):

https://codereview.chromium.org/2456783002/diff/40001/chrome/browser/BUILD.gn...
chrome/browser/BUILD.gn:2785: "power/process_power_collector.h",
nit: rename this file power_usage_monitor.* now.

https://codereview.chromium.org/2456783002/diff/40001/chrome/browser/chrome_b...
File chrome/browser/chrome_browser_main.cc (right):

https://codereview.chromium.org/2456783002/diff/40001/chrome/browser/chrome_b...
chrome/browser/chrome_browser_main.cc:1935:
PowerUsageMonitor::StartPowerUsageMonitor();
I think this could just be owned directly by ChromeBrowserMain now.

[leonhsl(Using Gerrit), 2016-10-28 04:00:35]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-28 04:00:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-28 05:30:42]

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

[commit-bot: I haz the power, 2016-10-28 05:30:43]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[leonhsl(Using Gerrit), 2016-10-28 08:30:03]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-28 08:30:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[leonhsl(Using Gerrit), 2016-10-28 08:32:22]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-28 08:32:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ke.he, 2016-10-28 08:48:25]

Description was changed from

==========
Decouple PowerUsageMonitor{Impl} from //content/browser

PowerUsageMonitor is currently only used by Chrome and has significant //content
dependencies, to decouple it we just move it to //chrome/browser at the current
time.
The power_usage_monitor.h is deleted as we don't need to export the
StartPowerUsageMonitor interface from chrome any more. Instead we add the
interfaceit as a static function of PowerUsageMonitorImpl.
Unittest are also changed accordingly.

BUG=647247
==========

to

==========
Decouple PowerUsageMonitor{Impl} from //content/browser

PowerUsageMonitor is currently only used by Chrome and has significant //content
dependencies, to decouple it we just move it to //chrome/browser at the current
time.
The power_usage_monitor.h is deleted as there is no need to export the
StartPowerUsageMonitor interface from chrome any more. We add the
PowerUsageMonitor as a member into the ChromeBrowserMain.
Unittest are also changed accordingly.

BUG=647247
==========

[ke.he, 2016-10-28 09:39:20]

https://codereview.chromium.org/2456783002/diff/40001/chrome/browser/BUILD.gn
File chrome/browser/BUILD.gn (right):

https://codereview.chromium.org/2456783002/diff/40001/chrome/browser/BUILD.gn...
chrome/browser/BUILD.gn:2785: "power/process_power_collector.h",
On 2016/10/27 21:21:55, blundell wrote:
> nit: rename this file power_usage_monitor.* now.

Done.

https://codereview.chromium.org/2456783002/diff/40001/chrome/browser/chrome_b...
File chrome/browser/chrome_browser_main.cc (right):

https://codereview.chromium.org/2456783002/diff/40001/chrome/browser/chrome_b...
chrome/browser/chrome_browser_main.cc:1935:
PowerUsageMonitor::StartPowerUsageMonitor();
On 2016/10/27 21:21:55, blundell wrote:
> I think this could just be owned directly by ChromeBrowserMain now.

Done.
The original implementation of this StartPowerUsageMonitor() uses the
"LazyInstance" to create the PoowerUsageMonitor object, because it is in content
layer and has the thread safe consideration. Now we move it into
ChromeBrowserMain, so we don't need to create in the "LazyInstance" way.
blundell@, Is it ok? thanks!

[commit-bot: I haz the power, 2016-10-28 09:46:45]

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

[commit-bot: I haz the power, 2016-10-28 09:46:46]

Dry run: This issue passed the CQ dry run.

[blundell, 2016-10-28 14:43:26]

lgtm, thanks!

[blundell, 2016-10-28 14:44:07]

blundell@chromium.org changed reviewers:
+ jam@chromium.org

[blundell, 2016-10-28 14:44:08]

+jam@ for OWNERS (whole CL, which is not that large)

[jam, 2016-10-31 16:51:07]

What is the benefit of moving this from content to chrome?

[blundell, 2016-10-31 17:00:40]

On 2016/10/31 16:51:07, jam wrote:
> What is the benefit of moving this from content to chrome?

Hey John,

This change is based on my analysis of decoupling power monitor from //content
(https://docs.google.com/document/d/1w95Cwmxn-xVmICf1Q7YQvJy4eoInpmMntsLycB5qq...):

There is additionally power_usage_monitor.h (in the Content API) and its
implementation. This is totally independent [from the power monitor IPC that the
rest of the doc was analyzing] and serves to record statistics about power usage
in the browser process. It is currently only used by Chrome. As this class is
currently only used by Chrome and has significant //content dependencies, I
suggest moving it to //chrome/browser at the current time and revisiting if
there is ever a desire to reuse the code by other embedders.

My reasoning was that I couldn't see a strong motivation for spending effort on
removing the //content dependencies from this code now, but we could at least
clarify its usage by moving it into //chrome rather than having it as part of
the Content API.

[ke.he, 2016-11-02 05:48:26]

blundell@, Thanks for the clarification.
jam@, any comments on this? thanks:)

[jam, 2016-11-04 17:25:50]

ok, if it's only used by chrome lgtm

[ke.he, 2016-11-07 06:43:10]

The CQ bit was checked by ke.he@intel.com

[commit-bot: I haz the power, 2016-11-07 06:43:26]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-07 07:41:41]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-11-07 07:43:38]

Description was changed from

==========
Decouple PowerUsageMonitor{Impl} from //content/browser

PowerUsageMonitor is currently only used by Chrome and has significant //content
dependencies, to decouple it we just move it to //chrome/browser at the current
time.
The power_usage_monitor.h is deleted as there is no need to export the
StartPowerUsageMonitor interface from chrome any more. We add the
PowerUsageMonitor as a member into the ChromeBrowserMain.
Unittest are also changed accordingly.

BUG=647247
==========

to

==========
Decouple PowerUsageMonitor{Impl} from //content/browser

PowerUsageMonitor is currently only used by Chrome and has significant //content
dependencies, to decouple it we just move it to //chrome/browser at the current
time.
The power_usage_monitor.h is deleted as there is no need to export the
StartPowerUsageMonitor interface from chrome any more. We add the
PowerUsageMonitor as a member into the ChromeBrowserMain.
Unittest are also changed accordingly.

BUG=647247

Committed: https://crrev.com/bf45805b20e15b30b0cecad0b9056136fb6d2849
Cr-Commit-Position: refs/heads/master@{#430233}
==========

[commit-bot: I haz the power, 2016-11-07 07:43:38]

Patchset 5 (id:??) landed as
https://crrev.com/bf45805b20e15b30b0cecad0b9056136fb6d2849
Cr-Commit-Position: refs/heads/master@{#430233}