Skip to content

Commit

Permalink
headless: Always specify thread affinity when posting tasks
Browse files Browse the repository at this point in the history
*** Note: There is no behavior change from this patch. ***

The PostTask APIs will shortly be changed to require all tasks to explicitly
specify their thread affinity, i.e., whether the task should run on the thread
pool or a specific named thread such as a BrowserThread. This patch updates all
call sites with thread affinity annotation. We also remove the "WithTraits"
suffix to make the call sites more readable.

Before:

    // Thread pool task.
    base::PostTaskWithTraits(FROM_HERE, {...}, ...);

    // UI thread task.
    base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI, ...}, ...);

After:

    // Thread pool task.
    base::PostTask(FROM_HERE, {base::ThreadPool(), ...}, ...);

    // UI thread task.
    base::PostTask(FROM_HERE, {BrowserThread::UI, ...}, ...);

This patch was semi-automatically prepared with these steps:

    1. Patch in https://chromium-review.googlesource.com/c/chromium/src/+/1635827
       to make thread affinity a build-time requirement.
    2. Run an initial pass with a clang rewriter:
       https://chromium-review.googlesource.com/c/chromium/src/+/1635623
    3. ninja -C out/Debug | grep 'requested here' | cut -d: -f1-3 | sort | \
           uniq > errors.txt
    4. while read line; do
         f=$(echo $line | cut -d: -f 1)
         r=$(echo $line | cut -d: -f 2)
         c=$(echo $line | cut -d: -f 3)
         sed -i "${r}s/./&base::ThreadPool(),/$c" $f
       done < errors.txt
    5. GOTO 3 until build succeeds.
    6. Remove the "WithTraits" suffix from task API call sites:

       $ tools/git/mffr.py -i <(cat <<EOF
       [
         ["PostTaskWithTraits",                            "PostTask"],
         ["PostDelayedTaskWithTraits",                     "PostDelayedTask"],
         ["PostTaskWithTraitsAndReply",                    "PostTaskAndReply"],
         ["CreateTaskRunnerWithTraits",                    "CreateTaskRunner"],
         ["CreateSequencedTaskRunnerWithTraits",           "CreateSequencedTaskRunner"],
         ["CreateUpdateableSequencedTaskRunnerWithTraits", "CreateUpdateableSequencedTaskRunner"],
         ["CreateSingleThreadTaskRunnerWithTraits",        "CreateSingleThreadTaskRunner"],
         ["CreateCOMSTATaskRunnerWithTraits",              "CreateCOMSTATaskRunner"]
       ]
       EOF
       )

This CL was uploaded by git cl split.

TBR=caseq@chromium.org

Bug: 968047
Change-Id: Icec07c2acaf89d4a2fc7d833f8144e31bbef6ecf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1729212
Commit-Queue: Sami Kyöstilä <skyostil@chromium.org>
Auto-Submit: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682774}
  • Loading branch information
skyostil authored and Commit Bot committed Jul 31, 2019
1 parent 6e7a4b6 commit 1d58980
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 13 deletions.
4 changes: 2 additions & 2 deletions headless/app/headless_shell.cc
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,8 @@ HeadlessShell::~HeadlessShell() = default;
void HeadlessShell::OnStart(HeadlessBrowser* browser) {
browser_ = browser;
devtools_client_ = HeadlessDevToolsClient::Create();
file_task_runner_ = base::CreateSequencedTaskRunnerWithTraits(
{base::MayBlock(), base::TaskPriority::BEST_EFFORT});
file_task_runner_ = base::CreateSequencedTaskRunner(
{base::ThreadPool(), base::MayBlock(), base::TaskPriority::BEST_EFFORT});

HeadlessBrowserContext::Builder context_builder =
browser_->CreateBrowserContextBuilder();
Expand Down
3 changes: 1 addition & 2 deletions headless/lib/browser/headless_browser_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ HeadlessBrowserImpl::CreateBrowserContextBuilder() {

scoped_refptr<base::SingleThreadTaskRunner>
HeadlessBrowserImpl::BrowserMainThread() const {
return base::CreateSingleThreadTaskRunnerWithTraits(
{content::BrowserThread::UI});
return base::CreateSingleThreadTaskRunner({content::BrowserThread::UI});
}

void HeadlessBrowserImpl::Shutdown() {
Expand Down
4 changes: 2 additions & 2 deletions headless/lib/browser/headless_devtools_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ void HeadlessDevToolsClientImpl::AttachToExternalHost(
}

void HeadlessDevToolsClientImpl::InitBrowserMainThread() {
browser_main_thread_ = base::CreateSingleThreadTaskRunnerWithTraits(
{content::BrowserThread::UI});
browser_main_thread_ =
base::CreateSingleThreadTaskRunner({content::BrowserThread::UI});
}

void HeadlessDevToolsClientImpl::ChannelClosed() {
Expand Down
5 changes: 2 additions & 3 deletions headless/lib/browser/protocol/browser_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,8 @@ Response BrowserHandler::GetWindowBounds(
}

Response BrowserHandler::Close() {
base::PostTaskWithTraits(
FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(&HeadlessBrowserImpl::Shutdown, browser()));
base::PostTask(FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(&HeadlessBrowserImpl::Shutdown, browser()));
return Response::OK();
}

Expand Down
8 changes: 4 additions & 4 deletions headless/test/test_network_interceptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ class TestNetworkInterceptor::Impl {
}

Response* FindResponse(const std::string& method, const std::string& url) {
base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI},
base::BindOnce(&TestNetworkInterceptor::LogRequest,
interceptor_, method, url));
base::PostTask(FROM_HERE, {BrowserThread::UI},
base::BindOnce(&TestNetworkInterceptor::LogRequest,
interceptor_, method, url));
auto it = response_map_.find(StripFragment(url));
return it == response_map_.end() ? nullptr : it->second.get();
}
Expand Down Expand Up @@ -177,7 +177,7 @@ TestNetworkInterceptor::~TestNetworkInterceptor() {

void TestNetworkInterceptor::InsertResponse(std::string url,
Response response) {
base::PostTaskWithTraits(
base::PostTask(
FROM_HERE, {BrowserThread::IO},
base::BindOnce(&Impl::InsertResponse, base::Unretained(impl_.get()),
std::move(url), std::move(response)));
Expand Down

0 comments on commit 1d58980

Please sign in to comment.