diff options
author | davidben@chromium.org <davidben@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-14 17:56:48 +0000 |
---|---|---|
committer | davidben@chromium.org <davidben@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-14 17:56:48 +0000 |
commit | 52c41b478ddaa2353c1a9fdb834557c51209dbf3 (patch) | |
tree | a9d55ff3e6b5641d18f128e0602cea1736def212 /content/browser/loader | |
parent | 296556cfa9dcef7ad152bad87caf23ae795a1d39 (diff) | |
download | chromium_src-52c41b478ddaa2353c1a9fdb834557c51209dbf3.zip chromium_src-52c41b478ddaa2353c1a9fdb834557c51209dbf3.tar.gz chromium_src-52c41b478ddaa2353c1a9fdb834557c51209dbf3.tar.bz2 |
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
Diffstat (limited to 'content/browser/loader')
11 files changed, 1045 insertions, 141 deletions
diff --git a/content/browser/loader/async_resource_handler.cc b/content/browser/loader/async_resource_handler.cc index 5326b66..7014a47 100644 --- a/content/browser/loader/async_resource_handler.cc +++ b/content/browser/loader/async_resource_handler.cc @@ -200,6 +200,15 @@ bool AsyncResourceHandler::OnResponseStarted(int request_id, net::GetHostOrSpecFromURL(request_url)))); } + // If the parent handler downloaded the resource to a file, grant the child + // read permissions on it. Note: there is similar logic in + // SyncResourceHandler. + if (!response->head.download_file_path.empty()) { + rdh_->RegisterDownloadedTempFile( + info->GetChildID(), info->GetRequestID(), + response->head.download_file_path); + } + response->head.request_start = request()->creation_time(); response->head.response_start = TimeTicks::Now(); info->filter()->Send(new ResourceMsg_ReceivedResponse(request_id, diff --git a/content/browser/loader/redirect_to_file_resource_handler.cc b/content/browser/loader/redirect_to_file_resource_handler.cc index 74848b6..84cc930 100644 --- a/content/browser/loader/redirect_to_file_resource_handler.cc +++ b/content/browser/loader/redirect_to_file_resource_handler.cc @@ -5,14 +5,12 @@ #include "content/browser/loader/redirect_to_file_resource_handler.h" #include "base/bind.h" -#include "base/files/file_util_proxy.h" #include "base/logging.h" -#include "base/message_loop/message_loop_proxy.h" #include "base/platform_file.h" #include "base/threading/thread_restrictions.h" -#include "content/browser/loader/resource_dispatcher_host_impl.h" #include "content/browser/loader/resource_request_info_impl.h" -#include "content/public/browser/browser_thread.h" +#include "content/browser/loader/temporary_file_stream.h" +#include "content/public/browser/resource_controller.h" #include "content/public/common/resource_response.h" #include "net/base/file_stream.h" #include "net/base/io_buffer.h" @@ -53,23 +51,108 @@ namespace content { static const int kInitialReadBufSize = 32768; static const int kMaxReadBufSize = 524288; +// A separate IO thread object to manage the lifetime of the net::FileStream and +// the ShareableFileReference. When the handler is destroyed, it asynchronously +// closes net::FileStream after all pending writes complete. Only after the +// stream is closed is the ShareableFileReference released, to ensure the +// temporary is not deleted before it is closed. +class RedirectToFileResourceHandler::Writer { + public: + Writer(RedirectToFileResourceHandler* handler, + scoped_ptr<net::FileStream> file_stream, + ShareableFileReference* deletable_file) + : handler_(handler), + file_stream_(file_stream.Pass()), + is_writing_(false), + deletable_file_(deletable_file) { + DCHECK(!deletable_file_->path().empty()); + } + + bool is_writing() const { return is_writing_; } + const base::FilePath& path() const { return deletable_file_->path(); } + + int Write(net::IOBuffer* buf, int buf_len) { + DCHECK(!is_writing_); + DCHECK(handler_); + int result = file_stream_->Write( + buf, buf_len, + base::Bind(&Writer::DidWriteToFile, base::Unretained(this))); + if (result == net::ERR_IO_PENDING) + is_writing_ = true; + return result; + } + + void Close() { + handler_ = NULL; + if (!is_writing_) + CloseAndDelete(); + } + + private: + // Only DidClose can delete this. + ~Writer() { + } + + void DidWriteToFile(int result) { + DCHECK(is_writing_); + is_writing_ = false; + if (handler_) { + handler_->DidWriteToFile(result); + } else { + CloseAndDelete(); + } + } + + void CloseAndDelete() { + DCHECK(!is_writing_); + int result = file_stream_->Close(base::Bind(&Writer::DidClose, + base::Unretained(this))); + if (result != net::ERR_IO_PENDING) + DidClose(result); + } + + void DidClose(int result) { + delete this; + } + + RedirectToFileResourceHandler* handler_; + + scoped_ptr<net::FileStream> file_stream_; + bool is_writing_; + + // We create a ShareableFileReference that's deletable for the temp file + // created as a result of the download. + scoped_refptr<webkit_blob::ShareableFileReference> deletable_file_; + + DISALLOW_COPY_AND_ASSIGN(Writer); +}; + RedirectToFileResourceHandler::RedirectToFileResourceHandler( scoped_ptr<ResourceHandler> next_handler, - net::URLRequest* request, - ResourceDispatcherHostImpl* host) + net::URLRequest* request) : LayeredResourceHandler(request, next_handler.Pass()), - weak_factory_(this), - host_(host), buf_(new net::GrowableIOBuffer()), buf_write_pending_(false), write_cursor_(0), - write_callback_pending_(false), + writer_(NULL), next_buffer_size_(kInitialReadBufSize), did_defer_(false), - completed_during_write_(false) { + completed_during_write_(false), + weak_factory_(this) { } RedirectToFileResourceHandler::~RedirectToFileResourceHandler() { + // Orphan the writer to asynchronously close and release the temporary file. + if (writer_) { + writer_->Close(); + writer_ = NULL; + } +} + +void RedirectToFileResourceHandler:: + SetCreateTemporaryFileStreamFunctionForTesting( + const CreateTemporaryFileStreamFunction& create_temporary_file_stream) { + create_temporary_file_stream_ = create_temporary_file_stream; } bool RedirectToFileResourceHandler::OnResponseStarted( @@ -78,8 +161,8 @@ bool RedirectToFileResourceHandler::OnResponseStarted( bool* defer) { if (response->head.error_code == net::OK || response->head.error_code == net::ERR_IO_PENDING) { - DCHECK(deletable_file_.get() && !deletable_file_->path().empty()); - response->head.download_file_path = deletable_file_->path(); + DCHECK(writer_); + response->head.download_file_path = writer_->path(); } return next_handler_->OnResponseStarted(request_id, response, defer); } @@ -87,19 +170,23 @@ bool RedirectToFileResourceHandler::OnResponseStarted( bool RedirectToFileResourceHandler::OnWillStart(int request_id, const GURL& url, bool* defer) { - if (!deletable_file_.get()) { - // Defer starting the request until we have created the temporary file. - // TODO(darin): This is sub-optimal. We should not delay starting the - // network request like this. - did_defer_ = *defer = true; - base::FileUtilProxy::CreateTemporary( - BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE).get(), - base::PLATFORM_FILE_ASYNC, + DCHECK(!writer_); + + // Defer starting the request until we have created the temporary file. + // TODO(darin): This is sub-optimal. We should not delay starting the + // network request like this. + will_start_url_ = url; + did_defer_ = *defer = true; + if (create_temporary_file_stream_.is_null()) { + CreateTemporaryFileStream( + base::Bind(&RedirectToFileResourceHandler::DidCreateTemporaryFile, + weak_factory_.GetWeakPtr())); + } else { + create_temporary_file_stream_.Run( base::Bind(&RedirectToFileResourceHandler::DidCreateTemporaryFile, weak_factory_.GetWeakPtr())); - return true; } - return next_handler_->OnWillStart(request_id, url, defer); + return true; } bool RedirectToFileResourceHandler::OnWillRead( @@ -151,7 +238,7 @@ void RedirectToFileResourceHandler::OnResponseCompleted( const net::URLRequestStatus& status, const std::string& security_info, bool* defer) { - if (write_callback_pending_) { + if (writer_ && writer_->is_writing()) { completed_during_write_ = true; completed_status_ = status; completed_security_info_ = security_info; @@ -163,25 +250,31 @@ void RedirectToFileResourceHandler::OnResponseCompleted( } void RedirectToFileResourceHandler::DidCreateTemporaryFile( - base::File::Error /*error_code*/, - base::PassPlatformFile file_handle, - const base::FilePath& file_path) { - deletable_file_ = ShareableFileReference::GetOrCreate( - file_path, - ShareableFileReference::DELETE_ON_FINAL_RELEASE, - BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE).get()); - file_stream_.reset( - new net::FileStream(file_handle.ReleaseValue(), - base::PLATFORM_FILE_WRITE | base::PLATFORM_FILE_ASYNC, - NULL)); - const ResourceRequestInfo* info = GetRequestInfo(); - host_->RegisterDownloadedTempFile( - info->GetChildID(), info->GetRequestID(), deletable_file_.get()); - ResumeIfDeferred(); + base::File::Error error_code, + scoped_ptr<net::FileStream> file_stream, + ShareableFileReference* deletable_file) { + DCHECK(!writer_); + if (error_code != base::File::FILE_OK) { + controller()->CancelWithError(net::FileErrorToNetError(error_code)); + return; + } + + writer_ = new Writer(this, file_stream.Pass(), deletable_file); + + // Resume the request. + DCHECK(did_defer_); + bool defer = false; + if (!next_handler_->OnWillStart(GetRequestID(), will_start_url_, &defer)) { + controller()->Cancel(); + } else if (!defer) { + ResumeIfDeferred(); + } else { + did_defer_ = false; + } + will_start_url_ = GURL(); } void RedirectToFileResourceHandler::DidWriteToFile(int result) { - write_callback_pending_ = false; int request_id = GetRequestID(); bool failed = false; @@ -194,20 +287,38 @@ void RedirectToFileResourceHandler::DidWriteToFile(int result) { } if (failed) { - ResumeIfDeferred(); - } else if (completed_during_write_) { + DCHECK(!writer_->is_writing()); + // TODO(davidben): Recover the error code from WriteMore or |result|, as + // appropriate. + if (completed_during_write_ && completed_status_.is_success()) { + // If the request successfully completed mid-write, but the write failed, + // convert the status to a failure for downstream. + completed_status_.set_status(net::URLRequestStatus::CANCELED); + completed_status_.set_error(net::ERR_FAILED); + } + if (!completed_during_write_) + controller()->CancelWithError(net::ERR_FAILED); + } + + if (completed_during_write_ && !writer_->is_writing()) { + // Resume shutdown now that all data has been written to disk. Note that + // this should run even in the |failed| case above, otherwise a failed write + // leaves the handler stuck. bool defer = false; next_handler_->OnResponseCompleted(request_id, completed_status_, completed_security_info_, &defer); - if (!defer) + if (!defer) { ResumeIfDeferred(); + } else { + did_defer_ = false; + } } } bool RedirectToFileResourceHandler::WriteMore() { - DCHECK(file_stream_.get()); + DCHECK(writer_); for (;;) { if (write_cursor_ == buf_->offset()) { // We've caught up to the network load, but it may be in the process of @@ -220,7 +331,7 @@ bool RedirectToFileResourceHandler::WriteMore() { } return true; } - if (write_callback_pending_) + if (writer_->is_writing()) return true; DCHECK(write_cursor_ < buf_->offset()); @@ -242,15 +353,9 @@ bool RedirectToFileResourceHandler::WriteMore() { buf_.get(), buf_->StartOfBuffer() + write_cursor_); int write_len = buf_->offset() - write_cursor_; - int rv = file_stream_->Write( - wrapped.get(), - write_len, - base::Bind(&RedirectToFileResourceHandler::DidWriteToFile, - base::Unretained(this))); - if (rv == net::ERR_IO_PENDING) { - write_callback_pending_ = true; + int rv = writer_->Write(wrapped.get(), write_len); + if (rv == net::ERR_IO_PENDING) return true; - } if (rv <= 0) return false; next_handler_->OnDataDownloaded(GetRequestID(), rv); diff --git a/content/browser/loader/redirect_to_file_resource_handler.h b/content/browser/loader/redirect_to_file_resource_handler.h index f83dd54..6ad2c66 100644 --- a/content/browser/loader/redirect_to_file_resource_handler.h +++ b/content/browser/loader/redirect_to_file_resource_handler.h @@ -5,6 +5,9 @@ #ifndef CONTENT_BROWSER_LOADER_REDIRECT_TO_FILE_RESOURCE_HANDLER_H_ #define CONTENT_BROWSER_LOADER_REDIRECT_TO_FILE_RESOURCE_HANDLER_H_ +#include "base/basictypes.h" +#include "base/callback.h" +#include "base/compiler_specific.h" #include "base/files/file.h" #include "base/files/file_path.h" #include "base/memory/ref_counted.h" @@ -12,8 +15,11 @@ #include "base/memory/weak_ptr.h" #include "base/platform_file.h" #include "content/browser/loader/layered_resource_handler.h" +#include "content/browser/loader/temporary_file_stream.h" +#include "content/common/content_export.h" #include "net/url_request/url_request.h" #include "net/url_request/url_request_status.h" +#include "url/gurl.h" namespace net { class FileStream; @@ -25,19 +31,32 @@ class ShareableFileReference; } namespace content { -class ResourceDispatcherHostImpl; -// Redirects network data to a file. This is intended to be layered in front -// of either the AsyncResourceHandler or the SyncResourceHandler. -class RedirectToFileResourceHandler : public LayeredResourceHandler { +// Redirects network data to a file. This is intended to be layered in front of +// either the AsyncResourceHandler or the SyncResourceHandler. The downstream +// resource handler does not see OnWillRead or OnReadCompleted calls. Instead, +// the ResourceResponse contains the path to a temporary file and +// OnDataDownloaded is called as the file downloads. +class CONTENT_EXPORT RedirectToFileResourceHandler + : public LayeredResourceHandler { public: - RedirectToFileResourceHandler( - scoped_ptr<ResourceHandler> next_handler, - net::URLRequest* request, - ResourceDispatcherHostImpl* resource_dispatcher_host); + typedef base::Callback<void(const CreateTemporaryFileStreamCallback&)> + CreateTemporaryFileStreamFunction; + + // Create a RedirectToFileResourceHandler for |request| which wraps + // |next_handler|. + RedirectToFileResourceHandler(scoped_ptr<ResourceHandler> next_handler, + net::URLRequest* request); virtual ~RedirectToFileResourceHandler(); - // ResourceHandler implementation: + // Replace the CreateTemporaryFileStream implementation with a mocked one for + // testing purposes. The function should create a net::FileStream and a + // ShareableFileReference and then asynchronously pass them to the + // CreateTemporaryFileStreamCallback. + void SetCreateTemporaryFileStreamFunctionForTesting( + const CreateTemporaryFileStreamFunction& create_temporary_file_stream); + + // LayeredResourceHandler implementation: virtual bool OnResponseStarted(int request_id, ResourceResponse* response, bool* defer) OVERRIDE; @@ -57,17 +76,19 @@ class RedirectToFileResourceHandler : public LayeredResourceHandler { bool* defer) OVERRIDE; private: - void DidCreateTemporaryFile(base::File::Error error_code, - base::PassPlatformFile file_handle, - const base::FilePath& file_path); + void DidCreateTemporaryFile( + base::File::Error error_code, + scoped_ptr<net::FileStream> file_stream, + webkit_blob::ShareableFileReference* deletable_file); + + // Called by RedirectToFileResourceHandler::Writer. void DidWriteToFile(int result); + bool WriteMore(); bool BufIsFull() const; void ResumeIfDeferred(); - base::WeakPtrFactory<RedirectToFileResourceHandler> weak_factory_; - - ResourceDispatcherHostImpl* host_; + CreateTemporaryFileStreamFunction create_temporary_file_stream_; // We allocate a single, fixed-size IO buffer (buf_) used to read from the // network (buf_write_pending_ is true while the system is copying data into @@ -80,8 +101,11 @@ class RedirectToFileResourceHandler : public LayeredResourceHandler { bool buf_write_pending_; int write_cursor_; - scoped_ptr<net::FileStream> file_stream_; - bool write_callback_pending_; + // Helper writer object which maintains references to the net::FileStream and + // webkit_blob::ShareableFileReference. This is maintained separately so that, + // on Windows, the temporary file isn't deleted until after it is closed. + class Writer; + Writer* writer_; // |next_buffer_size_| is the size of the buffer to be allocated on the next // OnWillRead() call. We exponentially grow the size of the buffer allocated @@ -90,16 +114,15 @@ class RedirectToFileResourceHandler : public LayeredResourceHandler { // was filled, up to a maximum size of 512k. int next_buffer_size_; - // We create a ShareableFileReference that's deletable for the temp - // file created as a result of the download. - scoped_refptr<webkit_blob::ShareableFileReference> deletable_file_; - - bool did_defer_ ; + bool did_defer_; bool completed_during_write_; + GURL will_start_url_; net::URLRequestStatus completed_status_; std::string completed_security_info_; + base::WeakPtrFactory<RedirectToFileResourceHandler> weak_factory_; + DISALLOW_COPY_AND_ASSIGN(RedirectToFileResourceHandler); }; 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<ResourceHandler> 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 <a ping> 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<ShareableFileReference> reference = + ShareableFileReference::Get(file_path); + DCHECK(reference); + registered_temp_files_[child_id][request_id] = reference; ChildProcessSecurityPolicyImpl::GetInstance()->GrantReadFile( child_id, reference->path()); diff --git a/content/browser/loader/resource_dispatcher_host_impl.h b/content/browser/loader/resource_dispatcher_host_impl.h index 8a8bdc3..abf2ada 100644 --- a/content/browser/loader/resource_dispatcher_host_impl.h +++ b/content/browser/loader/resource_dispatcher_host_impl.h @@ -186,7 +186,7 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl // no longer needed. void RegisterDownloadedTempFile( int child_id, int request_id, - webkit_blob::ShareableFileReference* reference); + const base::FilePath& file_path); void UnregisterDownloadedTempFile(int child_id, int request_id); // Needed for the sync IPC message dispatcher macros. diff --git a/content/browser/loader/resource_dispatcher_host_unittest.cc b/content/browser/loader/resource_dispatcher_host_unittest.cc index 040fe40..6c09723 100644 --- a/content/browser/loader/resource_dispatcher_host_unittest.cc +++ b/content/browser/loader/resource_dispatcher_host_unittest.cc @@ -4,7 +4,9 @@ #include <vector> +#include "base/basictypes.h" #include "base/bind.h" +#include "base/file_util.h" #include "base/files/file_path.h" #include "base/memory/scoped_vector.h" #include "base/message_loop/message_loop.h" @@ -32,6 +34,7 @@ #include "content/public/common/process_type.h" #include "content/public/common/resource_response.h" #include "content/public/test/test_browser_context.h" +#include "content/public/test/test_browser_thread_bundle.h" #include "content/test/test_content_browser_client.h" #include "net/base/net_errors.h" #include "net/base/request_priority.h" @@ -46,10 +49,13 @@ #include "net/url_request/url_request_test_util.h" #include "testing/gtest/include/gtest/gtest.h" #include "webkit/common/appcache/appcache_interfaces.h" +#include "webkit/common/blob/shareable_file_reference.h" // TODO(eroman): Write unit tests for SafeBrowsing that exercise // SafeBrowsingResourceHandler. +using webkit_blob::ShareableFileReference; + namespace content { namespace { @@ -86,6 +92,7 @@ static int RequestIDForMessage(const IPC::Message& msg) { case ResourceMsg_ReceivedRedirect::ID: case ResourceMsg_SetDataBuffer::ID: case ResourceMsg_DataReceived::ID: + case ResourceMsg_DataDownloaded::ID: case ResourceMsg_RequestComplete::ID: { bool result = PickleIterator(msg).ReadInt(&request_id); DCHECK(result); @@ -181,6 +188,7 @@ class ForwardingFilter : public ResourceMessageFilter { base::Unretained(this))), dest_(dest), resource_context_(resource_context) { + ChildProcessSecurityPolicyImpl::GetInstance()->Add(child_id()); set_peer_pid_for_testing(base::GetCurrentProcId()); } @@ -559,32 +567,47 @@ class TestResourceDispatcherHostDelegate scoped_ptr<base::SupportsUserData::Data> user_data_; }; +// Waits for a ShareableFileReference to be released. +class ShareableFileReleaseWaiter { + public: + ShareableFileReleaseWaiter(const base::FilePath& path) { + scoped_refptr<ShareableFileReference> file = + ShareableFileReference::Get(path); + file->AddFinalReleaseCallback( + base::Bind(&ShareableFileReleaseWaiter::Released, + base::Unretained(this))); + } + + void Wait() { + loop_.Run(); + } + + private: + void Released(const base::FilePath& path) { + loop_.Quit(); + } + + base::RunLoop loop_; + + DISALLOW_COPY_AND_ASSIGN(ShareableFileReleaseWaiter); +}; + class ResourceDispatcherHostTest : public testing::Test, public IPC::Sender { public: ResourceDispatcherHostTest() - : ui_thread_(BrowserThread::UI, &message_loop_), - file_thread_(BrowserThread::FILE_USER_BLOCKING, &message_loop_), - cache_thread_(BrowserThread::CACHE, &message_loop_), - io_thread_(BrowserThread::IO, &message_loop_), + : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP), old_factory_(NULL), send_data_received_acks_(false) { browser_context_.reset(new TestBrowserContext()); BrowserContext::EnsureResourceContextInitialized(browser_context_.get()); - message_loop_.RunUntilIdle(); + base::RunLoop().RunUntilIdle(); + filter_ = MakeForwardingFilter(); ResourceContext* resource_context = browser_context_->GetResourceContext(); - filter_ = new ForwardingFilter(this, resource_context); resource_context->GetRequestContext()->set_network_delegate( &network_delegate_); } - virtual ~ResourceDispatcherHostTest() { - for (std::set<int>::iterator it = child_ids_.begin(); - it != child_ids_.end(); ++it) { - host_.CancelRequestsForProcess(*it); - } - } - // IPC::Sender implementation virtual bool Send(IPC::Message* msg) OVERRIDE { accum_.AddMessage(*msg); @@ -594,13 +617,18 @@ class ResourceDispatcherHostTest : public testing::Test, GenerateDataReceivedACK(*msg); } + if (wait_for_request_complete_loop_ && + msg->type() == ResourceMsg_RequestComplete::ID) { + wait_for_request_complete_loop_->Quit(); + } + delete msg; return true; } protected: // testing::Test - virtual void SetUp() { + virtual void SetUp() OVERRIDE { DCHECK(!test_fixture_); test_fixture_ = this; ChildProcessSecurityPolicyImpl::GetInstance()->Add(0); @@ -626,6 +654,11 @@ class ResourceDispatcherHostTest : public testing::Test, DCHECK(test_fixture_ == this); test_fixture_ = NULL; + for (std::set<int>::iterator it = child_ids_.begin(); + it != child_ids_.end(); ++it) { + host_.CancelRequestsForProcess(*it); + } + host_.Shutdown(); ChildProcessSecurityPolicyImpl::GetInstance()->Remove(0); @@ -638,7 +671,16 @@ class ResourceDispatcherHostTest : public testing::Test, WorkerServiceImpl::GetInstance()->PerformTeardownForTesting(); browser_context_.reset(); - message_loop_.RunUntilIdle(); + base::RunLoop().RunUntilIdle(); + } + + // Creates a new ForwardingFilter and registers it with |child_ids_| so as not + // to leak per-child state on test shutdown. + ForwardingFilter* MakeForwardingFilter() { + ForwardingFilter* filter = + new ForwardingFilter(this, browser_context_->GetResourceContext()); + child_ids_.insert(filter->child_id()); + return filter; } // Creates a request using the current test object as the filter and @@ -774,11 +816,14 @@ class ResourceDispatcherHostTest : public testing::Test, return old_filter; } - base::MessageLoopForIO message_loop_; - BrowserThreadImpl ui_thread_; - BrowserThreadImpl file_thread_; - BrowserThreadImpl cache_thread_; - BrowserThreadImpl io_thread_; + void WaitForRequestComplete() { + DCHECK(!wait_for_request_complete_loop_); + wait_for_request_complete_loop_.reset(new base::RunLoop); + wait_for_request_complete_loop_->Run(); + wait_for_request_complete_loop_.reset(); + } + + content::TestBrowserThreadBundle thread_bundle_; scoped_ptr<TestBrowserContext> browser_context_; scoped_refptr<ForwardingFilter> filter_; net::TestNetworkDelegate network_delegate_; @@ -790,6 +835,7 @@ class ResourceDispatcherHostTest : public testing::Test, net::URLRequest::ProtocolFactory* old_factory_; bool send_data_received_acks_; std::set<int> child_ids_; + scoped_ptr<base::RunLoop> wait_for_request_complete_loop_; static ResourceDispatcherHostTest* test_fixture_; static bool delay_start_; static bool delay_complete_; @@ -816,9 +862,6 @@ void ResourceDispatcherHostTest::MakeTestRequestWithResourceType( int request_id, const GURL& url, ResourceType::Type type) { - // If it's already there, this'll be dropped on the floor, which is fine. - child_ids_.insert(filter->child_id()); - ResourceHostMsg_Request request = CreateResourceRequest("GET", type, url); ResourceHostMsg_RequestResource msg(render_view_id, request_id, request); @@ -1407,6 +1450,7 @@ class TestFilter : public ForwardingFilter { TEST_F(ResourceDispatcherHostTest, TestProcessCancel) { scoped_refptr<TestFilter> test_filter = new TestFilter( browser_context_->GetResourceContext()); + child_ids_.insert(test_filter->child_id()); // request 1 goes to the test delegate ResourceHostMsg_Request request = CreateResourceRequest( @@ -1623,8 +1667,7 @@ TEST_F(ResourceDispatcherHostTest, TestBlockingCancelingRequests) { // Tests that blocked requests are canceled if their associated process dies. TEST_F(ResourceDispatcherHostTest, TestBlockedRequestsProcessDies) { // This second filter is used to emulate a second process. - scoped_refptr<ForwardingFilter> second_filter = new ForwardingFilter( - this, browser_context_->GetResourceContext()); + scoped_refptr<ForwardingFilter> second_filter = MakeForwardingFilter(); host_.BlockRequestsForRoute(second_filter->child_id(), 0); @@ -1670,8 +1713,7 @@ TEST_F(ResourceDispatcherHostTest, TestBlockedRequestsProcessDies) { // destructor to make sure the blocked requests are deleted. TEST_F(ResourceDispatcherHostTest, TestBlockedRequestsDontLeak) { // This second filter is used to emulate a second process. - scoped_refptr<ForwardingFilter> second_filter = new ForwardingFilter( - this, browser_context_->GetResourceContext()); + scoped_refptr<ForwardingFilter> second_filter = MakeForwardingFilter(); host_.BlockRequestsForRoute(filter_->child_id(), 1); host_.BlockRequestsForRoute(filter_->child_id(), 2); @@ -1755,8 +1797,7 @@ TEST_F(ResourceDispatcherHostTest, TooMuchOutstandingRequestsMemory) { size_t kMaxRequests = kMaxCostPerProcess / kMemoryCostOfTest2Req; // This second filter is used to emulate a second process. - scoped_refptr<ForwardingFilter> second_filter = new ForwardingFilter( - this, browser_context_->GetResourceContext()); + scoped_refptr<ForwardingFilter> second_filter = MakeForwardingFilter(); // Saturate the number of outstanding requests for our process. for (size_t i = 0; i < kMaxRequests; ++i) { @@ -1824,10 +1865,8 @@ TEST_F(ResourceDispatcherHostTest, TooManyOutstandingRequests) { host_.set_max_num_in_flight_requests(kMaxRequests); // Needed to emulate additional processes. - scoped_refptr<ForwardingFilter> second_filter = new ForwardingFilter( - this, browser_context_->GetResourceContext()); - scoped_refptr<ForwardingFilter> third_filter = new ForwardingFilter( - this, browser_context_->GetResourceContext()); + scoped_refptr<ForwardingFilter> second_filter = MakeForwardingFilter(); + scoped_refptr<ForwardingFilter> third_filter = MakeForwardingFilter(); // Saturate the number of outstanding requests for our process. for (size_t i = 0; i < kMaxRequestsPerProcess; ++i) { @@ -2214,8 +2253,7 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationHtml) { SetBrowserClientForTesting(old_client); // This second filter is used to emulate a second process. - scoped_refptr<ForwardingFilter> second_filter = new ForwardingFilter( - this, browser_context_->GetResourceContext()); + scoped_refptr<ForwardingFilter> second_filter = MakeForwardingFilter(); int new_render_view_id = 1; int new_request_id = 2; @@ -2226,8 +2264,6 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationHtml) { request.transferred_request_child_id = filter_->child_id(); request.transferred_request_request_id = request_id; - // For cleanup. - child_ids_.insert(second_filter->child_id()); ResourceHostMsg_RequestResource transfer_request_msg( new_render_view_id, new_request_id, request); host_.OnMessageReceived( @@ -2292,8 +2328,7 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationText) { SetBrowserClientForTesting(old_client); // This second filter is used to emulate a second process. - scoped_refptr<ForwardingFilter> second_filter = new ForwardingFilter( - this, browser_context_->GetResourceContext()); + scoped_refptr<ForwardingFilter> second_filter = MakeForwardingFilter(); int new_render_view_id = 1; int new_request_id = 2; @@ -2304,8 +2339,6 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationText) { request.transferred_request_child_id = filter_->child_id(); request.transferred_request_request_id = request_id; - // For cleanup. - child_ids_.insert(second_filter->child_id()); ResourceHostMsg_RequestResource transfer_request_msg( new_render_view_id, new_request_id, request); host_.OnMessageReceived( @@ -2346,16 +2379,13 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationWithProcessCrash) { // Create a first filter that can be deleted before the second one starts. { - scoped_refptr<ForwardingFilter> first_filter = new ForwardingFilter( - this, browser_context_->GetResourceContext()); + scoped_refptr<ForwardingFilter> first_filter = MakeForwardingFilter(); first_child_id = first_filter->child_id(); ResourceHostMsg_Request first_request = CreateResourceRequest("GET", ResourceType::MAIN_FRAME, GURL("http://example.com/blah")); - // For cleanup. - child_ids_.insert(first_child_id); ResourceHostMsg_RequestResource first_request_msg( render_view_id, request_id, first_request); bool msg_was_ok; @@ -2385,8 +2415,7 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationWithProcessCrash) { GlobalRequestID first_global_request_id(first_child_id, request_id); // This second filter is used to emulate a second process. - scoped_refptr<ForwardingFilter> second_filter = new ForwardingFilter( - this, browser_context_->GetResourceContext()); + scoped_refptr<ForwardingFilter> second_filter = MakeForwardingFilter(); int new_render_view_id = 1; int new_request_id = 2; @@ -2468,8 +2497,7 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationWithTwoRedirects) { SetBrowserClientForTesting(old_client); // This second filter is used to emulate a second process. - scoped_refptr<ForwardingFilter> second_filter = new ForwardingFilter( - this, browser_context_->GetResourceContext()); + scoped_refptr<ForwardingFilter> second_filter = MakeForwardingFilter(); int new_render_view_id = 1; int new_request_id = 2; @@ -2712,4 +2740,161 @@ TEST_F(ResourceDispatcherHostTest, DataReceivedUnexpectedACKs) { } } +// Tests the dispatcher host's temporary file management. +TEST_F(ResourceDispatcherHostTest, RegisterDownloadedTempFile) { + const int kRequestID = 1; + + // Create a temporary file. + base::FilePath file_path; + ASSERT_TRUE(base::CreateTemporaryFile(&file_path)); + scoped_refptr<ShareableFileReference> deletable_file = + ShareableFileReference::GetOrCreate( + file_path, + ShareableFileReference::DELETE_ON_FINAL_RELEASE, + BrowserThread::GetMessageLoopProxyForThread( + BrowserThread::FILE).get()); + + // Not readable. + EXPECT_FALSE(ChildProcessSecurityPolicyImpl::GetInstance()->CanReadFile( + filter_->child_id(), file_path)); + + // Register it for a resource request. + host_.RegisterDownloadedTempFile(filter_->child_id(), kRequestID, file_path); + + // Should be readable now. + EXPECT_TRUE(ChildProcessSecurityPolicyImpl::GetInstance()->CanReadFile( + filter_->child_id(), file_path)); + + // The child releases from the request. + bool msg_was_ok = true; + ResourceHostMsg_ReleaseDownloadedFile release_msg(kRequestID); + host_.OnMessageReceived(release_msg, filter_, &msg_was_ok); + ASSERT_TRUE(msg_was_ok); + + // Still readable because there is another reference to the file. (The child + // may take additional blob references.) + EXPECT_TRUE(ChildProcessSecurityPolicyImpl::GetInstance()->CanReadFile( + filter_->child_id(), file_path)); + + // Release extra references and wait for the file to be deleted. (This relies + // on the delete happening on the FILE thread which is mapped to main thread + // in this test.) + deletable_file = NULL; + base::RunLoop().RunUntilIdle(); + + // The file is no longer readable to the child and has been deleted. + EXPECT_FALSE(ChildProcessSecurityPolicyImpl::GetInstance()->CanReadFile( + filter_->child_id(), file_path)); + EXPECT_FALSE(base::PathExists(file_path)); +} + +// Tests that temporary files held on behalf of child processes are released +// when the child process dies. +TEST_F(ResourceDispatcherHostTest, ReleaseTemporiesOnProcessExit) { + const int kRequestID = 1; + + // Create a temporary file. + base::FilePath file_path; + ASSERT_TRUE(base::CreateTemporaryFile(&file_path)); + scoped_refptr<ShareableFileReference> deletable_file = + ShareableFileReference::GetOrCreate( + file_path, + ShareableFileReference::DELETE_ON_FINAL_RELEASE, + BrowserThread::GetMessageLoopProxyForThread( + BrowserThread::FILE).get()); + + // Register it for a resource request. + host_.RegisterDownloadedTempFile(filter_->child_id(), kRequestID, file_path); + deletable_file = NULL; + + // Should be readable now. + EXPECT_TRUE(ChildProcessSecurityPolicyImpl::GetInstance()->CanReadFile( + filter_->child_id(), file_path)); + + // Let the process die. + filter_->OnChannelClosing(); + base::RunLoop().RunUntilIdle(); + + // The file is no longer readable to the child and has been deleted. + EXPECT_FALSE(ChildProcessSecurityPolicyImpl::GetInstance()->CanReadFile( + filter_->child_id(), file_path)); + EXPECT_FALSE(base::PathExists(file_path)); +} + +TEST_F(ResourceDispatcherHostTest, DownloadToFile) { + // Make a request which downloads to file. + ResourceHostMsg_Request request = CreateResourceRequest( + "GET", ResourceType::SUB_RESOURCE, net::URLRequestTestJob::test_url_1()); + request.download_to_file = true; + ResourceHostMsg_RequestResource request_msg(0, 1, request); + bool msg_was_ok; + host_.OnMessageReceived(request_msg, filter_, &msg_was_ok); + ASSERT_TRUE(msg_was_ok); + + // Running the message loop until idle does not work because + // RedirectToFileResourceHandler posts things to base::WorkerPool. Instead, + // wait for the ResourceMsg_RequestComplete to go out. Then run the event loop + // until idle so the loader is gone. + WaitForRequestComplete(); + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(0, host_.pending_requests()); + + ResourceIPCAccumulator::ClassifiedMessages msgs; + accum_.GetClassifiedMessages(&msgs); + + ASSERT_EQ(1U, msgs.size()); + const std::vector<IPC::Message>& messages = msgs[0]; + + // The request should contain the following messages: + // ReceivedResponse (indicates headers received and filename) + // DataDownloaded* (bytes downloaded and total length) + // RequestComplete (request is done) + + // ReceivedResponse + ResourceResponseHead response_head; + GetResponseHead(messages, &response_head); + ASSERT_FALSE(response_head.download_file_path.empty()); + + // DataDownloaded + size_t total_len = 0; + for (size_t i = 1; i < messages.size() - 1; i++) { + ASSERT_EQ(ResourceMsg_DataDownloaded::ID, messages[i].type()); + PickleIterator iter(messages[i]); + int request_id, data_len; + ASSERT_TRUE(IPC::ReadParam(&messages[i], &iter, &request_id)); + ASSERT_TRUE(IPC::ReadParam(&messages[i], &iter, &data_len)); + total_len += data_len; + } + EXPECT_EQ(net::URLRequestTestJob::test_data_1().size(), total_len); + + // RequestComplete + CheckRequestCompleteErrorCode(messages.back(), net::OK); + + // Verify that the data ended up in the temporary file. + std::string contents; + ASSERT_TRUE(base::ReadFileToString(response_head.download_file_path, + &contents)); + EXPECT_EQ(net::URLRequestTestJob::test_data_1(), contents); + + // The file should be readable by the child. + EXPECT_TRUE(ChildProcessSecurityPolicyImpl::GetInstance()->CanReadFile( + filter_->child_id(), response_head.download_file_path)); + + // When the renderer releases the file, it should be deleted. Again, + // RunUntilIdle doesn't work because base::WorkerPool is involved. + ShareableFileReleaseWaiter waiter(response_head.download_file_path); + ResourceHostMsg_ReleaseDownloadedFile release_msg(1); + host_.OnMessageReceived(release_msg, filter_, &msg_was_ok); + ASSERT_TRUE(msg_was_ok); + waiter.Wait(); + // The release callback runs before the delete is scheduled, so pump the + // message loop for the delete itself. (This relies on the delete happening on + // the FILE thread which is mapped to main thread in this test.) + base::RunLoop().RunUntilIdle(); + + EXPECT_FALSE(base::PathExists(response_head.download_file_path)); + EXPECT_FALSE(ChildProcessSecurityPolicyImpl::GetInstance()->CanReadFile( + filter_->child_id(), response_head.download_file_path)); +} + } // namespace content diff --git a/content/browser/loader/resource_loader_unittest.cc b/content/browser/loader/resource_loader_unittest.cc index 363c9ff..c4183d0 100644 --- a/content/browser/loader/resource_loader_unittest.cc +++ b/content/browser/loader/resource_loader_unittest.cc @@ -4,21 +4,33 @@ #include "content/browser/loader/resource_loader.h" +#include "base/file_util.h" +#include "base/files/file.h" +#include "base/message_loop/message_loop_proxy.h" +#include "base/platform_file.h" #include "base/run_loop.h" #include "content/browser/browser_thread_impl.h" +#include "content/browser/loader/redirect_to_file_resource_handler.h" #include "content/browser/loader/resource_loader_delegate.h" #include "content/public/browser/resource_request_info.h" +#include "content/public/common/resource_response.h" #include "content/public/test/mock_resource_context.h" #include "content/public/test/test_browser_thread_bundle.h" #include "content/test/test_content_browser_client.h" #include "ipc/ipc_message.h" +#include "net/base/mock_file_stream.h" #include "net/base/request_priority.h" #include "net/cert/x509_certificate.h" #include "net/ssl/client_cert_store.h" #include "net/ssl/ssl_cert_request_info.h" #include "net/url_request/url_request.h" +#include "net/url_request/url_request_job_factory_impl.h" +#include "net/url_request/url_request_test_job.h" #include "net/url_request/url_request_test_util.h" #include "testing/gtest/include/gtest/gtest.h" +#include "webkit/common/blob/shareable_file_reference.h" + +using webkit_blob::ShareableFileReference; namespace content { namespace { @@ -69,16 +81,34 @@ class ClientCertStoreStub : public net::ClientCertStore { // initialize ResourceLoader. class ResourceHandlerStub : public ResourceHandler { public: - ResourceHandlerStub() - : ResourceHandler(NULL), defer_request_on_will_start_(false) {} + explicit ResourceHandlerStub(net::URLRequest* request) + : ResourceHandler(request), + defer_request_on_will_start_(false), + received_response_completed_(false), + total_bytes_downloaded_(0) { + } void set_defer_request_on_will_start(bool defer_request_on_will_start) { defer_request_on_will_start_ = defer_request_on_will_start; } + const GURL& start_url() const { return start_url_; } + ResourceResponse* response() const { return response_.get(); } + bool received_response_completed() const { + return received_response_completed_; + } + const net::URLRequestStatus& status() const { return status_; } + int total_bytes_downloaded() const { return total_bytes_downloaded_; } + + void Resume() { + controller()->Resume(); + } + + // ResourceHandler implementation: virtual bool OnUploadProgress(int request_id, uint64 position, uint64 size) OVERRIDE { + NOTREACHED(); return true; } @@ -86,18 +116,23 @@ class ResourceHandlerStub : public ResourceHandler { const GURL& url, ResourceResponse* response, bool* defer) OVERRIDE { + NOTREACHED(); return true; } virtual bool OnResponseStarted(int request_id, ResourceResponse* response, bool* defer) OVERRIDE { + EXPECT_FALSE(response_); + response_ = response; return true; } virtual bool OnWillStart(int request_id, const GURL& url, bool* defer) OVERRIDE { + EXPECT_TRUE(start_url_.is_empty()); + start_url_ = url; *defer = defer_request_on_will_start_; return true; } @@ -112,26 +147,40 @@ class ResourceHandlerStub : public ResourceHandler { scoped_refptr<net::IOBuffer>* buf, int* buf_size, int min_size) OVERRIDE { - return true; + NOTREACHED(); + return false; } virtual bool OnReadCompleted(int request_id, int bytes_read, bool* defer) OVERRIDE { - return true; + NOTREACHED(); + return false; } virtual void OnResponseCompleted(int request_id, const net::URLRequestStatus& status, const std::string& security_info, bool* defer) OVERRIDE { + // TODO(davidben): This DCHECK currently fires everywhere. Fix the places in + // ResourceLoader where OnResponseCompleted is signaled twice. + // DCHECK(!received_response_completed_); + received_response_completed_ = true; + status_ = status; } virtual void OnDataDownloaded(int request_id, - int bytes_downloaded) OVERRIDE {} + int bytes_downloaded) OVERRIDE { + total_bytes_downloaded_ += bytes_downloaded; + } private: bool defer_request_on_will_start_; + GURL start_url_; + scoped_refptr<ResourceResponse> response_; + bool received_response_completed_; + net::URLRequestStatus status_; + int total_bytes_downloaded_; }; // Test browser client that captures calls to SelectClientCertificates and @@ -180,6 +229,16 @@ class ResourceContextStub : public MockResourceContext { scoped_ptr<net::ClientCertStore> dummy_cert_store_; }; +// Fails to create a temporary file with the given error. +void CreateTemporaryError( + base::File::Error error, + const CreateTemporaryFileStreamCallback& callback) { + base::MessageLoop::current()->PostTask( + FROM_HERE, + base::Bind(callback, error, base::Passed(scoped_ptr<net::FileStream>()), + scoped_refptr<ShareableFileReference>())); +} + } // namespace class ResourceLoaderTest : public testing::Test, @@ -187,7 +246,26 @@ class ResourceLoaderTest : public testing::Test, protected: ResourceLoaderTest() : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP), - resource_context_(&test_url_request_context_) { + resource_context_(&test_url_request_context_), + raw_ptr_resource_handler_(NULL), + raw_ptr_to_request_(NULL) { + job_factory_.SetProtocolHandler( + "test", net::URLRequestTestJob::CreateProtocolHandler()); + test_url_request_context_.set_job_factory(&job_factory_); + } + + GURL test_url() const { + return net::URLRequestTestJob::test_url_1(); + } + + std::string test_data() const { + return net::URLRequestTestJob::test_data_1(); + } + + virtual scoped_ptr<ResourceHandler> WrapResourceHandler( + scoped_ptr<ResourceHandlerStub> leaf_handler, + net::URLRequest* request) { + return leaf_handler.PassAs<ResourceHandler>(); } virtual void SetUp() OVERRIDE { @@ -195,7 +273,7 @@ class ResourceLoaderTest : public testing::Test, const int kRenderViewId = 2; scoped_ptr<net::URLRequest> request( - new net::URLRequest(GURL("dummy"), + new net::URLRequest(test_url(), net::DEFAULT_PRIORITY, NULL, resource_context_.GetRequestContext())); @@ -207,10 +285,13 @@ class ResourceLoaderTest : public testing::Test, kRenderViewId, MSG_ROUTING_NONE, false); - scoped_ptr<ResourceHandlerStub> resource_handler(new ResourceHandlerStub()); + scoped_ptr<ResourceHandlerStub> resource_handler( + new ResourceHandlerStub(request.get())); raw_ptr_resource_handler_ = resource_handler.get(); loader_.reset(new ResourceLoader( - request.Pass(), resource_handler.PassAs<ResourceHandler>(), this)); + request.Pass(), + WrapResourceHandler(resource_handler.Pass(), raw_ptr_to_request_), + this)); } // ResourceLoaderDelegate: @@ -231,6 +312,7 @@ class ResourceLoaderTest : public testing::Test, content::TestBrowserThreadBundle thread_bundle_; + net::URLRequestJobFactoryImpl job_factory_; net::TestURLRequestContext test_url_request_context_; ResourceContextStub resource_context_; @@ -322,4 +404,259 @@ TEST_F(ResourceLoaderTest, ResumeCancelledRequest) { static_cast<ResourceController*>(loader_.get())->Resume(); } +class ResourceLoaderRedirectToFileTest : public ResourceLoaderTest { + public: + ResourceLoaderRedirectToFileTest() + : file_stream_(NULL), + redirect_to_file_resource_handler_(NULL) { + } + + base::FilePath temp_path() const { return temp_path_; } + ShareableFileReference* deletable_file() const { + return deletable_file_.get(); + } + net::testing::MockFileStream* file_stream() const { return file_stream_; } + RedirectToFileResourceHandler* redirect_to_file_resource_handler() const { + return redirect_to_file_resource_handler_; + } + + void ReleaseLoader() { + file_stream_ = NULL; + deletable_file_ = NULL; + loader_.reset(); + } + + virtual scoped_ptr<ResourceHandler> WrapResourceHandler( + scoped_ptr<ResourceHandlerStub> leaf_handler, + net::URLRequest* request) OVERRIDE { + // Make a temporary file. + CHECK(base::CreateTemporaryFile(&temp_path_)); + base::PlatformFile platform_file = + base::CreatePlatformFile(temp_path_, + base::PLATFORM_FILE_WRITE | + base::PLATFORM_FILE_TEMPORARY | + base::PLATFORM_FILE_CREATE_ALWAYS | + base::PLATFORM_FILE_ASYNC, + NULL, NULL); + CHECK_NE(base::kInvalidPlatformFileValue, platform_file); + + // Create mock file streams and a ShareableFileReference. + scoped_ptr<net::testing::MockFileStream> file_stream( + new net::testing::MockFileStream( + platform_file, + base::PLATFORM_FILE_WRITE | base::PLATFORM_FILE_ASYNC, + NULL, base::MessageLoopProxy::current())); + file_stream_ = file_stream.get(); + deletable_file_ = ShareableFileReference::GetOrCreate( + temp_path_, + ShareableFileReference::DELETE_ON_FINAL_RELEASE, + BrowserThread::GetMessageLoopProxyForThread( + BrowserThread::FILE).get()); + + // Inject them into the handler. + scoped_ptr<RedirectToFileResourceHandler> handler( + new RedirectToFileResourceHandler( + leaf_handler.PassAs<ResourceHandler>(), request)); + redirect_to_file_resource_handler_ = handler.get(); + handler->SetCreateTemporaryFileStreamFunctionForTesting( + base::Bind(&ResourceLoaderRedirectToFileTest::PostCallback, + base::Unretained(this), + base::Passed(file_stream.PassAs<net::FileStream>()))); + return handler.PassAs<ResourceHandler>(); + } + + private: + void PostCallback( + scoped_ptr<net::FileStream> file_stream, + const CreateTemporaryFileStreamCallback& callback) { + base::MessageLoop::current()->PostTask( + FROM_HERE, + base::Bind(callback, base::File::FILE_OK, + base::Passed(&file_stream), deletable_file_)); + } + + base::FilePath temp_path_; + scoped_refptr<ShareableFileReference> deletable_file_; + // These are owned by the ResourceLoader. + net::testing::MockFileStream* file_stream_; + RedirectToFileResourceHandler* redirect_to_file_resource_handler_; +}; + +// Tests that a RedirectToFileResourceHandler works and forwards everything +// downstream. +TEST_F(ResourceLoaderRedirectToFileTest, Basic) { + // Run it to completion. + loader_->StartRequest(); + base::RunLoop().RunUntilIdle(); + + // Check that the handler forwarded all information to the downstream handler. + EXPECT_EQ(temp_path(), + raw_ptr_resource_handler_->response()->head.download_file_path); + EXPECT_EQ(test_url(), raw_ptr_resource_handler_->start_url()); + EXPECT_TRUE(raw_ptr_resource_handler_->received_response_completed()); + EXPECT_EQ(net::URLRequestStatus::SUCCESS, + raw_ptr_resource_handler_->status().status()); + EXPECT_EQ(test_data().size(), static_cast<size_t>( + raw_ptr_resource_handler_->total_bytes_downloaded())); + + // Check that the data was written to the file. + std::string contents; + ASSERT_TRUE(base::ReadFileToString(temp_path(), &contents)); + EXPECT_EQ(test_data(), contents); + + // Release the loader and the saved reference to file. The file should be gone + // now. + ReleaseLoader(); + base::RunLoop().RunUntilIdle(); + EXPECT_FALSE(base::PathExists(temp_path())); +} + +// Tests that RedirectToFileResourceHandler handles errors in creating the +// temporary file. +TEST_F(ResourceLoaderRedirectToFileTest, CreateTemporaryError) { + // Swap out the create temporary function. + redirect_to_file_resource_handler()-> + SetCreateTemporaryFileStreamFunctionForTesting( + base::Bind(&CreateTemporaryError, base::File::FILE_ERROR_FAILED)); + + // Run it to completion. + loader_->StartRequest(); + base::RunLoop().RunUntilIdle(); + + // To downstream, the request was canceled. + EXPECT_TRUE(raw_ptr_resource_handler_->received_response_completed()); + EXPECT_EQ(net::URLRequestStatus::CANCELED, + raw_ptr_resource_handler_->status().status()); + EXPECT_EQ(0, raw_ptr_resource_handler_->total_bytes_downloaded()); +} + +// Tests that RedirectToFileResourceHandler handles synchronous write errors. +TEST_F(ResourceLoaderRedirectToFileTest, WriteError) { + file_stream()->set_forced_error(net::ERR_FAILED); + + // Run it to completion. + loader_->StartRequest(); + base::RunLoop().RunUntilIdle(); + + // To downstream, the request was canceled sometime after it started, but + // before any data was written. + EXPECT_EQ(temp_path(), + raw_ptr_resource_handler_->response()->head.download_file_path); + EXPECT_EQ(test_url(), raw_ptr_resource_handler_->start_url()); + EXPECT_TRUE(raw_ptr_resource_handler_->received_response_completed()); + EXPECT_EQ(net::URLRequestStatus::CANCELED, + raw_ptr_resource_handler_->status().status()); + EXPECT_EQ(0, raw_ptr_resource_handler_->total_bytes_downloaded()); + + // Release the loader. The file should be gone now. + ReleaseLoader(); + base::RunLoop().RunUntilIdle(); + EXPECT_FALSE(base::PathExists(temp_path())); +} + +// Tests that RedirectToFileResourceHandler handles asynchronous write errors. +TEST_F(ResourceLoaderRedirectToFileTest, WriteErrorAsync) { + file_stream()->set_forced_error_async(net::ERR_FAILED); + + // Run it to completion. + loader_->StartRequest(); + base::RunLoop().RunUntilIdle(); + + // To downstream, the request was canceled sometime after it started, but + // before any data was written. + EXPECT_EQ(temp_path(), + raw_ptr_resource_handler_->response()->head.download_file_path); + EXPECT_EQ(test_url(), raw_ptr_resource_handler_->start_url()); + EXPECT_TRUE(raw_ptr_resource_handler_->received_response_completed()); + EXPECT_EQ(net::URLRequestStatus::CANCELED, + raw_ptr_resource_handler_->status().status()); + EXPECT_EQ(0, raw_ptr_resource_handler_->total_bytes_downloaded()); + + // Release the loader. The file should be gone now. + ReleaseLoader(); + base::RunLoop().RunUntilIdle(); + EXPECT_FALSE(base::PathExists(temp_path())); +} + +// Tests that RedirectToFileHandler defers completion if there are outstanding +// writes and accounts for errors which occur in that time. +TEST_F(ResourceLoaderRedirectToFileTest, DeferCompletion) { + // Program the MockFileStream to error asynchronously, but throttle the + // callback. + file_stream()->set_forced_error_async(net::ERR_FAILED); + file_stream()->ThrottleCallbacks(); + + // Run it as far as it will go. + loader_->StartRequest(); + base::RunLoop().RunUntilIdle(); + + // At this point, the request should have completed. + EXPECT_EQ(net::URLRequestStatus::SUCCESS, + raw_ptr_to_request_->status().status()); + + // However, the resource loader stack is stuck somewhere after receiving the + // response. + EXPECT_EQ(temp_path(), + raw_ptr_resource_handler_->response()->head.download_file_path); + EXPECT_EQ(test_url(), raw_ptr_resource_handler_->start_url()); + EXPECT_FALSE(raw_ptr_resource_handler_->received_response_completed()); + EXPECT_EQ(0, raw_ptr_resource_handler_->total_bytes_downloaded()); + + // Now, release the floodgates. + file_stream()->ReleaseCallbacks(); + base::RunLoop().RunUntilIdle(); + + // Although the URLRequest was successful, the leaf handler sees a failure + // because the write never completed. + EXPECT_TRUE(raw_ptr_resource_handler_->received_response_completed()); + EXPECT_EQ(net::URLRequestStatus::CANCELED, + raw_ptr_resource_handler_->status().status()); + + // Release the loader. The file should be gone now. + ReleaseLoader(); + base::RunLoop().RunUntilIdle(); + EXPECT_FALSE(base::PathExists(temp_path())); +} + +// Tests that a RedirectToFileResourceHandler behaves properly when the +// downstream handler defers OnWillStart. +TEST_F(ResourceLoaderRedirectToFileTest, DownstreamDeferStart) { + // Defer OnWillStart. + raw_ptr_resource_handler_->set_defer_request_on_will_start(true); + + // Run as far as we'll go. + loader_->StartRequest(); + base::RunLoop().RunUntilIdle(); + + // The request should have stopped at OnWillStart. + EXPECT_EQ(test_url(), raw_ptr_resource_handler_->start_url()); + EXPECT_FALSE(raw_ptr_resource_handler_->response()); + EXPECT_FALSE(raw_ptr_resource_handler_->received_response_completed()); + EXPECT_EQ(0, raw_ptr_resource_handler_->total_bytes_downloaded()); + + // Now resume the request. Now we complete. + raw_ptr_resource_handler_->Resume(); + base::RunLoop().RunUntilIdle(); + + // Check that the handler forwarded all information to the downstream handler. + EXPECT_EQ(temp_path(), + raw_ptr_resource_handler_->response()->head.download_file_path); + EXPECT_EQ(test_url(), raw_ptr_resource_handler_->start_url()); + EXPECT_TRUE(raw_ptr_resource_handler_->received_response_completed()); + EXPECT_EQ(net::URLRequestStatus::SUCCESS, + raw_ptr_resource_handler_->status().status()); + EXPECT_EQ(test_data().size(), static_cast<size_t>( + raw_ptr_resource_handler_->total_bytes_downloaded())); + + // Check that the data was written to the file. + std::string contents; + ASSERT_TRUE(base::ReadFileToString(temp_path(), &contents)); + EXPECT_EQ(test_data(), contents); + + // Release the loader. The file should be gone now. + ReleaseLoader(); + base::RunLoop().RunUntilIdle(); + EXPECT_FALSE(base::PathExists(temp_path())); +} + } // namespace content diff --git a/content/browser/loader/sync_resource_handler.cc b/content/browser/loader/sync_resource_handler.cc index 865b29e..e39ae1c 100644 --- a/content/browser/loader/sync_resource_handler.cc +++ b/content/browser/loader/sync_resource_handler.cc @@ -86,6 +86,18 @@ bool SyncResourceHandler::OnResponseStarted( DevToolsNetLogObserver::PopulateResponseInfo(request(), response); + // If the parent handler downloaded the resource to a file, grant the child + // read permissions on it. Note: there is similar logic in + // AsyncResourceHandler. + // + // TODO(davidben): Can we remove support for download_file in sync requests + // altogether? I don't think Blink ever makes such requests. + if (!response->head.download_file_path.empty()) { + rdh_->RegisterDownloadedTempFile( + info->GetChildID(), info->GetRequestID(), + response->head.download_file_path); + } + // We don't care about copying the status here. result_.headers = response->head.headers; result_.mime_type = response->head.mime_type; diff --git a/content/browser/loader/temporary_file_stream.cc b/content/browser/loader/temporary_file_stream.cc new file mode 100644 index 0000000..1d23ae9 --- /dev/null +++ b/content/browser/loader/temporary_file_stream.cc @@ -0,0 +1,61 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "content/browser/loader/temporary_file_stream.h" + +#include "base/bind.h" +#include "base/callback.h" +#include "base/files/file_util_proxy.h" +#include "base/memory/ref_counted.h" +#include "content/public/browser/browser_thread.h" +#include "net/base/file_stream.h" +#include "webkit/common/blob/shareable_file_reference.h" + +using webkit_blob::ShareableFileReference; + +namespace content { + +namespace { + +void DidCreateTemporaryFile( + const CreateTemporaryFileStreamCallback& callback, + base::File::Error error_code, + base::PassPlatformFile file_handle, + const base::FilePath& file_path) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + + if (error_code != base::File::FILE_OK) { + callback.Run(error_code, scoped_ptr<net::FileStream>(), NULL); + return; + } + + // Cancelled or not, create the deletable_file so the temporary is cleaned up. + scoped_refptr<ShareableFileReference> deletable_file = + ShareableFileReference::GetOrCreate( + file_path, + ShareableFileReference::DELETE_ON_FINAL_RELEASE, + BrowserThread::GetMessageLoopProxyForThread( + BrowserThread::FILE).get()); + + scoped_ptr<net::FileStream> file_stream(new net::FileStream( + file_handle.ReleaseValue(), + base::PLATFORM_FILE_WRITE | base::PLATFORM_FILE_ASYNC, + NULL)); + + callback.Run(error_code, file_stream.Pass(), deletable_file); +} + +} // namespace + +void CreateTemporaryFileStream( + const CreateTemporaryFileStreamCallback& callback) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + + base::FileUtilProxy::CreateTemporary( + BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE).get(), + base::PLATFORM_FILE_ASYNC, + base::Bind(&DidCreateTemporaryFile, callback)); +} + +} // namespace content diff --git a/content/browser/loader/temporary_file_stream.h b/content/browser/loader/temporary_file_stream.h new file mode 100644 index 0000000..51d524d --- /dev/null +++ b/content/browser/loader/temporary_file_stream.h @@ -0,0 +1,46 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CONTENT_BROWSER_LOADER_TEMPORARY_FILE_STREAM_H_ +#define CONTENT_BROWSER_LOADER_TEMPORARY_FILE_STREAM_H_ + +#include "base/callback_forward.h" +#include "base/files/file.h" +#include "base/memory/scoped_ptr.h" +#include "base/platform_file.h" +#include "content/common/content_export.h" + +namespace net { +class FileStream; +} + +namespace webkit_blob { +class ShareableFileReference; +} + +namespace content { + +typedef base::Callback<void(base::File::Error, + scoped_ptr<net::FileStream>, + webkit_blob::ShareableFileReference*)> + CreateTemporaryFileStreamCallback; + +// Creates a temporary file and asynchronously calls |callback| with a +// net::FileStream and webkit_blob::ShareableFileReference. The file is deleted +// when the webkit_blob::ShareableFileReference is deleted. Note it is the +// consumer's responsibility to ensure the webkit_blob::ShareableFileReference +// stays in scope until net::FileStream has finished closing the file. On error, +// |callback| is called with an error in the first parameter. +// +// This function may only be called on the IO thread. +// +// TODO(davidben): Juggling the net::FileStream and +// webkit_blob::ShareableFileReference lifetimes is a nuisance. The two should +// be tied together so the consumer need not deal with it. +CONTENT_EXPORT void CreateTemporaryFileStream( + const CreateTemporaryFileStreamCallback& callback); + +} // namespace content + +#endif // CONTENT_BROWSER_LOADER_TEMPORARY_FILE_STREAM_H_ diff --git a/content/browser/loader/temporary_file_stream_unittest.cc b/content/browser/loader/temporary_file_stream_unittest.cc new file mode 100644 index 0000000..1903d44 --- /dev/null +++ b/content/browser/loader/temporary_file_stream_unittest.cc @@ -0,0 +1,120 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "content/browser/loader/temporary_file_stream.h" + +#include <string.h> + +#include <string> + +#include "base/basictypes.h" +#include "base/bind.h" +#include "base/file_util.h" +#include "base/files/file_path.h" +#include "base/run_loop.h" +#include "content/public/test/test_browser_thread_bundle.h" +#include "net/base/file_stream.h" +#include "net/base/io_buffer.h" +#include "net/base/net_errors.h" +#include "net/base/test_completion_callback.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "webkit/common/blob/shareable_file_reference.h" + +using webkit_blob::ShareableFileReference; + +namespace content { + +namespace { + +const char kTestData[] = "0123456789"; +const int kTestDataSize = arraysize(kTestData) - 1; + +class WaitForFileStream { + public: + base::File::Error error() const { return error_; } + net::FileStream* file_stream() const { return file_stream_.get(); } + ShareableFileReference* deletable_file() const { + return deletable_file_.get(); + } + + void OnFileStreamCreated(base::File::Error error, + scoped_ptr<net::FileStream> file_stream, + ShareableFileReference* deletable_file) { + error_ = error; + file_stream_ = file_stream.Pass(); + deletable_file_ = deletable_file; + loop_.Quit(); + } + + void Wait() { + loop_.Run(); + } + + void Release() { + file_stream_.reset(NULL); + deletable_file_ = NULL; + } + private: + base::RunLoop loop_; + base::File::Error error_; + scoped_ptr<net::FileStream> file_stream_; + scoped_refptr<ShareableFileReference> deletable_file_; +}; + +} // namespace + +TEST(TemporaryFileStreamTest, Basic) { + TestBrowserThreadBundle thread_bundle(TestBrowserThreadBundle::IO_MAINLOOP); + + // Create a temporary. + WaitForFileStream file_stream_waiter; + CreateTemporaryFileStream(base::Bind(&WaitForFileStream::OnFileStreamCreated, + base::Unretained(&file_stream_waiter))); + file_stream_waiter.Wait(); + + // The temporary should exist. + EXPECT_EQ(base::File::FILE_OK, file_stream_waiter.error()); + base::FilePath file_path = file_stream_waiter.deletable_file()->path(); + EXPECT_TRUE(base::PathExists(file_path)); + + // Write some data to the temporary. + int bytes_written = 0; + scoped_refptr<net::IOBufferWithSize> buf = + new net::IOBufferWithSize(kTestDataSize); + memcpy(buf->data(), kTestData, kTestDataSize); + scoped_refptr<net::DrainableIOBuffer> drainable = + new net::DrainableIOBuffer(buf.get(), buf->size()); + while (bytes_written != kTestDataSize) { + net::TestCompletionCallback write_callback; + int rv = file_stream_waiter.file_stream()->Write( + drainable.get(), drainable->BytesRemaining(), + write_callback.callback()); + if (rv == net::ERR_IO_PENDING) + rv = write_callback.WaitForResult(); + ASSERT_LT(0, rv); + drainable->DidConsume(rv); + bytes_written += rv; + } + + // Verify the data matches. + std::string contents; + ASSERT_TRUE(base::ReadFileToString(file_path, &contents)); + EXPECT_EQ(kTestData, contents); + + // Close the file. + net::TestCompletionCallback close_callback; + int rv = file_stream_waiter.file_stream()->Close(close_callback.callback()); + if (rv == net::ERR_IO_PENDING) + rv = close_callback.WaitForResult(); + EXPECT_EQ(net::OK, rv); + + // Release everything. The file should be gone now. + file_stream_waiter.Release(); + base::MessageLoop::current()->RunUntilIdle(); + + // The temporary should be gone now. + EXPECT_FALSE(base::PathExists(file_path)); +} + +} // content |