Allow TabContentsDelegates to process unhandled keyboard messages coming back from the renderer. This is required

for ExternalTabContainer to be able to pass unhandled keyboard accelerator messages back to the external host.
This worked before as the focus manager would hand off these messages to keystroke listeners like the ExternalTabContainer.
However with the ExternalTabContainer no longer using the focus manager this does not work anymore.

Removed the keystroke listener code from the focus manager as the ExternalTabContainer was the only consumer and it does not use it anymore.

This fixes bug http://b/issue?id=1975749

Bug=1975749


Review URL: http://codereview.chromium.org/149520

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@20600 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 47 insertions, 100 deletions

diff --git a/chrome/browser/external_tab_container.cc b/chrome/browser/external_tab_container.cc
index 732aead..a2d71b5 100644
--- a/chrome/browser/external_tab_container.cc
+++ b/chrome/browser/external_tab_container.cc
@@ -299,6 +299,14 @@ bool ExternalTabContainer::ExecuteContextMenuCommand(int command) {
   return true;
 }
 
+bool ExternalTabContainer::HandleKeyboardEvent(
+    const NativeWebKeyboardEvent& event) {
+  return ProcessUnhandledKeyStroke(event.os_event.hwnd,
+                                   event.os_event.message,
+                                   event.os_event.wParam,
+                                   event.os_event.lParam);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // ExternalTabContainer, NotificationObserver implementation:
 
@@ -392,10 +400,25 @@ void ExternalTabContainer::OnDestroy() {
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// ExternalTabContainer, views::KeystrokeListener implementation:
+// ExternalTabContainer, private:
+
+void ExternalTabContainer::Uninitialize(HWND window) {
+  registrar_.RemoveAll();
+  if (tab_contents_) {
+    NotificationService::current()->Notify(
+        NotificationType::EXTERNAL_TAB_CLOSED,
+        Source<NavigationController>(&tab_contents_->controller()),
+        Details<ExternalTabContainer>(this));
+
+    delete tab_contents_;
+    tab_contents_ = NULL;
+  }
+}
 
-bool ExternalTabContainer::ProcessKeyStroke(HWND window, UINT message,
-                                            WPARAM wparam, LPARAM lparam) {
+bool ExternalTabContainer::ProcessUnhandledKeyStroke(HWND window,
+                                                     UINT message,
+                                                     WPARAM wparam,
+                                                     LPARAM lparam) {
   if (!automation_) {
     return false;
   }
@@ -430,19 +453,3 @@ bool ExternalTabContainer::ProcessKeyStroke(HWND window, UINT message,
 
   return false;
 }
-
-////////////////////////////////////////////////////////////////////////////////
-// ExternalTabContainer, private:
-
-void ExternalTabContainer::Uninitialize(HWND window) {
-  registrar_.RemoveAll();
-  if (tab_contents_) {
-    NotificationService::current()->Notify(
-        NotificationType::EXTERNAL_TAB_CLOSED,
-        Source<NavigationController>(&tab_contents_->controller()),
-        Details<ExternalTabContainer>(this));
-
-    delete tab_contents_;
-    tab_contents_ = NULL;
-  }
-}
diff --git a/chrome/browser/external_tab_container.h b/chrome/browser/external_tab_container.h
index e69283c..d40a0f4 100644
--- a/chrome/browser/external_tab_container.h
+++ b/chrome/browser/external_tab_container.h
@@ -25,8 +25,7 @@ class RenderViewContextMenuExternalWin;
 // TODO(beng): Should override WidgetWin instead of Widget.
 class ExternalTabContainer : public TabContentsDelegate,
                              public NotificationObserver,
-                             public views::WidgetWin,
-                             public views::KeystrokeListener {
+                             public views::WidgetWin {
  public:
   ExternalTabContainer(AutomationProvider* automation,
       AutomationResourceMessageFilter* filter);
@@ -89,6 +88,8 @@ class ExternalTabContainer : public TabContentsDelegate,
     return true;
   };
 
+  virtual bool HandleKeyboardEvent(const NativeWebKeyboardEvent& event);
+
   virtual bool TakeFocus(bool reverse);
 
   // Overridden from NotificationObserver:
@@ -96,10 +97,6 @@ class ExternalTabContainer : public TabContentsDelegate,
                        const NotificationSource& source,
                        const NotificationDetails& details);
 
-  // Overridden from views::KeystrokeListener:
-  virtual bool ProcessKeyStroke(HWND window, UINT message, WPARAM wparam,
-                                LPARAM lparam);
-
   // Handles the context menu display operation. This allows external
   // hosts to customize the menu.
   virtual bool HandleContextMenu(const ContextMenuParams& params);
@@ -119,6 +116,11 @@ class ExternalTabContainer : public TabContentsDelegate,
   // 2. In the destructor.
   void Uninitialize(HWND window);
 
+  // Helper function for processing keystokes coming back from the renderer
+  // process.
+  bool ProcessUnhandledKeyStroke(HWND window, UINT message, WPARAM wparam,
+                                 LPARAM lparam);
+
   TabContents* tab_contents_;
   scoped_refptr<AutomationProvider> automation_;
 
diff --git a/chrome/browser/tab_contents/tab_contents_delegate.h b/chrome/browser/tab_contents/tab_contents_delegate.h
index b93de6e..b00f31f 100644
--- a/chrome/browser/tab_contents/tab_contents_delegate.h
+++ b/chrome/browser/tab_contents/tab_contents_delegate.h
@@ -8,6 +8,7 @@
 #include "base/basictypes.h"
 #include "base/gfx/native_widget_types.h"
 #include "base/gfx/rect.h"
+#include "chrome/common/native_web_keyboard_event.h"
 #include "chrome/common/page_transition_types.h"
 #include "chrome/common/renderer_preferences.h"
 #include "webkit/glue/context_menu.h"
@@ -205,6 +206,13 @@ class TabContentsDelegate {
                                         Profile* profile) {
   }
 
+  // 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;
+  }
+
  protected:
   ~TabContentsDelegate() {}
   RendererPreferences renderer_preferences_;
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 7b6aabb..543cff4 100644
--- a/chrome/browser/views/tab_contents/tab_contents_view_win.cc
+++ b/chrome/browser/views/tab_contents/tab_contents_view_win.cc
@@ -421,6 +421,11 @@ void TabContentsViewWin::HandleKeyboardEvent(
     }
   }
 
+  if (tab_contents()->delegate() &&
+      tab_contents()->delegate()->HandleKeyboardEvent(event)) {
+    return;
+  }
+
   // Any unhandled keyboard/character messages should be defproced.
   // This allows stuff like Alt+F4, etc to work correctly.
   DefWindowProc(event.os_event.hwnd,
diff --git a/views/focus/focus_manager.cc b/views/focus/focus_manager.cc
index dc26649..04f1f9b 100644
--- a/views/focus/focus_manager.cc
+++ b/views/focus/focus_manager.cc
@@ -57,21 +57,6 @@ bool FocusManager::OnKeyDown(HWND window, UINT message, WPARAM wparam,
     return false;
   }
 
-  // First give the registered keystroke handlers a chance a processing
-  // the message
-  // Do some basic checking to try to catch evil listeners that change the list
-  // from under us.
-  KeystrokeListenerList::size_type original_count =
-      keystroke_listeners_.size();
-  for (int i = 0; i < static_cast<int>(keystroke_listeners_.size()); i++) {
-    if (keystroke_listeners_[i]->ProcessKeyStroke(window, message, wparam,
-                                                  lparam)) {
-      return false;
-    }
-  }
-  DCHECK_EQ(original_count, keystroke_listeners_.size())
-      << "KeystrokeListener list modified during notification";
-
   int virtual_key_code = static_cast<int>(wparam);
   int repeat_count = LOWORD(lparam);
   int flags = HIWORD(lparam);
@@ -133,18 +118,6 @@ bool FocusManager::OnKeyDown(HWND window, UINT message, WPARAM wparam,
   }
   return true;
 }
-
-bool FocusManager::OnKeyUp(HWND window, UINT message, WPARAM wparam,
-                           LPARAM lparam) {
-  for (int i = 0; i < static_cast<int>(keystroke_listeners_.size()); ++i) {
-    if (keystroke_listeners_[i]->ProcessKeyStroke(window, message, wparam,
-                                                  lparam)) {
-      return false;
-    }
-  }
-
-  return true;
-}
 #endif
 
 void FocusManager::ValidateFocusedView() {
@@ -464,24 +437,6 @@ void FocusManager::ViewRemoved(View* parent, View* removed) {
     ClearFocus();
 }
 
-void FocusManager::AddKeystrokeListener(KeystrokeListener* listener) {
-  DCHECK(std::find(keystroke_listeners_.begin(), keystroke_listeners_.end(),
-                   listener) == keystroke_listeners_.end())
-                       << "Adding a listener twice.";
-  keystroke_listeners_.push_back(listener);
-}
-
-void FocusManager::RemoveKeystrokeListener(KeystrokeListener* listener) {
-  KeystrokeListenerList::iterator place =
-      std::find(keystroke_listeners_.begin(), keystroke_listeners_.end(),
-                listener);
-  if (place == keystroke_listeners_.end()) {
-    NOTREACHED() << "Removing a listener that isn't registered.";
-    return;
-  }
-  keystroke_listeners_.erase(place);
-}
-
 void FocusManager::AddFocusChangeListener(FocusChangeListener* listener) {
   DCHECK(std::find(focus_change_listeners_.begin(),
                    focus_change_listeners_.end(), listener) ==
diff --git a/views/focus/focus_manager.h b/views/focus/focus_manager.h
index 8b4c8c9a..b1c87b6 100644
--- a/views/focus/focus_manager.h
+++ b/views/focus/focus_manager.h
@@ -128,19 +128,6 @@ class FocusTraversable {
   virtual View* GetFocusTraversableParentView() = 0;
 };
 
-// The KeystrokeListener interface is used by components (such as the
-// ExternalTabContainer class) which need a crack at handling all
-// keystrokes.
-class KeystrokeListener {
- public:
-  // If this returns true, then the component handled the keystroke and ate
-  // it.
-#if defined(OS_WIN)
-  virtual bool ProcessKeyStroke(HWND window, UINT message, WPARAM wparam,
-                                LPARAM lparam) = 0;
-#endif
-};
-
 // This interface should be implemented by classes that want to be notified when
 // the focus is about to change.  See the Add/RemoveFocusChangeListener methods.
 class FocusChangeListener {
@@ -159,10 +146,6 @@ class FocusManager {
                  UINT message,
                  WPARAM wparam,
                  LPARAM lparam);
-  bool OnKeyUp(HWND window,
-               UINT message,
-               WPARAM wparam,
-               LPARAM lparam);
 #endif
 
   // Returns true is the specified is part of the hierarchy of the window
@@ -224,9 +207,6 @@ class FocusManager {
   // that this FocusManager is attached to the parent Widget of.
   void ViewRemoved(View* parent, View* removed);
 
-  void AddKeystrokeListener(KeystrokeListener* listener);
-  void RemoveKeystrokeListener(KeystrokeListener* listener);
-
   // Adds/removes a listener.  The FocusChangeListener is notified every time
   // the focused view is about to change.
   void AddFocusChangeListener(FocusChangeListener* listener);
@@ -280,10 +260,6 @@ class FocusManager {
   typedef std::map<Accelerator, AcceleratorTargetList> AcceleratorMap;
   AcceleratorMap accelerators_;
 
-  // The list of registered keystroke listeners
-  typedef std::vector<KeystrokeListener*> KeystrokeListenerList;
-  KeystrokeListenerList keystroke_listeners_;
-
   // The list of registered FocusChange listeners.
   typedef std::vector<FocusChangeListener*> FocusChangeListenerList;
   FocusChangeListenerList focus_change_listeners_;
diff --git a/views/widget/accelerator_handler.cc b/views/widget/accelerator_handler.cc
index c171b50..7d1f991 100644
--- a/views/widget/accelerator_handler.cc
+++ b/views/widget/accelerator_handler.cc
@@ -26,12 +26,6 @@ bool AcceleratorHandler::Dispatch(const MSG& msg) {
           process_message = focus_manager->OnKeyDown(msg.hwnd, msg.message,
               msg.wParam, msg.lParam);
           break;
-
-        case WM_KEYUP:
-        case WM_SYSKEYUP:
-          process_message = focus_manager->OnKeyUp(msg.hwnd, msg.message,
-              msg.wParam, msg.lParam);
-          break;
       }
     }
   }