Commit 5a5450c8 authored by Gabriel Charette's avatar Gabriel Charette Committed by Commit Bot

[base] Add destination checks in TaskTraits to Java traits as well

Java was never migrated to explicit-destination-required like
was done on the C++ side. So there are lots of callers using
default=>THREAD_POOL :
https://source.chromium.org/search?q=%5CbTaskTraits%5C.(BEST_EFFORT%7CUSER_VISIBLE%7CUSER_BLOCKING)&ss=chromium

This is a problem now that we want to make the C++ checks
runtime (applies to Java as well) in
https://chromium-review.googlesource.com/c/chromium/src/+/1978821

Decouple the C++ and the Java migration by promoting implicit
Java destination to an explicit C++ request to run on the thread
pool.

Bug: 1026641
Change-Id: Ifcd18a53643154393fbb1338ea574c0ba219a914
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1983461
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: default avatarRoss McIlroy <rmcilroy@chromium.org>
Reviewed-by: default avatarSami Kyöstilä <skyostil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#730536}
parent a1341c48
......@@ -87,10 +87,11 @@ public class PostTask {
if (sPreNativeTaskRunners != null || taskTraits.mIsChoreographerFrame) {
getTaskExecutorForTraits(taskTraits).postDelayedTask(taskTraits, task, delay);
} else {
PostTaskJni.get().postDelayedTask(taskTraits.mPrioritySetExplicitly,
taskTraits.mPriority, taskTraits.mMayBlock, taskTraits.mUseThreadPool,
taskTraits.mUseCurrentThread, taskTraits.mExtensionId,
taskTraits.mExtensionData, task, delay);
TaskTraits postedTraits = taskTraits.withExplicitDestination();
PostTaskJni.get().postDelayedTask(postedTraits.mPrioritySetExplicitly,
postedTraits.mPriority, postedTraits.mMayBlock, postedTraits.mUseThreadPool,
postedTraits.mUseCurrentThread, postedTraits.mExtensionId,
postedTraits.mExtensionData, task, delay);
}
}
}
......
......@@ -57,7 +57,7 @@ public class TaskRunnerImpl implements TaskRunner {
*/
protected TaskRunnerImpl(
TaskTraits traits, String traceCategory, @TaskRunnerType int taskRunnerType) {
mTaskTraits = traits;
mTaskTraits = traits.withExplicitDestination();
mTraceEvent = traceCategory + ".PreNativeTask.run";
mTaskRunnerType = taskRunnerType;
if (!PostTask.registerPreNativeTaskRunnerLocked(this)) initNativeTaskRunner();
......
......@@ -22,7 +22,9 @@ public class TaskTraits {
// Keep in sync with base::TaskTraitsExtensionStorage::kStorageSize
public static final int EXTENSION_STORAGE_SIZE = 8;
// Convenience variables explicitly specifying common priorities
// Convenience variables explicitly specifying common priorities.
// These also imply THREAD_POOL unless explicitly overwritten.
// TODO(1026641): Make destination explicit in Java too.
// This task will only be scheduled when machine resources are available. Once
// running, it may be descheduled if higher priority work arrives (in this
......@@ -176,6 +178,21 @@ public class TaskTraits {
return taskTraits;
}
/**
* Returns a TaskTraits with an explicit destination.
*
* The C++ side enforces that a destination _must_ be specified. The Java
* side loosely considers lack of destination as implying THREAD_POOL
* destination.
* TODO(1026641): Bring the Java side inline with the C++ side.
*/
public TaskTraits withExplicitDestination() {
if (!mUseThreadPool && !mUseCurrentThread && !hasExtension()) {
return this.threadPool();
}
return this;
}
@Override
public boolean equals(@Nullable Object object) {
if (object == this) {
......
......@@ -386,6 +386,16 @@ class BASE_EXPORT TaskTraits {
use_thread_pool_(use_thread_pool),
use_current_thread_(use_current_thread) {
static_assert(sizeof(TaskTraits) == 16, "Keep this constructor up to date");
// Same assert as in the regular TaskTraits constructor.
const bool has_extension =
(extension_.extension_id !=
TaskTraitsExtensionStorage::kInvalidExtensionId);
DCHECK(!use_current_thread_ || !use_thread_pool_)
<< "base::CurrentThread is mutually exclusive with base::ThreadPool";
DCHECK(use_thread_pool_ ^ has_extension || use_current_thread_)
<< "Traits must explicitly specify a destination (e.g. ThreadPool or a "
"named thread like BrowserThread, or CurrentThread)";
}
// This bit is set in |priority_|, |shutdown_behavior_| and |thread_policy_|
......
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