Awake Mac host display when remote session starts.

Wake Mac host display when remote session starts.

Create a power management assertion when a remote session starts. The power
assertion will wake the display if it's asleep and will keep it awake while the
remote session is in progress.

BUG=120429
TEST=manual


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

[Wez, 2012-04-14 00:08:22]

https://chromiumcodereview.appspot.com/10082032/diff/1/remoting/host/capturer...
File remoting/host/capturer_mac.cc (right):

https://chromiumcodereview.appspot.com/10082032/diff/1/remoting/host/capturer...
remoting/host/capturer_mac.cc:29: const int64 kPMAssertionTimeoutMs = 5 * 1000;
// 30 seconds
This looks like five seconds, not thirty?

https://chromiumcodereview.appspot.com/10082032/diff/1/remoting/host/capturer...
remoting/host/capturer_mac.cc:29: const int64 kPMAssertionTimeoutMs = 5 * 1000;
// 30 seconds
See note below re naming.

https://chromiumcodereview.appspot.com/10082032/diff/1/remoting/host/capturer...
remoting/host/capturer_mac.cc:233: IOPMAssertionID pm_assertion_id_;
Style guide requires that names not be abbreviated unless the abbreviation is
well established - consider changing this to power_assertion_id, and renaming
the methods to Refresh/ReleasePowerAssertion.

https://chromiumcodereview.appspot.com/10082032/diff/1/remoting/host/capturer...
remoting/host/capturer_mac.cc:237: bool pm_assertion_created_;
Why not just check whether the timer is started, rather than keeping a separate
flag?

https://chromiumcodereview.appspot.com/10082032/diff/1/remoting/host/capturer...
remoting/host/capturer_mac.cc:678: pm_assertion_id_ = 0;
nit: Zeroing this when it's not started effectively duplicates the
pm_assertion_created_ flag's function, and since you never DCHECK()
pm_assertion_id_ there's no real need to zero it here.

[Wez, 2012-04-14 00:09:24]

Oh, and the first two "awake"s in the CL description should really just be
"wake". :)

[dcaiafa, 2012-04-14 00:21:40]

ptal.

https://chromiumcodereview.appspot.com/10082032/diff/1/remoting/host/capturer...
File remoting/host/capturer_mac.cc (right):

https://chromiumcodereview.appspot.com/10082032/diff/1/remoting/host/capturer...
remoting/host/capturer_mac.cc:29: const int64 kPMAssertionTimeoutMs = 5 * 1000;
// 30 seconds
On 2012/04/14 00:08:22, Wez wrote:
> This looks like five seconds, not thirty?

Done.

https://chromiumcodereview.appspot.com/10082032/diff/1/remoting/host/capturer...
remoting/host/capturer_mac.cc:29: const int64 kPMAssertionTimeoutMs = 5 * 1000;
// 30 seconds
On 2012/04/14 00:08:22, Wez wrote:
> This looks like five seconds, not thirty?

Done.

https://chromiumcodereview.appspot.com/10082032/diff/1/remoting/host/capturer...
remoting/host/capturer_mac.cc:233: IOPMAssertionID pm_assertion_id_;
On 2012/04/14 00:08:22, Wez wrote:
> Style guide requires that names not be abbreviated unless the abbreviation is
> well established - consider changing this to power_assertion_id, and renaming
> the methods to Refresh/ReleasePowerAssertion.

Done.

https://chromiumcodereview.appspot.com/10082032/diff/1/remoting/host/capturer...
remoting/host/capturer_mac.cc:237: bool pm_assertion_created_;
Because when ReleasePowerAssertion is called by the timer, the timer will have
been stopped.

On 2012/04/14 00:08:22, Wez wrote:
> Why not just check whether the timer is started, rather than keeping a
separate
> flag?

https://chromiumcodereview.appspot.com/10082032/diff/1/remoting/host/capturer...
remoting/host/capturer_mac.cc:678: pm_assertion_id_ = 0;
On 2012/04/14 00:08:22, Wez wrote:
> nit: Zeroing this when it's not started effectively duplicates the
> pm_assertion_created_ flag's function, and since you never DCHECK()
> pm_assertion_id_ there's no real need to zero it here.

Done.

[Wez, 2012-04-14 00:24:39]

