Skip to content

Commit

Permalink
ios: 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.

R=eugenebut@chromium.org

Bug: 968047
Change-Id: Ibebd81e88c85b0436356b25ec53495b86d6eac1a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1729075
Auto-Submit: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Commit-Queue: Rohit Rao <rohitrao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#685524}
  • Loading branch information
skyostil authored and Commit Bot committed Aug 9, 2019
1 parent c164bf6 commit dce49dd
Show file tree
Hide file tree
Showing 49 changed files with 210 additions and 212 deletions.
4 changes: 2 additions & 2 deletions ios/chrome/app/application_delegate/app_state.mm
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
namespace {
// Helper method to post |closure| on the UI thread.
void PostTaskOnUIThread(base::OnceClosure closure) {
base::PostTaskWithTraits(FROM_HERE, {web::WebThread::UI}, std::move(closure));
base::PostTask(FROM_HERE, {web::WebThread::UI}, std::move(closure));
}
NSString* const kStartupAttemptReset = @"StartupAttempReset";
} // namespace
Expand Down Expand Up @@ -242,7 +242,7 @@ - (void)applicationDidEnterBackground:(UIApplication*)application
DCHECK_CURRENTLY_ON(web::WebThread::UI);
_savingCookies = NO;
}));
base::PostTaskWithTraits(
base::PostTask(
FROM_HERE, {web::WebThread::IO}, base::BindOnce(^{
net::CookieStoreIOS* store = static_cast<net::CookieStoreIOS*>(
getter->GetURLRequestContext()->cookie_store());
Expand Down
7 changes: 4 additions & 3 deletions ios/chrome/app/application_delegate/metrics_mediator.mm
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ - (void)setAppGroupMetricsEnabled:(BOOL)enabled {
callback = ^(NSData* log_content) {
std::string log(static_cast<const char*>([log_content bytes]),
static_cast<size_t>([log_content length]));
base::PostTaskWithTraits(
base::PostTask(
FROM_HERE, {web::WebThread::UI}, base::BindOnce(^{
GetApplicationContext()->GetMetricsService()->PushExternalLog(log);
}));
Expand All @@ -270,8 +270,9 @@ - (void)setAppGroupMetricsEnabled:(BOOL)enabled {
}

app_group::main_app::RecordWidgetUsage();
base::PostTaskWithTraits(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::PostTask(
FROM_HERE,
{base::ThreadPool(), base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(&app_group::main_app::ProcessPendingLogs, callback));
}

Expand Down
8 changes: 4 additions & 4 deletions ios/chrome/app/main_controller.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1944,10 +1944,10 @@ - (void)lastIncognitoTabClosed {

// OffTheRecordProfileIOData cannot be deleted before all the requests are
// deleted. Queue browser state recreation on IO thread.
base::PostTaskWithTraitsAndReply(
FROM_HERE, {web::WebThread::IO}, base::DoNothing(), base::BindRepeating(^{
[self destroyAndRebuildIncognitoBrowserState];
}));
base::PostTaskAndReply(FROM_HERE, {web::WebThread::IO}, base::DoNothing(),
base::BindRepeating(^{
[self destroyAndRebuildIncognitoBrowserState];
}));

// a) The first condition can happen when the last incognito tab is closed
// from the tab switcher.
Expand Down
12 changes: 7 additions & 5 deletions ios/chrome/app/memory_monitor.mm
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,17 @@ void UpdateMemoryValues() {
// |kMemoryMonitorDelayInSeconds|.
void AsynchronousFreeMemoryMonitor() {
UpdateMemoryValues();
base::PostDelayedTaskWithTraits(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::PostDelayedTask(
FROM_HERE,
{base::ThreadPool(), base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(&AsynchronousFreeMemoryMonitor),
base::TimeDelta::FromSeconds(kMemoryMonitorDelayInSeconds));
}
} // namespace

void StartFreeMemoryMonitor() {
base::PostTaskWithTraits(FROM_HERE,
{base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(&AsynchronousFreeMemoryMonitor));
base::PostTask(
FROM_HERE,
{base::ThreadPool(), base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(&AsynchronousFreeMemoryMonitor));
}
16 changes: 8 additions & 8 deletions ios/chrome/browser/application_context_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,9 @@ void RequestProxyResolvingSocketFactoryOnUIThread(
void RequestProxyResolvingSocketFactory(
ApplicationContextImpl* app_context,
network::mojom::ProxyResolvingSocketFactoryRequest request) {
base::PostTaskWithTraits(
FROM_HERE, {web::WebThread::UI},
base::BindOnce(&RequestProxyResolvingSocketFactoryOnUIThread, app_context,
std::move(request)));
base::PostTask(FROM_HERE, {web::WebThread::UI},
base::BindOnce(&RequestProxyResolvingSocketFactoryOnUIThread,
app_context, std::move(request)));
}

// Passed to NetworkConnectionTracker to bind a NetworkChangeManagerRequest.
Expand Down Expand Up @@ -405,8 +404,9 @@ void ApplicationContextImpl::CreateGCMDriver() {
CHECK(base::PathService::Get(ios::DIR_GLOBAL_GCM_STORE, &store_path));

scoped_refptr<base::SequencedTaskRunner> blocking_task_runner(
base::CreateSequencedTaskRunnerWithTraits(
{base::MayBlock(), base::TaskPriority::BEST_EFFORT,
base::CreateSequencedTaskRunner(
{base::ThreadPool(), base::MayBlock(),
base::TaskPriority::BEST_EFFORT,
base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN}));

gcm_driver_ = gcm::CreateGCMDriverDesktop(
Expand All @@ -418,7 +418,7 @@ void ApplicationContextImpl::CreateGCMDriver() {
GetSharedURLLoaderFactory(),
GetApplicationContext()->GetNetworkConnectionTracker(), ::GetChannel(),
IOSChromeGCMProfileServiceFactory::GetProductCategoryForSubtypes(),
base::CreateSingleThreadTaskRunnerWithTraits({web::WebThread::UI}),
base::CreateSingleThreadTaskRunnerWithTraits({web::WebThread::IO}),
base::CreateSingleThreadTaskRunner({web::WebThread::UI}),
base::CreateSingleThreadTaskRunner({web::WebThread::IO}),
blocking_task_runner);
}
2 changes: 1 addition & 1 deletion ios/chrome/browser/bookmarks/bookmark_model_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ std::unique_ptr<KeyedService> BookmarkModelFactory::BuildServiceInstanceFor(
browser_state->GetPrefs(), browser_state->GetStatePath(),
ios::StartupTaskRunnerServiceFactory::GetForBrowserState(browser_state)
->GetBookmarkTaskRunner(),
base::CreateSingleThreadTaskRunnerWithTraits({web::WebThread::UI}));
base::CreateSingleThreadTaskRunner({web::WebThread::UI}));
ios::BookmarkUndoServiceFactory::GetForBrowserState(browser_state)
->Start(bookmark_model.get());
return bookmark_model;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ void ClearCookies(

if (IsRemoveDataMaskSet(mask, BrowsingDataRemoveMask::REMOVE_COOKIES)) {
base::RecordAction(base::UserMetricsAction("ClearBrowsingData_Cookies"));
base::PostTaskWithTraits(
base::PostTask(
FROM_HERE, task_traits,
base::BindOnce(
&ClearCookies, context_getter_,
Expand Down Expand Up @@ -336,7 +336,7 @@ void ClearCookies(
IOSChromeIOThread* ios_chrome_io_thread =
GetApplicationContext()->GetIOSChromeIOThread();
if (ios_chrome_io_thread) {
base::PostTaskWithTraitsAndReply(
base::PostTaskAndReply(
FROM_HERE, task_traits,
base::BindOnce(&IOSChromeIOThread::ClearHostCache,
base::Unretained(ios_chrome_io_thread)),
Expand Down Expand Up @@ -451,7 +451,7 @@ void ClearCookies(
if (IsRemoveDataMaskSet(mask, BrowsingDataRemoveMask::REMOVE_CACHE)) {
base::RecordAction(base::UserMetricsAction("ClearBrowsingData_Cache"));
ClearHttpCache(context_getter_,
base::CreateSingleThreadTaskRunnerWithTraits(task_traits),
base::CreateSingleThreadTaskRunner(task_traits),
delete_begin, delete_end,
base::BindOnce(&NetCompletionCallbackAdapter,
CreatePendingTaskCompletionClosure()));
Expand Down
9 changes: 4 additions & 5 deletions ios/chrome/browser/browsing_data/cache_counter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,9 @@ class IOThreadCacheCounter {
backend_(nullptr) {}

void Count() {
base::PostTaskWithTraits(
FROM_HERE, {web::WebThread::IO},
base::BindRepeating(&IOThreadCacheCounter::CountInternal,
base::Unretained(this), net::OK));
base::PostTask(FROM_HERE, {web::WebThread::IO},
base::BindRepeating(&IOThreadCacheCounter::CountInternal,
base::Unretained(this), net::OK));
}

private:
Expand Down Expand Up @@ -84,7 +83,7 @@ class IOThreadCacheCounter {
case STEP_CALLBACK: {
result_ = rv;

base::PostTaskWithTraits(
base::PostTask(
FROM_HERE, {web::WebThread::UI},
base::BindOnce(&IOThreadCacheCounter::OnCountingFinished,
base::Unretained(this)));
Expand Down
20 changes: 9 additions & 11 deletions ios/chrome/browser/browsing_data/cache_counter_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,9 @@ class CacheCounterTest : public PlatformTest {
current_operation_ = OPERATION_ADD_ENTRY;
next_step_ = STEP_GET_BACKEND;

base::PostTaskWithTraits(
FROM_HERE, {web::WebThread::IO},
base::BindOnce(&CacheCounterTest::CacheOperationStep,
base::Unretained(this), net::OK));
base::PostTask(FROM_HERE, {web::WebThread::IO},
base::BindOnce(&CacheCounterTest::CacheOperationStep,
base::Unretained(this), net::OK));
WaitForIOThread();
}

Expand All @@ -73,10 +72,9 @@ class CacheCounterTest : public PlatformTest {
current_operation_ = OPERATION_CLEAR_CACHE;
next_step_ = STEP_GET_BACKEND;

base::PostTaskWithTraits(
FROM_HERE, {web::WebThread::IO},
base::BindOnce(&CacheCounterTest::CacheOperationStep,
base::Unretained(this), net::OK));
base::PostTask(FROM_HERE, {web::WebThread::IO},
base::BindOnce(&CacheCounterTest::CacheOperationStep,
base::Unretained(this), net::OK));
WaitForIOThread();
}

Expand Down Expand Up @@ -196,9 +194,9 @@ class CacheCounterTest : public PlatformTest {
if (current_operation_ == OPERATION_ADD_ENTRY)
entry_->Close();

base::PostTaskWithTraits(FROM_HERE, {web::WebThread::UI},
base::BindOnce(&CacheCounterTest::Callback,
base::Unretained(this)));
base::PostTask(FROM_HERE, {web::WebThread::UI},
base::BindOnce(&CacheCounterTest::Callback,
base::Unretained(this)));

break;
}
Expand Down
5 changes: 3 additions & 2 deletions ios/chrome/browser/crash_report/breakpad_helper.mm
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,9 @@ bool UserEnabledUploading() {
void CleanupCrashReports() {
base::FilePath crash_directory;
base::PathService::Get(ios::DIR_CRASH_DUMPS, &crash_directory);
base::PostTaskWithTraits(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::PostTask(
FROM_HERE,
{base::ThreadPool(), base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(&DeleteAllReportsInDirectory, crash_directory));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,9 @@ void RemoveFilesWithOptions(NSSet* files_to_keep, NSInteger age_in_days) {
const NSInteger kMinimumAgeInDays = 30;
NSInteger age_in_days = all_files ? 0 : kMinimumAgeInDays;

base::PostTaskWithTraitsAndReply(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::PostTaskAndReply(
FROM_HERE,
{base::ThreadPool(), base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(&RemoveFilesWithOptions, referenced_files, age_in_days),
base::Bind(&RunCallback, base::Passed(&closure_runner)));
}
Expand Down
7 changes: 4 additions & 3 deletions ios/chrome/browser/ios_chrome_main_parts.mm
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,9 @@
// remaining BACKGROUND+BLOCK_SHUTDOWN tasks is bumped by the ThreadPool on
// shutdown.
scoped_refptr<base::SequencedTaskRunner> local_state_task_runner =
base::CreateSequencedTaskRunnerWithTraits(
{base::MayBlock(), base::TaskPriority::BEST_EFFORT,
base::CreateSequencedTaskRunner(
{base::ThreadPool(), base::MayBlock(),
base::TaskPriority::BEST_EFFORT,
base::TaskShutdownBehavior::BLOCK_SHUTDOWN});

base::FilePath local_state_path;
Expand Down Expand Up @@ -212,7 +213,7 @@
// This will be called after the command-line has been mutated by about:flags
void IOSChromeMainParts::SetupFieldTrials() {
base::SetRecordActionTaskRunner(
base::CreateSingleThreadTaskRunnerWithTraits({web::WebThread::UI}));
base::CreateSingleThreadTaskRunner({web::WebThread::UI}));

// Initialize FieldTrialList to support FieldTrials that use one-time
// randomization.
Expand Down
5 changes: 3 additions & 2 deletions ios/chrome/browser/json_parser/in_process_json_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ void ParseJsonOnBackgroundThread(
void InProcessJsonParser::Parse(const std::string& unsafe_json,
SuccessCallback success_callback,
ErrorCallback error_callback) {
base::PostTaskWithTraits(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::PostTask(
FROM_HERE,
{base::ThreadPool(), base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(&ParseJsonOnBackgroundThread,
base::ThreadTaskRunnerHandle::Get(), unsafe_json,
std::move(success_callback), std::move(error_callback)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ std::unique_ptr<KeyedService> WebDataServiceFactory::BuildServiceInstanceFor(
const base::FilePath& browser_state_path = context->GetStatePath();
return std::make_unique<WebDataServiceWrapper>(
browser_state_path, GetApplicationContext()->GetApplicationLocale(),
base::CreateSingleThreadTaskRunnerWithTraits({web::WebThread::UI}),
base::CreateSingleThreadTaskRunner({web::WebThread::UI}),
base::DoNothing());
}

Expand Down
9 changes: 4 additions & 5 deletions ios/components/io_thread/ios_io_thread.mm
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@
IOSIOThread* io_thread)
: io_thread_(io_thread),
network_task_runner_(
base::CreateSingleThreadTaskRunnerWithTraits({web::WebThread::IO})) {}
base::CreateSingleThreadTaskRunner({web::WebThread::IO})) {}

SystemURLRequestContextGetter::~SystemURLRequestContextGetter() {}

Expand Down Expand Up @@ -210,10 +210,9 @@

void IOSIOThread::ChangedToOnTheRecord() {
DCHECK_CURRENTLY_ON(web::WebThread::UI);
base::PostTaskWithTraits(
FROM_HERE, {web::WebThread::IO},
base::BindOnce(&IOSIOThread::ChangedToOnTheRecordOnIOThread,
base::Unretained(this)));
base::PostTask(FROM_HERE, {web::WebThread::IO},
base::BindOnce(&IOSIOThread::ChangedToOnTheRecordOnIOThread,
base::Unretained(this)));
}

net::URLRequestContextGetter* IOSIOThread::system_url_request_context_getter() {
Expand Down
48 changes: 24 additions & 24 deletions ios/web/download/download_task_impl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,12 @@ - (void)URLSession:(NSURLSession*)session
task:(NSURLSessionTask*)task
didCompleteWithError:(nullable NSError*)error {
__weak CRWURLSessionDelegate* weakSelf = self;
base::PostTaskWithTraits(FROM_HERE, {WebThread::UI}, base::BindOnce(^{
CRWURLSessionDelegate* strongSelf = weakSelf;
if (strongSelf.propertiesBlock)
strongSelf.propertiesBlock(
task, error, /*terminal_callback=*/true);
}));
base::PostTask(FROM_HERE, {WebThread::UI}, base::BindOnce(^{
CRWURLSessionDelegate* strongSelf = weakSelf;
if (strongSelf.propertiesBlock)
strongSelf.propertiesBlock(task, error,
/*terminal_callback=*/true);
}));
}

- (void)URLSession:(NSURLSession*)session
Expand All @@ -132,26 +132,26 @@ - (void)URLSession:(NSURLSession*)session
using Bytes = const void* _Nonnull;
[data enumerateByteRangesUsingBlock:^(Bytes bytes, NSRange range, BOOL*) {
auto buffer = GetBuffer(bytes, range.length);
base::PostTaskWithTraits(FROM_HERE, {WebThread::UI}, base::BindOnce(^{
CRWURLSessionDelegate* strongSelf = weakSelf;
if (!strongSelf.dataBlock) {
dispatch_semaphore_signal(semaphore);
return;
}
strongSelf.dataBlock(std::move(buffer), ^{
// Data was written to disk, unblock queue to
// read the next chunk of downloaded data.
dispatch_semaphore_signal(semaphore);
});
}));
base::PostTask(FROM_HERE, {WebThread::UI}, base::BindOnce(^{
CRWURLSessionDelegate* strongSelf = weakSelf;
if (!strongSelf.dataBlock) {
dispatch_semaphore_signal(semaphore);
return;
}
strongSelf.dataBlock(std::move(buffer), ^{
// Data was written to disk, unblock queue to
// read the next chunk of downloaded data.
dispatch_semaphore_signal(semaphore);
});
}));
dispatch_semaphore_wait(semaphore, DISPATCH_TIME_FOREVER);
}];
base::PostTaskWithTraits(FROM_HERE, {WebThread::UI}, base::BindOnce(^{
CRWURLSessionDelegate* strongSelf = weakSelf;
if (strongSelf.propertiesBlock)
weakSelf.propertiesBlock(
task, nil, /*terminal_callback=*/false);
}));
base::PostTask(FROM_HERE, {WebThread::UI}, base::BindOnce(^{
CRWURLSessionDelegate* strongSelf = weakSelf;
if (strongSelf.propertiesBlock)
weakSelf.propertiesBlock(task, nil,
/*terminal_callback=*/false);
}));
}

- (void)URLSession:(NSURLSession*)session
Expand Down
4 changes: 2 additions & 2 deletions ios/web/find_in_page/find_in_page_manager_impl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@
if (all_frames.size() == 0) {
// No frames to search in.
// Call asyncronously to match behavior if find was successful in frames.
base::PostTaskWithTraits(
base::PostTask(
FROM_HERE, {WebThread::UI},
base::BindOnce(&FindInPageManagerImpl::LastFindRequestCompleted,
weak_factory_.GetWeakPtr()));
Expand All @@ -135,7 +135,7 @@
last_find_request_.DidReceiveFindResponseFromOneFrame();
if (last_find_request_.AreAllFindResponsesReturned()) {
// Call asyncronously to match behavior if find was done in frames.
base::PostTaskWithTraits(
base::PostTask(
FROM_HERE, {WebThread::UI},
base::BindOnce(&FindInPageManagerImpl::LastFindRequestCompleted,
weak_factory_.GetWeakPtr()));
Expand Down
Loading

0 comments on commit dce49dd

Please sign in to comment.