[Extensions cleanup] Use ImageModel for action icon

Use ImageModel for the toolbar action icon. This removes the need of converting from ImageSkia to Image and back to ImageSkia in various callers. No functionality changed. Bug: n/a Change-Id: I262602fdda260d8d0c7a86c1563e77b2278ea584 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4204873 Reviewed-by: Peter Kasting <pkasting@chromium.org> Commit-Queue: Emilia Paz <emiliapaz@chromium.org> Cr-Commit-Position: refs/heads/main@{#1098987}
chromium · Jan 31, 2023 · 1545449 · 1545449
1 parent 2f5a44b
commit 1545449
Show file tree

Hide file tree

Showing 11 changed files with 44 additions and 42 deletions.
diff --git a/chrome/browser/ui/extensions/extension_action_view_controller.cc b/chrome/browser/ui/extensions/extension_action_view_controller.cc
@@ -39,9 +39,9 @@
 #include "extensions/common/extension_features.h"
 #include "extensions/common/manifest_constants.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/base/models/image_model.h"
 #include "ui/color/color_provider_manager.h"
 #include "ui/gfx/image/image_skia.h"
-#include "ui/gfx/image/image_skia_operations.h"
 #include "ui/native_theme/native_theme.h"
 
 using extensions::ActionInfo;
@@ -199,13 +199,13 @@ void ExtensionActionViewController::SetDelegate(
   }
 }
 
