harmony: partly harmonize permissions bubble

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

 // Copyright 2014 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 "chrome/browser/ui/views/website_settings/permission_prompt_impl.h"
 
 #include <stddef.h>
 
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string16.h"
 #include "chrome/browser/permissions/permission_request.h"
 #include "chrome/browser/platform_util.h"
 #include "chrome/browser/profiles/profile.h"
 #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"
 #include "components/strings/grit/components_strings.h"
 #include "components/url_formatter/elide_url.h"
 #include "ui/accessibility/ax_node_data.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/models/combobox_model.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/color_palette.h"
 #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"
 #include "ui/views/controls/button/menu_button.h"
 #include "ui/views/controls/button/menu_button_listener.h"
 #include "ui/views/controls/combobox/combobox.h"
 #include "ui/views/controls/combobox/combobox_listener.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/menu/menu_runner.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/grid_layout.h"
 #include "ui/views/layout/layout_constants.h"
 
 namespace {
 
-// Spacing between major items should be 9px.
-const int kItemMajorSpacing = 9;
-
 // (Square) pixel size of icon.
 const int kIconSize = 18;
 
 }  // namespace
 
 // This class is a MenuButton which is given a PermissionMenuModel. It
 // shows the current checked item in the menu model, and notifies its listener
 // about any updates to the state of the selection.
 // TODO(gbillock): refactor PermissionMenuButton to work like this and re-use?
 class PermissionCombobox : public views::MenuButton,
@@ skipping 132 matching lines @@
     PermissionPromptImpl* owner,
     const std::vector<PermissionRequest*>& requests,
     const std::vector<bool>& accept_state)
     : owner_(owner),
       multiple_requests_(requests.size() > 1),
       persist_checkbox_(nullptr) {
   DCHECK(!requests.empty());
 
   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(),
       url_formatter::SchemeDisplay::OMIT_CRYPTOGRAPHIC);
 
   bool show_persistence_toggle = true;
   for (size_t index = 0; index < requests.size(); index++) {
     DCHECK(index < accept_state.size());
     // The row is laid out containing a leading-aligned label area and a
     // trailing column which will be filled if there are multiple permission
     // requests.
     views::View* row = new views::View();
     views::GridLayout* row_layout = new views::GridLayout(row);
     row->SetLayoutManager(row_layout);
     views::ColumnSet* columns = row_layout->AddColumnSet(0);
     columns->AddColumn(views::GridLayout::LEADING, views::GridLayout::FILL,
                        0, views::GridLayout::USE_PREF, 0, 0);
     columns->AddColumn(views::GridLayout::TRAILING, views::GridLayout::FILL,
                        100, views::GridLayout::USE_PREF, 0, 0);
     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) {
       icon->SetImage(
           gfx::CreateVectorIcon(vector_id, kIconSize, gfx::kChromeIconGrey));
     }
     icon->SetTooltipText(base::string16());  // Redundant with the text fragment
     label_container->AddChildView(icon);
     views::Label* label =
         new views::Label(requests.at(index)->GetMessageTextFragment());
@@ skipping 244 matching lines @@
 }
 
 void PermissionPromptImpl::Deny() {
   if (delegate_)
     delegate_->Deny();
 }
 
 Profile* PermissionPromptImpl::GetProfile() {
   return browser_->profile();
 }