Add screen power save block level.

Add screen power save block level.

BUG=100054
TEST=no change


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=105401

Reverted: http://src.chromium.org/viewvc/chrome?view=rev&revision=105415

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=105507

[Avi (use Gerrit), 2011-10-13 03:42:34]

mark- mac stuff
jam- windows and rest

[Mark Mentovai, 2011-10-13 04:29:18]

This is an intentionally incomplete review but I thought you might be able to
get a jump on this. I’m already two reviews past where I promised myself I’d
stop tonight.

http://codereview.chromium.org/8251008/diff/1013/content/browser/power_save_b...
File content/browser/power_save_blocker.h (right):

http://codereview.chromium.org/8251008/diff/1013/content/browser/power_save_b...
content/browser/power_save_blocker.h:25: kPowerSaveBlockPreventStateCount
Can you make this be a constant that’s not part of the enum? It makes the
interface a little questionable, since the compiler will be happy to accept this
as an argument to the PowerSaveBlocker constructor and to ApplyBlock.

[jam, 2011-10-13 16:40:28]

lgtm for my parts

http://codereview.chromium.org/8251008/diff/1013/content/browser/power_save_b...
File content/browser/power_save_blocker.h (right):

http://codereview.chromium.org/8251008/diff/1013/content/browser/power_save_b...
content/browser/power_save_blocker.h:37: static void
AdjustBlockCount(std::vector<int> deltas);
const ref

[Avi (use Gerrit), 2011-10-13 17:52:44]

http://codereview.chromium.org/8251008/diff/1013/content/browser/power_save_b...
File content/browser/power_save_blocker.h (right):

http://codereview.chromium.org/8251008/diff/1013/content/browser/power_save_b...
content/browser/power_save_blocker.h:37: static void
AdjustBlockCount(std::vector<int> deltas);
The reason I didn't was because I have a question. Is PostTask smart enough to
make a copy when crossing threads? Otherwise, passing by const reference is the
same as passing by pointer, and that would be bad.

[jam, 2011-10-13 18:38:35]

http://codereview.chromium.org/8251008/diff/1013/content/browser/power_save_b...
File content/browser/power_save_blocker.h (right):

http://codereview.chromium.org/8251008/diff/1013/content/browser/power_save_b...
content/browser/power_save_blocker.h:37: static void
AdjustBlockCount(std::vector<int> deltas);
On 2011/10/13 17:52:44, Avi wrote:
> The reason I didn't was because I have a question. Is PostTask smart enough to
> make a copy when crossing threads? Otherwise, passing by const reference is
the
> same as passing by pointer, and that would be bad.

yep it makes a copy

[Avi (use Gerrit), 2011-10-13 18:44:56]

PTAL

[Mark Mentovai, 2011-10-13 19:09:13]

LG otherwise.

http://codereview.chromium.org/8251008/diff/12001/content/browser/power_save_...
File content/browser/power_save_blocker.h (right):

http://codereview.chromium.org/8251008/diff/12001/content/browser/power_save_...
content/browser/power_save_blocker.h:26: explicit
PowerSaveBlocker(PowerSaveBlockerType type);
This should never accept kPowerSaveBlockPreventNone as an argument, right? If
you don’t have a good way to make the compiler enforce that, you should at least
document the fact in a comment here.

I’d of course prefer to have the compiler enforce it, if you can come up with a
way to do that.

http://codereview.chromium.org/8251008/diff/12001/content/browser/power_save_...
File content/browser/power_save_blocker_mac.cc (right):

http://codereview.chromium.org/8251008/diff/12001/content/browser/power_save_...
content/browser/power_save_blocker_mac.cc:44: default:
You can make this be |case kPowerSaveBlockPreventNone:| instead of |default:|.
Then, when someone adds a new value to the enum, the compiler will warn at this
switch about an unhandled value, which will prompt that same someone to come
here and make the code handle the new enum value.

[commit-bot: I haz the power, 2011-10-13 20:46:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/avi@chromium.org/8251008/12008

[commit-bot: I haz the power, 2011-10-13 23:00:37]

Change committed as 105401

[Peter Mayo (wrong one), 2011-10-13 23:37:55]

On 2011/10/13 23:00:37, I haz the power (commit-bot) wrote:
> Change committed as 105401
From
http://build.chromium.org/p/chromium.chromiumos/builders/ChromiumOS%20%28tegr...


chromeos-chrome-16.0.908.0_alpha-r1: In file included from
./base/memory/weak_ptr.h:63:0,
chromeos-chrome-16.0.908.0_alpha-r1:                  from
./base/bind_helpers.h:69,
chromeos-chrome-16.0.908.0_alpha-r1:                  from
./base/bind_internal.h:15,
chromeos-chrome-16.0.908.0_alpha-r1:                  from ./base/bind.h:15,
chromeos-chrome-16.0.908.0_alpha-r1:                  from
content/browser/power_save_blocker_common.cc:7:
chromeos-chrome-16.0.908.0_alpha-r1: ./base/logging.h: In function ‘std::string*
logging::CheckGTImpl(const t1&, const t2&, const char*) [with t1 =
PowerSaveBlocker::<anonymous enum>, t2 = PowerSaveBlocker::PowerSaveBlockerType,
std::string = std::basic_string<char>]’:
chromeos-chrome-16.0.908.0_alpha-r1:
content/browser/power_save_blocker_common.cc:15:3:   instantiated from here
chromeos-chrome-16.0.908.0_alpha-r1: ./base/logging.h:518:1: error: comparison
between ‘const enum PowerSaveBlocker::<anonymous>’ and ‘const enum
PowerSaveBlocker::PowerSaveBlockerType’ [-Werror=enum-compare]
chromeos-chrome-16.0.908.0_alpha-r1: cc1plus: all warnings being treated as
errors
chromeos-chrome-16.0.908.0_alpha-r1: 
chromeos-chrome-16.0.908.0_alpha-r1: make: ***
[c/Release/obj.target/content_browser/content/browser/power_save_blocker_common.o]
Error 1
chromeos-chrome-16.0.908.0_alpha-r1: make: *** Waiting for unfinished jobs....
chromeos-chrome-16.0.908.0_alpha-r1: emake failed
chromeos-chrome-16.0.908.0_alpha-r1:  * ERROR:
chromeos-base/chromeos-chrome-16.0.908.0_alpha-r1 failed (compile phase):


Is this responsible?

[Avi (use Gerrit), 2011-10-14 00:16:06]

http://codereview.chromium.org/8251008/diff/9002/content/browser/power_save_b...
File content/browser/power_save_blocker.h (right):

http://codereview.chromium.org/8251008/diff/9002/content/browser/power_save_b...
content/browser/power_save_blocker.h:26: kPowerSaveBlockPreventStateCount
The pattern of putting a count as the last item in an enum is as old as time. If
I don't do this, compiles die.