Fix Harmony popover buttons being flush with their content.

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 329 matching lines @@
   // 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).
   // So add it, hidden, to |this| so it can be observed.
   if (extra_view_ && !views[0])
     AddChildView(extra_view_);
 
   if (std::count(views.begin(), views.end(), nullptr) == kNumButtons)
     return;
 
-  gfx::Insets insets = button_row_insets_;
-  LayoutProvider* const layout_provider = LayoutProvider::Get();
-  // Support dialogs that clear |button_row_insets_| to do their own layout.
-  // They expect GetDialogRelatedControlVerticalSpacing() in this case.
-  if (insets.top() == 0 &&

[Bret, 2017/06/07 22:52:18]

Removing this block reduces the pre-Harmony content/button spacing from 21 to 13
pixels on all dialogs.

If we really want to keep it I can add DISTANCE_BUBBLE_BUTTON_TOP_MARGIN to
INSETS_DIALOG_CONTENTS bottom in LayoutProvider, and move the existing
INSETS_DIALOG_CONTENTS definition to HarmonyLayoutProvider. But doing that
refactor was the point at which I questioned whether we actually want that
behavior.

-      !ui::MaterialDesignController::IsSecondaryUiMaterial()) {
-    const int top =
-        layout_provider->GetDistanceMetric(DISTANCE_RELATED_CONTROL_VERTICAL);
-    insets.Set(top, insets.left(), insets.bottom(), insets.right());
-  }
-
   // The |resize_percent| constants. There's only one stretchy column (padding
   // to the left of ok/cancel buttons).
   constexpr float kFixed = 0.f;
   constexpr float kStretchy = 1.f;
 
   // Button row is [ extra <pad+stretchy> second <pad> third ]. Ensure the <pad>
   // column is zero width if there isn't a button on either side.
   // GetExtraViewSpacing() handles <pad+stretchy>.
+  LayoutProvider* const layout_provider = LayoutProvider::Get();
   const int button_spacing = (ok_button_ && cancel_button_)
                                  ? layout_provider->GetDistanceMetric(
                                        DISTANCE_RELATED_BUTTON_HORIZONTAL)
                                  : 0;
 
   constexpr int kButtonRowId = 0;
   ColumnSet* column_set = layout->AddColumnSet(kButtonRowId);
 
   // Rather than giving |button_row_container_| a Border, incorporate the insets
   // into the layout. This simplifies min/max size calculations.
-  column_set->AddPaddingColumn(kFixed, insets.left());
+  column_set->AddPaddingColumn(kFixed, button_row_insets_.left());
   column_set->AddColumn(GridLayout::FILL, GridLayout::FILL, kFixed,
                         GridLayout::USE_PREF, 0, 0);
   column_set->AddPaddingColumn(kStretchy, GetExtraViewSpacing());
   column_set->AddColumn(GridLayout::FILL, GridLayout::FILL, kFixed,
                         GridLayout::USE_PREF, 0, 0);
   column_set->AddPaddingColumn(kFixed, button_spacing);
   column_set->AddColumn(GridLayout::FILL, GridLayout::FILL, kFixed,
                         GridLayout::USE_PREF, 0, 0);
-  column_set->AddPaddingColumn(kFixed, insets.right());
+  column_set->AddPaddingColumn(kFixed, button_row_insets_.right());
 
   // Track which columns to link sizes under MD.
   constexpr int kViewToColumnIndex[] = {1, 3, 5};
   int link[] = {-1, -1, -1};
   size_t link_index = 0;
 
-  layout->StartRowWithPadding(kFixed, kButtonRowId, kFixed, insets.top());
+  layout->StartRowWithPadding(kFixed, kButtonRowId, kFixed,
+                              button_row_insets_.top());
   for (size_t view_index = 0; view_index < kNumButtons; ++view_index) {
     if (views[view_index]) {
       layout->AddView(views[view_index]);
       link[link_index++] = kViewToColumnIndex[view_index];
     } else {
       layout->SkipColumns(1);
     }
   }
 
   column_set->set_linked_column_size_limit(
       layout_provider->GetDistanceMetric(DISTANCE_BUTTON_MAX_LINKABLE_WIDTH));
 
   // If |views[0]| is non-null, it is a visible |extra_view_| and its column
   // will be in |link[0]|. Skip that if it is not a button, or if it is a
   // Checkbox (which extends LabelButton). Otherwise, link everything.
   bool skip_first_link =
       views[0] && (!CustomButton::AsCustomButton(views[0]) ||
                    views[0]->GetClassName() == Checkbox::kViewClassName);
   if (skip_first_link)
     column_set->LinkColumnSizes(link[1], link[2], -1);
   else
     column_set->LinkColumnSizes(link[0], link[1], link[2], -1);
 
-  layout->AddPaddingRow(kFixed, insets.bottom());
+  layout->AddPaddingRow(kFixed, button_row_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* previously_focused_view = view_tracker.view();
   if (previously_focused_view && !focus_manager->GetFocusedView() &&
       Contains(previously_focused_view)) {
     previously_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