diff options
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() {} |