Commit 00d4ff28 authored by Liquan (Max) Gu's avatar Liquan (Max) Gu Committed by Commit Bot

[PRImpl] PaymentRequestImpl retains the values it needs

Context:
CL[1] moved some PRImpl's parameters it needed into CPRImpl, and
retrieved them from CPRImpl when PRImpl needs it. This is problematic,
because PRImpl could outlive CPRImpl, and so doing it could cause
NullPointerError (see the bug). This CL partially reverts CL[1] but
keeps the changes of the constructor signature because this is more
succinct than passing each one of the parameters. Since CPRImpl would
take over the PRImpl logic going forwards, passing only CPRImpl would
avoid the frequent change of the PRImpl constructor's signature.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/2357630

Change:
* Before: CPRImpl retains the values that PRImpl need; after: PRImpl
keeps a reference of the value that itself needs
* Added a few null check or assert for mComponentPaymentRequestImpl
in case it's used after release.

Bug: 1122148

Change-Id: I00ac6e4ca3fccccd114132209465757f52fa0dc9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2378538Reviewed-by: default avatarDanyao Wang <danyao@chromium.org>
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803902}
parent 3ccd13ca
......@@ -161,6 +161,9 @@ public class PaymentRequestImpl
*/
private static PaymentRequestImpl sShowingPaymentRequest;
// Null-check is necessary because retainers of PaymentRequestImpl could still reference
// PaymentRequestImpl after mComponentPaymentRequestImpl is set null, e.g., crbug.com/1122148.
@Nullable
private ComponentPaymentRequestImpl mComponentPaymentRequestImpl;
/** Monitors changes in the TabModelSelector. */
......@@ -193,9 +196,17 @@ public class PaymentRequestImpl
};
private final Handler mHandler = new Handler();
private final RenderFrameHost mRenderFrameHost;
private final Delegate mDelegate;
private final WebContents mWebContents;
private final String mTopLevelOrigin;
private final String mPaymentRequestOrigin;
private final Origin mPaymentRequestSecurityOrigin;
private final String mMerchantName;
@Nullable
private final byte[][] mCertificateChain;
private final JourneyLogger mJourneyLogger;
private final boolean mIsOffTheRecord;
private final PaymentUIsManager mPaymentUIsManager;
......@@ -319,13 +330,26 @@ public class PaymentRequestImpl
assert componentPaymentRequestImpl != null;
assert delegate != null;
mComponentPaymentRequestImpl = componentPaymentRequestImpl;
mRenderFrameHost = componentPaymentRequestImpl.getRenderFrameHost();
assert mRenderFrameHost != null;
mPaymentRequestOrigin = componentPaymentRequestImpl.getPaymentRequestOrigin();
assert mPaymentRequestOrigin != null;
mPaymentRequestSecurityOrigin =
componentPaymentRequestImpl.getPaymentRequestSecurityOrigin();
assert mPaymentRequestSecurityOrigin != null;
mTopLevelOrigin = componentPaymentRequestImpl.getTopLevelOrigin();
assert mTopLevelOrigin != null;
mCertificateChain = componentPaymentRequestImpl.getCertificateChain();
assert mCertificateChain != null;
mIsOffTheRecord = componentPaymentRequestImpl.isOffTheRecord();
mDelegate = delegate;
mWebContents = componentPaymentRequestImpl.getWebContents();
mMerchantName = mWebContents.getTitle();
mJourneyLogger = componentPaymentRequestImpl.getJourneyLogger();
mComponentPaymentRequestImpl = componentPaymentRequestImpl;
mPaymentUIsManager = new PaymentUIsManager(/*delegate=*/this,
/*params=*/this, mWebContents, mComponentPaymentRequestImpl.isOffTheRecord(),
mJourneyLogger, mComponentPaymentRequestImpl.getTopLevelOrigin(),
/*params=*/this, mWebContents, mIsOffTheRecord, mJourneyLogger, mTopLevelOrigin,
mComponentPaymentRequestImpl);
mComponentPaymentRequestImpl.registerPaymentRequestLifecycleObserver(mPaymentUIsManager);
}
......@@ -892,6 +916,8 @@ public class PaymentRequestImpl
assert mInvokedPaymentApp != null;
assert mInvokedPaymentApp.getPaymentAppType() == PaymentAppType.SERVICE_WORKER_APP;
if (mComponentPaymentRequestImpl == null) return false;
boolean success = mPaymentUIsManager.showPaymentHandlerUI(mWebContents, url,
paymentHandlerWebContentsObserver, mComponentPaymentRequestImpl.isOffTheRecord());
if (success) {
......@@ -1332,13 +1358,10 @@ public class PaymentRequestImpl
mInvokedPaymentApp.handlesShippingAddress()
? mRawShippingOptions
: Collections.unmodifiableList(new ArrayList<>());
mInvokedPaymentApp.invokePaymentApp(mId, mComponentPaymentRequestImpl.getMerchantName(),
mComponentPaymentRequestImpl.getTopLevelOrigin(),
mComponentPaymentRequestImpl.getPaymentRequestOrigin(),
mComponentPaymentRequestImpl.getCertificateChain(),
Collections.unmodifiableMap(methodData), mRawTotal, mRawLineItems,
Collections.unmodifiableMap(modifiers), paymentOptions, redactedShippingOptions,
this);
mInvokedPaymentApp.invokePaymentApp(mId, mMerchantName, mTopLevelOrigin,
mPaymentRequestOrigin, mCertificateChain, Collections.unmodifiableMap(methodData),
mRawTotal, mRawLineItems, Collections.unmodifiableMap(modifiers), paymentOptions,
redactedShippingOptions, this);
mJourneyLogger.setEventOccurred(Event.PAY_CLICKED);
boolean isAutofillCard = mInvokedPaymentApp.isAutofillInstrument();
......@@ -1553,9 +1576,8 @@ public class PaymentRequestImpl
mIsHasEnrolledInstrumentResponsePending = false;
if (CanMakePaymentQuery.canQuery(mWebContents,
mComponentPaymentRequestImpl.getTopLevelOrigin(),
mComponentPaymentRequestImpl.getPaymentRequestOrigin(), mQueryForQuota)) {
if (CanMakePaymentQuery.canQuery(
mWebContents, mTopLevelOrigin, mPaymentRequestOrigin, mQueryForQuota)) {
mComponentPaymentRequestImpl.onHasEnrolledInstrument(response
? HasEnrolledInstrumentQueryResult.HAS_ENROLLED_INSTRUMENT
: HasEnrolledInstrumentQueryResult.HAS_NO_ENROLLED_INSTRUMENT);
......@@ -1568,8 +1590,7 @@ public class PaymentRequestImpl
: HasEnrolledInstrumentQueryResult.WARNING_HAS_NO_ENROLLED_INSTRUMENT);
}
mJourneyLogger.setHasEnrolledInstrumentValue(
response || mComponentPaymentRequestImpl.isOffTheRecord());
mJourneyLogger.setHasEnrolledInstrumentValue(response || mIsOffTheRecord);
if (ComponentPaymentRequestImpl.getObserverForTest() != null) {
ComponentPaymentRequestImpl.getObserverForTest()
......@@ -1616,7 +1637,7 @@ public class PaymentRequestImpl
// PaymentAppFactoryParams implementation.
@Override
public RenderFrameHost getRenderFrameHost() {
return mComponentPaymentRequestImpl.getRenderFrameHost();
return mRenderFrameHost;
}
// PaymentAppFactoryParams implementation.
......@@ -1634,26 +1655,26 @@ public class PaymentRequestImpl
// PaymentAppFactoryParams implementation.
@Override
public String getTopLevelOrigin() {
return mComponentPaymentRequestImpl.getTopLevelOrigin();
return mTopLevelOrigin;
}
// PaymentAppFactoryParams implementation.
@Override
public String getPaymentRequestOrigin() {
return mComponentPaymentRequestImpl.getPaymentRequestOrigin();
return mPaymentRequestOrigin;
}
// PaymentAppFactoryParams implementation.
@Override
public Origin getPaymentRequestSecurityOrigin() {
return mComponentPaymentRequestImpl.getPaymentRequestSecurityOrigin();
return mPaymentRequestSecurityOrigin;
}
// PaymentAppFactoryParams implementation.
@Override
@Nullable
public byte[][] getCertificateChain() {
return mComponentPaymentRequestImpl.getCertificateChain();
return mCertificateChain;
}
// PaymentAppFactoryParams implementation.
......@@ -1902,7 +1923,7 @@ public class PaymentRequestImpl
// Chrome always refuses payments with invalid SSL and in prohibited origin types.
disconnectFromClientWithDebugMessage(
mRejectShowErrorMessage, PaymentErrorReason.NOT_SUPPORTED);
} else if (mComponentPaymentRequestImpl.isOffTheRecord()) {
} else if (mIsOffTheRecord) {
// If the user is in the OffTheRecord mode, hide the absence of their payment
// methods from the merchant site.
disconnectFromClientWithDebugMessage(
......
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