New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 25
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Feature



Sign in to add a comment
link

Issue 902379: Global Media Controls: Add Windows implementation of MediaKeysListener

Reported by steimel@chromium.org, Nov 6 Project Member

Issue description

We need to create a Windows implementaiton of MediaKeysListener that can be used to capture media keys events for global media controls
 

Comment 1 by bugdroid1@chromium.org, Dec 11

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5dc65dbd15d3ef846eef2b1a34721dd45140ece0

commit 5dc65dbd15d3ef846eef2b1a34721dd45140ece0
Author: Tommy Steimel <steimel@chromium.org>
Date: Tue Dec 11 22:18:27 2018

[Global Media Controls] Add GlobalMediaKeysListenerWin implementation

This CL adds a Windows implementation of a global MediaKeysListener.
This implementation uses a KeyboardHook to receive media key events.

Bug:  902379 
Change-Id: Ieb4003292e2a0622e500b83c1d8b298129e1f1fe
Reviewed-on: https://chromium-review.googlesource.com/c/1362197
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615685}
[modify] https://crrev.com/5dc65dbd15d3ef846eef2b1a34721dd45140ece0/chrome/test/BUILD.gn
[modify] https://crrev.com/5dc65dbd15d3ef846eef2b1a34721dd45140ece0/ui/base/BUILD.gn
[add] https://crrev.com/5dc65dbd15d3ef846eef2b1a34721dd45140ece0/ui/base/accelerators/global_media_keys_listener_win.cc
[add] https://crrev.com/5dc65dbd15d3ef846eef2b1a34721dd45140ece0/ui/base/accelerators/global_media_keys_listener_win.h
[add] https://crrev.com/5dc65dbd15d3ef846eef2b1a34721dd45140ece0/ui/base/accelerators/global_media_keys_listener_win_interactive_test.cc
[modify] https://crrev.com/5dc65dbd15d3ef846eef2b1a34721dd45140ece0/ui/base/accelerators/media_keys_listener.h
[modify] https://crrev.com/5dc65dbd15d3ef846eef2b1a34721dd45140ece0/ui/base/accelerators/media_keys_listener_mac.mm
[add] https://crrev.com/5dc65dbd15d3ef846eef2b1a34721dd45140ece0/ui/base/accelerators/media_keys_listener_win.cc
[modify] https://crrev.com/5dc65dbd15d3ef846eef2b1a34721dd45140ece0/ui/events/win/keyboard_hook_win_base.cc

Comment 2 by bugdroid1@chromium.org, Dec 14

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5eb1773a439166fee05887db3375e0e809da27fe

commit 5eb1773a439166fee05887db3375e0e809da27fe
Author: Tommy Steimel <steimel@chromium.org>
Date: Fri Dec 14 19:00:12 2018

Update MediaKeysListener API to set specific keys to listen to

This CL updates the MediaKeysListener API to listen for a specific set
of keys instead of listening to all media keys and depending on a
response from the MediaKeysListener::Delegate to decide whether or not
to propagate.

The advantage of this change is that it allows the
MediaKeysListener::Delegate::OnMediaKeysAccelerator callback to be
called on the delegate's own thread for the Windows implementation.
Before, since the Windows implementation depends on a keyboard hook
which runs on a dedicated thread, it needed to immediately know the
delegate's response so that it would know whether or not to propagate
the key event. Now, the listener is already aware of which keys to
propagate and can immediately propagate (or not) and post a task to
the main thread to call delegate_->OnMediaKeysAccelerator
asynchronously.

