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

Fix ChromePaymentRequestFactory crash

Before the CL, ChromePaymentRequestFactory.sObserver was used before
assigned in production code.

To fix it, this CL null-check the observer, suffix it with
"ForTest", and annotate it with @Nullable.

This CL also add unit-tests for ChromePaymentRequestFactory which is
able to capture this crash.

Bug: 1143141
Binary-Size: Recovering after the wrong reduction of crrev.com/c/2499323
Change-Id: I6947fcee22d07c7641c9d22bb63defb1306c9079
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2504283Reviewed-by: default avatarSahel Sharify <sahel@chromium.org>
Reviewed-by: default avatarLiquan (Max) Gu <maxlg@chromium.org>
Reviewed-by: default avatarAndrew Grieve <agrieve@chromium.org>
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821990}
parent 25dd7d47
...@@ -905,6 +905,7 @@ junit_binary("chrome_junit_tests") { ...@@ -905,6 +905,7 @@ junit_binary("chrome_junit_tests") {
"//services/device/public/mojom:mojom_java", "//services/device/public/mojom:mojom_java",
"//services/media_session/public/cpp/android:media_session_java", "//services/media_session/public/cpp/android:media_session_java",
"//services/media_session/public/mojom:mojom_java", "//services/media_session/public/mojom:mojom_java",
"//services/service_manager/public/java:service_manager_java",
"//third_party/android_deps:android_support_v7_appcompat_java", "//third_party/android_deps:android_support_v7_appcompat_java",
"//third_party/android_deps:androidx_annotation_annotation_java", "//third_party/android_deps:androidx_annotation_annotation_java",
"//third_party/android_deps:androidx_collection_collection_java", "//third_party/android_deps:androidx_collection_collection_java",
......
...@@ -197,6 +197,10 @@ chrome_junit_test_java_sources = [ ...@@ -197,6 +197,10 @@ chrome_junit_test_java_sources = [
"junit/src/org/chromium/chrome/browser/password_manager/settings/TimedCallbackDelayerTest.java", "junit/src/org/chromium/chrome/browser/password_manager/settings/TimedCallbackDelayerTest.java",
"junit/src/org/chromium/chrome/browser/payments/AutofillContactTest.java", "junit/src/org/chromium/chrome/browser/payments/AutofillContactTest.java",
"junit/src/org/chromium/chrome/browser/payments/AutofillContactUnitTest.java", "junit/src/org/chromium/chrome/browser/payments/AutofillContactUnitTest.java",
"junit/src/org/chromium/chrome/browser/payments/ChromePaymentRequestFactoryTest.java",
"junit/src/org/chromium/chrome/browser/payments/ShadowPaymentFeatureList.java",
"junit/src/org/chromium/chrome/browser/payments/ShadowProfile.java",
"junit/src/org/chromium/chrome/browser/payments/ShadowWebContentsStatics.java",
"junit/src/org/chromium/chrome/browser/payments/handler/toolbar/PaymentHandlerToolbarMediatorTest.java", "junit/src/org/chromium/chrome/browser/payments/handler/toolbar/PaymentHandlerToolbarMediatorTest.java",
"junit/src/org/chromium/chrome/browser/policy/EnterpriseInfoTest.java", "junit/src/org/chromium/chrome/browser/policy/EnterpriseInfoTest.java",
"junit/src/org/chromium/chrome/browser/privacy/settings/PrivacyPreferencesManagerTest.java", "junit/src/org/chromium/chrome/browser/privacy/settings/PrivacyPreferencesManagerTest.java",
......
...@@ -30,7 +30,8 @@ import org.chromium.services.service_manager.InterfaceFactory; ...@@ -30,7 +30,8 @@ import org.chromium.services.service_manager.InterfaceFactory;
public class ChromePaymentRequestFactory implements InterfaceFactory<PaymentRequest> { public class ChromePaymentRequestFactory implements InterfaceFactory<PaymentRequest> {
// Tests can inject behaviour on future PaymentRequests via these objects. // Tests can inject behaviour on future PaymentRequests via these objects.
public static ChromePaymentRequestService.Delegate sDelegateForTest; public static ChromePaymentRequestService.Delegate sDelegateForTest;
private static ChromePaymentRequestDelegateImplObserverForTest sObserver; @Nullable
private static ChromePaymentRequestDelegateImplObserverForTest sObserverForTest;
private final RenderFrameHost mRenderFrameHost; private final RenderFrameHost mRenderFrameHost;
/** Observes the {@link ChromePaymentRequestDelegateImpl} for testing. */ /** Observes the {@link ChromePaymentRequestDelegateImpl} for testing. */
...@@ -133,7 +134,7 @@ public class ChromePaymentRequestFactory implements InterfaceFactory<PaymentRequ ...@@ -133,7 +134,7 @@ public class ChromePaymentRequestFactory implements InterfaceFactory<PaymentRequ
public static void setChromePaymentRequestDelegateImplObserverForTest( public static void setChromePaymentRequestDelegateImplObserverForTest(
ChromePaymentRequestDelegateImplObserverForTest observer) { ChromePaymentRequestDelegateImplObserverForTest observer) {
assert observer != null; assert observer != null;
sObserver = observer; sObserverForTest = observer;
} }
@Override @Override
...@@ -155,7 +156,10 @@ public class ChromePaymentRequestFactory implements InterfaceFactory<PaymentRequ ...@@ -155,7 +156,10 @@ public class ChromePaymentRequestFactory implements InterfaceFactory<PaymentRequ
} else { } else {
ChromePaymentRequestDelegateImpl delegateImpl = ChromePaymentRequestDelegateImpl delegateImpl =
new ChromePaymentRequestDelegateImpl(mRenderFrameHost); new ChromePaymentRequestDelegateImpl(mRenderFrameHost);
sObserver.onCreatedChromePaymentRequestDelegateImpl(/*delegateImpl=*/delegateImpl); if (sObserverForTest != null) {
sObserverForTest.onCreatedChromePaymentRequestDelegateImpl(/*delegateImpl=*/
delegateImpl);
}
delegate = delegateImpl; delegate = delegateImpl;
} }
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.payments;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;
import org.mockito.quality.Strictness;
import org.robolectric.annotation.Config;
import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.base.test.util.Feature;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.components.payments.InvalidPaymentRequest;
import org.chromium.content_public.browser.FeaturePolicyFeature;
import org.chromium.content_public.browser.RenderFrameHost;
import org.chromium.content_public.browser.WebContents;
import org.chromium.services.service_manager.InterfaceProvider;
import java.util.concurrent.atomic.AtomicBoolean;
/** A test for ChromePaymentRequestFactory. */
@RunWith(BaseRobolectricTestRunner.class)
@Config(manifest = Config.NONE,
shadows = {ShadowPaymentFeatureList.class, ShadowWebContentsStatics.class,
ShadowProfile.class})
public class ChromePaymentRequestFactoryTest {
@Rule
public MockitoRule mMockitoRule = MockitoJUnit.rule().strictness(Strictness.LENIENT);
@Mock
RenderFrameHost mRenderFrameHost;
@Mock
WebContents mWebContents;
@Mock
Profile mProfile;
@Before
public void setUp() {
ShadowPaymentFeatureList.setWebPaymentsFeatureEnabled(true);
setWebContentsDestroyed(false);
ShadowWebContentsStatics.setWebContents(mWebContents);
Mockito.doReturn(true).when(mProfile).isOffTheRecord();
ShadowProfile.setProfile(mProfile);
setPaymentFeaturePolicy(true);
}
private void setPaymentFeaturePolicy(boolean enabled) {
Mockito.doReturn(enabled)
.when(mRenderFrameHost)
.isFeatureEnabled(FeaturePolicyFeature.PAYMENT);
}
private void setWebContentsDestroyed(boolean isDestroyed) {
Mockito.doReturn(isDestroyed).when(mWebContents).isDestroyed();
}
private ChromePaymentRequestFactory createFactory(RenderFrameHost renderFrameHost) {
return new ChromePaymentRequestFactory(renderFrameHost);
}
@Test
@Feature({"Payments"})
public void testNullFrameCausesInvalidPaymentRequest() {
Assert.assertTrue(createFactory(/*renderFrameHost=*/null).createImpl()
instanceof InvalidPaymentRequest);
}
@Test
@Feature({"Payments"})
public void testDisabledPolicyCausesNullReturn() {
setPaymentFeaturePolicy(false);
InterfaceProvider provider = Mockito.mock(InterfaceProvider.class);
Mockito.doReturn(provider).when(mRenderFrameHost).getRemoteInterfaces();
AtomicBoolean isConnectionError = new AtomicBoolean(false);
Mockito.doAnswer((args) -> {
isConnectionError.set(true);
return null;
})
.when(provider)
.onConnectionError(Mockito.any());
Assert.assertNull(createFactory(mRenderFrameHost).createImpl());
Assert.assertTrue(isConnectionError.get());
}
@Test
@Feature({"Payments"})
public void testDisabledFeatureCausesInvalidPaymentRequest() {
ShadowPaymentFeatureList.setWebPaymentsFeatureEnabled(false);
Assert.assertTrue(
createFactory(mRenderFrameHost).createImpl() instanceof InvalidPaymentRequest);
}
@Test
@Feature({"Payments"})
public void testNullWebContentsCausesInvalidPaymentRequest() {
ShadowWebContentsStatics.setWebContents(null);
Assert.assertTrue(
createFactory(mRenderFrameHost).createImpl() instanceof InvalidPaymentRequest);
}
@Test
@Feature({"Payments"})
public void testDestroyedWebContentsCausesInvalidPaymentRequest() {
setWebContentsDestroyed(true);
Assert.assertTrue(
createFactory(mRenderFrameHost).createImpl() instanceof InvalidPaymentRequest);
}
@Test
@Feature({"Payments"})
public void testPaymentRequestIsReturned() {
Assert.assertFalse(
createFactory(mRenderFrameHost).createImpl() instanceof InvalidPaymentRequest);
}
}
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.payments;
import org.robolectric.annotation.Implementation;
import org.robolectric.annotation.Implements;
import org.robolectric.annotation.Resetter;
import org.chromium.components.payments.PaymentFeatureList;
/** The shadow of PaymentFeatureList. */
@Implements(PaymentFeatureList.class)
public class ShadowPaymentFeatureList {
private static boolean sIsWebPaymentFeatureEnabled;
/**
* Set the {@link PaymentFeatureList.WEB_PAYMENTS} feature to be enabled.
* @param enabled Whether to enable the feature.
*/
public static void setWebPaymentsFeatureEnabled(boolean enabled) {
sIsWebPaymentFeatureEnabled = enabled;
}
@Resetter
public static void reset() {
sIsWebPaymentFeatureEnabled = false;
}
@Implementation
public static boolean isEnabled(String featureName) {
if (featureName.equals(PaymentFeatureList.WEB_PAYMENTS)) {
return sIsWebPaymentFeatureEnabled;
}
return false;
}
}
\ No newline at end of file
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.payments;
import androidx.annotation.Nullable;
import org.robolectric.annotation.Implementation;
import org.robolectric.annotation.Implements;
import org.robolectric.annotation.Resetter;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.content_public.browser.WebContents;
/** The shadow of Profile. */
@Implements(Profile.class)
public class ShadowProfile {
private static Profile sProfile;
/**
* Set the profile to be returned for {@link #fromWebContents}.
* @param profile The profile to be returned.
*/
public static void setProfile(Profile profile) {
sProfile = profile;
}
@Resetter
public static void reset() {
sProfile = null;
}
@Implementation
@Nullable
public static Profile fromWebContents(WebContents webContents) {
return sProfile;
}
}
\ No newline at end of file
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.payments;
import androidx.annotation.Nullable;
import org.robolectric.annotation.Implementation;
import org.robolectric.annotation.Implements;
import org.robolectric.annotation.Resetter;
import org.chromium.content_public.browser.RenderFrameHost;
import org.chromium.content_public.browser.WebContents;
import org.chromium.content_public.browser.WebContentsStatics;
/** The shadow of WebContentsStatics. */
@Implements(WebContentsStatics.class)
public class ShadowWebContentsStatics {
private static WebContents sWebContents;
/**
* Sets the WebContents to be returned from {@link #fromRenderFrameHost}.
* @param webContents The WebContents to be returned.
*/
public static void setWebContents(WebContents webContents) {
sWebContents = webContents;
}
@Resetter
public static void reset() {
sWebContents = null;
}
@Implementation
@Nullable
public static WebContents fromRenderFrameHost(RenderFrameHost rfh) {
return sWebContents;
}
}
\ 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