Relanding focus traversal on Linux toolkit views.

It was previously breaking the interactive UI tests.

That was because I changed the button base class to be focusable by default which led to many button beings added to the tab traversal order.
I now explicitly make these buttons non focusable.

Also because of the way we clear the native focus on Gtk (by setting focus to NULL on the top level window), views now default to clearing the focus when focused (so that views non backed by a native window don't leave the native focus on the previously native window).
On Windows we used to focus the WidgetWin's HWND, and now we focus the top-level HWND. The WidgetWin class had to be changed to forward the key events to the root view that contains the focused view because of that. (otherwise it would send it to the top-level root-view, and if the focused view was in a child root-view it would not recieve the keyboard messages).
Note that the mouse wheel does not need that, as the mouse-wheel Windows messages are sent to the HWND under the cursor.

See:
http://codereview.chromium.org/246030/show

TEST=Run the tests
BUG=NONE

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

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

13 files changed, 134 insertions, 9 deletions

diff --git a/views/controls/button/custom_button.h b/views/controls/button/custom_button.h
index bd43d4d..f5b5cbe 100644
--- a/views/controls/button/custom_button.h
+++ b/views/controls/button/custom_button.h
@@ -14,6 +14,9 @@ namespace views {
 
 // A button with custom rendering. The common base class of ImageButton and
 // TextButton.
+// Note that this type of button is not focusable by default and will not be
+// part of the focus chain.  Call SetFocusable(true) to make it part of the
+// focus chain.
 class CustomButton : public Button,
                      public AnimationDelegate {
  public:
diff --git a/views/controls/button/image_button.h b/views/controls/button/image_button.h
index 716a67e..654c4a0 100644
--- a/views/controls/button/image_button.h
+++ b/views/controls/button/image_button.h
@@ -11,6 +11,11 @@
 namespace views {
 
 // An image button.
+
+// Note that this type of button is not focusable by default and will not be
+// part of the focus chain.  Call SetFocusable(true) to make it part of the
+// focus chain.
+
 class ImageButton : public CustomButton {
  public:
   explicit ImageButton(ButtonListener* listener);
diff --git a/views/controls/button/text_button.cc b/views/controls/button/text_button.cc
index b5733c0..6bb3bec 100644
--- a/views/controls/button/text_button.cc
+++ b/views/controls/button/text_button.cc
@@ -330,6 +330,7 @@ void TextButton::SetEnabled(bool enabled) {
 }
 
 bool TextButton::OnMousePressed(const MouseEvent& e) {
+  RequestFocus();
   return true;
 }
 
diff --git a/views/controls/button/text_button.h b/views/controls/button/text_button.h
index 3fbc2b4..bd2fcb69 100644
--- a/views/controls/button/text_button.h
+++ b/views/controls/button/text_button.h
@@ -20,6 +20,10 @@ namespace views {
 //  A Border subclass that paints a TextButton's background layer -
 //  basically the button frame in the hot/pushed states.
 //
+// Note that this type of button is not focusable by default and will not be
+// part of the focus chain.  Call SetFocusable(true) to make it part of the
+// focus chain.
+//
 ////////////////////////////////////////////////////////////////////////////////
 class TextButtonBorder : public Border {
  public:
diff --git a/views/controls/tabbed_pane/native_tabbed_pane_gtk.cc b/views/controls/tabbed_pane/native_tabbed_pane_gtk.cc
index 6516c42..96e26df 100644
--- a/views/controls/tabbed_pane/native_tabbed_pane_gtk.cc
+++ b/views/controls/tabbed_pane/native_tabbed_pane_gtk.cc
@@ -119,6 +119,13 @@ void NativeTabbedPaneGtk::CreateNativeControl() {
 }
 
 ////////////////////////////////////////////////////////////////////////////////
+// NativeTabbedPaneGtk, View override:
+
+FocusTraversable* NativeTabbedPaneGtk::GetFocusTraversable() {
+  return GetWidgetAt(GetSelectedTabIndex());
+}
+
+////////////////////////////////////////////////////////////////////////////////
 // NativeTabbedPaneGtk, private:
 void NativeTabbedPaneGtk::DoAddTabAtIndex(int index, const std::wstring& title,
                                           View* contents,
@@ -159,11 +166,17 @@ void NativeTabbedPaneGtk::DoAddTabAtIndex(int index, const std::wstring& title,
     gtk_notebook_set_current_page(GTK_NOTEBOOK(native_view()), 0);
 }
 
-View* NativeTabbedPaneGtk::GetTabViewAt(int index) {
+WidgetGtk* NativeTabbedPaneGtk::GetWidgetAt(int index) {
   DCHECK(index <= GetTabCount());
   GtkWidget* page = gtk_notebook_get_nth_page(GTK_NOTEBOOK(native_view()),
                                               index);
   WidgetGtk* widget = WidgetGtk::GetViewForNative(page);
+  DCHECK(widget);
+  return widget;
+}
+
+View* NativeTabbedPaneGtk::GetTabViewAt(int index) {
+  WidgetGtk* widget = GetWidgetAt(index);
   DCHECK(widget && widget->GetRootView()->GetChildViewCount() == 1);
   return widget->GetRootView()->GetChildViewAt(0);
 }
diff --git a/views/controls/tabbed_pane/native_tabbed_pane_gtk.h b/views/controls/tabbed_pane/native_tabbed_pane_gtk.h
index 63a70f7..d586e74 100644
--- a/views/controls/tabbed_pane/native_tabbed_pane_gtk.h
+++ b/views/controls/tabbed_pane/native_tabbed_pane_gtk.h
@@ -36,11 +36,18 @@ class NativeTabbedPaneGtk : public NativeControlGtk,
   // NativeControlGtk overrides.
   virtual void CreateNativeControl();
 
+  // View override:
+  virtual FocusTraversable* GetFocusTraversable();
+
  private:
   void DoAddTabAtIndex(int index,
                        const std::wstring& title,
                        View* contents,
                        bool select_if_first_tab);
+
+  // Returns the WidgetGtk containing the tab contents at |index|.
+  WidgetGtk* GetWidgetAt(int index);
+
   View* GetTabViewAt(int index);
   void OnSwitchPage(int selected_tab_index);
 
diff --git a/views/focus/accelerator_handler_win.cc b/views/focus/accelerator_handler_win.cc
index 8363928..f617c6f 100644
--- a/views/focus/accelerator_handler_win.cc
+++ b/views/focus/accelerator_handler_win.cc
@@ -29,6 +29,9 @@ bool AcceleratorHandler::Dispatch(const MSG& msg) {
                          msg.lParam & 0xFFFF,
                          (msg.lParam & 0xFFFF0000) >> 16);
           process_message = focus_manager->OnKeyEvent(event);
+          // TODO(jcampan): http://crbug.com/23383 We should not translate and
+          //                dispatch the associated WM_KEYUP if process_message
+          //                is true.
           break;
         }
       }
diff --git a/views/focus/focus_manager_gtk.cc b/views/focus/focus_manager_gtk.cc
index 93b9cb7..3038d93 100644
--- a/views/focus/focus_manager_gtk.cc
+++ b/views/focus/focus_manager_gtk.cc
@@ -8,11 +8,20 @@
 
 #include "base/logging.h"
 #include "views/widget/widget_gtk.h"
+#include "views/window/window_gtk.h"
 
 namespace views {
 
 void FocusManager::ClearNativeFocus() {
-  gtk_widget_grab_focus(widget_->GetNativeView());
+  GtkWidget* gtk_widget = widget_->GetNativeView();
+  if (!gtk_widget) {
+    NOTREACHED();
+    return;
+  }
+
+  // Since only top-level WidgetGtk have a focus manager, the native view is
+  // expected to be a GtkWindow.
+  gtk_window_set_focus(GTK_WINDOW(gtk_widget), NULL);
 }
 
 void FocusManager::FocusNativeView(gfx::NativeView native_view) {
diff --git a/views/view.cc b/views/view.cc
index c5db5db..3c5d77b 100644
--- a/views/view.cc
+++ b/views/view.cc
@@ -277,11 +277,11 @@ bool View::HasFocus() {
 }
 
 void View::Focus() {
-  // Set the native focus to the root view window so it receives the keyboard
-  // messages.
+  // By default, we clear the native focus. This ensures that no visible native
+  // view as the focus and that we still receive keyboard inputs.
   FocusManager* focus_manager = GetFocusManager();
   if (focus_manager)
-    focus_manager->FocusNativeView(GetRootView()->GetWidget()->GetNativeView());
+    focus_manager->ClearNativeFocus();
 }
 
 void View::SetHotTracked(bool flag) {
diff --git a/views/widget/widget_gtk.cc b/views/widget/widget_gtk.cc
index 1453250..8fcac89 100644
--- a/views/widget/widget_gtk.cc
+++ b/views/widget/widget_gtk.cc
@@ -492,6 +492,43 @@ void WidgetGtk::DidProcessEvent(GdkEvent* event) {
 }
 
 ////////////////////////////////////////////////////////////////////////////////
+// FocusTraversable
+
+View* WidgetGtk::FindNextFocusableView(
+    View* starting_view, bool reverse, Direction direction,
+    bool check_starting_view, FocusTraversable** focus_traversable,
+    View** focus_traversable_view) {
+  return root_view_->FindNextFocusableView(starting_view,
+                                           reverse,
+                                           direction,
+                                           check_starting_view,
+                                           focus_traversable,
+                                           focus_traversable_view);
+}
+
+FocusTraversable* WidgetGtk::GetFocusTraversableParent() {
+  // We are a proxy to the root view, so we should be bypassed when traversing
+  // up and as a result this should not be called.
+  NOTREACHED();
+  return NULL;
+}
+
+void WidgetGtk::SetFocusTraversableParent(FocusTraversable* parent) {
+  root_view_->SetFocusTraversableParent(parent);
+}
+
+View* WidgetGtk::GetFocusTraversableParentView() {
+  // We are a proxy to the root view, so we should be bypassed when traversing
+  // up and as a result this should not be called.
+  NOTREACHED();
+  return NULL;
+}
+
+void WidgetGtk::SetFocusTraversableParentView(View* parent_view) {
+  root_view_->SetFocusTraversableParentView(parent_view);
+}
+
+////////////////////////////////////////////////////////////////////////////////
 // TODO(beng): organize into sections:
 
 void WidgetGtk::CreateGtkWidget(GtkWidget* parent, const gfx::Rect& bounds) {
@@ -545,6 +582,7 @@ void WidgetGtk::CreateGtkWidget(GtkWidget* parent, const gfx::Rect& bounds) {
     if (transparent_)
       ConfigureWidgetForTransparentBackground();
   }
+
   // The widget needs to be realized before handlers like size-allocate can
   // function properly.
   gtk_widget_realize(widget_);
diff --git a/views/widget/widget_gtk.h b/views/widget/widget_gtk.h
index 7ce3329..080c1a0 100644
--- a/views/widget/widget_gtk.h
+++ b/views/widget/widget_gtk.h
@@ -8,6 +8,7 @@
 #include <gtk/gtk.h>
 
 #include "base/message_loop.h"
+#include "views/focus/focus_manager.h"
 #include "views/widget/widget.h"
 
 class OSExchangeData;
@@ -26,7 +27,10 @@ class View;
 class WindowGtk;
 
 // Widget implementation for GTK.
-class WidgetGtk : public Widget, public MessageLoopForUI::Observer {
+class WidgetGtk
+    : public Widget,
+      public MessageLoopForUI::Observer,
+      public FocusTraversable {
  public:
   // Type of widget.
   enum Type {
@@ -112,6 +116,20 @@ class WidgetGtk : public Widget, public MessageLoopForUI::Observer {
   virtual void WillProcessEvent(GdkEvent* event);
   virtual void DidProcessEvent(GdkEvent* event);
 
+  // FocusTraversable implementation:
+  virtual View* FindNextFocusableView(View* starting_view,
+                                      bool reverse,
+                                      Direction direction,
+                                      bool check_starting_view,
+                                      FocusTraversable** focus_traversable,
+                                      View** focus_traversable_view);
+  virtual FocusTraversable* GetFocusTraversableParent();
+  virtual View* GetFocusTraversableParentView();
+
+  // Sets the focus traversable parents.
+  void SetFocusTraversableParent(FocusTraversable* parent);
+  void SetFocusTraversableParentView(View* parent_view);
+
   // Retrieves the WidgetGtk associated with |widget|.
   static WidgetGtk* GetViewForNative(GtkWidget* widget);
 
diff --git a/views/widget/widget_win.cc b/views/widget/widget_win.cc
index 3017e05..d272e1f 100644
--- a/views/widget/widget_win.cc
+++ b/views/widget/widget_win.cc
@@ -573,13 +573,21 @@ void WidgetWin::OnInitMenuPopup(HMENU menu,
 void WidgetWin::OnKeyDown(TCHAR c, UINT rep_cnt, UINT flags) {
   KeyEvent event(Event::ET_KEY_PRESSED,
                  win_util::WinToKeyboardCode(c), rep_cnt, flags);
-  SetMsgHandled(root_view_->ProcessKeyEvent(event));
+  RootView* root_view = GetFocusedViewRootView();
+  if (!root_view)
+    root_view = root_view_.get();
+
+  SetMsgHandled(root_view->ProcessKeyEvent(event));
 }
 
 void WidgetWin::OnKeyUp(TCHAR c, UINT rep_cnt, UINT flags) {
   KeyEvent event(Event::ET_KEY_RELEASED,
                  win_util::WinToKeyboardCode(c), rep_cnt, flags);
-  SetMsgHandled(root_view_->ProcessKeyEvent(event));
+  RootView* root_view = GetFocusedViewRootView();
+  if (!root_view)
+    root_view = root_view_.get();
+
+  SetMsgHandled(root_view->ProcessKeyEvent(event));
 }
 
 // TODO(pkasting): ORing the pressed/released button into the flags is _wrong_.
@@ -1028,6 +1036,18 @@ void WidgetWin::UpdateWindowFromContents(HDC dib_dc) {
   }
 }
 
+RootView* WidgetWin::GetFocusedViewRootView() {
+  FocusManager* focus_manager = GetFocusManager();
+  if (!focus_manager) {
+    NOTREACHED();
+    return NULL;
+  }
+  View* focused_view = focus_manager->GetFocusedView();
+  if (!focused_view)
+    return NULL;
+  return focused_view->GetRootView();
+}
+
 // Get the source HWND of the specified message. Depending on the message, the
 // source HWND is encoded in either the WPARAM or the LPARAM value.
 HWND GetControlHWNDForMessage(UINT message, WPARAM w_param, LPARAM l_param) {
diff --git a/views/widget/widget_win.h b/views/widget/widget_win.h
index c6f0359..0615c14e 100644
--- a/views/widget/widget_win.h
+++ b/views/widget/widget_win.h
@@ -467,7 +467,11 @@ class WidgetWin : public base::WindowImpl,
 
   // Invoked from WM_DESTROY. Does appropriate cleanup and invokes OnDestroy
   // so that subclasses can do any cleanup they need to.
-  void OnDestroyImpl();
+  // void OnDestroyImpl();
+
+  // Returns the RootView that contains the focused view, or NULL if there is no
+  // focused view.
+  RootView* GetFocusedViewRootView();
 
   // Called after the WM_ACTIVATE message has been processed by the default
   // windows procedure.