Introduce content::GetUIThreadTaskRunner() and its IO counterpart
As agreed upon in design doc @
https://docs.google.com/document/d/1tssusPykvx3g0gvbvU4HxGyn3MjJlIylnsH13-Tv6s4/edit?ts=5e1c66d5&pli=1#bookmark=id.ll79iqi5rlpp
base::ThreadPool counterpart to move away from post_task.h is @
https://chromium-review.googlesource.com/c/chromium/src/+/1977964
API usage migration will be done in a follow-up mega CL.
This CL at least uses it in its own tests.
See https://chromium-review.googlesource.com/c/chromium/src/+/2015655
for how this will look in practice for a few callsites.
Bug: 1026641
Change-Id: I98771fd68a513bf0a4776672a8d75fcbbb200bea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2014055
Commit-Queue: Gabriel Charette <[email protected]>
Reviewed-by: Alexander Timin <[email protected]>
Reviewed-by: François Doray <[email protected]>
Reviewed-by: Darin Fisher <[email protected]>
Cr-Commit-Position: refs/heads/master@{#735734}
diff --git a/base/task/post_task.cc b/base/task/post_task.cc
index 195a29a42..95d4302 100644
--- a/base/task/post_task.cc
+++ b/base/task/post_task.cc
@@ -45,7 +45,8 @@
traits.extension_id() != TaskTraitsExtensionStorage::kInvalidExtensionId;
DCHECK(has_extension ^ traits.use_thread_pool())
<< "A destination (e.g. ThreadPool or BrowserThread) must be specified "
- "to use the post_task.h API.";
+ "to use the post_task.h API. However, you should prefer the direct "
+ "thread_pool.h or browser_thread.h APIs in new code.";
if (traits.use_thread_pool()) {
DCHECK(ThreadPoolInstance::Get())
diff --git a/content/browser/browser_thread_impl.cc b/content/browser/browser_thread_impl.cc
index b3eedf8..87535e6 100644
--- a/content/browser/browser_thread_impl.cc
+++ b/content/browser/browser_thread_impl.cc
@@ -84,6 +84,16 @@
} // namespace
+scoped_refptr<base::SingleThreadTaskRunner> GetUIThreadTaskRunner(
+ const BrowserTaskTraits& traits) {
+ return BrowserTaskExecutor::GetUIThreadTaskRunner(traits);
+}
+
+scoped_refptr<base::SingleThreadTaskRunner> GetIOThreadTaskRunner(
+ const BrowserTaskTraits& traits) {
+ return BrowserTaskExecutor::GetIOThreadTaskRunner(traits);
+}
+
BrowserThreadImpl::BrowserThreadImpl(
ID identifier,
scoped_refptr<base::SingleThreadTaskRunner> task_runner)
diff --git a/content/browser/browser_thread_unittest.cc b/content/browser/browser_thread_unittest.cc
index 2fa2e583..8456adf 100644
--- a/content/browser/browser_thread_unittest.cc
+++ b/content/browser/browser_thread_unittest.cc
@@ -165,8 +165,7 @@
explicit UIThreadDestructionObserver(bool* did_shutdown,
base::OnceClosure callback)
: callback_task_runner_(base::ThreadTaskRunnerHandle::Get()),
- ui_task_runner_(
- base::CreateSingleThreadTaskRunner({BrowserThread::UI})),
+ ui_task_runner_(GetUIThreadTaskRunner({})),
callback_(std::move(callback)),
did_shutdown_(did_shutdown) {
ui_task_runner_->PostTask(FROM_HERE, base::BindOnce(&Watch, this));
@@ -223,8 +222,7 @@
}
TEST_F(BrowserThreadTest, PostTaskViaTaskRunner) {
- scoped_refptr<base::TaskRunner> task_runner =
- base::CreateTaskRunner({BrowserThread::IO});
+ scoped_refptr<base::TaskRunner> task_runner = GetIOThreadTaskRunner({});
base::RunLoop run_loop;
EXPECT_TRUE(task_runner->PostTask(
FROM_HERE, base::BindOnce(&BasicFunction, run_loop.QuitWhenIdleClosure(),
@@ -234,7 +232,7 @@
TEST_F(BrowserThreadTest, PostTaskViaSequencedTaskRunner) {
scoped_refptr<base::SequencedTaskRunner> task_runner =
- base::CreateSequencedTaskRunner({BrowserThread::IO});
+ GetIOThreadTaskRunner({});
base::RunLoop run_loop;
EXPECT_TRUE(task_runner->PostTask(
FROM_HERE, base::BindOnce(&BasicFunction, run_loop.QuitWhenIdleClosure(),
@@ -244,7 +242,7 @@
TEST_F(BrowserThreadTest, PostTaskViaSingleThreadTaskRunner) {
scoped_refptr<base::SingleThreadTaskRunner> task_runner =
- base::CreateSingleThreadTaskRunner({BrowserThread::IO});
+ GetIOThreadTaskRunner({});
base::RunLoop run_loop;
EXPECT_TRUE(task_runner->PostTask(
FROM_HERE, base::BindOnce(&BasicFunction, run_loop.QuitWhenIdleClosure(),
@@ -255,7 +253,7 @@
#if defined(OS_WIN)
TEST_F(BrowserThreadTest, PostTaskViaCOMSTATaskRunner) {
scoped_refptr<base::SingleThreadTaskRunner> task_runner =
- base::CreateCOMSTATaskRunner({BrowserThread::UI});
+ GetUIThreadTaskRunner({});
base::RunLoop run_loop;
EXPECT_TRUE(task_runner->PostTask(
FROM_HERE, base::BindOnce(&BasicFunction, run_loop.QuitWhenIdleClosure(),
@@ -266,7 +264,7 @@
TEST_F(BrowserThreadTest, ReleaseViaTaskRunner) {
scoped_refptr<base::SingleThreadTaskRunner> task_runner =
- base::CreateSingleThreadTaskRunner({BrowserThread::UI});
+ GetUIThreadTaskRunner({});
base::RunLoop run_loop;
ExpectRelease(run_loop.QuitWhenIdleClosure());
task_runner->ReleaseSoon(FROM_HERE, base::WrapRefCounted(this));
@@ -277,9 +275,8 @@
// Most of the heavy testing for PostTaskAndReply() is done inside the
// task runner test. This just makes sure we get piped through at all.
base::RunLoop run_loop;
- ASSERT_TRUE(base::PostTaskAndReply(FROM_HERE, {BrowserThread::IO},
- base::DoNothing(),
- run_loop.QuitWhenIdleClosure()));
+ ASSERT_TRUE(GetIOThreadTaskRunner({})->PostTaskAndReply(
+ FROM_HERE, base::DoNothing(), run_loop.QuitWhenIdleClosure()));
run_loop.Run();
}
diff --git a/content/browser/scheduler/browser_task_executor.cc b/content/browser/scheduler/browser_task_executor.cc
index 02a59ec..eb9f505 100644
--- a/content/browser/scheduler/browser_task_executor.cc
+++ b/content/browser/scheduler/browser_task_executor.cc
@@ -12,6 +12,7 @@
#include "base/no_destructor.h"
#include "base/run_loop.h"
#include "base/task/post_task.h"
+#include "base/task/task_traits_extension.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/trace_event/trace_event.h"
#include "build/build_config.h"
@@ -24,47 +25,27 @@
#include "base/android/task_scheduler/post_task_android.h"
#endif
+using QueueType = content::BrowserTaskQueues::QueueType;
+
namespace content {
namespace {
-using QueueType = ::content::BrowserTaskQueues::QueueType;
+// Returns the BrowserThread::ID stored in |traits| which must be coming from a
+// call through BaseBrowserTaskExecutor and hence have the
+// BrowserTaskTraitsExtension.
+BrowserThread::ID ExtractBrowserThreadId(const base::TaskTraits& traits) {
+ DCHECK_EQ(BrowserTaskTraitsExtension::kExtensionId, traits.extension_id());
+ const BrowserTaskTraitsExtension extension =
+ traits.GetExtension<BrowserTaskTraitsExtension>();
+
+ const BrowserThread::ID thread_id = extension.browser_thread();
+ DCHECK_GE(thread_id, 0);
+ return thread_id;
+}
// |g_browser_task_executor| is intentionally leaked on shutdown.
BrowserTaskExecutor* g_browser_task_executor = nullptr;
-QueueType GetQueueType(const base::TaskTraits& traits,
- BrowserTaskType task_type) {
- switch (task_type) {
- case BrowserTaskType::kBootstrap:
- // Note we currently ignore the priority for bootstrap tasks.
- return QueueType::kBootstrap;
-
- case BrowserTaskType::kNavigation:
- case BrowserTaskType::kPreconnect:
- // Note we currently ignore the priority for navigation and preconnection
- // tasks.
- return QueueType::kNavigationAndPreconnection;
-
- case BrowserTaskType::kDefault:
- // Defer to traits.priority() below.
- break;
-
- case BrowserTaskType::kBrowserTaskType_Last:
- NOTREACHED();
- }
-
- switch (traits.priority()) {
- case base::TaskPriority::BEST_EFFORT:
- return QueueType::kBestEffort;
-
- case base::TaskPriority::USER_VISIBLE:
- return QueueType::kUserVisible;
-
- case base::TaskPriority::USER_BLOCKING:
- return QueueType::kUserBlocking;
- }
-}
-
} // namespace
BaseBrowserTaskExecutor::BaseBrowserTaskExecutor() = default;
@@ -77,30 +58,30 @@
base::TimeDelta delay) {
if (traits.extension_id() != BrowserTaskTraitsExtension::kExtensionId ||
traits.GetExtension<BrowserTaskTraitsExtension>().nestable()) {
- return GetTaskRunner(traits)->PostDelayedTask(from_here, std::move(task),
- delay);
+ return GetTaskRunner(ExtractBrowserThreadId(traits), traits)
+ ->PostDelayedTask(from_here, std::move(task), delay);
} else {
- return GetTaskRunner(traits)->PostNonNestableDelayedTask(
- from_here, std::move(task), delay);
+ return GetTaskRunner(ExtractBrowserThreadId(traits), traits)
+ ->PostNonNestableDelayedTask(from_here, std::move(task), delay);
}
}
scoped_refptr<base::TaskRunner> BaseBrowserTaskExecutor::CreateTaskRunner(
const base::TaskTraits& traits) {
- return GetTaskRunner(traits);
+ return GetTaskRunner(ExtractBrowserThreadId(traits), traits);
}
scoped_refptr<base::SequencedTaskRunner>
BaseBrowserTaskExecutor::CreateSequencedTaskRunner(
const base::TaskTraits& traits) {
- return GetTaskRunner(traits);
+ return GetTaskRunner(ExtractBrowserThreadId(traits), traits);
}
scoped_refptr<base::SingleThreadTaskRunner>
BaseBrowserTaskExecutor::CreateSingleThreadTaskRunner(
const base::TaskTraits& traits,
base::SingleThreadTaskRunnerThreadMode thread_mode) {
- return GetTaskRunner(traits);
+ return GetTaskRunner(ExtractBrowserThreadId(traits), traits);
}
#if defined(OS_WIN)
@@ -108,42 +89,71 @@
BaseBrowserTaskExecutor::CreateCOMSTATaskRunner(
const base::TaskTraits& traits,
base::SingleThreadTaskRunnerThreadMode thread_mode) {
- return GetTaskRunner(traits);
+ return GetTaskRunner(ExtractBrowserThreadId(traits), traits);
}
#endif // defined(OS_WIN)
scoped_refptr<base::SingleThreadTaskRunner>
-BaseBrowserTaskExecutor::GetTaskRunner(const base::TaskTraits& traits) const {
- auto id_and_queue = GetThreadIdAndQueueType(traits);
+BaseBrowserTaskExecutor::GetTaskRunner(BrowserThread::ID identifier,
+ const base::TaskTraits& traits) const {
+ DCHECK(traits.extension_id() ==
+ base::TaskTraitsExtensionStorage::kInvalidExtensionId ||
+ ExtractBrowserThreadId(traits) == identifier);
- switch (id_and_queue.thread_id) {
+ const QueueType queue_type = GetQueueType(traits);
+
+ switch (identifier) {
case BrowserThread::UI: {
- return browser_ui_thread_handle_->GetBrowserTaskRunner(
- id_and_queue.queue_type);
+ return browser_ui_thread_handle_->GetBrowserTaskRunner(queue_type);
}
case BrowserThread::IO:
- return browser_io_thread_handle_->GetBrowserTaskRunner(
- id_and_queue.queue_type);
+ return browser_io_thread_handle_->GetBrowserTaskRunner(queue_type);
case BrowserThread::ID_COUNT:
NOTREACHED();
}
return nullptr;
}
-BaseBrowserTaskExecutor::ThreadIdAndQueueType
-BaseBrowserTaskExecutor::GetThreadIdAndQueueType(
- const base::TaskTraits& traits) const {
- DCHECK_EQ(BrowserTaskTraitsExtension::kExtensionId, traits.extension_id());
- const BrowserTaskTraitsExtension extension =
- traits.GetExtension<BrowserTaskTraitsExtension>();
+// static
+QueueType BaseBrowserTaskExecutor::GetQueueType(
+ const base::TaskTraits& traits) {
+ if (traits.extension_id() == BrowserTaskTraitsExtension::kExtensionId) {
+ const BrowserTaskTraitsExtension extension =
+ traits.GetExtension<BrowserTaskTraitsExtension>();
- const BrowserThread::ID thread_id = extension.browser_thread();
- DCHECK_GE(thread_id, 0);
+ const BrowserTaskType task_type = extension.task_type();
+ DCHECK_LT(task_type, BrowserTaskType::kBrowserTaskType_Last);
- const BrowserTaskType task_type = extension.task_type();
- DCHECK_LT(task_type, BrowserTaskType::kBrowserTaskType_Last);
+ switch (task_type) {
+ case BrowserTaskType::kBootstrap:
+ // Note we currently ignore the priority for bootstrap tasks.
+ return QueueType::kBootstrap;
- return {thread_id, GetQueueType(traits, task_type)};
+ case BrowserTaskType::kNavigation:
+ case BrowserTaskType::kPreconnect:
+ // Note we currently ignore the priority for navigation and
+ // preconnection tasks.
+ return QueueType::kNavigationAndPreconnection;
+
+ case BrowserTaskType::kDefault:
+ // Defer to traits.priority() below.
+ break;
+
+ case BrowserTaskType::kBrowserTaskType_Last:
+ NOTREACHED();
+ }
+ }
+
+ switch (traits.priority()) {
+ case base::TaskPriority::BEST_EFFORT:
+ return QueueType::kBestEffort;
+
+ case base::TaskPriority::USER_VISIBLE:
+ return QueueType::kUserVisible;
+
+ case base::TaskPriority::USER_BLOCKING:
+ return QueueType::kUserBlocking;
+ }
}
BrowserTaskExecutor::BrowserTaskExecutor(
@@ -283,6 +293,20 @@
}
// static
+scoped_refptr<base::SingleThreadTaskRunner>
+BrowserTaskExecutor::GetUIThreadTaskRunner(const BrowserTaskTraits& traits) {
+ DCHECK(g_browser_task_executor);
+ return g_browser_task_executor->GetTaskRunner(BrowserThread::UI, traits);
+}
+
+// static
+scoped_refptr<base::SingleThreadTaskRunner>
+BrowserTaskExecutor::GetIOThreadTaskRunner(const BrowserTaskTraits& traits) {
+ DCHECK(g_browser_task_executor);
+ return g_browser_task_executor->GetTaskRunner(BrowserThread::IO, traits);
+}
+
+// static
void BrowserTaskExecutor::InitializeIOThread() {
DCHECK(g_browser_task_executor);
g_browser_task_executor->browser_io_thread_handle_
diff --git a/content/browser/scheduler/browser_task_executor.h b/content/browser/scheduler/browser_task_executor.h
index 9973575f..e80275d6 100644
--- a/content/browser/scheduler/browser_task_executor.h
+++ b/content/browser/scheduler/browser_task_executor.h
@@ -58,20 +58,25 @@
base::SingleThreadTaskRunnerThreadMode thread_mode) override;
#endif // defined(OS_WIN)
- struct ThreadIdAndQueueType {
- BrowserThread::ID thread_id;
- BrowserTaskQueues::QueueType queue_type;
- };
-
- ThreadIdAndQueueType GetThreadIdAndQueueType(
+ // Returns the task runner for |traits| under |identifier|. Note: during the
+ // migration away from task traits extension, |traits| may also contain a
+ // browser thread id, if so, it should match |identifier| (|identifier| has to
+ // be provided explicitly because in the new source of tasks it's not part of
+ // |traits|) -- ref. crbug.com/1026641.
+ scoped_refptr<base::SingleThreadTaskRunner> GetTaskRunner(
+ BrowserThread::ID identifier,
const base::TaskTraits& traits) const;
+ // Helper to match a QueueType from TaskTraits.
+ // TODO(1026641): Take BrowserTaskTraits as a parameter when getting off the
+ // need to support base::TaskTraits currently passed to this class in its role
+ // as a base::TaskExecutor.
+ static content::BrowserTaskQueues::QueueType GetQueueType(
+ const base::TaskTraits& traits);
+
protected:
virtual BrowserThread::ID GetCurrentThreadID() const = 0;
- scoped_refptr<base::SingleThreadTaskRunner> GetTaskRunner(
- const base::TaskTraits& traits) const;
-
scoped_refptr<BrowserUIThreadScheduler::Handle> browser_ui_thread_handle_;
scoped_refptr<BrowserIOThreadDelegate::Handle> browser_io_thread_handle_;
};
@@ -126,6 +131,16 @@
// Can be called multiple times.
static void EnableAllQueues();
+ // Helpers to statically call into BaseBrowserTaskExecutor::GetTaskRunner()
+ // from browser_thread_impl.cc. Callers should use browser_thread.h's
+ // GetUIThreadTaskRunner over this.
+ // TODO(1026641): Clean up this indirection after the migration (once
+ // registering a base::BrowserTaskExecutor is no longer necessary).
+ static scoped_refptr<base::SingleThreadTaskRunner> GetUIThreadTaskRunner(
+ const BrowserTaskTraits& traits);
+ static scoped_refptr<base::SingleThreadTaskRunner> GetIOThreadTaskRunner(
+ const BrowserTaskTraits& traits);
+
// As Create but with the user provided objects. Must call
// BindToUIThreadForTesting before tasks can be run on the UI thread.
static void CreateForTesting(
diff --git a/content/browser/scheduler/browser_task_executor_unittest.cc b/content/browser/scheduler/browser_task_executor_unittest.cc
index 97a1340..660ae58a 100644
--- a/content/browser/scheduler/browser_task_executor_unittest.cc
+++ b/content/browser/scheduler/browser_task_executor_unittest.cc
@@ -11,7 +11,6 @@
#include "base/bind.h"
#include "base/bind_helpers.h"
-#include "base/task/post_task.h"
#include "base/task/task_traits.h"
#include "base/task/thread_pool/thread_pool_instance.h"
#include "base/test/bind_test_util.h"
@@ -49,15 +48,15 @@
testing::StrictMock<base::MockCallback<base::RepeatingCallback<void()>>>;
TEST_F(BrowserTaskExecutorTest, RegisterExecutorForBothThreads) {
- base::PostTask(FROM_HERE, {BrowserThread::UI}, base::BindOnce([]() {
- EXPECT_THAT(base::GetTaskExecutorForCurrentThread(),
- NotNull());
- }));
+ GetUIThreadTaskRunner({})->PostTask(
+ FROM_HERE, base::BindOnce([]() {
+ EXPECT_THAT(base::GetTaskExecutorForCurrentThread(), NotNull());
+ }));
- base::PostTask(FROM_HERE, {BrowserThread::IO}, base::BindOnce([]() {
- EXPECT_THAT(base::GetTaskExecutorForCurrentThread(),
- NotNull());
- }));
+ GetIOThreadTaskRunner({})->PostTask(
+ FROM_HERE, base::BindOnce([]() {
+ EXPECT_THAT(base::GetTaskExecutorForCurrentThread(), NotNull());
+ }));
BrowserTaskExecutor::RunAllPendingTasksOnThreadForTesting(BrowserThread::UI);
BrowserTaskExecutor::RunAllPendingTasksOnThreadForTesting(BrowserThread::IO);
@@ -70,7 +69,7 @@
base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, task_2.Get());
}));
- base::PostTask(FROM_HERE, {BrowserThread::UI}, task_1.Get());
+ GetUIThreadTaskRunner({})->PostTask(FROM_HERE, task_1.Get());
BrowserTaskExecutor::RunAllPendingTasksOnThreadForTesting(BrowserThread::UI);
@@ -84,10 +83,10 @@
StrictMockTask task_1;
StrictMockTask task_2;
EXPECT_CALL(task_1, Run).WillOnce(testing::Invoke([&]() {
- base::PostTask(FROM_HERE, {BrowserThread::IO}, task_2.Get());
+ GetIOThreadTaskRunner({})->PostTask(FROM_HERE, task_2.Get());
}));
- base::PostTask(FROM_HERE, {BrowserThread::IO}, task_1.Get());
+ GetIOThreadTaskRunner({})->PostTask(FROM_HERE, task_1.Get());
BrowserTaskExecutor::RunAllPendingTasksOnThreadForTesting(BrowserThread::IO);
@@ -103,15 +102,15 @@
StrictMockTask task_3;
EXPECT_CALL(task_1, Run).WillOnce(Invoke([&]() {
- base::PostTask(FROM_HERE, {BrowserThread::IO}, task_2.Get());
+ GetIOThreadTaskRunner({})->PostTask(FROM_HERE, task_2.Get());
BrowserTaskExecutor::RunAllPendingTasksOnThreadForTesting(
BrowserThread::IO);
}));
EXPECT_CALL(task_2, Run).WillOnce(Invoke([&]() {
- base::PostTask(FROM_HERE, {BrowserThread::IO}, task_3.Get());
+ GetIOThreadTaskRunner({})->PostTask(FROM_HERE, task_3.Get());
}));
- base::PostTask(FROM_HERE, {BrowserThread::IO}, task_1.Get());
+ GetIOThreadTaskRunner({})->PostTask(FROM_HERE, task_1.Get());
BrowserTaskExecutor::RunAllPendingTasksOnThreadForTesting(BrowserThread::IO);
// Cleanup pending tasks, as BrowserTaskEnvironment will run them.
@@ -134,44 +133,33 @@
}
};
- template <BrowserThread::ID ID>
- void CheckExpectations() {
- EXPECT_EQ(GetQueueType({ID, TaskPriority::BEST_EFFORT}),
- QueueType::kBestEffort);
- EXPECT_EQ(GetQueueType({ID, TaskPriority::USER_VISIBLE}),
- QueueType::kUserVisible);
- EXPECT_EQ(GetQueueType({ID, TaskPriority::USER_BLOCKING}),
- QueueType::kUserBlocking);
-
- EXPECT_EQ(GetQueueType({ID, BrowserTaskType::kBootstrap}),
- QueueType::kBootstrap);
- EXPECT_EQ(GetQueueType({ID, BrowserTaskType::kDefault}),
- QueueType::kUserBlocking);
- EXPECT_EQ(GetQueueType({ID, BrowserTaskType::kNavigation}),
- QueueType::kNavigationAndPreconnection);
- EXPECT_EQ(GetQueueType({ID, BrowserTaskType::kPreconnect}),
- QueueType::kNavigationAndPreconnection);
-
- EXPECT_EQ(GetQueueType({ID}), QueueType::kUserBlocking);
- }
-
private:
- QueueType GetQueueType(const base::TaskTraits& traits) {
- return test_executor_.GetThreadIdAndQueueType(traits).queue_type;
- }
-
TestExecutor test_executor_;
};
TEST_F(BrowserTaskTraitsMappingTest, BrowserTaskTraitsMapToProperPriorities) {
- CheckExpectations<BrowserThread::UI>();
- CheckExpectations<BrowserThread::IO>();
+ EXPECT_EQ(BrowserTaskExecutor::GetQueueType({TaskPriority::BEST_EFFORT}),
+ QueueType::kBestEffort);
+ EXPECT_EQ(BrowserTaskExecutor::GetQueueType({TaskPriority::USER_VISIBLE}),
+ QueueType::kUserVisible);
+ EXPECT_EQ(BrowserTaskExecutor::GetQueueType({TaskPriority::USER_BLOCKING}),
+ QueueType::kUserBlocking);
+
+ EXPECT_EQ(BrowserTaskExecutor::GetQueueType({BrowserTaskType::kBootstrap}),
+ QueueType::kBootstrap);
+ EXPECT_EQ(BrowserTaskExecutor::GetQueueType({BrowserTaskType::kDefault}),
+ QueueType::kUserBlocking);
+ EXPECT_EQ(BrowserTaskExecutor::GetQueueType({BrowserTaskType::kNavigation}),
+ QueueType::kNavigationAndPreconnection);
+ EXPECT_EQ(BrowserTaskExecutor::GetQueueType({BrowserTaskType::kPreconnect}),
+ QueueType::kNavigationAndPreconnection);
+
+ EXPECT_EQ(BrowserTaskExecutor::GetQueueType({}), QueueType::kUserBlocking);
}
TEST_F(BrowserTaskTraitsMappingTest,
UIThreadTaskRunnerHasSamePriorityAsUIBlocking) {
- auto ui_blocking = base::CreateSingleThreadTaskRunner(
- {BrowserThread::UI, TaskPriority::USER_BLOCKING});
+ auto ui_blocking = GetUIThreadTaskRunner({TaskPriority::USER_BLOCKING});
auto thread_task_runner = base::ThreadTaskRunnerHandle::Get();
std::vector<int> order;
@@ -213,8 +201,6 @@
};
public:
- using QueueType = BrowserTaskQueues::QueueType;
-
~BrowserTaskExecutorWithCustomSchedulerTest() override {
BrowserTaskExecutor::ResetForTesting();
}
@@ -229,14 +215,17 @@
StrictMockTask user_visible;
StrictMockTask user_blocking;
- base::PostTask(FROM_HERE,
- {BrowserThread::UI, base::TaskPriority::BEST_EFFORT},
+ GetUIThreadTaskRunner({base::TaskPriority::BEST_EFFORT})
+ ->PostTask(FROM_HERE,
+
best_effort.Get());
- base::PostTask(FROM_HERE,
- {BrowserThread::UI, base::TaskPriority::USER_VISIBLE},
+ GetUIThreadTaskRunner({base::TaskPriority::USER_VISIBLE})
+ ->PostTask(FROM_HERE,
+
user_visible.Get());
- base::PostTask(FROM_HERE,
- {BrowserThread::UI, base::TaskPriority::USER_BLOCKING},
+ GetUIThreadTaskRunner({base::TaskPriority::USER_BLOCKING})
+ ->PostTask(FROM_HERE,
+
user_blocking.Get());
EXPECT_CALL(user_visible, Run);
@@ -247,19 +236,20 @@
TEST_F(BrowserTaskExecutorWithCustomSchedulerTest,
BestEffortTasksRunAfterStartup) {
- auto ui_best_effort_runner = base::CreateSingleThreadTaskRunner(
- {BrowserThread::UI, base::TaskPriority::BEST_EFFORT});
+ auto ui_best_effort_runner =
+ GetUIThreadTaskRunner({base::TaskPriority::BEST_EFFORT});
StrictMockTask best_effort;
ui_best_effort_runner->PostTask(FROM_HERE, best_effort.Get());
ui_best_effort_runner->PostDelayedTask(
FROM_HERE, best_effort.Get(), base::TimeDelta::FromMilliseconds(100));
- base::PostDelayedTask(
- FROM_HERE, {BrowserThread::UI, base::TaskPriority::BEST_EFFORT},
- best_effort.Get(), base::TimeDelta::FromMilliseconds(100));
- base::PostTask(FROM_HERE,
- {BrowserThread::UI, base::TaskPriority::BEST_EFFORT},
+ GetUIThreadTaskRunner({base::TaskPriority::BEST_EFFORT})
+ ->PostDelayedTask(FROM_HERE, best_effort.Get(),
+ base::TimeDelta::FromMilliseconds(100));
+ GetUIThreadTaskRunner({base::TaskPriority::BEST_EFFORT})
+ ->PostTask(FROM_HERE,
+
best_effort.Get());
task_environment_.RunUntilIdle();
diff --git a/content/public/browser/browser_task_traits.h b/content/public/browser/browser_task_traits.h
index 060ba21..cb0637e 100644
--- a/content/public/browser/browser_task_traits.h
+++ b/content/public/browser/browser_task_traits.h
@@ -80,7 +80,8 @@
base::trait_helpers::AreValidTraits<ValidTrait, ArgTypes...>::value>>
constexpr BrowserTaskTraitsExtension(ArgTypes... args)
: browser_thread_(
- base::trait_helpers::GetEnum<BrowserThread::ID>(args...)),
+ base::trait_helpers::GetEnum<BrowserThread::ID,
+ BrowserThread::ID_COUNT>(args...)),
task_type_(
base::trait_helpers::GetEnum<BrowserTaskType,
BrowserTaskType::kDefault>(args...)),
@@ -105,7 +106,13 @@
static_cast<bool>(extension.data[2]));
}
- constexpr BrowserThread::ID browser_thread() const { return browser_thread_; }
+ constexpr BrowserThread::ID browser_thread() const {
+ // TODO(1026641): Migrate to BrowserTaskTraits under which BrowserThread is
+ // not a trait. Until then, only code that knows traits have explicitly set
+ // the BrowserThread trait should check this field.
+ DCHECK_NE(browser_thread_, BrowserThread::ID_COUNT);
+ return browser_thread_;
+ }
constexpr BrowserTaskType task_type() const { return task_type_; }
@@ -135,6 +142,40 @@
.Serialize();
}
+class CONTENT_EXPORT BrowserTaskTraits : public base::TaskTraits {
+ public:
+ struct ValidTrait : public base::TaskTraits::ValidTrait {
+ ValidTrait(BrowserTaskType);
+ ValidTrait(NonNestable);
+
+ // TODO(1026641): Reconsider whether BrowserTaskTraits should really be
+ // supporting base::TaskPriority.
+ ValidTrait(base::TaskPriority);
+ };
+
+ // TODO(1026641): Get rid of BrowserTaskTraitsExtension and store its members
+ // (|task_type_| & |nestable_|) directly in BrowserTaskTraits.
+ template <
+ class... ArgTypes,
+ class CheckArgumentsAreValid = std::enable_if_t<
+ base::trait_helpers::AreValidTraits<ValidTrait, ArgTypes...>::value>>
+ constexpr BrowserTaskTraits(ArgTypes... args) : base::TaskTraits(args...) {}
+
+ BrowserTaskType task_type() {
+ return GetExtension<BrowserTaskTraitsExtension>().task_type();
+ }
+
+ // Returns true if tasks with these traits may run in a nested RunLoop.
+ bool nestable() const {
+ return GetExtension<BrowserTaskTraitsExtension>().nestable();
+ }
+};
+
+static_assert(sizeof(BrowserTaskTraits) == sizeof(base::TaskTraits),
+ "During the migration away from BrowserTasktraitsExtension, "
+ "BrowserTaskTraits must only use base::TaskTraits for storage "
+ "to prevent slicing.");
+
} // namespace content
#endif // CONTENT_PUBLIC_BROWSER_BROWSER_TASK_TRAITS_H_
diff --git a/content/public/browser/browser_task_traits_unittest.nc b/content/public/browser/browser_task_traits_unittest.nc
index f38fb703..e92fba5 100644
--- a/content/public/browser/browser_task_traits_unittest.nc
+++ b/content/public/browser/browser_task_traits_unittest.nc
@@ -14,11 +14,9 @@
#if defined(NCTEST_BROWSER_TASK_TRAITS_MULTIPLE_THREADS) // [r"The traits bag contains multiple traits of the same type."]
constexpr base::TaskTraits traits = {BrowserThread::UI,
BrowserThread::IO};
-#elif defined(NCTEST_BROWSER_TASK_TRAITS_MULTIPLE_TASK_TYPES) // [r"The traits bag is missing a required trait."]
+#elif defined(NCTEST_BROWSER_TASK_TRAITS_MULTIPLE_TASK_TYPES) // [r"The traits bag contains multiple traits of the same type."]
constexpr base::TaskTraits traits = {BrowserTaskType::kNavigation,
BrowserTaskType::kBootstrap};
-#elif defined(NCTEST_BROWSER_TASK_TRAITS_NO_THREAD_ID) // [r"The traits bag is missing a required trait."]
-constexpr base::TaskTraits traits = {BrowserTaskType::kNavigation};
#endif
diff --git a/content/public/browser/browser_thread.h b/content/public/browser/browser_thread.h
index af24784..3afe558 100644
--- a/content/public/browser/browser_thread.h
+++ b/content/public/browser/browser_thread.h
@@ -21,6 +21,11 @@
namespace content {
+// TODO(1026641): Include browser_task_traits.h directly when the migration to
+// Get(UI|IO)ThreadTaskrunner() is complete and the cyclic dependency of
+// browser_task_traits.h on BrowserThread::ID is broken.
+class BrowserTaskTraits;
+
class BrowserThreadImpl;
// Use DCHECK_CURRENTLY_ON(BrowserThread::ID) to assert that a function can only
@@ -30,34 +35,45 @@
<< ::content::BrowserThread::GetDCheckCurrentlyOnErrorMessage( \
thread_identifier))
+// The main entry point to post tasks to the UI thread. Tasks posted with the
+// same |traits| will run in posting order (i.e. according to the
+// SequencedTaskRunner contract). Tasks posted with different |traits| can be
+// re-ordered. You may keep a reference to this task runner, it's always
+// thread-safe to post to it though it may start returning false at some point
+// during shutdown when it definitely is no longer accepting tasks.
+//
+// In unit tests, there must be a content::BrowserTaskEnvironment in scope for
+// this API to be available.
+//
+// TODO(1026641): Make default traits |{}| the default param when it's possible
+// to include browser_task_traits.h in this file (see note above on the
+// BrowserTaskTraits fwd-decl).
+CONTENT_EXPORT scoped_refptr<base::SingleThreadTaskRunner>
+GetUIThreadTaskRunner(const BrowserTaskTraits& traits);
+
+// The BrowserThread::IO counterpart to GetUIThreadTaskRunner().
+CONTENT_EXPORT scoped_refptr<base::SingleThreadTaskRunner>
+GetIOThreadTaskRunner(const BrowserTaskTraits& traits);
+
///////////////////////////////////////////////////////////////////////////////
// BrowserThread
//
-// Utility functions for threads that are known by a browser-wide name. For
-// example, there is one IO thread for the entire browser process, and various
-// pieces of code find it useful to retrieve a pointer to the IO thread's
-// message loop.
-//
-// See browser_task_traits.h for posting Tasks to a BrowserThread.
-//
-// This class automatically handles the lifetime of different threads. You
-// should never need to cache pointers to MessageLoops, since they're not thread
-// safe.
+// Utility functions for threads that are known by a browser-wide name.
class CONTENT_EXPORT BrowserThread {
public:
// An enumeration of the well-known threads.
- // NOTE: threads must be listed in the order of their life-time, with each
- // thread outliving every other thread below it.
enum ID {
- // The main thread in the browser.
+ // The main thread in the browser. It stops running tasks during shutdown
+ // and is never joined.
UI,
- // This is the thread that processes non-blocking IO, i.e. IPC and network.
- // Blocking I/O should happen in ThreadPool.
+ // This is the thread that processes non-blocking I/O, i.e. IPC and network.
+ // Blocking I/O should happen in base::ThreadPool. It is joined on shutdown
+ // (and thus any task posted to it may block shutdown).
IO,
// NOTE: do not add new threads here. Instead you should just use
- // base::Create*TaskRunner to run tasks on the ThreadPool.
+ // base::ThreadPool::Create*TaskRunner to run tasks on the base::ThreadPool.
// This identifier does not represent a thread. Instead it counts the
// number of well-known threads. Insert new well-known threads before this
@@ -65,15 +81,12 @@
ID_COUNT
};
- // NOTE: Task posting APIs have moved to post_task.h. See
- // browser_task_traits.h.
-
// Delete/ReleaseSoon() helpers allow future deletion of an owned object on
// its associated thread. If you already have a task runner bound to a
// BrowserThread you should use its SequencedTaskRunner::DeleteSoon() member
- // method. If you don't, the helpers below avoid having to do
- // base::CreateSingleThreadTaskRunner({BrowserThread::ID})->DeleteSoon(...)
- // which is equivalent.
+ // method.
+ // TODO(1026641): Get rid of the last few callers to these in favor of an
+ // explicit call to content::GetUIThreadTaskRunner({})->DeleteSoon(...).
template <class T>
static bool DeleteSoon(ID identifier,
@@ -98,7 +111,7 @@
}
// Posts a |task| to run at BEST_EFFORT priority using an arbitrary
- // |task_runner| for which we do not control the priority
+ // |task_runner| for which we do not control the priority.
//
// This is useful when a task needs to run on |task_runner| (for thread-safety
// reasons) but should be delayed until after critical phases (e.g. startup).
@@ -196,8 +209,8 @@
private:
friend class BrowserThreadImpl;
+ BrowserThread() = default;
- BrowserThread() {}
DISALLOW_COPY_AND_ASSIGN(BrowserThread);
};
diff --git a/docs/threading_and_tasks.md b/docs/threading_and_tasks.md
index 030c8908..998e5251 100644
--- a/docs/threading_and_tasks.md
+++ b/docs/threading_and_tasks.md
@@ -109,7 +109,8 @@
* in the browser process (BrowserThread::UI): updates the UI
* in renderer processes (Blink main thread): runs most of Blink
* an IO thread
- * in the browser process (BrowserThread::IO): handles IPCs and network requests
+ * in the browser process (BrowserThread::IO): handles IPCs and network
+ requests
* in renderer processes: handles IPCs
* a few more special-purpose threads
* and a pool of general-purpose threads
@@ -222,8 +223,8 @@
```
Unless a test needs to control precisely how tasks are executed, it is preferred
-to call `base::ThreadPool::PostTask*()` directly (ref. [Testing](#Testing) for less invasive
-ways of controlling tasks in tests).
+to call `base::ThreadPool::PostTask*()` directly (ref. [Testing](#Testing) for
+less invasive ways of controlling tasks in tests).
## Posting a Sequenced Task
@@ -336,10 +337,17 @@
### Posting to the Main Thread or to the IO Thread in the Browser Process
To post tasks to the main thread or to the IO thread, use
-`base::PostTask()` or get the appropriate SingleThreadTaskRunner using
-`base::CreateSingleThreadTaskRunner`, supplying a `BrowserThread::ID`
-as trait. For this, you'll also need to include
-[`content/public/browser/browser_task_traits.h`](https://cs.chromium.org/chromium/src/content/public/browser/browser_task_traits.h).
+`content::GetUIThreadTaskRunner({})` or `content::GetUIThreadTaskRunner({})`
+from
+[`content/public/browser/browser_thread.h`](https://cs.chromium.org/chromium/src/content/public/browser/browser_thread.h)
+
+You may provide additional BrowserTaskTraits as a parameter to those methods
+though this is generally still uncommon in BrowserThreads and should be reserved
+for advanced use cases.
+
+There's an ongoing migration ([task APIs v3]) away from the previous
+base-API-with-traits which you may still find throughout the codebase (it's
+equivalent):
```cpp
base::PostTask(FROM_HERE, {content::BrowserThread::UI}, ...);
@@ -348,8 +356,10 @@
->PostTask(FROM_HERE, ...);
```
-Note: This API will soon be updated to follow the API-as-a-destination design
-for [task APIs v3], stay tuned!
+Note: For the duration of the migration, you'll unfortunately need to continue
+manually including
+[`content/public/browser/browser_task_traits.h`](https://cs.chromium.org/chromium/src/content/public/browser/browser_task_traits.h).
+to use the browser_thread.h API.
The main thread and the IO thread are already super busy. Therefore, prefer
posting to a general purpose thread when possible (ref.
@@ -362,13 +372,14 @@
send/receive an IPC or send/receive data on the network.
### Posting to the Main Thread in a Renderer Process
-TODO
+TODO(blink-dev)
### Posting to a Custom SingleThreadTaskRunner
If multiple tasks need to run on the same thread and that thread doesn’t have to
be the main thread or the IO thread, post them to a
-`base::SingleThreadTaskRunner` created by `base::Threadpool::CreateSingleThreadTaskRunner`.
+`base::SingleThreadTaskRunner` created by
+`base::Threadpool::CreateSingleThreadTaskRunner`.
```cpp
scoped_refptr<SingleThreadTaskRunner> single_thread_task_runner =
@@ -387,13 +398,14 @@
*** note
**IMPORTANT:** To post a task that needs mutual exclusion with the current
-sequence of tasks but doesn’t absolutely need to run on the current physical thread,
-use `base::SequencedTaskRunnerHandle::Get()` instead of
+sequence of tasks but doesn’t absolutely need to run on the current physical
+thread, use `base::SequencedTaskRunnerHandle::Get()` instead of
`base::ThreadTaskRunnerHandle::Get()` (ref. [Posting to the Current
-Sequence](#Posting-to-the-Current-Virtual_Thread)). That will better document the
-requirements of the posted task and will avoid unnecessarily making your API
-physical thread-affine. In a single-thread task, `base::SequencedTaskRunnerHandle::Get()`
-is equivalent to `base::ThreadTaskRunnerHandle::Get()`.
+Sequence](#Posting-to-the-Current-Virtual_Thread)). That will better document
+the requirements of the posted task and will avoid unnecessarily making your API
+physical thread-affine. In a single-thread task,
+`base::SequencedTaskRunnerHandle::Get()` is equivalent to
+`base::ThreadTaskRunnerHandle::Get()`.
***
If you must post a task to the current physical thread nonetheless, use
@@ -481,11 +493,6 @@
base::ThreadPool::PostTask(
FROM_HERE, {base::TaskShutdownBehavior::BLOCK_SHUTDOWN},
base::BindOnce(...));
-
-// This task will run on the Browser UI thread.
-base::ThreadPool::PostTask(
- FROM_HERE, {content::BrowserThread::UI},
- base::BindOnce(...));
```
## Keeping the Browser Responsive
