Fix extensions-button visibility in PWA windows

Adds an GetIconColor method to ToolbarIconContainerView and a method to
override it. The latter is used by WebAppFrameToolbarView to set the
icon color.

Before this the icon color was never set for the container hosted inside
WebAppFrameToolbarView so the icon image was never loaded -> never
visible.

Bug: chromium:1006162
Change-Id: Ic3566461f1bd7319944060db766b91ac38adef87
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1900090
Commit-Queue: Caroline Rising <corising@chromium.org>
Reviewed-by: Caroline Rising <corising@chromium.org>
Auto-Submit: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712753}

chrome/browser/ui/views/extensions/extensions_toolbar_button.cc

@@ -7,6 +7,7 @@
 #include "chrome/app/vector_icons/vector_icons.h"
 #include "chrome/browser/themes/theme_properties.h"
 #include "chrome/browser/ui/views/extensions/extensions_menu_view.h"
+#include "chrome/browser/ui/views/extensions/extensions_toolbar_container.h"
 #include "chrome/grit/generated_resources.h"
 #include "extensions/browser/extension_registry.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -18,7 +19,7 @@
 
 ExtensionsToolbarButton::ExtensionsToolbarButton(
     Browser* browser,
-    ExtensionsContainer* extensions_container)
+    ExtensionsToolbarContainer* extensions_container)
     : ToolbarButton(this),
       browser_(browser),
       extensions_container_(extensions_container) {
@@ -32,10 +33,9 @@ void ExtensionsToolbarButton::UpdateIcon() {
   const int icon_size = ui::MaterialDesignController::touch_ui()
                             ? kDefaultTouchableIconSize
                             : kDefaultIconSize;
-  const SkColor normal_color =
-      GetThemeProvider()->GetColor(ThemeProperties::COLOR_TOOLBAR_BUTTON_ICON);
   SetImage(views::Button::STATE_NORMAL,
-           gfx::CreateVectorIcon(kExtensionIcon, icon_size, normal_color));
+           gfx::CreateVectorIcon(kExtensionIcon, icon_size,
+                                 extensions_container_->GetIconColor()));
 }
 
 void ExtensionsToolbarButton::ButtonPressed(views::Button* sender,

chrome/browser/ui/views/extensions/extensions_toolbar_button.h

@@ -8,7 +8,7 @@
 #include "chrome/browser/ui/views/toolbar/toolbar_button.h"
 
 class Browser;
-class ExtensionsContainer;
+class ExtensionsToolbarContainer;
 
 // Button in the toolbar that provides access to the corresponding extensions
 // menu.
@@ -16,7 +16,7 @@ class ExtensionsToolbarButton : public ToolbarButton,
                                 public views::ButtonListener {
  public:
   ExtensionsToolbarButton(Browser* browser,
-                          ExtensionsContainer* extensions_container);
+                          ExtensionsToolbarContainer* extensions_container);
 
   void UpdateIcon();
 
@@ -25,7 +25,7 @@ class ExtensionsToolbarButton : public ToolbarButton,
   void ButtonPressed(views::Button* sender, const ui::Event& event) override;
 
   Browser* const browser_;
-  ExtensionsContainer* const extensions_container_;
+  ExtensionsToolbarContainer* const extensions_container_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionsToolbarButton);
 };

chrome/browser/ui/views/profiles/avatar_toolbar_button_unittest.cc

