Fix and refactor downloaded file handling in the loading stack

content/browser/loader/resource_dispatcher_host_impl.cc

 // Copyright (c) 2012 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.
 
 // See http://dev.chromium.org/developers/design-documents/multi-process-resource-loading
 
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
 
 #include <stddef.h>
 
@@ skipping 27 matching lines @@
 #include "content/browser/bad_message.h"
 #include "content/browser/blob_storage/chrome_blob_storage_context.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/frame_host/frame_tree.h"
 #include "content/browser/frame_host/navigation_handle_impl.h"
 #include "content/browser/frame_host/navigation_request_info.h"
 #include "content/browser/frame_host/navigator.h"
 #include "content/browser/loader/async_resource_handler.h"
 #include "content/browser/loader/async_revalidation_manager.h"
 #include "content/browser/loader/detachable_resource_handler.h"
+#include "content/browser/loader/downloaded_temp_file_impl.h"
 #include "content/browser/loader/intercepting_resource_handler.h"
 #include "content/browser/loader/loader_delegate.h"
 #include "content/browser/loader/mime_sniffing_resource_handler.h"
 #include "content/browser/loader/mojo_async_resource_handler.h"
 #include "content/browser/loader/navigation_resource_handler.h"
 #include "content/browser/loader/navigation_resource_throttle.h"
 #include "content/browser/loader/navigation_url_loader_impl_core.h"
 #include "content/browser/loader/power_save_block_resource_throttle.h"
 #include "content/browser/loader/redirect_to_file_resource_handler.h"
 #include "content/browser/loader/resource_loader.h"
@@ skipping 1704 matching lines @@
   // The request may go away before processing this message, so |loader| can
   // legitimately be null.
   if (!loader)
     return;
 
   scheduler_->ReprioritizeRequest(loader->request(), new_priority,
                                   intra_priority_value);
 }
 
 void ResourceDispatcherHostImpl::RegisterDownloadedTempFile(
-    int child_id, int request_id, const base::FilePath& file_path) {
-  scoped_refptr<ShareableFileReference> reference =
-      ShareableFileReference::Get(file_path);
-  DCHECK(reference.get());
+    std::unique_ptr<DownloadedTempFileImpl> downloaded_file) {

[mmenke, 2016/11/17 16:59:33]

Do we even need to change all this?  Would it be better to just make
DownloadedTempFileImpl call
ChildProcessSecurityPolicyImpl::GetInstance()->GrantReadFile itself, and rely on
the connection_error_handler to ensure teardown (Or just let the Mojo object in
the other process own it directly), and leave the old code alone?  Then when we
switch over to mojo, we can remove all this stuff from the RDH as well.

[tzik, 2016/11/22 13:54:45]

OK, let me take the second: Let the Mojo object in the other process own it.

+  int child_id = downloaded_file->child_id();
+  int request_id = downloaded_file->request_id();
+  scoped_refptr<storage::ShareableFileReference> reference =

[dcheng, 2016/11/17 11:16:02]

Are lines 1774-1776 still needed?

[tzik, 2016/11/22 13:54:45]

Reverted.

+      downloaded_file->reference();
 
-  registered_temp_files_[child_id][request_id] = reference;
+  registered_temp_files_[child_id][request_id] = std::move(downloaded_file);
   ChildProcessSecurityPolicyImpl::GetInstance()->GrantReadFile(
       child_id, reference->path());
 
   // When the temp file is deleted, revoke permissions that the renderer has
   // to that file. This covers an edge case where the file is deleted and then
   // the same name is re-used for some other purpose, we don't want the old
   // renderer to still have access to it.
   //
   // We do this when the file is deleted because the renderer can take a blob
   // reference to the temp file that outlives the url loaded that it was
@@ skipping 1011 matching lines @@
                                  &throttles);
     if (!throttles.empty()) {
       handler.reset(new ThrottlingResourceHandler(std::move(handler), request,
                                                   std::move(throttles)));
     }
   }
   return handler;
 }
 
 }  // namespace content