Use ImageModel for PageInfo bubble icons.

This avoids accessing GetNativeTheme() before the bubble widget exists. This requires changing ImageView to hold an ImageModel and plumbing badges through ImageModel, VectorIconModel, and ThemedVectorIcon. Bug: 1056916 Change-Id: Ia848058caa9d765f0248b6761f47d9ca5b337d4f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2832126 Reviewed-by: Evan Stade <estade@chromium.org> Reviewed-by: Tommy Steimel <steimel@chromium.org> Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Steven Valdez <svaldez@chromium.org> Reviewed-by: Bill Budge <bbudge@chromium.org> Reviewed-by: Theresa <twellington@chromium.org> Reviewed-by: Jesse Doherty <jwd@chromium.org> Reviewed-by: Thomas Guilbert <tguilbert@chromium.org> Reviewed-by: Reilly Grant <reillyg@chromium.org> Reviewed-by: James Cook <jamescook@chromium.org> Reviewed-by: Peter Boström <pbos@chromium.org> Reviewed-by: Patti <patricialor@chromium.org> Commit-Queue: Peter Kasting <pkasting@chromium.org> Auto-Submit: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#874892}
Denger-Network · Apr 21, 2021 · 11957ed · 11957ed
1 parent 3e8569d
commit 11957ed
Show file tree

Hide file tree

Showing 52 changed files with 350 additions and 321 deletions.
diff --git a/ash/ambient/ui/ambient_background_image_view.cc b/ash/ambient/ui/ambient_background_image_view.cc
@@ -122,7 +122,7 @@ void AmbientBackgroundImageView::UpdateImageDetails(
   ambient_info_view_->UpdateImageDetails(details);
 }
 
