Commit ee630768 authored by Eric Seckler's avatar Eric Seckler Committed by Commit Bot

Reland "tracing: Address TracingControllerAndroid test failures."

This is a reland of 2fc9f6f2

The crashes in flakiness were due to calling destroy() on the wrong
thread, which has been addressed in the new patch set.

Original change's description:
> tracing: Address TracingControllerAndroid test failures.
>
> Fixes the flakiness of buffer usage / known categories tests by
> resolving an accidental race condition. Also restricts disabling
> the other two tests to <= kitkat devices since those seem to be
> the only ones that show (inexplicable) flakiness.
>
> TBR=yfriedman@chromium.org
>
> Bug: 899894
> Change-Id: I0225c1519ea19910492ffb60a242bcc71b2275b4
> Reviewed-on: https://chromium-review.googlesource.com/c/1307433
> Reviewed-by: Eric Seckler <eseckler@chromium.org>
> Commit-Queue: Eric Seckler <eseckler@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#604209}

Bug: 899894
Change-Id: I3cb002950f32bc6b6b4d25892adc3073c2fdb967
Reviewed-on: https://chromium-review.googlesource.com/c/1327407Reviewed-by: default avatarSami Kyöstilä <skyostil@chromium.org>
Reviewed-by: default avatarYaron Friedman <yfriedman@chromium.org>
Commit-Queue: Eric Seckler <eseckler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606570}
parent 1453d9de
...@@ -19,7 +19,7 @@ import org.junit.runner.RunWith; ...@@ -19,7 +19,7 @@ import org.junit.runner.RunWith;
import org.chromium.base.Callback; import org.chromium.base.Callback;
import org.chromium.base.ThreadUtils; import org.chromium.base.ThreadUtils;
import org.chromium.base.test.BaseJUnit4ClassRunner; import org.chromium.base.test.BaseJUnit4ClassRunner;
import org.chromium.base.test.util.DisabledTest; import org.chromium.base.test.util.DisableIf;
import org.chromium.base.test.util.Feature; import org.chromium.base.test.util.Feature;
import org.chromium.content_public.browser.test.util.CriteriaHelper; import org.chromium.content_public.browser.test.util.CriteriaHelper;
import org.chromium.content_shell_apk.ContentShellActivity; import org.chromium.content_shell_apk.ContentShellActivity;
...@@ -42,7 +42,7 @@ public class TracingControllerAndroidImplTest { ...@@ -42,7 +42,7 @@ public class TracingControllerAndroidImplTest {
@Test @Test
@MediumTest @MediumTest
@Feature({"GPU"}) @Feature({"GPU"})
@DisabledTest(message = "crbug.com/899894") @DisableIf.Build(sdk_is_less_than = 21, message = "crbug.com/899894")
public void testTraceFileCreation() throws Exception { public void testTraceFileCreation() throws Exception {
ContentShellActivity activity = mActivityTestRule.launchContentShellWithUrl("about:blank"); ContentShellActivity activity = mActivityTestRule.launchContentShellWithUrl("about:blank");
mActivityTestRule.waitForActiveShellToBeDoneLoading(); mActivityTestRule.waitForActiveShellToBeDoneLoading();
...@@ -60,7 +60,7 @@ public class TracingControllerAndroidImplTest { ...@@ -60,7 +60,7 @@ public class TracingControllerAndroidImplTest {
File file = new File(tracingController.getOutputPath()); File file = new File(tracingController.getOutputPath());
Assert.assertTrue(file.getName().startsWith("chrome-profile-results")); Assert.assertTrue(file.getName().startsWith("chrome-profile-results"));
ThreadUtils.runOnUiThreadBlocking(() -> { tracingController.stopTracing(null); }); ThreadUtils.runOnUiThreadBlocking(() -> tracingController.stopTracing(null));
// The tracer stops asynchronously, because it needs to wait for native code to flush and // The tracer stops asynchronously, because it needs to wait for native code to flush and
// close the output file. Give it a little time. // close the output file. Give it a little time.
...@@ -70,14 +70,14 @@ public class TracingControllerAndroidImplTest { ...@@ -70,14 +70,14 @@ public class TracingControllerAndroidImplTest {
// It says it stopped, so it should have written the output file. // It says it stopped, so it should have written the output file.
Assert.assertTrue(file.exists()); Assert.assertTrue(file.exists());
Assert.assertTrue(file.delete()); Assert.assertTrue(file.delete());
tracingController.destroy(); ThreadUtils.runOnUiThreadBlocking(() -> tracingController.destroy());
} }
private class TestCallback<T> implements Callback<T> { private class TestCallback<T> implements Callback<T> {
@Override @Override
public void onResult(T result) { public void onResult(T result) {
mWasCalled.open();
mResult = result; mResult = result;
mWasCalled.open();
} }
public ConditionVariable mWasCalled = new ConditionVariable(); public ConditionVariable mWasCalled = new ConditionVariable();
...@@ -87,7 +87,6 @@ public class TracingControllerAndroidImplTest { ...@@ -87,7 +87,6 @@ public class TracingControllerAndroidImplTest {
@Test @Test
@MediumTest @MediumTest
@Feature({"GPU"}) @Feature({"GPU"})
@DisabledTest(message = "crbug.com/899894")
public void testGetKnownCategories() throws Exception { public void testGetKnownCategories() throws Exception {
ContentShellActivity activity = mActivityTestRule.launchContentShellWithUrl("about:blank"); ContentShellActivity activity = mActivityTestRule.launchContentShellWithUrl("about:blank");
mActivityTestRule.waitForActiveShellToBeDoneLoading(); mActivityTestRule.waitForActiveShellToBeDoneLoading();
...@@ -102,13 +101,12 @@ public class TracingControllerAndroidImplTest { ...@@ -102,13 +101,12 @@ public class TracingControllerAndroidImplTest {
Assert.assertTrue(callback.mWasCalled.block(TIMEOUT_MILLIS)); Assert.assertTrue(callback.mWasCalled.block(TIMEOUT_MILLIS));
Assert.assertThat(Arrays.asList(callback.mResult), CoreMatchers.hasItem("toplevel")); Assert.assertThat(Arrays.asList(callback.mResult), CoreMatchers.hasItem("toplevel"));
tracingController.destroy(); ThreadUtils.runOnUiThreadBlocking(() -> tracingController.destroy());
} }
@Test @Test
@MediumTest @MediumTest
@Feature({"GPU"}) @Feature({"GPU"})
@DisabledTest(message = "crbug.com/899894")
public void testBufferUsage() throws Exception { public void testBufferUsage() throws Exception {
ContentShellActivity activity = mActivityTestRule.launchContentShellWithUrl("about:blank"); ContentShellActivity activity = mActivityTestRule.launchContentShellWithUrl("about:blank");
mActivityTestRule.waitForActiveShellToBeDoneLoading(); mActivityTestRule.waitForActiveShellToBeDoneLoading();
...@@ -125,13 +123,13 @@ public class TracingControllerAndroidImplTest { ...@@ -125,13 +123,13 @@ public class TracingControllerAndroidImplTest {
Assert.assertTrue(callback.mWasCalled.block(TIMEOUT_MILLIS)); Assert.assertTrue(callback.mWasCalled.block(TIMEOUT_MILLIS));
Assert.assertEquals(0f, (double) callback.mResult.first, 0.5f); Assert.assertEquals(0f, (double) callback.mResult.first, 0.5f);
Assert.assertEquals(0, (long) callback.mResult.second); Assert.assertEquals(0, (long) callback.mResult.second);
tracingController.destroy(); ThreadUtils.runOnUiThreadBlocking(() -> tracingController.destroy());
} }
@Test @Test
@MediumTest @MediumTest
@Feature({"GPU"}) @Feature({"GPU"})
@DisabledTest(message = "crbug.com/899894") @DisableIf.Build(sdk_is_less_than = 21, message = "crbug.com/899894")
public void testStopCallbackAndCompression() throws Exception { public void testStopCallbackAndCompression() throws Exception {
ContentShellActivity activity = mActivityTestRule.launchContentShellWithUrl("about:blank"); ContentShellActivity activity = mActivityTestRule.launchContentShellWithUrl("about:blank");
mActivityTestRule.waitForActiveShellToBeDoneLoading(); mActivityTestRule.waitForActiveShellToBeDoneLoading();
...@@ -150,7 +148,7 @@ public class TracingControllerAndroidImplTest { ...@@ -150,7 +148,7 @@ public class TracingControllerAndroidImplTest {
File file = new File(tracingController.getOutputPath()); File file = new File(tracingController.getOutputPath());
TestCallback<Void> callback = new TestCallback<>(); TestCallback<Void> callback = new TestCallback<>();
ThreadUtils.runOnUiThreadBlocking(() -> { tracingController.stopTracing(callback); }); ThreadUtils.runOnUiThreadBlocking(() -> tracingController.stopTracing(callback));
// Callback should be run once stopped. // Callback should be run once stopped.
Assert.assertTrue(callback.mWasCalled.block(TIMEOUT_MILLIS)); Assert.assertTrue(callback.mWasCalled.block(TIMEOUT_MILLIS));
...@@ -163,6 +161,6 @@ public class TracingControllerAndroidImplTest { ...@@ -163,6 +161,6 @@ public class TracingControllerAndroidImplTest {
Assert.assertEquals((byte) 0x1f, bytes[0]); Assert.assertEquals((byte) 0x1f, bytes[0]);
Assert.assertEquals((byte) 0x8b, bytes[1]); Assert.assertEquals((byte) 0x8b, bytes[1]);
Assert.assertTrue(file.delete()); Assert.assertTrue(file.delete());
tracingController.destroy(); ThreadUtils.runOnUiThreadBlocking(() -> tracingController.destroy());
} }
} }
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