@@ -19,7 +19,7 @@ class AvatarToolbarButtonTest : public TestWithBrowserView {};
 #if !defined(OS_CHROMEOS)
 
 TEST_F(AvatarToolbarButtonTest, HighlightMeetsMinimumContrast) {
-  auto parent = std::make_unique<ToolbarIconContainerView>(browser());
+  auto parent = std::make_unique<ToolbarAccountIconContainerView>(browser());
   auto button = std::make_unique<AvatarToolbarButton>(browser(), parent.get());
   button->set_owned_by_client();
 

chrome/browser/ui/views/toolbar/toolbar_account_icon_container_view.cc

@@ -5,7 +5,6 @@
 #include "chrome/browser/ui/views/toolbar/toolbar_account_icon_container_view.h"
 
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/themes/theme_properties.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_command_controller.h"
 #include "chrome/browser/ui/views/autofill/payments/local_card_migration_icon_view.h"
@@ -93,11 +92,6 @@ const char* ToolbarAccountIconContainerView::GetClassName() const {
   return kToolbarAccountIconContainerViewClassName;
 }
 
-SkColor ToolbarAccountIconContainerView::GetIconColor() const {
-  return GetThemeProvider()->GetColor(
-      ThemeProperties::COLOR_TOOLBAR_BUTTON_ICON);
-}
-
 const views::View::Views& ToolbarAccountIconContainerView::GetChildren() const {
   return page_action_icon_container_view_->children();
 }

chrome/browser/ui/views/toolbar/toolbar_account_icon_container_view.h

@@ -49,8 +49,6 @@ class ToolbarAccountIconContainerView : public ToolbarIconContainerView,
   // ToolbarIconContainerView:
   const views::View::Views& GetChildren() const override;
 
-  SkColor GetIconColor() const;
-
   PageActionIconContainerView* page_action_icon_container_view_ = nullptr;
 
   AvatarToolbarButton* const avatar_ = nullptr;

chrome/browser/ui/views/toolbar/toolbar_icon_container_view.cc

@@ -7,6 +7,7 @@
 #include <memory>
 
 #include "base/stl_util.h"
+#include "chrome/browser/themes/theme_properties.h"
 #include "chrome/browser/ui/layout_constants.h"
 #include "chrome/browser/ui/views/chrome_layout_provider.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_button.h"
@@ -40,8 +41,6 @@ ToolbarIconContainerView::~ToolbarIconContainerView() {
   RemoveAllChildViews(true);
 }
 
-void ToolbarIconContainerView::UpdateAllIcons() {}
-
 void ToolbarIconContainerView::AddMainButton(views::Button* main_button) {
   DCHECK(!main_button_);
   main_button->AddObserver(this);
@@ -159,6 +158,18 @@ void ToolbarIconContainerView::UpdateHighlight() {
     observer.OnHighlightChanged();
 }
 
+void ToolbarIconContainerView::OverrideIconColor(SkColor color) {
+  icon_color_ = color;
+  UpdateAllIcons();
+}
+
+SkColor ToolbarIconContainerView::GetIconColor() const {
+  if (icon_color_)
+    return icon_color_.value();
+  return GetThemeProvider()->GetColor(
+      ThemeProperties::COLOR_TOOLBAR_BUTTON_ICON);
+}
+
 bool ToolbarIconContainerView::IsHighlighted() {
   return ShouldDisplayHighlight();
 }

chrome/browser/ui/views/toolbar/toolbar_icon_container_view.h

@@ -27,8 +27,8 @@ class ToolbarIconContainerView : public views::View,
   ToolbarIconContainerView& operator=(const ToolbarIconContainerView&) = delete;
   ~ToolbarIconContainerView() override;
 
-  // Update all the icons it contains. Override by subclass.
-  virtual void UpdateAllIcons();
+  // Update all the icons it contains.
+  virtual void UpdateAllIcons() = 0;
 
   // Adds the RHS child as well as setting its margins.
   void AddMainButton(views::Button* main_button);
@@ -36,6 +36,9 @@ class ToolbarIconContainerView : public views::View,
   void AddObserver(Observer* obs);
   void RemoveObserver(const Observer* obs);
 
+  void OverrideIconColor(SkColor icon_color);
+  SkColor GetIconColor() const;
+
   bool IsHighlighted();
 
   // views::ButtonObserver:
@@ -84,6 +87,10 @@ class ToolbarIconContainerView : public views::View,
   // hierarchy.
   views::Button* main_button_ = nullptr;
 
+  // Override for the icon color. If not set, |COLOR_TOOLBAR_BUTTON_ICON| is
+  // used.
+  base::Optional<SkColor> icon_color_;
+
   // Points to the child buttons that we know are currently highlighted.
   // TODO(pbos): Consider observing buttons leaving our hierarchy and removing
   // them from this set.

chrome/browser/ui/views/web_apps/web_app_frame_toolbar_view.cc

@@ -613,6 +613,8 @@ void WebAppFrameToolbarView::UpdateChildrenColor() {
     web_app_origin_text_->SetTextColor(icon_color);
   if (content_settings_container_)
     content_settings_container_->SetIconColor(icon_color);
+  if (extensions_container_)
+    extensions_container_->OverrideIconColor(icon_color);
   page_action_icon_container_view_->SetIconColor(icon_color);
   web_app_menu_button_->SetColor(icon_color);
 }