Screen cap hotkey shouldn't be triggering an volume down event.

Hmm. Looks like ash::PowerButtonController observes key events to track the volume-down key state, and then takes a screenshot and returns early when powerd notifies Chrome that the power button is pressed if the volume-down key is already held. This means that there's nothing to impede the normal volume-down accelerator, and probably also means that the accelerator doesn't take a screenshot if the user happens to hold the power button before they press the volume button instead of the other way around.

Do minnie's volume buttons get reported as normal volume keys? I don't see how we can avoid this unless we delay the processing of the volume-down accelerator for a brief period of time to allow the power button to be pressed, and then also set some shared state when we see the power event to prevent the volume from being decreased later.

warx@ can you take this one?  Coordinate with afakhry@ and derat@ if you have questions.

OK. Let me look into it.

Question here, two options: (1) hold on the power button for some delay waiting for the volume down button pressed, (2) hold on the volume down button for some delay waiting for the power button pressed.

I prefer the first one. It looks like comment 2 preferring (2). Here is my reasoning. It seems to me volume button needs instant reaction more than power button locking screen.

What do you think?

Oh, I wasn't trying to say I prefer either. I actually think that both are terrible -- accelerators involving keys that usually perform actions or start animations on key-down seems like a bad idea, since adding a delay makes the UI feel janky. :-P

If we decide to introduce delays here, I think we should do it for both keys, though, since we don't know which users are going to press first.

Does this function have to strictly follow Android's example? 
Taking screenshots already feels "fussy" using power + volume down in Android's case.

What about a rapid doubleclick on just the power button? just throwing it out there...

Double-tapping the power button launches the camera app on Android.

I think that this is always going to feel fiddly. As a quick fix, how about hiding the volume slider just before taking the screenshot when volume-down + power is used?

That would work. Unless what you're trying to get a screenshot of is the volume slider.

I definitely DON'T want to introduce a delay.

FWIW it's really fiddly on Android too. I can only manage to get a screenshot without accidentally turning off my phone half of the time. :-/

As far as the volume-down is pressed and hold on, it will keep decreasing the volume. It must have a delay or check if power putton is pressed. But now as far as power button is pressed, it directly locks the screen. That is the conflict here. We should absolutely define a rule here.

'power + volume-down' looks inconvenient to chromebook either (at least for minnie), they are both on the left edge. It is a little hard to operate and easy to go wrong ways.

For comment 8, it still needs the volume adjust delay.

I think it would be better. And there's no need for double-tapping power to open the camera in cros. 

Unless you have a chromebook with a detachable keyboard and a camera on the back in the pipeline. 

Yeah, I thought about the intentional-screenshot-of-volume-slider case. If the slider is only hidden when the Volume-Down+Power combination is used, it'll still be possible to capture the slider by using Ctrl+Overview. Hopefully that's sufficient.

Power should not instantly lock.  The defined behavior for Power in Chrome OS is to have the windows expand and start to become translucent. It should lock if the button is held past that point.

What about this for volume:
1: User holds volume key
2: We remember the current volume
3: We immediately start reducing volume
4: If the power key is tapped while still holding volume we:
   A: Hide the volume slider
   B: Take the screenshot
   C: Restore the volume

The power-pressed-first case is not an issue since power should already have a delay

#13: That sounds great.

From my reading of the code, it looks like the accelerator won't work at all when holding the power button first and then pressing Volume-Down. Probably best to track that in another bug (if we even care about it -- I suspect that most users would expect Volume-Down to be the "modifier key" here and hold it first).

The power-pressed-first case doesn't have a delay on touch view mode (let me check the code to be sure). And I am wondering if so, the windows expansion could affect the screenshot. That means I never successfully took a screenshot on power first operation. This is probably another bug.

In regards to #13, while in Touchview, the lock action is triggered immediately when the power button is pressed as per  issue 371608 .  The animation still looks the same with the expanding windows, it's just that you can't "cancel" out of the lock action by releasing the power button.  As such, I don't think it's even possible, currently, to take a screenshot by pushing the power button first, and then the volume key.
However, when you press the <volume down> button first, and then the <power button> a screen shot is taken and the lock action is NOT triggered.

---

For the record, the <volume down> + <power button> combination is a workaround to allow screenshots to be taken while in Touchview because the regular keyboard is completely disabled.  (This CL shows how the keyboard was disabled: https://codereview.chromium.org/313913004/)

---

If we really need it we could use <volume up> + <power> to capture a screen shot with the volume bar, perhaps with the original volume value, and make <volume down> + <power> capture a screen shot without the volume bar, as per suggested in #13.

Heh. I hate it when my own bugs come back to haunt me. :-)

Seems like a combination of #13 and #14 will resolve this issue though.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fd7c37b98e7c6eca718257916ab65409ee7b2c1a

commit fd7c37b98e7c6eca718257916ab65409ee7b2c1a
Author: warx <warx@chromium.org>
Date: Fri Aug 05 19:38:07 2016

Fix Volume slider is captured in screenshot done in touchview mode

Solution:
1. User holds volume-down key.
2. Remember the non-repeated pressed volume.
3. Immediately start reducing volume.
4. If the power key is tapped,
4a TrayAudio hides the volume slider.
4b Taking the screenshot.
4c Restore the volume (do not notify UI)
BUG= 628479 
TEST=tested on veyron_minnie touchview mode and it works fine. The only defect I can image is taking screenshot when the device is playing audio. If volume-down is presssed for a long time, you can feel audio volume is firstly decreased and then restored back.

Review-Url: https://codereview.chromium.org/2190773002
Cr-Commit-Position: refs/heads/master@{#410145}

[modify] https://crrev.com/fd7c37b98e7c6eca718257916ab65409ee7b2c1a/ash/common/system/chromeos/network/tray_sms.cc
[modify] https://crrev.com/fd7c37b98e7c6eca718257916ab65409ee7b2c1a/ash/common/system/tray/system_tray.cc
[modify] https://crrev.com/fd7c37b98e7c6eca718257916ab65409ee7b2c1a/ash/common/system/tray/system_tray.h
[modify] https://crrev.com/fd7c37b98e7c6eca718257916ab65409ee7b2c1a/ash/common/system/tray/system_tray_item.cc
[modify] https://crrev.com/fd7c37b98e7c6eca718257916ab65409ee7b2c1a/ash/common/system/tray/system_tray_item.h
[modify] https://crrev.com/fd7c37b98e7c6eca718257916ab65409ee7b2c1a/ash/wm/power_button_controller.cc
[modify] https://crrev.com/fd7c37b98e7c6eca718257916ab65409ee7b2c1a/ash/wm/power_button_controller.h
[modify] https://crrev.com/fd7c37b98e7c6eca718257916ab65409ee7b2c1a/chromeos/audio/cras_audio_handler.cc
[modify] https://crrev.com/fd7c37b98e7c6eca718257916ab65409ee7b2c1a/chromeos/audio/cras_audio_handler.h
[modify] https://crrev.com/fd7c37b98e7c6eca718257916ab65409ee7b2c1a/chromeos/audio/cras_audio_handler_unittest.cc

Verified on ChromeOS  8697.0.0, 54.0.2826.0  veyron minnie