Make curtain-mode controllable by policy.

Make curtain-mode controllable by policy.

BUG=110111
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=151714

[Jamie, 2012-08-13 23:22:43]

Gary: PTAL at the policy changes.
Lambros: PTAL at the host changes.

https://chromiumcodereview.appspot.com/10829306/diff/2001/remoting/host/polic...
File remoting/host/policy_hack/policy_watcher.cc (right):

https://chromiumcodereview.appspot.com/10829306/diff/2001/remoting/host/polic...
remoting/host/policy_hack/policy_watcher.cc:127:
PolicyWatcher::kHostRequireCurtainPolicyName
This fixes a bug in the previous CL in which the policy was introduced.

https://chromiumcodereview.appspot.com/10829306/diff/2001/remoting/host/remot...
File remoting/host/remoting_me2me_host.cc (right):

https://chromiumcodereview.appspot.com/10829306/diff/2001/remoting/host/remot...
remoting/host/remoting_me2me_host.cc:331: StartHost();
Starting the host before calling any of the OnPolicy handlers means that they
can all assume that it exists.

[garykac, 2012-08-14 19:58:48]

lgtm for policy stuff

http://codereview.chromium.org/10829306/diff/2002/remoting/host/curtain_mode_...
File remoting/host/curtain_mode_mac.cc (right):

http://codereview.chromium.org/10829306/diff/2002/remoting/host/curtain_mode_...
remoting/host/curtain_mode_mac.cc:77: // TODO(jamiewalch): This code assumes at
most one client connection at a time.
Do we have a bug for adding support for multiple client connections? If so, it
would be nice to reference it here.

http://codereview.chromium.org/10829306/diff/2002/remoting/host/remoting_me2m...
File remoting/host/remoting_me2me_host.cc (right):

http://codereview.chromium.org/10829306/diff/2002/remoting/host/remoting_me2m...
remoting/host/remoting_me2me_host.cc:397: // daemon architecture
Is there a bug tracking the multi-proc daemon work?

[Lambros, 2012-08-14 20:21:06]

http://codereview.chromium.org/10829306/diff/2002/remoting/host/remoting_me2m...
File remoting/host/remoting_me2me_host.cc (right):

http://codereview.chromium.org/10829306/diff/2002/remoting/host/remoting_me2m...
remoting/host/remoting_me2me_host.cc:394: // the host system uncurtained.
I don't understand this.  Why does disconnection immediately after login leave
the host system uncurtained?  Why is this different from any other
disconnection?  Wouldn't disconnection from a desktop session also leave the
host system uncurtained?  Why is this a problem for the login screen but not for
a desktop session?

Anyway, it's not clear that our curtain implementation even works on the login
screen.  We fast-switch the display from the login screen to, what, the login
screen?

And even if uncurtaining on disconnection were a problem, couldn't we mitigate
this by activating the screen-lock?

[Jamie, 2012-08-14 22:04:22]

http://codereview.chromium.org/10829306/diff/2002/remoting/host/remoting_me2m...
File remoting/host/remoting_me2me_host.cc (right):

http://codereview.chromium.org/10829306/diff/2002/remoting/host/remoting_me2m...
remoting/host/remoting_me2me_host.cc:394: // the host system uncurtained.
On 2012/08/14 20:21:06, Lambros wrote:
> I don't understand this.  Why does disconnection immediately after login leave
> the host system uncurtained?  Why is this different from any other
> disconnection?  Wouldn't disconnection from a desktop session also leave the
> host system uncurtained?  Why is this a problem for the login screen but not
for
> a desktop session?
> 
> Anyway, it's not clear that our curtain implementation even works on the login
> screen.  We fast-switch the display from the login screen to, what, the login
> screen?
> 
> And even if uncurtaining on disconnection were a problem, couldn't we mitigate
> this by activating the screen-lock?

If you connect while at the login screen, we don't curtain. If you log in, you
are then disconnected because of the way we integrate with launchd. At that
point, the physical display is showing (or will very soon show) your uncurtained
desktop. Once you connect again, it will be curtained, but there's a window of
opportunity for someone at the host system to be naughty.

[Lambros, 2012-08-14 23:28:50]

lgtm after the comment is clarified.

http://codereview.chromium.org/10829306/diff/2002/remoting/host/remoting_me2m...
File remoting/host/remoting_me2me_host.cc (right):

http://codereview.chromium.org/10829306/diff/2002/remoting/host/remoting_me2m...
remoting/host/remoting_me2me_host.cc:394: // the host system uncurtained.
OK I get it now :)  The missing piece here is that our implementation of
curtain-mode does not work on the login screen, and so we don't do it.  (A
different implementation of curtain-mode would not have this problem.)  Please
can you rephrase the explanation to add this detail?

[commit-bot: I haz the power, 2012-08-15 00:09:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamiewalch@chromium.org/10829306/7002

[commit-bot: I haz the power, 2012-08-15 01:42:30]

Try job failure for 10829306-7002 (retry) on linux_rel for step
"interactive_ui_tests".
It's a second try, previously, step "interactive_ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-08-15 03:44:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamiewalch@chromium.org/10829306/7002

[commit-bot: I haz the power, 2012-08-15 05:16:13]

Try job failure for 10829306-7002 (retry) on linux_rel for step
"interactive_ui_tests".
It's a second try, previously, step "interactive_ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-08-15 05:28:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamiewalch@chromium.org/10829306/7002

[commit-bot: I haz the power, 2012-08-15 07:01:53]

Try job failure for 10829306-7002 (retry) on linux_rel for step
"interactive_ui_tests".
It's a second try, previously, step "interactive_ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-08-15 15:33:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamiewalch@chromium.org/10829306/7002

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

Try job failure for 10829306-7002 (retry) on win_rel for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[Jamie, 2012-08-15 18:22:56]

fyi

http://codereview.chromium.org/10829306/diff/2002/remoting/host/curtain_mode_...
File remoting/host/curtain_mode_mac.cc (right):

http://codereview.chromium.org/10829306/diff/2002/remoting/host/curtain_mode_...
remoting/host/curtain_mode_mac.cc:77: // TODO(jamiewalch): This code assumes at
most one client connection at a time.
On 2012/08/14 19:58:48, garykac wrote:
> Do we have a bug for adding support for multiple client connections? If so, it
> would be nice to reference it here.

There's no bug for this, and I don't think it's something we care about enough
right now to warrant one.

http://codereview.chromium.org/10829306/diff/2002/remoting/host/remoting_me2m...
File remoting/host/remoting_me2me_host.cc (right):

http://codereview.chromium.org/10829306/diff/2002/remoting/host/remoting_me2m...
remoting/host/remoting_me2me_host.cc:394: // the host system uncurtained.
On 2012/08/14 23:28:50, Lambros wrote:
> OK I get it now :)  The missing piece here is that our implementation of
> curtain-mode does not work on the login screen, and so we don't do it.  (A
> different implementation of curtain-mode would not have this problem.)  Please
> can you rephrase the explanation to add this detail?

Done.

http://codereview.chromium.org/10829306/diff/2002/remoting/host/remoting_me2m...
remoting/host/remoting_me2me_host.cc:397: // daemon architecture
On 2012/08/14 19:58:48, garykac wrote:
> Is there a bug tracking the multi-proc daemon work?

Yes. I've added a reference to it here.