Commit 896fd559 authored by Wez's avatar Wez Committed by Commit Bot

Fix ResourceDispatcherHostImpl to InvalidateWeakPtrs() on IO thread.

ResourceDispatcherHostImpl is a UI-thread-owned class which also does
work on the IO thread. It is built to intrinsically rely on being
destroyed only after all threads but the main thread have been torn-
down - the BrowserMainLoop invokes a Shutdown() API which posts a task
to perform teardown of state which is used on the IO thread, which is
thereby guaranteed to have run before the object is deleted.

Internally a WeakPtrFactory is also used, to ensure that replies posted
to the object on the IO thread are dropped if the object has already
performed its IO-thread teardown.  However, although the WeakPtrs were
bound to tasks run on the IO thread, the factory itself was being left
to implicitly InvalidateWeakPtrs() on destruction (i.e. on the UI
thread).

This CL adds explicit InvalidateWeakPtrs() on the IO thread, during
the IO-thread teardown task.

Bug: 729716
Change-Id: Idce31d52476df529dd5a877dc06747426c4e19c6
Reviewed-on: https://chromium-review.googlesource.com/1056255
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: default avatarCharlie Harrison <csharrison@chromium.org>
Reviewed-by: default avatarMatt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558332}
parent 80aa7d64
...@@ -332,7 +332,7 @@ ResourceDispatcherHostImpl::ResourceDispatcherHostImpl( ...@@ -332,7 +332,7 @@ ResourceDispatcherHostImpl::ResourceDispatcherHostImpl(
create_download_handler_intercept_(download_handler_intercept), create_download_handler_intercept_(download_handler_intercept),
main_thread_task_runner_(base::ThreadTaskRunnerHandle::Get()), main_thread_task_runner_(base::ThreadTaskRunnerHandle::Get()),
io_thread_task_runner_(io_thread_runner), io_thread_task_runner_(io_thread_runner),
weak_ptr_factory_(this) { weak_factory_on_io_(this) {
DCHECK(main_thread_task_runner_->BelongsToCurrentThread()); DCHECK(main_thread_task_runner_->BelongsToCurrentThread());
DCHECK(!g_resource_dispatcher_host); DCHECK(!g_resource_dispatcher_host);
g_resource_dispatcher_host = this; g_resource_dispatcher_host = this;
...@@ -689,6 +689,10 @@ void ResourceDispatcherHostImpl::OnShutdown() { ...@@ -689,6 +689,10 @@ void ResourceDispatcherHostImpl::OnShutdown() {
is_shutdown_ = true; is_shutdown_ = true;
// Explicitly invalidate while on the IO thread, where the associated WeakPtrs
// are used.
weak_factory_on_io_.InvalidateWeakPtrs();
pending_loaders_.clear(); pending_loaders_.clear();
// Make sure we shutdown the timer now, otherwise by the time our destructor // Make sure we shutdown the timer now, otherwise by the time our destructor
...@@ -2117,7 +2121,7 @@ void ResourceDispatcherHostImpl::UpdateLoadInfo() { ...@@ -2117,7 +2121,7 @@ void ResourceDispatcherHostImpl::UpdateLoadInfo() {
FROM_HERE, FROM_HERE,
base::BindOnce(UpdateLoadStateOnUI, loader_delegate_, std::move(infos)), base::BindOnce(UpdateLoadStateOnUI, loader_delegate_, std::move(infos)),
base::BindOnce(&ResourceDispatcherHostImpl::AckUpdateLoadInfo, base::BindOnce(&ResourceDispatcherHostImpl::AckUpdateLoadInfo,
weak_ptr_factory_.GetWeakPtr())); weak_factory_on_io_.GetWeakPtr()));
} }
void ResourceDispatcherHostImpl::AckUpdateLoadInfo() { void ResourceDispatcherHostImpl::AckUpdateLoadInfo() {
......
...@@ -762,7 +762,9 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl ...@@ -762,7 +762,9 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl
// Task runner for the IO thead. // Task runner for the IO thead.
scoped_refptr<base::SingleThreadTaskRunner> io_thread_task_runner_; scoped_refptr<base::SingleThreadTaskRunner> io_thread_task_runner_;
base::WeakPtrFactory<ResourceDispatcherHostImpl> weak_ptr_factory_; // Used on the IO thread to allow PostTaskAndReply replies to the IO thread
// to be abandoned if they run after OnShutdown().
base::WeakPtrFactory<ResourceDispatcherHostImpl> weak_factory_on_io_;
DISALLOW_COPY_AND_ASSIGN(ResourceDispatcherHostImpl); DISALLOW_COPY_AND_ASSIGN(ResourceDispatcherHostImpl);
}; };
......
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