Commit f4dc9b1c authored by Danyao Wang's avatar Danyao Wang Committed by Commit Bot

[PaymentRequest][Desktop] Sort JIT payment apps below autofill apps.

Before this CL, autofill payment apps are always ranked below other
types of payment apps, including just-in-time installable (but not yet
installed) service-worker based pyament handler.

This CL introduces a new payment app ranking order behind a feature flag
such ranks payment apps first by types:
1. Installed 3rd-party payment handlers
2. Complete autofill instruments
3. Just-in-time installable 3rd-party payment handlers
4. Incomplete autofill instruments

Within each group, payment apps are sorted by frecency as before.

Bug: 1046875
Change-Id: I34ed0fec1f7b6fc4fbf8c6984d0893c3fad73443
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2028740
Commit-Queue: Danyao Wang <danyao@chromium.org>
Reviewed-by: default avatarSahel Sharify <sahel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737048}
parent c69bf85b
......@@ -405,6 +405,10 @@ void ServiceWorkerPaymentApp::RecordUse() {
NOTIMPLEMENTED();
}
bool ServiceWorkerPaymentApp::NeedsInstallation() const {
return needs_installation_;
}
base::string16 ServiceWorkerPaymentApp::GetLabel() const {
return base::UTF8ToUTF16(needs_installation_
? installable_web_app_info_->name
......
......@@ -84,6 +84,7 @@ class ServiceWorkerPaymentApp : public PaymentApp {
base::string16 GetMissingInfoLabel() const override;
bool IsValidForCanMakePayment() const override;
void RecordUse() override;
bool NeedsInstallation() const override;
base::string16 GetLabel() const override;
base::string16 GetSublabel() const override;
bool IsValidForModifier(
......
......@@ -38,6 +38,8 @@ jumbo_static_library("core") {
"payment_prefs.h",
"payment_shipping_option.cc",
"payment_shipping_option.h",
"payments_experimental_features.cc",
"payments_experimental_features.h",
"payments_validators.cc",
"payments_validators.h",
"url_util.cc",
......@@ -63,8 +65,6 @@ jumbo_static_library("core") {
"payment_request_delegate.h",
"payment_response.cc",
"payment_response.h",
"payments_experimental_features.cc",
"payments_experimental_features.h",
"payments_profile_comparator.cc",
"payments_profile_comparator.h",
"strings_util.cc",
......
......@@ -122,6 +122,11 @@ void AutofillPaymentApp::RecordUse() {
credit_card_);
}
bool AutofillPaymentApp::NeedsInstallation() const {
// Autofill payment app is built-in, so it doesn't need installation.
return false;
}
base::string16 AutofillPaymentApp::GetLabel() const {
return credit_card_.NetworkAndLastFourDigits();
}
......
......@@ -45,6 +45,7 @@ class AutofillPaymentApp
base::string16 GetMissingInfoLabel() const override;
bool IsValidForCanMakePayment() const override;
void RecordUse() override;
bool NeedsInstallation() const override;
base::string16 GetLabel() const override;
base::string16 GetSublabel() const override;
bool IsValidForModifier(
......
......@@ -56,5 +56,8 @@ const base::Feature kPaymentRequestSkipToGPayIfNoCard{
const base::Feature kWebPaymentsMinimalUI{"WebPaymentsMinimalUI",
base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature kDownRankJustInTimePaymentApp{
"DownRankJustInTimePaymentApp", base::FEATURE_DISABLED_BY_DEFAULT};
} // namespace features
} // namespace payments
......@@ -63,6 +63,10 @@ extern const base::Feature kPaymentRequestSkipToGPayIfNoCard;
// Controls whether the minimal payment request ui features are enabled.
extern const base::Feature kWebPaymentsMinimalUI;
// If enabled, just-in-time installable payment handlers are ranked lower than
// complete autofill instruments in payment sheet's method selection section.
extern const base::Feature kDownRankJustInTimePaymentApp;
} // namespace features
} // namespace payments
......
......@@ -8,8 +8,40 @@
#include "components/autofill/core/common/autofill_clock.h"
#include "components/payments/core/autofill_payment_app.h"
#include "components/payments/core/features.h"
#include "components/payments/core/payments_experimental_features.h"
namespace payments {
namespace {
// Returns the sorting group of a payment app. This is used to order payment
// apps in the order of:
// 1. Installed 3rd-party payment handlers
// 2. Complete autofill instruments
// 3. Just-in-time installable payment handlers that is not yet installed.
// 4. Incomplete autofill instruments
int GetSortingGroup(const PaymentApp& app) {
switch (app.type()) {
case PaymentApp::Type::SERVICE_WORKER_APP:
case PaymentApp::Type::NATIVE_MOBILE_APP:
// If the experimental feature is enabled, sort 3rd-party payment handlers
// that needs installation below autofill instruments.
if (app.NeedsInstallation() &&
PaymentsExperimentalFeatures::IsEnabled(
features::kDownRankJustInTimePaymentApp)) {
return 3;
}
return 1;
break;
case PaymentApp::Type::AUTOFILL:
if (app.IsCompleteForPayment()) {
return 2;
} else {
return 4;
}
}
}
} // namespace
PaymentApp::PaymentApp(int icon_resource_id, Type type)
: icon_resource_id_(icon_resource_id), type_(type) {}
......@@ -47,12 +79,15 @@ void PaymentApp::SortApps(std::vector<PaymentApp*>* apps) {
}
bool PaymentApp::operator<(const PaymentApp& other) const {
// Non-autofill apps before autofill.
if (type_ == Type::AUTOFILL && other.type() != Type::AUTOFILL)
return false;
if (type_ != Type::AUTOFILL && other.type() == Type::AUTOFILL)
return true;
int sorting_group = GetSortingGroup(*this);
int other_sorting_group = GetSortingGroup(other);
// First sort payment apps by their sorting group.
if (sorting_group != other_sorting_group) {
return sorting_group < other_sorting_group;
}
// Within a group, compare by completeness.
// Non-autofill apps have max completeness score. Autofill cards are sorted
// based on completeness. (Each autofill card is considered an app.)
int completeness = GetCompletenessScore() - other.GetCompletenessScore();
......
......@@ -66,6 +66,8 @@ class PaymentApp {
virtual bool IsValidForCanMakePayment() const = 0;
// Records the use of this payment app.
virtual void RecordUse() = 0;
// Check whether this payment app needs installation before it can be used.
virtual bool NeedsInstallation() const = 0;
// Return the sub/label of payment app, to be displayed to the user.
virtual base::string16 GetLabel() const = 0;
virtual base::string16 GetSublabel() const = 0;
......
......@@ -56,6 +56,7 @@ class IOSPaymentInstrument : public PaymentApp {
base::string16 GetMissingInfoLabel() const override;
bool IsValidForCanMakePayment() const override;
void RecordUse() override;
bool NeedsInstallation() const override;
base::string16 GetLabel() const override;
base::string16 GetSublabel() const override;
bool IsValidForModifier(
......
......@@ -90,6 +90,10 @@ void IOSPaymentInstrument::RecordUse() {
// TODO(crbug.com/60266): Record the use of the native payment app.
}
bool IOSPaymentInstrument::NeedsInstallation() const {
return false;
}
base::string16 IOSPaymentInstrument::GetLabel() const {
return base::ASCIIToUTF16(app_name_);
}
......
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