Move content/browser/power_save_blocker to //device/power_save_blocker

Move content/browser/power_save_blocker to //device/power_save_blocker

This is mostly a mechanical move. The only "new" code is the new build
files, and the Android JNI registration that has its own version in
//device now instead of being part of the //content registration.

R=jam@chromium.org
TBR=rockot,tedchoc
BUG=612337, 612563, 257943
Committed: https://crrev.com/327630a7735fc91aedea5e164064c4dd18c04cc4
Cr-Commit-Position: refs/heads/master@{#400361}

[scottmg, 2016-05-24 20:03:02]

Description was changed from

==========
Move content/browser/power_save_blocker to //device/power_save_blocker

BUG=612337, 612563
==========

to

==========
Move content/browser/power_save_blocker to //device/power_save_blocker

TODO gyp and gn and java thingys

BUG=612337, 612563
==========

[scottmg, 2016-06-14 01:43:49]

Description was changed from

==========
Move content/browser/power_save_blocker to //device/power_save_blocker

TODO gyp and gn and java thingys

BUG=612337, 612563
==========

to

==========
Move content/browser/power_save_blocker to //device/power_save_blocker

TODO gyp

BUG=612337, 612563
==========

[scottmg, 2016-06-14 01:49:32]

Patchset #2 (id:20001) has been deleted

[scottmg, 2016-06-14 21:57:00]

Patchset #4 (id:80001) has been deleted

[scottmg, 2016-06-15 23:44:42]

Description was changed from

==========
Move content/browser/power_save_blocker to //device/power_save_blocker

TODO gyp

BUG=612337, 612563
==========

to

==========
Move content/browser/power_save_blocker to //device/power_save_blocker

R=jam@chromium.org
BUG=612337, 612563
==========

[scottmg, 2016-06-15 23:44:42]

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

[scottmg, 2016-06-15 23:46:14]

Description was changed from

==========
Move content/browser/power_save_blocker to //device/power_save_blocker

R=jam@chromium.org
BUG=612337, 612563
==========

to

==========
Move content/browser/power_save_blocker to //device/power_save_blocker

This is mostly a mechanical move. The only "new" code is the new build
files, and the Android JNI registration that has its own version in
//device now instead of being part of the //content registration.

R=jam@chromium.org
BUG=612337, 612563
==========

[scottmg, 2016-06-15 23:49:32]

Description was changed from

==========
Move content/browser/power_save_blocker to //device/power_save_blocker

This is mostly a mechanical move. The only "new" code is the new build
files, and the Android JNI registration that has its own version in
//device now instead of being part of the //content registration.

R=jam@chromium.org
BUG=612337, 612563
==========

to

==========
Move content/browser/power_save_blocker to //device/power_save_blocker

This is mostly a mechanical move. The only "new" code is the new build
files, and the Android JNI registration that has its own version in
//device now instead of being part of the //content registration.

R=jam@chromium.org
BUG=612337, 612563
==========

[scottmg, 2016-06-15 23:49:32]

scottmg@chromium.org changed reviewers:
+ hashimoto@chromium.org, oshima@chromium.org, tedchoc@chromium.org

[scottmg, 2016-06-16 00:26:27]

scottmg@chromium.org changed reviewers:
+ sadrul@chromium.org
- oshima@chromium.org

[scottmg, 2016-06-16 00:26:28]

jam: overall review
hashimoto: +dbus DEPS addition
tedchoc: +ui/android DEPS addition
sadrul: +ui/gfx/x DEPS addition

[hashimoto, 2016-06-16 03:31:28]

device/power_save_blocker/DEPS lgtm as an owner of //dbus.

[sadrul, 2016-06-16 15:15:53]

> sadrul: +ui/gfx/x DEPS addition

lgtm

[jam, 2016-06-16 17:04:05]

https://codereview.chromium.org/1999953002/diff/260001/content/public/browser...
File content/public/browser/DEPS (right):

https://codereview.chromium.org/1999953002/diff/260001/content/public/browser...
content/public/browser/DEPS:2: ".*\.(cc|h)": [
why make this change?

We don't want public headers to be allowed to include internal headers, as that
would allow code outside of content to include internal headers.

https://codereview.chromium.org/1999953002/diff/260001/content/public/browser...
File content/public/browser/power_save_blocker_factory.h (right):

https://codereview.chromium.org/1999953002/diff/260001/content/public/browser...
content/public/browser/power_save_blocker_factory.h:18: CONTENT_EXPORT
std::unique_ptr<device::PowerSaveBlocker> CreatePowerSaveBlocker(
why isn't this moved?

https://codereview.chromium.org/1999953002/diff/260001/device/power_save_bloc...
File device/power_save_blocker/power_save_blocker.h (right):

https://codereview.chromium.org/1999953002/diff/260001/device/power_save_bloc...
device/power_save_blocker/power_save_blocker.h:20: class
DEVICE_POWER_SAVE_BLOCKER_EXPORT PowerSaveBlocker {
(can do this in a followup)

now that it's moved out of content, there's no need to have an interface and an
impl class. this was done because it was in content so we had to have a public
interface in content/public.

[scottmg, 2016-06-16 17:45:35]

Description was changed from

==========
Move content/browser/power_save_blocker to //device/power_save_blocker

This is mostly a mechanical move. The only "new" code is the new build
files, and the Android JNI registration that has its own version in
//device now instead of being part of the //content registration.

R=jam@chromium.org
BUG=612337, 612563
==========

to

==========
Move content/browser/power_save_blocker to //device/power_save_blocker

This is mostly a mechanical move. The only "new" code is the new build
files, and the Android JNI registration that has its own version in
//device now instead of being part of the //content registration.

R=jam@chromium.org
BUG=612337, 612563, 257943
==========

[scottmg, 2016-06-16 18:10:34]

Thanks!

https://codereview.chromium.org/1999953002/diff/260001/content/public/browser...
File content/public/browser/DEPS (right):

https://codereview.chromium.org/1999953002/diff/260001/content/public/browser...
content/public/browser/DEPS:2: ".*\.(cc|h)": [
On 2016/06/16 17:04:04, jam wrote:
> why make this change?
> 
> We don't want public headers to be allowed to include internal headers, as
that
> would allow code outside of content to include internal headers.

Ah, it was just because content/public/browser/power_save_blocker_factory.h
needed to include it. But once that's moved with other changes to non-content
files, this is unnecessary now. Removed.

https://codereview.chromium.org/1999953002/diff/260001/content/public/browser...
File content/public/browser/power_save_blocker_factory.h (right):

https://codereview.chromium.org/1999953002/diff/260001/content/public/browser...
content/public/browser/power_save_blocker_factory.h:18: CONTENT_EXPORT
std::unique_ptr<device::PowerSaveBlocker> CreatePowerSaveBlocker(
On 2016/06/16 17:04:04, jam wrote:
> why isn't this moved?

This was just supposed to be a helper for content code so that it's not so
verbose to create a PowerSaveBlocker when in content.
https://cs.chromium.org/chromium/src/content/browser/power_save_blocker_facto...

But I forgot to move it out of public/, did that now.

Doing that meant I needed to add a +browser_thread.h to
content/browser/loader/DEPS but it's not really a new dependency. I'll take a
look at that separately in the future, I'm not sure how loader should be getting
task runners at this point.

https://codereview.chromium.org/1999953002/diff/260001/device/power_save_bloc...
File device/power_save_blocker/power_save_blocker.h (right):

https://codereview.chromium.org/1999953002/diff/260001/device/power_save_bloc...
device/power_save_blocker/power_save_blocker.h:20: class
DEVICE_POWER_SAVE_BLOCKER_EXPORT PowerSaveBlocker {
On 2016/06/16 17:04:04, jam wrote:
> (can do this in a followup)
> 
> now that it's moved out of content, there's no need to have an interface and
an
> impl class. this was done because it was in content so we had to have a public
> interface in content/public.

That sounds good, I was thinking the same thing. I'll do that in a followup.

[scottmg, 2016-06-16 18:10:59]

scottmg@chromium.org changed reviewers:
+ rockot@chromium.org

[scottmg, 2016-06-16 18:11:00]

+rockot for extensions/browser/DEPS addition

[jam, 2016-06-16 18:19:09]

lgtm

https://codereview.chromium.org/1999953002/diff/260001/content/public/browser...
File content/public/browser/power_save_blocker_factory.h (right):

https://codereview.chromium.org/1999953002/diff/260001/content/public/browser...
content/public/browser/power_save_blocker_factory.h:18: CONTENT_EXPORT
std::unique_ptr<device::PowerSaveBlocker> CreatePowerSaveBlocker(
On 2016/06/16 18:10:34, scottmg wrote:
> On 2016/06/16 17:04:04, jam wrote:
> > why isn't this moved?
> 
> This was just supposed to be a helper for content code so that it's not so
> verbose to create a PowerSaveBlocker when in content.
>
https://cs.chromium.org/chromium/src/content/browser/power_save_blocker_facto...
> 
> But I forgot to move it out of public/, did that now.
> 
> Doing that meant I needed to add a +browser_thread.h to
> content/browser/loader/DEPS but it's not really a new dependency. I'll take a
> look at that separately in the future, I'm not sure how loader should be
getting
> task runners at this point.

we can pass them at construction time from content.

i think this factory serves very little purpose, and we should just remove it in
the followup (especially once the interface is gone)

[scottmg, 2016-06-16 18:22:50]

On 2016/06/16 18:19:09, jam wrote:
> lgtm
> 
>
https://codereview.chromium.org/1999953002/diff/260001/content/public/browser...
> File content/public/browser/power_save_blocker_factory.h (right):
> 
>
https://codereview.chromium.org/1999953002/diff/260001/content/public/browser...
> content/public/browser/power_save_blocker_factory.h:18: CONTENT_EXPORT
> std::unique_ptr<device::PowerSaveBlocker> CreatePowerSaveBlocker(
> On 2016/06/16 18:10:34, scottmg wrote:
> > On 2016/06/16 17:04:04, jam wrote:
> > > why isn't this moved?
> > 
> > This was just supposed to be a helper for content code so that it's not so
> > verbose to create a PowerSaveBlocker when in content.
> >
>
https://cs.chromium.org/chromium/src/content/browser/power_save_blocker_facto...
> > 
> > But I forgot to move it out of public/, did that now.
> > 
> > Doing that meant I needed to add a +browser_thread.h to
> > content/browser/loader/DEPS but it's not really a new dependency. I'll take
a
> > look at that separately in the future, I'm not sure how loader should be
> getting
> > task runners at this point.
> 
> we can pass them at construction time from content.
> 
> i think this factory serves very little purpose, and we should just remove it
in
> the followup (especially once the interface is gone)

Sure.

[scottmg, 2016-06-17 04:18:41]

To avoid patch collision, I'm going to tbr for the following trivial changes:

rockot: 
https://codereview.chromium.org/1999953002/diff/400001/extensions/browser/DEPS

tedchoc:
+ui/android in

https://codereview.chromium.org/1999953002/diff/400001/device/power_save_bloc...

If you have any concerns let me know and I'll address them in followups.

[scottmg, 2016-06-17 04:25:24]

Description was changed from

==========
Move content/browser/power_save_blocker to //device/power_save_blocker

This is mostly a mechanical move. The only "new" code is the new build
files, and the Android JNI registration that has its own version in
//device now instead of being part of the //content registration.

R=jam@chromium.org
BUG=612337, 612563, 257943
==========

to

==========
Move content/browser/power_save_blocker to //device/power_save_blocker

This is mostly a mechanical move. The only "new" code is the new build
files, and the Android JNI registration that has its own version in
//device now instead of being part of the //content registration.

R=jam@chromium.org
TBR=rockot,tedchoc
BUG=612337, 612563, 257943
==========

[scottmg, 2016-06-17 04:25:35]

The CQ bit was checked by scottmg@chromium.org

[scottmg, 2016-06-17 04:25:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
hashimoto@chromium.org, sadrul@chromium.org, jam@chromium.org
Link to the patchset: https://codereview.chromium.org/1999953002/#ps400001
(title: ".")

[commit-bot: I haz the power, 2016-06-17 04:25:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1999953002/400001

[commit-bot: I haz the power, 2016-06-17 05:09:11]

Description was changed from

==========
Move content/browser/power_save_blocker to //device/power_save_blocker

This is mostly a mechanical move. The only "new" code is the new build
files, and the Android JNI registration that has its own version in
//device now instead of being part of the //content registration.

R=jam@chromium.org
TBR=rockot,tedchoc
BUG=612337, 612563, 257943
==========

to

==========
Move content/browser/power_save_blocker to //device/power_save_blocker

This is mostly a mechanical move. The only "new" code is the new build
files, and the Android JNI registration that has its own version in
//device now instead of being part of the //content registration.

R=jam@chromium.org
TBR=rockot,tedchoc
BUG=612337, 612563, 257943
==========

[commit-bot: I haz the power, 2016-06-17 05:09:13]

Committed patchset #19 (id:400001)

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

Description was changed from

==========
Move content/browser/power_save_blocker to //device/power_save_blocker

This is mostly a mechanical move. The only "new" code is the new build
files, and the Android JNI registration that has its own version in
//device now instead of being part of the //content registration.

R=jam@chromium.org
TBR=rockot,tedchoc
BUG=612337, 612563, 257943
==========

to

==========
Move content/browser/power_save_blocker to //device/power_save_blocker

This is mostly a mechanical move. The only "new" code is the new build
files, and the Android JNI registration that has its own version in
//device now instead of being part of the //content registration.

R=jam@chromium.org
TBR=rockot,tedchoc
BUG=612337, 612563, 257943

Committed: https://crrev.com/327630a7735fc91aedea5e164064c4dd18c04cc4
Cr-Commit-Position: refs/heads/master@{#400361}
==========

[commit-bot: I haz the power, 2016-06-17 05:10:28]

Patchset 19 (id:??) landed as
https://crrev.com/327630a7735fc91aedea5e164064c4dd18c04cc4
Cr-Commit-Position: refs/heads/master@{#400361}

[scottmg, 2016-06-17 05:18:38]

A revert of this CL (patchset #19 id:400001) has been created in
https://codereview.chromium.org/2075973002/ by scottmg@chromium.org.

The reason for reverting is: Broke
https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac%20Builder%...
but not CQ bots.