diff options
author | suzhe@chromium.org <suzhe@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-12-10 06:01:48 +0000 |
---|---|---|
committer | suzhe@chromium.org <suzhe@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-12-10 06:01:48 +0000 |
commit | 867125a08f77a22b770f197d90519a8672af83aa (patch) | |
tree | 3d2ba6406f0932155b333af29bb2d9bff3e2d276 /chrome | |
parent | 44d64924960f793fd92f964d1e349216757c1856 (diff) | |
download | chromium_src-867125a08f77a22b770f197d90519a8672af83aa.zip chromium_src-867125a08f77a22b770f197d90519a8672af83aa.tar.gz chromium_src-867125a08f77a22b770f197d90519a8672af83aa.tar.bz2 |
Refactor the keyboard events handling code related to RenderViewHostDelegate::View and TabContentsDelegate interfaces.
Significant changes made by this CL:
1. The keyboard event handling code has been moved from TabContentsView implementation classes into BrowserWindow implementation classes.
Please refer to this discussion thread: http://groups.google.com/group/chromium-dev/browse_thread/thread/e6e0b5cc105659b7/9953c4308bb0000c
This change makes the keyboard event flow comply with the relationship between TabContents/TabContentsView and TabContentsDelegate/BrowserWindow.
Besides it, the code is also simplified a lot, for example, the keyboard event handling code in chrome/browser/views/tab_contents/tab_contents_view_{gtk,win}.cc are now merged into one copy and moved into chrome/browser/views/frame/browser_view.cc.
2. A pre-handle phrase has been added into the keyboard event handling flow. A keyboard event will be first sent to the browser for pre-handling before being sent to the renderer. Then if the event was not handled by the renderer, it'll be sent to the browser again for post-handling.
3. The keyboard accelerator handling code of Windows and Linux ports have been optimized to get rid off extra command lookup.
4. The keyboard event message flow between the browser and the renderer is changed back to full async mode, all complex logics introduced by revision 29857 are removed.
BUG=24479, 26054, 26131, 28839
TEST=See bug reports.
Review URL: http://codereview.chromium.org/400012
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@34234 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
41 files changed, 753 insertions, 923 deletions
diff --git a/chrome/browser/blocked_popup_container_interactive_uitest.cc b/chrome/browser/blocked_popup_container_interactive_uitest.cc index 754be30..e69a934 100644 --- a/chrome/browser/blocked_popup_container_interactive_uitest.cc +++ b/chrome/browser/blocked_popup_container_interactive_uitest.cc @@ -283,7 +283,7 @@ TEST_F(BrowserInteractiveTest, ReserveKeyboardAccelerators) { #if !defined(OS_MACOSX) // see BrowserWindowCocoa::GetCommandId ASSERT_TRUE(browser->ActivateTab(1)); ASSERT_TRUE(window->SimulateOSKeyPress( - base::VKEY_W, views::Event::EF_CONTROL_DOWN)); + base::VKEY_F4, views::Event::EF_CONTROL_DOWN)); ASSERT_TRUE(browser->WaitForTabCountToBecome(1, action_max_timeout_ms())); #endif } diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc index cf1af8c..a52e157 100644 --- a/chrome/browser/browser.cc +++ b/chrome/browser/browser.cc @@ -140,7 +140,10 @@ Browser::Browser(Type type, Profile* profile) is_attempting_to_close_browser_(false), cancel_download_confirmation_state_(NOT_PROMPTED), maximized_state_(MAXIMIZED_STATE_DEFAULT), - method_factory_(this) { + method_factory_(this), + block_command_execution_(false), + last_blocked_command_id_(-1), + last_blocked_command_disposition_(CURRENT_TAB) { tabstrip_model_.AddObserver(this); registrar_.Add(this, NotificationType::SSL_VISIBLE_STATE_CHANGED, @@ -1402,6 +1405,16 @@ void Browser::ExecuteCommandWithDisposition( DCHECK(command_updater_.IsCommandEnabled(id)) << "Invalid/disabled command"; + // If command execution is blocked then just record the command and return. + if (block_command_execution_) { + // We actually only allow no more than one blocked command, otherwise some + // commands maybe lost. + DCHECK(last_blocked_command_id_ == -1); + last_blocked_command_id_ = id; + last_blocked_command_disposition_ = disposition; + return; + } + // The order of commands in this switch statement must match the function // declaration order in browser.h! switch (id) { @@ -1563,6 +1576,33 @@ void Browser::ExecuteCommandWithDisposition( } } +bool Browser::IsReservedCommand(int command_id) { + return command_id == IDC_CLOSE_TAB || + command_id == IDC_CLOSE_POPUPS || + command_id == IDC_CLOSE_WINDOW || + command_id == IDC_NEW_INCOGNITO_WINDOW || + command_id == IDC_NEW_TAB || + command_id == IDC_NEW_WINDOW || + command_id == IDC_RESTORE_TAB || + command_id == IDC_SELECT_NEXT_TAB || + command_id == IDC_SELECT_PREVIOUS_TAB || + command_id == IDC_EXIT; +} + +void Browser::SetBlockCommandExecution(bool block) { + block_command_execution_ = block; + if (block) { + last_blocked_command_id_ = -1; + last_blocked_command_disposition_ = CURRENT_TAB; + } +} + +int Browser::GetLastBlockedCommand(WindowOpenDisposition* disposition) { + if (disposition) + *disposition = last_blocked_command_disposition_; + return last_blocked_command_id_; +} + /////////////////////////////////////////////////////////////////////////////// // Browser, CommandUpdater::CommandUpdaterDelegate implementation: @@ -2197,25 +2237,13 @@ void Browser::ShowPageInfo(Profile* profile, window()->ShowPageInfo(profile, url, ssl, show_history); } -bool Browser::IsReservedAccelerator(const NativeWebKeyboardEvent& event) { - // Other platforms don't send close-app keyboard shortcuts to apps first. -#if defined(OS_WIN) - if ((event.modifiers & NativeWebKeyboardEvent::AltKey) && - event.windowsKeyCode == VK_F4) { - return true; - } -#endif +bool Browser::PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event, + bool* is_keyboard_shortcut) { + return window()->PreHandleKeyboardEvent(event, is_keyboard_shortcut); +} - int command_id = window()->GetCommandId(event); - return command_id == IDC_CLOSE_TAB || - command_id == IDC_CLOSE_POPUPS || - command_id == IDC_CLOSE_WINDOW || - command_id == IDC_NEW_INCOGNITO_WINDOW || - command_id == IDC_NEW_TAB || - command_id == IDC_NEW_WINDOW || - command_id == IDC_RESTORE_TAB || - command_id == IDC_SELECT_NEXT_TAB || - command_id == IDC_SELECT_PREVIOUS_TAB; +void Browser::HandleKeyboardEvent(const NativeWebKeyboardEvent& event) { + window()->HandleKeyboardEvent(event); } void Browser::ShowRepostFormWarningDialog(TabContents *tab_contents) { diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h index 5192bf0..05fa0d8 100644 --- a/chrome/browser/browser.h +++ b/chrome/browser/browser.h @@ -468,6 +468,22 @@ class Browser : public TabStripModelDelegate, // Calls ExecuteCommandWithDisposition with the given disposition. void ExecuteCommandWithDisposition(int id, WindowOpenDisposition); + // Returns whether the |id| is a reserved command, whose keyboard shortcuts + // should not be sent to the renderer. + bool IsReservedCommand(int id); + + // Sets if command execution shall be blocked. If |block| is true then + // following calls to ExecuteCommand() or ExecuteCommandWithDisposition() + // method will not execute the command, and the last blocked command will be + // recorded for retrieval. + void SetBlockCommandExecution(bool block); + + // Gets the last blocked command after calling SetBlockCommandExecution(true). + // Returns the command id or -1 if there is no command blocked. The + // disposition type of the command will be stored in |*disposition| if it's + // not null. + int GetLastBlockedCommand(WindowOpenDisposition* disposition); + // Interface implementations //////////////////////////////////////////////// // Overridden from PageNavigator @@ -570,7 +586,9 @@ class Browser : public TabStripModelDelegate, const GURL& url, const NavigationEntry::SSLStatus& ssl, bool show_history); - virtual bool IsReservedAccelerator(const NativeWebKeyboardEvent& event); + virtual bool PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event, + bool* is_keyboard_shortcut); + virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event); virtual void ShowRepostFormWarningDialog(TabContents* tab_contents); virtual bool ShouldAddNavigationsToHistory() const; virtual void OnDidGetApplicationInfo(TabContents* tab_contents, @@ -824,6 +842,15 @@ class Browser : public TabStripModelDelegate, // Keep track of the encoding auto detect pref. BooleanPrefMember encoding_auto_detect_; + // Indicates if command execution is blocked. + bool block_command_execution_; + + // Stores the last blocked command id when |block_command_execution_| is true. + int last_blocked_command_id_; + + // Stores the disposition type of the last blocked command. + WindowOpenDisposition last_blocked_command_disposition_; + DISALLOW_COPY_AND_ASSIGN(Browser); }; diff --git a/chrome/browser/browser_keyevents_browsertest.cc b/chrome/browser/browser_keyevents_browsertest.cc index 64f9be5..897b264 100644 --- a/chrome/browser/browser_keyevents_browsertest.cc +++ b/chrome/browser/browser_keyevents_browsertest.cc @@ -536,12 +536,6 @@ IN_PROC_BROWSER_TEST_F(BrowserKeyEventsTest, AccessKeys) { } IN_PROC_BROWSER_TEST_F(BrowserKeyEventsTest, ReservedAccelerators) { - static const KeyEventTestData kTestCtrlT = { - base::VKEY_T, true, false, false, - false, false, false, false, 1, - { "D 17 0 true false false" } - }; - HTTPTestServer* server = StartHTTPServer(); GURL url = server->TestServerPageW(kTestingPage); @@ -550,21 +544,99 @@ IN_PROC_BROWSER_TEST_F(BrowserKeyEventsTest, ReservedAccelerators) { ASSERT_NO_FATAL_FAILURE(ClickOnView(VIEW_ID_TAB_CONTAINER)); ASSERT_TRUE(IsViewFocused(VIEW_ID_TAB_CONTAINER_FOCUS_VIEW)); - int tab_index = browser()->selected_index(); +#if defined(OS_WIN) + static const KeyEventTestData kTestCtrlT = { + base::VKEY_T, true, false, false, + true, false, false, false, 1, + { "D 17 0 true false false" } + }; + ASSERT_EQ(1, browser()->tab_count()); // Press Ctrl+T, which will open a new tab. - EXPECT_NO_FATAL_FAILURE(TestKeyEvent(tab_index, kTestCtrlT)); + EXPECT_NO_FATAL_FAILURE(TestKeyEvent(0, kTestCtrlT)); EXPECT_EQ(2, browser()->tab_count()); - browser()->SelectNumberedTab(tab_index); - ASSERT_EQ(tab_index, browser()->selected_index()); + browser()->SelectNumberedTab(0); + ASSERT_EQ(0, browser()->selected_index()); int result_length; - ASSERT_NO_FATAL_FAILURE(GetResultLength(tab_index, &result_length)); + ASSERT_NO_FATAL_FAILURE(GetResultLength(0, &result_length)); EXPECT_EQ(1, result_length); // Reserved accelerators can't be suppressed. - ASSERT_NO_FATAL_FAILURE(SuppressAllEvents(tab_index, true)); + ASSERT_NO_FATAL_FAILURE(SuppressAllEvents(0, true)); // Press Ctrl+W, which will close the tab. ASSERT_NO_FATAL_FAILURE(SendKey(base::VKEY_W, true, false, false)); EXPECT_EQ(1, browser()->tab_count()); + +#elif defined(OS_LINUX) && !defined(TOOLKIT_VIEWS) + // Ctrl-[a-z] are not treated as reserved accelerators on Linux. + static const KeyEventTestData kTestCtrlT = { + base::VKEY_T, true, false, false, + false, false, false, false, 2, + { "D 17 0 true false false", + "D 84 0 true false false" } + }; + + static const KeyEventTestData kTestCtrlPageDown = { + base::VKEY_NEXT, true, false, false, + true, false, false, false, 1, + { "D 17 0 true false false" } + }; + + static const KeyEventTestData kTestCtrlTab = { + base::VKEY_TAB, true, false, false, + true, false, false, false, 1, + { "D 17 0 true false false" } + }; + + static const KeyEventTestData kTestCtrlTBlocked = { + base::VKEY_T, true, false, false, + true, false, false, false, 4, + { "D 17 0 true false false", + "D 84 0 true false false", + "U 84 0 true false false", + "U 17 0 true false false" } + }; + + static const KeyEventTestData kTestCtrlWBlocked = { + base::VKEY_W, true, false, false, + true, false, false, false, 4, + { "D 17 0 true false false", + "D 87 0 true false false", + "U 87 0 true false false", + "U 17 0 true false false" } + }; + + ASSERT_EQ(1, browser()->tab_count()); + + // Ctrl+T should be blockable. + EXPECT_NO_FATAL_FAILURE(TestKeyEvent(0, kTestCtrlTBlocked)); + ASSERT_EQ(1, browser()->tab_count()); + + EXPECT_NO_FATAL_FAILURE(TestKeyEvent(0, kTestCtrlT)); + ASSERT_EQ(2, browser()->tab_count()); + ASSERT_EQ(1, browser()->selected_index()); + browser()->SelectNumberedTab(0); + ASSERT_EQ(0, browser()->selected_index()); + + // Ctrl+PageDown and Ctrl+Tab switches to the next tab. + EXPECT_NO_FATAL_FAILURE(TestKeyEvent(0, kTestCtrlPageDown)); + ASSERT_EQ(1, browser()->selected_index()); + + browser()->SelectNumberedTab(0); + ASSERT_EQ(0, browser()->selected_index()); + EXPECT_NO_FATAL_FAILURE(TestKeyEvent(0, kTestCtrlTab)); + ASSERT_EQ(1, browser()->selected_index()); + + // Ctrl+W should be blockable. + browser()->SelectNumberedTab(0); + ASSERT_EQ(0, browser()->selected_index()); + EXPECT_NO_FATAL_FAILURE(TestKeyEvent(0, kTestCtrlWBlocked)); + ASSERT_EQ(2, browser()->tab_count()); + + // Ctrl+F4 to close the tab. + ASSERT_NO_FATAL_FAILURE(SuppressAllEvents(0, true)); + ASSERT_NO_FATAL_FAILURE(SendKey(base::VKEY_F4, true, false, false)); + ASSERT_EQ(1, browser()->tab_count()); +#endif } diff --git a/chrome/browser/browser_window.h b/chrome/browser/browser_window.h index 387ea39..d6b311b 100644 --- a/chrome/browser/browser_window.h +++ b/chrome/browser/browser_window.h @@ -267,9 +267,17 @@ class BrowserWindow { // Shows the app menu (for accessibility). virtual void ShowAppMenu() = 0; - // Returns the id of the keyboard accelerator associated with the given - // keyboard event if one exists, otherwise -1. - virtual int GetCommandId(const NativeWebKeyboardEvent& event) = 0; + // Allows the BrowserWindow object to handle the specified keyboard event + // before sending it to the renderer. + // Returns true if the |event| was handled. Otherwise, if the |event| would + // be handled in HandleKeyboardEvent() method as a normal keyboard shortcut, + // |*is_keyboard_shortcut| should be set to true. + virtual bool PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event, + bool* is_keyboard_shortcut) = 0; + + // Allows the BrowserWindow object to handle the specified keyboard event, + // if the renderer did not process it. + virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event) = 0; // Shows the create web app shortcut dialog box. virtual void ShowCreateShortcutsDialog(TabContents* tab_contents) = 0; diff --git a/chrome/browser/chromeos/main_menu.h b/chrome/browser/chromeos/main_menu.h index fcf61d4..055c506 100644 --- a/chrome/browser/chromeos/main_menu.h +++ b/chrome/browser/chromeos/main_menu.h @@ -163,12 +163,11 @@ class MainMenu : public RenderViewHostDelegate, virtual void UpdateDragCursor(WebKit::WebDragOperation operation) {} virtual void GotFocus() {} virtual void TakeFocus(bool reverse) {} - virtual bool IsReservedAccelerator(const NativeWebKeyboardEvent& event) { - return false; - } - virtual bool HandleKeyboardEvent(const NativeWebKeyboardEvent& event) { + virtual bool PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event, + bool* is_keyboard_shortcut) { return false; } + virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event) {} virtual void HandleMouseEvent() {} virtual void HandleMouseLeave() {} virtual void UpdatePreferredSize(const gfx::Size& pref_size) {} diff --git a/chrome/browser/cocoa/browser_window_cocoa.h b/chrome/browser/cocoa/browser_window_cocoa.h index e7883e1..54fb2a5 100644 --- a/chrome/browser/cocoa/browser_window_cocoa.h +++ b/chrome/browser/cocoa/browser_window_cocoa.h @@ -91,7 +91,9 @@ class BrowserWindowCocoa : public BrowserWindow, bool show_history); virtual void ShowPageMenu(); virtual void ShowAppMenu(); - virtual int GetCommandId(const NativeWebKeyboardEvent& event); + virtual bool PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event, + bool* is_keyboard_shortcut); + virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event); virtual void ShowCreateShortcutsDialog(TabContents* tab_contents); // Overridden from NotificationObserver @@ -109,6 +111,9 @@ class BrowserWindowCocoa : public BrowserWindow, virtual void DestroyBrowser(); private: + int GetCommandId(const NativeWebKeyboardEvent& event); + bool HandleKeyboardEventInternal(NSEvent* event); + NotificationRegistrar registrar_; NSWindow* window_; // weak, owned by controller Browser* browser_; // weak, owned by controller diff --git a/chrome/browser/cocoa/browser_window_cocoa.mm b/chrome/browser/cocoa/browser_window_cocoa.mm index 58cd1ee..707a8a2 100644 --- a/chrome/browser/cocoa/browser_window_cocoa.mm +++ b/chrome/browser/cocoa/browser_window_cocoa.mm @@ -4,6 +4,7 @@ #include "app/l10n_util_mac.h" #include "base/gfx/rect.h" +#include "base/keyboard_codes.h" #include "base/logging.h" #include "base/sys_string_conversions.h" #include "chrome/app/chrome_dll_resource.h" @@ -13,6 +14,7 @@ #import "chrome/browser/cocoa/browser_window_controller.h" #import "chrome/browser/cocoa/bug_report_window_controller.h" #import "chrome/browser/cocoa/clear_browsing_data_controller.h" +#import "chrome/browser/cocoa/chrome_browser_window.h" #import "chrome/browser/cocoa/download_shelf_controller.h" #import "chrome/browser/cocoa/html_dialog_window_controller.h" #import "chrome/browser/cocoa/import_settings_dialog.h" @@ -368,6 +370,34 @@ void BrowserWindowCocoa::ShowAppMenu() { // No-op. Mac doesn't support showing the menus via alt keys. } +bool BrowserWindowCocoa::PreHandleKeyboardEvent( + const NativeWebKeyboardEvent& event, bool* is_keyboard_shortcut) { + if (event.skip_in_browser || event.type == NativeWebKeyboardEvent::Char) + return false; + + DCHECK(event.os_event != NULL); + int id = GetCommandId(event); + if (id == -1) + return false; + + if (browser_->IsReservedCommand(id)) + return HandleKeyboardEventInternal(event.os_event); + + DCHECK(is_keyboard_shortcut != NULL); + *is_keyboard_shortcut = true; + + return false; +} + +void BrowserWindowCocoa::HandleKeyboardEvent( + const NativeWebKeyboardEvent& event) { + if (event.skip_in_browser || event.type == NativeWebKeyboardEvent::Char) + return; + + DCHECK(event.os_event != NULL); + HandleKeyboardEventInternal(event.os_event); +} + @interface MenuWalker : NSObject + (NSMenuItem*)itemForKeyEquivalent:(NSEvent*)key menu:(NSMenu*)menu; @@ -409,10 +439,14 @@ int BrowserWindowCocoa::GetCommandId(const NativeWebKeyboardEvent& event) { // "Close window" doesn't use the |commandDispatch:| mechanism. Menu items // that do not correspond to IDC_ constants need no special treatment however, - // as they can't be blacklisted in |Browser::IsReservedAccelerator()| anyhow. + // as they can't be blacklisted in |Browser::IsReservedCommand()| anyhow. if (item && [item action] == @selector(performClose:)) return IDC_CLOSE_WINDOW; + // "Exit" doesn't use the |commandDispatch:| mechanism either. + if (item && [item action] == @selector(terminate:)) + return IDC_EXIT; + // Look in secondary keyboard shortcuts. NSUInteger modifiers = [event.os_event modifierFlags]; const bool cmdKey = (modifiers & NSCommandKeyMask) != 0; @@ -434,6 +468,31 @@ int BrowserWindowCocoa::GetCommandId(const NativeWebKeyboardEvent& event) { return -1; } +bool BrowserWindowCocoa::HandleKeyboardEventInternal(NSEvent* event) { + ChromeEventProcessingWindow* event_window = + static_cast<ChromeEventProcessingWindow*>(window_); + DCHECK([event_window isKindOfClass:[ChromeEventProcessingWindow class]]); + + // Do not fire shortcuts on key up. + if ([event type] == NSKeyDown) { + // Send the event to the menu before sending it to the browser/window + // shortcut handling, so that if a user configures cmd-left to mean + // "previous tab", it takes precedence over the built-in "history back" + // binding. Other than that, the |redispatchEvent| call would take care of + // invoking the original menu item shortcut as well. + if ([[NSApp mainMenu] performKeyEquivalent:event]) + return true; + + if ([event_window handleExtraBrowserKeyboardShortcut:event]) + return true; + + if ([event_window handleExtraWindowKeyboardShortcut:event]) + return true; + } + + return [event_window redispatchEvent:event]; +} + void BrowserWindowCocoa::ShowCreateShortcutsDialog(TabContents* tab_contents) { NOTIMPLEMENTED(); } diff --git a/chrome/browser/extensions/extension_host.cc b/chrome/browser/extensions/extension_host.cc index 77bee6c..5313852 100644 --- a/chrome/browser/extensions/extension_host.cc +++ b/chrome/browser/extensions/extension_host.cc @@ -519,20 +519,24 @@ void ExtensionHost::GotFocus() { void ExtensionHost::TakeFocus(bool reverse) { } -bool ExtensionHost::IsReservedAccelerator(const NativeWebKeyboardEvent& event) { +bool ExtensionHost::PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event, + bool* is_keyboard_shortcut) { + if (extension_host_type_ == ViewType::EXTENSION_POPUP && + event.windowsKeyCode == base::VKEY_ESCAPE) { + DCHECK(is_keyboard_shortcut != NULL); + *is_keyboard_shortcut = true; + } return false; } -bool ExtensionHost::HandleKeyboardEvent(const NativeWebKeyboardEvent& event) { +void ExtensionHost::HandleKeyboardEvent(const NativeWebKeyboardEvent& event) { if (extension_host_type_ == ViewType::EXTENSION_POPUP && event.windowsKeyCode == base::VKEY_ESCAPE) { NotificationService::current()->Notify( NotificationType::EXTENSION_HOST_VIEW_SHOULD_CLOSE, Source<Profile>(profile_), Details<ExtensionHost>(this)); - return true; } - return false; } void ExtensionHost::HandleMouseEvent() { diff --git a/chrome/browser/extensions/extension_host.h b/chrome/browser/extensions/extension_host.h index 5a27030..a743218d 100644 --- a/chrome/browser/extensions/extension_host.h +++ b/chrome/browser/extensions/extension_host.h @@ -140,8 +140,9 @@ class ExtensionHost : public ExtensionPopupHost::PopupDelegate, virtual void UpdateDragCursor(WebKit::WebDragOperation operation); virtual void GotFocus(); virtual void TakeFocus(bool reverse); - virtual bool IsReservedAccelerator(const NativeWebKeyboardEvent& event); - virtual bool HandleKeyboardEvent(const NativeWebKeyboardEvent& event); + virtual bool PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event, + bool* is_keyboard_shortcut); + virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event); virtual void HandleMouseEvent(); virtual void HandleMouseLeave(); virtual void UpdatePreferredSize(const gfx::Size& new_size); diff --git a/chrome/browser/external_tab_container.cc b/chrome/browser/external_tab_container.cc index d43df00..2a6c237 100644 --- a/chrome/browser/external_tab_container.cc +++ b/chrome/browser/external_tab_container.cc @@ -459,12 +459,15 @@ bool ExternalTabContainer::ExecuteContextMenuCommand(int command) { return true; } -bool ExternalTabContainer::HandleKeyboardEvent( +bool ExternalTabContainer::PreHandleKeyboardEvent( + const NativeWebKeyboardEvent& event, bool* is_keyboard_shortcut) { + return false; +} + +void ExternalTabContainer::HandleKeyboardEvent( const NativeWebKeyboardEvent& event) { - return ProcessUnhandledKeyStroke(event.os_event.hwnd, - event.os_event.message, - event.os_event.wParam, - event.os_event.lParam); + ProcessUnhandledKeyStroke(event.os_event.hwnd, event.os_event.message, + event.os_event.wParam, event.os_event.lParam); } void ExternalTabContainer::ShowHtmlDialog(HtmlDialogUIDelegate* delegate, @@ -716,4 +719,3 @@ void ExternalTabContainer::InitializeAutomationRequestContext( DCHECK(request_context_.get() != NULL); tab_contents_->set_request_context(request_context_.get()); } - diff --git a/chrome/browser/external_tab_container.h b/chrome/browser/external_tab_container.h index 0d47d96..760505f 100644 --- a/chrome/browser/external_tab_container.h +++ b/chrome/browser/external_tab_container.h @@ -117,7 +117,9 @@ class ExternalTabContainer : public TabContentsDelegate, }; virtual gfx::NativeWindow GetFrameNativeWindow(); - virtual bool HandleKeyboardEvent(const NativeWebKeyboardEvent& event); + virtual bool PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event, + bool* is_keyboard_shortcut); + virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event); virtual bool TakeFocus(bool reverse); diff --git a/chrome/browser/gtk/browser_window_gtk.cc b/chrome/browser/gtk/browser_window_gtk.cc index f9709cd..b8328ee 100644 --- a/chrome/browser/gtk/browser_window_gtk.cc +++ b/chrome/browser/gtk/browser_window_gtk.cc @@ -17,6 +17,7 @@ #include "base/base_paths.h" #include "base/command_line.h" #include "base/gfx/rect.h" +#include "base/keyboard_codes.h" #include "base/logging.h" #include "base/message_loop.h" #include "base/path_service.h" @@ -204,10 +205,6 @@ const struct AcceleratorMapping { { GDK_Page_Up, IDC_MOVE_TAB_PREVIOUS, GdkModifierType(GDK_CONTROL_MASK | GDK_SHIFT_MASK) }, { GDK_Page_Up, IDC_SELECT_PREVIOUS_TAB, GDK_CONTROL_MASK }, - { GDK_t, IDC_NEW_TAB, GDK_CONTROL_MASK }, - { GDK_n, IDC_NEW_WINDOW, GDK_CONTROL_MASK }, - { GDK_n, IDC_NEW_INCOGNITO_WINDOW, - GdkModifierType(GDK_CONTROL_MASK | GDK_SHIFT_MASK) }, { GDK_w, IDC_CLOSE_TAB, GDK_CONTROL_MASK }, { GDK_t, IDC_RESTORE_TAB, GdkModifierType(GDK_CONTROL_MASK | GDK_SHIFT_MASK) }, @@ -317,8 +314,6 @@ const struct AcceleratorMapping { #if defined(OS_CHROMEOS) -namespace { - // This draws the spacer below the tab strip when we're using the compact // location bar (i.e. no location bar). This basically duplicates the painting // that the tab strip would have done for this region so that it blends @@ -352,8 +347,6 @@ gboolean OnCompactNavSpacerExpose(GtkWidget* widget, return FALSE; } -} // namespace - // Callback from GTK when the user clicks the main menu button. static void OnMainMenuButtonClicked(GtkWidget* widget, BrowserWindowGtk* browser) { @@ -362,27 +355,12 @@ static void OnMainMenuButtonClicked(GtkWidget* widget, #endif // OS_CHROMEOS -int GetCommandId(guint accel_key, GdkModifierType modifier) { - // Bug 9806: If capslock is on, we will get a capital letter as accel_key. - accel_key = gdk_keyval_to_lower(accel_key); +// Get the command ids of the key combinations that are not valid gtk +// accelerators. +int GetCustomCommandId(GdkEventKey* event) { // Filter modifier to only include accelerator modifiers. - modifier = static_cast<GdkModifierType>( - modifier & gtk_accelerator_get_default_mod_mask()); - for (size_t i = 0; i < arraysize(kAcceleratorMap); ++i) { - if (kAcceleratorMap[i].keyval == accel_key && - kAcceleratorMap[i].modifier_type == modifier) - return kAcceleratorMap[i].command_id; - } - - return -1; -} - -int GetCustomCommandId(guint keyval, GdkModifierType modifier) { - // Filter modifier to only include accelerator modifiers. - modifier = static_cast<GdkModifierType>( - modifier & gtk_accelerator_get_default_mod_mask()); - - switch (keyval) { + guint modifier = event->state & gtk_accelerator_get_default_mod_mask(); + switch (event->keyval) { // Gtk doesn't allow GDK_Tab or GDK_ISO_Left_Tab to be an accelerator (see // gtk_accelerator_valid), so we need to handle these accelerators // manually. @@ -405,84 +383,32 @@ int GetCustomCommandId(guint keyval, GdkModifierType modifier) { return -1; } -// An event handler for key press events. We need to special case key -// combinations that are not valid gtk accelerators. This function returns -// TRUE if it can handle the key press. -gboolean HandleCustomAccelerator(guint keyval, GdkModifierType modifier, - Browser* browser) { - int command = GetCustomCommandId(keyval, modifier); - if (command == -1) - return FALSE; - - browser->ExecuteCommand(command); - return TRUE; -} - -// Handle accelerators that we don't want the native widget to be able to -// override. -gboolean PreHandleAccelerator(guint keyval, GdkModifierType modifier, - Browser* browser) { +// Get the command ids of the accelerators that we don't want the native widget +// to be able to override. +int GetPreHandleCommandId(GdkEventKey* event) { // Filter modifier to only include accelerator modifiers. - modifier = static_cast<GdkModifierType>( - modifier & gtk_accelerator_get_default_mod_mask()); - switch (keyval) { + guint modifier = event->state & gtk_accelerator_get_default_mod_mask(); + switch (event->keyval) { case GDK_Page_Down: if (GDK_CONTROL_MASK == modifier) { - browser->ExecuteCommand(IDC_SELECT_NEXT_TAB); - return TRUE; + return IDC_SELECT_NEXT_TAB; } else if ((GDK_CONTROL_MASK | GDK_SHIFT_MASK) == modifier) { - browser->ExecuteCommand(IDC_MOVE_TAB_NEXT); - return TRUE; + return IDC_MOVE_TAB_NEXT; } break; case GDK_Page_Up: if (GDK_CONTROL_MASK == modifier) { - browser->ExecuteCommand(IDC_SELECT_PREVIOUS_TAB); - return TRUE; + return IDC_SELECT_PREVIOUS_TAB; } else if ((GDK_CONTROL_MASK | GDK_SHIFT_MASK) == modifier) { - browser->ExecuteCommand(IDC_MOVE_TAB_PREVIOUS); - return TRUE; + return IDC_MOVE_TAB_PREVIOUS; } break; default: break; } - return FALSE; -} - -// Let the focused widget have first crack at the key event so we don't -// override their accelerators. -gboolean OnKeyPress(GtkWindow* window, GdkEventKey* event, Browser* browser) { - // If a widget besides the native view is focused, we have to try to handle - // the custom accelerators before letting it handle them. - TabContents* current_tab_contents = - browser->tabstrip_model()->GetSelectedTabContents(); - // The current tab might not have a render view if it crashed. - if (!current_tab_contents || !current_tab_contents->GetContentNativeView() || - !gtk_widget_is_focus(current_tab_contents->GetContentNativeView())) { - if (HandleCustomAccelerator(event->keyval, - GdkModifierType(event->state), browser) || - PreHandleAccelerator(event->keyval, - GdkModifierType(event->state), browser)) { - return TRUE; - } - - // Propagate the key event to child widget first, so we don't override their - // accelerators. - if (!gtk_window_propagate_key_event(window, event)) { - if (!gtk_window_activate_key(window, event)) { - gtk_bindings_activate_event(GTK_OBJECT(window), event); - } - } - } else { - bool rv = gtk_window_propagate_key_event(window, event); - DCHECK(rv); - } - - // Prevents the default handler from handling this event. - return TRUE; + return -1; } GdkCursorType GdkWindowEdgeToGdkCursorType(GdkWindowEdge edge) { @@ -528,6 +454,31 @@ void SetWindowSize(GtkWindow* window, int width, int height) { } } +GQuark GetBrowserWindowQuarkKey() { + static GQuark quark = g_quark_from_static_string(kBrowserWindowKey); + return quark; +} + +// Checks if a reserved accelerator key should be processed immediately, rather +// than being sent to the renderer first. +bool ShouldExecuteReservedCommandImmediately( + const NativeWebKeyboardEvent& event, int command_id) { + // IDC_EXIT is now only bound to Ctrl+Shift+q, so we should always execute it + // immediately. + if (command_id == IDC_EXIT) + return true; + + // Keys like Ctrl+w, Ctrl+n, etc. should always be sent to the renderer first, + // otherwise some web apps or the Emacs key bindings may not work correctly. + int vkey = event.windowsKeyCode; + if ((vkey >= base::VKEY_0 && vkey <= base::VKEY_9) || + (vkey >= base::VKEY_A && vkey <= base::VKEY_Z)) + return false; + + // All other reserved accelerators should be processed immediately. + return true; +} + } // namespace std::map<XID, GtkWindow*> BrowserWindowGtk::xid_map_; @@ -557,7 +508,7 @@ BrowserWindowGtk::BrowserWindowGtk(Browser* browser) browser_->profile()->GetPrefs(), this); window_ = GTK_WINDOW(gtk_window_new(GTK_WINDOW_TOPLEVEL)); - g_object_set_data(G_OBJECT(window_), kBrowserWindowKey, this); + g_object_set_qdata(G_OBJECT(window_), GetBrowserWindowQuarkKey(), this); gtk_widget_add_events(GTK_WIDGET(window_), GDK_BUTTON_PRESS_MASK | GDK_POINTER_MOTION_MASK); @@ -601,21 +552,6 @@ BrowserWindowGtk::~BrowserWindowGtk() { } } -bool BrowserWindowGtk::HandleKeyboardEvent(GdkEventKey* event) { - // Handles a key event in following sequence: - // 1. Our special key accelerators, such as ctrl-tab, etc. - // 2. Gtk mnemonics and accelerators. - // This sequence matches the default key press handler of GtkWindow. - // - // It's not necessary to care about the keyboard layout, as - // gtk_window_activate_key() takes care of it automatically. - if (!HandleCustomAccelerator(event->keyval, GdkModifierType(event->state), - browser_.get())) { - return gtk_window_activate_key(window_, event); - } - return true; -} - gboolean BrowserWindowGtk::OnCustomFrameExpose(GtkWidget* widget, GdkEventExpose* event, BrowserWindowGtk* window) { @@ -1209,16 +1145,90 @@ void BrowserWindowGtk::ShowAppMenu() { // BrowserToolbarGtk. } -int BrowserWindowGtk::GetCommandId(const NativeWebKeyboardEvent& event) { - if (!event.os_event) - return -1; +bool BrowserWindowGtk::PreHandleKeyboardEvent( + const NativeWebKeyboardEvent& event, bool* is_keyboard_shortcut) { + GdkEventKey* os_event = event.os_event; + + if (!os_event || event.type != WebKit::WebInputEvent::RawKeyDown) + return false; + + // We first find out the browser command associated to the |event|. + // Then if the command is a reserved one, and should be processed immediately + // according to the |event|, the command will be executed immediately. + // Otherwise we just set |*is_keyboard_shortcut| properly and return false. + + // First check if it's a custom accelerator. + int id = GetCustomCommandId(os_event); + + // Then check if it's a predefined accelerator bound to the window. + if (id == -1) { + // This piece of code is based on the fact that calling + // gtk_window_activate_key() method against |window_| may only trigger a + // browser command execution, by matching either a global accelerator + // defined in above |kAcceleratorMap| or the accelerator key of a menu + // item defined in chrome/browser/gtk/standard_menus.cc. + // + // Here we need to retrieve the command id (if any) associated to the + // keyboard event. Instead of looking up the command id in above + // |kAcceleratorMap| table by ourselves, we block the command execution of + // the |browser_| object then send the keyboard event to the |window_| by + // calling gtk_window_activate_key() method, as if we are activating an + // accelerator key. Then we can retrieve the command id from the + // |browser_| object. + // + // Pros of this approach: + // 1. We can handle accelerators defined not only in above + // |kAcceleratorMap| table, but also those in standard_menus.cc. + // 2. We don't need to care about keyboard layout problem, as + // gtk_window_activate_key() method handles it for us. + // + // Cons: + // 1. The logic is a little complicated. + // 2. We should be careful not to introduce any accelerators that trigger + // customized code instead of browser commands. + browser_->SetBlockCommandExecution(true); + gtk_window_activate_key(window_, os_event); + // We don't need to care about the WindowOpenDisposition value, + // because all commands executed in this path use the default value. + id = browser_->GetLastBlockedCommand(NULL); + browser_->SetBlockCommandExecution(false); + } + + if (id == -1) + return false; + + if (browser_->IsReservedCommand(id) && + ShouldExecuteReservedCommandImmediately(event, id)) { + // Executing the command may cause |this| object to be destroyed. + return ExecuteBrowserCommand(id); + } + + // The |event| is a keyboard shortcut. + DCHECK(is_keyboard_shortcut != NULL); + *is_keyboard_shortcut = true; + + return false; +} + +void BrowserWindowGtk::HandleKeyboardEvent( + const NativeWebKeyboardEvent& event) { + GdkEventKey* os_event = event.os_event; + + if (!os_event || event.type != WebKit::WebInputEvent::RawKeyDown) + return; - guint keyval = event.os_event->keyval; - GdkModifierType modifier = GdkModifierType(event.os_event->state); - int command = ::GetCommandId(keyval, modifier); - if (command == -1) - command = GetCustomCommandId(keyval, modifier); - return command; + // Handles a key event in following sequence: + // 1. Our special key accelerators, such as ctrl-tab, etc. + // 2. Gtk accelerators. + // This sequence matches the default key press handler of GtkWindow. + // + // It's not necessary to care about the keyboard layout, as + // gtk_window_activate_key() takes care of it automatically. + int id = GetCustomCommandId(os_event); + if (id != -1) + ExecuteBrowserCommand(id); + else + gtk_window_activate_key(window_, os_event); } void BrowserWindowGtk::ShowCreateShortcutsDialog(TabContents* tab_contents) { @@ -1544,7 +1554,7 @@ BrowserWindowGtk* BrowserWindowGtk::GetBrowserWindowForNativeWindow( gfx::NativeWindow window) { if (window) { return static_cast<BrowserWindowGtk*>( - g_object_get_data(G_OBJECT(window), kBrowserWindowKey)); + g_object_get_qdata(G_OBJECT(window), GetBrowserWindowQuarkKey())); } return NULL; @@ -1628,7 +1638,7 @@ void BrowserWindowGtk::ConnectHandlersToSignals() { g_signal_connect(window_, "unmap", G_CALLBACK(MainWindowUnMapped), this); g_signal_connect(window_, "key-press-event", - G_CALLBACK(OnKeyPress), browser_.get()); + G_CALLBACK(OnKeyPress), this); g_signal_connect(window_, "motion-notify-event", G_CALLBACK(OnMouseMoveEvent), this); g_signal_connect(window_, "button-press-event", @@ -1946,7 +1956,8 @@ void BrowserWindowGtk::ConnectAccelerators() { kAcceleratorMap[i].keyval, kAcceleratorMap[i].modifier_type, GtkAccelFlags(0), - g_cclosure_new(G_CALLBACK(OnGtkAccelerator), this, NULL)); + g_cclosure_new(G_CALLBACK(OnGtkAccelerator), + GINT_TO_POINTER(kAcceleratorMap[i].command_id), NULL)); } } @@ -1995,53 +2006,88 @@ gboolean BrowserWindowGtk::OnGtkAccelerator(GtkAccelGroup* accel_group, GObject* acceleratable, guint keyval, GdkModifierType modifier, - BrowserWindowGtk* browser_window) { - int command_id = ::GetCommandId(keyval, modifier); - DCHECK_NE(command_id, -1); - browser_window->ExecuteBrowserCommand(command_id); + void* user_data) { + int command_id = GPOINTER_TO_INT(user_data); + BrowserWindowGtk* browser_window = + GetBrowserWindowForNativeWindow(GTK_WINDOW(acceleratable)); + DCHECK(browser_window != NULL); + return browser_window->ExecuteBrowserCommand(command_id); +} + +// static +// Let the focused widget have first crack at the key event so we don't +// override their accelerators. +gboolean BrowserWindowGtk::OnKeyPress( + GtkWidget* widget, GdkEventKey* event, BrowserWindowGtk* window) { + // If a widget besides the native view is focused, we have to try to handle + // the custom accelerators before letting it handle them. + TabContents* current_tab_contents = + window->browser()->tabstrip_model()->GetSelectedTabContents(); + // The current tab might not have a render view if it crashed. + if (!current_tab_contents || !current_tab_contents->GetContentNativeView() || + !gtk_widget_is_focus(current_tab_contents->GetContentNativeView())) { + int command_id = GetCustomCommandId(event); + if (command_id == -1) + command_id = GetPreHandleCommandId(event); + + if (command_id != -1 && window->ExecuteBrowserCommand(command_id)) + return TRUE; + + // Propagate the key event to child widget first, so we don't override their + // accelerators. + if (!gtk_window_propagate_key_event(GTK_WINDOW(widget), event)) { + if (!gtk_window_activate_key(GTK_WINDOW(widget), event)) { + gtk_bindings_activate_event(GTK_OBJECT(widget), event); + } + } + } else { + bool rv = gtk_window_propagate_key_event(GTK_WINDOW(widget), event); + DCHECK(rv); + } + // Prevents the default handler from handling this event. return TRUE; } // static gboolean BrowserWindowGtk::OnMouseMoveEvent(GtkWidget* widget, - GdkEventMotion* event, BrowserWindowGtk* browser) { + GdkEventMotion* event, BrowserWindowGtk* window) { // This method is used to update the mouse cursor when over the edge of the // custom frame. If the custom frame is off or we're over some other widget, // do nothing. - if (!browser->UseCustomFrame() || event->window != widget->window) { + if (!window->UseCustomFrame() || event->window != widget->window) { // Reset the cursor. - if (browser->frame_cursor_) { - gdk_cursor_unref(browser->frame_cursor_); - browser->frame_cursor_ = NULL; - gdk_window_set_cursor(GTK_WIDGET(browser->window_)->window, NULL); + if (window->frame_cursor_) { + gdk_cursor_unref(window->frame_cursor_); + window->frame_cursor_ = NULL; + gdk_window_set_cursor(GTK_WIDGET(window->window_)->window, NULL); } return FALSE; } // Update the cursor if we're on the custom frame border. GdkWindowEdge edge; - bool has_hit_edge = browser->GetWindowEdge(static_cast<int>(event->x), + bool has_hit_edge = window->GetWindowEdge(static_cast<int>(event->x), static_cast<int>(event->y), &edge); GdkCursorType new_cursor = GDK_LAST_CURSOR; if (has_hit_edge) new_cursor = GdkWindowEdgeToGdkCursorType(edge); GdkCursorType last_cursor = GDK_LAST_CURSOR; - if (browser->frame_cursor_) - last_cursor = browser->frame_cursor_->type; + if (window->frame_cursor_) + last_cursor = window->frame_cursor_->type; if (last_cursor != new_cursor) { - if (browser->frame_cursor_) { - gdk_cursor_unref(browser->frame_cursor_); - browser->frame_cursor_ = NULL; + if (window->frame_cursor_) { + gdk_cursor_unref(window->frame_cursor_); + window->frame_cursor_ = NULL; } if (has_hit_edge) { - browser->frame_cursor_ = gtk_util::GetCursor(new_cursor); - gdk_window_set_cursor(GTK_WIDGET(browser->window_)->window, - browser->frame_cursor_); + window->frame_cursor_ = gtk_util::GetCursor(new_cursor); + gdk_window_set_cursor(GTK_WIDGET(window->window_)->window, + window->frame_cursor_); } else { - gdk_window_set_cursor(GTK_WIDGET(browser->window_)->window, NULL); + gdk_window_set_cursor(GTK_WIDGET(window->window_)->window, NULL); } } return FALSE; @@ -2049,15 +2095,15 @@ gboolean BrowserWindowGtk::OnMouseMoveEvent(GtkWidget* widget, // static gboolean BrowserWindowGtk::OnButtonPressEvent(GtkWidget* widget, - GdkEventButton* event, BrowserWindowGtk* browser) { + GdkEventButton* event, BrowserWindowGtk* window) { // Handle back/forward. // TODO(jhawkins): Investigate the possibility of the button numbers being // different for other mice. if (event->button == 8) { - browser->browser_->GoBack(CURRENT_TAB); + window->browser_->GoBack(CURRENT_TAB); return TRUE; } else if (event->button == 9) { - browser->browser_->GoForward(CURRENT_TAB); + window->browser_->GoForward(CURRENT_TAB); return TRUE; } @@ -2066,36 +2112,36 @@ gboolean BrowserWindowGtk::OnButtonPressEvent(GtkWidget* widget, // Make the button press coordinate relative to the browser window. int win_x, win_y; - gdk_window_get_origin(GTK_WIDGET(browser->window_)->window, &win_x, &win_y); + gdk_window_get_origin(GTK_WIDGET(window->window_)->window, &win_x, &win_y); GdkWindowEdge edge; gfx::Point point(static_cast<int>(event->x_root - win_x), static_cast<int>(event->y_root - win_y)); - bool has_hit_edge = browser->GetWindowEdge(point.x(), point.y(), &edge); + bool has_hit_edge = window->GetWindowEdge(point.x(), point.y(), &edge); // Ignore clicks that are in/below the browser toolbar. - GtkWidget* toolbar = browser->toolbar_->widget(); + GtkWidget* toolbar = window->toolbar_->widget(); if (!GTK_WIDGET_VISIBLE(toolbar)) { // If the toolbar is not showing, use the location of web contents as the // boundary of where to ignore clicks. - toolbar = browser->render_area_vbox_; + toolbar = window->render_area_vbox_; } gint toolbar_y; gtk_widget_get_pointer(toolbar, NULL, &toolbar_y); - bool has_hit_titlebar = !browser->IsFullscreen() && (toolbar_y < 0) + bool has_hit_titlebar = !window->IsFullscreen() && (toolbar_y < 0) && !has_hit_edge; if (event->button == 1) { if (GDK_BUTTON_PRESS == event->type) { - guint32 last_click_time = browser->last_click_time_; - gfx::Point last_click_position = browser->last_click_position_; - browser->last_click_time_ = event->time; - browser->last_click_position_ = gfx::Point(static_cast<int>(event->x), + guint32 last_click_time = window->last_click_time_; + gfx::Point last_click_position = window->last_click_position_; + window->last_click_time_ = event->time; + window->last_click_position_ = gfx::Point(static_cast<int>(event->x), static_cast<int>(event->y)); // Raise the window after a click on either the titlebar or the border to // match the behavior of most window managers. if (has_hit_titlebar || has_hit_edge) - gdk_window_raise(GTK_WIDGET(browser->window_)->window); + gdk_window_raise(GTK_WIDGET(window->window_)->window); if (has_hit_titlebar) { // We want to start a move when the user single clicks, but not start a @@ -2121,14 +2167,14 @@ gboolean BrowserWindowGtk::OnButtonPressEvent(GtkWidget* widget, if (click_time > static_cast<guint32>(double_click_time) || click_move_x > double_click_distance || click_move_y > double_click_distance) { - gtk_window_begin_move_drag(browser->window_, event->button, + gtk_window_begin_move_drag(window->window_, event->button, static_cast<gint>(event->x_root), static_cast<gint>(event->y_root), event->time); return TRUE; } } else if (has_hit_edge) { - gtk_window_begin_resize_drag(browser->window_, edge, event->button, + gtk_window_begin_resize_drag(window->window_, edge, event->button, static_cast<gint>(event->x_root), static_cast<gint>(event->y_root), event->time); @@ -2137,22 +2183,22 @@ gboolean BrowserWindowGtk::OnButtonPressEvent(GtkWidget* widget, } else if (GDK_2BUTTON_PRESS == event->type) { if (has_hit_titlebar) { // Maximize/restore on double click. - if (browser->IsMaximized()) { - browser->UnMaximize(); + if (window->IsMaximized()) { + window->UnMaximize(); } else { - gtk_window_maximize(browser->window_); + gtk_window_maximize(window->window_); } return TRUE; } } } else if (event->button == 2) { if (has_hit_titlebar || has_hit_edge) { - gdk_window_lower(GTK_WIDGET(browser->window_)->window); + gdk_window_lower(GTK_WIDGET(window->window_)->window); } return TRUE; } else if (event->button == 3) { if (has_hit_titlebar) { - browser->titlebar_->ShowContextMenu(); + window->titlebar_->ShowContextMenu(); return TRUE; } } @@ -2180,11 +2226,11 @@ void BrowserWindowGtk::MainWindowUnMapped(GtkWidget* widget, // static gboolean BrowserWindowGtk::OnFocusIn(GtkWidget* widget, GdkEventFocus* event, - BrowserWindowGtk* browser) { - BrowserList::SetLastActive(browser->browser_.get()); + BrowserWindowGtk* window) { + BrowserList::SetLastActive(window->browser_.get()); #if defined(OS_CHROMEOS) - if (browser->panel_controller_) { - browser->panel_controller_->OnFocusIn(); + if (window->panel_controller_) { + window->panel_controller_->OnFocusIn(); } #endif return FALSE; @@ -2193,18 +2239,21 @@ gboolean BrowserWindowGtk::OnFocusIn(GtkWidget* widget, // static gboolean BrowserWindowGtk::OnFocusOut(GtkWidget* widget, GdkEventFocus* event, - BrowserWindowGtk* browser) { + BrowserWindowGtk* window) { #if defined(OS_CHROMEOS) - if (browser->panel_controller_) { - browser->panel_controller_->OnFocusOut(); + if (window->panel_controller_) { + window->panel_controller_->OnFocusOut(); } #endif return FALSE; } -void BrowserWindowGtk::ExecuteBrowserCommand(int id) { - if (browser_->command_updater()->IsCommandEnabled(id)) +bool BrowserWindowGtk::ExecuteBrowserCommand(int id) { + if (browser_->command_updater()->IsCommandEnabled(id)) { browser_->ExecuteCommand(id); + return true; + } + return false; } void BrowserWindowGtk::ShowSupportedWindowFeatures() { diff --git a/chrome/browser/gtk/browser_window_gtk.h b/chrome/browser/gtk/browser_window_gtk.h index 7d55e6c..b379408 100644 --- a/chrome/browser/gtk/browser_window_gtk.h +++ b/chrome/browser/gtk/browser_window_gtk.h @@ -54,9 +54,6 @@ class BrowserWindowGtk : public BrowserWindow, explicit BrowserWindowGtk(Browser* browser); virtual ~BrowserWindowGtk(); - // Process a keyboard event which was not handled by webkit. - bool HandleKeyboardEvent(GdkEventKey* event); - // Overridden from BrowserWindow virtual void Show(); virtual void SetBounds(const gfx::Rect& bounds); @@ -121,7 +118,9 @@ class BrowserWindowGtk : public BrowserWindow, bool show_history); virtual void ShowPageMenu(); virtual void ShowAppMenu(); - virtual int GetCommandId(const NativeWebKeyboardEvent& event); + virtual bool PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event, + bool* is_keyboard_shortcut); + virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event); virtual void ShowCreateShortcutsDialog(TabContents* tab_contents); // Overridden from NotificationObserver: @@ -279,19 +278,26 @@ class BrowserWindowGtk : public BrowserWindow, // border during an expose. static void DrawContentShadow(cairo_t* cr, BrowserWindowGtk* window); + // Callback for accelerator activation. |user_data| stores the command id + // of the matched accelerator. static gboolean OnGtkAccelerator(GtkAccelGroup* accel_group, GObject* acceleratable, guint keyval, GdkModifierType modifier, - BrowserWindowGtk* browser_window); + void* user_data); + + // Key press event callback. + static gboolean OnKeyPress(GtkWidget* widget, + GdkEventKey* event, + BrowserWindowGtk* window); // Mouse move and mouse button press callbacks. static gboolean OnMouseMoveEvent(GtkWidget* widget, GdkEventMotion* event, - BrowserWindowGtk* browser); + BrowserWindowGtk* window); static gboolean OnButtonPressEvent(GtkWidget* widget, GdkEventButton* event, - BrowserWindowGtk* browser); + BrowserWindowGtk* window); // Maps and Unmaps the xid of |widget| to |window|. static void MainWindowMapped(GtkWidget* widget, BrowserWindowGtk* window); @@ -300,13 +306,14 @@ class BrowserWindowGtk : public BrowserWindow, // Tracks focus state of browser. static gboolean OnFocusIn(GtkWidget* widget, GdkEventFocus* event, - BrowserWindowGtk* browser); + BrowserWindowGtk* window); static gboolean OnFocusOut(GtkWidget* widget, GdkEventFocus* event, - BrowserWindowGtk* browser); + BrowserWindowGtk* window); // A small shim for browser_->ExecuteCommand. - void ExecuteBrowserCommand(int id); + // Returns true if the command was executed. + bool ExecuteBrowserCommand(int id); // Callback for the loading animation(s) associated with this window. void LoadingAnimationCallback(); diff --git a/chrome/browser/renderer_host/render_view_host.cc b/chrome/browser/renderer_host/render_view_host.cc index a473519..e214972 100644 --- a/chrome/browser/renderer_host/render_view_host.cc +++ b/chrome/browser/renderer_host/render_view_host.cc @@ -1477,20 +1477,17 @@ void RenderViewHost::OnUserMetricsRecordAction(const std::string& action) { UserMetrics::RecordComputedAction(action.c_str(), process()->profile()); } -bool RenderViewHost::ShouldSendToRenderer(const NativeWebKeyboardEvent& event) { +bool RenderViewHost::PreHandleKeyboardEvent( + const NativeWebKeyboardEvent& event, bool* is_keyboard_shortcut) { RenderViewHostDelegate::View* view = delegate_->GetViewDelegate(); - if (!view) - return true; - return !view->IsReservedAccelerator(event); + return view && view->PreHandleKeyboardEvent(event, is_keyboard_shortcut); } -bool RenderViewHost::UnhandledKeyboardEvent( +void RenderViewHost::UnhandledKeyboardEvent( const NativeWebKeyboardEvent& event) { RenderViewHostDelegate::View* view = delegate_->GetViewDelegate(); - if (view) { - return view->HandleKeyboardEvent(event); - } - return false; + if (view) + view->HandleKeyboardEvent(event); } void RenderViewHost::OnUserGesture() { diff --git a/chrome/browser/renderer_host/render_view_host.h b/chrome/browser/renderer_host/render_view_host.h index 99ee05f..0d923ca 100644 --- a/chrome/browser/renderer_host/render_view_host.h +++ b/chrome/browser/renderer_host/render_view_host.h @@ -443,8 +443,9 @@ class RenderViewHost : public RenderWidgetHost, protected: // RenderWidgetHost protected overrides. - virtual bool ShouldSendToRenderer(const NativeWebKeyboardEvent& event); - virtual bool UnhandledKeyboardEvent(const NativeWebKeyboardEvent& event); + virtual bool PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event, + bool* is_keyboard_shortcut); + virtual void UnhandledKeyboardEvent(const NativeWebKeyboardEvent& event); virtual void OnUserGesture(); virtual void NotifyRendererUnresponsive(); virtual void NotifyRendererResponsive(); diff --git a/chrome/browser/renderer_host/render_view_host_delegate.h b/chrome/browser/renderer_host/render_view_host_delegate.h index a3d75c6..972a75b 100644 --- a/chrome/browser/renderer_host/render_view_host_delegate.h +++ b/chrome/browser/renderer_host/render_view_host_delegate.h @@ -126,15 +126,18 @@ class RenderViewHostDelegate { // true, it means the focus was retrieved by doing a Shift-Tab. virtual void TakeFocus(bool reverse) = 0; - // Returns whether the event is a reserved keyboard shortcut that should not - // be sent to the renderer. - virtual bool IsReservedAccelerator(const NativeWebKeyboardEvent& event) = 0; + // Callback to give the browser a chance to handle the specified keyboard + // event before sending it to the renderer. + // Returns true if the |event| was handled. Otherwise, if the |event| would + // be handled in HandleKeyboardEvent() method as a normal keyboard shortcut, + // |*is_keyboard_shortcut| should be set to true. + virtual bool PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event, + bool* is_keyboard_shortcut) = 0; // Callback to inform the browser that the renderer did not process the // specified events. This gives an opportunity to the browser to process the // event (used for keyboard shortcuts). - // Returns true if the event was handled. - virtual bool HandleKeyboardEvent(const NativeWebKeyboardEvent& event) = 0; + virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event) = 0; // Notifications about mouse events in this view. This is useful for // implementing global 'on hover' features external to the view. diff --git a/chrome/browser/renderer_host/render_widget_host.cc b/chrome/browser/renderer_host/render_widget_host.cc index 1ba09234..c40bbf8 100644 --- a/chrome/browser/renderer_host/render_widget_host.cc +++ b/chrome/browser/renderer_host/render_widget_host.cc @@ -51,9 +51,6 @@ static const int kPaintMsgTimeoutMS = 40; // How long to wait before we consider a renderer hung. static const int kHungRendererDelayMs = 20000; -// How long a PaintRect_ACK message can be postponed at most, in milliseconds. -static const int kPaintACKMsgMaxPostponedDurationMS = 1000; - /////////////////////////////////////////////////////////////////////////////// // RenderWidgetHost @@ -76,10 +73,7 @@ RenderWidgetHost::RenderWidgetHost(RenderProcessHost* process, text_direction_updated_(false), text_direction_(WebKit::WebTextDirectionLeftToRight), text_direction_canceled_(false), - pending_key_events_(0), - suppress_next_char_events_(false), - paint_ack_postponed_(false), - death_flag_(NULL) { + suppress_next_char_events_(false) { if (routing_id_ == MSG_ROUTING_NONE) routing_id_ = process_->GetNextRoutingID(); @@ -90,10 +84,6 @@ RenderWidgetHost::RenderWidgetHost(RenderProcessHost* process, } RenderWidgetHost::~RenderWidgetHost() { - // Force the method that destroys this object to exit immediately. - if (death_flag_) - *death_flag_ = true; - // Clear our current or cached backing store if either remains. BackingStoreManager::RemoveBackingStore(this); @@ -375,7 +365,7 @@ void RenderWidgetHost::ForwardMouseEvent(const WebMouseEvent& mouse_event) { OnUserGesture(); } - ForwardInputEvent(mouse_event, sizeof(WebMouseEvent)); + ForwardInputEvent(mouse_event, sizeof(WebMouseEvent), false); } void RenderWidgetHost::ForwardWheelEvent( @@ -383,7 +373,7 @@ void RenderWidgetHost::ForwardWheelEvent( if (process_->ignore_input_events()) return; - ForwardInputEvent(wheel_event, sizeof(WebMouseWheelEvent)); + ForwardInputEvent(wheel_event, sizeof(WebMouseWheelEvent), false); } void RenderWidgetHost::ForwardKeyboardEvent( @@ -400,104 +390,52 @@ void RenderWidgetHost::ForwardKeyboardEvent( // Double check the type to make sure caller hasn't sent us nonsense that // will mess up our key queue. if (WebInputEvent::isKeyboardEventType(key_event.type)) { - // Don't add this key to the queue if we have no way to send the message... - if (!process_->HasConnection()) - return; - - // To help understand following logic, please refer to the comments - // explaining |pending_key_events_| and |suppress_next_char_events_| - // members. And the comments in OnMsgInputEventAck() method, which contains - // the bottom half of the logic. - // - // There are to different situations for us to handle key events: - // 1. After sending a key event to the renderer, we receive its ACK message - // from the renderer before sending the next key event. - // In this case, there is always no more than 1 key event in |key_queue_|, - // and we send the key event one by one in this method, except that a - // sequence of Char events may be suppressed directly if the preceding - // RawKeyDown event was handled as an accelerator key in the browser. - // - // 2. We get the next key event before receving the previous one's ACK - // message from the renderer. - // In this case, when we get a key event, the previous key event is still in - // |keq_queue_| waiting for being handled in OnMsgInputEventAck() method. - // Then we need handle following cases differently: - // 1) If we get a Char event, then the previous key event in |key_queue_| - // waiting for ACK must be a RawKeyDown event. Then we need keep this Char - // event in |key_queue_| rather than sending it immediately, because we - // can't determine if we should send it to the renderer or just suppress - // it, until we receive the preceding RawKeyDown event's ACK message. - // We increase the count of |pending_key_events_| to help remember this - // Char event is not sent to the renderer yet. - // 2) If there is already one or more key event pending in |key_queue_| - // (|pending_key_events_| > 0), we can not send a new key event - // immediately no matter what type it is. Otherwise the key events will be - // in wrong order. In this case we just keep them in |key_queue_| and - // increase |pending_key_events_| to remember them. - // - // Then all pending key events in |key_queue_| will be handled properly in - // OnMsgInputEventAck() method. Please refer to that method for details. - - // Tab switching/closing accelerators aren't sent to the renderer to avoid a - // hung/malicious renderer from interfering. - if (!ShouldSendToRenderer(key_event)) { - if (key_event.type == WebKeyboardEvent::RawKeyDown) - suppress_next_char_events_ = true; - UnhandledKeyboardEvent(key_event); - // We might be deleted now. - return; - } - - bool is_char = (key_event.type == WebKeyboardEvent::Char); - - // Handle the first situation mentioned above. if (suppress_next_char_events_) { // If preceding RawKeyDown event was handled by the browser, then we need // suppress all Char events generated by it. Please note that, one // RawKeyDown event may generate multiple Char events, so we can't reset // |suppress_next_char_events_| until we get a KeyUp or a RawKeyDown. - if (is_char) + if (key_event.type == WebKeyboardEvent::Char) return; // We get a KeyUp or a RawKeyDown event. suppress_next_char_events_ = false; } + // We need to set |suppress_next_char_events_| to true if + // PreHandleKeyboardEvent() returns true, but |this| may already be + // destroyed at that time. So set |suppress_next_char_events_| true here, + // then revert it afterwards when necessary. + if (key_event.type == WebKeyboardEvent::RawKeyDown) + suppress_next_char_events_ = true; + + bool is_keyboard_shortcut = false; + // Tab switching/closing accelerators aren't sent to the renderer to avoid a + // hung/malicious renderer from interfering. + if (PreHandleKeyboardEvent(key_event, &is_keyboard_shortcut)) + return; + + if (key_event.type == WebKeyboardEvent::RawKeyDown) + suppress_next_char_events_ = false; + + // Don't add this key to the queue if we have no way to send the message... + if (!process_->HasConnection()) + return; + // Put all WebKeyboardEvent objects in a queue since we can't trust the // renderer and we need to give something to the UnhandledInputEvent // handler. key_queue_.push_back(key_event); HISTOGRAM_COUNTS_100("Renderer.KeyboardQueueSize", key_queue_.size()); - // Handle the second situation mentioned above. - size_t key_queue_size = key_queue_.size(); - if ((is_char && key_queue_size > 1) || pending_key_events_) { - // The event is not send to the renderer, increase |pending_key_events_| - // to remember it. - ++pending_key_events_; - - // Some sanity checks. - // At least one key event in the front of |key_queue_| has been sent to - // the renderer, otherwise we won't be able to get any ACK back from the - // renderer. - DCHECK(key_queue_size > pending_key_events_); - // Char events must be preceded by RawKeyDown events. - // TODO(suzhe): verify it on all platforms. - DCHECK(key_queue_[key_queue_size - pending_key_events_ - 1].type == - WebKeyboardEvent::RawKeyDown); - // The first pending key event must be a Char event. Other key events must - // already be sent to the renderer at this point. - DCHECK(key_queue_[key_queue_size - pending_key_events_].type == - WebKeyboardEvent::Char); - } else { - // Fall into the first situation, so we can send the event immediately. - // Only forward the non-native portions of our event. - ForwardInputEvent(key_event, sizeof(WebKeyboardEvent)); - } + // Only forward the non-native portions of our event. + ForwardInputEvent(key_event, sizeof(WebKeyboardEvent), + is_keyboard_shortcut); } } void RenderWidgetHost::ForwardInputEvent(const WebInputEvent& input_event, - int event_size) { + int event_size, + bool is_keyboard_shortcut) { if (!process_->HasConnection()) return; @@ -506,6 +444,9 @@ void RenderWidgetHost::ForwardInputEvent(const WebInputEvent& input_event, IPC::Message* message = new ViewMsg_HandleInputEvent(routing_id_); message->WriteData( reinterpret_cast<const char*>(&input_event), event_size); + // |is_keyboard_shortcut| only makes sense for RawKeyDown events. + if (input_event.type == WebInputEvent::RawKeyDown) + message->WriteBool(is_keyboard_shortcut); input_event_start_time_ = TimeTicks::Now(); Send(message); @@ -539,13 +480,11 @@ void RenderWidgetHost::RendererExited() { // Must reset these to ensure that keyboard events work with a new renderer. key_queue_.clear(); - pending_key_events_ = 0; suppress_next_char_events_ = false; // Reset some fields in preparation for recovering from a crash. resize_ack_pending_ = false; repaint_ack_pending_ = false; - paint_ack_postponed_ = false; in_flight_size_.SetSize(0, 0); current_size_.SetSize(0, 0); @@ -681,10 +620,6 @@ void RenderWidgetHost::OnMsgRequestMove(const gfx::Rect& pos) { void RenderWidgetHost::OnMsgPaintRect( const ViewHostMsg_PaintRect_Params& params) { - // We shouldn't receive PaintRect message when the last PaintRect_ACK has not - // been sent to the renderer yet. - DCHECK(!paint_ack_postponed_); - TimeTicks paint_start = TimeTicks::Now(); // Update our knowledge of the RenderWidget's size. @@ -732,16 +667,7 @@ void RenderWidgetHost::OnMsgPaintRect( // This must be done AFTER we're done painting with the bitmap supplied by the // renderer. This ACK is a signal to the renderer that the backing store can // be re-used, so the bitmap may be invalid after this call. - // - // Postpone the ACK message until all pending key events have been sent to the - // renderer, so that the renderer can process the updates caused by the key - // events in batch. - if (pending_key_events_ > 0) { - paint_ack_postponed_ = true; - paint_ack_postponed_time_ = TimeTicks::Now(); - } else { - Send(new ViewMsg_PaintRect_ACK(routing_id_)); - } + Send(new ViewMsg_PaintRect_ACK(routing_id_)); // We don't need to update the view if the view is hidden. We must do this // early return after the ACK is sent, however, or the renderer will not send @@ -1009,92 +935,20 @@ void RenderWidgetHost::ProcessKeyboardEventAck(int type, bool processed) { << "the renderer. (" << key_queue_.front().type << " vs. " << type << "). Ignoring event."; - // Something must be wrong. |key_queue_| must be cleared here to make sure - // the feedback loop for sending upcoming keyboard events can be resumed - // correctly. + // Something must be wrong. Clear the |key_queue_| and + // |suppress_next_char_events_| so that we can resume from the error. key_queue_.clear(); - pending_key_events_ = 0; suppress_next_char_events_ = false; } else { - // Track if |this| is destroyed or not. - bool is_dead = false; - death_flag_ = &is_dead; - NativeWebKeyboardEvent front_item = key_queue_.front(); key_queue_.pop_front(); - bool processed_by_browser = false; if (!processed) { - processed_by_browser = UnhandledKeyboardEvent(front_item); + UnhandledKeyboardEvent(front_item); // WARNING: This RenderWidgetHost can be deallocated at this point // (i.e. in the case of Ctrl+W, where the call to // UnhandledKeyboardEvent destroys this RenderWidgetHost). } - - // This RenderWidgetHost was already deallocated, we can't do anything - // from now on, including resetting |death_flag_|. So just return. - if (is_dead) - return; - - // Reset |death_flag_| to NULL, otherwise it'll point to an invalid memory - // address after returning from this method. - death_flag_ = NULL; - - // Suppress the following Char events if the RawKeyDown event was handled - // by the browser rather than the renderer. - if (front_item.type == WebKeyboardEvent::RawKeyDown) - suppress_next_char_events_ = processed_by_browser; - - // If more than one key events in |key_queue_| were already sent to the - // renderer but haven't got ACK messages yet, we must wait for ACK - // messages of these key events before sending more key events to the - // renderer. - if (pending_key_events_ && key_queue_.size() == pending_key_events_) { - size_t i = 0; - if (suppress_next_char_events_) { - // Suppress the sequence of pending Char events if preceding - // RawKeyDown event was handled by the browser. - while (pending_key_events_ && - key_queue_[0].type == WebKeyboardEvent::Char) { - --pending_key_events_; - key_queue_.pop_front(); - } - } else { - // Otherwise, send these pending Char events to the renderer. - // Note: we can't remove them from |key_queue_|, as we still need to - // wait for their ACK messages from the renderer. - while (pending_key_events_ && - key_queue_[i].type == WebKeyboardEvent::Char) { - --pending_key_events_; - ForwardInputEvent(key_queue_[i++], sizeof(WebKeyboardEvent)); - } - } - - // Reset |suppress_next_char_events_| if there is still one or more - // pending KeyUp or RawKeyDown events in the queue. - // We can't reset it if there is no pending event anymore, because we - // don't know if the following event is a Char event or not. - if (pending_key_events_) - suppress_next_char_events_ = false; - - // We can safely send following pending KeyUp and RawKeyDown events to - // the renderer, until we meet another Char event. - while (pending_key_events_ && - key_queue_[i].type != WebKeyboardEvent::Char) { - --pending_key_events_; - ForwardInputEvent(key_queue_[i++], sizeof(WebKeyboardEvent)); - } - } - } - - // Send the pending PaintRect_ACK message after sending all pending key - // events or after a certain duration, so that the renderer can process - // the updates caused by the key events in batch. - if (paint_ack_postponed_ && (pending_key_events_ == 0 || - (TimeTicks::Now() - paint_ack_postponed_time_).InMilliseconds() > - kPaintACKMsgMaxPostponedDurationMS)) { - paint_ack_postponed_ = false; - Send(new ViewMsg_PaintRect_ACK(routing_id_)); } } diff --git a/chrome/browser/renderer_host/render_widget_host.h b/chrome/browser/renderer_host/render_widget_host.h index 4104c6d..dc425b0 100644 --- a/chrome/browser/renderer_host/render_widget_host.h +++ b/chrome/browser/renderer_host/render_widget_host.h @@ -345,8 +345,8 @@ class RenderWidgetHost : public IPC::Channel::Listener, protected: // Internal implementation of the public Forward*Event() methods. - void ForwardInputEvent( - const WebKit::WebInputEvent& input_event, int event_size); + void ForwardInputEvent(const WebKit::WebInputEvent& input_event, + int event_size, bool is_keyboard_shortcut); // Called when we receive a notification indicating that the renderer // process has gone. This will reset our state so that our state will be @@ -357,18 +357,20 @@ class RenderWidgetHost : public IPC::Channel::Listener, // This is used for various IPC messages, including plugins. gfx::NativeViewId GetNativeViewId(); - // Called when an InputEvent is received to check if the event should be sent - // to the renderer or not. - virtual bool ShouldSendToRenderer(const NativeWebKeyboardEvent& event) { - return true; + // Called to handled a keyboard event before sending it to the renderer. + // This is overridden by RenderView to send upwards to its delegate. + // Returns true if the event was handled, and then the keyboard event will + // not be sent to the renderer anymore. Otherwise, if the |event| would + // be handled in HandleKeyboardEvent() method as a normal keyboard shortcut, + // |*is_keyboard_shortcut| should be set to true. + virtual bool PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event, + bool* is_keyboard_shortcut) { + return false; } - // Called when we an InputEvent was not processed by the renderer. This is + // Called when a keyboard event was not processed by the renderer. This is // overridden by RenderView to send upwards to its delegate. - // Returns true if the event was handled by its delegate. - virtual bool UnhandledKeyboardEvent(const NativeWebKeyboardEvent& event) { - return false; - } + virtual void UnhandledKeyboardEvent(const NativeWebKeyboardEvent& event) {} // Notification that the user has made some kind of input that could // perform an action. The render view host overrides this to forward the @@ -541,7 +543,6 @@ class RenderWidgetHost : public IPC::Channel::Listener, // A queue of keyboard events. We can't trust data from the renderer so we // stuff key events into a queue and pop them out on ACK, feeding our copy // back to whatever unhandled handler instead of the returned version. - // See the description of |pending_key_events_| below for more details. KeyQueue key_queue_; // Set when we update the text direction of the selected input element. @@ -553,19 +554,6 @@ class RenderWidgetHost : public IPC::Channel::Listener, // NotifyTextDirection(). bool text_direction_canceled_; - // How many key events in |key_queue_| are not sent to the renderer yet, - // counted from the back of |key_queue_|. - // In order to suppress a Char event when necessary (see the description of - // |suppress_next_char_events_| below), we can't just send it to the - // renderer as soon as we get it. Instead, we need wait for the result of - // preceding RawKeyDown event back from the renderer, and then decide how to - // process the pending Char events according to the result. - // So if we get one or more Char events before receiving the result of - // preceding RawKeyDown event, we need keep them in |key_queue_|. And in - // order to keep the order the key events, all following key events must be - // pending until the pending Char events got processed. - size_t pending_key_events_; - // Indicates if the next sequence of Char events should be suppressed or not. // System may translate a RawKeyDown event into zero or more Char events, // usually we send them to the renderer directly in sequence. However, If a @@ -581,23 +569,6 @@ class RenderWidgetHost : public IPC::Channel::Listener, // changed. bool suppress_next_char_events_; - // True if the PaintRect_ACK message for the last PaintRect message is still - // not sent yet. This is used for optimizing the painting overhead when there - // are many pending key events in the queue. - bool paint_ack_postponed_; - - // The time when a PaintRect_ACK message is postponed, so that we can send the - // message after a certain duration. - base::TimeTicks paint_ack_postponed_time_; - - // During the call to some methods, eg. OnMsgInputEventAck, this - // RenderWidgetHost object may be destroyed before executing some code that - // still want to access this object. To avoid this situation, |death_flag_| - // shall be pointed to a local variable in the method, and then |*death_flag_| - // will be set to true when destroying this RenderWidgetHost object, then the - // method shall exit immediately when |*death_flag_| becomes true. - bool* death_flag_; - DISALLOW_COPY_AND_ASSIGN(RenderWidgetHost); }; diff --git a/chrome/browser/renderer_host/render_widget_host_unittest.cc b/chrome/browser/renderer_host/render_widget_host_unittest.cc index 027ab8b..e3f4517 100644 --- a/chrome/browser/renderer_host/render_widget_host_unittest.cc +++ b/chrome/browser/renderer_host/render_widget_host_unittest.cc @@ -124,8 +124,10 @@ class MockRenderWidgetHost : public RenderWidgetHost { public: MockRenderWidgetHost(RenderProcessHost* process, int routing_id) : RenderWidgetHost(process, routing_id), + prehandle_keyboard_event_(false), + prehandle_keyboard_event_called_(false), + prehandle_keyboard_event_type_(WebInputEvent::Undefined), unhandled_keyboard_event_called_(false), - handle_unhandled_keyboard_event_(false), unhandled_keyboard_event_type_(WebInputEvent::Undefined) { } @@ -139,20 +141,37 @@ class MockRenderWidgetHost : public RenderWidgetHost { return unhandled_keyboard_event_type_; } - void set_handle_unhandled_keyboard_event(bool handle) { - handle_unhandled_keyboard_event_ = handle; + bool prehandle_keyboard_event_called() const { + return prehandle_keyboard_event_called_; + } + + WebInputEvent::Type prehandle_keyboard_event_type() const { + return prehandle_keyboard_event_type_; + } + + void set_prehandle_keyboard_event(bool handle) { + prehandle_keyboard_event_ = handle; } protected: - virtual bool UnhandledKeyboardEvent(const NativeWebKeyboardEvent& event) { + virtual bool PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event, + bool* is_keyboard_shortcut) { + prehandle_keyboard_event_type_ = event.type; + prehandle_keyboard_event_called_ = true; + return prehandle_keyboard_event_; + } + + virtual void UnhandledKeyboardEvent(const NativeWebKeyboardEvent& event) { unhandled_keyboard_event_type_ = event.type; unhandled_keyboard_event_called_ = true; - return handle_unhandled_keyboard_event_; } private: + bool prehandle_keyboard_event_; + bool prehandle_keyboard_event_called_; + WebInputEvent::Type prehandle_keyboard_event_type_; + bool unhandled_keyboard_event_called_; - bool handle_unhandled_keyboard_event_; WebInputEvent::Type unhandled_keyboard_event_type_; }; @@ -496,67 +515,23 @@ TEST_F(RenderWidgetHostTest, IgnoreKeyEventsHandledByRenderer) { EXPECT_FALSE(host_->unhandled_keyboard_event_called()); } -TEST_F(RenderWidgetHostTest, DontSuppressNextCharEventsNoPending) { - // Simulate a keyboard event. - SimulateKeyboardEvent(WebInputEvent::RawKeyDown); - - // Make sure we sent the input event to the renderer. - EXPECT_TRUE(process_->sink().GetUniqueMessageMatching( - ViewMsg_HandleInputEvent::ID)); - process_->sink().ClearMessages(); - - // Send the simulated response from the renderer back. - SendInputEventACK(WebInputEvent::RawKeyDown, false); - - EXPECT_TRUE(host_->unhandled_keyboard_event_called()); - EXPECT_EQ(WebInputEvent::RawKeyDown, host_->unhandled_keyboard_event_type()); - - // Forward the Char event. - SimulateKeyboardEvent(WebInputEvent::Char); - - // Make sure we sent the input event to the renderer. - EXPECT_TRUE(process_->sink().GetUniqueMessageMatching( - ViewMsg_HandleInputEvent::ID)); - process_->sink().ClearMessages(); - - // Send the simulated response from the renderer back. - SendInputEventACK(WebInputEvent::Char, false); - - EXPECT_TRUE(host_->unhandled_keyboard_event_called()); - EXPECT_EQ(WebInputEvent::Char, host_->unhandled_keyboard_event_type()); - - // Forward the KeyUp event. - SimulateKeyboardEvent(WebInputEvent::KeyUp); - - // Make sure we sent the input event to the renderer. - EXPECT_TRUE(process_->sink().GetUniqueMessageMatching( - ViewMsg_HandleInputEvent::ID)); +TEST_F(RenderWidgetHostTest, PreHandleRawKeyDownEvent) { + // Simluate the situation that the browser handled the key down event during + // pre-handle phrase. + host_->set_prehandle_keyboard_event(true); process_->sink().ClearMessages(); - // Send the simulated response from the renderer back. - SendInputEventACK(WebInputEvent::KeyUp, false); - - EXPECT_TRUE(host_->unhandled_keyboard_event_called()); - EXPECT_EQ(WebInputEvent::KeyUp, host_->unhandled_keyboard_event_type()); -} - -TEST_F(RenderWidgetHostTest, SuppressNextCharEventsNoPending) { // Simulate a keyboard event. SimulateKeyboardEvent(WebInputEvent::RawKeyDown); - // Make sure we sent the input event to the renderer. - EXPECT_TRUE(process_->sink().GetUniqueMessageMatching( - ViewMsg_HandleInputEvent::ID)); - process_->sink().ClearMessages(); + EXPECT_TRUE(host_->prehandle_keyboard_event_called()); + EXPECT_EQ(WebInputEvent::RawKeyDown, host_->prehandle_keyboard_event_type()); - // Simluate the situation that the browser handled the key down event. - host_->set_handle_unhandled_keyboard_event(true); - - // Send the simulated response from the renderer back. - SendInputEventACK(WebInputEvent::RawKeyDown, false); + // Make sure the RawKeyDown event is not sent to the renderer. + EXPECT_EQ(0U, process_->sink().message_count()); - EXPECT_TRUE(host_->unhandled_keyboard_event_called()); - EXPECT_EQ(WebInputEvent::RawKeyDown, host_->unhandled_keyboard_event_type()); + // The browser won't pre-handle a Char event. + host_->set_prehandle_keyboard_event(false); // Forward the Char event. SimulateKeyboardEvent(WebInputEvent::Char); @@ -564,114 +539,10 @@ TEST_F(RenderWidgetHostTest, SuppressNextCharEventsNoPending) { // Make sure the Char event is suppressed. EXPECT_EQ(0U, process_->sink().message_count()); - // Forward another Char event. - SimulateKeyboardEvent(WebInputEvent::Char); - - // Make sure the Char event is suppressed. - EXPECT_EQ(0U, process_->sink().message_count()); - // Forward the KeyUp event. SimulateKeyboardEvent(WebInputEvent::KeyUp); - // Make sure we sent the input event to the renderer. - EXPECT_TRUE(process_->sink().GetUniqueMessageMatching( - ViewMsg_HandleInputEvent::ID)); - process_->sink().ClearMessages(); - - // The browser does not handle KeyUp events. - host_->set_handle_unhandled_keyboard_event(false); - - // Send the simulated response from the renderer back. - SendInputEventACK(WebInputEvent::KeyUp, false); - - EXPECT_TRUE(host_->unhandled_keyboard_event_called()); - EXPECT_EQ(WebInputEvent::KeyUp, host_->unhandled_keyboard_event_type()); -} - -TEST_F(RenderWidgetHostTest, DontSuppressNextCharEventsPending) { - SimulateKeyboardEvent(WebInputEvent::RawKeyDown); - - // Make sure we sent the input event to the renderer. - EXPECT_TRUE(process_->sink().GetUniqueMessageMatching( - ViewMsg_HandleInputEvent::ID)); - process_->sink().ClearMessages(); - - // Forward the Char event before receiving the ACK of previous KeyDown event. - SimulateKeyboardEvent(WebInputEvent::Char); - - // Make sure the Char event is pending. - EXPECT_EQ(0U, process_->sink().message_count()); - - // Forward the KeyUp event before receiving the ACK of previous KeyDown event. - SimulateKeyboardEvent(WebInputEvent::KeyUp); - - // Make sure the KeyUp event is pending. - EXPECT_EQ(0U, process_->sink().message_count()); - - // Send the simulated response of the KeyDown event from the renderer back. - SendInputEventACK(WebInputEvent::RawKeyDown, false); - - EXPECT_TRUE(host_->unhandled_keyboard_event_called()); - EXPECT_EQ(WebInputEvent::RawKeyDown, host_->unhandled_keyboard_event_type()); - - // Make sure both pending Char and KeyUp were sent to the renderer. - EXPECT_EQ(2U, process_->sink().message_count()); - EXPECT_EQ(ViewMsg_HandleInputEvent::ID, - process_->sink().GetMessageAt(0)->type()); - EXPECT_EQ(ViewMsg_HandleInputEvent::ID, - process_->sink().GetMessageAt(1)->type()); - process_->sink().ClearMessages(); - - // Send the simulated response from the renderer back. - SendInputEventACK(WebInputEvent::Char, false); - - EXPECT_TRUE(host_->unhandled_keyboard_event_called()); - EXPECT_EQ(WebInputEvent::Char, host_->unhandled_keyboard_event_type()); - - // Send the simulated response from the renderer back. - SendInputEventACK(WebInputEvent::KeyUp, false); - - EXPECT_TRUE(host_->unhandled_keyboard_event_called()); - EXPECT_EQ(WebInputEvent::KeyUp, host_->unhandled_keyboard_event_type()); -} - -TEST_F(RenderWidgetHostTest, SuppressNextCharEventsPending) { - SimulateKeyboardEvent(WebInputEvent::RawKeyDown); - - // Make sure we sent the KeyDown event to the renderer. - EXPECT_TRUE(process_->sink().GetUniqueMessageMatching( - ViewMsg_HandleInputEvent::ID)); - process_->sink().ClearMessages(); - - // Forward the Char event before receiving the ACK of previous KeyDown event. - SimulateKeyboardEvent(WebInputEvent::Char); - - // Make sure the Char event is pending. - EXPECT_EQ(0U, process_->sink().message_count()); - - // Forward another Char event before receiving the ACK of previous KeyDown - // event. - SimulateKeyboardEvent(WebInputEvent::Char); - - // Make sure the Char event is pending. - EXPECT_EQ(0U, process_->sink().message_count()); - - // Forward the KeyUp event before receiving the ACK of previous KeyDown event. - SimulateKeyboardEvent(WebInputEvent::KeyUp); - - // Make sure the KeyUp event is pending. - EXPECT_EQ(0U, process_->sink().message_count()); - - // Simluate the situation that the browser handled the key down event. - host_->set_handle_unhandled_keyboard_event(true); - - // Send the simulated response of the KeyDown event from the renderer back. - SendInputEventACK(WebInputEvent::RawKeyDown, false); - - EXPECT_TRUE(host_->unhandled_keyboard_event_called()); - EXPECT_EQ(WebInputEvent::RawKeyDown, host_->unhandled_keyboard_event_type()); - - // Make sure only pending KeyUp was sent to the renderer. + // Make sure only KeyUp was sent to the renderer. EXPECT_EQ(1U, process_->sink().message_count()); EXPECT_EQ(ViewMsg_HandleInputEvent::ID, process_->sink().GetMessageAt(0)->type()); @@ -683,51 +554,3 @@ TEST_F(RenderWidgetHostTest, SuppressNextCharEventsPending) { EXPECT_TRUE(host_->unhandled_keyboard_event_called()); EXPECT_EQ(WebInputEvent::KeyUp, host_->unhandled_keyboard_event_type()); } - -TEST_F(RenderWidgetHostTest, ManyKeyEventsPending) { - process_->sink().ClearMessages(); - - for (int i = 0; i < 10; ++i) { - // Forward a KeyDown event. - SimulateKeyboardEvent(WebInputEvent::RawKeyDown); - - // Forward a Char event before receiving the ACK of previous KeyDown event. - SimulateKeyboardEvent(WebInputEvent::Char); - - // Forward a KeyUp event before receiving the ACK of previous KeyDown event. - SimulateKeyboardEvent(WebInputEvent::KeyUp); - } - - // Make sure only the first KeyDown event was sent to the renderer. All others - // are pending. - EXPECT_EQ(1U, process_->sink().message_count()); - EXPECT_TRUE(process_->sink().GetUniqueMessageMatching( - ViewMsg_HandleInputEvent::ID)); - process_->sink().ClearMessages(); - - for (int i = 0; i < 10; ++i) { - // Send the simulated response of the KeyDown event from the renderer back. - SendInputEventACK(WebInputEvent::RawKeyDown, false); - EXPECT_TRUE(host_->unhandled_keyboard_event_called()); - EXPECT_EQ(WebInputEvent::RawKeyDown, - host_->unhandled_keyboard_event_type()); - - // Make sure the following pending Char, KeyUp and KeyDown event were sent - // to the renderer. - if (i < 9) - EXPECT_EQ(3U, process_->sink().message_count()); - else - EXPECT_EQ(2U, process_->sink().message_count()); - process_->sink().ClearMessages(); - - // Send the simulated response of the Char event from the renderer back. - SendInputEventACK(WebInputEvent::Char, false); - EXPECT_TRUE(host_->unhandled_keyboard_event_called()); - EXPECT_EQ(WebInputEvent::Char, host_->unhandled_keyboard_event_type()); - - // Send the simulated response of the KeyUp event from the renderer back. - SendInputEventACK(WebInputEvent::KeyUp, false); - EXPECT_TRUE(host_->unhandled_keyboard_event_called()); - EXPECT_EQ(WebInputEvent::KeyUp, host_->unhandled_keyboard_event_type()); - } -} diff --git a/chrome/browser/renderer_host/render_widget_host_view_mac.h b/chrome/browser/renderer_host/render_widget_host_view_mac.h index 0e65cef..c661578 100644 --- a/chrome/browser/renderer_host/render_widget_host_view_mac.h +++ b/chrome/browser/renderer_host/render_widget_host_view_mac.h @@ -43,18 +43,12 @@ class RWHVMEditCommandHelper; NSTrackingRectTag lastToolTipTag_; scoped_nsobject<NSString> toolTip_; - BOOL ignoreKeyEvents_; scoped_nsobject<NSEvent> lastKeyPressedEvent_; } - (void)setCanBeKeyView:(BOOL)can; - (void)setCloseOnDeactivate:(BOOL)b; - (void)setToolTipAtMousePoint:(NSString *)string; - -// When a keyboard event comes back from the renderer, we redispatch it. This -// makes sure we ignore it if we should receive it during redispatch, instead -// of sending it to the renderer again. -- (void)setIgnoreKeyEvents:(BOOL)ignorekeyEvents; @end /////////////////////////////////////////////////////////////////////////////// diff --git a/chrome/browser/renderer_host/render_widget_host_view_mac.mm b/chrome/browser/renderer_host/render_widget_host_view_mac.mm index 8317479..17f4a66 100644 --- a/chrome/browser/renderer_host/render_widget_host_view_mac.mm +++ b/chrome/browser/renderer_host/render_widget_host_view_mac.mm @@ -589,14 +589,7 @@ void RenderWidgetHostViewMac::SetBackground(const SkBitmap& background) { renderWidgetHostView_->render_widget_host_->ForwardMouseEvent(event); } -- (void)setIgnoreKeyEvents:(BOOL)ignorekeyEvents { - ignoreKeyEvents_ = ignorekeyEvents; -} - - (BOOL)performKeyEquivalent:(NSEvent*)theEvent { - if (ignoreKeyEvents_) - return NO; - // |performKeyEquivalent:| is sent to all views of a window, not only down the // responder chain (cf. "Handling Key Equivalents" in // http://developer.apple.com/mac/library/documentation/Cocoa/Conceptual/EventOverview/HandlingKeyEvents/HandlingKeyEvents.html @@ -642,9 +635,6 @@ void RenderWidgetHostViewMac::SetBackground(const SkBitmap& background) { } - (void)keyEvent:(NSEvent *)theEvent wasKeyEquivalent:(BOOL)equiv { - if (ignoreKeyEvents_) - return; - DCHECK([theEvent type] != NSKeyDown || !equiv == !([theEvent modifierFlags] & NSCommandKeyMask)); diff --git a/chrome/browser/tab_contents/interstitial_page.cc b/chrome/browser/tab_contents/interstitial_page.cc index aae90da..7212566 100644 --- a/chrome/browser/tab_contents/interstitial_page.cc +++ b/chrome/browser/tab_contents/interstitial_page.cc @@ -98,8 +98,9 @@ class InterstitialPage::InterstitialPageRVHViewDelegate virtual void UpdateDragCursor(WebDragOperation operation); virtual void GotFocus(); virtual void TakeFocus(bool reverse); - virtual bool IsReservedAccelerator(const NativeWebKeyboardEvent& event); - virtual bool HandleKeyboardEvent(const NativeWebKeyboardEvent& event); + virtual bool PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event, + bool* is_keyboard_shortcut); + virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event); virtual void HandleMouseEvent(); virtual void HandleMouseLeave(); virtual void OnFindReply(int request_id, @@ -586,17 +587,18 @@ void InterstitialPage::InterstitialPageRVHViewDelegate::TakeFocus( interstitial_page_->tab()->GetViewDelegate()->TakeFocus(reverse); } -bool InterstitialPage::InterstitialPageRVHViewDelegate::IsReservedAccelerator( - const NativeWebKeyboardEvent& event) { +bool InterstitialPage::InterstitialPageRVHViewDelegate::PreHandleKeyboardEvent( + const NativeWebKeyboardEvent& event, bool* is_keyboard_shortcut) { + if (interstitial_page_->tab() && interstitial_page_->tab()->GetViewDelegate()) + return interstitial_page_->tab()->GetViewDelegate()->PreHandleKeyboardEvent( + event, is_keyboard_shortcut); return false; } -bool InterstitialPage::InterstitialPageRVHViewDelegate::HandleKeyboardEvent( +void InterstitialPage::InterstitialPageRVHViewDelegate::HandleKeyboardEvent( const NativeWebKeyboardEvent& event) { if (interstitial_page_->tab() && interstitial_page_->tab()->GetViewDelegate()) - return interstitial_page_->tab()->GetViewDelegate()-> - HandleKeyboardEvent(event); - return false; + interstitial_page_->tab()->GetViewDelegate()->HandleKeyboardEvent(event); } void InterstitialPage::InterstitialPageRVHViewDelegate::HandleMouseEvent() { diff --git a/chrome/browser/tab_contents/tab_contents_delegate.h b/chrome/browser/tab_contents/tab_contents_delegate.h index 8e1379d..f67d8bb 100644 --- a/chrome/browser/tab_contents/tab_contents_delegate.h +++ b/chrome/browser/tab_contents/tab_contents_delegate.h @@ -222,18 +222,18 @@ class TabContentsDelegate { bool show_history) { } - // Returns whether the event is a reserved keyboard shortcut that should not - // be sent to the renderer. - virtual bool IsReservedAccelerator(const NativeWebKeyboardEvent& event) { + // Allows delegates to handle keyboard events before sending to the renderer. + // Returns true if the |event| was handled. Otherwise, if the |event| would be + // handled in HandleKeyboardEvent() method as a normal keyboard shortcut, + // |*is_keyboard_shortcut| should be set to true. + virtual bool PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event, + bool* is_keyboard_shortcut) { return false; } // Allows delegates to handle unhandled keyboard messages coming back from // the renderer. - // Returns true if the keyboard message was handled. - virtual bool HandleKeyboardEvent(const NativeWebKeyboardEvent& event) { - return false; - } + virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event) {} // Shows the repost form confirmation dialog box. virtual void ShowRepostFormWarningDialog(TabContents* tab_contents) {} diff --git a/chrome/browser/tab_contents/tab_contents_view.cc b/chrome/browser/tab_contents/tab_contents_view.cc index 9505069..533e6dc 100644 --- a/chrome/browser/tab_contents/tab_contents_view.cc +++ b/chrome/browser/tab_contents/tab_contents_view.cc @@ -58,10 +58,16 @@ void TabContentsView::ShowCreatedWidget(int route_id, ShowCreatedWidgetInternal(widget_host_view, initial_pos); } -bool TabContentsView::IsReservedAccelerator( - const NativeWebKeyboardEvent& event) { - return tab_contents()->delegate() && - tab_contents()->delegate()->IsReservedAccelerator(event); +bool TabContentsView::PreHandleKeyboardEvent( + const NativeWebKeyboardEvent& event, bool* is_keyboard_shortcut) { + return tab_contents_->delegate() && + tab_contents_->delegate()->PreHandleKeyboardEvent( + event, is_keyboard_shortcut); +} + +void TabContentsView::HandleKeyboardEvent(const NativeWebKeyboardEvent& event) { + if (tab_contents_->delegate()) + tab_contents_->delegate()->HandleKeyboardEvent(event); } RenderWidgetHostView* TabContentsView::CreateNewWidgetInternal( diff --git a/chrome/browser/tab_contents/tab_contents_view.h b/chrome/browser/tab_contents/tab_contents_view.h index 4f55ab5..a7ca049 100644 --- a/chrome/browser/tab_contents/tab_contents_view.h +++ b/chrome/browser/tab_contents/tab_contents_view.h @@ -115,6 +115,17 @@ class TabContentsView : public RenderViewHostDelegate::View { // invoked, SetInitialFocus is invoked. virtual void RestoreFocus() = 0; + // Keyboard events forwarding from the RenderViewHost. + // The default implementation just forward the events to the + // TabContentsDelegate object. + virtual bool PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event, + bool* is_keyboard_shortcut); + + // Keyboard events forwarding from the RenderViewHost. + // The default implementation just forward the events to the + // TabContentsDelegate object. + virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event); + // Simple mouse event forwarding from the RenderViewHost. virtual void HandleMouseEvent() {} virtual void HandleMouseLeave() {} @@ -175,7 +186,6 @@ class TabContentsView : public RenderViewHostDelegate::View { bool user_gesture, const GURL& creator_url); virtual void ShowCreatedWidget(int route_id, const gfx::Rect& initial_pos); - virtual bool IsReservedAccelerator(const NativeWebKeyboardEvent& event); // The TabContents whose contents we display. TabContents* tab_contents_; diff --git a/chrome/browser/tab_contents/tab_contents_view_gtk.cc b/chrome/browser/tab_contents/tab_contents_view_gtk.cc index d0a2635..f37e3e6 100644 --- a/chrome/browser/tab_contents/tab_contents_view_gtk.cc +++ b/chrome/browser/tab_contents/tab_contents_view_gtk.cc @@ -318,36 +318,6 @@ void TabContentsViewGtk::TakeFocus(bool reverse) { } } -bool TabContentsViewGtk::HandleKeyboardEvent( - const NativeWebKeyboardEvent& event) { - // This may be an accelerator. Try to pass it on to our browser window - // to handle. - GtkWindow* window = GetTopLevelNativeWindow(); - if (!window) { - NOTREACHED(); - return false; - } - - // Filter out pseudo key events created by GtkIMContext signal handlers. - // Since GtkIMContext signal handlers don't use GdkEventKey objects, its - // |event.os_event| values are dummy values (or NULL.) - // We should filter out these pseudo key events to prevent unexpected - // behaviors caused by them. - // We should also filter out the KeyUp event as it should not be processed - // as an accelerator. - if (event.type == WebKit::WebInputEvent::Char || - event.type == WebKit::WebInputEvent::KeyUp) - return false; - - BrowserWindowGtk* browser_window = - BrowserWindowGtk::GetBrowserWindowForNativeWindow(window); - - // If this is a dialog, the top level window isn't a browser. In this case, - // we just return false. - return browser_window ? - browser_window->HandleKeyboardEvent(event.os_event) : false; -} - void TabContentsViewGtk::Observe(NotificationType type, const NotificationSource& source, const NotificationDetails& details) { diff --git a/chrome/browser/tab_contents/tab_contents_view_gtk.h b/chrome/browser/tab_contents/tab_contents_view_gtk.h index 6e60232..61e5e21 100644 --- a/chrome/browser/tab_contents/tab_contents_view_gtk.h +++ b/chrome/browser/tab_contents/tab_contents_view_gtk.h @@ -69,7 +69,6 @@ class TabContentsViewGtk : public TabContentsView, virtual void UpdateDragCursor(WebKit::WebDragOperation operation); virtual void GotFocus(); virtual void TakeFocus(bool reverse); - virtual bool HandleKeyboardEvent(const NativeWebKeyboardEvent& event); // NotificationObserver implementation --------------------------------------- diff --git a/chrome/browser/tab_contents/tab_contents_view_mac.h b/chrome/browser/tab_contents/tab_contents_view_mac.h index 4da8267..c3801a7 100644 --- a/chrome/browser/tab_contents/tab_contents_view_mac.h +++ b/chrome/browser/tab_contents/tab_contents_view_mac.h @@ -77,7 +77,6 @@ class TabContentsViewMac : public TabContentsView, virtual void UpdateDragCursor(WebKit::WebDragOperation operation); virtual void GotFocus(); virtual void TakeFocus(bool reverse); - virtual bool HandleKeyboardEvent(const NativeWebKeyboardEvent& event); // NotificationObserver implementation --------------------------------------- diff --git a/chrome/browser/tab_contents/tab_contents_view_mac.mm b/chrome/browser/tab_contents/tab_contents_view_mac.mm index 763aa3b..14b16c6 100644 --- a/chrome/browser/tab_contents/tab_contents_view_mac.mm +++ b/chrome/browser/tab_contents/tab_contents_view_mac.mm @@ -47,7 +47,6 @@ COMPILE_ASSERT_MATCHING_ENUM(DragOperationEvery); @interface TabContentsViewCocoa (Private) - (id)initWithTabContentsViewMac:(TabContentsViewMac*)w; -- (BOOL)processKeyboardEvent:(NativeWebKeyboardEvent*)event; - (void)registerDragTypes; - (void)setCurrentDragOperation:(NSDragOperation)operation; - (void)startDragWithDropData:(const WebDropData&)dropData @@ -231,12 +230,6 @@ void TabContentsViewMac::TakeFocus(bool reverse) { } } -bool TabContentsViewMac::HandleKeyboardEvent( - const NativeWebKeyboardEvent& event) { - return [cocoa_view_.get() processKeyboardEvent: - const_cast<NativeWebKeyboardEvent*>(&event)] == YES; -} - void TabContentsViewMac::ShowContextMenu(const ContextMenuParams& params) { RenderViewContextMenuMac menu(tab_contents(), params, @@ -355,48 +348,6 @@ void TabContentsViewMac::Observe(NotificationType type, return tabContentsView_->tab_contents(); } -- (BOOL)processKeyboardEvent:(NativeWebKeyboardEvent*)wkEvent { - if (wkEvent->skip_in_browser || wkEvent->type == WebKit::WebInputEvent::Char) - return NO; - - NSEvent* event = wkEvent->os_event; - DCHECK(event != NULL); - - // If this tab is no longer active, its window will be |nil|. In that case, - // best ignore the event. - if (![self window]) - return NO; - ChromeEventProcessingWindow* window = - (ChromeEventProcessingWindow*)[self window]; - DCHECK([window isKindOfClass:[ChromeEventProcessingWindow class]]); - - // Do not fire shortcuts on key up. - if ([event type] == NSKeyDown) { - // Send the event to the menu before sending it to the browser/window - // shortcut handling, so that if a user configures cmd-left to mean - // "previous tab", it takes precedence over the built-in "history back" - // binding. Other than that, the |redispatchEvent| call would take care of - // invoking the original menu item shortcut as well. - if ([[NSApp mainMenu] performKeyEquivalent:event]) - return YES; - - if ([window handleExtraBrowserKeyboardShortcut:event]) - return YES; - if ([window handleExtraWindowKeyboardShortcut:event]) - return YES; - } - - // We need to re-dispatch the event, so that it is sent to the menu or other - // cocoa mechanisms (such as the cmd-` handler). - RenderWidgetHostViewCocoa* rwhv = static_cast<RenderWidgetHostViewCocoa*>( - tabContentsView_->GetContentNativeView()); - DCHECK([rwhv isKindOfClass:[RenderWidgetHostViewCocoa class]]); - [rwhv setIgnoreKeyEvents:YES]; - BOOL eventHandled = [window redispatchEvent:event]; - [rwhv setIgnoreKeyEvents:NO]; - return eventHandled; -} - - (void)mouseEvent:(NSEvent *)theEvent { TabContents* tabContents = [self tabContents]; if (tabContents->delegate()) { diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc index 5e10dd2..c422d76 100644 --- a/chrome/browser/views/frame/browser_view.cc +++ b/chrome/browser/views/frame/browser_view.cc @@ -1225,7 +1225,25 @@ void BrowserView::ShowAppMenu() { toolbar_->app_menu()->Activate(); } -int BrowserView::GetCommandId(const NativeWebKeyboardEvent& event) { +bool BrowserView::PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event, + bool* is_keyboard_shortcut) { + if (event.type != WebKit::WebInputEvent::RawKeyDown) + return false; + +#if defined(OS_WIN) + // As Alt+F4 is the close-app keyboard shortcut, it needs processing + // immediately. + if (event.windowsKeyCode == base::VKEY_F4 && + event.modifiers == NativeWebKeyboardEvent::AltKey) { + DefWindowProc(event.os_event.hwnd, event.os_event.message, + event.os_event.wParam, event.os_event.lParam); + return true; + } +#endif + + views::FocusManager* focus_manager = GetFocusManager(); + DCHECK(focus_manager); + views::Accelerator accelerator( static_cast<base::KeyboardCode>(event.windowsKeyCode), (event.modifiers & NativeWebKeyboardEvent::ShiftKey) == @@ -1235,12 +1253,68 @@ int BrowserView::GetCommandId(const NativeWebKeyboardEvent& event) { (event.modifiers & NativeWebKeyboardEvent::AltKey) == NativeWebKeyboardEvent::AltKey); - std::map<views::Accelerator, int>::const_iterator iter = - accelerator_table_.find(accelerator); - if (iter == accelerator_table_.end()) - return -1; + // We first find out the browser command associated to the |event|. + // Then if the command is a reserved one, and should be processed + // immediately according to the |event|, the command will be executed + // immediately. Otherwise we just set |*is_keyboard_shortcut| properly and + // return false. - return iter->second; + // This piece of code is based on the fact that accelerators registered + // into the |focus_manager| may only trigger a browser command execution. + // + // Here we need to retrieve the command id (if any) associated to the + // keyboard event. Instead of looking up the command id in the + // |accelerator_table_| by ourselves, we block the command execution of + // the |browser_| object then send the keyboard event to the + // |focus_manager| as if we are activating an accelerator key. + // Then we can retrieve the command id from the |browser_| object. + browser_->SetBlockCommandExecution(true); + focus_manager->ProcessAccelerator(accelerator); + int id = browser_->GetLastBlockedCommand(NULL); + browser_->SetBlockCommandExecution(false); + + if (id == -1) + return false; + + if (browser_->IsReservedCommand(id)) { + // TODO(suzhe): For Linux, should we send things like Ctrl+w, Ctrl+n + // to the renderer first, just like what + // BrowserWindowGtk::HandleKeyboardEvent() does? + // Executing the command may cause |this| object to be destroyed. + browser_->ExecuteCommand(id); + return true; + } + + DCHECK(is_keyboard_shortcut != NULL); + *is_keyboard_shortcut = true; + + return false; +} + +void BrowserView::HandleKeyboardEvent(const NativeWebKeyboardEvent& event) { + if (event.type == WebKit::WebInputEvent::RawKeyDown) { + views::FocusManager* focus_manager = GetFocusManager(); + DCHECK(focus_manager); + + views::Accelerator accelerator( + static_cast<base::KeyboardCode>(event.windowsKeyCode), + (event.modifiers & NativeWebKeyboardEvent::ShiftKey) == + NativeWebKeyboardEvent::ShiftKey, + (event.modifiers & NativeWebKeyboardEvent::ControlKey) == + NativeWebKeyboardEvent::ControlKey, + (event.modifiers & NativeWebKeyboardEvent::AltKey) == + NativeWebKeyboardEvent::AltKey); + + if (focus_manager->ProcessAccelerator(accelerator)) + return; + } + +#if defined(OS_WIN) + // Any unhandled keyboard/character messages should be defproced. + // This allows stuff like F10, etc to work correctly. + DefWindowProc(event.os_event.hwnd, event.os_event.message, + event.os_event.wParam, event.os_event.lParam); +#endif } #if defined(TOOLKIT_VIEWS) diff --git a/chrome/browser/views/frame/browser_view.h b/chrome/browser/views/frame/browser_view.h index 5b3de61..f070cdf 100644 --- a/chrome/browser/views/frame/browser_view.h +++ b/chrome/browser/views/frame/browser_view.h @@ -279,7 +279,9 @@ class BrowserView : public BrowserWindow, bool show_history); virtual void ShowAppMenu(); virtual void ShowPageMenu(); - virtual int GetCommandId(const NativeWebKeyboardEvent& event); + virtual bool PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event, + bool* is_keyboard_shortcut); + virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event); virtual void ShowCreateShortcutsDialog(TabContents* tab_contents); #if defined(TOOLKIT_VIEWS) virtual void ToggleCompactNavigationBar(); diff --git a/chrome/browser/views/notifications/balloon_view_host.h b/chrome/browser/views/notifications/balloon_view_host.h index 6e18dc3..de3b741 100644 --- a/chrome/browser/views/notifications/balloon_view_host.h +++ b/chrome/browser/views/notifications/balloon_view_host.h @@ -75,12 +75,11 @@ class BalloonViewHost : public views::NativeViewHost, virtual void UpdateDragCursor(WebKit::WebDragOperation operation) {} virtual void GotFocus() {} virtual void TakeFocus(bool reverse) {} - virtual bool IsReservedAccelerator(const NativeWebKeyboardEvent& event) { - return false; - } - virtual bool HandleKeyboardEvent(const NativeWebKeyboardEvent& event) { + virtual bool PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event, + bool* is_keyboard_shortcut) { return false; } + virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event) {} virtual void HandleMouseEvent() {} virtual void HandleMouseLeave() {} virtual void UpdatePreferredSize(const gfx::Size& pref_size); diff --git a/chrome/browser/views/tab_contents/tab_contents_view_gtk.cc b/chrome/browser/views/tab_contents/tab_contents_view_gtk.cc index 86a2452..db41397 100644 --- a/chrome/browser/views/tab_contents/tab_contents_view_gtk.cc +++ b/chrome/browser/views/tab_contents/tab_contents_view_gtk.cc @@ -314,37 +314,6 @@ void TabContentsViewGtk::TakeFocus(bool reverse) { reverse ? GTK_DIR_TAB_BACKWARD : GTK_DIR_TAB_FORWARD); } -bool TabContentsViewGtk::HandleKeyboardEvent( - const NativeWebKeyboardEvent& event) { - // The renderer returned a keyboard event it did not process. This may be - // a keyboard shortcut that we have to process. - if (event.type != WebInputEvent::RawKeyDown) - return false; - - views::FocusManager* focus_manager = - views::FocusManager::GetFocusManagerForNativeView(GetNativeView()); - // We may not have a focus_manager at this point (if the tab has been switched - // by the time this message returned). - if (!focus_manager) - return false; - - bool shift_pressed = (event.modifiers & WebInputEvent::ShiftKey) == - WebInputEvent::ShiftKey; - bool ctrl_pressed = (event.modifiers & WebInputEvent::ControlKey) == - WebInputEvent::ControlKey; - bool alt_pressed = (event.modifiers & WebInputEvent::AltKey) == - WebInputEvent::AltKey; - - return focus_manager->ProcessAccelerator( - views::Accelerator(static_cast<base::KeyboardCode>(event.windowsKeyCode), - shift_pressed, ctrl_pressed, alt_pressed)); - // DANGER: |this| could be deleted now! - - // Note that we do not handle Gtk mnemonics/accelerators or binding set here - // (as it is done in BrowserWindowGtk::HandleKeyboardEvent), as we override - // Gtk behavior completely. -} - void TabContentsViewGtk::ShowContextMenu(const ContextMenuParams& params) { // Allow delegates to handle the context menu operation first. if (tab_contents()->delegate()->HandleContextMenu(params)) diff --git a/chrome/browser/views/tab_contents/tab_contents_view_gtk.h b/chrome/browser/views/tab_contents/tab_contents_view_gtk.h index 9e3cbdda..19ecf51 100644 --- a/chrome/browser/views/tab_contents/tab_contents_view_gtk.h +++ b/chrome/browser/views/tab_contents/tab_contents_view_gtk.h @@ -65,7 +65,6 @@ class TabContentsViewGtk : public TabContentsView, virtual void UpdateDragCursor(WebKit::WebDragOperation operation); virtual void GotFocus(); virtual void TakeFocus(bool reverse); - virtual bool HandleKeyboardEvent(const NativeWebKeyboardEvent& event); private: // Signal handlers ----------------------------------------------------------- diff --git a/chrome/browser/views/tab_contents/tab_contents_view_win.cc b/chrome/browser/views/tab_contents/tab_contents_view_win.cc index 90a3457..b46290b 100644 --- a/chrome/browser/views/tab_contents/tab_contents_view_win.cc +++ b/chrome/browser/views/tab_contents/tab_contents_view_win.cc @@ -47,7 +47,6 @@ TabContentsView* TabContentsView::Create(TabContents* tab_contents) { TabContentsViewWin::TabContentsViewWin(TabContents* tab_contents) : TabContentsView(tab_contents), - ignore_next_char_event_(false), focus_manager_(NULL), close_tab_after_drag_ends_(false), sad_tab_(NULL) { @@ -362,71 +361,6 @@ void TabContentsViewWin::TakeFocus(bool reverse) { } } -bool TabContentsViewWin::HandleKeyboardEvent( - const NativeWebKeyboardEvent& event) { - // Previous calls to TranslateMessage can generate CHAR events as well as - // RAW_KEY_DOWN events, even if the latter triggered an accelerator. In these - // cases, we discard the CHAR events. - if (event.type == WebInputEvent::Char && ignore_next_char_event_) { - ignore_next_char_event_ = false; - return true; - } - ignore_next_char_event_ = false; - - // The renderer returned a keyboard event it did not process. This may be - // a keyboard shortcut that we have to process. - if (event.type == WebInputEvent::RawKeyDown) { - views::FocusManager* focus_manager = - views::FocusManager::GetFocusManagerForNativeView(GetNativeView()); - // We may not have a focus_manager at this point (if the tab has been - // switched by the time this message returned). - if (focus_manager) { - views::Accelerator accelerator( - win_util::WinToKeyboardCode(event.windowsKeyCode), - (event.modifiers & WebInputEvent::ShiftKey) == - WebInputEvent::ShiftKey, - (event.modifiers & WebInputEvent::ControlKey) == - WebInputEvent::ControlKey, - (event.modifiers & WebInputEvent::AltKey) == - WebInputEvent::AltKey); - - // This is tricky: we want to set ignore_next_char_event_ if - // ProcessAccelerator returns true. But ProcessAccelerator might delete - // |this| if the accelerator is a "close tab" one. So we speculatively - // set the flag and fix it if no event was handled. - ignore_next_char_event_ = true; - if (focus_manager->ProcessAccelerator(accelerator)) { - // DANGER: |this| could be deleted now! - return true; - } else { - // ProcessAccelerator didn't handle the accelerator, so we know both - // that |this| is still valid, and that we didn't want to set the flag. - ignore_next_char_event_ = false; - } - } - } - - if (tab_contents()->delegate() && - tab_contents()->delegate()->HandleKeyboardEvent(event)) { - // At this point the only tab contents delegate which handles a keyboard - // event is ChromeFrame. When the message comes back from ChromeFrame it - // is DefWindowProc'ed. We return false here on the same lines as below:- - return false; - } - - // Any unhandled keyboard/character messages should be defproced. - // This allows stuff like Alt+F4, etc to work correctly. - DefWindowProc(event.os_event.hwnd, event.os_event.message, - event.os_event.wParam, event.os_event.lParam); - - // DefWindowProc() always returns 0, which means it handled the event. - // But actually DefWindowProc() will only handle very few system key strokes, - // such as F10, Alt+Tab, Alt+F4, Alt+Esc, etc. - // So returning false here is just ok for most cases. - // Reference: http://msdn.microsoft.com/en-us/library/ms646267(VS.85).aspx - return false; -} - views::FocusManager* TabContentsViewWin::GetFocusManager() { views::FocusManager* focus_manager = WidgetWin::GetFocusManager(); if (focus_manager) { diff --git a/chrome/browser/views/tab_contents/tab_contents_view_win.h b/chrome/browser/views/tab_contents/tab_contents_view_win.h index 5b63dfc..5987baa 100644 --- a/chrome/browser/views/tab_contents/tab_contents_view_win.h +++ b/chrome/browser/views/tab_contents/tab_contents_view_win.h @@ -58,7 +58,6 @@ class TabContentsViewWin : public TabContentsView, virtual void UpdateDragCursor(WebKit::WebDragOperation operation); virtual void GotFocus(); virtual void TakeFocus(bool reverse); - virtual bool HandleKeyboardEvent(const NativeWebKeyboardEvent& event); // WidgetWin overridde. virtual views::FocusManager* GetFocusManager(); @@ -109,9 +108,6 @@ class TabContentsViewWin : public TabContentsView, // visible. SadTabView* sad_tab_; - // Whether to ignore the next CHAR keyboard event. - bool ignore_next_char_event_; - // The id used in the ViewStorage to store the last focused view. int last_focused_view_storage_id_; diff --git a/chrome/common/render_messages_internal.h b/chrome/common/render_messages_internal.h index d704238..e8d8676 100644 --- a/chrome/common/render_messages_internal.h +++ b/chrome/common/render_messages_internal.h @@ -151,7 +151,10 @@ IPC_BEGIN_MESSAGES(View) // This signals the render view that it can send another ScrollRect message. IPC_MESSAGE_ROUTED0(ViewMsg_ScrollRect_ACK) - // Message payload is a blob that should be cast to WebInputEvent + // Message payload includes: + // 1. A blob that should be cast to WebInputEvent + // 2. An optional boolean value indicating if a RawKeyDown event is associated + // to a keyboard shortcut of the browser. IPC_MESSAGE_ROUTED0(ViewMsg_HandleInputEvent) // This message notifies the renderer that the next key event is bound to one diff --git a/chrome/renderer/render_widget.cc b/chrome/renderer/render_widget.cc index fd60468..14c7a7d 100644 --- a/chrome/renderer/render_widget.cc +++ b/chrome/renderer/render_widget.cc @@ -67,7 +67,8 @@ RenderWidget::RenderWidget(RenderThreadBase* render_thread, bool activatable) ime_control_updated_(false), ime_control_busy_(false), activatable_(activatable), - pending_window_rect_count_(0) { + pending_window_rect_count_(0), + suppress_next_char_events_(false) { RenderProcess::current()->AddRefProcess(); DCHECK(render_thread_); } @@ -304,9 +305,24 @@ void RenderWidget::OnHandleInputEvent(const IPC::Message& message) { const WebInputEvent* input_event = reinterpret_cast<const WebInputEvent*>(data); + + bool is_keyboard_shortcut = false; + // is_keyboard_shortcut flag is only available for RawKeyDown events. + if (input_event->type == WebInputEvent::RawKeyDown) + message.ReadBool(&iter, &is_keyboard_shortcut); + bool processed = false; - if (webwidget_) - processed = webwidget_->handleInputEvent(*input_event); + if (input_event->type != WebInputEvent::Char || !suppress_next_char_events_) { + suppress_next_char_events_ = false; + if (webwidget_) + processed = webwidget_->handleInputEvent(*input_event); + } + + // If this RawKeyDown event corresponds to a browser keyboard shortcut and + // it's not processed by webkit, then we need to suppress the upcoming Char + // events. + if (!processed && is_keyboard_shortcut) + suppress_next_char_events_ = true; IPC::Message* response = new ViewHostMsg_HandleInputEvent_ACK(routing_id_); response->WriteInt(input_event->type); @@ -323,9 +339,7 @@ void RenderWidget::OnHandleInputEvent(const IPC::Message& message) { handling_input_event_ = false; - WebInputEvent::Type type = input_event->type; - if (type == WebInputEvent::RawKeyDown || type == WebInputEvent::KeyDown || - type == WebInputEvent::KeyUp || type == WebInputEvent::Char) + if (WebInputEvent::isKeyboardEventType(input_event->type)) DidHandleKeyEvent(); } diff --git a/chrome/renderer/render_widget.h b/chrome/renderer/render_widget.h index 0dcc98a..c25c99a 100644 --- a/chrome/renderer/render_widget.h +++ b/chrome/renderer/render_widget.h @@ -316,6 +316,9 @@ class RenderWidget : public IPC::Channel::Listener, scoped_ptr<IPC::Message> pending_input_event_ack_; + // Indicates if the next sequence of Char events should be suppressed or not. + bool suppress_next_char_events_; + DISALLOW_COPY_AND_ASSIGN(RenderWidget); }; diff --git a/chrome/test/test_browser_window.h b/chrome/test/test_browser_window.h index af73777..1028b0a 100644 --- a/chrome/test/test_browser_window.h +++ b/chrome/test/test_browser_window.h @@ -50,7 +50,11 @@ class TestBrowserWindow : public BrowserWindow { virtual void FocusToolbar() {} virtual void ShowPageMenu() {} virtual void ShowAppMenu() {} - virtual int GetCommandId(const NativeWebKeyboardEvent& event) { return -1; } + virtual bool PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event, + bool* is_keyboard_shortcut) { + return false; + } + virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event) {} virtual void ShowCreateShortcutsDialog(TabContents* tab_contents) {} #if defined(TOOLKIT_VIEWS) virtual void ToggleCompactNavigationBar() {} |