Skip to content

Commit

Permalink
mac: Load the system hotkeys after launch. (reland)
Browse files Browse the repository at this point in the history
---------------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).

Review URL: https://codereview.chromium.org/408973002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@286737 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
erikchen@chromium.org committed Jul 31, 2014
1 parent 97b9212 commit cc405e4
Show file tree
Hide file tree
Showing 16 changed files with 621 additions and 113 deletions.
44 changes: 0 additions & 44 deletions chrome/browser/ui/cocoa/system_hotkey_map.h

This file was deleted.

51 changes: 0 additions & 51 deletions chrome/browser/ui/cocoa/system_hotkey_map_unittest.mm

This file was deleted.

2 changes: 0 additions & 2 deletions chrome/chrome_browser_ui.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -665,8 +665,6 @@
'browser/ui/cocoa/styled_text_field.mm',
'browser/ui/cocoa/styled_text_field_cell.h',
'browser/ui/cocoa/styled_text_field_cell.mm',
'browser/ui/cocoa/system_hotkey_map.h',
'browser/ui/cocoa/system_hotkey_map.mm',
'browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac.h',
'browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac.mm',
'browser/ui/cocoa/tab_contents/chrome_web_contents_view_delegate_mac.h',
Expand Down
1 change: 0 additions & 1 deletion chrome/chrome_tests_unit.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -1677,7 +1677,6 @@
'browser/ui/cocoa/styled_text_field_test_helper.h',
'browser/ui/cocoa/styled_text_field_test_helper.mm',
'browser/ui/cocoa/styled_text_field_unittest.mm',
'browser/ui/cocoa/system_hotkey_map_unittest.mm',
'browser/ui/cocoa/tab_contents/sad_tab_controller_unittest.mm',
'browser/ui/cocoa/tab_contents/sad_tab_view_unittest.mm',
'browser/ui/cocoa/table_row_nsimage_cache_unittest.mm',
Expand Down
2 changes: 2 additions & 0 deletions content/browser/browser_main_loop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@

#if defined(OS_MACOSX) && !defined(OS_IOS)
#include "content/browser/bootstrap_sandbox_mac.h"
#include "content/browser/cocoa/system_hotkey_helper_mac.h"
#include "content/browser/theme_helper_mac.h"
#endif

Expand Down Expand Up @@ -1048,6 +1049,7 @@ int BrowserMainLoop::BrowserThreadsStarted() {

#if defined(OS_MACOSX)
ThemeHelperMac::GetInstance();
SystemHotkeyHelperMac::GetInstance()->DeferredLoadSystemHotkeys();
if (ShouldEnableBootstrapSandbox()) {
TRACE_EVENT0("startup",
"BrowserMainLoop::BrowserThreadsStarted:BootstrapSandbox");
Expand Down
56 changes: 56 additions & 0 deletions content/browser/cocoa/system_hotkey_helper_mac.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CONTENT_BROWSER_COCOA_SYSTEM_HOTKEY_HELPER_MAC_H_
#define CONTENT_BROWSER_COCOA_SYSTEM_HOTKEY_HELPER_MAC_H_

#include "base/memory/singleton.h"
#include "base/memory/weak_ptr.h"

#ifdef __OBJC__
@class NSDictionary;
#else
class NSDictionary;
#endif

namespace content {

class SystemHotkeyMap;

// This singleton holds a global mapping of hotkeys reserved by OSX.
class SystemHotkeyHelperMac {
public:
// Return pointer to the singleton instance for the current process.
static SystemHotkeyHelperMac* GetInstance();

// Loads the system hot keys after a brief delay, to reduce file system access
// immediately after launch.
void DeferredLoadSystemHotkeys();

// Guaranteed to not be NULL.
SystemHotkeyMap* map() { return map_.get(); }

private:
friend struct DefaultSingletonTraits<SystemHotkeyHelperMac>;

SystemHotkeyHelperMac();
~SystemHotkeyHelperMac();

// Must be called from the FILE thread. Loads the file containing the system
// hotkeys into a NSDictionary* object, and passes the result to FileDidLoad
// on the UI thread.
void LoadSystemHotkeys();

// Must be called from the UI thread. This takes ownership of |dictionary|.
// Parses the system hotkeys from the plist stored in |dictionary|.
void FileDidLoad(NSDictionary* dictionary);

scoped_ptr<SystemHotkeyMap> map_;

DISALLOW_COPY_AND_ASSIGN(SystemHotkeyHelperMac);
};

} // namespace content

#endif // CONTENT_BROWSER_COCOA_SYSTEM_HOTKEY_HELPER_MAC_H_
77 changes: 77 additions & 0 deletions content/browser/cocoa/system_hotkey_helper_mac.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "content/browser/cocoa/system_hotkey_helper_mac.h"

#include "base/bind.h"
#include "base/files/file_path.h"
#include "base/mac/foundation_util.h"
#include "base/message_loop/message_loop.h"
#include "base/metrics/histogram.h"
#include "content/browser/cocoa/system_hotkey_map.h"
#include "content/public/browser/browser_thread.h"

namespace {

NSString* kSystemHotkeyPlistExtension =
@"/Preferences/com.apple.symbolichotkeys.plist";

// Amount of time to delay loading the hotkeys in seconds.
const int kLoadHotkeysDelaySeconds = 10;

} // namespace

namespace content {

// static
SystemHotkeyHelperMac* SystemHotkeyHelperMac::GetInstance() {
return Singleton<SystemHotkeyHelperMac>::get();
}

void SystemHotkeyHelperMac::DeferredLoadSystemHotkeys() {
BrowserThread::PostDelayedTask(
BrowserThread::FILE,
FROM_HERE,
base::Bind(&SystemHotkeyHelperMac::LoadSystemHotkeys,
base::Unretained(this)),
base::TimeDelta::FromSeconds(kLoadHotkeysDelaySeconds));
}

SystemHotkeyHelperMac::SystemHotkeyHelperMac() : map_(new SystemHotkeyMap) {
}

SystemHotkeyHelperMac::~SystemHotkeyHelperMac() {
}

void SystemHotkeyHelperMac::LoadSystemHotkeys() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));

std::string library_path(base::mac::GetUserLibraryPath().value());
NSString* expanded_file_path =
[NSString stringWithFormat:@"%s%@",
library_path.c_str(),
kSystemHotkeyPlistExtension];

// Loads the file into memory.
NSData* data = [NSData dataWithContentsOfFile:expanded_file_path];
// Intentionally create the object with +1 retain count, as FileDidLoad
// will destroy the object.
NSDictionary* dictionary = [SystemHotkeyMap::DictionaryFromData(data) retain];

BrowserThread::PostTask(BrowserThread::UI,
FROM_HERE,
base::Bind(&SystemHotkeyHelperMac::FileDidLoad,
base::Unretained(this),
dictionary));
}

