Fix and refactor downloaded file handling in the loading stack

content/browser/loader/resource_dispatcher_host_unittest.cc

Index: content/browser/loader/resource_dispatcher_host_unittest.cc
diff --git a/content/browser/loader/resource_dispatcher_host_unittest.cc b/content/browser/loader/resource_dispatcher_host_unittest.cc
index 8dfae97d5e54ab6ba82dc27c46538fdafe87f82c..650b71c3dea69ef51f46ae9de23d9e7c711b5eca 100644
--- a/content/browser/loader/resource_dispatcher_host_unittest.cc
+++ b/content/browser/loader/resource_dispatcher_host_unittest.cc
@@ -28,6 +28,7 @@
 #include "content/browser/download/download_resource_handler.h"
 #include "content/browser/frame_host/navigation_request_info.h"
 #include "content/browser/loader/detachable_resource_handler.h"
+#include "content/browser/loader/downloaded_temp_file_impl.h"
 #include "content/browser/loader/navigation_resource_throttle.h"
 #include "content/browser/loader/navigation_url_loader.h"
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
@@ -3393,7 +3394,56 @@ TEST_P(ResourceDispatcherHostTest, RegisterDownloadedTempFile) {
   // 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;
+  deletable_file = nullptr;
+  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 the dispatcher host's temporary file management in the mojo-enabled
+// loading.
+TEST_P(ResourceDispatcherHostTest, RegisterDownloadedTempFileWithMojo) {
+  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::GetTaskRunnerForThread(BrowserThread::FILE).get());
+
+  // Not readable.
+  EXPECT_FALSE(ChildProcessSecurityPolicyImpl::GetInstance()->CanReadFile(
+      filter_->child_id(), file_path));
+
+  // Register it for a resource request.
+  auto downloaded_file =
+      DownloadedTempFileImpl::Create(filter_->child_id(), kRequestID);
+  mojom::DownloadedTempFilePtr downloaded_file_ptr =
+      DownloadedTempFileImpl::Create(filter_->child_id(), kRequestID);
+  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.
+  downloaded_file_ptr = nullptr;
+  base::RunLoop().RunUntilIdle();
+
+  // Still readable because there is another reference to the file. (The child
+  // may take additional blob references.)

[mmenke, 2016/11/22 16:19:34]

I don't see an API for a child process to create additional
DownloadedTempFilePtrs for the same file?

[tzik, 2016/11/24 14:06:40]

"another reference to the file" means a ShareableFileReference rather than a
DownloadedTempFilePtr.

That's a bit far from these code, but the renderer may take a
ShareableFileReference by creating a blob from the responding file path.
E.g. XMLHttpRequest::createBlobDataHandleFromResponse() makes a blob from the
file path, and that eventually calls BlobDataBuilder::AppendFile(), that holds a
ShareableFileReference in a BlobDataItem.

+  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 = nullptr;
   base::RunLoop().RunUntilIdle();
 
   // The file is no longer readable to the child and has been deleted.