Commit 3e70616d authored by Liquan (Max) Gu's avatar Liquan (Max) Gu Committed by Commit Bot

[WebLayer] Remove PaymentRequestService#Delegate#isWebContentsActive

Context:
The isWebContentsActive() method used for testing purpose, but it's
always set to true in tests, so this CL removes the method. Going
forwards, if there's a need to create "inactive" WebContents in tests,
we should fabricate an inactive WebContents instead.

Bug: 1128658
Change-Id: Idc22ee0471c960781554418ce2035fa25b7c4695
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2500569Reviewed-by: default avatarSahel Sharify <sahel@chromium.org>
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821809}
parent f5fbe5a2
...@@ -17,7 +17,6 @@ import org.chromium.components.payments.PaymentRequestService; ...@@ -17,7 +17,6 @@ import org.chromium.components.payments.PaymentRequestService;
import org.chromium.components.user_prefs.UserPrefs; import org.chromium.components.user_prefs.UserPrefs;
import org.chromium.content_public.browser.FeaturePolicyFeature; import org.chromium.content_public.browser.FeaturePolicyFeature;
import org.chromium.content_public.browser.RenderFrameHost; import org.chromium.content_public.browser.RenderFrameHost;
import org.chromium.content_public.browser.Visibility;
import org.chromium.content_public.browser.WebContents; import org.chromium.content_public.browser.WebContents;
import org.chromium.content_public.browser.WebContentsStatics; import org.chromium.content_public.browser.WebContentsStatics;
import org.chromium.mojo.system.MojoException; import org.chromium.mojo.system.MojoException;
...@@ -84,14 +83,6 @@ public class ChromePaymentRequestFactory implements InterfaceFactory<PaymentRequ ...@@ -84,14 +83,6 @@ public class ChromePaymentRequestFactory implements InterfaceFactory<PaymentRequ
return SslValidityChecker.getInvalidSslCertificateErrorMessage(liveWebContents); return SslValidityChecker.getInvalidSslCertificateErrorMessage(liveWebContents);
} }
@Override
public boolean isWebContentsActive() {
// TODO(crbug.com/1128658): Try making the WebContents inactive for instrumentation
// tests rather than mocking it with this method.
WebContents liveWebContents = getLiveWebContents();
return liveWebContents != null && liveWebContents.getVisibility() == Visibility.VISIBLE;
}
@Override @Override
public boolean prefsCanMakePayment() { public boolean prefsCanMakePayment() {
// TODO(crbug.com/1128658): Try replacing Profile with BrowserContextHandle, which // TODO(crbug.com/1128658): Try replacing Profile with BrowserContextHandle, which
......
...@@ -58,7 +58,9 @@ import org.chromium.components.payments.Section; ...@@ -58,7 +58,9 @@ import org.chromium.components.payments.Section;
import org.chromium.components.payments.SkipToGPayHelper; import org.chromium.components.payments.SkipToGPayHelper;
import org.chromium.components.payments.UrlUtil; import org.chromium.components.payments.UrlUtil;
import org.chromium.content_public.browser.RenderFrameHost; import org.chromium.content_public.browser.RenderFrameHost;
import org.chromium.content_public.browser.Visibility;
import org.chromium.content_public.browser.WebContents; import org.chromium.content_public.browser.WebContents;
import org.chromium.content_public.browser.WebContentsStatics;
import org.chromium.payments.mojom.CanMakePaymentQueryResult; import org.chromium.payments.mojom.CanMakePaymentQueryResult;
import org.chromium.payments.mojom.HasEnrolledInstrumentQueryResult; import org.chromium.payments.mojom.HasEnrolledInstrumentQueryResult;
import org.chromium.payments.mojom.PayerDetail; import org.chromium.payments.mojom.PayerDetail;
...@@ -354,10 +356,22 @@ public class ChromePaymentRequestService ...@@ -354,10 +356,22 @@ public class ChromePaymentRequestService
return true; return true;
} }
@Nullable
private WebContents getLiveWebContents() {
WebContents webContents = WebContentsStatics.fromRenderFrameHost(mRenderFrameHost);
return webContents != null && !webContents.isDestroyed() ? webContents : null;
}
private boolean isWebContentsActive() {
WebContents webContents = getLiveWebContents();
return webContents != null && webContents.getVisibility() == Visibility.VISIBLE;
}
/** @return Whether the UI was built. */ /** @return Whether the UI was built. */
private boolean buildUI(ChromeActivity activity) { private boolean buildUI(ChromeActivity activity) {
WebContents webContents = getLiveWebContents();
String error = mPaymentUiService.buildPaymentRequestUI(activity, String error = mPaymentUiService.buildPaymentRequestUI(activity,
/*isWebContentsActive=*/mDelegate.isWebContentsActive(), /*isWebContentsActive=*/isWebContentsActive(),
/*waitForUpdatedDetails=*/mWaitForUpdatedDetails); /*waitForUpdatedDetails=*/mWaitForUpdatedDetails);
if (error != null) { if (error != null) {
mJourneyLogger.setNotShown(NotShownReason.OTHER); mJourneyLogger.setNotShown(NotShownReason.OTHER);
......
...@@ -37,9 +37,8 @@ public class PaymentRequestTestBridge { ...@@ -37,9 +37,8 @@ public class PaymentRequestTestBridge {
private final String mTwaPackageName; private final String mTwaPackageName;
ChromePaymentRequestDelegateForTest(boolean isOffTheRecord, boolean isValidSsl, ChromePaymentRequestDelegateForTest(boolean isOffTheRecord, boolean isValidSsl,
boolean isWebContentsActive, boolean prefsCanMakePayment, String twaPackageName, boolean prefsCanMakePayment, String twaPackageName, boolean skipUiForBasicCard) {
boolean skipUiForBasicCard) { super(isOffTheRecord, isValidSsl, prefsCanMakePayment);
super(isOffTheRecord, isValidSsl, isWebContentsActive, prefsCanMakePayment);
mSkipUiForBasicCard = skipUiForBasicCard; mSkipUiForBasicCard = skipUiForBasicCard;
mTwaPackageName = twaPackageName; mTwaPackageName = twaPackageName;
} }
...@@ -64,14 +63,12 @@ public class PaymentRequestTestBridge { ...@@ -64,14 +63,12 @@ public class PaymentRequestTestBridge {
private static class PaymentRequestDelegateForTest implements PaymentRequestService.Delegate { private static class PaymentRequestDelegateForTest implements PaymentRequestService.Delegate {
private final boolean mIsOffTheRecord; private final boolean mIsOffTheRecord;
private final boolean mIsValidSsl; private final boolean mIsValidSsl;
private final boolean mIsWebContentsActive;
private final boolean mPrefsCanMakePayment; private final boolean mPrefsCanMakePayment;
PaymentRequestDelegateForTest(boolean isOffTheRecord, boolean isValidSsl, PaymentRequestDelegateForTest(
boolean isWebContentsActive, boolean prefsCanMakePayment) { boolean isOffTheRecord, boolean isValidSsl, boolean prefsCanMakePayment) {
mIsOffTheRecord = isOffTheRecord; mIsOffTheRecord = isOffTheRecord;
mIsValidSsl = isValidSsl; mIsValidSsl = isValidSsl;
mIsWebContentsActive = isWebContentsActive;
mPrefsCanMakePayment = prefsCanMakePayment; mPrefsCanMakePayment = prefsCanMakePayment;
} }
...@@ -86,11 +83,6 @@ public class PaymentRequestTestBridge { ...@@ -86,11 +83,6 @@ public class PaymentRequestTestBridge {
return "Invalid SSL certificate"; return "Invalid SSL certificate";
} }
@Override
public boolean isWebContentsActive() {
return mIsWebContentsActive;
}
@Override @Override
public boolean prefsCanMakePayment() { public boolean prefsCanMakePayment() {
return mPrefsCanMakePayment; return mPrefsCanMakePayment;
...@@ -210,12 +202,12 @@ public class PaymentRequestTestBridge { ...@@ -210,12 +202,12 @@ public class PaymentRequestTestBridge {
@CalledByNative @CalledByNative
private static void setUseDelegateForTest(boolean useDelegate, boolean isOffTheRecord, private static void setUseDelegateForTest(boolean useDelegate, boolean isOffTheRecord,
boolean isValidSsl, boolean isWebContentsActive, boolean prefsCanMakePayment, boolean isValidSsl, boolean prefsCanMakePayment, boolean skipUiForBasicCard,
boolean skipUiForBasicCard, String twaPackageName) { String twaPackageName) {
if (useDelegate) { if (useDelegate) {
ChromePaymentRequestFactory.sDelegateForTest = new ChromePaymentRequestDelegateForTest( ChromePaymentRequestFactory.sDelegateForTest =
isOffTheRecord, isValidSsl, isWebContentsActive, prefsCanMakePayment, new ChromePaymentRequestDelegateForTest(isOffTheRecord, isValidSsl,
twaPackageName, skipUiForBasicCard); prefsCanMakePayment, twaPackageName, skipUiForBasicCard);
} else { } else {
ChromePaymentRequestFactory.sDelegateForTest = null; ChromePaymentRequestFactory.sDelegateForTest = null;
} }
......
...@@ -16,14 +16,13 @@ void SetUseDelegateOnPaymentRequestForTesting( ...@@ -16,14 +16,13 @@ void SetUseDelegateOnPaymentRequestForTesting(
bool use_delegate, bool use_delegate,
bool is_incognito, bool is_incognito,
bool is_valid_ssl, bool is_valid_ssl,
bool is_web_contents_active,
bool prefs_can_make_payment, bool prefs_can_make_payment,
bool skip_ui_for_basic_card, bool skip_ui_for_basic_card,
const std::string& twa_package_name) { const std::string& twa_package_name) {
JNIEnv* env = base::android::AttachCurrentThread(); JNIEnv* env = base::android::AttachCurrentThread();
Java_PaymentRequestTestBridge_setUseDelegateForTest( Java_PaymentRequestTestBridge_setUseDelegateForTest(
env, use_delegate, is_incognito, is_valid_ssl, is_web_contents_active, env, use_delegate, is_incognito, is_valid_ssl, prefs_can_make_payment,
prefs_can_make_payment, skip_ui_for_basic_card, skip_ui_for_basic_card,
base::android::ConvertUTF8ToJavaString(env, twa_package_name)); base::android::ConvertUTF8ToJavaString(env, twa_package_name));
} }
......
...@@ -24,7 +24,6 @@ void SetUseDelegateOnPaymentRequestForTesting( ...@@ -24,7 +24,6 @@ void SetUseDelegateOnPaymentRequestForTesting(
bool use_delegate, bool use_delegate,
bool is_incognito, bool is_incognito,
bool is_valid_ssl, bool is_valid_ssl,
bool is_web_contents_active,
bool prefs_can_make_payment, bool prefs_can_make_payment,
bool skip_ui_for_basic_card, bool skip_ui_for_basic_card,
const std::string& twa_package_name); const std::string& twa_package_name);
......
...@@ -79,7 +79,7 @@ void PaymentRequestTestController::SetUpOnMainThread() { ...@@ -79,7 +79,7 @@ void PaymentRequestTestController::SetUpOnMainThread() {
SetUseDelegateOnPaymentRequestForTesting( SetUseDelegateOnPaymentRequestForTesting(
/*use_delegate_for_test=*/true, is_off_the_record_, valid_ssl_, /*use_delegate_for_test=*/true, is_off_the_record_, valid_ssl_,
/*is_browser_window_active=*/true, can_make_payment_pref_, can_make_payment_pref_,
/*skip_ui_for_basic_card=*/false, twa_package_name_); /*skip_ui_for_basic_card=*/false, twa_package_name_);
} }
...@@ -92,7 +92,7 @@ void PaymentRequestTestController::SetOffTheRecord(bool is_off_the_record) { ...@@ -92,7 +92,7 @@ void PaymentRequestTestController::SetOffTheRecord(bool is_off_the_record) {
is_off_the_record_ = is_off_the_record; is_off_the_record_ = is_off_the_record;
SetUseDelegateOnPaymentRequestForTesting( SetUseDelegateOnPaymentRequestForTesting(
/*use_delegate_for_test=*/true, is_off_the_record_, valid_ssl_, /*use_delegate_for_test=*/true, is_off_the_record_, valid_ssl_,
/*is_browser_window_active=*/true, can_make_payment_pref_, can_make_payment_pref_,
/*skip_ui_for_basic_card=*/false, twa_package_name_); /*skip_ui_for_basic_card=*/false, twa_package_name_);
} }
...@@ -100,7 +100,7 @@ void PaymentRequestTestController::SetValidSsl(bool valid_ssl) { ...@@ -100,7 +100,7 @@ void PaymentRequestTestController::SetValidSsl(bool valid_ssl) {
valid_ssl_ = valid_ssl; valid_ssl_ = valid_ssl;
SetUseDelegateOnPaymentRequestForTesting( SetUseDelegateOnPaymentRequestForTesting(
/*use_delegate_for_test=*/true, is_off_the_record_, valid_ssl_, /*use_delegate_for_test=*/true, is_off_the_record_, valid_ssl_,
/*is_browser_window_active=*/true, can_make_payment_pref_, can_make_payment_pref_,
/*skip_ui_for_basic_card=*/false, twa_package_name_); /*skip_ui_for_basic_card=*/false, twa_package_name_);
} }
...@@ -109,7 +109,7 @@ void PaymentRequestTestController::SetCanMakePaymentEnabledPref( ...@@ -109,7 +109,7 @@ void PaymentRequestTestController::SetCanMakePaymentEnabledPref(
can_make_payment_pref_ = can_make_payment_enabled; can_make_payment_pref_ = can_make_payment_enabled;
SetUseDelegateOnPaymentRequestForTesting( SetUseDelegateOnPaymentRequestForTesting(
/*use_delegate_for_test=*/true, is_off_the_record_, valid_ssl_, /*use_delegate_for_test=*/true, is_off_the_record_, valid_ssl_,
/*is_browser_window_active=*/true, can_make_payment_pref_, can_make_payment_pref_,
/*skip_ui_for_basic_card=*/false, twa_package_name_); /*skip_ui_for_basic_card=*/false, twa_package_name_);
} }
...@@ -118,7 +118,7 @@ void PaymentRequestTestController::SetTwaPackageName( ...@@ -118,7 +118,7 @@ void PaymentRequestTestController::SetTwaPackageName(
twa_package_name_ = twa_package_name; twa_package_name_ = twa_package_name;
SetUseDelegateOnPaymentRequestForTesting( SetUseDelegateOnPaymentRequestForTesting(
/*use_delegate_for_test=*/true, is_off_the_record_, valid_ssl_, /*use_delegate_for_test=*/true, is_off_the_record_, valid_ssl_,
/*is_browser_window_active=*/true, can_make_payment_pref_, can_make_payment_pref_,
/*skip_ui_for_basic_card=*/false, twa_package_name_); /*skip_ui_for_basic_card=*/false, twa_package_name_);
} }
......
...@@ -114,11 +114,6 @@ public class PaymentRequestService { ...@@ -114,11 +114,6 @@ public class PaymentRequestService {
*/ */
String getInvalidSslCertificateErrorMessage(); String getInvalidSslCertificateErrorMessage();
/**
* @return True if the merchant's web contents that initiated the payment request is active.
*/
boolean isWebContentsActive();
/** /**
* @return Whether the preferences allow CAN_MAKE_PAYMENT. * @return Whether the preferences allow CAN_MAKE_PAYMENT.
*/ */
......
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