chromeos: Request screen lock directly from session manager.

chromeos: Request screen lock directly from session manager.

Screen lock and unlock requests were previously sent to
powerd, which just forwarded them on to the session manager
without doing anything extra.  It's simpler for Chrome to
just ask the session manager to lock or unlock directly.

I'm also moving the LockScreen and UnlockScreen
notifications from PowerManagerClient::Observer to
SessionManagerClient::Observer.  Confusingly, the signals
that prompt those notifications are sent by the session
manager but were being listened for in PowerManagerClient.

BUG=chromium-os:24003
TEST=manual: able to lock the screen from the UI, power button, and by closing the lid; unlocking works too


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

[Daniel Erat, 2012-07-03 22:44:27]

This can't go in until https://chromiumcodereview.appspot.com/10703081/ and
https://gerrit.chromium.org/gerrit/#change,26707 are in.

http://codereview.chromium.org/10693087/diff/4001/chromeos/dbus/session_manag...
File chromeos/dbus/session_manager_client.cc (right):

http://codereview.chromium.org/10693087/diff/4001/chromeos/dbus/session_manag...
chromeos/dbus/session_manager_client.cc:22: enum LockScreensState {
Rob, are these metrics still useful?  If not, I can delete a bunch of this code.

[flackr, 2012-07-04 12:55:52]

Can you also check that idle timeout still locks the screen?

http://codereview.chromium.org/10693087/diff/4001/chromeos/dbus/session_manag...
File chromeos/dbus/session_manager_client.cc (right):

http://codereview.chromium.org/10693087/diff/4001/chromeos/dbus/session_manag...
chromeos/dbus/session_manager_client.cc:22: enum LockScreensState {
On 2012/07/03 22:44:27, Daniel Erat wrote:
> Rob, are these metrics still useful?  If not, I can delete a bunch of this
code.

We still have to get lid and idle notifications from powerd right? It would be
useful to know if that path was failing.

[oshima, 2012-07-06 17:31:12]

lgtm with one question

http://codereview.chromium.org/10693087/diff/4001/chromeos/dbus/session_manag...
File chromeos/dbus/session_manager_client.h (right):

http://codereview.chromium.org/10693087/diff/4001/chromeos/dbus/session_manag...
chromeos/dbus/session_manager_client.h:66: virtual void RequestLockScreen() = 0;
will a lock request never fail?

[Daniel Erat, 2012-07-09 16:01:42]

I've confirmed that idle timeout still locks the screen.  (But I believe that
this is the same path that is used the user closes the lid -- in both cases, the
power manager is suspending, and the "Require password to wake from sleep"
setting tells it to ask the session manager to lock the screen beforehand.)

https://chromiumcodereview.appspot.com/10693087/diff/4001/chromeos/dbus/sessi...
File chromeos/dbus/session_manager_client.cc (right):

https://chromiumcodereview.appspot.com/10693087/diff/4001/chromeos/dbus/sessi...
chromeos/dbus/session_manager_client.cc:22: enum LockScreensState {
On 2012/07/04 12:55:52, flackr wrote:
> On 2012/07/03 22:44:27, Daniel Erat wrote:
> > Rob, are these metrics still useful?  If not, I can delete a bunch of this
> code.
> 
> We still have to get lid and idle notifications from powerd right? It would be
> useful to know if that path was failing.

Screen locking can be initiated by either Chrome or powerd; in both cases, the
process that wants the screen to be locked calls a method on the session manager
asking it to lock the screen*, which then emits a signal telling Chrome to lock,
after which Chrome calls a method on powerd telling it that the screen has been
locked.  Which part of the path are you hoping to cover with the metrics?  The
Chrome->session_manager->Chrome part of the path is pretty simple (or at least
will be, after this change is in).

* Chrome currently calls it indirectly via powerd; this is the bit that I'm
cleaning up here.

https://chromiumcodereview.appspot.com/10693087/diff/4001/chromeos/dbus/sessi...
File chromeos/dbus/session_manager_client.h (right):

https://chromiumcodereview.appspot.com/10693087/diff/4001/chromeos/dbus/sessi...
chromeos/dbus/session_manager_client.h:66: virtual void RequestLockScreen() = 0;
On 2012/07/06 17:31:12, oshima wrote:
> will a lock request never fail?

There's no way to report failure currently.  This is another issue with the way
that screen-locking currently works; I'll post a more detailed explanation of
the current state and the way that things should work instead in the bug after
this change is in.

[flackr, 2012-07-09 18:19:57]

LGTM

https://chromiumcodereview.appspot.com/10693087/diff/4001/chromeos/dbus/sessi...
File chromeos/dbus/session_manager_client.cc (right):

https://chromiumcodereview.appspot.com/10693087/diff/4001/chromeos/dbus/sessi...
chromeos/dbus/session_manager_client.cc:22: enum LockScreensState {
On 2012/07/09 16:01:42, Daniel Erat wrote:
> On 2012/07/04 12:55:52, flackr wrote:
> > On 2012/07/03 22:44:27, Daniel Erat wrote:
> > > Rob, are these metrics still useful?  If not, I can delete a bunch of this
> > code.
> > 
> > We still have to get lid and idle notifications from powerd right? It would
be
> > useful to know if that path was failing.
> 
> Screen locking can be initiated by either Chrome or powerd; in both cases, the
> process that wants the screen to be locked calls a method on the session
manager
> asking it to lock the screen*, which then emits a signal telling Chrome to
lock,
> after which Chrome calls a method on powerd telling it that the screen has
been
> locked.  Which part of the path are you hoping to cover with the metrics?  The
> Chrome->session_manager->Chrome part of the path is pretty simple (or at least
> will be, after this change is in).
> 
> * Chrome currently calls it indirectly via powerd; this is the bit that I'm
> cleaning up here.

Thanks for clarifying, I don't think the current metrics are useful anymore
then. The original problem was the chrome -> power_manager -> chrome path was
failing.

[Daniel Erat, 2012-07-09 18:46:40]

Thanks, I've removed the metrics code.

[commit-bot: I haz the power, 2012-07-09 19:17:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/derat@chromium.org/10693087/18001

[commit-bot: I haz the power, 2012-07-09 20:32:20]

Change committed as 145737