Commit 4cd5d8a5 authored by Sky Malice's avatar Sky Malice Committed by Commit Bot

Revert "[remoteobjects] Use new codepath for Java bridge in instrumentation tests"

This reverts commit 2ea849e2.

Reason for revert: Flakes on Android, see crbug.com/1117003 and crbug.com/1116744

Original change's description:
> [remoteobjects] Use new codepath for Java bridge in instrumentation tests
> 
> This CL is parameterizing the Java bridge instrumentation tests to
> run with and without using Mojo in JavascriptInjector, running all
> the relevant tests and using
> JavaBridgeActivityTestRule#MojoTestParams for the ones that pass
> and JavaBridgeActivityTestRule#LegacyTestParams for the ones that
> fails. So, all tests still run for the old codepath.
> 
> Bug: 1105928
> Change-Id: Idb3ee3ed4d1cb49fa0fc37d7609ac2e8ef166177
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2348488
> Reviewed-by: Bo <boliu@chromium.org>
> Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
> Commit-Queue: Miyoung Shin <myid.shin@igalia.com>
> Cr-Commit-Position: refs/heads/master@{#798370}

TBR=boliu@chromium.org,oksamyt@chromium.org,myid.shin@igalia.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1105928
Change-Id: I348787f3ca85c778a1ad9ebfb6ec0e97dada5dbb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2359738Reviewed-by: default avatarSky Malice <skym@chromium.org>
Commit-Queue: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798786}
parent caad51c8
......@@ -11,8 +11,6 @@ import org.junit.runners.model.Statement;
import org.chromium.base.Log;
import org.chromium.base.test.SetUpStatement;
import org.chromium.base.test.SetUpTestRule;
import org.chromium.base.test.params.ParameterProvider;
import org.chromium.base.test.params.ParameterSet;
import org.chromium.base.test.util.UrlUtils;
import org.chromium.content_public.browser.JavascriptInjector;
import org.chromium.content_public.browser.LoadUrlParams;
......@@ -22,44 +20,14 @@ import org.chromium.content_shell_apk.ContentShellActivity;
import org.chromium.content_shell_apk.ContentShellActivityTestRule;
import java.lang.annotation.Annotation;
import java.util.Arrays;
import java.util.List;
/**
* ActivityTestRule with common functionality for testing the Java Bridge.
*/
public class JavaBridgeActivityTestRule
extends ContentShellActivityTestRule implements SetUpTestRule<JavaBridgeActivityTestRule> {
/**
* {@link ParameterProvider} used for parameterized test that provides the Mojo usage state.
*/
public static class MojoTestParams implements ParameterProvider {
private static List<ParameterSet> sMojoTestParams =
Arrays.asList(new ParameterSet().value(false).name("MojoUnused"),
new ParameterSet().value(true).name("MojoUsed"));
@Override
public List<ParameterSet> getParameters() {
return sMojoTestParams;
}
}
/**
* {@link ParameterProvider} used for parameterized test that keeps the legacy tests.
*/
public static class LegacyTestParams implements ParameterProvider {
private static List<ParameterSet> sLegacyTestParams =
Arrays.asList(new ParameterSet().value(false));
@Override
public List<ParameterSet> getParameters() {
return sLegacyTestParams;
}
}
private TestCallbackHelperContainer mTestCallbackHelperContainer;
private boolean mSetup;
private boolean mUseMojo;
public static class Controller {
private static final int RESULT_WAIT_TIME = 5000;
......@@ -148,8 +116,7 @@ public class JavaBridgeActivityTestRule
@Override
public void run() {
WebContents webContents = getWebContents();
JavascriptInjector injector =
JavascriptInjector.fromWebContents(webContents, mUseMojo);
JavascriptInjector injector = JavascriptInjector.fromWebContents(webContents);
injector.addPossiblyUnsafeInterface(object1, name1, requiredAnnotation);
if (object2 != null && name2 != null) {
injector.addPossiblyUnsafeInterface(object2, name2, requiredAnnotation);
......@@ -163,9 +130,6 @@ public class JavaBridgeActivityTestRule
"Failed to injectObjectsAndReload: " + Log.getStackTraceString(e));
}
}
public void setupMojoTest(boolean useMojo) {
mUseMojo = useMojo;
}
public void synchronousPageReload() throws Throwable {
TestCallbackHelperContainer.OnPageFinishedHelper onPageFinishedHelper =
......
......@@ -12,11 +12,7 @@ import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.base.test.params.BaseJUnit4RunnerDelegate;
import org.chromium.base.test.params.ParameterAnnotations.UseMethodParameter;
import org.chromium.base.test.params.ParameterAnnotations.UseMethodParameterBefore;
import org.chromium.base.test.params.ParameterAnnotations.UseRunnerDelegate;
import org.chromium.base.test.params.ParameterizedRunner;
import org.chromium.base.test.BaseJUnit4ClassRunner;
import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.UrlUtils;
import org.chromium.content_public.browser.test.util.TestCallbackHelperContainer;
......@@ -26,21 +22,13 @@ import org.chromium.content_public.browser.test.util.TestCallbackHelperContainer
/**
* Common functionality for testing the Java Bridge.
*/
@RunWith(ParameterizedRunner.class)
@UseRunnerDelegate(BaseJUnit4RunnerDelegate.class)
@RunWith(BaseJUnit4ClassRunner.class)
public class JavaBridgeBareboneTest {
@Rule
public JavaBridgeActivityTestRule mActivityTestRule =
new JavaBridgeActivityTestRule().shouldSetUp(false);
private TestCallbackHelperContainer mTestCallbackHelperContainer;
private boolean mUseMojo;
@UseMethodParameterBefore(JavaBridgeActivityTestRule.MojoTestParams.class)
public void setupMojoTest(boolean useMojo) {
mUseMojo = useMojo;
mActivityTestRule.setupMojoTest(useMojo);
}
@Before
public void setUp() {
......@@ -55,7 +43,7 @@ public class JavaBridgeBareboneTest {
mActivityTestRule.runOnUiThread(new Runnable() {
@Override
public void run() {
mActivityTestRule.getJavascriptInjector(mUseMojo).addPossiblyUnsafeInterface(
mActivityTestRule.getJavascriptInjector().addPossiblyUnsafeInterface(
new Object(), name, null);
}
});
......@@ -95,8 +83,7 @@ public class JavaBridgeBareboneTest {
@Test
@SmallTest
@Feature({"AndroidWebView", "Android-JavaBridge"})
@UseMethodParameter(JavaBridgeActivityTestRule.MojoTestParams.class)
public void testImmediateAddition(boolean useMojo) throws Throwable {
public void testImmediateAddition() throws Throwable {
injectDummyObject("testObject");
Assert.assertEquals("\"object\"", evaluateJsSync("typeof testObject"));
}
......@@ -106,8 +93,7 @@ public class JavaBridgeBareboneTest {
@Test
@SmallTest
@Feature({"AndroidWebView", "Android-JavaBridge"})
@UseMethodParameter(JavaBridgeActivityTestRule.MojoTestParams.class)
public void testNoImmediateAdditionAfterJSEvaluation(boolean useMojo) throws Throwable {
public void testNoImmediateAdditionAfterJSEvaluation() throws Throwable {
evaluateJsSync("true");
injectDummyObject("testObject");
Assert.assertEquals("\"undefined\"", evaluateJsSync("typeof testObject"));
......@@ -116,8 +102,7 @@ public class JavaBridgeBareboneTest {
@Test
@SmallTest
@Feature({"AndroidWebView", "Android-JavaBridge"})
@UseMethodParameter(JavaBridgeActivityTestRule.MojoTestParams.class)
public void testImmediateAdditionAfterReload(boolean useMojo) throws Throwable {
public void testImmediateAdditionAfterReload() throws Throwable {
reloadSync();
injectDummyObject("testObject");
Assert.assertEquals("\"object\"", evaluateJsSync("typeof testObject"));
......@@ -126,8 +111,7 @@ public class JavaBridgeBareboneTest {
@Test
@SmallTest
@Feature({"AndroidWebView", "Android-JavaBridge"})
@UseMethodParameter(JavaBridgeActivityTestRule.MojoTestParams.class)
public void testReloadAfterAddition(boolean useMojo) throws Throwable {
public void testReloadAfterAddition() throws Throwable {
injectDummyObject("testObject");
reloadSync();
Assert.assertEquals("\"object\"", evaluateJsSync("typeof testObject"));
......
......@@ -14,10 +14,6 @@ import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.base.test.params.ParameterAnnotations.UseMethodParameter;
import org.chromium.base.test.params.ParameterAnnotations.UseMethodParameterBefore;
import org.chromium.base.test.params.ParameterAnnotations.UseRunnerDelegate;
import org.chromium.base.test.params.ParameterizedRunner;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.DisabledTest;
import org.chromium.base.test.util.Feature;
......@@ -26,7 +22,7 @@ import org.chromium.content_public.browser.JavaScriptCallback;
import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.content_public.browser.NavigationController;
import org.chromium.content_public.browser.WebContents;
import org.chromium.content_public.browser.test.ContentJUnit4RunnerDelegate;
import org.chromium.content_public.browser.test.ContentJUnit4ClassRunner;
import java.lang.ref.WeakReference;
import java.util.concurrent.CountDownLatch;
......@@ -39,8 +35,7 @@ import java.util.concurrent.TimeoutException;
* Ensures that injected objects are exposed to child frames as well as the
* main frame.
*/
@RunWith(ParameterizedRunner.class)
@UseRunnerDelegate(ContentJUnit4RunnerDelegate.class)
@RunWith(ContentJUnit4ClassRunner.class)
public class JavaBridgeChildFrameTest {
@Rule
public JavaBridgeActivityTestRule mActivityTestRule =
......@@ -61,11 +56,6 @@ public class JavaBridgeChildFrameTest {
}
}
@UseMethodParameterBefore(JavaBridgeActivityTestRule.MojoTestParams.class)
public void setupMojoTest(boolean useMojo) {
mActivityTestRule.setupMojoTest(useMojo);
}
TestController mTestController;
@Before
......@@ -77,8 +67,7 @@ public class JavaBridgeChildFrameTest {
@Test
@SmallTest
@Feature({"AndroidWebView", "Android-JavaBridge"})
@UseMethodParameter(JavaBridgeActivityTestRule.LegacyTestParams.class)
public void testInjectedObjectPresentInChildFrame(boolean useMojo) throws Throwable {
public void testInjectedObjectPresentInChildFrame() throws Throwable {
loadDataSync(mActivityTestRule.getWebContents().getNavigationController(),
"<html><body><iframe></iframe></body></html>", "text/html", false);
// We are not executing this code as a part of page loading routine to avoid races
......@@ -96,8 +85,7 @@ public class JavaBridgeChildFrameTest {
@Test
@SmallTest
@Feature({"AndroidWebView", "Android-JavaBridge"})
@UseMethodParameter(JavaBridgeActivityTestRule.MojoTestParams.class)
public void testMainPageWrapperIsNotBrokenByChildFrame(boolean useMojo) throws Throwable {
public void testMainPageWrapperIsNotBrokenByChildFrame() throws Throwable {
loadDataSync(mActivityTestRule.getWebContents().getNavigationController(),
"<html><body><iframe></iframe></body></html>", "text/html", false);
// In case there is anything wrong with the JS wrapper, an attempt
......@@ -120,8 +108,7 @@ public class JavaBridgeChildFrameTest {
@Test
@SmallTest
@Feature({"AndroidWebView", "Android-JavaBridge"})
@UseMethodParameter(JavaBridgeActivityTestRule.LegacyTestParams.class)
public void testWrapperIsNotSharedWithChildFrame(boolean useMojo) throws Throwable {
public void testWrapperIsNotSharedWithChildFrame() throws Throwable {
// Test by setting a custom property on the parent page's injected
// object and then checking that child frame doesn't see the property.
loadDataSync(mActivityTestRule.getWebContents().getNavigationController(),
......@@ -150,8 +137,7 @@ public class JavaBridgeChildFrameTest {
@SmallTest
@Feature({"AndroidWebView", "Android-JavaBridge"})
@DisabledTest(message = "https://crbug.com/677182")
@UseMethodParameter(JavaBridgeActivityTestRule.MojoTestParams.class)
public void testRemovingTransientObjectHolders(boolean useMojo) throws Throwable {
public void testRemovingTransientObjectHolders() throws Throwable {
class Test {
private Object mInner = new Object();
// Expecting the inner object to be retrieved twice.
......@@ -213,8 +199,7 @@ public class JavaBridgeChildFrameTest {
@Feature({"AndroidWebView", "Android-JavaBridge"})
@CommandLineFlags.Add("js-flags=--expose-gc")
@DisabledTest(message = "https://crbug.com/646843")
@UseMethodParameter(JavaBridgeActivityTestRule.MojoTestParams.class)
public void testHolderFrame(boolean useMojo) throws Throwable {
public void testHolderFrame() throws Throwable {
class Test {
WeakReference<Object> mWeakRefForInner;
private CountDownLatch mLatch = new CountDownLatch(1);
......
......@@ -12,11 +12,7 @@ import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.base.test.params.BaseJUnit4RunnerDelegate;
import org.chromium.base.test.params.ParameterAnnotations.UseMethodParameter;
import org.chromium.base.test.params.ParameterAnnotations.UseMethodParameterBefore;
import org.chromium.base.test.params.ParameterAnnotations.UseRunnerDelegate;
import org.chromium.base.test.params.ParameterizedRunner;
import org.chromium.base.test.BaseJUnit4ClassRunner;
import org.chromium.base.test.util.Feature;
import org.chromium.content.browser.JavaBridgeActivityTestRule.Controller;
......@@ -24,8 +20,7 @@ import org.chromium.content.browser.JavaBridgeActivityTestRule.Controller;
* Part of the test suite for the Java Bridge. This test tests the
* use of fields.
*/
@RunWith(ParameterizedRunner.class)
@UseRunnerDelegate(BaseJUnit4RunnerDelegate.class)
@RunWith(BaseJUnit4ClassRunner.class)
public class JavaBridgeFieldsTest {
@Rule
public JavaBridgeActivityTestRule mActivityTestRule =
......@@ -61,11 +56,6 @@ public class JavaBridgeFieldsTest {
private static class CustomType {
}
@UseMethodParameterBefore(JavaBridgeActivityTestRule.MojoTestParams.class)
public void setupMojoTest(boolean useMojo) {
mActivityTestRule.setupMojoTest(useMojo);
}
TestObject mTestObject;
@Before
......@@ -85,8 +75,7 @@ public class JavaBridgeFieldsTest {
@Test
@SmallTest
@Feature({"AndroidWebView", "Android-JavaBridge"})
@UseMethodParameter(JavaBridgeActivityTestRule.MojoTestParams.class)
public void testFieldTypes(boolean useMojo) throws Throwable {
public void testFieldTypes() throws Throwable {
Assert.assertEquals(
"undefined", executeJavaScriptAndGetStringResult("typeof testObject.booleanField"));
Assert.assertEquals(
......
......@@ -12,11 +12,7 @@ import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.base.test.params.BaseJUnit4RunnerDelegate;
import org.chromium.base.test.params.ParameterAnnotations.UseMethodParameter;
import org.chromium.base.test.params.ParameterAnnotations.UseMethodParameterBefore;
import org.chromium.base.test.params.ParameterAnnotations.UseRunnerDelegate;
import org.chromium.base.test.params.ParameterizedRunner;
import org.chromium.base.test.BaseJUnit4ClassRunner;
import org.chromium.base.test.util.Feature;
import org.chromium.content.browser.JavaBridgeActivityTestRule.Controller;
......@@ -31,8 +27,7 @@ import org.chromium.content.browser.JavaBridgeActivityTestRule.Controller;
* FIXME: Consider making our implementation more compliant, if it will not
* break backwards-compatibility. See b/4408210.
*/
@RunWith(ParameterizedRunner.class)
@UseRunnerDelegate(BaseJUnit4RunnerDelegate.class)
@RunWith(BaseJUnit4ClassRunner.class)
public class JavaBridgeReturnValuesTest {
@Rule
public JavaBridgeActivityTestRule mActivityTestRule =
......@@ -118,11 +113,6 @@ public class JavaBridgeReturnValuesTest {
private static class CustomType {
}
@UseMethodParameterBefore(JavaBridgeActivityTestRule.MojoTestParams.class)
public void setupMojoTest(boolean useMojo) {
mActivityTestRule.setupMojoTest(useMojo);
}
TestObject mTestObject;
@Before
......@@ -146,8 +136,7 @@ public class JavaBridgeReturnValuesTest {
@Test
@SmallTest
@Feature({"AndroidWebView", "Android-JavaBridge"})
@UseMethodParameter(JavaBridgeActivityTestRule.MojoTestParams.class)
public void testMethodReturnTypes(boolean useMojo) throws Throwable {
public void testMethodReturnTypes() throws Throwable {
Assert.assertEquals("boolean",
executeJavaScriptAndGetStringResult("typeof testObject.getBooleanValue()"));
Assert.assertEquals(
......@@ -189,8 +178,7 @@ public class JavaBridgeReturnValuesTest {
@Test
@SmallTest
@Feature({"AndroidWebView", "Android-JavaBridge"})
@UseMethodParameter(JavaBridgeActivityTestRule.MojoTestParams.class)
public void testMethodReturnValues(boolean useMojo) throws Throwable {
public void testMethodReturnValues() throws Throwable {
// We do the string comparison in JavaScript, to avoid relying on the
// coercion algorithm from JavaScript to Java.
Assert.assertTrue(executeJavaScriptAndGetBooleanResult("testObject.getBooleanValue()"));
......
......@@ -34,7 +34,6 @@ android_library("content_java_test_support") {
"javatests/src/org/chromium/content_public/browser/test/ChildProcessAllocatorSettings.java",
"javatests/src/org/chromium/content_public/browser/test/ChildProcessAllocatorSettingsHook.java",
"javatests/src/org/chromium/content_public/browser/test/ContentJUnit4ClassRunner.java",
"javatests/src/org/chromium/content_public/browser/test/ContentJUnit4RunnerDelegate.java",
"javatests/src/org/chromium/content_public/browser/test/NativeLibraryTestUtils.java",
"javatests/src/org/chromium/content_public/browser/test/RenderFrameHostTestExt.java",
"javatests/src/org/chromium/content_public/browser/test/mock/MockNavigationController.java",
......
// 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.content_public.browser.test;
import org.junit.runners.model.FrameworkMethod;
import org.junit.runners.model.InitializationError;
import org.chromium.base.test.params.ParameterizedRunner.ParameterizedTestInstantiationException;
import org.chromium.base.test.params.ParameterizedRunnerDelegate;
import org.chromium.base.test.params.ParameterizedRunnerDelegateCommon;
import java.util.List;
/**
* A custom runner delegate for running //content JUnit4 parameterized tests.
*/
public final class ContentJUnit4RunnerDelegate
extends ContentJUnit4ClassRunner implements ParameterizedRunnerDelegate {
private final ParameterizedRunnerDelegateCommon mDelegateCommon;
public ContentJUnit4RunnerDelegate(Class<?> klass,
ParameterizedRunnerDelegateCommon delegateCommon) throws InitializationError {
super(klass);
mDelegateCommon = delegateCommon;
}
@Override
public void collectInitializationErrors(List<Throwable> errors) {
ParameterizedRunnerDelegateCommon.collectInitializationErrors(errors);
}
@Override
public List<FrameworkMethod> computeTestMethods() {
return mDelegateCommon.computeTestMethods();
}
@Override
public Object createTest() throws ParameterizedTestInstantiationException {
return mDelegateCommon.createTest();
}
}
......@@ -226,11 +226,7 @@ public class ContentShellActivityTestRule extends ActivityTestRule<ContentShellA
}
public JavascriptInjector getJavascriptInjector() {
return getJavascriptInjector(false);
}
public JavascriptInjector getJavascriptInjector(boolean useMojo) {
return JavascriptInjector.fromWebContents(getWebContents(), useMojo);
return JavascriptInjector.fromWebContents(getWebContents());
}
/**
......
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