harmony: partly harmonize permissions bubble

chrome/browser/ui/views/website_settings/permission_prompt_impl.cc

Index: chrome/browser/ui/views/website_settings/permission_prompt_impl.cc
diff --git a/chrome/browser/ui/views/website_settings/permission_prompt_impl.cc b/chrome/browser/ui/views/website_settings/permission_prompt_impl.cc
index 37d4cc62d4aada07ed8265a3e4c8d234b29e80fa..1fa223104250a9e207552eb1ec4de1dc1b47db47 100644
--- a/chrome/browser/ui/views/website_settings/permission_prompt_impl.cc
+++ b/chrome/browser/ui/views/website_settings/permission_prompt_impl.cc
@@ -15,6 +15,7 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/views/exclusive_access_bubble_views.h"
+#include "chrome/browser/ui/views/harmony/layout_delegate.h"
 #include "chrome/browser/ui/views/website_settings/permission_selector_row.h"
 #include "chrome/browser/ui/views/website_settings/permission_selector_row_observer.h"
 #include "chrome/grit/generated_resources.h"
@@ -28,6 +29,7 @@
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/gfx/text_constants.h"
 #include "ui/gfx/vector_icons_public.h"
+#include "ui/views/background.h"
 #include "ui/views/bubble/bubble_dialog_delegate.h"
 #include "ui/views/bubble/bubble_frame_view.h"
 #include "ui/views/controls/button/checkbox.h"
@@ -43,9 +45,6 @@
 
 namespace {
 
-// Spacing between major items should be 9px.
-const int kItemMajorSpacing = 9;
-
 // (Square) pixel size of icon.
 const int kIconSize = 18;
 
@@ -198,8 +197,12 @@ PermissionsBubbleDialogDelegateView::PermissionsBubbleDialogDelegateView(
 
   set_close_on_deactivate(false);
 
-  SetLayoutManager(new views::BoxLayout(views::BoxLayout::kVertical, 0, 0,
-                                        kItemMajorSpacing));
+  LayoutDelegate* layout_delegate = LayoutDelegate::Get();
+  SetLayoutManager(
+      new views::BoxLayout(views::BoxLayout::kVertical, 0, 0,
+                           layout_delegate->GetLayoutDistance(
+                               LayoutDelegate::LayoutDistanceType::
+                                   RELATED_CONTROL_VERTICAL_SPACING)));
 
   display_origin_ = url_formatter::FormatUrlForSecurityDisplay(
       requests[0]->GetOrigin(),
@@ -222,9 +225,15 @@ PermissionsBubbleDialogDelegateView::PermissionsBubbleDialogDelegateView(
     row_layout->StartRow(0, 0);
 
     views::View* label_container = new views::View();
+    int indent = layout_delegate->UseExtraDialogPadding()
+                     ? layout_delegate->GetLayoutDistance(
+                           LayoutDelegate::LayoutDistanceType::
+                               RELATED_CONTROL_HORIZONTAL_SPACING)

[Peter Kasting, 2017/01/26 21:54:29]

This changes the indent for non-Harmony from 10 px to 8.  More critically, it
doesn't seem like RELATED_CONTROL_HORIZONTAL_SPACING, which is meant to be the
horizontal distance between the end of one control and the start of a second
control on the same line, is the right semantic value for "indentation for a
section of controls related to some block above it".  kCheckboxIndent was the
only "indenting"-related constant in layout_constants.h AFAIK; maybe the right
solution here is to add a new LayoutDistanceType enum value for this that's
kCheckboxIndent in pre-Harmony and 0 in Harmony, and then not bother to call
UseExtraDialogPadding here.

If we do this, I don't know that the new enum should be called CHECKBOX_INDENT
-- it would be nice to be more generic with the name in some way.

[Elly Fong-Jones, 2017/01/30 19:47:34]

CHECKBOX_INDENT already was 0 in Harmony and kCheckboxIndent pre-Harmony, so
I've renamed it to SUBSECTION_INDENT and made this code use it instead of
RELATED_CONTROL_HORIZONTAL_SPACING.

+                     : 0;
     label_container->SetLayoutManager(new views::BoxLayout(
-        views::BoxLayout::kHorizontal, views::kCheckboxIndent, 0,
-        views::kItemLabelSpacing));
+        views::BoxLayout::kHorizontal, indent, 0,
+        layout_delegate->GetLayoutDistance(
+            LayoutDelegate::LayoutDistanceType::ITEM_LABEL_SPACING)));
     views::ImageView* icon = new views::ImageView();
     gfx::VectorIconId vector_id = requests[index]->GetIconId();
     if (vector_id != gfx::VectorIconId::VECTOR_ICON_NONE) {