Skip to content

Commit

Permalink
[basic-card] Remove basic-card feature flag
Browse files Browse the repository at this point in the history 
This removes the last uses of the basic-card feature flag and removes
the flag from the codebase. There is still basic-card code to be removed
after this CL, which has been left dead from previous removals.

Bug: 1209835
Change-Id: I0da220f6a6f32b91b6a4a55629535fa95965008c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3828505
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Auto-Submit: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1035174}
  • Loading branch information
@stephenmcgruer
stephenmcgruer authored and Chromium LUCI CQ committed Aug 15, 2022
1 parent c55836c commit b5bcb1c
Show file tree
Hide file tree
Showing 13 changed files with 15 additions and 236 deletions.
4 changes: 0 additions & 4 deletions chrome/browser/about_flags.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4938,10 +4938,6 @@ const FeatureEntry kFeatureEntries[] = {
{"web-payment-api-csp", flag_descriptions::kWebPaymentAPICSPName,
flag_descriptions::kWebPaymentAPICSPDescription, kOsAll,
FEATURE_VALUE_TYPE(features::kWebPaymentAPICSP)},
{"enable-payment-request-basic-card",
flag_descriptions::kPaymentRequestBasicCardName,
flag_descriptions::kPaymentRequestBasicCardDescription, kOsAll,
FEATURE_VALUE_TYPE(features::kPaymentRequestBasicCard)},
{"identity-in-can-make-payment",
flag_descriptions::kIdentityInCanMakePaymentEventFeatureName,
flag_descriptions::kIdentityInCanMakePaymentEventFeatureDescription,
Expand Down
17 changes: 0 additions & 17 deletions ...ts/payments/content/android/java/src/org/chromium/components/payments/BasicCardUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,22 +73,5 @@ public static Map<String, Integer> getNetworkIdentifiers() {
return networksByString;
}

/** @return True if the merchant methodDataMap supports basic card payment method. */
public static boolean merchantSupportsBasicCard(Map<String, PaymentMethodData> methodDataMap) {
if (!PaymentFeatureList.isEnabled(PaymentFeatureList.PAYMENT_REQUEST_BASIC_CARD)) {
return false;
}
assert methodDataMap != null;
PaymentMethodData basicCardData = methodDataMap.get(MethodStrings.BASIC_CARD);
if (basicCardData != null) {
Set<String> basicCardNetworks = convertBasicCardToNetworks(basicCardData);
if (basicCardNetworks != null && !basicCardNetworks.isEmpty()) return true;
}

// Card issuer networks as payment method names was removed in Chrome 77.
// https://www.chromestatus.com/feature/5725727580225536
return false;
}

private BasicCardUtils() {}
}
1 change: 0 additions & 1 deletion ...ayments/content/android/java/src/org/chromium/components/payments/PaymentFeatureList.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ public class PaymentFeatureList {
"IdentityInCanMakePaymentEventFeature";
public static final String SECURE_PAYMENT_CONFIRMATION = "SecurePaymentConfirmationBrowser";
public static final String SERVICE_WORKER_PAYMENT_APPS = "ServiceWorkerPaymentApps";
public static final String PAYMENT_REQUEST_BASIC_CARD = "PaymentRequestBasicCard";
public static final String WEB_PAYMENTS = "WebPayments";
public static final String WEB_PAYMENTS_APP_STORE_BILLING = "AppStoreBilling";
public static final String WEB_PAYMENTS_APP_STORE_BILLING_DEBUG = "AppStoreBillingDebug";
Expand Down
3 changes: 0 additions & 3 deletions ...ents/content/android/java/src/org/chromium/components/payments/PaymentRequestService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -684,9 +684,6 @@ private void logRequestedMethods(Map<String, PaymentMethodData> methodDataMap) {
methodTypes.add(PaymentMethodCategory.OTHER);
}
}
if (BasicCardUtils.merchantSupportsBasicCard(methodDataMap)) {
methodTypes.add(PaymentMethodCategory.BASIC_CARD);
}

mJourneyLogger.setRequestedPaymentMethods(methodTypes);
}
Expand Down
2 changes: 0 additions & 2 deletions ...oid/junit/src/org/chromium/components/payments/test_support/ShadowPaymentFeatureList.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ public static void setDefaultStatuses() {
PaymentFeatureList.WEB_PAYMENTS_EXPERIMENTAL_FEATURES, true);
ShadowPaymentFeatureList.setFeatureEnabled(
PaymentFeatureList.SECURE_PAYMENT_CONFIRMATION, true);
ShadowPaymentFeatureList.setFeatureEnabled(
PaymentFeatureList.PAYMENT_REQUEST_BASIC_CARD, true);
ShadowPaymentFeatureList.setFeatureEnabled(
PaymentFeatureList.IDENTITY_IN_CAN_MAKE_PAYMENT_EVENT_FEATURE, true);
}
Expand Down
1 change: 0 additions & 1 deletion components/payments/content/android/payment_feature_list.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ namespace {
// or the .h file (for Android only features).
const base::Feature* const kFeaturesExposedToJava[] = {
&::features::kIdentityInCanMakePaymentEventFeature,
&::features::kPaymentRequestBasicCard,
&::features::kSecurePaymentConfirmation,
&::features::kServiceWorkerPaymentApps,
&::features::kWebPayments,
Expand Down
47 changes: 13 additions & 34 deletions components/payments/content/payment_request_spec_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ namespace payments {
using ::testing::ElementsAre;
using ::testing::UnorderedElementsAre;

class PaymentRequestSpecTestBase : public testing::Test,
public PaymentRequestSpec::Observer {
class PaymentRequestSpecTest : public testing::Test,
public PaymentRequestSpec::Observer {
protected:
~PaymentRequestSpecTestBase() override = default;
~PaymentRequestSpecTest() override = default;

void OnSpecUpdated() override { on_spec_updated_called_ = true; }

Expand All @@ -51,29 +51,12 @@ class PaymentRequestSpecTestBase : public testing::Test,
private:
std::unique_ptr<PaymentRequestSpec> spec_;
bool on_spec_updated_called_ = false;
base::WeakPtrFactory<PaymentRequestSpecTestBase> weak_ptr_factory_{this};
};

class PaymentRequestSpecBasiCardEnabledTest
: public PaymentRequestSpecTestBase {
public:
PaymentRequestSpecBasiCardEnabledTest(
const PaymentRequestSpecBasiCardEnabledTest&) = delete;
PaymentRequestSpecBasiCardEnabledTest& operator=(
const PaymentRequestSpecBasiCardEnabledTest&) = delete;

protected:
PaymentRequestSpecBasiCardEnabledTest() {
feature_list_.InitAndEnableFeature(::features::kPaymentRequestBasicCard);
}

private:
base::test::ScopedFeatureList feature_list_;
base::WeakPtrFactory<PaymentRequestSpecTest> weak_ptr_factory_{this};
};

// Test that the last shipping option is selected, even in the case of
// updateWith.
TEST_F(PaymentRequestSpecBasiCardEnabledTest, ShippingOptionsSelection) {
TEST_F(PaymentRequestSpecTest, ShippingOptionsSelection) {
std::vector<mojom::PaymentShippingOptionPtr> shipping_options;
mojom::PaymentShippingOptionPtr option = mojom::PaymentShippingOption::New();
option->id = "option:1";
Expand Down Expand Up @@ -116,8 +99,7 @@ TEST_F(PaymentRequestSpecBasiCardEnabledTest, ShippingOptionsSelection) {

// Test that the last shipping option is selected, even in the case of
// updateWith.
TEST_F(PaymentRequestSpecBasiCardEnabledTest,
ShippingOptionsSelection_NoOptionsAtAll) {
TEST_F(PaymentRequestSpecTest, ShippingOptionsSelection_NoOptionsAtAll) {
// No options are provided at first.
mojom::PaymentOptionsPtr options = mojom::PaymentOptions::New();
options->request_shipping = true;
Expand Down Expand Up @@ -156,7 +138,7 @@ TEST_F(PaymentRequestSpecBasiCardEnabledTest,

// Test that the last shipping option is selected, even in the case of
// updateWith.
TEST_F(PaymentRequestSpecBasiCardEnabledTest, UpdateWithNoShippingOptions) {
TEST_F(PaymentRequestSpecTest, UpdateWithNoShippingOptions) {
std::vector<mojom::PaymentShippingOptionPtr> shipping_options;
mojom::PaymentShippingOptionPtr option = mojom::PaymentShippingOption::New();
option->id = "option:1";
Expand All @@ -183,8 +165,7 @@ TEST_F(PaymentRequestSpecBasiCardEnabledTest, UpdateWithNoShippingOptions) {
EXPECT_TRUE(spec()->selected_shipping_option_error().empty());
}

TEST_F(PaymentRequestSpecBasiCardEnabledTest,
SingleCurrencyWithoutDisplayItems) {
TEST_F(PaymentRequestSpecTest, SingleCurrencyWithoutDisplayItems) {
mojom::PaymentDetailsPtr details = mojom::PaymentDetails::New();
mojom::PaymentItemPtr total = mojom::PaymentItem::New();
mojom::PaymentCurrencyAmountPtr amount = mojom::PaymentCurrencyAmount::New();
Expand All @@ -198,7 +179,7 @@ TEST_F(PaymentRequestSpecBasiCardEnabledTest,
EXPECT_FALSE(spec()->IsMixedCurrency());
}

TEST_F(PaymentRequestSpecBasiCardEnabledTest, SingleCurrencyWithDisplayItems) {
TEST_F(PaymentRequestSpecTest, SingleCurrencyWithDisplayItems) {
mojom::PaymentDetailsPtr details = mojom::PaymentDetails::New();
mojom::PaymentItemPtr total = mojom::PaymentItem::New();
mojom::PaymentCurrencyAmountPtr amount = mojom::PaymentCurrencyAmount::New();
Expand All @@ -221,8 +202,7 @@ TEST_F(PaymentRequestSpecBasiCardEnabledTest, SingleCurrencyWithDisplayItems) {
EXPECT_FALSE(spec()->IsMixedCurrency());
}

TEST_F(PaymentRequestSpecBasiCardEnabledTest,
MultipleCurrenciesWithOneDisplayItem) {
TEST_F(PaymentRequestSpecTest, MultipleCurrenciesWithOneDisplayItem) {
mojom::PaymentDetailsPtr details = mojom::PaymentDetails::New();
mojom::PaymentItemPtr total = mojom::PaymentItem::New();
mojom::PaymentCurrencyAmountPtr amount = mojom::PaymentCurrencyAmount::New();
Expand All @@ -246,8 +226,7 @@ TEST_F(PaymentRequestSpecBasiCardEnabledTest,
EXPECT_TRUE(spec()->IsMixedCurrency());
}

TEST_F(PaymentRequestSpecBasiCardEnabledTest,
MultipleCurrenciesWithTwoDisplayItem) {
TEST_F(PaymentRequestSpecTest, MultipleCurrenciesWithTwoDisplayItem) {
mojom::PaymentDetailsPtr details = mojom::PaymentDetails::New();
mojom::PaymentItemPtr total = mojom::PaymentItem::New();
mojom::PaymentCurrencyAmountPtr amount = mojom::PaymentCurrencyAmount::New();
Expand Down Expand Up @@ -278,7 +257,7 @@ TEST_F(PaymentRequestSpecBasiCardEnabledTest,
EXPECT_TRUE(spec()->IsMixedCurrency());
}

TEST_F(PaymentRequestSpecBasiCardEnabledTest, RetryWithShippingAddressErrors) {
TEST_F(PaymentRequestSpecTest, RetryWithShippingAddressErrors) {
mojom::PaymentOptionsPtr options = mojom::PaymentOptions::New();
options->request_shipping = true;
RecreateSpecWithOptionsAndDetails(std::move(options),
Expand All @@ -305,7 +284,7 @@ TEST_F(PaymentRequestSpecBasiCardEnabledTest, RetryWithShippingAddressErrors) {
EXPECT_TRUE(spec()->has_shipping_address_error());
}

TEST_F(PaymentRequestSpecBasiCardEnabledTest, RetryWithPayerErrors) {
TEST_F(PaymentRequestSpecTest, RetryWithPayerErrors) {
mojom::PaymentOptionsPtr options = mojom::PaymentOptions::New();
options->request_payer_email = true;
options->request_payer_name = true;
Expand Down
35 changes: 1 addition & 34 deletions components/payments/content/service_worker_payment_app_finder.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,47 +43,14 @@
namespace payments {
namespace {

// Returns true if |requested| is empty or contains at least one of the items in
// |capabilities|.
template <typename T>
bool CapabilityMatches(const std::vector<T>& requested,
const std::vector<int32_t>& capabilities) {
if (requested.empty())
return true;
for (const auto& request : requested) {
if (base::Contains(capabilities, static_cast<int32_t>(request)))
return true;
}
return false;
}

// Returns true if the basic-card |capabilities| of the payment app match the
// |request|.
bool BasicCardCapabilitiesMatch(
const std::vector<content::StoredCapabilities>& capabilities,
const mojom::PaymentMethodDataPtr& request) {
for (const auto& capability : capabilities) {
if (CapabilityMatches(request->supported_networks,
capability.supported_card_networks)) {
return true;
}
}
return capabilities.empty() && request->supported_networks.empty();
}

// Returns true if |app| supports at least one of the |requests|.
bool AppSupportsAtLeastOneRequestedMethodData(
const content::StoredPaymentApp& app,
const std::vector<mojom::PaymentMethodDataPtr>& requests) {
for (const auto& enabled_method : app.enabled_methods) {
for (const auto& request : requests) {
if (enabled_method == request->supported_method) {
if (!base::FeatureList::IsEnabled(::features::kPaymentRequestBasicCard))
return true;
if (enabled_method != methods::kBasicCard ||
BasicCardCapabilitiesMatch(app.capabilities, request)) {
return true;
}
return true;
}
}
}
Expand Down
54 changes: 0 additions & 54 deletions components/payments/content/service_worker_payment_app_finder_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

#include "components/payments/content/service_worker_payment_app_finder.h"

#include "base/test/scoped_feature_list.h"
#include "content/public/common/content_features.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/mojom/payments/payment_request.mojom.h"

Expand Down Expand Up @@ -162,56 +160,4 @@ TEST_F(ServiceWorkerPaymentAppFinderTest,
apps.find(0)->second->enabled_methods);
}

class ServiceWorkerPaymentAppFinderBasicCardEnabledTest
: public ServiceWorkerPaymentAppFinderTest {
public:
ServiceWorkerPaymentAppFinderBasicCardEnabledTest(
const ServiceWorkerPaymentAppFinderBasicCardEnabledTest&) = delete;
ServiceWorkerPaymentAppFinderBasicCardEnabledTest& operator=(
const ServiceWorkerPaymentAppFinderBasicCardEnabledTest&) = delete;

protected:
ServiceWorkerPaymentAppFinderBasicCardEnabledTest() {
feature_list_.InitAndEnableFeature(::features::kPaymentRequestBasicCard);
}

private:
base::test::ScopedFeatureList feature_list_;
};

TEST_F(ServiceWorkerPaymentAppFinderBasicCardEnabledTest,
RemoveAppsWithoutMatchingMethodData_NoNetworkCapabilities) {
std::vector<mojom::PaymentMethodDataPtr> requested_methods;
requested_methods.emplace_back(mojom::PaymentMethodData::New());
requested_methods.back()->supported_method = "basic-card";
requested_methods.back()->supported_networks = {
mojom::BasicCardNetwork::AMEX};
content::InstalledPaymentAppsFinder::PaymentApps apps;
apps[0] = std::make_unique<content::StoredPaymentApp>();
apps[0]->enabled_methods = {"basic-card"};

RemoveAppsWithoutMatchingMethodData(requested_methods, &apps);

EXPECT_TRUE(apps.empty());
}

TEST_F(ServiceWorkerPaymentAppFinderBasicCardEnabledTest,
RemoveAppsWithoutMatchingMethodData_NoMatchingNetworkCapabilities) {
std::vector<mojom::PaymentMethodDataPtr> requested_methods;
requested_methods.emplace_back(mojom::PaymentMethodData::New());
requested_methods.back()->supported_method = "basic-card";
requested_methods.back()->supported_networks = {
mojom::BasicCardNetwork::AMEX};
content::InstalledPaymentAppsFinder::PaymentApps apps;
apps[0] = std::make_unique<content::StoredPaymentApp>();
apps[0]->enabled_methods = {"basic-card"};
apps[0]->capabilities.emplace_back();
apps[0]->capabilities.back().supported_card_networks = {
static_cast<int32_t>(mojom::BasicCardNetwork::VISA)};

RemoveAppsWithoutMatchingMethodData(requested_methods, &apps);

EXPECT_TRUE(apps.empty());
}

} // namespace payments
20 changes: 0 additions & 20 deletions content/browser/payments/payment_app_database.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include "content/browser/service_worker/service_worker_context_wrapper.h"
#include "content/browser/service_worker/service_worker_registration.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/common/content_features.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/blink/public/common/manifest/manifest.h"
#include "third_party/blink/public/common/storage_key/storage_key.h"
Expand Down Expand Up @@ -84,10 +83,6 @@ PaymentInstrumentPtr ToPaymentInstrumentForMojo(const std::string& input) {
instrument->icons.emplace_back(icon);
}
instrument->method = instrument_proto.method();
if (base::FeatureList::IsEnabled(::features::kPaymentRequestBasicCard)) {
instrument->stringified_capabilities =
instrument_proto.stringified_capabilities();
}

return instrument;
}
Expand Down Expand Up @@ -745,13 +740,6 @@ void PaymentAppDatabase::DidReadAllPaymentInstruments(
continue;

apps[id]->enabled_methods.emplace_back(instrument_proto.method());
if (base::FeatureList::IsEnabled(::features::kPaymentRequestBasicCard)) {
apps[id]->capabilities.emplace_back(StoredCapabilities());
for (const auto& network : instrument_proto.supported_card_networks()) {
apps[id]->capabilities.back().supported_card_networks.emplace_back(
network);
}
}
}

std::move(callback).Run(std::move(apps));
Expand Down Expand Up @@ -939,14 +927,6 @@ void PaymentAppDatabase::DidFindRegistrationToWritePaymentInstrument(
size_proto->set_height(size.height());
}
}
if (base::FeatureList::IsEnabled(::features::kPaymentRequestBasicCard)) {
instrument_proto.set_stringified_capabilities(
instrument->stringified_capabilities);
for (const auto& network : instrument->supported_networks) {
instrument_proto.add_supported_card_networks(
static_cast<int32_t>(network));
}
}

std::string serialized_instrument;
bool success = instrument_proto.SerializeToString(&serialized_instrument);
Expand Down

0 comments on commit b5bcb1c

Please sign in to comment.