void SystemHotkeyHelperMac::FileDidLoad(NSDictionary* dictionary) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

bool success = map()->ParseDictionary(dictionary);
UMA_HISTOGRAM_BOOLEAN("OSX.SystemHotkeyMap.LoadSuccess", success);
[dictionary release];
}

} // namespace content
66 changes: 66 additions & 0 deletions content/browser/cocoa/system_hotkey_map.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CONTENT_BROWSER_COCOA_SYSTEM_HOTKEY_MAP_H_
#define CONTENT_BROWSER_COCOA_SYSTEM_HOTKEY_MAP_H_

#import <Cocoa/Cocoa.h>
#include <vector>

#include "base/gtest_prod_util.h"
#include "base/macros.h"
#include "content/common/content_export.h"

namespace content {

struct SystemHotkey;

// Maintains a listing of all OSX system hotkeys. e.g. (cmd + `) These hotkeys
// should have higher priority than web content, so NSEvents that correspond to
// a system hotkey should not be passed to the renderer.
class CONTENT_EXPORT SystemHotkeyMap {
public:
SystemHotkeyMap();
~SystemHotkeyMap();

// Converts the plist stored in |data| into an NSDictionary. Returns nil on
// error.
static NSDictionary* DictionaryFromData(NSData* data);

// Parses the property list data commonly stored at
// ~/Library/Preferences/com.apple.symbolichotkeys.plist
// Returns false on encountering an irrecoverable error.
// Can be called multiple times. Only the results from the most recent
// invocation are stored.
bool ParseDictionary(NSDictionary* dictionary);

// Whether the event corresponds to a hotkey that has been reserved by the
// system.
bool IsEventReserved(NSEvent* event) const;

private:
FRIEND_TEST_ALL_PREFIXES(SystemHotkeyMapTest, Parse);
FRIEND_TEST_ALL_PREFIXES(SystemHotkeyMapTest, ParseCustomEntries);
FRIEND_TEST_ALL_PREFIXES(SystemHotkeyMapTest, ParseMouse);

// Whether the hotkey has been reserved by the user.
bool IsHotkeyReserved(unsigned short key_code, NSUInteger modifiers) const;

// Create at least one record of a hotkey that is reserved by the user.
// Certain system hotkeys automatically reserve multiple key combinations.
void ReserveHotkey(unsigned short key_code,
NSUInteger modifiers,
NSString* system_effect);

// Create a record of a hotkey that is reserved by the user.
void ReserveHotkey(unsigned short key_code, NSUInteger modifiers);

std::vector<SystemHotkey> system_hotkeys_;

DISALLOW_COPY_AND_ASSIGN(SystemHotkeyMap);
};

} // namespace content

#endif // CONTENT_BROWSER_COCOA_SYSTEM_HOTKEY_MAP_H_
Loading

0 comments on commit cc405e4

Please sign in to comment.