-gfx::Image ExtensionActionViewController::GetIcon(
+ui::ImageModel ExtensionActionViewController::GetIcon(
     content::WebContents* web_contents,
     const gfx::Size& size) {
   if (!ExtensionIsValid())
-    return gfx::Image();
+    return ui::ImageModel();
 
-  return gfx::Image(
+  return ui::ImageModel::FromImageSkia(
       gfx::ImageSkia(GetIconImageSource(web_contents, size), size));
 }
 

diff --git a/chrome/browser/ui/extensions/extension_action_view_controller.h b/chrome/browser/ui/extensions/extension_action_view_controller.h
@@ -17,7 +17,6 @@
 #include "extensions/browser/extension_host_observer.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_id.h"
-#include "ui/gfx/image/image.h"
 
 class Browser;
 class ExtensionActionPlatformDelegate;
@@ -34,6 +33,10 @@ class ExtensionViewHost;
 class SitePermissionsHelper;
 }
 
+namespace ui {
+class ImageModel;
+}
+
 // The platform-independent controller for an ExtensionAction that is shown on
 // the toolbar (such as a page or browser action).
 // Since this class doesn't own the extension or extension action in question,
@@ -64,8 +67,8 @@ class ExtensionActionViewController
   // ToolbarActionViewController:
   std::string GetId() const override;
   void SetDelegate(ToolbarActionViewDelegate* delegate) override;
-  gfx::Image GetIcon(content::WebContents* web_contents,
-                     const gfx::Size& size) override;
+  ui::ImageModel GetIcon(content::WebContents* web_contents,
+                         const gfx::Size& size) override;
   std::u16string GetActionName() const override;
   std::u16string GetAccessibleName(
       content::WebContents* web_contents) const override;

diff --git a/chrome/browser/ui/extensions/extension_action_view_controller_unittest.cc b/chrome/browser/ui/extensions/extension_action_view_controller_unittest.cc
@@ -45,6 +45,7 @@
 #include "extensions/common/mojom/run_location.mojom-shared.h"
 #include "extensions/test/test_extension_dir.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/base/models/image_model.h"
 #include "ui/gfx/image/image_skia_rep.h"
 #include "ui/native_theme/native_theme.h"
 
@@ -810,7 +811,7 @@ TEST_F(ExtensionActionViewControllerUnitTest, TestGetIconWithNullWebContents) {
   // a non-empty icon should be returned.
   ExtensionActionViewController* const controller =
       GetViewControllerForId(extension->id());
-  gfx::Image icon = controller->GetIcon(nullptr, view_size());
+  ui::ImageModel icon = controller->GetIcon(nullptr, view_size());
   EXPECT_FALSE(icon.IsEmpty());
 }
 

diff --git a/chrome/browser/ui/toolbar/test_toolbar_action_view_controller.cc b/chrome/browser/ui/toolbar/test_toolbar_action_view_controller.cc
@@ -8,7 +8,7 @@
 
 #include "chrome/browser/extensions/site_permissions_helper.h"
 #include "chrome/browser/ui/toolbar/toolbar_action_view_delegate.h"
-#include "ui/gfx/image/image.h"
+#include "ui/base/models/image_model.h"
 
 TestToolbarActionViewController::TestToolbarActionViewController(
     const std::string& id)
@@ -29,10 +29,10 @@ void TestToolbarActionViewController::SetDelegate(
   delegate_ = delegate;
 }
 
-gfx::Image TestToolbarActionViewController::GetIcon(
+ui::ImageModel TestToolbarActionViewController::GetIcon(
     content::WebContents* web_contents,
     const gfx::Size& size) {
-  return gfx::Image();
+  return ui::ImageModel();
 }
 
 std::u16string TestToolbarActionViewController::GetActionName() const {

diff --git a/chrome/browser/ui/toolbar/test_toolbar_action_view_controller.h b/chrome/browser/ui/toolbar/test_toolbar_action_view_controller.h
@@ -25,8 +25,8 @@ class TestToolbarActionViewController : public ToolbarActionViewController {
   // ToolbarActionViewController:
   std::string GetId() const override;
   void SetDelegate(ToolbarActionViewDelegate* delegate) override;
-  gfx::Image GetIcon(content::WebContents* web_contents,
-                     const gfx::Size& size) override;
+  ui::ImageModel GetIcon(content::WebContents* web_contents,
+                         const gfx::Size& size) override;
   std::u16string GetActionName() const override;
   std::u16string GetAccessibleName(
       content::WebContents* web_contents) const override;

diff --git a/chrome/browser/ui/toolbar/toolbar_action_view_controller.h b/chrome/browser/ui/toolbar/toolbar_action_view_controller.h
@@ -11,6 +11,7 @@
 #include "chrome/browser/extensions/site_permissions_helper.h"
 #include "chrome/browser/ui/extensions/extension_popup_types.h"
 #include "chrome/browser/ui/toolbar/toolbar_action_hover_card_types.h"
+#include "ui/base/models/image_model.h"
 #include "ui/gfx/image/image.h"
 
 namespace content {
@@ -104,8 +105,8 @@ class ToolbarActionViewController {
   virtual void SetDelegate(ToolbarActionViewDelegate* delegate) = 0;
 
   // Returns the icon to use for the given |web_contents| and |size|.
-  virtual gfx::Image GetIcon(content::WebContents* web_contents,
-                             const gfx::Size& size) = 0;
+  virtual ui::ImageModel GetIcon(content::WebContents* web_contents,
+                                 const gfx::Size& size) = 0;
 
   // Returns the name of the action, which can be separate from the accessible
   // name or name for the tooltip.

diff --git a/chrome/browser/ui/views/extensions/extensions_dialogs_utils.cc b/chrome/browser/ui/views/extensions/extensions_dialogs_utils.cc
@@ -65,9 +65,9 @@ ExtensionsToolbarContainer* GetExtensionsToolbarContainer(
 // "action" action based on the web contents.
 ui::ImageModel GetIcon(ToolbarActionViewController* action,
                        content::WebContents* web_contents) {
-  return ui::ImageModel::FromImage(action->GetIcon(
-      web_contents, gfx::Size(extension_misc::EXTENSION_ICON_SMALLISH,
-                              extension_misc::EXTENSION_ICON_SMALLISH)));
+  return action->GetIcon(web_contents,
+                         gfx::Size(extension_misc::EXTENSION_ICON_SMALLISH,
+                                   extension_misc::EXTENSION_ICON_SMALLISH));
 }
 
 std::u16string GetCurrentHost(content::WebContents* web_contents) {

diff --git a/chrome/browser/ui/views/extensions/extensions_menu_button.cc b/chrome/browser/ui/views/extensions/extensions_menu_button.cc
@@ -74,10 +74,9 @@ void ExtensionsMenuButton::UpdateState() {
   ChromeLayoutProvider* const provider = ChromeLayoutProvider::Get();
   const int icon_size =
       provider->GetDistanceMetric(DISTANCE_EXTENSIONS_MENU_EXTENSION_ICON_SIZE);
-  SetImage(Button::STATE_NORMAL, controller_
-                                     ->GetIcon(GetCurrentWebContents(),
-                                               gfx::Size(icon_size, icon_size))
-                                     .AsImageSkia());
+  SetImageModel(Button::STATE_NORMAL,
+                controller_->GetIcon(GetCurrentWebContents(),
+                                     gfx::Size(icon_size, icon_size)));
 
   SetText(controller_->GetActionName());
   SetTooltipText(controller_->GetTooltip(GetCurrentWebContents()));

diff --git a/chrome/browser/ui/views/extensions/extensions_toolbar_container.cc b/chrome/browser/ui/views/extensions/extensions_toolbar_container.cc
@@ -37,6 +37,7 @@
 #include "ui/base/dragdrop/mojom/drag_drop_types.mojom-shared.h"
 #include "ui/base/dragdrop/mojom/drag_drop_types.mojom.h"
 #include "ui/base/metadata/metadata_impl_macros.h"
+#include "ui/base/models/image_model.h"
 #include "ui/views/interaction/element_tracker_views.h"
 #include "ui/views/layout/animating_layout_manager.h"
 #include "ui/views/layout/flex_layout.h"
@@ -665,13 +666,14 @@ void ExtensionsToolbarContainer::WriteDragDataForView(
       model_->pinned_action_ids(), sender,
       [this](const std::string& action_id) { return GetViewForId(action_id); });
   DCHECK(it != model_->pinned_action_ids().cend());
-
-  size_t index = it - model_->pinned_action_ids().cbegin();
-
   ToolbarActionView* extension_view = GetViewForId(*it);
-  data->provider().SetDragImage(GetExtensionIcon(extension_view),
+
+  ui::ImageModel icon = GetExtensionIcon(extension_view);
+  data->provider().SetDragImage(icon.Rasterize(GetColorProvider()),
                                 press_pt.OffsetFromOrigin());
+
   // Fill in the remaining info.
+  size_t index = it - model_->pinned_action_ids().cbegin();
   BrowserActionDragData drag_data(extension_view->view_controller()->GetId(),
                                   index);
   drag_data.Write(browser_->profile(), data);
@@ -809,11 +811,10 @@ size_t ExtensionsToolbarContainer::WidthToIconCount(int x_offset) {
   return std::min(unclamped_count, actions_.size());
 }
 
-gfx::ImageSkia ExtensionsToolbarContainer::GetExtensionIcon(
+ui::ImageModel ExtensionsToolbarContainer::GetExtensionIcon(
     ToolbarActionView* extension_view) {
-  return extension_view->view_controller()
-      ->GetIcon(GetCurrentWebContents(), GetToolbarActionSize())
-      .AsImageSkia();
+  return extension_view->view_controller()->GetIcon(GetCurrentWebContents(),
+                                                    GetToolbarActionSize());
 }
 
 void ExtensionsToolbarContainer::SetExtensionIconVisibility(
@@ -831,8 +832,7 @@ void ExtensionsToolbarContainer::SetExtensionIconVisibility(
 
   extension_view->SetImageModel(
       views::Button::STATE_NORMAL,
-      visible ? ui::ImageModel::FromImageSkia(GetExtensionIcon(extension_view))
-              : ui::ImageModel());
+      visible ? GetExtensionIcon(extension_view) : ui::ImageModel());
 }
 
 void ExtensionsToolbarContainer::UpdateContainerVisibility() {

diff --git a/chrome/browser/ui/views/extensions/extensions_toolbar_container.h b/chrome/browser/ui/views/extensions/extensions_toolbar_container.h
@@ -22,6 +22,7 @@
 #include "chrome/browser/ui/views/toolbar/toolbar_icon_container_view.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 #include "ui/base/metadata/metadata_header_macros.h"
+#include "ui/base/models/image_model.h"
 #include "ui/views/widget/widget_observer.h"
 
 class Browser;
@@ -238,7 +239,7 @@ class ExtensionsToolbarContainer
   // Utility function for going from width to icon counts.
   size_t WidthToIconCount(int x_offset);
 
-  gfx::ImageSkia GetExtensionIcon(ToolbarActionView* extension_view);
+  ui::ImageModel GetExtensionIcon(ToolbarActionView* extension_view);
 
   // Sets a pinned extension button's image to be shown/hidden.
   void SetExtensionIconVisibility(ToolbarActionsModel::ActionId id,

diff --git a/chrome/browser/ui/views/toolbar/toolbar_action_view.cc b/chrome/browser/ui/views/toolbar/toolbar_action_view.cc
@@ -28,14 +28,13 @@
 #include "extensions/common/extension_features.h"
 #include "ui/accessibility/ax_node_data.h"
 #include "ui/base/metadata/metadata_impl_macros.h"
+#include "ui/base/models/image_model.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/base/theme_provider.h"
 #include "ui/compositor/paint_recorder.h"
 #include "ui/events/event.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/image/image_skia.h"
-#include "ui/gfx/image/image_skia_operations.h"
-#include "ui/gfx/image/image_skia_source.h"
 #include "ui/native_theme/native_theme.h"
 #include "ui/views/animation/ink_drop.h"
 #include "ui/views/animation/ink_drop_impl.h"
@@ -152,13 +151,11 @@ void ToolbarActionView::UpdateState() {
   if (!sessions::SessionTabHelper::IdForTab(web_contents).is_valid())
     return;
 
-  gfx::ImageSkia icon(
-      view_controller_->GetIcon(web_contents, GetPreferredSize())
-          .AsImageSkia());
-
-  if (!icon.isNull())
-    SetImageModel(views::Button::STATE_NORMAL,
-                  ui::ImageModel::FromImageSkia(icon));
+  ui::ImageModel icon =
+      view_controller_->GetIcon(web_contents, GetPreferredSize());
+  if (!icon.IsEmpty()) {
+    SetImageModel(views::Button::STATE_NORMAL, icon);
+  }
 
   if (!base::FeatureList::IsEnabled(
           extensions_features::kExtensionsMenuAccessControl)) {