From 52c41b478ddaa2353c1a9fdb834557c51209dbf3 Mon Sep 17 00:00:00 2001 From: "davidben@chromium.org" Date: Fri, 14 Mar 2014 17:56:48 +0000 Subject: Reland "Fix various issues in RedirectToFileResourceHandler." It got reverted due to ASan issues. Fix child_id registration in ResourceDispatcherHostTests so that they happen on ForwardingFilter creation, rather than ad-hoc on a per-request basis. Original description: > - Handle errors in creating temporary files. > - Cancel on write failure instead of resuming. This used to work, but got > lost in some refactoring in r142108. > - Fix the OnResponseCompleted resume logic to account for partial writes. > - Don't lose write errors which occur after OnResponseCompleted is received. > - WeakPtrFactory goes after other members. > - OnWillStart was never forwarded to downstream handlers. > - Make sure the file is closed before deleting it. > > Also add a lot of machinery so we can better unit test this stack. Original Review URL: https://codereview.chromium.org/82273002 BUG=316634,347663 TEST=ResourceDispatcherHostTest.DownloadToFile ResourceDispatcherHostTest.RegisterDownloadedTempFile ResourceLoaderRedirectToFileTest.* TemporaryFileStreamTest.Basic Review URL: https://codereview.chromium.org/199453002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@257147 0039d316-1c4b-4281-b951-d872f2087c98 --- .../loader/resource_dispatcher_host_impl.cc | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) (limited to 'content/browser/loader/resource_dispatcher_host_impl.cc') diff --git a/content/browser/loader/resource_dispatcher_host_impl.cc b/content/browser/loader/resource_dispatcher_host_impl.cc index 4d91e01..69f9a95 100644 --- a/content/browser/loader/resource_dispatcher_host_impl.cc +++ b/content/browser/loader/resource_dispatcher_host_impl.cc @@ -1165,18 +1165,20 @@ scoped_ptr ResourceDispatcherHostImpl::CreateResourceHandler( handler.reset(new SyncResourceHandler(request, sync_result, this)); } else { handler.reset(new AsyncResourceHandler(request, this)); - if (IsDetachableResourceType(request_data.resource_type)) { - handler.reset(new DetachableResourceHandler( - request, - base::TimeDelta::FromMilliseconds(kDefaultDetachableCancelDelayMs), - handler.Pass())); - } } // The RedirectToFileResourceHandler depends on being next in the chain. if (request_data.download_to_file) { handler.reset( - new RedirectToFileResourceHandler(handler.Pass(), request, this)); + new RedirectToFileResourceHandler(handler.Pass(), request)); + } + + // Prefetches and requests outlive their child process. + if (!sync_result && IsDetachableResourceType(request_data.resource_type)) { + handler.reset(new DetachableResourceHandler( + request, + base::TimeDelta::FromMilliseconds(kDefaultDetachableCancelDelayMs), + handler.Pass())); } // Install a CrossSiteResourceHandler for all main frame requests. This will @@ -1231,7 +1233,11 @@ void ResourceDispatcherHostImpl::OnDataDownloadedACK(int request_id) { } void ResourceDispatcherHostImpl::RegisterDownloadedTempFile( - int child_id, int request_id, ShareableFileReference* reference) { + int child_id, int request_id, const base::FilePath& file_path) { + scoped_refptr reference = + ShareableFileReference::Get(file_path); + DCHECK(reference); + registered_temp_files_[child_id][request_id] = reference; ChildProcessSecurityPolicyImpl::GetInstance()->GrantReadFile( child_id, reference->path()); -- cgit v1.1