Commit 6534af2b authored by Wei-Yin Chen (陳威尹)'s avatar Wei-Yin Chen (陳威尹) Committed by Commit Bot

Revert "Reland "[PlayBilling] Support app-store billing in AndroidPaymentAppFinder""

This reverts commit 6e35691a.

Reason for revert: errorprone failure https://ci.chromium.org/p/chromium/builders/try/android_compile_dbg/515354

Original change's description:
> Reland "[PlayBilling] Support app-store billing in AndroidPaymentAppFinder"
> 
> Original patch: https://crrev.com/2135152
> Revert: https://crrev.com/2141209
> Reason for revert:
>  Test failure -
>    AndroidPaymentAppFinderTest#testFindAppStoreBillingAppIgnoreNonAppStoreBillingApps
> Reason for reland:
>  The test get fixed.
>  Before: the test expectation assumed a fixed order in Set elements.
>  After: the test doesn't assume it, compare set with set instead.
> 
> Original change's description:
> > Revert "[PlayBilling] Support app-store billing in AndroidPaymentAppFinder"
> >
> > This reverts commit cc5487e9.
> >
> > Reason for revert: testFindAppStoreBillingAppIgnoreNonAppStoreBillingApps is failing consistently on KitKat:
> > https://ci.chromium.org/p/chromium/builders/ci/KitKat%20Phone%20Tester%20%28dbg%29
> >
> > Original change's description:
> > > [PlayBilling] Support app-store billing in AndroidPaymentAppFinder
> > >
> > > After:
> > > When a merchant page runs in a Trusted Web Activity that's installed
> > > from an app store (e.g., Google Play) and the PaymentRequest is not
> > > requesting shipping or payer contact, if it satisfies the following
> > > conditions, the TWA itself would be included:
> > > - the PaymentRequest supports the (TWA installer) app store's
> > > billing method in the payment request.
> > > - the TWA can handle pay intents.
> > > - the TWA can handle the app store billing method.
> > >
> > > Before:
> > > AndroidPaymentAppFinder could not include a TWA for an app store
> > > billing method.
> > >
> > > Change:
> > > * In AndroidPaymentAppFinder#findAndroidPaymentApps, check if the
> > > merchant page is a TWA installed from app store and if the
> > > PaymentRequest is requesting shipping or payer contact. If it is,
> > > add the TWA itself as a payment app if it's eligible.
> > > * remove mIgnoredMethods.
> > >
> > > Note:
> > > * Counterintuitively, the merchant page would send the pay intent to
> > > the TWA instead of the Play Store, because it would be the TWA who
> > > is responsible to interact with the app stores.
> > >
> > > Bug: 1064740
> > >
> > > Change-Id: I0a2f6baebae422aaeab574e3c39a10cd61fafb4a
> > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2135152
> > > Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
> > > Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#757128}
> >
> > TBR=rouslan@chromium.org,maxlg@chromium.org
> >
> > Change-Id: I954a5d5589704b2c8ad13400fcb487eac00bc677
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Bug: 1064740
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2141209
> > Reviewed-by: Patrick Noland <pnoland@chromium.org>
> > Commit-Queue: Patrick Noland <pnoland@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#757427}
> 
> TBR=rouslan@chromium.org,pnoland@chromium.org,maxlg@chromium.org
> 
> Change-Id: Id55022716d27a12c46d1cf4d0166d7bdb7a9ffcf
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 1064740
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2142334
> Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
> Reviewed-by: Liquan (Max) Gu <maxlg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#757456}

TBR=rouslan@chromium.org,pnoland@chromium.org,maxlg@chromium.org