https://chromiumcodereview.appspot.com/10082032/diff/1/remoting/host/capturer...
File remoting/host/capturer_mac.cc (right):

https://chromiumcodereview.appspot.com/10082032/diff/1/remoting/host/capturer...
remoting/host/capturer_mac.cc:237: bool pm_assertion_created_;
OK.  Is zero a valid power management assertion Id?  If not then testing the Id
for being zero would avoid the need for the separate flag.

On 2012/04/14 00:21:41, dcaiafa wrote:
> Because when ReleasePowerAssertion is called by the timer, the timer will have
> been stopped.
> 
> On 2012/04/14 00:08:22, Wez wrote:
> > Why not just check whether the timer is started, rather than keeping a
> separate
> > flag?

[dcaiafa, 2012-04-14 00:37:43]

ptal

https://chromiumcodereview.appspot.com/10082032/diff/1/remoting/host/capturer...
File remoting/host/capturer_mac.cc (right):

https://chromiumcodereview.appspot.com/10082032/diff/1/remoting/host/capturer...
remoting/host/capturer_mac.cc:237: bool pm_assertion_created_;
Turns out that there is a constant called kIOPMNullAssertionID. I failed to find
it in my first look at the headers.

On 2012/04/14 00:24:39, Wez wrote:
> OK.  Is zero a valid power management assertion Id?  If not then testing the
Id
> for being zero would avoid the need for the separate flag.
> 
> On 2012/04/14 00:21:41, dcaiafa wrote:
> > Because when ReleasePowerAssertion is called by the timer, the timer will
have
> > been stopped.
> > 
> > On 2012/04/14 00:08:22, Wez wrote:
> > > Why not just check whether the timer is started, rather than keeping a
> > separate
> > > flag?
>

[Wez, 2012-04-14 00:41:50]

lgtm

[Avi (use Gerrit), 2012-04-14 00:42:33]

A thought: there is already an RAII class to do exactly this in PowerSaveBlocker
in content. Would it be worth it to move it to base/ or something like that so
you can share it?

https://chromiumcodereview.appspot.com/10082032/diff/5/remoting/host/capturer...
File remoting/host/capturer_mac.cc (right):

https://chromiumcodereview.appspot.com/10082032/diff/5/remoting/host/capturer...
remoting/host/capturer_mac.cc:656: IOReturn result =
IOPMAssertionCreateWithName(
This is only available in 10.6. Do you not require 10.5 support like the rest of
chrome?

[Jamie, 2012-04-14 01:26:35]

https://chromiumcodereview.appspot.com/10082032/diff/5/remoting/host/capturer...
File remoting/host/capturer_mac.cc (right):

https://chromiumcodereview.appspot.com/10082032/diff/5/remoting/host/capturer...
remoting/host/capturer_mac.cc:656: IOReturn result =
IOPMAssertionCreateWithName(
On 2012/04/14 00:42:33, Avi wrote:
> This is only available in 10.6. Do you not require 10.5 support like the rest
of
> chrome?

On 10.5, moving the mouse will wake up the display, so we don't need this to run
on 10.5. However, we do need it to compile and the resulting binary has to run
on 10.5, so maybe there's some dynamic loading missing here.

[dcaiafa, 2012-04-16 17:28:07]

Replaced call to IOPMCreateAssertionWithName which is only available in 10.6+
with IOPMCreateAssertion. Verified it builds with 10.5 and it works as intended
in Lion.

Avi: 
We need this fixed soon so I will stay the course. I'll follow up on your
suggestion soon afterwards.

[commit-bot: I haz the power, 2012-04-16 17:50:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dcaiafa@chromium.org/10082032/7001

[commit-bot: I haz the power, 2012-04-16 19:23:08]

Try job failure for 10082032-7001 (retry) on mac_rel for step
"remoting_unittests".
It's a second try, previously, step "remoting_unittests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[dcaiafa, 2012-04-16 20:24:22]

ptal.

The usage of a timer by the capturer broke the unittest since it was not setting
up a message loop.

[Wez, 2012-04-16 20:31:24]

lgtm

[commit-bot: I haz the power, 2012-04-16 20:43:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dcaiafa@chromium.org/10082032/16001

[commit-bot: I haz the power, 2012-04-16 22:15:24]

Change committed as 132475