Views/Harmony: Use LayoutDelegate::GetMetric for views::kPanelVertMargin.

chrome/browser/ui/views/website_settings/website_settings_popup_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 "chrome/browser/ui/views/website_settings/website_settings_popup_view.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <vector>
 
 #include "base/i18n/rtl.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/certificate_viewer.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/browser/ui/layout_constants.h"
 #include "chrome/browser/ui/views/collected_cookies_views.h"
+#include "chrome/browser/ui/views/harmony/layout_delegate.h"
 #include "chrome/browser/ui/views/website_settings/chosen_object_row.h"
 #include "chrome/browser/ui/views/website_settings/non_accessible_image_view.h"
 #include "chrome/browser/ui/views/website_settings/permission_selector_row.h"
 #include "chrome/browser/ui/website_settings/website_settings.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/grit/theme_resources.h"
 #include "components/content_settings/core/common/content_settings_types.h"
 #include "components/strings/grit/components_chromium_strings.h"
@@ skipping 12 matching lines @@
 #include "ui/resources/grit/ui_resources.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/button/image_button.h"
 #include "ui/views/controls/button/md_text_button.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/link.h"
 #include "ui/views/controls/styled_label.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/grid_layout.h"
-#include "ui/views/layout/layout_constants.h"
 #include "ui/views/layout/layout_manager.h"
 #include "ui/views/view.h"
 #include "ui/views/widget/widget.h"
 #include "url/gurl.h"
 
 namespace {
 
 // NOTE(jdonnelly): This use of this process-wide variable assumes that there's
 // never more than one website settings popup shown and that it's associated
 // with the current window. If this assumption fails in the future, we'll need
 // to return a weak pointer from ShowPopup so callers can associate it with the
 // current window (or other context) and check if the popup they care about is
 // showing.
 WebsiteSettingsPopupView::PopupType g_shown_popup_type =
     WebsiteSettingsPopupView::POPUP_NONE;
 
 // General constants -----------------------------------------------------------
 
 // Popup width constraints.
 const int kMinPopupWidth = 320;
 const int kMaxPopupWidth = 1000;
 
 // Security Section (PopupHeaderView) ------------------------------------------
 
 // Margin and padding values for the |PopupHeaderView|.
 const int kHeaderMarginBottom = 10;
-const int kHeaderPaddingBottom = views::kPanelVertMargin;
 
 // Spacing between labels in the header.
 const int kHeaderLabelSpacing = 4;
 
 // Site Settings Section -------------------------------------------------------
 
 // Spacing above and below the cookies view.
 const int kCookiesViewVerticalPadding = 6;
 
 // Spacing between a permission image and the text.
@@ skipping 112 matching lines @@
   layout->AddView(details_label_, 1, 1, views::GridLayout::FILL,
                   views::GridLayout::LEADING);
 
   layout->StartRow(0, label_column_status);
   reset_decisions_label_container_ = new views::View();
   reset_decisions_label_container_->SetLayoutManager(
       new views::BoxLayout(views::BoxLayout::kHorizontal, 0, 0, 0));
   layout->AddView(reset_decisions_label_container_, 1, 1,
                   views::GridLayout::FILL, views::GridLayout::LEADING);
 
-  layout->AddPaddingRow(1, kHeaderPaddingBottom);
+  layout->AddPaddingRow(1,

[tapted, 2017/02/27 03:24:51]

This may have been coincidence too.. I think we should put the constant into
`kHeaderPaddingBottom` above so it's consistent with kHeaderMarginBottom

[Peter Kasting, 2017/02/28 02:21:39]

I don't have a strong opinion on this one.  It seems like all this layout and
these constants will need rethinking for Harmony anyway, so whichever way we do
it we're likely to have to revisit this.

[Patti Lor, 2017/03/03 08:24:39]

Done.

[Patti Lor, 2017/03/03 08:24:39]

Acknowledged.

+                        LayoutDelegate::Get()->GetMetric(
+                            LayoutDelegate::Metric::PANEL_CONTENT_MARGIN));
 }
 
 PopupHeaderView::~PopupHeaderView() {}
 
 void PopupHeaderView::SetDetails(const base::string16& details_text) {
   std::vector<base::string16> subst;
   subst.push_back(details_text);
   subst.push_back(l10n_util::GetStringUTF16(IDS_LEARN_MORE));
 
   std::vector<size_t> offsets;
@@ skipping 204 matching lines @@
   layout->AddPaddingRow(1, kHeaderMarginBottom);
   layout->StartRow(1, content_column);
 
   site_settings_view_ = CreateSiteSettingsView(side_margin);
   layout->AddView(site_settings_view_);
 
   if (!ui::MaterialDesignController::IsSecondaryUiMaterial()) {
     // In non-material, titles are inset from the dialog margin. Ensure the
     // horizontal insets match.
     set_title_margins(
-        gfx::Insets(views::kPanelVertMargin, side_margin, 0, side_margin));
+        gfx::Insets(LayoutDelegate::Get()->GetMetric(
+                        LayoutDelegate::Metric::PANEL_CONTENT_MARGIN),

[tapted, 2017/02/27 03:24:51]

This only affects non-material, so I'm not sure it should change here (i.e. the
offset here really should be a compile-time constant, and this code should be
deleted when harmony is default)

[Peter Kasting, 2017/02/28 02:21:39]

I'm fine with changing this anyway, because the goal is to delete these views
constants entirely, and that may happen before we remove this code.

[Patti Lor, 2017/03/03 08:24:39]

Acknowledged, left as is.

+                    side_margin, 0, side_margin));
   }
   views::BubbleDialogDelegateView::CreateBubble(this);
 
   presenter_.reset(new WebsiteSettings(
       this, profile, TabSpecificContentSettings::FromWebContents(web_contents),
       web_contents, url, security_info));
 }
 
 void WebsiteSettingsPopupView::RenderFrameDeleted(
     content::RenderFrameHost* render_frame_host) {
@@ skipping 288 matching lines @@
           WebsiteSettings::WEBSITE_SETTINGS_CONNECTION_HELP_OPENED);
       break;
     case STYLED_LABEL_RESET_CERTIFICATE_DECISIONS:
       presenter_->OnRevokeSSLErrorBypassButtonPressed();
       GetWidget()->Close();
       break;
     default:
       NOTREACHED();
   }
 }