Fixing activation state problem with the launcher acessiblity pane

BUG=165089
TEST=unittest


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=183728

Review URL: https://chromiumcodereview.appspot.com/12315021

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

10 files changed, 95 insertions, 17 deletions

diff --git a/ash/focus_cycler.cc b/ash/focus_cycler.cc
index 801cd35..80ef432 100644
--- a/ash/focus_cycler.cc
+++ b/ash/focus_cycler.cc
@@ -76,16 +76,12 @@ void FocusCycler::RotateFocus(Direction direction) {
 }
 
 bool FocusCycler::FocusWidget(views::Widget* widget) {
-  views::AccessiblePaneView* view =
-      static_cast<views::AccessiblePaneView*>(widget->GetContentsView());
-  if (view->SetPaneFocusAndFocusDefault()) {
-    widget_activating_ = widget;
-    widget->Activate();
-    widget_activating_ = NULL;
-    if (widget->IsActive())
-      return true;
-  }
-  return false;
+  // Note: It is not necessary to set the focus directly to the pane since that
+  // will be taken care of by the widget activation.
+  widget_activating_ = widget;
+  widget->Activate();
+  widget_activating_ = NULL;
+  return widget->IsActive();
 }
 
 }  // namespace internal
diff --git a/ash/launcher/launcher.cc b/ash/launcher/launcher.cc
index 3d3efd4..b794bac 100644
--- a/ash/launcher/launcher.cc
+++ b/ash/launcher/launcher.cc
@@ -158,6 +158,9 @@ Launcher::DelegateView::DelegateView(Launcher* launcher)
     : launcher_(launcher),
       focus_cycler_(NULL),
       alpha_(0) {
+  // Allow the launcher to surrender the focus to another window upon
+  // navigation completion by the user.
+  set_allow_deactivate_on_esc(true);
 }
 
 Launcher::DelegateView::~DelegateView() {
diff --git a/ash/system/status_area_widget_delegate.cc b/ash/system/status_area_widget_delegate.cc
index 662f943..99eeb20 100644
--- a/ash/system/status_area_widget_delegate.cc
+++ b/ash/system/status_area_widget_delegate.cc
@@ -24,6 +24,9 @@ namespace internal {
 StatusAreaWidgetDelegate::StatusAreaWidgetDelegate()
     : focus_cycler_for_testing_(NULL),
       alignment_(SHELF_ALIGNMENT_BOTTOM) {
+  // Allow the launcher to surrender the focus to another window upon
+  // navigation completion by the user.
+  set_allow_deactivate_on_esc(true);
 }
 
 StatusAreaWidgetDelegate::~StatusAreaWidgetDelegate() {
diff --git a/ui/aura/test/test_activation_client.cc b/ui/aura/test/test_activation_client.cc
index 2d0e0dd..14f6b30 100644
--- a/ui/aura/test/test_activation_client.cc
+++ b/ui/aura/test/test_activation_client.cc
@@ -15,7 +15,8 @@ namespace test {
 ////////////////////////////////////////////////////////////////////////////////
 // TestActivationClient, public:
 
-TestActivationClient::TestActivationClient(RootWindow* root_window) {
+TestActivationClient::TestActivationClient(RootWindow* root_window)
+    : last_active_(NULL) {
   client::SetActivationClient(root_window, this);
 }
 
@@ -43,6 +44,7 @@ void TestActivationClient::ActivateWindow(Window* window) {
   if (last_active == window)
     return;
 
+  last_active_ = last_active;
   RemoveActiveWindow(window);
   active_windows_.push_back(window);
   window->parent()->StackChildAtTop(window);
@@ -66,6 +68,8 @@ void TestActivationClient::DeactivateWindow(Window* window) {
       aura::client::GetActivationChangeObserver(window);
   if (observer)
     observer->OnWindowActivated(NULL, window);
+  if (last_active_)
+    ActivateWindow(last_active_);
 }
 
 Window* TestActivationClient::GetActiveWindow() {
@@ -95,6 +99,9 @@ bool TestActivationClient::CanActivateWindow(Window* window) const {
 // TestActivationClient, WindowObserver implementation:
 
 void TestActivationClient::OnWindowDestroyed(Window* window) {
+  if (window == last_active_)
+    last_active_ = NULL;
+
   if (window == GetActiveWindow()) {
     window->RemoveObserver(this);
     active_windows_.pop_back();
diff --git a/ui/aura/test/test_activation_client.h b/ui/aura/test/test_activation_client.h
index b590862..e5eb397 100644
--- a/ui/aura/test/test_activation_client.h
+++ b/ui/aura/test/test_activation_client.h
@@ -49,6 +49,9 @@ class TestActivationClient : public client::ActivationClient,
   // fail.
   std::vector<Window*> active_windows_;
 
+  // The window which was active before the currently active one.
+  Window* last_active_;
+
   ObserverList<client::ActivationChangeObserver> observers_;
 
   DISALLOW_COPY_AND_ASSIGN(TestActivationClient);
diff --git a/ui/views/accessible_pane_view.cc b/ui/views/accessible_pane_view.cc
index 27eecdb..5ec5f43 100644
--- a/ui/views/accessible_pane_view.cc
+++ b/ui/views/accessible_pane_view.cc
@@ -42,6 +42,7 @@ class AccessiblePaneViewFocusSearch : public FocusSearch {
 
 AccessiblePaneView::AccessiblePaneView()
     : pane_has_focus_(false),
+      allow_deactivate_on_esc_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)),
       focus_manager_(NULL),
       home_key_(ui::VKEY_HOME, ui::EF_NONE),
@@ -156,14 +157,16 @@ views::FocusTraversable* AccessiblePaneView::GetPaneFocusTraversable() {
 bool AccessiblePaneView::AcceleratorPressed(
     const ui::Accelerator& accelerator) {
 
-  const views::View* focused_view = focus_manager_->GetFocusedView();
+  views::View* focused_view = focus_manager_->GetFocusedView();
   if (!ContainsForFocusSearch(this, focused_view))
     return false;
 
   switch (accelerator.key_code()) {
     case ui::VKEY_ESCAPE:
       RemovePaneFocus();
-      focus_manager_->RestoreFocusedView();
+      if (!focus_manager_->RestoreFocusedView() &&
+          allow_deactivate_on_esc_)
+        focused_view->GetWidget()->Deactivate();
       return true;
     case ui::VKEY_LEFT:
       focus_manager_->AdvanceFocus(true);
diff --git a/ui/views/accessible_pane_view.h b/ui/views/accessible_pane_view.h
index f15bbed..982cb04 100644
--- a/ui/views/accessible_pane_view.h
+++ b/ui/views/accessible_pane_view.h
@@ -88,9 +88,20 @@ class VIEWS_EXPORT AccessiblePaneView : public View,
 
   FocusManager* focus_manager() const { return focus_manager_; }
 
+  // When finishing navigation by pressing ESC, it is allowed to surrender the
+  // focus to another window if if |allow| is set and no previous view can be
+  // found.
+  void set_allow_deactivate_on_esc(bool allow) {
+    allow_deactivate_on_esc_ = allow;
+  }
+
  private:
   bool pane_has_focus_;
 
+  // If true, the panel should be de-activated upon escape when no active view
+  // is known where to return to.
+  bool allow_deactivate_on_esc_;
+
   base::WeakPtrFactory<AccessiblePaneView> method_factory_;
 
   // Save the focus manager rather than calling GetFocusManager(),
diff --git a/ui/views/accessible_pane_view_unittest.cc b/ui/views/accessible_pane_view_unittest.cc
index 077d9e0..78bb350 100644
--- a/ui/views/accessible_pane_view_unittest.cc
+++ b/ui/views/accessible_pane_view_unittest.cc
@@ -45,6 +45,7 @@ class TestBarView : public AccessiblePaneView,
 
 TestBarView::TestBarView() {
   Init();
+  set_allow_deactivate_on_esc(true);
 }
 
 TestBarView::~TestBarView() {}
@@ -96,6 +97,54 @@ TEST_F(AccessiblePaneViewTest, SimpleSetPaneFocus) {
   widget.reset();
 }
 
+// This test will not work properly in Windows because it uses ::GetNextWindow
+// on deactivate which is rather unpredictable where the focus will land.
+#if !defined(OS_WIN)||defined(USE_AURA)
+TEST_F(AccessiblePaneViewTest, SetPaneFocusAndRestore) {
+  View* test_view_main = new View();
+  scoped_ptr<Widget> widget_main(new Widget());
+  Widget::InitParams params_main = CreateParams(Widget::InitParams::TYPE_POPUP);
+  params_main.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  params_main.bounds = gfx::Rect(0, 0, 20, 20);
+  widget_main->Init(params_main);
+  View* root_main = widget_main->GetRootView();
+  root_main->AddChildView(test_view_main);
+  widget_main->Activate();
+  test_view_main->GetFocusManager()->SetFocusedView(test_view_main);
+  EXPECT_TRUE(widget_main->IsActive());
+  EXPECT_TRUE(test_view_main->HasFocus());
+
+  TestBarView* test_view_bar = new TestBarView();
+  scoped_ptr<Widget> widget_bar(new Widget());
+  Widget::InitParams params_bar = CreateParams(Widget::InitParams::TYPE_POPUP);
+  params_bar.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  params_bar.bounds = gfx::Rect(50, 50, 650, 650);
+  widget_bar->Init(params_bar);
+  View* root_bar = widget_bar->GetRootView();
+  root_bar->AddChildView(test_view_bar);
+  widget_bar->Show();
+  widget_bar->Activate();
+
+  // Set pane focus succeeds, focus on child.
+  EXPECT_TRUE(test_view_bar->SetPaneFocusAndFocusDefault());
+  EXPECT_FALSE(test_view_main->HasFocus());
+  EXPECT_FALSE(widget_main->IsActive());
+  EXPECT_EQ(test_view_bar, test_view_bar->GetPaneFocusTraversable());
+  EXPECT_EQ(test_view_bar->child_button(),
+            test_view_bar->GetWidget()->GetFocusManager()->GetFocusedView());
+
+  test_view_bar->AcceleratorPressed(test_view_bar->escape_key());
+  EXPECT_TRUE(widget_main->IsActive());
+  EXPECT_FALSE(widget_bar->IsActive());
+
+  widget_bar->CloseNow();
+  widget_bar.reset();
+
+  widget_main->CloseNow();
+  widget_main.reset();
+}
+#endif  // !defined(OS_WIN)||defined(USE_AURA)
+
 TEST_F(AccessiblePaneViewTest, TwoSetPaneFocus) {
   TestBarView* test_view = new TestBarView();
   TestBarView* test_view_2 = new TestBarView();
diff --git a/ui/views/focus/focus_manager.cc b/ui/views/focus/focus_manager.cc
index 9491cda..24baff1 100644
--- a/ui/views/focus/focus_manager.cc
+++ b/ui/views/focus/focus_manager.cc
@@ -310,12 +310,12 @@ void FocusManager::StoreFocusedView(bool clear_native_focus) {
     v->SchedulePaint();  // Remove focus border.
 }
 
-void FocusManager::RestoreFocusedView() {
+bool FocusManager::RestoreFocusedView() {
   ViewStorage* view_storage = ViewStorage::GetInstance();
   if (!view_storage) {
     // This should never happen but bug 981648 seems to indicate it could.
     NOTREACHED();
-    return;
+    return false;
   }
 
   View* view = view_storage->RetrieveView(stored_focused_view_storage_id_);
@@ -336,7 +336,9 @@ void FocusManager::RestoreFocusedView() {
           focus_change_reason_ = kReasonFocusRestore;
       }
     }
+    return true;
   }
+  return false;
 }
 
 void FocusManager::ClearStoredFocusedView() {
diff --git a/ui/views/focus/focus_manager.h b/ui/views/focus/focus_manager.h
index 9d252ac1..7686cf6 100644
--- a/ui/views/focus/focus_manager.h
+++ b/ui/views/focus/focus_manager.h
@@ -182,8 +182,9 @@ class VIEWS_EXPORT FocusManager {
   void StoreFocusedView(bool clear_native_focus);
 
   // Restore the view saved with a previous call to StoreFocusedView(). Used
-  // when the widget becomes active.
-  void RestoreFocusedView();
+  // when the widget becomes active. Returns true when the previous view was
+  // successfully refocused - otherwise false.
+  bool RestoreFocusedView();
 
   // Clears the stored focused view.
   void ClearStoredFocusedView();