Keep focus inside BubbleDialogDelegateView

Removes Widget::SetFocusTraversableParent* calls since we don't want
focus to escape bubble dialogs once they get here. There are several
bubbles that close once focus escapes.

We still set kAnchoredDialogKey of the anchor view to make sure that
focus can move into attached dialogs (this makes them keyboard
accessible).

Bug: chromium:899601, chromium:899996
Change-Id: Idc5d60c2f41ae20a343324ae75b0fd304f273c94
Reviewed-on: https://chromium-review.googlesource.com/c/1311090Reviewed-by: Trent Apted <tapted@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604464}

ui/views/bubble/bubble_dialog_delegate_view.cc

@@ -164,12 +164,6 @@ const char* BubbleDialogDelegateView::GetClassName() const {
   return kViewClassName;
 }
 
-void BubbleDialogDelegateView::AddedToWidget() {
-  DialogDelegateView::AddedToWidget();
-  if (GetAnchorView())
-    EnableFocusTraversalFromAnchorView();
-}
-
 void BubbleDialogDelegateView::OnWidgetDestroying(Widget* widget) {
   if (anchor_widget() == widget)
     SetAnchorView(NULL);
@@ -271,17 +265,6 @@ void BubbleDialogDelegateView::OnAnchorBoundsChanged() {
   SizeToContents();
 }
 
-void BubbleDialogDelegateView::EnableFocusTraversalFromAnchorView() {
-  DCHECK(GetWidget());
-  DCHECK(GetAnchorView());
-  DCHECK(anchor_widget());
-  GetWidget()->SetFocusTraversableParent(
-      anchor_widget()->GetFocusTraversable());
-  GetWidget()->SetFocusTraversableParentView(GetAnchorView());
-  GetAnchorView()->SetProperty(kAnchoredDialogKey,
-                               static_cast<BubbleDialogDelegateView*>(this));
-}
-
 BubbleDialogDelegateView::BubbleDialogDelegateView()
     : BubbleDialogDelegateView(nullptr, BubbleBorder::TOP_LEFT) {}
 
@@ -349,13 +332,8 @@ void BubbleDialogDelegateView::OnNativeThemeChanged(
 void BubbleDialogDelegateView::Init() {}
 
 void BubbleDialogDelegateView::SetAnchorView(View* anchor_view) {
-  if (GetAnchorView()) {
-    if (GetWidget()) {
-      GetWidget()->SetFocusTraversableParent(nullptr);
-      GetWidget()->SetFocusTraversableParentView(nullptr);
-    }
+  if (GetAnchorView())
     GetAnchorView()->ClearProperty(kAnchoredDialogKey);
-  }
 
   // When the anchor view gets set the associated anchor widget might
   // change as well.
@@ -388,10 +366,12 @@ void BubbleDialogDelegateView::SetAnchorView(View* anchor_view) {
     // point. (It's safe to skip this, since if we were to update the
     // bounds when |anchor_view| is NULL, the bubble won't move.)
     OnAnchorBoundsChanged();
+  }
 
-    // Make sure that focus can move into here from the anchor view. If there's
-    // no widget yet, focus traversal will be set up in ::AddedToWidget().
-    EnableFocusTraversalFromAnchorView();
+  if (anchor_view) {
+    // Make sure that focus can move into here from the anchor view (but not
+    // out, focus will cycle inside the dialog once it gets here).
+    anchor_view->SetProperty(kAnchoredDialogKey, this);
   }
 }
 

ui/views/bubble/bubble_dialog_delegate_view.h

@@ -49,7 +49,6 @@ class VIEWS_EXPORT BubbleDialogDelegateView : public DialogDelegateView,
   static Widget* CreateBubble(BubbleDialogDelegateView* bubble_delegate);
 
   // DialogDelegateView:
-  void AddedToWidget() override;
   BubbleDialogDelegateView* AsBubbleDialogDelegate() override;
   bool ShouldShowCloseButton() const override;
   ClientView* CreateClientView(Widget* widget) override;
@@ -122,11 +121,6 @@ class VIEWS_EXPORT BubbleDialogDelegateView : public DialogDelegateView,
   // bounds change as a result of the widget's bounds changing.
   void OnAnchorBoundsChanged();
 
-  // If this is called, enables focus to traverse from the anchor view
-  // to inside this dialog and back out. This may become the default in
-  // the future.
-  void EnableFocusTraversalFromAnchorView();
-
  protected:
   BubbleDialogDelegateView();
   // |shadow| usually doesn't need to be explicitly set, just uses the default

ui/views/focus/focus_manager_unittest.cc

@@ -942,7 +942,10 @@ TEST_F(FocusManagerTest, NavigateIntoAnchoredDialog) {
       new TestBubbleDialogDelegateView(parent3);
   test::WidgetTest::WidgetAutoclosePtr bubble_widget(
       BubbleDialogDelegateView::CreateBubble(bubble_delegate));
-  bubble_delegate->EnableFocusTraversalFromAnchorView();
+  bubble_widget->SetFocusTraversableParent(
+      bubble_delegate->anchor_widget()->GetFocusTraversable());
+
+  bubble_widget->SetFocusTraversableParentView(parent3);
   View* child1 = new View();
   View* child2 = new View();
   child1->SetFocusBehavior(View::FocusBehavior::ALWAYS);
@@ -1003,7 +1006,9 @@ TEST_F(FocusManagerTest, AnchoredDialogOnContainerView) {
       new TestBubbleDialogDelegateView(parent_group);
   test::WidgetTest::WidgetAutoclosePtr bubble_widget(
       BubbleDialogDelegateView::CreateBubble(bubble_delegate));
-  bubble_delegate->EnableFocusTraversalFromAnchorView();
+  bubble_widget->SetFocusTraversableParent(
+      bubble_delegate->anchor_widget()->GetFocusTraversable());
+  bubble_widget->SetFocusTraversableParentView(parent_group);
   View* child1 = new View();
   View* child2 = new View();
   child1->SetFocusBehavior(View::FocusBehavior::ALWAYS);
@@ -1067,7 +1072,9 @@ TEST_F(FocusManagerTest, AnchoredDialogInDesktopNativeWidgetAura) {
   bubble_delegate->UseNativeWidgetAura();
   test::WidgetTest::WidgetAutoclosePtr bubble_widget(
       BubbleDialogDelegateView::CreateBubble(bubble_delegate));
-  bubble_delegate->EnableFocusTraversalFromAnchorView();
+  bubble_widget->SetFocusTraversableParent(
+      bubble_delegate->anchor_widget()->GetFocusTraversable());
+  bubble_widget->SetFocusTraversableParentView(parent2);
   View* child = new View();
   child->SetFocusBehavior(View::FocusBehavior::ALWAYS);
   bubble_widget->GetRootView()->AddChildView(child);