Bug:  902379 
Change-Id: Ib7cb27bd6bbcec382fb4a050166df19aa9131673
Reviewed-on: https://chromium-review.googlesource.com/c/1374832
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616770}
[modify] https://crrev.com/5eb1773a439166fee05887db3375e0e809da27fe/chrome/browser/extensions/extension_keybinding_registry.cc
[modify] https://crrev.com/5eb1773a439166fee05887db3375e0e809da27fe/chrome/browser/extensions/extension_keybinding_registry.h
[modify] https://crrev.com/5eb1773a439166fee05887db3375e0e809da27fe/chrome/browser/extensions/global_shortcut_listener_mac.h
[modify] https://crrev.com/5eb1773a439166fee05887db3375e0e809da27fe/chrome/browser/extensions/global_shortcut_listener_mac.mm
[modify] https://crrev.com/5eb1773a439166fee05887db3375e0e809da27fe/chrome/common/extensions/command.cc
[modify] https://crrev.com/5eb1773a439166fee05887db3375e0e809da27fe/ui/base/accelerators/global_media_keys_listener_win.cc
[modify] https://crrev.com/5eb1773a439166fee05887db3375e0e809da27fe/ui/base/accelerators/global_media_keys_listener_win.h
[modify] https://crrev.com/5eb1773a439166fee05887db3375e0e809da27fe/ui/base/accelerators/global_media_keys_listener_win_interactive_test.cc
[modify] https://crrev.com/5eb1773a439166fee05887db3375e0e809da27fe/ui/base/accelerators/media_keys_listener.cc
[modify] https://crrev.com/5eb1773a439166fee05887db3375e0e809da27fe/ui/base/accelerators/media_keys_listener.h
[modify] https://crrev.com/5eb1773a439166fee05887db3375e0e809da27fe/ui/base/accelerators/media_keys_listener_mac.mm

Comment 3 by bugdroid1@chromium.org, Jan 11

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2d655f60eebc3d2484e68eb79e779ef6f75666dc

commit 2d655f60eebc3d2484e68eb79e779ef6f75666dc
Author: Tommy Steimel <steimel@chromium.org>
Date: Fri Jan 11 06:25:44 2019

[Global Media Controls] RegisterHotKey for GlobalMediaKeysListenerWin

This CL refactors the GlobalMediaKeysListenerWin to use RegisterHotKey
to receive media key input instead of using a low-level keyboard hook.

This has a few key advantages:
- No need for separate, dedicated thread to avoid key input delays
- Will be cleaner to use with GlobalShortcutListenerWin since we need
  to support extension shortcut listeners alongside the upcoming
  HardwareKeyMediaController
- Will play more nicely with other native Windows applications (albeit
  at the cost of being slightly less powerful)

Bug:  902379 
Change-Id: I055df46a0231a4abb08cf3786fc58140c2a47241
Reviewed-on: https://chromium-review.googlesource.com/c/1401363
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621922}
[modify] https://crrev.com/2d655f60eebc3d2484e68eb79e779ef6f75666dc/chrome/browser/extensions/global_shortcut_listener_win.cc
[modify] https://crrev.com/2d655f60eebc3d2484e68eb79e779ef6f75666dc/chrome/browser/extensions/global_shortcut_listener_win.h
[modify] https://crrev.com/2d655f60eebc3d2484e68eb79e779ef6f75666dc/tools/metrics/histograms/histograms.xml
[modify] https://crrev.com/2d655f60eebc3d2484e68eb79e779ef6f75666dc/ui/base/accelerators/global_media_keys_listener_win.cc
[modify] https://crrev.com/2d655f60eebc3d2484e68eb79e779ef6f75666dc/ui/base/accelerators/global_media_keys_listener_win.h
[modify] https://crrev.com/2d655f60eebc3d2484e68eb79e779ef6f75666dc/ui/base/accelerators/global_media_keys_listener_win_interactive_test.cc
[modify] https://crrev.com/2d655f60eebc3d2484e68eb79e779ef6f75666dc/ui/base/accelerators/media_keys_listener.h
[modify] https://crrev.com/2d655f60eebc3d2484e68eb79e779ef6f75666dc/ui/base/accelerators/media_keys_listener_mac.mm
[modify] https://crrev.com/2d655f60eebc3d2484e68eb79e779ef6f75666dc/ui/gfx/BUILD.gn
[add] https://crrev.com/2d655f60eebc3d2484e68eb79e779ef6f75666dc/ui/gfx/win/singleton_hwnd_hot_key_observer.cc
[add] https://crrev.com/2d655f60eebc3d2484e68eb79e779ef6f75666dc/ui/gfx/win/singleton_hwnd_hot_key_observer.h
[modify] https://crrev.com/2d655f60eebc3d2484e68eb79e779ef6f75666dc/ui/gfx/win/singleton_hwnd_observer.h

Comment 4 by steimel@chromium.org, Jan 25

Status: Fixed (was: Started)

Sign in to add a comment