Commit b7bb401a authored by Liquan (Max) Gu's avatar Liquan (Max) Gu

Revert "[PRImpl] PaymentRequestImpl retains the values it needs"

This reverts commit 00d4ff28.

Reason for revert: the waterfall fails to build.

Original change's description:
> [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/+/2378538
> Reviewed-by: Danyao Wang <danyao@chromium.org>
> Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#803902}

TBR=danyao@chromium.org,maxlg@chromium.org

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