Condition added to prevent lock animation on Supervised User creation.

Condition added to prevent lock animation on Supervised User creation.

BUG=430642
TBR=oshima@chromium.org

Committed: https://crrev.com/34195f8c24ba24267674c78d91b887bf3d955ea4
Cr-Commit-Position: refs/heads/master@{#312856}

[merkulova, 2014-12-23 13:31:45]

merkulova@chromium.org changed reviewers:
+ dzhioev@chromium.org

[dzhioev (left Google), 2014-12-23 14:08:52]

Please, add a test for this fix to ash/wm/lock_state_controller_unittest.cc

https://codereview.chromium.org/826483002/diff/1/ash/wm/power_button_controll...
File ash/wm/power_button_controller.cc (right):

https://codereview.chromium.org/826483002/diff/1/ash/wm/power_button_controll...
ash/wm/power_button_controller.cc:75: if (has_legacy_power_button_) {
I think this branch should be updated as well, though I'm not quite sure how.

https://codereview.chromium.org/826483002/diff/1/ash/wm/power_button_controll...
ash/wm/power_button_controller.cc:95: !session_state_delegate->IsScreenLocked()
&&
IsScreenLocked() implies IsUserSessionBlocked(), so you can drop
!IsScreenLocked() condition.

[merkulova, 2014-12-23 14:49:11]

merkulova@chromium.org changed reviewers:
+ pkotwicz@chromium.org

[merkulova, 2014-12-23 14:50:39]

+pkotwicz@

Unit test is not useful here, it leads to testing implementation, not behavior.
It requires a browsertest.

[pkotwicz, 2014-12-23 18:40:24]

pkotwicz@chromium.org changed reviewers:
+ derat@chromium.org

[pkotwicz, 2014-12-23 18:40:25]

Replacing myself with derat@ (who is an ash/ OWNER) derat@ wrote most of
power_button_controller.cc According to his calendar, his availability is better
than mine over the next two weeks.

https://codereview.chromium.org/826483002/diff/20001/ash/accelerators/acceler...
File ash/accelerators/accelerator_controller.cc (right):

https://codereview.chromium.org/826483002/diff/20001/ash/accelerators/acceler...
ash/accelerators/accelerator_controller.cc:1213: // There is no power in linux
desktop, so call the
I think that the comment should stay as is. powerd is the name of a service
which runs on Chromebooks.

[Daniel Erat, 2014-12-23 18:50:47]

lgtm after you update or revert the comment change

https://codereview.chromium.org/826483002/diff/20001/ash/accelerators/acceler...
File ash/accelerators/accelerator_controller.cc (right):

https://codereview.chromium.org/826483002/diff/20001/ash/accelerators/acceler...
ash/accelerators/accelerator_controller.cc:1213: // There is no power in linux
desktop, so call the
On 2014/12/23 18:40:25, pkotwicz wrote:
> I think that the comment should stay as is. powerd is the name of a service
> which runs on Chromebooks.

yes. feel free to reword this as something like "powerd, the Chrome OS power
manager, isn't present on Linux desktops, so notify PowerButtonController
instead." if that's clearer.

[merkulova, 2014-12-24 13:08:34]

https://codereview.chromium.org/826483002/diff/20001/ash/accelerators/acceler...
File ash/accelerators/accelerator_controller.cc (right):

https://codereview.chromium.org/826483002/diff/20001/ash/accelerators/acceler...
ash/accelerators/accelerator_controller.cc:1213: // There is no power in linux
desktop, so call the
On 2014/12/23 18:50:47, Daniel Erat wrote:
> On 2014/12/23 18:40:25, pkotwicz wrote:
> > I think that the comment should stay as is. powerd is the name of a service
> > which runs on Chromebooks.
> 
> yes. feel free to reword this as something like "powerd, the Chrome OS power
> manager, isn't present on Linux desktops, so notify PowerButtonController
> instead." if that's clearer.

Thanks

[dzhioev (left Google), 2015-01-23 13:12:29]

On 2014/12/24 13:08:34, merkulova wrote:
>
https://codereview.chromium.org/826483002/diff/20001/ash/accelerators/acceler...
> File ash/accelerators/accelerator_controller.cc (right):
> 
>
https://codereview.chromium.org/826483002/diff/20001/ash/accelerators/acceler...
> ash/accelerators/accelerator_controller.cc:1213: // There is no power in linux
> desktop, so call the
> On 2014/12/23 18:50:47, Daniel Erat wrote:
> > On 2014/12/23 18:40:25, pkotwicz wrote:
> > > I think that the comment should stay as is. powerd is the name of a
service
> > > which runs on Chromebooks.
> > 
> > yes. feel free to reword this as something like "powerd, the Chrome OS power
> > manager, isn't present on Linux desktops, so notify PowerButtonController
> > instead." if that's clearer.
> 
> Thanks

LGTM

[merkulova, 2015-01-23 13:12:58]

The CQ bit was checked by merkulova@chromium.org

[commit-bot: I haz the power, 2015-01-23 13:13:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/826483002/40001

[commit-bot: I haz the power, 2015-01-23 16:05:29]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-01-23 16:06:28]

Patchset 3 (id:??) landed as
https://crrev.com/34195f8c24ba24267674c78d91b887bf3d955ea4
Cr-Commit-Position: refs/heads/master@{#312856}