See original review at

http://codereview.chromium.org/251001/show

This CL hooks the focus traversal for the TabbedPaneView on Linux with toolkit views. 
It also has few focus related fixed (focus with buttons, clearing focus effectively in the focus manager). 

BUG=None 
TEST=Run the view_examples app. Press tab to move the focus. Try the different tabs.

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

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

10 files changed, 100 insertions, 8 deletions

diff --git a/views/controls/button/button.cc b/views/controls/button/button.cc
index 504a2f5..460c424 100644
--- a/views/controls/button/button.cc
+++ b/views/controls/button/button.cc
@@ -64,6 +64,7 @@ Button::Button(ButtonListener* listener)
     : listener_(listener),
       tag_(-1),
       mouse_event_flags_(0) {
+  SetFocusable(true);
 }
 
 void Button::NotifyClick(const views::Event& event) {
diff --git a/views/controls/button/native_button.cc b/views/controls/button/native_button.cc
index 811fb56..76fb0ff 100644
--- a/views/controls/button/native_button.cc
+++ b/views/controls/button/native_button.cc
@@ -36,7 +36,6 @@ NativeButton::NativeButton(ButtonListener* listener)
       is_default_(false),
       ignore_minimum_size_(false) {
   InitBorder();
-  SetFocusable(true);
 }
 
 NativeButton::NativeButton(ButtonListener* listener, const std::wstring& label)
@@ -46,7 +45,6 @@ NativeButton::NativeButton(ButtonListener* listener, const std::wstring& label)
       ignore_minimum_size_(false) {
   SetLabel(label);  // SetLabel takes care of label layout in RTL UI.
   InitBorder();
-  SetFocusable(true);
 }
 
 NativeButton::~NativeButton() {
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/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/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 81c214d..fb50103 100644
--- a/views/widget/widget_gtk.cc
+++ b/views/widget/widget_gtk.cc
@@ -490,6 +490,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) {
@@ -543,6 +580,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/window/custom_frame_view.cc b/views/window/custom_frame_view.cc
index d3fd085..689b87b 100644
--- a/views/window/custom_frame_view.cc
+++ b/views/window/custom_frame_view.cc
@@ -86,6 +86,13 @@ CustomFrameView::CustomFrameView(Window* frame)
 
   ResourceBundle& rb = ResourceBundle::GetSharedInstance();
 
+  // The frame's buttons should not be focusable.
+  close_button_->SetFocusable(false);
+  restore_button_->SetFocusable(false);
+  maximize_button_->SetFocusable(false);
+  minimize_button_->SetFocusable(false);
+  system_menu_button_->SetFocusable(false);
+
   // Close button images will be set in LayoutWindowControls().
   AddChildView(close_button_);