Ash: Support "Next Song", "Previous Song", "Play/Pause" Multi Media buttons

Ash: Support "Next Song", "Previous Song", "Play/Pause" Multi Media buttons
in our Media Player that is part of the File Manager right now.

Added shortcut handlers and plumbing to the Audio and Video player of the
Ash File Manager.

see also:
http://code.google.com/p/chromium/issues/detail?id=123739#c17

BUG=138745
TEST=Attach an (Wired USB) Apple keyboard or Windows Natural keyboard
to a Chromebook.
Either download three mp3 and an mp4 file to your chromebook or navigate
to a Google Drive folder that has these.
Bring up File Manager from the Launcher and click on an mp3.
Press the Previous Track, Play/Pause, Next Track keys on the MAC keyboard and
observe same behavior as clicking the corresponding Audio Control buttons
on the screen.
Click on an mp4 and press the Play/Pause key and observe same
behavior as clicking Play/Pause button on the screen.
Repeat for Windows Natural keyboard with the Play/Pause key. (Note
it does not have any prev./next track keys.)
PS: The Goldtouch USB keyboard works as well. The media keys require holding done the "Fn" key and F1, F2, or F3.

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

[Daniel Erat, 2012-08-21 20:38:41]

I don't have any background on the extension part of this but the rest LGTM.

https://chromiumcodereview.appspot.com/10823439/diff/1/chrome/browser/chromeo...
File chrome/browser/chromeos/extensions/media_player_event_router.cc (right):

https://chromiumcodereview.appspot.com/10823439/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/media_player_event_router.cc:28:
"mediaPlayerPrivate.onNextTrack", args.Pass(), NULL, GURL());
nit: indent four spaces beyond previous line

https://chromiumcodereview.appspot.com/10823439/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/media_player_event_router.cc:44:
"mediaPlayerPrivate.onPrevTrack", args.Pass(), NULL, GURL());
ditto

https://chromiumcodereview.appspot.com/10823439/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/media_player_event_router.cc:52:
"mediaPlayerPrivate.onTogglePlayState", args.Pass(), NULL, GURL());
ditto

[sschmitz, 2012-08-21 20:44:03]

Dan, thanks for the speedy reply. I have the following dirs in mind
for each reviewer:

ash/accelerators/
ash/shell/
ash/
ash/test/
chrome/browser/ui/ash/
- derat@chromium.org

chrome/browser/chromeos/extensions/
chrome/common/extensions/api/
- asargent@chromium.org

chrome/browser/resources/file_manager/js/
- serya@chromium.org

On Tue, Aug 21, 2012 at 1:38 PM,  <derat@chromium.org> wrote:
>
> I don't have any background on the extension part of this but the rest LGTM.
>
>
>
https://chromiumcodereview.appspot.com/10823439/diff/1/chrome/browser/chromeo...
> File chrome/browser/chromeos/extensions/media_player_event_router.cc
> (right):
>
>
https://chromiumcodereview.appspot.com/10823439/diff/1/chrome/browser/chromeo...
> chrome/browser/chromeos/extensions/media_player_event_router.cc:28:
> "mediaPlayerPrivate.onNextTrack", args.Pass(), NULL, GURL());
> nit: indent four spaces beyond previous line
>
>
https://chromiumcodereview.appspot.com/10823439/diff/1/chrome/browser/chromeo...
> chrome/browser/chromeos/extensions/media_player_event_router.cc:44:
> "mediaPlayerPrivate.onPrevTrack", args.Pass(), NULL, GURL());
> ditto
>
>
https://chromiumcodereview.appspot.com/10823439/diff/1/chrome/browser/chromeo...
> chrome/browser/chromeos/extensions/media_player_event_router.cc:52:
> "mediaPlayerPrivate.onTogglePlayState", args.Pass(), NULL, GURL());
> ditto
>
> https://chromiumcodereview.appspot.com/10823439/

[sschmitz, 2012-08-21 20:55:48]

https://chromiumcodereview.appspot.com/10823439/diff/1/chrome/browser/chromeo...
File chrome/browser/chromeos/extensions/media_player_event_router.cc (right):