-const gfx::ImageSkia& AmbientBackgroundImageView::GetCurrentImage() {
+gfx::ImageSkia AmbientBackgroundImageView::GetCurrentImage() {
   return image_view_->GetImage();
 }
 

diff --git a/ash/ambient/ui/ambient_background_image_view.h b/ash/ambient/ui/ambient_background_image_view.h
@@ -48,7 +48,7 @@ class ASH_EXPORT AmbientBackgroundImageView : public views::View,
   // Updates the details for the currently displayed image.
   void UpdateImageDetails(const std::u16string& details);
 
-  const gfx::ImageSkia& GetCurrentImage();
+  gfx::ImageSkia GetCurrentImage();
 
   gfx::Rect GetImageBoundsForTesting() const;
   gfx::Rect GetRelatedImageBoundsForTesting() const;

diff --git a/ash/ambient/ui/photo_view.cc b/ash/ambient/ui/photo_view.cc
@@ -145,7 +145,7 @@ bool PhotoView::NeedToAnimateTransition() const {
   return !image_views_.back()->GetCurrentImage().isNull();
 }
 
-const gfx::ImageSkia& PhotoView::GetVisibleImageForTesting() {
+gfx::ImageSkia PhotoView::GetVisibleImageForTesting() {
   return image_views_.at(image_index_)->GetCurrentImage();
 }
 

diff --git a/ash/ambient/ui/photo_view.h b/ash/ambient/ui/photo_view.h
@@ -57,7 +57,7 @@ class ASH_EXPORT PhotoView : public views::View,
   // Return if can start transition animation.
   bool NeedToAnimateTransition() const;
 
-  const gfx::ImageSkia& GetVisibleImageForTesting();
+  gfx::ImageSkia GetVisibleImageForTesting();
 
   // Note that we should be careful when using |delegate_|, as there is no
   // strong guarantee on the life cycle.

diff --git a/ash/assistant/ui/main_stage/assistant_onboarding_suggestion_view.cc b/ash/assistant/ui/main_stage/assistant_onboarding_suggestion_view.cc
@@ -113,7 +113,7 @@ void AssistantOnboardingSuggestionView::RemoveLayerBeneathView(
   ink_drop_container_->RemoveLayerBeneathView(layer);
 }
 
-const gfx::ImageSkia& AssistantOnboardingSuggestionView::GetIcon() const {
+gfx::ImageSkia AssistantOnboardingSuggestionView::GetIcon() const {
   return icon_->GetImage();
 }
 

diff --git a/ash/assistant/ui/main_stage/assistant_onboarding_suggestion_view.h b/ash/assistant/ui/main_stage/assistant_onboarding_suggestion_view.h
@@ -49,7 +49,7 @@ class COMPONENT_EXPORT(ASSISTANT_UI) AssistantOnboardingSuggestionView
   void RemoveLayerBeneathView(ui::Layer* layer) override;
 
   // Returns the icon for the suggestion.
-  const gfx::ImageSkia& GetIcon() const;
+  gfx::ImageSkia GetIcon() const;
 
   // Returns the text for the suggestion.
   const std::u16string& GetText() const;

diff --git a/ash/assistant/ui/main_stage/suggestion_chip_view.cc b/ash/assistant/ui/main_stage/suggestion_chip_view.cc
@@ -170,7 +170,7 @@ void SuggestionChipView::SetIcon(const gfx::ImageSkia& icon) {
   icon_view_->SetVisible(!icon.isNull());
 }
 
-const gfx::ImageSkia& SuggestionChipView::GetIcon() const {
+gfx::ImageSkia SuggestionChipView::GetIcon() const {
   return icon_view_->GetImage();
 }
 

diff --git a/ash/assistant/ui/main_stage/suggestion_chip_view.h b/ash/assistant/ui/main_stage/suggestion_chip_view.h
@@ -44,7 +44,7 @@ class COMPONENT_EXPORT(ASSISTANT_UI) SuggestionChipView : public views::Button {
   bool OnKeyPressed(const ui::KeyEvent& event) override;
 
   void SetIcon(const gfx::ImageSkia& icon);
-  const gfx::ImageSkia& GetIcon() const;
+  gfx::ImageSkia GetIcon() const;
 
   void SetText(const std::u16string& text);
   const std::u16string& GetText() const;

diff --git a/ash/drag_drop/drag_drop_controller_unittest.cc b/ash/drag_drop/drag_drop_controller_unittest.cc
@@ -446,7 +446,7 @@ class DragDropControllerTest : public AshTestBase {
     drag_source_window->AddObserver(drag_drop_controller_.get());
   }
 
-  const gfx::ImageSkia& GetDragImage() {
+  gfx::ImageSkia GetDragImage() {
     return static_cast<DragImageView*>(
                drag_drop_controller_->drag_image_widget_->GetContentsView())
         ->GetImage();

diff --git a/ash/public/cpp/network_icon_image_source.cc b/ash/public/cpp/network_icon_image_source.cc
@@ -14,6 +14,7 @@
 #include "ui/gfx/scoped_canvas.h"
 #include "ui/gfx/skia_util.h"
 #include "ui/gfx/vector_icon_types.h"
+#include "ui/gfx/vector_icon_utils.h"
 
 namespace ash {
 namespace network_icon {

diff --git a/ash/shelf/shelf_app_button.cc b/ash/shelf/shelf_app_button.cc
@@ -360,7 +360,7 @@ void ShelfAppButton::SetImage(const gfx::ImageSkia& image) {
       image, skia::ImageOperations::RESIZE_BEST, preferred_size));
 }
 
-const gfx::ImageSkia& ShelfAppButton::GetImage() const {
+gfx::ImageSkia ShelfAppButton::GetImage() const {
   return icon_view_->GetImage();
 }
 

diff --git a/ash/shelf/shelf_app_button.h b/ash/shelf/shelf_app_button.h
@@ -69,7 +69,7 @@ class ASH_EXPORT ShelfAppButton : public ShelfButton,
   void SetImage(const gfx::ImageSkia& image);
 
   // Retrieve the image to show proxy operations.
-  const gfx::ImageSkia& GetImage() const;
+  gfx::ImageSkia GetImage() const;
 
   // |state| is or'd into the current state.
   void AddState(State state);

diff --git a/ash/style/ash_color_provider.cc b/ash/style/ash_color_provider.cc
@@ -25,6 +25,7 @@
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/paint_vector_icon.h"
+#include "ui/gfx/vector_icon_utils.h"
 #include "ui/views/background.h"
 #include "ui/views/controls/button/image_button.h"
 #include "ui/views/controls/button/label_button.h"

diff --git a/ash/system/accessibility/floating_menu_button.cc b/ash/system/accessibility/floating_menu_button.cc
@@ -9,8 +9,8 @@
 #include "ui/accessibility/ax_node_data.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/canvas.h"
-#include "ui/gfx/paint_vector_icon.h"
 #include "ui/gfx/vector_icon_types.h"
+#include "ui/gfx/vector_icon_utils.h"
 #include "ui/views/accessibility/view_accessibility.h"
 #include "ui/views/animation/flood_fill_ink_drop_ripple.h"
 #include "ui/views/animation/ink_drop_highlight.h"

diff --git a/ash/system/audio/unified_volume_view.cc b/ash/system/audio/unified_volume_view.cc
@@ -25,6 +25,7 @@
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/image/image_skia_operations.h"
 #include "ui/gfx/paint_vector_icon.h"
+#include "ui/gfx/vector_icon_utils.h"
 #include "ui/views/animation/flood_fill_ink_drop_ripple.h"
 #include "ui/views/animation/ink_drop_highlight.h"
 #include "ui/views/animation/ink_drop_impl.h"

diff --git a/ash/system/tray/system_menu_button.cc b/ash/system/tray/system_menu_button.cc
@@ -9,7 +9,7 @@
 #include "ash/system/tray/tray_popup_ink_drop_style.h"
 #include "ash/system/tray/tray_popup_utils.h"
 #include "ui/base/l10n/l10n_util.h"
-#include "ui/gfx/paint_vector_icon.h"
+#include "ui/gfx/vector_icon_utils.h"
 #include "ui/views/animation/flood_fill_ink_drop_ripple.h"
 #include "ui/views/animation/ink_drop_highlight.h"
 #include "ui/views/animation/ink_drop_impl.h"

diff --git a/ash/system/tray/tray_popup_utils.cc b/ash/system/tray/tray_popup_utils.cc
@@ -23,6 +23,7 @@
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/geometry/insets.h"
 #include "ui/gfx/paint_vector_icon.h"
+#include "ui/gfx/vector_icon_utils.h"
 #include "ui/views/animation/flood_fill_ink_drop_ripple.h"
 #include "ui/views/animation/ink_drop_highlight.h"
 #include "ui/views/animation/ink_drop_impl.h"

diff --git a/ash/system/unified/notification_counter_view.cc b/ash/system/unified/notification_counter_view.cc
@@ -24,6 +24,7 @@
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/font_list.h"
 #include "ui/gfx/image/canvas_image_source.h"
+#include "ui/gfx/vector_icon_utils.h"
 #include "ui/message_center/message_center.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/image_view.h"

diff --git a/ash/system/unified/unified_slider_view.cc b/ash/system/unified/unified_slider_view.cc
@@ -8,7 +8,7 @@
 #include "ash/system/tray/tray_popup_utils.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/canvas.h"
-#include "ui/gfx/paint_vector_icon.h"
+#include "ui/gfx/vector_icon_utils.h"
 #include "ui/views/accessibility/view_accessibility.h"
 #include "ui/views/animation/flood_fill_ink_drop_ripple.h"
 #include "ui/views/animation/ink_drop_highlight.h"

diff --git a/chrome/browser/ui/views/page_info/chosen_object_view.cc b/chrome/browser/ui/views/page_info/chosen_object_view.cc
@@ -129,6 +129,8 @@ ChosenObjectView::ChosenObjectView(
   }
 
   layout->AddPaddingRow(column_set_id, list_item_padding);
+
+  UpdateIconImage(/*is_deleted=*/false);
 }
 
 void ChosenObjectView::AddObserver(ChosenObjectViewObserver* observer) {
@@ -158,17 +160,8 @@ void ChosenObjectView::ExecuteDeleteCommand() {
   }
 }
 
-void ChosenObjectView::OnThemeChanged() {
-  views::View::OnThemeChanged();
-  UpdateIconImage(/*is_deleted=*/false);
-}
-
 void ChosenObjectView::UpdateIconImage(bool is_deleted) const {
-  // TODO(crbug.com/1096944): Why are we using label color for an icon?
-  icon_->SetImage(PageInfoUI::GetChosenObjectIcon(
-      *info_, is_deleted,
-      views::style::GetColor(*this, views::style::CONTEXT_LABEL,
-                             views::style::STYLE_PRIMARY)));
+  icon_->SetImage(PageInfoUI::GetChosenObjectIcon(*info_, is_deleted));
 }
 
 BEGIN_METADATA(ChosenObjectView, views::View)

diff --git a/chrome/browser/ui/views/page_info/chosen_object_view.h b/chrome/browser/ui/views/page_info/chosen_object_view.h
@@ -32,9 +32,6 @@ class ChosenObjectView : public views::View {
 
   void AddObserver(ChosenObjectViewObserver* observer);
 
-  // views:View:
-  void OnThemeChanged() override;
-
  private:
   void UpdateIconImage(bool is_deleted) const;
 

diff --git a/chrome/browser/ui/views/page_info/page_info_bubble_view.cc b/chrome/browser/ui/views/page_info/page_info_bubble_view.cc
@@ -102,12 +102,6 @@ namespace {
 constexpr int kMinBubbleWidth = 320;
 constexpr int kMaxBubbleWidth = 1000;
 
-SkColor GetRelatedTextColor() {
-  views::Label label;
-  return views::style::GetColor(label, views::style::CONTEXT_LABEL,
-                                views::style::STYLE_PRIMARY);
-}
-
 }  // namespace
 
 // The regular PageInfoBubbleView is not supported for internal Chrome pages and
@@ -310,8 +304,8 @@ PageInfoBubbleView::PageInfoBubbleView(
               view->HandleMoreInfoRequest(view->site_settings_link);
             },
             this),
-        PageInfoUI::GetSiteSettingsIcon(GetRelatedTextColor()),
-        IDS_PAGE_INFO_SITE_SETTINGS_LINK, std::u16string(),
+        PageInfoUI::GetSiteSettingsIcon(), IDS_PAGE_INFO_SITE_SETTINGS_LINK,
+        std::u16string(),
         PageInfoBubbleView::VIEW_ID_PAGE_INFO_LINK_OR_BUTTON_SITE_SETTINGS,
         tooltip, std::u16string()));
   }
@@ -402,8 +396,7 @@ void PageInfoBubbleView::SetCookieInfo(const CookieInfoList& cookie_info_list) {
     PageInfo::PermissionInfo info;
     info.type = ContentSettingsType::COOKIES;
     info.setting = CONTENT_SETTING_ALLOW;
-    const gfx::ImageSkia icon =
-        PageInfoUI::GetPermissionIcon(info, GetRelatedTextColor());
+    const ui::ImageModel icon = PageInfoUI::GetPermissionIcon(info);
 
     const std::u16string& tooltip =
         l10n_util::GetStringUTF16(IDS_PAGE_INFO_COOKIES_TOOLTIP);
@@ -562,8 +555,7 @@ void PageInfoBubbleView::SetIdentityInfo(const IdentityInfo& identity_info) {
     }
 
     // Add the Certificate Section.
-    const gfx::ImageSkia icon =
-        PageInfoUI::GetCertificateIcon(GetRelatedTextColor());
+    const ui::ImageModel icon = PageInfoUI::GetCertificateIcon();
     const std::u16string secondary_text = l10n_util::GetStringUTF16(
         valid_identity ? IDS_PAGE_INFO_CERTIFICATE_VALID_PARENTHESIZED
                        : IDS_PAGE_INFO_CERTIFICATE_INVALID_PARENTHESIZED);
@@ -646,7 +638,7 @@ void PageInfoBubbleView::SetPageFeatureInfo(const PageFeatureInfo& info) {
       views::BoxLayout::CrossAxisAlignment::kStretch);
 
   auto icon = std::make_unique<NonAccessibleImageView>();
-  icon->SetImage(PageInfoUI::GetVrSettingsIcon(GetRelatedTextColor()));
+  icon->SetImage(PageInfoUI::GetVrSettingsIcon());
   auto exit_button = std::make_unique<views::MdTextButton>(
       base::BindRepeating(
           [](PageInfoBubbleView* view) {

diff --git a/chrome/browser/ui/views/page_info/page_info_hover_button.cc b/chrome/browser/ui/views/page_info/page_info_hover_button.cc
@@ -12,6 +12,7 @@
 #include "chrome/browser/ui/views/chrome_typography.h"
 #include "components/page_info/features.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/base/models/image_model.h"
 #include "ui/views/animation/ink_drop.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/label.h"
@@ -23,7 +24,7 @@
 
 namespace {
 
-std::unique_ptr<views::View> CreateIconView(const gfx::ImageSkia& icon_image) {
+std::unique_ptr<views::View> CreateIconView(const ui::ImageModel& icon_image) {
   auto icon = std::make_unique<NonAccessibleImageView>();
   icon->SetImage(icon_image);
   // Make sure hovering over the icon also hovers the `PageInfoHoverButton`.
@@ -38,13 +39,13 @@ std::unique_ptr<views::View> CreateIconView(const gfx::ImageSkia& icon_image) {
 
 PageInfoHoverButton::PageInfoHoverButton(
     views::Button::PressedCallback callback,
-    const gfx::ImageSkia& main_image_icon,
+    const ui::ImageModel& main_image_icon,
     int title_resource_id,
     const std::u16string& secondary_text,
     int click_target_id,
     const std::u16string& tooltip_text,
     const std::u16string& subtitle_text,
-    base::Optional<gfx::ImageSkia> action_image_icon)
+    base::Optional<ui::ImageModel> action_image_icon)
     : HoverButton(std::move(callback), std::u16string()) {
   label()->SetHandlesTooltips(false);
 

diff --git a/chrome/browser/ui/views/page_info/page_info_hover_button.h b/chrome/browser/ui/views/page_info/page_info_hover_button.h
@@ -10,14 +10,14 @@
 #include "base/macros.h"
 #include "chrome/browser/ui/views/hover_button.h"
 
-namespace gfx {
-class ImageSkia;
-}  // namespace gfx
-
 namespace test {
 class PageInfoBubbleViewTestApi;
 }  // namespace test
 
+namespace ui {
+class ImageModel;
+}  // namespace ui
+
 namespace views {
 class Label;
 class StyledLabel;
@@ -54,14 +54,14 @@ class PageInfoHoverButton : public HoverButton {
   // *-------------------------------------------------------------------------*
   PageInfoHoverButton(
       views::Button::PressedCallback callback,
-      const gfx::ImageSkia& main_image_icon,
+      const ui::ImageModel& main_image_icon,
       int title_resource_id,
       const std::u16string& secondary_text,
       int click_target_id,
       const std::u16string& tooltip_text,
       const std::u16string& subtitle_text,
-      base::Optional<gfx::ImageSkia> action_image_icon = base::nullopt);
-  ~PageInfoHoverButton() override {}
+      base::Optional<ui::ImageModel> action_image_icon = base::nullopt);
+  ~PageInfoHoverButton() override = default;
 
   // Updates the title text, and applies the secondary style to the secondary
   // text portion, if present.