mac: Load the system hotkeys after launch. (reland)

mac: Load the system hotkeys after launch. (reland)

---------------Reland CL Description---------------------------
System mouse hotkeys were incorrectly being parsed as system keyboard hotkeys.

Other minor changes include:
- Expanded unit tests to include a sparsely populated symbolichotkeys.plist
from a real machine.
- Use correct types for the key_code and modifiers of the event.
- An event must have at least one of ctr/alt/cmd down to be considered a
hotkey.
- Use the NSDeviceIndependentModifierFlagsMask mask to prune out
device-dependent modifier flags, including event coalescing information.

---------------Original CL Description---------------------------
Original CL: https://codereview.chromium.org/370293004/

Shortly after launch, the system hotkeys are loaded and parsed. If a hotkey is
reserved by the system, it is not passed to the renderer. This allows system
hotkeys like (cmd + `) to work even if a flash plugin is selected.

Add a histogram to ensure that the system hotkey plist is being correctly
loaded and parsed.

BUG=383558, 395187
TEST=Open 2 Chrome windows. Navigate one to www.twitch.tv. The site should
include a flash plugin that automatically starts playing a video. Select the
flash plugin. The hotkey combination (cmd + `) should switch between the open
windows. The hotkey combination (cmd + L) should have no effect (it is a Chrome
hotkey, not a browser hotkey).

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=286737

[erikchen, 2014-07-21 23:10:27]

rsesek: Please review.

[Robert Sesek, 2014-07-22 17:15:25]

https://codereview.chromium.org/408973002/diff/60001/content/browser/cocoa/sy...
File content/browser/cocoa/system_hotkey_map_unittest.mm (right):

https://codereview.chromium.org/408973002/diff/60001/content/browser/cocoa/sy...
content/browser/cocoa/system_hotkey_map_unittest.mm:17: NSData*
DataFromTestFile(const char* file) {
I'd put this as a method on SystemHotkeyMapTest.

https://codereview.chromium.org/408973002/diff/60001/content/browser/cocoa/sy...
content/browser/cocoa/system_hotkey_map_unittest.mm:96: int key_code =
kVK_ANSI_Grave;
Use the right types here, and on line 97.

[erikchen, 2014-07-22 18:30:57]

rsesek: PTAL

Diff patch set 4 against patch set 3.

https://codereview.chromium.org/408973002/diff/60001/content/browser/cocoa/sy...
File content/browser/cocoa/system_hotkey_map_unittest.mm (right):

https://codereview.chromium.org/408973002/diff/60001/content/browser/cocoa/sy...
content/browser/cocoa/system_hotkey_map_unittest.mm:17: NSData*
DataFromTestFile(const char* file) {
On 2014/07/22 17:15:24, rsesek wrote:
> I'd put this as a method on SystemHotkeyMapTest.

Done.

https://codereview.chromium.org/408973002/diff/60001/content/browser/cocoa/sy...
content/browser/cocoa/system_hotkey_map_unittest.mm:96: int key_code =
kVK_ANSI_Grave;
On 2014/07/22 17:15:24, rsesek wrote:
> Use the right types here, and on line 97.

Done.

[Robert Sesek, 2014-07-23 15:34:07]

Is the regression test for
https://code.google.com/p/chromium/issues/detail?id=395187 just parsing the
mouse keys? Also, you should set BUG= to that as well.

https://codereview.chromium.org/408973002/diff/100001/content/browser/cocoa/s...
File content/browser/cocoa/system_hotkey_map_unittest.mm (right):

https://codereview.chromium.org/408973002/diff/100001/content/browser/cocoa/s...
content/browser/cocoa/system_hotkey_map_unittest.mm:64:
EXPECT_FALSE(map.IsHotkeyReserved(key_code, modifiers));
You don't ever test if modifiers = 0.

[erikchen, 2014-07-23 18:05:34]

rsesek: PTAL

https://codereview.chromium.org/408973002/diff/100001/content/browser/cocoa/s...
File content/browser/cocoa/system_hotkey_map_unittest.mm (right):

https://codereview.chromium.org/408973002/diff/100001/content/browser/cocoa/s...
content/browser/cocoa/system_hotkey_map_unittest.mm:64:
EXPECT_FALSE(map.IsHotkeyReserved(key_code, modifiers));
On 2014/07/23 15:34:07, rsesek wrote:
> You don't ever test if modifiers = 0.

I added a new batch of tests.

[Robert Sesek, 2014-07-23 18:36:31]

LGTM

[erikchen, 2014-07-23 18:39:40]

isherman: Looking for an owner review of
tools/metrics/histograms/histograms.xml. This CL is a reland, and no changes
were made to that file.

[Ilya Sherman, 2014-07-23 19:45:18]

On 2014/07/23 18:39:40, erikchen wrote:
> isherman: Looking for an owner review of
> tools/metrics/histograms/histograms.xml. This CL is a reland, and no changes
> were made to that file.

LGTM.  For future reference, it's fine to TBR an owner when relanding code that
has already been reviewed, and that is unchanged from the previous code review.

[erikchen, 2014-07-23 19:49:27]

avi: Please review.

[Avi (use Gerrit), 2014-07-28 15:25:13]

On 2014/07/23 19:49:27, erikchen wrote:
> avi: Please review.

Is there something that you want me to look at?

[erikchen, 2014-07-28 17:36:40]

avi: This is a reland of a change for which you were an original reviewer.
Diffing patch set 3 against patch set 1 shows the functional difference and a
unit test. Diffing patch set 6 against patch set 3 shows another unit test. 

The full set of changes is included in the CL description (see Reland CL
Description)

[Avi (use Gerrit), 2014-07-28 17:55:59]

Thanks!

This all LGTM.

[erikchen, 2014-07-30 17:10:48]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2014-07-30 17:12:13]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/erikchen@chromium.org/408973002/140001

[commit-bot: I haz the power, 2014-07-30 19:09:12]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-07-31 00:54:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-31 00:54:33]

Try jobs failed on following builders:
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[erikchen, 2014-07-31 07:23:28]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2014-07-31 07:25:35]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/erikchen@chromium.org/408973002/140001

[commit-bot: I haz the power, 2014-07-31 11:35:58]

Change committed as 286737