Content: Implement PowerSaveBlocker2 for windows.

Content: Implement PowerSaveBlocker2 for windows.

BUG=126591
TEST=none
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=140668

[rvargas (doing something else), 2012-06-05 17:57:15]

cpu: please review
Avi: mostly for owners.

[Avi (use Gerrit), 2012-06-05 18:11:55]

https://chromiumcodereview.appspot.com/10538013/diff/1/content/browser/power_...
File content/browser/power_save_blocker_win.cc (right):

https://chromiumcodereview.appspot.com/10538013/diff/1/content/browser/power_...
content/browser/power_save_blocker_win.cc:189: : delegate_(new 
PowerSaveBlocker::Delegate(type, reason)) {
The only thing I can do: nitpick. Too many spaces between "new" and
"PowerSaveBlocker"

https://chromiumcodereview.appspot.com/10538013/diff/1/content/browser/power_...
content/browser/power_save_blocker_win.cc:195:
PowerSaveBlocker::~PowerSaveBlocker(void) {
no void

[Mike Mammarella, 2012-06-05 18:28:48]

https://chromiumcodereview.appspot.com/10538013/diff/1/content/browser/power_...
File content/browser/power_save_blocker_win.cc (right):

https://chromiumcodereview.appspot.com/10538013/diff/1/content/browser/power_...
content/browser/power_save_blocker_win.cc:39: #define PowerSaveBlocker
PowerSaveBlocker2
Hey, this is clever. I'm going to do the same thing for the Linux version I'm
working on.

https://chromiumcodereview.appspot.com/10538013/diff/1/content/browser/power_...
content/browser/power_save_blocker_win.cc:189: : delegate_(new 
PowerSaveBlocker::Delegate(type, reason)) {
You don't need the PowerSaveBlocker:: here or below in base::Bind(), although it
won't hurt to keep it.

[rvargas (doing something else), 2012-06-05 18:39:50]

Thanks.

https://chromiumcodereview.appspot.com/10538013/diff/1/content/browser/power_...
File content/browser/power_save_blocker_win.cc (right):

https://chromiumcodereview.appspot.com/10538013/diff/1/content/browser/power_...
content/browser/power_save_blocker_win.cc:189: : delegate_(new 
PowerSaveBlocker::Delegate(type, reason)) {
On 2012/06/05 18:11:55, Avi wrote:
> The only thing I can do: nitpick. Too many spaces between "new" and
> "PowerSaveBlocker"

Done.

https://chromiumcodereview.appspot.com/10538013/diff/1/content/browser/power_...
content/browser/power_save_blocker_win.cc:189: : delegate_(new 
PowerSaveBlocker::Delegate(type, reason)) {
On 2012/06/05 18:28:49, Mike Mammarella wrote:
> You don't need the PowerSaveBlocker:: here or below in base::Bind(), although
it
> won't hurt to keep it.

Done.

https://chromiumcodereview.appspot.com/10538013/diff/1/content/browser/power_...
content/browser/power_save_blocker_win.cc:195:
PowerSaveBlocker::~PowerSaveBlocker(void) {
On 2012/06/05 18:11:55, Avi wrote:
> no void

Ouch!. done.

[Avi (use Gerrit), 2012-06-05 18:48:34]

https://chromiumcodereview.appspot.com/10538013/diff/1/content/browser/power_...
File content/browser/power_save_blocker_win.cc (right):

https://chromiumcodereview.appspot.com/10538013/diff/1/content/browser/power_...
content/browser/power_save_blocker_win.cc:147: ~Delegate() {}
When I copy this code, I get a style failure:

error: [chromium-style] Classes that are ref-counted should not have public
destructors.

Don't do this either. Make this private.

Of course, that leads to lots of weird cascading errors. Investigating.

[rvargas (doing something else), 2012-06-05 18:58:19]

https://chromiumcodereview.appspot.com/10538013/diff/1/content/browser/power_...
File content/browser/power_save_blocker_win.cc (right):

https://chromiumcodereview.appspot.com/10538013/diff/1/content/browser/power_...
content/browser/power_save_blocker_win.cc:147: ~Delegate() {}
On 2012/06/05 18:48:34, Avi wrote:
> When I copy this code, I get a style failure:
> 
> error: [chromium-style] Classes that are ref-counted should not have public
> destructors.
> 
> Don't do this either. Make this private.
> 
> Of course, that leads to lots of weird cascading errors. Investigating.

And this is fixed now.

[Avi (use Gerrit), 2012-06-05 19:02:54]

lgtm

I'm happy enough with what I see. Find someone else to review the windows bits.

[Avi (use Gerrit), 2012-06-05 19:03:29]

https://chromiumcodereview.appspot.com/10538013/diff/9001/content/browser/pow...
File content/browser/power_save_blocker_win.cc (right):

https://chromiumcodereview.appspot.com/10538013/diff/9001/content/browser/pow...
content/browser/power_save_blocker_win.cc:160: const std::string reason_;
why is this const?

[rvargas (doing something else), 2012-06-05 19:06:04]

https://chromiumcodereview.appspot.com/10538013/diff/9001/content/browser/pow...
File content/browser/power_save_blocker_win.cc (right):

https://chromiumcodereview.appspot.com/10538013/diff/9001/content/browser/pow...
content/browser/power_save_blocker_win.cc:160: const std::string reason_;
On 2012/06/05 19:03:29, Avi wrote:
> why is this const?

no particular reason, but there's no need to be able to modify it.

Do you want me to remove the const?

[Avi (use Gerrit), 2012-06-05 19:15:11]

I don't particularly care, but I've never seen anyone do it that way before.

[cpu_(ooo_6.6-7.5), 2012-06-05 23:26:22]

lgtm

http://codereview.chromium.org/10538013/diff/9001/content/browser/power_save_...
File content/browser/power_save_blocker_win.cc (right):

http://codereview.chromium.org/10538013/diff/9001/content/browser/power_save_...
content/browser/power_save_blocker_win.cc:162: };
disallow copy and assign?

[rvargas (doing something else), 2012-06-05 23:52:12]

Thanks

https://chromiumcodereview.appspot.com/10538013/diff/9001/content/browser/pow...
File content/browser/power_save_blocker_win.cc (right):

https://chromiumcodereview.appspot.com/10538013/diff/9001/content/browser/pow...
content/browser/power_save_blocker_win.cc:162: };
On 2012/06/05 23:26:22, cpu wrote:
> disallow copy and assign?

Done.

[commit-bot: I haz the power, 2012-06-05 23:53:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/10538013/4

[commit-bot: I haz the power, 2012-06-05 23:57:25]

Try job failure for 10538013-4 on win for step "update".
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

Step "update" is always a major failure.
Look at the try server FAQ for more details.