Fix Harmony popover buttons being flush with their content.

ui/views/layout/layout_provider.h

 // Copyright 2017 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.
 
 #ifndef UI_VIEWS_LAYOUT_LAYOUT_PROVIDER_H_
 #define UI_VIEWS_LAYOUT_LAYOUT_PROVIDER_H_
 
 #include "base/macros.h"
 #include "ui/gfx/geometry/insets.h"
 #include "ui/views/style/typography_provider.h"
@@ skipping 31 matching lines @@
   // All Insets enum values must be below this value.
   VIEWS_INSETS_MAX = 0x1000
 };
 
 enum DistanceMetric {
   // DistanceMetric enum values must always be greater than any InsetsMetric
   // value. This allows the code to verify at runtime that arguments of the
   // two types have not been interchanged.
   VIEWS_DISTANCE_START = VIEWS_INSETS_MAX,
 
+  // If a bubble has buttons, this is the margin between them and the rest of
+  // the content.
+  DISTANCE_BUBBLE_BUTTON_TOP_MARGIN = VIEWS_DISTANCE_START,

[Peter Kasting, 2017/06/10 01:51:11]

I'm not sure whether adding this is right.

We seem to have two sets of constants today: one for "dialogs" and one for
"bubbles".  AFAICT the distinction between those two is that dialogs have a
button row and bubbles do not.  That's certainly how I've been operating.

But the premise of this patch is that we have "bubbles" with buttons.  In which
case, my question is, why aren't these just dialogs, made to use the
DIALOG_BUTTON_ROW insets (and dialog content insets, etc.)?

[Bret, 2017/06/11 00:14:09]

No, lots of bubbles have buttons (e.g. the bookmark bubble, crashed session
bubble, translate bubble, and others). You might be right that they SHOULD be
dialogs, but that's not really how the code works.

Post-Harmony the distinction I've been using is that dialogs are modal and
bubbles aren't, with the attendant consequences: bubbles have a close-X and
close when you click away. Pre-Harmony there are a few extra differences:
* Bubbles have arrows and dialogs don't.
* Bubbles are anchored and not draggable, and dialogs are floating and
draggable.
* Dialogs have slightly larger margins.

All that is baked into DialogDelegateView vs BubbleDialogDelegateView. This is
why I mentioned in an earlier CL I thought we'd eventually merge the
distinction, because conceptually these ought to be dictated by the same
delegate class (with a switch for modality). But I think we should hold off on
doing that refactor till Harmony is done.

[Peter Kasting, 2017/06/12 22:47:14]

OK, that all makes sense.  But in that case it seems like we should make the
bubble and dialog insets all just match (as the dialog insets) for now.  The
dialog inset pre-Harmony is kButtonHorizontalMargin; if some bubbles can have
buttons too, then they should also be using those margins.  The only exception
would be bubbles that lack any buttons, which I suppose could stay the old way,
but it doesn't seem like a win to make them do so.

If we were going to do this, then basically all the BUBBLE_ insets/distances
would disappear, and we'd use the dialog ones everywhere.  And that, I think,
would still mean we could avoid adding this distance metric, and you could just
use the .top() value of INSETS_DIALOG_BUTTON_ROW (or something like that) in the
callers instead of this.

The actual change to nuke the BUBBLE_ constants would probably happen
separately, maybe as a precursor to this change.

WDYT?

[Bret, 2017/06/13 21:00:41]

I'd really like to get everything into a working state before I do
refactor-style changes. Plus removing the special case in
DialogClientView::SetupLayout will make refactors easier. I think getting those
two things is worth adding this constant, even if I have to remove it later.

I think it will stick around, though. My goal is to collapse all the constants
into a metric for the edges of the dialog, and then have this and a "between
title and contents" metric.

I do like having a mandate to make the bubble and dialog margins the same
though, it will make my life a lot easier.

   // Margin on the left and right of the contents of a bubble.
-  DISTANCE_BUBBLE_CONTENTS_HORIZONTAL_MARGIN = VIEWS_DISTANCE_START,
+  DISTANCE_BUBBLE_CONTENTS_HORIZONTAL_MARGIN,
   // Margin on the top and bottom of the contents of a bubble.
   DISTANCE_BUBBLE_CONTENTS_VERTICAL_MARGIN,
   // The default padding to add on each side of a button's label.
   DISTANCE_BUTTON_HORIZONTAL_PADDING,
   // The maximum width a button can have and still influence the sizes of
   // other linked buttons.  This allows short buttons to have linked widths
   // without long buttons making things overly wide.
   DISTANCE_BUTTON_MAX_LINKABLE_WIDTH,
   // The distance between a dialog's edge and the close button in the upper
   // trailing corner.
@@ skipping 44 matching lines @@
 
  private:
   DefaultTypographyProvider typography_provider_;
 
   DISALLOW_COPY_AND_ASSIGN(LayoutProvider);
 };
 
 }  // namespace views
 
 #endif  // UI_VIEWS_LAYOUT_LAYOUT_PROVIDER_H_