Use Params struct to construct PageActionIconContainerView

The PageActionIconContainerView constructor takes 7 arguments.
This CL replaces them with a single Params struct for readability at the
call sites (in the absence of named function parameter passing in C++).

This CL has no changes in behaviour.

Bug: 788051
Change-Id: I423faf616c0f06236479c01579dd971c86251361
Reviewed-on: https://chromium-review.googlesource.com/c/1347640
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611545}

chrome/browser/ui/views/frame/hosted_app_button_container.cc

@@ -184,15 +184,6 @@ HostedAppButtonContainer::HostedAppButtonContainer(
       inactive_color_(inactive_color),
       hosted_app_origin_text_(new HostedAppOriginText(browser_view->browser())),
       content_settings_container_(new ContentSettingsContainer(this)),
-      page_action_icon_container_view_(new PageActionIconContainerView(
-          {PageActionIconType::kManagePasswords, PageActionIconType::kFind,
-           PageActionIconType::kZoom},
-          GetLayoutConstant(HOSTED_APP_PAGE_ACTION_ICON_SIZE),
-          HorizontalPaddingBetweenItems(),
-          browser_view->browser(),
-          browser_view->browser()->command_controller(),
-          this,
-          nullptr)),
       browser_actions_container_(
           new BrowserActionsContainer(browser_view->browser(),
                                       nullptr,
@@ -224,6 +215,17 @@ HostedAppButtonContainer::HostedAppButtonContainer(
   AddChildView(content_settings_container_);
   UpdateContentSettingViewsVisibility();
 
+  PageActionIconContainerView::Params params;
+  params.types_enabled.push_back(PageActionIconType::kManagePasswords);
+  params.types_enabled.push_back(PageActionIconType::kFind);
+  params.types_enabled.push_back(PageActionIconType::kZoom);
+  params.icon_size = GetLayoutConstant(HOSTED_APP_PAGE_ACTION_ICON_SIZE);
+  params.icon_color = GetIconColor();
+  params.between_icon_spacing = HorizontalPaddingBetweenItems();
+  params.browser = browser_view_->browser();
+  params.command_updater = browser_view_->browser()->command_controller();
+  params.page_action_icon_delegate = this;
+  page_action_icon_container_view_ = new PageActionIconContainerView(params);
   views::SetHitTestComponent(page_action_icon_container_view_,
                              static_cast<int>(HTCLIENT));
   AddChildView(page_action_icon_container_view_);

chrome/browser/ui/views/location_bar/location_bar_view.cc

@@ -212,20 +212,23 @@ void LocationBarView::Init() {
     AddChildView(image_view);
   }
 
-  std::vector<PageActionIconType> page_action_icon_types;
-  page_action_icon_types.push_back(PageActionIconType::kManagePasswords);
+  PageActionIconContainerView::Params params;
+  params.types_enabled.push_back(PageActionIconType::kManagePasswords);
   // |browser_| may be null when LocationBarView is used for non-Browser windows
   // such as PresentationReceiverWindowView, which do not support page actions.
   if (browser_) {
-    page_action_icon_types.push_back(PageActionIconType::kFind);
-    page_action_icon_types.push_back(PageActionIconType::kZoom);
+    params.types_enabled.push_back(PageActionIconType::kFind);
+    params.types_enabled.push_back(PageActionIconType::kZoom);
   }
-
-  page_action_icon_container_view_ = new PageActionIconContainerView(
-      page_action_icon_types, GetLayoutConstant(LOCATION_BAR_ICON_SIZE), 0,
-      browser_, command_updater(), this, delegate_);
+  params.icon_size = GetLayoutConstant(LOCATION_BAR_ICON_SIZE);
+  params.icon_color = icon_color;
+  params.between_icon_spacing = 0;
+  params.browser = browser_;
+  params.command_updater = command_updater();
+  params.page_action_icon_delegate = this;
+  params.location_bar_delegate = delegate_;
+  page_action_icon_container_view_ = new PageActionIconContainerView(params);
   AddChildView(page_action_icon_container_view_);
-  page_action_icon_container_view_->SetIconColor(icon_color);
 
   if (browser_) {
     save_credit_card_icon_view_ = new autofill::SaveCardIconView(

chrome/browser/ui/views/page_action/page_action_icon_container_view.cc

@@ -11,35 +11,38 @@
 #include "chrome/browser/ui/views/passwords/manage_passwords_icon_views.h"
 #include "ui/views/layout/box_layout.h"
 
-PageActionIconContainerView::PageActionIconContainerView(
-    const std::vector<PageActionIconType>& types_enabled,
-    int icon_size,
-    int between_icon_spacing,
-    Browser* browser,
-    CommandUpdater* command_updater,
-    PageActionIconView::Delegate* page_action_icon_delegate,
-    LocationBarView::Delegate* location_bar_delegate)
+PageActionIconContainerView::Params::Params() = default;
+PageActionIconContainerView::Params::~Params() = default;
+
+PageActionIconContainerView::PageActionIconContainerView(const Params& params)
     : zoom_observer_(this) {
+  DCHECK_GT(params.icon_size, 0);
+  DCHECK_NE(params.icon_color, gfx::kPlaceholderColor);
+  DCHECK(params.page_action_icon_delegate);
+
   views::BoxLayout& layout =
       *SetLayoutManager(std::make_unique<views::BoxLayout>(
-          views::BoxLayout::kHorizontal, gfx::Insets(), between_icon_spacing));
+          views::BoxLayout::kHorizontal, gfx::Insets(),
+          params.between_icon_spacing));
   // Right align to clip the leftmost items first when not enough space.
   layout.set_main_axis_alignment(views::BoxLayout::MAIN_AXIS_ALIGNMENT_END);
 
-  for (PageActionIconType type : types_enabled) {
+  for (PageActionIconType type : params.types_enabled) {
     switch (type) {
       case PageActionIconType::kFind:
-        find_bar_icon_ = new FindBarIcon(browser, page_action_icon_delegate);
+        find_bar_icon_ =
+            new FindBarIcon(params.browser, params.page_action_icon_delegate);
         page_action_icons_.push_back(find_bar_icon_);
         break;
       case PageActionIconType::kManagePasswords:
+        DCHECK(params.command_updater);
         manage_passwords_icon_ = new ManagePasswordsIconViews(
-            command_updater, page_action_icon_delegate);
+            params.command_updater, params.page_action_icon_delegate);
         page_action_icons_.push_back(manage_passwords_icon_);
         break;
       case PageActionIconType::kZoom:
-        zoom_view_ =
-            new ZoomView(location_bar_delegate, page_action_icon_delegate);
+        zoom_view_ = new ZoomView(params.location_bar_delegate,
+                                  params.page_action_icon_delegate);
         page_action_icons_.push_back(zoom_view_);
         break;
     }
@@ -47,14 +50,15 @@ PageActionIconContainerView::PageActionIconContainerView(
 
   for (PageActionIconView* icon : page_action_icons_) {
     icon->SetVisible(false);
-    icon->set_icon_size(icon_size);
+    icon->set_icon_size(params.icon_size);
     icon->Init();
+    icon->SetIconColor(params.icon_color);
     AddChildView(icon);
   }
 
-  if (browser) {
-    zoom_observer_.Add(
-        zoom::ZoomEventManager::GetForBrowserContext(browser->profile()));
+  if (params.browser) {
+    zoom_observer_.Add(zoom::ZoomEventManager::GetForBrowserContext(
+        params.browser->profile()));
   }
 }
 

chrome/browser/ui/views/page_action/page_action_icon_container_view.h

@@ -12,6 +12,7 @@
 #include "components/zoom/zoom_event_manager.h"
 #include "components/zoom/zoom_event_manager_observer.h"
 #include "third_party/skia/include/core/SkColor.h"
+#include "ui/gfx/color_palette.h"
 #include "ui/views/view.h"
 
 class Browser;
@@ -24,14 +25,24 @@ class PageActionIconContainerView : public views::View,
                                     public PageActionIconContainer,
                                     public zoom::ZoomEventManagerObserver {
  public:
-  PageActionIconContainerView(
-      const std::vector<PageActionIconType>& types_enabled,
-      int icon_size,
-      int between_icon_spacing,
-      Browser* browser,
-      CommandUpdater* command_updater,
-      PageActionIconView::Delegate* page_action_icon_delegate,
-      LocationBarView::Delegate* location_bar_delegate);
+  struct Params {
+    Params();
+    ~Params();
+
+    std::vector<PageActionIconType> types_enabled;
+    int icon_size = 0;
+    SkColor icon_color = gfx::kPlaceholderColor;
+    int between_icon_spacing = 0;
+    Browser* browser = nullptr;
+    CommandUpdater* command_updater = nullptr;
+    PageActionIconView::Delegate* page_action_icon_delegate = nullptr;
+    LocationBarView::Delegate* location_bar_delegate = nullptr;
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(Params);
+  };
+
+  explicit PageActionIconContainerView(const Params& params);
   ~PageActionIconContainerView() override;
 
   PageActionIconView* GetPageActionIconView(PageActionIconType type);
@@ -43,7 +54,7 @@ class PageActionIconContainerView : public views::View,
   // whether any icons were activated.
   bool ActivateFirstInactiveBubbleForAccessibility();
 
-  // Update the icons color, must be called before painting.
+  // Update the icons color.
   void SetIconColor(SkColor icon_color);
 
   // See comment in browser_window.h for more info.