Commit 263b3d04 authored by Rouslan Solomakhin's avatar Rouslan Solomakhin Committed by Commit Bot

[Web Payment] Bitmap app icon.

Before this patch, the C++ payment_app.h interface exposed a
gfx::ImageSkia type icon, which is difficult to pass to Java.

This patch changes payment_app.h interface from gfx::ImageSkia to
SkBitmap.

After this patch, the C++ payment_app.h interface exposes an SkBitmap
type icon, which is easier to pass to Java.

Design: https://bit.ly/cross-platform-pay-app-factory

Bug: 1022512
Change-Id: I506bec37b1bb298452e1ebe51cc4bd1731d8d032
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2212495
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: default avatarLiquan (Max) Gu <maxlg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772078}
parent b892c887
...@@ -233,7 +233,7 @@ CreditCardEditorViewController::CreateHeaderView() { ...@@ -233,7 +233,7 @@ CreditCardEditorViewController::CreateHeaderView() {
std::unique_ptr<views::ImageView> card_icon_view = CreateAppIconView( std::unique_ptr<views::ImageView> card_icon_view = CreateAppIconView(
autofill::data_util::GetPaymentRequestData(autofill_card_network) autofill::data_util::GetPaymentRequestData(autofill_card_network)
.icon_resource_id, .icon_resource_id,
gfx::ImageSkia(), base::UTF8ToUTF16(supported_network), opacity); /*icon_bitmap=*/nullptr, base::UTF8ToUTF16(supported_network), opacity);
card_icon_view->SetImageSize(kCardIconSize); card_icon_view->SetImageSize(kCardIconSize);
// Keep track of this card icon to later adjust opacity. // Keep track of this card icon to later adjust opacity.
......
...@@ -24,6 +24,7 @@ ...@@ -24,6 +24,7 @@
#include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_frame_host.h"
#include "content/public/browser/render_process_host.h" #include "content/public/browser/render_process_host.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
#include "ui/gfx/color_palette.h" #include "ui/gfx/color_palette.h"
#include "ui/gfx/color_utils.h" #include "ui/gfx/color_utils.h"
...@@ -61,7 +62,7 @@ class ReadOnlyOriginView : public views::View { ...@@ -61,7 +62,7 @@ class ReadOnlyOriginView : public views::View {
public: public:
ReadOnlyOriginView(const base::string16& page_title, ReadOnlyOriginView(const base::string16& page_title,
const GURL& origin, const GURL& origin,
gfx::ImageSkia icon_image_skia, const SkBitmap* icon_bitmap,
SkColor background_color, SkColor background_color,
views::ButtonListener* site_settings_listener) { views::ButtonListener* site_settings_listener) {
auto title_origin_container = std::make_unique<views::View>(); auto title_origin_container = std::make_unique<views::View>();
...@@ -114,13 +115,13 @@ class ReadOnlyOriginView : public views::View { ...@@ -114,13 +115,13 @@ class ReadOnlyOriginView : public views::View {
top_level_columns->AddColumn( top_level_columns->AddColumn(
views::GridLayout::LEADING, views::GridLayout::CENTER, 1.0, views::GridLayout::LEADING, views::GridLayout::CENTER, 1.0,
views::GridLayout::ColumnSize::kUsePreferred, 0, 0); views::GridLayout::ColumnSize::kUsePreferred, 0, 0);
const bool has_icon = icon_image_skia.width() && icon_image_skia.height(); const bool has_icon = icon_bitmap && !icon_bitmap->drawsNothing();
float adjusted_width = base::checked_cast<float>(icon_image_skia.width()); float adjusted_width = base::checked_cast<float>(icon_bitmap->width());
if (has_icon) { if (has_icon) {
adjusted_width = adjusted_width =
adjusted_width * adjusted_width *
IconSizeCalculator::kPaymentAppDeviceIndependentIdealIconHeight / IconSizeCalculator::kPaymentAppDeviceIndependentIdealIconHeight /
icon_image_skia.height(); icon_bitmap->height();
// A column for the app icon. // A column for the app icon.
top_level_columns->AddColumn( top_level_columns->AddColumn(
views::GridLayout::LEADING, views::GridLayout::FILL, views::GridLayout::LEADING, views::GridLayout::FILL,
...@@ -134,7 +135,7 @@ class ReadOnlyOriginView : public views::View { ...@@ -134,7 +135,7 @@ class ReadOnlyOriginView : public views::View {
top_level_layout->AddView(std::move(title_origin_container)); top_level_layout->AddView(std::move(title_origin_container));
if (has_icon) { if (has_icon) {
views::ImageView* app_icon_view = top_level_layout->AddView( views::ImageView* app_icon_view = top_level_layout->AddView(
CreateAppIconView(/*icon_id=*/0, icon_image_skia, CreateAppIconView(/*icon_id=*/0, icon_bitmap,
/*label=*/page_title)); /*label=*/page_title));
// We should set image size in density independent pixels here, since // We should set image size in density independent pixels here, since
// views::ImageView objects are rastered at the device scale factor. // views::ImageView objects are rastered at the device scale factor.
...@@ -237,8 +238,7 @@ PaymentHandlerWebFlowViewController::CreateHeaderContentView( ...@@ -237,8 +238,7 @@ PaymentHandlerWebFlowViewController::CreateHeaderContentView(
GetHeaderBackground(header_view); GetHeaderBackground(header_view);
return std::make_unique<ReadOnlyOriginView>( return std::make_unique<ReadOnlyOriginView>(
GetPaymentHandlerDialogTitle(web_contents()), origin, GetPaymentHandlerDialogTitle(web_contents()), origin,
state()->selected_app()->icon_image_skia(), background->get_color(), state()->selected_app()->icon_bitmap(), background->get_color(), this);
this);
} }
std::unique_ptr<views::Background> std::unique_ptr<views::Background>
......
...@@ -101,7 +101,7 @@ class PaymentMethodListItem : public PaymentRequestItemList::Item { ...@@ -101,7 +101,7 @@ class PaymentMethodListItem : public PaymentRequestItemList::Item {
// PaymentRequestItemList::Item: // PaymentRequestItemList::Item:
std::unique_ptr<views::View> CreateExtraView() override { std::unique_ptr<views::View> CreateExtraView() override {
std::unique_ptr<views::ImageView> icon_view = CreateAppIconView( std::unique_ptr<views::ImageView> icon_view = CreateAppIconView(
app_->icon_resource_id(), app_->icon_image_skia(), app_->GetLabel()); app_->icon_resource_id(), app_->icon_bitmap(), app_->GetLabel());
return icon_view; return icon_view;
} }
......
...@@ -231,13 +231,16 @@ void PopulateSheetHeaderView(bool show_back_arrow, ...@@ -231,13 +231,16 @@ void PopulateSheetHeaderView(bool show_back_arrow,
std::unique_ptr<views::ImageView> CreateAppIconView( std::unique_ptr<views::ImageView> CreateAppIconView(
int icon_resource_id, int icon_resource_id,
gfx::ImageSkia img, const SkBitmap* icon_bitmap,
const base::string16& tooltip_text, const base::string16& tooltip_text,
float opacity) { float opacity) {
std::unique_ptr<views::ImageView> icon_view = std::unique_ptr<views::ImageView> icon_view =
std::make_unique<views::ImageView>(); std::make_unique<views::ImageView>();
icon_view->set_can_process_events_within_subtree(false); icon_view->set_can_process_events_within_subtree(false);
if (!img.isNull() || !icon_resource_id) { if (icon_bitmap || !icon_resource_id) {
gfx::ImageSkia img = gfx::ImageSkia::CreateFrom1xBitmap(
(icon_bitmap ? *icon_bitmap : SkBitmap()))
.DeepCopy();
icon_view->SetImage(img); icon_view->SetImage(img);
float width = base::checked_cast<float>(img.width()); float width = base::checked_cast<float>(img.width());
float height = base::checked_cast<float>(img.height()); float height = base::checked_cast<float>(img.height());
......
...@@ -78,12 +78,13 @@ void PopulateSheetHeaderView(bool show_back_arrow, ...@@ -78,12 +78,13 @@ void PopulateSheetHeaderView(bool show_back_arrow,
views::View* container, views::View* container,
std::unique_ptr<views::Background> background); std::unique_ptr<views::Background> background);
// Returns an instrument image view for the given |img| or |icon_resource_id| // Returns an instrument image view for the given |icon_bitmap| or
// and wanted |opacity|. Includes a rounded rect border. Callers need to set the // |icon_resource_id| and wanted |opacity|. Includes a rounded rect border.
// size of the resulting ImageView. Callers should set a |tooltip_text|. // Callers need to set the size of the resulting ImageView. Callers should set a
// |tooltip_text|.
std::unique_ptr<views::ImageView> CreateAppIconView( std::unique_ptr<views::ImageView> CreateAppIconView(
int icon_resource_id, int icon_resource_id,
gfx::ImageSkia img, const SkBitmap* icon_bitmap,
const base::string16& tooltip_text, const base::string16& tooltip_text,
float opacity = 1.0f); float opacity = 1.0f);
......
...@@ -786,7 +786,7 @@ PaymentSheetViewController::CreatePaymentMethodRow() { ...@@ -786,7 +786,7 @@ PaymentSheetViewController::CreatePaymentMethodRow() {
layout->AddView(std::move(selected_app_sublabel)); layout->AddView(std::move(selected_app_sublabel));
std::unique_ptr<views::ImageView> icon_view = CreateAppIconView( std::unique_ptr<views::ImageView> icon_view = CreateAppIconView(
selected_app->icon_resource_id(), selected_app->icon_image_skia(), selected_app->icon_resource_id(), selected_app->icon_bitmap(),
selected_app->GetLabel()); selected_app->GetLabel());
return builder.AccessibleContent(selected_app->GetLabel()) return builder.AccessibleContent(selected_app->GetLabel())
......
...@@ -47,8 +47,8 @@ PaymentApp::PaymentApp(int icon_resource_id, Type type) ...@@ -47,8 +47,8 @@ PaymentApp::PaymentApp(int icon_resource_id, Type type)
PaymentApp::~PaymentApp() {} PaymentApp::~PaymentApp() {}
gfx::ImageSkia PaymentApp::icon_image_skia() const { const SkBitmap* PaymentApp::icon_bitmap() const {
return gfx::ImageSkia(); return nullptr;
} }
void PaymentApp::IsValidForPaymentMethodIdentifier( void PaymentApp::IsValidForPaymentMethodIdentifier(
......
...@@ -18,7 +18,7 @@ ...@@ -18,7 +18,7 @@
#include "components/payments/core/payer_data.h" #include "components/payments/core/payer_data.h"
#include "services/metrics/public/cpp/ukm_source_id.h" #include "services/metrics/public/cpp/ukm_source_id.h"
#include "third_party/blink/public/mojom/payments/payment_request.mojom.h" #include "third_party/blink/public/mojom/payments/payment_request.mojom.h"
#include "ui/gfx/image/image_skia.h" #include "third_party/skia/include/core/SkBitmap.h"
namespace payments { namespace payments {
...@@ -74,7 +74,9 @@ class PaymentApp { ...@@ -74,7 +74,9 @@ class PaymentApp {
// Return the sub/label of payment app, to be displayed to the user. // Return the sub/label of payment app, to be displayed to the user.
virtual base::string16 GetLabel() const = 0; virtual base::string16 GetLabel() const = 0;
virtual base::string16 GetSublabel() const = 0; virtual base::string16 GetSublabel() const = 0;
virtual gfx::ImageSkia icon_image_skia() const;
// Returns the icon bitmap or null.
virtual const SkBitmap* icon_bitmap() const;
// Returns true if this payment app can be used to fulfill a request // Returns true if this payment app can be used to fulfill a request
// specifying |method| as supported method of payment. The parsed basic-card // specifying |method| as supported method of payment. The parsed basic-card
......
...@@ -58,15 +58,6 @@ ServiceWorkerPaymentApp::ServiceWorkerPaymentApp( ...@@ -58,15 +58,6 @@ ServiceWorkerPaymentApp::ServiceWorkerPaymentApp(
app_method_names_.insert(stored_payment_app_info_->enabled_methods.begin(), app_method_names_.insert(stored_payment_app_info_->enabled_methods.begin(),
stored_payment_app_info_->enabled_methods.end()); stored_payment_app_info_->enabled_methods.end());
if (stored_payment_app_info_->icon) {
icon_image_ =
gfx::ImageSkia::CreateFrom1xBitmap(*(stored_payment_app_info_->icon))
.DeepCopy();
} else {
// Create an empty icon image to avoid using invalid icon resource id.
icon_image_ = gfx::ImageSkia::CreateFrom1xBitmap(SkBitmap()).DeepCopy();
}
} }
// Service worker payment app provides icon through bitmap, so set 0 as invalid // Service worker payment app provides icon through bitmap, so set 0 as invalid
...@@ -101,15 +92,6 @@ ServiceWorkerPaymentApp::ServiceWorkerPaymentApp( ...@@ -101,15 +92,6 @@ ServiceWorkerPaymentApp::ServiceWorkerPaymentApp(
DCHECK(spec_); DCHECK(spec_);
app_method_names_.insert(installable_enabled_method_); app_method_names_.insert(installable_enabled_method_);
if (installable_web_app_info_->icon) {
icon_image_ =
gfx::ImageSkia::CreateFrom1xBitmap(*(installable_web_app_info_->icon))
.DeepCopy();
} else {
// Create an empty icon image to avoid using invalid icon resource id.
icon_image_ = gfx::ImageSkia::CreateFrom1xBitmap(SkBitmap()).DeepCopy();
}
} }
ServiceWorkerPaymentApp::~ServiceWorkerPaymentApp() { ServiceWorkerPaymentApp::~ServiceWorkerPaymentApp() {
...@@ -391,7 +373,7 @@ uint32_t ServiceWorkerPaymentApp::GetCompletenessScore() const { ...@@ -391,7 +373,7 @@ uint32_t ServiceWorkerPaymentApp::GetCompletenessScore() const {
bool ServiceWorkerPaymentApp::CanPreselect() const { bool ServiceWorkerPaymentApp::CanPreselect() const {
// Do not preselect the payment app when the name and/or icon is missing. // Do not preselect the payment app when the name and/or icon is missing.
return !GetLabel().empty() && !icon_image_.size().IsEmpty(); return !GetLabel().empty() && icon_bitmap() && !icon_bitmap()->drawsNothing();
} }
base::string16 ServiceWorkerPaymentApp::GetMissingInfoLabel() const { base::string16 ServiceWorkerPaymentApp::GetMissingInfoLabel() const {
...@@ -487,8 +469,9 @@ base::WeakPtr<PaymentApp> ServiceWorkerPaymentApp::AsWeakPtr() { ...@@ -487,8 +469,9 @@ base::WeakPtr<PaymentApp> ServiceWorkerPaymentApp::AsWeakPtr() {
return weak_ptr_factory_.GetWeakPtr(); return weak_ptr_factory_.GetWeakPtr();
} }
gfx::ImageSkia ServiceWorkerPaymentApp::icon_image_skia() const { const SkBitmap* ServiceWorkerPaymentApp::icon_bitmap() const {
return icon_image_; return stored_payment_app_info_ ? stored_payment_app_info_->icon.get()
: installable_web_app_info_->icon.get();
} }
bool ServiceWorkerPaymentApp::HandlesShippingAddress() const { bool ServiceWorkerPaymentApp::HandlesShippingAddress() const {
......
...@@ -89,7 +89,7 @@ class ServiceWorkerPaymentApp : public PaymentApp { ...@@ -89,7 +89,7 @@ class ServiceWorkerPaymentApp : public PaymentApp {
bool supported_networks_specified, bool supported_networks_specified,
const std::set<std::string>& supported_networks) const override; const std::set<std::string>& supported_networks) const override;
base::WeakPtr<PaymentApp> AsWeakPtr() override; base::WeakPtr<PaymentApp> AsWeakPtr() override;
gfx::ImageSkia icon_image_skia() const override; const SkBitmap* icon_bitmap() const override;
bool HandlesShippingAddress() const override; bool HandlesShippingAddress() const override;
bool HandlesPayerName() const override; bool HandlesPayerName() const override;
bool HandlesPayerEmail() const override; bool HandlesPayerEmail() const override;
...@@ -126,7 +126,6 @@ class ServiceWorkerPaymentApp : public PaymentApp { ...@@ -126,7 +126,6 @@ class ServiceWorkerPaymentApp : public PaymentApp {
GURL frame_origin_; GURL frame_origin_;
const PaymentRequestSpec* spec_; const PaymentRequestSpec* spec_;
std::unique_ptr<content::StoredPaymentApp> stored_payment_app_info_; std::unique_ptr<content::StoredPaymentApp> stored_payment_app_info_;
gfx::ImageSkia icon_image_;
// Weak pointer is fine here since the owner of this object is // Weak pointer is fine here since the owner of this object is
// PaymentRequestState which also owns PaymentResponseHelper. // PaymentRequestState which also owns PaymentResponseHelper.
......
...@@ -148,9 +148,9 @@ TEST_F(ServiceWorkerPaymentAppTest, AppInfo) { ...@@ -148,9 +148,9 @@ TEST_F(ServiceWorkerPaymentAppTest, AppInfo) {
EXPECT_EQ(base::UTF16ToUTF8(GetApp()->GetLabel()), "bobpay"); EXPECT_EQ(base::UTF16ToUTF8(GetApp()->GetLabel()), "bobpay");
EXPECT_EQ(base::UTF16ToUTF8(GetApp()->GetSublabel()), "bobpay.com"); EXPECT_EQ(base::UTF16ToUTF8(GetApp()->GetSublabel()), "bobpay.com");
const gfx::Size expected_size{icon_bitmap()->width(), ASSERT_NE(nullptr, GetApp()->icon_bitmap());
icon_bitmap()->height()}; EXPECT_EQ(GetApp()->icon_bitmap()->width(), icon_bitmap()->width());
EXPECT_EQ(GetApp()->icon_image_skia().size(), expected_size); EXPECT_EQ(GetApp()->icon_bitmap()->height(), icon_bitmap()->height());
} }
// Test payment request event data can be correctly constructed for invoking // Test payment request event data can be correctly constructed for invoking
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment