Ensure default dialog button focus remains after a dialog update.

ui/views/window/dialog_client_view.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 "ui/views/window/dialog_client_view.h"
 
 #include <algorithm>
 
 #include "build/build_config.h"
 #include "ui/base/material_design/material_design_controller.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 #include "ui/views/background.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/button/blue_button.h"
 #include "ui/views/controls/button/custom_button.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/button/md_text_button.h"
 #include "ui/views/layout/grid_layout.h"
 #include "ui/views/style/platform_style.h"
+#include "ui/views/view_observer.h"
 #include "ui/views/views_delegate.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/window/dialog_delegate.h"
 
+namespace {

[sky, 2017/05/17 19:08:30]

Move this code to around line 80ish, inside the view namespace so you don't need
view:: everywhere.

[ackermanb, 2017/05/17 22:03:49]

Done.

+
+class ViewDeletionObserver : public views::ViewObserver {

[sky, 2017/05/17 19:08:30]

Add comment.

[ackermanb, 2017/05/17 22:03:50]

Done.

+ public:
+  ViewDeletionObserver(views::View* observed_view)

[sky, 2017/05/17 19:08:30]

explicit

[ackermanb, 2017/05/17 22:03:50]

Done.

+      : observed_view_(observed_view) {
+    if (observed_view_)
+      observed_view_->AddObserver(this);
+  }
+
+  ~ViewDeletionObserver() override {
+    if (observed_view_)
+      observed_view_->RemoveObserver(this);
+  }
+
+  void OnViewIsDeleting(views::View* observed_view) override {

[sky, 2017/05/17 19:08:30]

Prefix with:
// ViewObserver:

[ackermanb, 2017/05/17 22:03:50]

Done.

+    DCHECK_EQ(observed_view, observed_view_);
+    observed_view_ = nullptr;

[sky, 2017/05/17 19:08:30]

Remove observer here.

[ackermanb, 2017/05/17 22:03:49]

Done.

+  }
+
+  views::View* focused_view() { return observed_view_; }
+
+ private:
+  views::View* observed_view_ = nullptr;

[Devlin, 2017/05/17 18:13:49]

DISALLOW_COPY_AND_ASSIGN()

[ackermanb, 2017/05/17 22:03:50]

Done.

+};
+
+}  // namespace
+
 namespace views {
 
 namespace {
 
 // The group used by the buttons.  This name is chosen voluntarily big not to
 // conflict with other groups that could be in the dialog content.
 const int kButtonGroup = 6666;
 
 #if defined(OS_WIN) || defined(OS_CHROMEOS)
 const bool kIsOkButtonOnLeftSide = true;
@@ skipping 278 matching lines @@
   View* third = ok_button_;
   if (kIsOkButtonOnLeftSide)
     std::swap(second, third);
   return {{first, second, third}};
 }
 
 void DialogClientView::SetupLayout() {
   base::AutoReset<bool> auto_reset(&adding_or_removing_views_, true);
   GridLayout* layout = new GridLayout(button_row_container_);
   layout->set_minimum_size(minimum_size_);
+  ViewDeletionObserver deletion_observer(GetFocusManager()->GetFocusedView());

[Devlin, 2017/05/17 18:13:49]

nit: Since we use the FocusManager twice in this function, we can cache it in a
variable.

[ackermanb, 2017/05/17 22:03:50]

Done.

 
   // Clobber any existing LayoutManager since it has weak references to child
   // Views which may be removed by SetupViews().
   button_row_container_->SetLayoutManager(layout);
   SetupViews();
   const std::array<View*, kNumButtons> views = GetButtonRowViews();
 
   // Visibility changes on |extra_view_| must be observed to re-Layout. However,
   // when hidden it's not included in the button row (it can't influence layout)
   // and it can't be added to |button_row_container_| (GridLayout complains).
@@ skipping 60 matching lines @@
   }
 
   if (ui::MaterialDesignController::IsSecondaryUiMaterial()) {
     // Only link the extra view column if it is a button.
     if (views[0] && !CustomButton::AsCustomButton(views[0]))
       column_set->LinkColumnSizes(link[1], link[2], -1);
     else
       column_set->LinkColumnSizes(link[0], link[1], link[2], -1);
   }
   layout->AddPaddingRow(kFixed, insets.bottom());
+
+  // The default focus is lost when child views are added back into the dialog.
+  // This restores focus if the button is still available.
+  View* focused_view = deletion_observer.focused_view();

[sky, 2017/05/17 19:08:30]

Name this previously_focused_view.

[ackermanb, 2017/05/17 22:03:50]

Done.

+  if (!GetFocusManager()->GetFocusedView() && focused_view)

[Devlin, 2017/05/17 18:13:49]

drive-by nit: I'd invert these, since checking focused_view is cheaper:
if (focused_view && !GetFocusManaged()->GetFocusedView())

[sky, 2017/05/17 19:08:30]

Also, add a conditional of Contains(focused_view);

[ackermanb, 2017/05/17 22:03:49]

Done.

[ackermanb, 2017/05/17 22:03:50]

Done.

+    focused_view->RequestFocus();
 }
 
 void DialogClientView::SetupViews() {
   button_row_container_->RemoveAllChildViews(false /* delete children */);
   // If SetupLayout() "stored" a hidden |extra_view_| in |this|, ensure it can
   // be re-added to the layout when becoming visible.
   if (extra_view_)
     RemoveChildView(extra_view_);
 
   UpdateDialogButton(&ok_button_, ui::DIALOG_BUTTON_OK);
   UpdateDialogButton(&cancel_button_, ui::DIALOG_BUTTON_CANCEL);
 
   if (extra_view_)
     return;
 
   extra_view_ = GetDialogDelegate()->CreateExtraView();
   if (extra_view_)
     extra_view_->SetGroup(kButtonGroup);
 }
 
 }  // namespace views