Commit 8532e551 authored by Rohit Rao's avatar Rohit Rao Committed by Commit Bot

[ios] Clarifies that WebThread::SetDelegate can only be called for the IO thread.

This is the //web counterpart to https://codereview.chromium.org/2558943002.

BUG=826465

Change-Id: I83e09e189ad0cbbdccaf9fa0de104efa7a6386a3
Reviewed-on: https://chromium-review.googlesource.com/c/1333898
Commit-Queue: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: default avatarGabriel Charette <gab@chromium.org>
Reviewed-by: default avatarSylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608342}
parent 73b48815
...@@ -182,13 +182,13 @@ IOSIOThread::IOSIOThread(PrefService* local_state, net::NetLog* net_log) ...@@ -182,13 +182,13 @@ IOSIOThread::IOSIOThread(PrefService* local_state, net::NetLog* net_log)
system_proxy_config_service_ = ProxyServiceFactory::CreateProxyConfigService( system_proxy_config_service_ = ProxyServiceFactory::CreateProxyConfigService(
pref_proxy_config_tracker_.get()); pref_proxy_config_tracker_.get());
web::WebThread::SetDelegate(web::WebThread::IO, this); web::WebThread::SetIOThreadDelegate(this);
} }
IOSIOThread::~IOSIOThread() { IOSIOThread::~IOSIOThread() {
// This isn't needed for production code, but in tests, IOSIOThread may // This isn't needed for production code, but in tests, IOSIOThread may
// be multiply constructed. // be multiply constructed.
web::WebThread::SetDelegate(web::WebThread::IO, nullptr); web::WebThread::SetIOThreadDelegate(nullptr);
pref_proxy_config_tracker_->DetachFromPrefService(); pref_proxy_config_tracker_->DetachFromPrefService();
DCHECK(!globals_); DCHECK(!globals_);
......
...@@ -88,7 +88,9 @@ class WebThread { ...@@ -88,7 +88,9 @@ class WebThread {
// sets identifier to its ID. // sets identifier to its ID.
static bool GetCurrentThreadIdentifier(ID* identifier) WARN_UNUSED_RESULT; static bool GetCurrentThreadIdentifier(ID* identifier) WARN_UNUSED_RESULT;
// Sets the delegate for the specified WebThread. // Sets the delegate for WebThread::IO.
//
// This only supports the IO thread.
// //
// Only one delegate may be registered at a time. Delegates may be // Only one delegate may be registered at a time. Delegates may be
// unregistered by providing a nullptr pointer. // unregistered by providing a nullptr pointer.
...@@ -96,7 +98,7 @@ class WebThread { ...@@ -96,7 +98,7 @@ class WebThread {
// If the caller unregisters a delegate before CleanUp has been // If the caller unregisters a delegate before CleanUp has been
// called, it must perform its own locking to ensure the delegate is // called, it must perform its own locking to ensure the delegate is
// not deleted while unregistering. // not deleted while unregistering.
static void SetDelegate(ID identifier, WebThreadDelegate* delegate); static void SetIOThreadDelegate(WebThreadDelegate* delegate);
// Returns an appropriate error message for when DCHECK_CURRENTLY_ON() fails. // Returns an appropriate error message for when DCHECK_CURRENTLY_ON() fails.
static std::string GetDCheckCurrentlyOnErrorMessage(ID expected); static std::string GetDCheckCurrentlyOnErrorMessage(ID expected);
......
...@@ -7,7 +7,8 @@ ...@@ -7,7 +7,8 @@
namespace web { namespace web {
// A class with this type may be registered via WebThread::SetDelegate. // WebThread::SetDelegate was deprecated, this is now only used by
// WebThread::SetIOThreadDelegate.
// //
// If registered as such, it will schedule to run Init() before the // If registered as such, it will schedule to run Init() before the
// message loop begins, and receive a CleanUp() call right after the message // message loop begins, and receive a CleanUp() call right after the message
......
...@@ -86,11 +86,11 @@ struct WebThreadTaskRunners { ...@@ -86,11 +86,11 @@ struct WebThreadTaskRunners {
base::LazyInstance<WebThreadTaskRunners>::Leaky g_task_runners = base::LazyInstance<WebThreadTaskRunners>::Leaky g_task_runners =
LAZY_INSTANCE_INITIALIZER; LAZY_INSTANCE_INITIALIZER;
using WebThreadDelegateAtomicPtr = base::subtle::AtomicWord;
struct WebThreadGlobals { struct WebThreadGlobals {
WebThreadGlobals() { WebThreadGlobals() {
memset(threads, 0, WebThread::ID_COUNT * sizeof(threads[0])); memset(threads, 0, WebThread::ID_COUNT * sizeof(threads[0]));
memset(thread_delegates, 0,
WebThread::ID_COUNT * sizeof(thread_delegates[0]));
} }
// This lock protects |threads|. Do not read or modify that array // This lock protects |threads|. Do not read or modify that array
...@@ -103,9 +103,10 @@ struct WebThreadGlobals { ...@@ -103,9 +103,10 @@ struct WebThreadGlobals {
// array upon destruction. // array upon destruction.
WebThreadImpl* threads[WebThread::ID_COUNT]; WebThreadImpl* threads[WebThread::ID_COUNT];
// Only atomic operations are used on this array. The delegates are not owned // Only atomic operations are used on this pointer. The delegate isn't owned
// by this array, rather by whoever calls WebThread::SetDelegate. // by WebThreadGlobals, rather by whoever calls
WebThreadDelegate* thread_delegates[WebThread::ID_COUNT]; // WebThread::SetIOThreadDelegate.
WebThreadDelegateAtomicPtr io_thread_delegate;
}; };
base::LazyInstance<WebThreadGlobals>::Leaky g_globals = base::LazyInstance<WebThreadGlobals>::Leaky g_globals =
...@@ -216,19 +217,13 @@ WebThreadImpl::WebThreadImpl(ID identifier, base::MessageLoop* message_loop) ...@@ -216,19 +217,13 @@ WebThreadImpl::WebThreadImpl(ID identifier, base::MessageLoop* message_loop)
} }
void WebThreadImpl::Init() { void WebThreadImpl::Init() {
WebThreadGlobals& globals = g_globals.Get(); if (identifier_ == WebThread::IO) {
WebThreadGlobals& globals = g_globals.Get();
using base::subtle::AtomicWord; WebThreadDelegateAtomicPtr delegate =
AtomicWord* storage = base::subtle::NoBarrier_Load(&globals.io_thread_delegate);
reinterpret_cast<AtomicWord*>(&globals.thread_delegates[identifier_]); if (delegate)
AtomicWord stored_pointer = base::subtle::NoBarrier_Load(storage); reinterpret_cast<WebThreadDelegate*>(delegate)->Init();
WebThreadDelegate* delegate =
reinterpret_cast<WebThreadDelegate*>(stored_pointer);
if (delegate) {
delegate->Init();
}
if (WebThread::CurrentlyOn(WebThread::IO)) {
// Though this thread is called the "IO" thread, it actually just routes // Though this thread is called the "IO" thread, it actually just routes
// messages around; it shouldn't be allowed to perform any blocking disk // messages around; it shouldn't be allowed to perform any blocking disk
// I/O. // I/O.
...@@ -266,20 +261,15 @@ void WebThreadImpl::Run(base::RunLoop* run_loop) { ...@@ -266,20 +261,15 @@ void WebThreadImpl::Run(base::RunLoop* run_loop) {
} }
void WebThreadImpl::CleanUp() { void WebThreadImpl::CleanUp() {
if (WebThread::CurrentlyOn(WebThread::IO)) if (identifier_ == WebThread::IO) {
IOThreadPreCleanUp(); IOThreadPreCleanUp();
WebThreadGlobals& globals = g_globals.Get(); WebThreadGlobals& globals = g_globals.Get();
WebThreadDelegateAtomicPtr delegate =
using base::subtle::AtomicWord; base::subtle::NoBarrier_Load(&globals.io_thread_delegate);
AtomicWord* storage = if (delegate)
reinterpret_cast<AtomicWord*>(&globals.thread_delegates[identifier_]); reinterpret_cast<WebThreadDelegate*>(delegate)->CleanUp();
AtomicWord stored_pointer = base::subtle::NoBarrier_Load(storage); }
WebThreadDelegate* delegate =
reinterpret_cast<WebThreadDelegate*>(stored_pointer);
if (delegate)
delegate->CleanUp();
} }
void WebThreadImpl::Initialize() { void WebThreadImpl::Initialize() {
...@@ -375,16 +365,16 @@ scoped_refptr<base::SingleThreadTaskRunner> WebThread::GetTaskRunnerForThread( ...@@ -375,16 +365,16 @@ scoped_refptr<base::SingleThreadTaskRunner> WebThread::GetTaskRunnerForThread(
} }
// static // static
void WebThread::SetDelegate(ID identifier, WebThreadDelegate* delegate) { void WebThread::SetIOThreadDelegate(WebThreadDelegate* delegate) {
using base::subtle::AtomicWord; using base::subtle::AtomicWord;
WebThreadGlobals& globals = g_globals.Get(); WebThreadGlobals& globals = g_globals.Get();
AtomicWord* storage = WebThreadDelegateAtomicPtr old_delegate =
reinterpret_cast<AtomicWord*>(&globals.thread_delegates[identifier]); base::subtle::NoBarrier_AtomicExchange(
AtomicWord old_pointer = base::subtle::NoBarrier_AtomicExchange( &globals.io_thread_delegate,
storage, reinterpret_cast<AtomicWord>(delegate)); reinterpret_cast<WebThreadDelegateAtomicPtr>(delegate));
// This catches registration when previously registered. // This catches registration when previously registered.
DCHECK(!delegate || !old_pointer); DCHECK(!delegate || !old_delegate);
} }
// static // static
......
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