Give up focus if the focused view becomes unfocusable

ui/views/view_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <map>
 
 #include "base/memory/scoped_ptr.h"
 #include "base/rand_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
@@ skipping 2624 matching lines @@
   // Check that calling |AddChildView()| does not change the order.
   v1.AddChildView(&v2);
   EXPECT_EQ(0, v1.GetIndexOf(&v2));
   EXPECT_EQ(1, v1.GetIndexOf(&v3));
   v1.AddChildView(&v3);
   EXPECT_EQ(0, v1.GetIndexOf(&v2));
   EXPECT_EQ(1, v1.GetIndexOf(&v3));
 }
 
 ////////////////////////////////////////////////////////////////////////////////
+// FocusManager
+////////////////////////////////////////////////////////////////////////////////
+
+TEST_F(ViewTest, ReviseFocusedViewForUnfocusableView) {
+  // Create a widget with two views and give the first one focus.
+  Widget widget;
+  Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP);
+  params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  widget.Init(params);
+  widget.Activate();

[sky, 2014/08/07 21:40:34]

I suspect doing this is going to require the test be in interactive ui tests.
Maybe you could create a test with a fake NativeWidgetDelegate? I believe there
are tests that create a fake NativeWidgetDelegate already.

[mohsen, 2014/08/08 01:32:07]

This test is not meant to test activation. It is only testing if focus is
advanced properly inside a widget. So, it is not really needed to be an
interactive ui test. The only activation requirement for this test is that
widget is always active. For that, I've subclasses a new widget class that
always returns true in its IsActive() function. Please check if it's fine.

[sky, 2014/08/08 14:52:30]

You're new code triggers calling off to NativeWidgetDelegate::IsActive() below.
I'm worried about the effects of this with other tests running at the same time.
I think it best to mock out NativeWidgetDelegate.

[mohsen, 2014/08/08 17:45:07]

I guess by NativeWidgetDelegate you mean NativeWidgetPrivate. Right?

If yes, its mock implementation needs a lot of pure virtual function overrides
and yet I'm not sure if that would work as some of them might need proper
implementation (not just an empty implementation). The problem as I understand
is that we need Widget::IsActive() return true for the purpose of this test.
Isn't my above ActiveWidget enough for that matter? (Or a similar subclass of
NativeWidgetAura that overrides IsActive()?)

By the way, I believe this test should work properly even without providing any
mock IsActive() implementation. Normally, in views tests, NativeWidgetAura would
be used as the native widget implementation for widgets (rather than
DesktopNativeWidgetAura, even on desktop aura) and as far as I understand
system-level activation and parallel tests will not interfere with
NativeWidgetAura's notion of activation. Is that right?

+
+  View* view1 = new View();
+  view1->SetFocusable(true);
+  widget.GetRootView()->AddChildView(view1);
+  View* view2 = new View();
+  view2->SetFocusable(true);
+  widget.GetRootView()->AddChildView(view2);
+
+  FocusManager* focus_manager = widget.GetFocusManager();
+  ASSERT_TRUE(focus_manager);
+
+  focus_manager->SetFocusedView(view1);
+  EXPECT_EQ(view1, focus_manager->GetFocusedView());
+
+  // Disable the focused view and check if the next view gets focused.
+  view1->SetEnabled(false);
+  EXPECT_EQ(view2, focus_manager->GetFocusedView());
+
+  // Re-enable and re-focus.
+  view1->SetEnabled(true);
+  focus_manager->SetFocusedView(view1);
+  EXPECT_EQ(view1, focus_manager->GetFocusedView());
+
+  // Hide the focused view and check it the next view gets focused.
+  view1->SetVisible(false);
+  EXPECT_EQ(view2, focus_manager->GetFocusedView());
+
+  // Re-show and re-focus.
+  view1->SetVisible(true);
+  focus_manager->SetFocusedView(view1);
+  EXPECT_EQ(view1, focus_manager->GetFocusedView());
+
+  // Set the focused view as not focusable and check if the next view gets
+  // focused.
+  view1->SetFocusable(false);
+  EXPECT_EQ(view2, focus_manager->GetFocusedView());
+}
+
+////////////////////////////////////////////////////////////////////////////////
 // Layers
 ////////////////////////////////////////////////////////////////////////////////
 
 namespace {
 
 // Test implementation of LayerAnimator.
 class TestLayerAnimator : public ui::LayerAnimator {
  public:
   TestLayerAnimator();
 
@@ skipping 980 matching lines @@
   // notification.
   TestView* test_view_child_2 = new TestView();
   test_view->AddChildView(test_view_child_2);
   EXPECT_TRUE(test_view_child_2->native_theme_);
   EXPECT_EQ(widget->GetNativeTheme(), test_view_child_2->native_theme_);
 
   widget->CloseNow();
 }
 
 }  // namespace views