Fix missing buttons on the Edit Bookmark dialog.

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"
@@ skipping 172 matching lines @@
   if (details.is_add) {
     if (child == this)
       UpdateDialogButtons();
     return;
   }
 
   if (details.parent != button_row_container_)
     return;
 
   // SetupViews() removes all children, managing data members itself.
-  if (preserve_button_row_data_members_)
+  if (adding_or_removing_views_)
     return;
 
   // Otherwise, this should only happen during teardown. Ensure there are no
   // references to deleted Views.
   button_row_container_->SetLayoutManager(nullptr);
 
   if (child == ok_button_)
     ok_button_ = nullptr;
   else if (child == cancel_button_)
     cancel_button_ = nullptr;
@@ skipping 29 matching lines @@
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // DialogClientView, private:
 
 DialogDelegate* DialogClientView::GetDialogDelegate() const {
   return GetWidget()->widget_delegate()->AsDialogDelegate();
 }
 
 void DialogClientView::ChildPreferredSizeChanged(View* child) {
-  if (child == extra_view_)
+  if (!adding_or_removing_views_ && child == extra_view_)
     Layout();

[tapted, 2017/03/08 11:25:26]

This can probably just be InvalidateLayout()... but it wasn't before the
refactor, so there might be a more subtle reason not to do that.

[Peter Kasting, 2017/03/08 11:33:17]

Would that even do anything?  IIRC, InvalidateLayout() doesn't force a future
layout, it just says layouts should propagate to this object.

[tapted, 2017/03/08 12:19:03]

Yeah it would just ensure a future call to Layout() at a higher level actually
re-layouts this view.

But I think Layout() here is bad, because it can't be coalesced. This probably
got added because a button changed its text - maybe it was from a time the
focused button had bold text or something. But in that case (e.g. tabbing
between buttons) there are 2 buttons that need to be updated - they shouldn't
both trigger a re-layout.

But also finding who "should" be responsible for the final layout would require
a lot of digging - maybe this is the right place after all :/

 }
 
 void DialogClientView::ChildVisibilityChanged(View* child) {
   // Showing or hiding |extra_view_| can alter which columns have linked sizes.
   if (child == extra_view_)
     UpdateDialogButtons();
   ChildPreferredSizeChanged(child);
 }
 
 void DialogClientView::UpdateDialogButton(LabelButton** member,
@@ skipping 54 matching lines @@
 DialogClientView::GetButtonRowViews() {
   View* first = ShouldShow(extra_view_) ? extra_view_ : nullptr;
   View* second = cancel_button_;
   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_);
 
   // 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,
@@ skipping 65 matching lines @@
     // 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());
 }
 
 void DialogClientView::SetupViews() {
-  {
-    base::AutoReset<bool> auto_reset(&preserve_button_row_data_members_, true);
-    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_);
-  }
+  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