Change-Id: I3470242894219da445ed70238eb21b7e572c027f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1064740
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2142396Reviewed-by: default avatarWei-Yin Chen (陳威尹) <wychen@chromium.org>
Commit-Queue: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757498}
parent 8cf113f1
...@@ -15,6 +15,7 @@ import androidx.annotation.VisibleForTesting; ...@@ -15,6 +15,7 @@ import androidx.annotation.VisibleForTesting;
import org.chromium.base.Log; import org.chromium.base.Log;
import org.chromium.chrome.browser.ChromeActivity; import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.customtabs.CustomTabActivity;
import org.chromium.chrome.browser.payments.PaymentManifestVerifier.ManifestVerifyCallback; import org.chromium.chrome.browser.payments.PaymentManifestVerifier.ManifestVerifyCallback;
import org.chromium.components.payments.MethodStrings; import org.chromium.components.payments.MethodStrings;
import org.chromium.components.payments.PaymentManifestDownloader; import org.chromium.components.payments.PaymentManifestDownloader;
...@@ -22,7 +23,6 @@ import org.chromium.components.payments.PaymentManifestParser; ...@@ -22,7 +23,6 @@ import org.chromium.components.payments.PaymentManifestParser;
import org.chromium.components.payments.intent.WebPaymentIntentHelper; import org.chromium.components.payments.intent.WebPaymentIntentHelper;
import org.chromium.payments.mojom.PaymentDetailsModifier; import org.chromium.payments.mojom.PaymentDetailsModifier;
import org.chromium.payments.mojom.PaymentMethodData; import org.chromium.payments.mojom.PaymentMethodData;
import org.chromium.url.GURL;
import org.chromium.url.URI; import org.chromium.url.URI;
import java.util.ArrayList; import java.util.ArrayList;
...@@ -69,6 +69,12 @@ public class AndroidPaymentAppFinder implements ManifestVerifyCallback { ...@@ -69,6 +69,12 @@ public class AndroidPaymentAppFinder implements ManifestVerifyCallback {
/* package */ static final String META_DATA_NAME_OF_SUPPORTED_DELEGATIONS = /* package */ static final String META_DATA_NAME_OF_SUPPORTED_DELEGATIONS =
"org.chromium.payment_supported_delegations"; "org.chromium.payment_supported_delegations";
/*
* The ignored payment method identifiers. Payment apps with this payment method identifier are
* ignored.
*/
private final Set<String> mIgnoredMethods = new HashSet<>();
private final Set<String> mNonUriPaymentMethods = new HashSet<>(); private final Set<String> mNonUriPaymentMethods = new HashSet<>();
private final Set<URI> mUriPaymentMethods = new HashSet<>(); private final Set<URI> mUriPaymentMethods = new HashSet<>();
private final PaymentManifestDownloader mDownloader; private final PaymentManifestDownloader mDownloader;
...@@ -80,13 +86,10 @@ public class AndroidPaymentAppFinder implements ManifestVerifyCallback { ...@@ -80,13 +86,10 @@ public class AndroidPaymentAppFinder implements ManifestVerifyCallback {
private final boolean mIsIncognito; private final boolean mIsIncognito;
/** /**
* The app stores that supports app-store billing methods. * A map from an app-store app's package name to its billing method. All of the supported
* * app-store billing method must insert an entry to this map.
* key: the app-store app's package name, e.g., "com.google.vendor" (Google Play Store).
* value: the app-store app's billing method identifier, e.g.,
* "https://play.google.com/billing". Only valid GURLs are allowed.
*/ */
private final Map<String, GURL> mAppStores = new HashMap(); private final Map<String, String> mAppStoreBillingMethodMap = new HashMap();
/** /**
* A mapping from an Android package name to the payment app with that package name. The apps * A mapping from an Android package name to the payment app with that package name. The apps
...@@ -170,10 +173,9 @@ public class AndroidPaymentAppFinder implements ManifestVerifyCallback { ...@@ -170,10 +173,9 @@ public class AndroidPaymentAppFinder implements ManifestVerifyCallback {
PaymentAppFactoryInterface factory) { PaymentAppFactoryInterface factory) {
mDelegate = delegate; mDelegate = delegate;
mAppStores.put(PLAY_STORE_PACKAGE_NAME, new GURL(MethodStrings.GOOGLE_PLAY_BILLING)); mIgnoredMethods.add(MethodStrings.GOOGLE_PLAY_BILLING);
for (GURL method : mAppStores.values()) {
assert method.isValid(); mAppStoreBillingMethodMap.put(PLAY_STORE_PACKAGE_NAME, MethodStrings.GOOGLE_PLAY_BILLING);
}
mDownloader = downloader; mDownloader = downloader;
mWebDataService = webDataService; mWebDataService = webDataService;
...@@ -185,62 +187,19 @@ public class AndroidPaymentAppFinder implements ManifestVerifyCallback { ...@@ -185,62 +187,19 @@ public class AndroidPaymentAppFinder implements ManifestVerifyCallback {
mIsIncognito = activity != null && activity.getCurrentTabModel().isIncognito(); mIsIncognito = activity != null && activity.getCurrentTabModel().isIncognito();
} }
private boolean isInTwaInstalledFromAppStore(ChromeActivity activity) { private boolean isInTwaInstalledFromAppStore() {
assert activity != null; ChromeActivity activity =
String twaPackageName = mPackageManagerDelegate.getTwaPackageName(activity); ChromeActivity.fromWebContents(mDelegate.getParams().getWebContents());
if (activity == null) return false;
if (!(activity instanceof CustomTabActivity)) return false;
CustomTabActivity customTabActivity = ((CustomTabActivity) activity);
if (!customTabActivity.isInTwaMode()) return false;
String twaPackageName = customTabActivity.getTwaPackage();
if (twaPackageName == null) return false; if (twaPackageName == null) return false;
String installerPackageName = mPackageManagerDelegate.getInstallerPackage(twaPackageName); String installerPackageName =
activity.getPackageManager().getInstallerPackageName(twaPackageName);
if (installerPackageName == null) return false; if (installerPackageName == null) return false;
return mAppStores.containsKey(installerPackageName); return mAppStoreBillingMethodMap.keySet().contains(installerPackageName);
}
/** Precondition: {@link #isInTwaInstalledFromAppStore} returns true. */
private void findAppStoreBillingApp(
ChromeActivity activity, List<ResolveInfo> allInstalledPaymentApps) {
assert activity != null;
// The following asserts are assumed to have been checked in {@link
// isInTwaInstalledFromAppStore}.
String twaPackageName = mPackageManagerDelegate.getTwaPackageName(activity);
assert twaPackageName != null;
String installerAppStorePackageName =
mPackageManagerDelegate.getInstallerPackage(twaPackageName);
assert installerAppStorePackageName != null;
GURL appStoreBillingUriMethod = mAppStores.get(installerAppStorePackageName);
assert appStoreBillingUriMethod != null;
assert appStoreBillingUriMethod.isValid();
String appStoreBillingMethod = appStoreBillingUriMethod.getSpec();
if (!mDelegate.getParams().getMethodData().containsKey(appStoreBillingMethod)) return;
ResolveInfo twaApp = findAppWithPackageNameAndSupportedMethod(
allInstalledPaymentApps, twaPackageName, appStoreBillingUriMethod);
if (twaApp == null) {
android.util.Log.d(TAG, "The current TWA cannot handle Payment Request.");
return;
}
onValidPaymentAppForPaymentMethodName(twaApp, appStoreBillingMethod);
}
private ResolveInfo findAppWithPackageNameAndSupportedMethod(
List<ResolveInfo> apps, String packageName, GURL uriMethod) {
assert packageName != null;
assert uriMethod != null;
for (int i = 0; i < apps.size(); i++) {
ResolveInfo app = apps.get(i);
String appPackageName = app.activityInfo.packageName;
if (!packageName.equals(appPackageName)) continue;
String defaultMethod = app.activityInfo.metaData == null
? null
: app.activityInfo.metaData.getString(
META_DATA_NAME_OF_DEFAULT_PAYMENT_METHOD_NAME);
GURL defaultUriMethod = new GURL(defaultMethod);
if ((uriMethod.isValid()
&& getSupportedPaymentMethods(app.activityInfo)
.contains(uriMethod.getSpec()))
|| (defaultUriMethod.isValid() && uriMethod.equals(defaultUriMethod))) {
return app;
}
}
return null;
} }
/** /**
...@@ -259,7 +218,7 @@ public class AndroidPaymentAppFinder implements ManifestVerifyCallback { ...@@ -259,7 +218,7 @@ public class AndroidPaymentAppFinder implements ManifestVerifyCallback {
for (String method : mDelegate.getParams().getMethodData().keySet()) { for (String method : mDelegate.getParams().getMethodData().keySet()) {
assert !TextUtils.isEmpty(method); assert !TextUtils.isEmpty(method);
if (mAppStores.containsValue(new GURL(method))) continue; if (mIgnoredMethods.contains(method)) continue;
if (supportedNonUriPaymentMethods.contains(method)) { if (supportedNonUriPaymentMethods.contains(method)) {
mNonUriPaymentMethods.add(method); mNonUriPaymentMethods.add(method);
} else if (UriUtils.looksLikeUriMethod(method)) { } else if (UriUtils.looksLikeUriMethod(method)) {
...@@ -286,17 +245,9 @@ public class AndroidPaymentAppFinder implements ManifestVerifyCallback { ...@@ -286,17 +245,9 @@ public class AndroidPaymentAppFinder implements ManifestVerifyCallback {
} }
} }
// WebContents is possible to attach to different activities on {@link PaymentRequest} if (isInTwaInstalledFromAppStore()) {
// created and shown. Ideally {@link #findAppStoreBillingApp} should have based on the // TODO(crbug.com/1064740): the finder would special-case the TWA installed from App
// activity that is used when PaymentRequest is shown. But we intentionally not do that for // Store to return only the app-store app.
// the sake of simple design and better performance. Plus, for app store billing case in
// particular, it's unusual for a TWA to switch to CCT without destroying JavaScript context
// and, consequently, the {@link PaymentRequest} object.
ChromeActivity activity =
ChromeActivity.fromWebContents(mDelegate.getParams().getWebContents());
if (!mDelegate.getParams().requestShippingOrPayerContact() && activity != null
&& isInTwaInstalledFromAppStore(activity)) {
findAppStoreBillingApp(activity, allInstalledPaymentApps);
} }
// All URI methods for which manifests should be downloaded. For example, if merchant // All URI methods for which manifests should be downloaded. For example, if merchant
...@@ -695,14 +646,14 @@ public class AndroidPaymentAppFinder implements ManifestVerifyCallback { ...@@ -695,14 +646,14 @@ public class AndroidPaymentAppFinder implements ManifestVerifyCallback {
} }
/** /**
* Add an app store for testing. * Ignores the given payment method identifier, so no Android payment apps for this method are
* looked up in findAndroidPaymentApps(). Calling this multiple times will union the new payment
* methods with the existing set.
* *
* @param packageName The package name of the app store. * @param ignoredPaymentMethodIdentifier The ignored payment method identifier.
* @param paymentMethod The payment method identifier of the app store.
*/ */
@VisibleForTesting @VisibleForTesting
/* package */ void addAppStoreForTest(String packageName, GURL paymentMethod) { /* package */ void ignorePaymentMethodForTest(String ignoredPaymentMethodIdentifier) {
assert paymentMethod.isValid(); mIgnoredMethods.add(ignoredPaymentMethodIdentifier);
mAppStores.put(packageName, paymentMethod);
} }
} }
...@@ -20,8 +20,6 @@ import androidx.annotation.Nullable; ...@@ -20,8 +20,6 @@ import androidx.annotation.Nullable;
import org.chromium.base.ContextUtils; import org.chromium.base.ContextUtils;
import org.chromium.base.PackageManagerUtils; import org.chromium.base.PackageManagerUtils;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.customtabs.CustomTabActivity;
import java.util.List; import java.util.List;
...@@ -125,31 +123,4 @@ public class PackageManagerDelegate { ...@@ -125,31 +123,4 @@ public class PackageManagerDelegate {
} }
return resources == null ? null : resources.getStringArray(resourceId); return resources == null ? null : resources.getStringArray(resourceId);
} }
/**
* Get the package name of an activity if it is a Trusted Web Activity.
* @param activity An activity that is intended to check whether its a Trusted Web Activity and
* get the package name from. Not allowed to be null.
* @return The package name of a given activity if it is a Trusted Web Activity; null otherwise.
*/
@Nullable
public String getTwaPackageName(ChromeActivity activity) {
assert activity != null;
if (!(activity instanceof CustomTabActivity)) return null;
CustomTabActivity customTabActivity = ((CustomTabActivity) activity);
if (!customTabActivity.isInTwaMode()) return null;
return customTabActivity.getTwaPackage();
}
/**
* Get the package name of a specified package's installer app.
* @param packageName The package name of the specified package. Not allowed to be null.
* @return The package name of the installer app.
*/
@Nullable
public String getInstallerPackage(String packageName) {
assert packageName != null;
return ContextUtils.getApplicationContext().getPackageManager().getInstallerPackageName(
packageName);
}
} }
...@@ -95,12 +95,4 @@ public interface PaymentAppFactoryParams { ...@@ -95,12 +95,4 @@ public interface PaymentAppFactoryParams {
default String getTotalAmountCurrency() { default String getTotalAmountCurrency() {
return null; return null;
} }
/**
* @return Whether the PaymentRequest is requesting delegation of either shipping or payer
* contact.
*/
default boolean requestShippingOrPayerContact() {
return false;
}
} }
...@@ -2622,12 +2622,6 @@ public class PaymentRequestImpl ...@@ -2622,12 +2622,6 @@ public class PaymentRequestImpl
return mRawTotal.amount.currency; return mRawTotal.amount.currency;
} }
// PaymentAppFactoryParams implementation.
@Override
public boolean requestShippingOrPayerContact() {
return mRequestShipping || mRequestPayerName || mRequestPayerPhone || mRequestPayerEmail;
}
// PaymentAppFactoryDelegate implementation. // PaymentAppFactoryDelegate implementation.
@Override @Override
public PaymentAppFactoryParams getParams() { public PaymentAppFactoryParams getParams() {
......
...@@ -16,8 +16,6 @@ import android.os.Bundle; ...@@ -16,8 +16,6 @@ import android.os.Bundle;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import org.chromium.chrome.browser.ChromeActivity;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.HashMap; import java.util.HashMap;
import java.util.List; import java.util.List;
...@@ -33,10 +31,6 @@ class MockPackageManagerDelegate extends PackageManagerDelegate { ...@@ -33,10 +31,6 @@ class MockPackageManagerDelegate extends PackageManagerDelegate {
private final List<ResolveInfo> mServices = new ArrayList<>(); private final List<ResolveInfo> mServices = new ArrayList<>();
private final Map<ApplicationInfo, String[]> mResources = new HashMap<>(); private final Map<ApplicationInfo, String[]> mResources = new HashMap<>();
private String mMockTwaPackage;
// A map of a package name to its installer's package name.
private Map<String, String> mMockInstallerPackageMap = new HashMap<>();
/** /**
* Simulates an installed payment app with no supported delegations. * Simulates an installed payment app with no supported delegations.
* *
...@@ -146,27 +140,6 @@ class MockPackageManagerDelegate extends PackageManagerDelegate { ...@@ -146,27 +140,6 @@ class MockPackageManagerDelegate extends PackageManagerDelegate {
mLabels.clear(); mLabels.clear();
} }
/**
* Mock the current package to be a Trust Web Activity package.
* @param mockTwaPackage The intended package nam, not allowed to be null.
*/
public void setMockTrustedWebActivity(String mockTwaPackage) {
assert mockTwaPackage != null;
mMockTwaPackage = mockTwaPackage;
}
/**
* Mock the installer of a specified package.
* @param packageName The package name that is intended to mock a installer for.
* @param installerPackageName The package name intended to be set as the installer of the
* specified package. not allowed to be null.
*/
public void mockInstallerForPackage(String packageName, String installerPackageName) {
assert installerPackageName != null;
assert packageName != null;
mMockInstallerPackageMap.put(packageName, installerPackageName);
}
@Override @Override
public List<ResolveInfo> getActivitiesThatCanRespondToIntentWithMetaData(Intent intent) { public List<ResolveInfo> getActivitiesThatCanRespondToIntentWithMetaData(Intent intent) {
return mActivities; return mActivities;
...@@ -204,17 +177,4 @@ class MockPackageManagerDelegate extends PackageManagerDelegate { ...@@ -204,17 +177,4 @@ class MockPackageManagerDelegate extends PackageManagerDelegate {
assert STRING_ARRAY_RESOURCE_ID == resourceId; assert STRING_ARRAY_RESOURCE_ID == resourceId;
return mResources.get(applicationInfo); return mResources.get(applicationInfo);
} }
@Override
@Nullable
public String getInstallerPackage(String packageName) {
return !mMockInstallerPackageMap.isEmpty() ? mMockInstallerPackageMap.get(packageName)
: super.getInstallerPackage(packageName);
}
@Override
@Nullable
public String getTwaPackageName(ChromeActivity activity) {
return mMockTwaPackage != null ? mMockTwaPackage : super.getTwaPackageName(activity);
}
} }
\ No newline at end of file
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