https://chromiumcodereview.appspot.com/10823439/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/media_player_event_router.cc:28:
"mediaPlayerPrivate.onNextTrack", args.Pass(), NULL, GURL());
On 2012/08/21 20:38:42, Daniel Erat wrote:
> nit: indent four spaces beyond previous line

Done.

https://chromiumcodereview.appspot.com/10823439/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/media_player_event_router.cc:44:
"mediaPlayerPrivate.onPrevTrack", args.Pass(), NULL, GURL());
On 2012/08/21 20:38:42, Daniel Erat wrote:
> ditto

Done.

https://chromiumcodereview.appspot.com/10823439/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/media_player_event_router.cc:52:
"mediaPlayerPrivate.onTogglePlayState", args.Pass(), NULL, GURL());
On 2012/08/21 20:38:42, Daniel Erat wrote:
> ditto

Done.

[asargent_no_longer_on_chrome, 2012-08-21 22:20:25]

LGTM

https://chromiumcodereview.appspot.com/10823439/diff/6001/chrome/browser/chro...
File chrome/browser/chromeos/extensions/media_player_event_router.cc (right):

https://chromiumcodereview.appspot.com/10823439/diff/6001/chrome/browser/chro...
chrome/browser/chromeos/extensions/media_player_event_router.cc:28:
"mediaPlayerPrivate.onNextTrack", args.Pass(), NULL, GURL());
Can you just put "new ListValue()" instead of having a scoped_ptr that you call
.Pass on? I vaguely recall there being some trick about rvalues vs lvalues for
this case in our template magic for scoped_ptr arguments to functions. If I'm
wrong no worries though. 

(same question below)

[sschmitz, 2012-08-21 23:52:10]

On 2012/08/21 22:20:25, Antony Sargent wrote:
> LGTM
> 
>
https://chromiumcodereview.appspot.com/10823439/diff/6001/chrome/browser/chro...
> File chrome/browser/chromeos/extensions/media_player_event_router.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10823439/diff/6001/chrome/browser/chro...
> chrome/browser/chromeos/extensions/media_player_event_router.cc:28:
> "mediaPlayerPrivate.onNextTrack", args.Pass(), NULL, GURL());
> Can you just put "new ListValue()" instead of having a scoped_ptr that you
call
> .Pass on? I vaguely recall there being some trick about rvalues vs lvalues for
> this case in our template magic for scoped_ptr arguments to functions. If I'm
> wrong no worries though. 
> 
> (same question below)

I don't think I can pass "new ListValue()" instead. The conversion constructor
in scoped_ptr is marked 'explicit', disallowing implicit type conversions....
besides I tried it and got a corresponding compile error: "no matching function
call".
Thanks for your quick review.

[sschmitz, 2012-08-22 00:11:55]

https://chromiumcodereview.appspot.com/10823439/diff/6001/chrome/browser/chro...
File chrome/browser/chromeos/extensions/media_player_event_router.cc (right):

https://chromiumcodereview.appspot.com/10823439/diff/6001/chrome/browser/chro...
chrome/browser/chromeos/extensions/media_player_event_router.cc:28:
"mediaPlayerPrivate.onNextTrack", args.Pass(), NULL, GURL());
On 2012/08/21 22:20:26, Antony Sargent wrote:
> Can you just put "new ListValue()" instead of having a scoped_ptr that you
call
> .Pass on? I vaguely recall there being some trick about rvalues vs lvalues for
> this case in our template magic for scoped_ptr arguments to functions. If I'm
> wrong no worries though. 
> 
> (same question below)
> 

The short answer is no. (see my email reply)

[sschmitz, 2012-08-22 16:06:37]

Adding kaznacheev@ as reviewer. I did not hear from SeRya@. Either one of you to
review this soon would be great. Thanks.

[sschmitz, 2012-08-24 22:57:26]

Adding rginda as reviewer: Robert would you please review
chrome/browser/resources/file_manager/js/media/media_controls.js.
I have not heard from serya or kaznacheev for several days. Thanks.

[Vladislav Kaznacheev, 2012-08-27 10:18:48]

LGTM chrome/browser/resources/file_manager/js/media/media_controls.js

[commit-bot: I haz the power, 2012-08-27 15:05:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sschmitz@chromium.org/10823439/6001

[commit-bot: I haz the power, 2012-08-27 17:38:02]

Change committed as 153502