Commit dfbd9d89 authored by Michael Thiessen's avatar Michael Thiessen Committed by Commit Bot

Fix Task Executor/Runner flakiness and lifecycles.

Problems this is addressing:
1. The TaskRunnerImpl hierarchy is currently too complicated, with
derived classes changing how base classes function in ways that require
the two classes to update in sync without breaking each other.
2. BrowserTaskExecutor holds onto WeakReferences to
SingleThreadTaskRunnerImpls, allowing them to leak the native memory if
the caller doesn't destroy, or continuing to return destroyed runners if
the caller does destroy but GC hasn't cleaned up the runner.
3. SequencedTaskRunnerImpl creates and deletes the native side
potentially with every posted task, taking time.
4. TaskExecutors aren't reset between tests leading to tasks flakily
failing to run, or crashes in native code (haven't figured out what's
not holding onto what it should hold onto, it was too complicated, so I
fixed the caching instead).
5. UnitTestUtils#pollUiThread could time out if nothing posts tasks to
the UI thread while waiting on an AtomicBoolean or similar.

TaskRunnerImpl is now in full control of its native pointer, not
exposing it or its lock to derived classes, and only creates its native
side if a task is posted to it, as many task runners are created
statically and may never actually have a task posted to them.

A ReferenceQueue is used to detect when the TaskRunner is GC'd (or about
to be), and that queue is polled when creating new TaskRunners, or when
initializing native for a TaskRunner (basically the two operations that
aren't performance critical). In practice when running Chrome we have
approximately 5-10 TaskRunners alive at any given time, and we can't
build up many TaskRunners without creating them, so polling for deletion
at creation time should be sufficient to keep the number of dead but in
memory task runners small.

PostTask now also holds a Strong Reference to the PreNativeTaskRunners
so that they can't be deleted before their tasks are placed on the
refcounted native runner.

Bug: 1058790
Change-Id: Ib58320e6f23d83ff95dc52c52bbdcf5bf7435968
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2091899Reviewed-by: default avatarYaron Friedman <yfriedman@chromium.org>
Reviewed-by: default avatarSami Kyöstilä <skyostil@chromium.org>
Reviewed-by: default avatarSam Maier <smaier@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749731}
parent 5a23f189
......@@ -36,16 +36,6 @@ final class ChoreographerTaskRunner implements SingleThreadTaskRunner {
});
}
@Override
public void destroy() {
// NOP
}
@Override
public void disableLifetimeCheck() {
// NOP
}
@Override
public void postDelayedTask(Runnable task, long delayMillis) {
mChoreographer.postFrameCallbackDelayed(new Choreographer.FrameCallback() {
......@@ -55,9 +45,4 @@ final class ChoreographerTaskRunner implements SingleThreadTaskRunner {
}
}, delayMillis);
}
@Override
public void initNativeTaskRunner() {
// NOP
}
}
......@@ -44,16 +44,12 @@ class DefaultTaskExecutor implements TaskExecutor {
@Override
public synchronized void postDelayedTask(TaskTraits taskTraits, Runnable task, long delay) {
if (taskTraits.hasExtension()) {
TaskRunner runner = createTaskRunner(taskTraits);
runner.postDelayedTask(task, delay);
runner.destroy();
createTaskRunner(taskTraits).postDelayedTask(task, delay);
} else {
// Caching TaskRunners only for common TaskTraits.
TaskRunner runner = mTraitsToRunnerMap.get(taskTraits);
if (runner == null) {
runner = createTaskRunner(taskTraits);
// Disable destroy() check since object will live forever.
runner.disableLifetimeCheck();
mTraitsToRunnerMap.put(taskTraits, runner);
}
runner.postDelayedTask(task, delay);
......
......@@ -8,13 +8,14 @@ import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.annotations.NativeMethods;
import java.util.Collections;
import java.util.Set;
import java.util.WeakHashMap;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.Executor;
import java.util.concurrent.FutureTask;
import javax.annotation.concurrent.GuardedBy;
/**
* Java interface to the native chromium scheduler. Note tasks can be posted before native
* initialization, but task prioritization is extremely limited. Once the native scheduler
......@@ -23,11 +24,10 @@ import java.util.concurrent.FutureTask;
@JNINamespace("base")
public class PostTask {
private static final Object sLock = new Object();
private static Set<TaskRunner> sPreNativeTaskRunners =
Collections.newSetFromMap(new WeakHashMap<TaskRunner, Boolean>());
private static List<TaskRunnerImpl> sPreNativeTaskRunners = new ArrayList<>();
private static final Executor sPrenativeThreadPoolExecutor = new ChromeThreadPoolExecutor();
private static Executor sPrenativeThreadPoolExecutorOverride;
private static final TaskExecutor sTaskExecutors[] = getInitialTaskExecutors();
private static TaskExecutor sTaskExecutors[] = getInitialTaskExecutors();
private static TaskExecutor[] getInitialTaskExecutors() {
TaskExecutor taskExecutors[] = new TaskExecutor[TaskTraits.MAX_EXTENSION_ID + 1];
......@@ -108,7 +108,11 @@ public class PostTask {
* @param task The task to be run with the specified traits.
*/
public static void runOrPostTask(TaskTraits taskTraits, Runnable task) {
if (getTaskExecutorForTraits(taskTraits).canRunTaskImmediately(taskTraits)) {
TaskExecutor taskExecutor;
synchronized (sLock) {
taskExecutor = getTaskExecutorForTraits(taskTraits);
}
if (taskExecutor.canRunTaskImmediately(taskTraits)) {
task.run();
} else {
postTask(taskTraits, task);
......@@ -219,22 +223,24 @@ public class PostTask {
}
/**
* Called by every TaskRunner on its creation, attempts to register this
* TaskRunner as pre-native, unless the native scheduler has been
* initialised already, and informs the caller about the outcome. Called
* only when sLock has already been acquired.
* Called by every TaskRunnerImpl on its creation, attempts to register this TaskRunner as
* pre-native, unless the native scheduler has been initialized already, and informs the caller
* about the outcome.
*
* @param taskRunner The TaskRunner to be registered.
* @param taskRunner The TaskRunnerImpl to be registered.
* @return If the taskRunner got registered as pre-native.
*/
static boolean registerPreNativeTaskRunnerLocked(TaskRunner taskRunner) {
if (sPreNativeTaskRunners != null) {
sPreNativeTaskRunners.add(taskRunner);
return true;
static boolean registerPreNativeTaskRunner(TaskRunnerImpl taskRunner) {
synchronized (sLock) {
if (sPreNativeTaskRunners != null) {
sPreNativeTaskRunners.add(taskRunner);
return true;
}
}
return false;
}
@GuardedBy("sLock")
private static TaskExecutor getTaskExecutorForTraits(TaskTraits traits) {
return sTaskExecutors[traits.mExtensionId];
}
......@@ -242,9 +248,9 @@ public class PostTask {
@CalledByNative
private static void onNativeSchedulerReady() {
synchronized (sLock) {
Set<TaskRunner> preNativeTaskRunners = sPreNativeTaskRunners;
List<TaskRunnerImpl> preNativeTaskRunners = sPreNativeTaskRunners;
sPreNativeTaskRunners = null;
for (TaskRunner taskRunner : preNativeTaskRunners) {
for (TaskRunnerImpl taskRunner : preNativeTaskRunners) {
taskRunner.initNativeTaskRunner();
}
}
......@@ -254,8 +260,8 @@ public class PostTask {
@CalledByNative
private static void onNativeSchedulerShutdown() {
synchronized (sLock) {
sPreNativeTaskRunners =
Collections.newSetFromMap(new WeakHashMap<TaskRunner, Boolean>());
sPreNativeTaskRunners = new ArrayList<>();
sTaskExecutors = getInitialTaskExecutors();
}
}
......
......@@ -6,30 +6,18 @@ package org.chromium.base.task;
import java.util.concurrent.atomic.AtomicInteger;
import javax.annotation.concurrent.GuardedBy;
/**
* Implementation of the abstract class {@link SequencedTaskRunner}. Uses AsyncTasks until
* native APIs are available.
*/
public class SequencedTaskRunnerImpl extends TaskRunnerImpl implements SequencedTaskRunner {
private AtomicInteger mPendingTasks = new AtomicInteger();
@GuardedBy("mLock")
private int mNumUnfinishedNativeTasks;
/**
* @param traits The TaskTraits associated with this SequencedTaskRunnerImpl.
*/
SequencedTaskRunnerImpl(TaskTraits traits) {
super(traits, "SequencedTaskRunnerImpl", TaskRunnerType.SEQUENCED);
disableLifetimeCheck();
}
@Override
public void initNativeTaskRunner() {
synchronized (mLock) {
migratePreNativeTasksToNative();
}
}
@Override
......@@ -46,24 +34,4 @@ public class SequencedTaskRunnerImpl extends TaskRunnerImpl implements Sequenced
super.schedulePreNativeTask();
}
}
@Override
@SuppressWarnings("GuardedBy") // The GuardedBy check doesn't understand super.mLock == mLock.
public void postDelayedTaskToNative(Runnable runnable, long delay) {
synchronized (mLock) {
if (mNumUnfinishedNativeTasks++ == 0) {
initNativeTaskRunnerInternal();
}
Runnable r = () -> {
// No need for try/finally since exceptions here will kill the app entirely.
runnable.run();
synchronized (mLock) {
if (--mNumUnfinishedNativeTasks == 0) {
destroyInternal();
}
}
};
super.postDelayedTaskToNative(r, delay);
}
}
}
......@@ -46,13 +46,10 @@ public class SingleThreadTaskRunnerImpl extends TaskRunnerImpl implements Single
@Override
public boolean belongsToCurrentThread() {
synchronized (mLock) {
if (mNativeTaskRunnerAndroid != 0)
return TaskRunnerImplJni.get().belongsToCurrentThread(mNativeTaskRunnerAndroid);
}
if (mHandler != null) return mHandler.getLooper().getThread() == Thread.currentThread();
assert (false);
return false;
Boolean belongs = belongsToCurrentThreadInternal();
if (belongs != null) return belongs.booleanValue();
assert mHandler != null;
return mHandler.getLooper().getThread() == Thread.currentThread();
}
@Override
......
......@@ -19,18 +19,6 @@ public interface TaskRunner {
*/
void postTask(Runnable task);
/**
* Cleans up native side of TaskRunner. This must be called if the TaskRunner will have a
* shorter lifetime than the process it is created in. Thread-safe and safe to be called
* multiple times.
*/
void destroy();
/**
* Set this for instances that are cached between tests.
*/
void disableLifetimeCheck();
/**
* Posts a task to run after a specified delay.
*
......@@ -38,10 +26,4 @@ public interface TaskRunner {
* @param delay The delay in milliseconds before the task can be run.
*/
void postDelayedTask(Runnable task, long delay);
/**
* Instructs the TaskRunner to initialize the native TaskRunner and migrate any tasks over to
* it.
*/
void initNativeTaskRunner();
}
......@@ -9,14 +9,17 @@ import android.util.Pair;
import androidx.annotation.Nullable;
import org.chromium.base.LifetimeAssert;
import org.chromium.base.TraceEvent;
import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.annotations.NativeMethods;
import java.lang.ref.ReferenceQueue;
import java.lang.ref.WeakReference;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Set;
import javax.annotation.concurrent.GuardedBy;
......@@ -26,27 +29,79 @@ import javax.annotation.concurrent.GuardedBy;
*/
@JNINamespace("base")
public class TaskRunnerImpl implements TaskRunner {
// TaskRunnerCleaners are enqueued to this queue when their WeakReference to a TaskRunnerIml is
// cleared.
private static final ReferenceQueue<Object> sQueue = new ReferenceQueue<>();
// Holds a strong reference to the pending TaskRunnerCleaners so they don't get GC'd before the
// TaskRunnerImpl they're weakly referencing does.
@GuardedBy("sCleaners")
private static final Set<TaskRunnerCleaner> sCleaners = new HashSet<>();
private final TaskTraits mTaskTraits;
private final String mTraceEvent;
private final @TaskRunnerType int mTaskRunnerType;
protected final Object mLock = new Object();
private final Object mLock = new Object();
@GuardedBy("mLock")
protected long mNativeTaskRunnerAndroid;
protected final Runnable mRunPreNativeTaskClosure = this::runPreNativeTask;
private long mNativeTaskRunnerAndroid;
@GuardedBy("mLock")
private boolean mIsDestroying;
private final LifetimeAssert mLifetimeAssert = LifetimeAssert.create(this);
private boolean mDidOneTimeInitialization;
protected final Runnable mRunPreNativeTaskClosure = this::runPreNativeTask;
@Nullable
protected LinkedList<Runnable> mPreNativeTasks = new LinkedList<>();
private LinkedList<Runnable> mPreNativeTasks;
@Nullable
protected List<Pair<Runnable, Long>> mPreNativeDelayedTasks = new ArrayList<>();
private List<Pair<Runnable, Long>> mPreNativeDelayedTasks;
private static class TaskRunnerCleaner extends WeakReference<TaskRunnerImpl> {
final long mNativePtr;
TaskRunnerCleaner(TaskRunnerImpl runner) {
super(runner, sQueue);
mNativePtr = runner.mNativeTaskRunnerAndroid;
}
void destroy() {
TaskRunnerImplJni.get().destroy(mNativePtr);
}
}
/**
* The lifecycle for a TaskRunner is very complicated. Some task runners are static and never
* destroyed, some have a task posted to them and are immediately allowed to be GC'd by the
* creator, but if native isn't initialized the task would be lost if this were to be GC'd.
* This makes an explicit destroy impractical as it can't be enforced on static runners, and
* wouldn't actually destroy the runner before native initialization as that would cause tasks
* to be lost. A finalizer could give us the correct behaviour here, but finalizers are banned
* due to the performance cost they impose, and all of the many correctness gotchas around
* implementing a finalizer.
*
* The strategy we've gone with here is to use a ReferenceQueue to keep track of which
* TaskRunners are no longer reachable (and may have been GC'd), and to delete the native
* counterpart for those TaskRunners by polling the queue when doing non-performance-critical
* operations on TaskRunners, like creating a new one. In order to prevent this TaskRunner from
* being GC'd before its tasks can be posted to the native runner, PostTask holds a strong
* reference to each TaskRunner with a task posted to it before native initialization.
*/
private static void destroyGarbageCollectedTaskRunners() {
while (true) {
// ReferenceQueue#poll immediately removes and returns an element from the queue,
// returning null if the queue is empty.
@SuppressWarnings("unchecked")
TaskRunnerCleaner cleaner = (TaskRunnerCleaner) sQueue.poll();
if (cleaner == null) return;
cleaner.destroy();
synchronized (sCleaners) {
sCleaners.remove(cleaner);
}
}
}
/**
* @param traits The TaskTraits associated with this TaskRunnerImpl.
*/
TaskRunnerImpl(TaskTraits traits) {
this(traits, "TaskRunnerImpl", TaskRunnerType.BASE);
destroyGarbageCollectedTaskRunners();
}
/**
......@@ -60,29 +115,6 @@ public class TaskRunnerImpl implements TaskRunner {
mTaskTraits = traits.withExplicitDestination();
mTraceEvent = traceCategory + ".PreNativeTask.run";
mTaskRunnerType = taskRunnerType;
if (!PostTask.registerPreNativeTaskRunnerLocked(this)) initNativeTaskRunner();
}
@Override
public void destroy() {
synchronized (mLock) {
LifetimeAssert.setSafeToGc(mLifetimeAssert, true);
mIsDestroying = true;
destroyInternal();
}
}
@GuardedBy("mLock")
protected void destroyInternal() {
if (mNativeTaskRunnerAndroid != 0) {
TaskRunnerImplJni.get().destroy(mNativeTaskRunnerAndroid);
}
mNativeTaskRunnerAndroid = 0;
}
@Override
public void disableLifetimeCheck() {
LifetimeAssert.setSafeToGc(mLifetimeAssert, true);
}
@Override
......@@ -93,9 +125,9 @@ public class TaskRunnerImpl implements TaskRunner {
@Override
public void postDelayedTask(Runnable task, long delay) {
synchronized (mLock) {
assert !mIsDestroying;
oneTimeInitialization();
if (mPreNativeTasks == null) {
postDelayedTaskToNative(task, delay);
TaskRunnerImplJni.get().postDelayedTask(mNativeTaskRunnerAndroid, task, delay);
return;
}
// We don't expect a whole lot of these, if that changes consider pooling them.
......@@ -112,6 +144,30 @@ public class TaskRunnerImpl implements TaskRunner {
}
}
protected Boolean belongsToCurrentThreadInternal() {
// TODO(https://crbug.com/1026641): This function shouldn't be here, and should only be used
// by derived classes (eg. SingleThreadTaskRunner) until it is moved there, as TaskRunner
// has no notion of belonging to a thread.
assert !getClass().equals(TaskRunnerImpl.class);
synchronized (mLock) {
oneTimeInitialization();
if (mNativeTaskRunnerAndroid == 0) return null;
return TaskRunnerImplJni.get().belongsToCurrentThread(mNativeTaskRunnerAndroid);
}
}
@GuardedBy("mLock")
private void oneTimeInitialization() {
if (mDidOneTimeInitialization) return;
mDidOneTimeInitialization = true;
if (!PostTask.registerPreNativeTaskRunner(this)) {
initNativeTaskRunner();
} else {
mPreNativeTasks = new LinkedList<>();
mPreNativeDelayedTasks = new ArrayList<>();
}
}
/**
* Must be overridden in subclasses, schedules a call to runPreNativeTask() at an appropriate
* time.
......@@ -151,42 +207,40 @@ public class TaskRunnerImpl implements TaskRunner {
* Instructs the TaskRunner to initialize the native TaskRunner and migrate any tasks over to
* it.
*/
@Override
public void initNativeTaskRunner() {
/* package */ void initNativeTaskRunner() {
synchronized (mLock) {
initNativeTaskRunnerInternal();
migratePreNativeTasksToNative();
}
}
@GuardedBy("mLock")
protected void initNativeTaskRunnerInternal() {
if (mNativeTaskRunnerAndroid == 0) {
assert mNativeTaskRunnerAndroid == 0;
mNativeTaskRunnerAndroid = TaskRunnerImplJni.get().init(mTaskRunnerType,
mTaskTraits.mPriority, mTaskTraits.mMayBlock, mTaskTraits.mUseThreadPool,
mTaskTraits.mExtensionId, mTaskTraits.mExtensionData);
synchronized (sCleaners) {
sCleaners.add(new TaskRunnerCleaner(this));
}
migratePreNativeTasksToNative();
}
// Destroying GC'd task runners here isn't strictly necessary, but the performance of
// initNativeTaskRunner() isn't critical, and calling the function more often will help
// prevent any potential build-up of orphaned native task runners.
destroyGarbageCollectedTaskRunners();
}
@GuardedBy("mLock")
protected void migratePreNativeTasksToNative() {
private void migratePreNativeTasksToNative() {
if (mPreNativeTasks != null) {
for (Runnable task : mPreNativeTasks) {
postDelayedTaskToNative(task, 0);
TaskRunnerImplJni.get().postDelayedTask(mNativeTaskRunnerAndroid, task, 0);
}
mPreNativeTasks = null;
}
if (mPreNativeDelayedTasks != null) {
for (Pair<Runnable, Long> task : mPreNativeDelayedTasks) {
postDelayedTaskToNative(task.first, task.second);
TaskRunnerImplJni.get().postDelayedTask(
mNativeTaskRunnerAndroid, task.first, task.second);
}
mPreNativeTasks = null;
mPreNativeDelayedTasks = null;
}
}
@GuardedBy("mLock")
protected void postDelayedTaskToNative(Runnable r, long delay) {
TaskRunnerImplJni.get().postDelayedTask(mNativeTaskRunnerAndroid, r, delay);
}
@NativeMethods
interface Natives {
// NB due to Proguard obfuscation it's easiest to pass the traits via arguments.
......
......@@ -65,7 +65,6 @@ public class PostTaskTest {
// A SingleThreadTaskRunner with default traits will run in the native thread pool
// and tasks posted won't run until after the native library has loaded.
assertNotNull(taskQueue);
taskQueue.destroy();
}
@Test
......@@ -73,14 +72,10 @@ public class PostTaskTest {
public void testCreateSequencedTaskRunner() {
TaskRunner taskQueue = PostTask.createSequencedTaskRunner(TaskTraits.USER_BLOCKING);
List<Integer> orderList = new ArrayList<>();
try {
SchedulerTestHelpers.postRecordOrderTask(taskQueue, orderList, 1);
SchedulerTestHelpers.postRecordOrderTask(taskQueue, orderList, 2);
SchedulerTestHelpers.postRecordOrderTask(taskQueue, orderList, 3);
SchedulerTestHelpers.postTaskAndBlockUntilRun(taskQueue);
} finally {
taskQueue.destroy();
}
SchedulerTestHelpers.postRecordOrderTask(taskQueue, orderList, 1);
SchedulerTestHelpers.postRecordOrderTask(taskQueue, orderList, 2);
SchedulerTestHelpers.postRecordOrderTask(taskQueue, orderList, 3);
SchedulerTestHelpers.postTaskAndBlockUntilRun(taskQueue);
assertThat(orderList, contains(1, 2, 3));
}
......@@ -91,11 +86,7 @@ public class PostTaskTest {
TaskRunner taskQueue = PostTask.createTaskRunner(TaskTraits.USER_BLOCKING);
// This should not timeout.
try {
SchedulerTestHelpers.postTaskAndBlockUntilRun(taskQueue);
} finally {
taskQueue.destroy();
}
SchedulerTestHelpers.postTaskAndBlockUntilRun(taskQueue);
}
@Test
......
......@@ -33,14 +33,10 @@ public class SequencedTaskRunnerImplTest {
public void testPreNativeTasksRunInOrder() {
TaskRunner taskQueue = new SequencedTaskRunnerImpl(TaskTraits.USER_BLOCKING);
List<Integer> orderList = new ArrayList<>();
try {
SchedulerTestHelpers.postRecordOrderTask(taskQueue, orderList, 1);
SchedulerTestHelpers.postRecordOrderTask(taskQueue, orderList, 2);
SchedulerTestHelpers.postRecordOrderTask(taskQueue, orderList, 3);
SchedulerTestHelpers.postTaskAndBlockUntilRun(taskQueue);
} finally {
taskQueue.destroy();
}
SchedulerTestHelpers.postRecordOrderTask(taskQueue, orderList, 1);
SchedulerTestHelpers.postRecordOrderTask(taskQueue, orderList, 2);
SchedulerTestHelpers.postRecordOrderTask(taskQueue, orderList, 3);
SchedulerTestHelpers.postTaskAndBlockUntilRun(taskQueue);
assertThat(orderList, contains(1, 2, 3));
}
}
......@@ -62,7 +62,6 @@ public class SingleThreadTaskRunnerImplTest {
SchedulerTestHelpers.postRecordOrderTask(taskQueue, orderList, 1);
SchedulerTestHelpers.postRecordOrderTask(taskQueue, orderList, 2);
SchedulerTestHelpers.postRecordOrderTask(taskQueue, orderList, 3);
taskQueue.destroy();
SchedulerTestHelpers.preNativeRunUntilIdle(mHandlerThread);
assertThat(orderList, contains(1, 2, 3));
......@@ -74,21 +73,13 @@ public class SingleThreadTaskRunnerImplTest {
// The handler created during test setup belongs to a different thread.
SingleThreadTaskRunner taskQueue =
new SingleThreadTaskRunnerImpl(mHandler, TaskTraits.USER_BLOCKING, false);
try {
Assert.assertFalse(taskQueue.belongsToCurrentThread());
} finally {
taskQueue.destroy();
}
Assert.assertFalse(taskQueue.belongsToCurrentThread());
// We create a handler belonging to current thread.
Looper.prepare();
SingleThreadTaskRunner taskQueueCurrentThread =
new SingleThreadTaskRunnerImpl(new Handler(), TaskTraits.USER_BLOCKING, false);
try {
Assert.assertTrue(taskQueueCurrentThread.belongsToCurrentThread());
} finally {
taskQueueCurrentThread.destroy();
}
Assert.assertTrue(taskQueueCurrentThread.belongsToCurrentThread());
}
@Test
......@@ -98,27 +89,22 @@ public class SingleThreadTaskRunnerImplTest {
new SingleThreadTaskRunnerImpl(mHandler, TaskTraits.USER_BLOCKING, true);
TaskRunner lowPriorityQueue =
new SingleThreadTaskRunnerImpl(mHandler, TaskTraits.USER_BLOCKING, false);
try {
List<Integer> orderList = new ArrayList<>();
// We want to post all these tasks atomically but we're not on the mHandlerThread so
// we need to post a task to do that for us.
lowPriorityQueue.postTask(new Runnable() {
@Override
public void run() {
SchedulerTestHelpers.postRecordOrderTask(lowPriorityQueue, orderList, 1);
SchedulerTestHelpers.postRecordOrderTask(lowPriorityQueue, orderList, 2);
SchedulerTestHelpers.postRecordOrderTask(lowPriorityQueue, orderList, 3);
SchedulerTestHelpers.postRecordOrderTask(highPriorityQueue, orderList, 10);
SchedulerTestHelpers.postRecordOrderTask(highPriorityQueue, orderList, 20);
SchedulerTestHelpers.postRecordOrderTask(highPriorityQueue, orderList, 30);
}
});
List<Integer> orderList = new ArrayList<>();
// We want to post all these tasks atomically but we're not on the mHandlerThread so
// we need to post a task to do that for us.
lowPriorityQueue.postTask(new Runnable() {
@Override
public void run() {
SchedulerTestHelpers.postRecordOrderTask(lowPriorityQueue, orderList, 1);
SchedulerTestHelpers.postRecordOrderTask(lowPriorityQueue, orderList, 2);
SchedulerTestHelpers.postRecordOrderTask(lowPriorityQueue, orderList, 3);
SchedulerTestHelpers.postRecordOrderTask(highPriorityQueue, orderList, 10);
SchedulerTestHelpers.postRecordOrderTask(highPriorityQueue, orderList, 20);
SchedulerTestHelpers.postRecordOrderTask(highPriorityQueue, orderList, 30);
}
});
SchedulerTestHelpers.preNativeRunUntilIdle(mHandlerThread);
assertThat(orderList, contains(10, 20, 30, 1, 2, 3));
} finally {
highPriorityQueue.destroy();
lowPriorityQueue.destroy();
}
SchedulerTestHelpers.preNativeRunUntilIdle(mHandlerThread);
assertThat(orderList, contains(10, 20, 30, 1, 2, 3));
}
}
......@@ -28,10 +28,6 @@ public class TaskRunnerImplTest {
TaskRunner taskQueue = new TaskRunnerImpl(TaskTraits.USER_BLOCKING);
// This should not time out.
try {
SchedulerTestHelpers.postTaskAndBlockUntilRun(taskQueue);
} finally {
taskQueue.destroy();
}
SchedulerTestHelpers.postTaskAndBlockUntilRun(taskQueue);
}
}
......@@ -387,7 +387,6 @@ class BLEHandler extends BluetoothGattServerCallback implements Closeable {
mTaskRunner.postTask(() -> {
maybeStopAdvertising();
BLEHandlerJni.get().stop();
mTaskRunner.destroy();
});
}
......
......@@ -241,11 +241,6 @@ public class TabPersistentStore extends TabPersister {
mPrefetchTabListToMergeTasks.add(Pair.create(task, mergedFileName));
}
}
if (!needsInitialization) {
// If a non-sequenced task runner was created above, destroy it now.
taskRunner.destroy();
}
}
@Override
......
......@@ -4,6 +4,7 @@
package org.chromium.chrome.browser;
import android.os.Handler;
import android.os.Looper;
import org.junit.Assert;
......@@ -14,6 +15,7 @@ import org.chromium.content_public.browser.test.NestedSystemMessageHandler;
import org.chromium.content_public.browser.test.util.CriteriaHelper;
import java.util.concurrent.Callable;
import java.util.concurrent.atomic.AtomicBoolean;
/**
* Utilities for use in Native Java Unit Tests.
......@@ -29,10 +31,21 @@ public class UnitTestUtils {
assert ThreadUtils.runningOnUiThread();
boolean isSatisfied = criteria.call();
TimeoutTimer timer = new TimeoutTimer(CriteriaHelper.DEFAULT_MAX_TIME_TO_POLL);
Handler handler = new Handler(Looper.myLooper());
AtomicBoolean called = new AtomicBoolean(true);
while (!isSatisfied && !timer.isTimedOut()) {
// Ensure we pump the message handler in case no new tasks arrive.
if (called.get()) {
called.set(false);
handler.postDelayed(
() -> { called.set(true); }, CriteriaHelper.DEFAULT_POLLING_INTERVAL);
}
NestedSystemMessageHandler.runSingleNestedLooperTask(Looper.myQueue());
isSatisfied = criteria.call();
}
Assert.assertFalse("Timed out waiting for condition", timer.isTimedOut());
Assert.assertTrue(isSatisfied);
}
}
\ No newline at end of file
......@@ -248,7 +248,6 @@ public class WebApkServiceConnectionManager {
private void destroyTaskRunner() {
if (mTaskRunner == null) return;
mTaskRunner.destroy();
mTaskRunner = null;
}
}
......@@ -4,18 +4,14 @@
#include "base/android/jni_android.h"
#include "chrome/android/native_j_unittests_jni_headers/InstalledAppProviderTest_jni.h"
#include "content/public/test/browser_task_environment.h"
#include "testing/gtest/include/gtest/gtest.h"
using base::android::AttachCurrentThread;
// TODO(crbug.com/1058790): Reduce the test suite flakiness and re-enable the
// tests.
class DISABLED_InstalledAppProviderTest : public ::testing::Test {
class InstalledAppProviderTest : public ::testing::Test {
public:
DISABLED_InstalledAppProviderTest()
: task_environment_(content::BrowserTaskEnvironment::MainThreadType::UI),
j_test_(
InstalledAppProviderTest()
: j_test_(
Java_InstalledAppProviderTest_Constructor(AttachCurrentThread())) {}
void SetUp() override {
......@@ -27,8 +23,7 @@ class DISABLED_InstalledAppProviderTest : public ::testing::Test {
}
private:
content::BrowserTaskEnvironment task_environment_;
base::android::ScopedJavaGlobalRef<jobject> j_test_;
};
JAVA_TESTS(DISABLED_InstalledAppProviderTest, j_test())
JAVA_TESTS(InstalledAppProviderTest, j_test())
......@@ -54,7 +54,6 @@ public class BrowserTaskExecutor implements TaskExecutor {
SingleThreadTaskRunner taskRunner =
new SingleThreadTaskRunnerImpl(ThreadUtils.getUiThreadHandler(), taskTraits,
shouldPrioritizeTraits(taskTraits));
taskRunner.disableLifetimeCheck();
mTaskRunners.put(taskTraits, new WeakReference<>(taskRunner));
return taskRunner;
}
......
......@@ -101,30 +101,22 @@ public class NativePostTaskTest {
startNativeScheduler();
TaskRunner taskQueue = PostTask.createTaskRunner(TaskTraits.USER_BLOCKING);
// This should not time out.
try {
SchedulerTestHelpers.postDelayedTaskAndBlockUntilRun(taskQueue, 1);
} finally {
taskQueue.destroy();
}
SchedulerTestHelpers.postDelayedTaskAndBlockUntilRun(taskQueue, 1);
}
private void testRunningTasksInSequence(TaskRunner taskQueue) {
try {
List<Integer> orderListImmediate = new ArrayList<>();
List<Integer> orderListDelayed = new ArrayList<>();
List<Integer> orderListImmediate = new ArrayList<>();
List<Integer> orderListDelayed = new ArrayList<>();
SchedulerTestHelpers.postThreeTasksInOrder(taskQueue, orderListImmediate);
SchedulerTestHelpers.postTaskAndBlockUntilRun(taskQueue);
SchedulerTestHelpers.postThreeTasksInOrder(taskQueue, orderListImmediate);
SchedulerTestHelpers.postTaskAndBlockUntilRun(taskQueue);
assertThat(orderListImmediate, contains(1, 2, 3));
assertThat(orderListImmediate, contains(1, 2, 3));
SchedulerTestHelpers.postThreeDelayedTasksInOrder(taskQueue, orderListDelayed);
SchedulerTestHelpers.postDelayedTaskAndBlockUntilRun(taskQueue, 1);
SchedulerTestHelpers.postThreeDelayedTasksInOrder(taskQueue, orderListDelayed);
SchedulerTestHelpers.postDelayedTaskAndBlockUntilRun(taskQueue, 1);
assertThat(orderListDelayed, contains(1, 2, 3));
} finally {
taskQueue.destroy();
}
assertThat(orderListDelayed, contains(1, 2, 3));
}
@Test
......@@ -176,16 +168,12 @@ public class NativePostTaskTest {
}
});
try {
// The task should run at some point after the migration, so the test shouldn't
// time out.
synchronized (lock) {
while (!taskExecuted.get()) {
lock.wait();
}
// The task should run at some point after the migration, so the test shouldn't
// time out.
synchronized (lock) {
while (!taskExecuted.get()) {
lock.wait();
}
} finally {
taskQueue.destroy();
}
}
......@@ -195,11 +183,7 @@ public class NativePostTaskTest {
List<Integer> orderListImmediate = new ArrayList<>();
List<Integer> orderListDelayed = new ArrayList<>();
TaskRunner taskQueue = PostTask.createSequencedTaskRunner(TaskTraits.USER_BLOCKING);
try {
performSequencedTestSchedulerMigration(taskQueue, orderListImmediate, orderListDelayed);
} finally {
taskQueue.destroy();
}
performSequencedTestSchedulerMigration(taskQueue, orderListImmediate, orderListDelayed);
assertThat(orderListImmediate, contains(1, 2, 3, 4));
assertThat(orderListDelayed, contains(1, 2, 3));
......@@ -211,11 +195,7 @@ public class NativePostTaskTest {
List<Integer> orderListImmediate = new ArrayList<>();
List<Integer> orderListDelayed = new ArrayList<>();
TaskRunner taskQueue = PostTask.createSingleThreadTaskRunner(TaskTraits.USER_BLOCKING);
try {
performSequencedTestSchedulerMigration(taskQueue, orderListImmediate, orderListDelayed);
} finally {
taskQueue.destroy();
}
performSequencedTestSchedulerMigration(taskQueue, orderListImmediate, orderListDelayed);
assertThat(orderListImmediate, contains(1, 2, 3, 4));
assertThat(orderListDelayed, contains(1, 2, 3));
......
......@@ -71,17 +71,13 @@ public class UiThreadSchedulerTest {
public void testSimpleUiThreadPostingBeforeNativeLoaded() {
TaskRunner uiThreadTaskRunner =
PostTask.createSingleThreadTaskRunner(UiThreadTaskTraits.DEFAULT);
try {
List<Integer> orderList = new ArrayList<>();
SchedulerTestHelpers.postRecordOrderTask(uiThreadTaskRunner, orderList, 1);
SchedulerTestHelpers.postRecordOrderTask(uiThreadTaskRunner, orderList, 2);
SchedulerTestHelpers.postRecordOrderTask(uiThreadTaskRunner, orderList, 3);
SchedulerTestHelpers.postTaskAndBlockUntilRun(uiThreadTaskRunner);
List<Integer> orderList = new ArrayList<>();
SchedulerTestHelpers.postRecordOrderTask(uiThreadTaskRunner, orderList, 1);
SchedulerTestHelpers.postRecordOrderTask(uiThreadTaskRunner, orderList, 2);
SchedulerTestHelpers.postRecordOrderTask(uiThreadTaskRunner, orderList, 3);
SchedulerTestHelpers.postTaskAndBlockUntilRun(uiThreadTaskRunner);
assertThat(orderList, contains(1, 2, 3));
} finally {
uiThreadTaskRunner.destroy();
}
assertThat(orderList, contains(1, 2, 3));
}
@Test
......@@ -91,28 +87,23 @@ public class UiThreadSchedulerTest {
PostTask.createSingleThreadTaskRunner(UiThreadTaskTraits.DEFAULT);
TaskRunner bootstrapTaskRunner =
PostTask.createSingleThreadTaskRunner(UiThreadTaskTraits.BOOTSTRAP);
try {
List<Integer> orderList = new ArrayList<>();
// We want to enqueue these tasks atomically but we're not on the mUiThread. So
// we post a task to enqueue them.
PostTask.postTask(UiThreadTaskTraits.DEFAULT, new Runnable() {
@Override
public void run() {
SchedulerTestHelpers.postRecordOrderTask(defaultTaskRunner, orderList, 1);
SchedulerTestHelpers.postRecordOrderTask(defaultTaskRunner, orderList, 2);
SchedulerTestHelpers.postRecordOrderTask(defaultTaskRunner, orderList, 3);
SchedulerTestHelpers.postRecordOrderTask(bootstrapTaskRunner, orderList, 10);
SchedulerTestHelpers.postRecordOrderTask(bootstrapTaskRunner, orderList, 20);
SchedulerTestHelpers.postRecordOrderTask(bootstrapTaskRunner, orderList, 30);
}
});
List<Integer> orderList = new ArrayList<>();
// We want to enqueue these tasks atomically but we're not on the mUiThread. So
// we post a task to enqueue them.
PostTask.postTask(UiThreadTaskTraits.DEFAULT, new Runnable() {
@Override
public void run() {
SchedulerTestHelpers.postRecordOrderTask(defaultTaskRunner, orderList, 1);
SchedulerTestHelpers.postRecordOrderTask(defaultTaskRunner, orderList, 2);
SchedulerTestHelpers.postRecordOrderTask(defaultTaskRunner, orderList, 3);
SchedulerTestHelpers.postRecordOrderTask(bootstrapTaskRunner, orderList, 10);
SchedulerTestHelpers.postRecordOrderTask(bootstrapTaskRunner, orderList, 20);
SchedulerTestHelpers.postRecordOrderTask(bootstrapTaskRunner, orderList, 30);
}
});
SchedulerTestHelpers.preNativeRunUntilIdle(mUiThread);
assertThat(orderList, contains(10, 20, 30, 1, 2, 3));
} finally {
defaultTaskRunner.destroy();
bootstrapTaskRunner.destroy();
}
SchedulerTestHelpers.preNativeRunUntilIdle(mUiThread);
assertThat(orderList, contains(10, 20, 30, 1, 2, 3));
}
@Test
......@@ -120,21 +111,17 @@ public class UiThreadSchedulerTest {
public void testUiThreadTaskRunnerMigrationToNative() {
TaskRunner uiThreadTaskRunner =
PostTask.createSingleThreadTaskRunner(UiThreadTaskTraits.DEFAULT);
try {
List<Integer> orderList = new ArrayList<>();
SchedulerTestHelpers.postRecordOrderTask(uiThreadTaskRunner, orderList, 1);
List<Integer> orderList = new ArrayList<>();
SchedulerTestHelpers.postRecordOrderTask(uiThreadTaskRunner, orderList, 1);
postRepeatingTaskAndStartNativeSchedulerThenWaitForTaskToRun(
uiThreadTaskRunner, new Runnable() {
@Override
public void run() {
orderList.add(2);
}
});
assertThat(orderList, contains(1, 2));
} finally {
uiThreadTaskRunner.destroy();
}
postRepeatingTaskAndStartNativeSchedulerThenWaitForTaskToRun(
uiThreadTaskRunner, new Runnable() {
@Override
public void run() {
orderList.add(2);
}
});
assertThat(orderList, contains(1, 2));
}
@Test
......@@ -142,19 +129,15 @@ public class UiThreadSchedulerTest {
public void testSimpleUiThreadPostingAfterNativeLoaded() {
TaskRunner uiThreadTaskRunner =
PostTask.createSingleThreadTaskRunner(UiThreadTaskTraits.DEFAULT);
try {
startContentMainOnUiThread();
startContentMainOnUiThread();
uiThreadTaskRunner.postTask(new Runnable() {
@Override
public void run() {
Assert.assertTrue(ThreadUtils.runningOnUiThread());
}
});
SchedulerTestHelpers.postTaskAndBlockUntilRun(uiThreadTaskRunner);
} finally {
uiThreadTaskRunner.destroy();
}
uiThreadTaskRunner.postTask(new Runnable() {
@Override
public void run() {
Assert.assertTrue(ThreadUtils.runningOnUiThread());
}
});
SchedulerTestHelpers.postTaskAndBlockUntilRun(uiThreadTaskRunner);
}
@Test
......@@ -162,22 +145,18 @@ public class UiThreadSchedulerTest {
public void testTaskNotRunOnUiThreadWithoutUiThreadTaskTraits() {
TaskRunner uiThreadTaskRunner =
PostTask.createSingleThreadTaskRunner(TaskTraits.USER_BLOCKING);
try {
// Test times out without this.
UiThreadSchedulerTestUtils.postBrowserMainLoopStartupTasks(true);
startContentMainOnUiThread();
// Test times out without this.
UiThreadSchedulerTestUtils.postBrowserMainLoopStartupTasks(true);
startContentMainOnUiThread();
uiThreadTaskRunner.postTask(new Runnable() {
@Override
public void run() {
Assert.assertFalse(ThreadUtils.runningOnUiThread());
}
});
uiThreadTaskRunner.postTask(new Runnable() {
@Override
public void run() {
Assert.assertFalse(ThreadUtils.runningOnUiThread());
}
});
SchedulerTestHelpers.postTaskAndBlockUntilRun(uiThreadTaskRunner);
} finally {
uiThreadTaskRunner.destroy();
}
SchedulerTestHelpers.postTaskAndBlockUntilRun(uiThreadTaskRunner);
}
@Test
......
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