Fix various issues in RedirectToFileResourceHandler.

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 <set>
 #include <vector>
@@ skipping 22 matching lines @@
 #include "content/browser/loader/async_resource_handler.h"
 #include "content/browser/loader/buffered_resource_handler.h"
 #include "content/browser/loader/cross_site_resource_handler.h"
 #include "content/browser/loader/detachable_resource_handler.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_message_filter.h"
 #include "content/browser/loader/resource_request_info_impl.h"
 #include "content/browser/loader/stream_resource_handler.h"
 #include "content/browser/loader/sync_resource_handler.h"
+#include "content/browser/loader/temporary_file_manager.h"
 #include "content/browser/loader/throttling_resource_handler.h"
 #include "content/browser/loader/upload_data_stream_builder.h"
 #include "content/browser/plugin_service_impl.h"
 #include "content/browser/renderer_host/render_view_host_delegate.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/resource_context_impl.h"
 #include "content/browser/streams/stream.h"
 #include "content/browser/streams/stream_context.h"
 #include "content/browser/streams/stream_registry.h"
 #include "content/browser/worker_host/worker_service_impl.h"
@@ skipping 177 matching lines @@
                        << iter->filesystem_url().spec();
           return false;
         }
       }
     }
   }
 
   return true;
 }
 
-void RemoveDownloadFileFromChildSecurityPolicy(int child_id,
-                                               const base::FilePath& path) {
-  ChildProcessSecurityPolicyImpl::GetInstance()->RevokeAllPermissionsForFile(
-      child_id, path);
-}
-
 #if defined(OS_WIN)
 #pragma warning(disable: 4748)
 #pragma optimize("", off)
 #endif
 
 #if defined(OS_WIN)
 #pragma optimize("", on)
 #pragma warning(default: 4748)
 #endif
 
@@ skipping 534 matching lines @@
     *render_process_id = -1;
     *render_view_id = -1;
     return false;
   }
 
   return info->GetAssociatedRenderView(render_process_id, render_view_id);
 }
 
 void ResourceDispatcherHostImpl::OnInit() {
   scheduler_.reset(new ResourceScheduler);
+  temporary_file_manager_.reset(new TemporaryFileManager);
   appcache::AppCacheInterceptor::EnsureRegistered();
 }
 
 void ResourceDispatcherHostImpl::OnShutdown() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   is_shutdown_ = true;
   pending_loaders_.clear();
 
   // Make sure we shutdown the timer now, otherwise by the time our destructor
@@ skipping 11 matching lines @@
     std::pair<std::set<GlobalRoutingID>::iterator, bool> result =
         ids.insert(iter->first);
     // We should not have duplicates.
     DCHECK(result.second);
   }
   for (std::set<GlobalRoutingID>::const_iterator iter = ids.begin();
        iter != ids.end(); ++iter) {
     CancelBlockedRequestsForRoute(iter->child_id, iter->route_id);
   }
 
+  temporary_file_manager_.reset();
   scheduler_.reset();
 }
 
 bool ResourceDispatcherHostImpl::OnMessageReceived(
     const IPC::Message& message,
     ResourceMessageFilter* filter,
     bool* message_was_ok) {
   filter_ = filter;
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP_EX(ResourceDispatcherHostImpl, message, *message_was_ok)
@@ skipping 306 matching lines @@
     int route_id,
     int process_type,
     int child_id,
     ResourceContext* resource_context) {
   // Construct the IPC resource handler.
   scoped_ptr<ResourceHandler> handler;
   if (sync_result) {
     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,
+                                          temporary_file_manager_.get()));
+  }
+
+  // Prefetches and <a ping> requests outlive their child process.

[darin (slow to review), 2013/12/06 17:56:07]

Is this part of a different CL?

[davidben, 2013/12/19 22:21:01]

I moved it out from the code above where AsyncResourceHandler is created.
Noticed that it results in RedirectToFileResourceHandler not being next in the
chain, so I swapped the order here. (Shouldn't ever happen, but if the renderer
decides to create a download_to_file prefetch for some incomprehensible
reason...) This way DetachableResourceHandler never gets its
OnWillRead/OnReadCompleted preempted in favor of OnDataDownloaded and it
preserves the behavior that RedirectToFileResourceHandler gets destroyed when
the request is cancelled via CancelRequestsForRoute.

+  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
   // let us check whether a transfer is required and pause for the unload
   // handler either if so or if a cross-process navigation is already under way.
  if (request_data.resource_type == ResourceType::MAIN_FRAME &&
      process_type == PROCESS_TYPE_RENDERER) {
     handler.reset(new CrossSiteResourceHandler(handler.Pass(), request));
   }
 
@@ skipping 20 matching lines @@
   throttles.push_back(
       scheduler_->ScheduleRequest(child_id, route_id, request).release());
 
   handler.reset(
       new ThrottlingResourceHandler(handler.Pass(), request, throttles.Pass()));
 
   return handler.Pass();
 }
 
 void ResourceDispatcherHostImpl::OnReleaseDownloadedFile(int request_id) {
-  UnregisterDownloadedTempFile(filter_->child_id(), request_id);
+  if (temporary_file_manager_) {
+    temporary_file_manager_->UnregisterDownloadedTempFile(filter_->child_id(),
+                                                          request_id);
+  } else {
+    // Can the RDH still receive messages after being shut down?
+    NOTREACHED();
+  }
 }
 
 void ResourceDispatcherHostImpl::OnDataDownloadedACK(int request_id) {
   // TODO(michaeln): maybe throttle DataDownloaded messages
 }
 
-void ResourceDispatcherHostImpl::RegisterDownloadedTempFile(
-    int child_id, int request_id, ShareableFileReference* reference) {
-  registered_temp_files_[child_id][request_id] = reference;
-  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
-  // loaded with to keep the file (and permissions) alive.
-  reference->AddFinalReleaseCallback(
-      base::Bind(&RemoveDownloadFileFromChildSecurityPolicy,
-                 child_id));
-}
-
-void ResourceDispatcherHostImpl::UnregisterDownloadedTempFile(
-    int child_id, int request_id) {
-  DeletableFilesMap& map = registered_temp_files_[child_id];
-  DeletableFilesMap::iterator found = map.find(request_id);
-  if (found == map.end())
-    return;
-
-  map.erase(found);
-
-  // Note that we don't remove the security bits here. This will be done
-  // when all file refs are deleted (see RegisterDownloadedTempFile).
-}
-
 bool ResourceDispatcherHostImpl::Send(IPC::Message* message) {
   delete message;
   return false;
 }
 
 void ResourceDispatcherHostImpl::OnUploadProgressACK(int request_id) {
   ResourceLoader* loader = GetLoader(filter_->child_id(), request_id);
   if (loader)
     loader->OnUploadProgressACK();
 }
@@ skipping 114 matching lines @@
     if (info->cross_site_handler())
       info->cross_site_handler()->ResumeResponse();
   }
 }
 
 // The object died, so cancel and detach all requests associated with it except
 // for downloads and detachable resources, which belong to the browser process
 // even if initiated via a renderer.
 void ResourceDispatcherHostImpl::CancelRequestsForProcess(int child_id) {
   CancelRequestsForRoute(child_id, -1 /* cancel all */);
-  registered_temp_files_.erase(child_id);
+  // The RDH may already have been shutdown by the time this is called.
+  if (temporary_file_manager_)
+    temporary_file_manager_->UnregisterFilesForChild(child_id);
 }
 
 void ResourceDispatcherHostImpl::CancelRequestsForRoute(int child_id,
                                                         int route_id) {
   // Since pending_requests_ is a map, we first build up a list of all of the
   // matching requests to be cancelled, and then we cancel them.  Since there
   // may be more than one request to cancel, we cannot simply hold onto the map
   // iterators found in the first loop.
 
   // Find the global ID of all matching elements.
@@ skipping 581 matching lines @@
   if ((load_flags & net::LOAD_REPORT_RAW_HEADERS)
       && !policy->CanReadRawCookies(child_id)) {
     VLOG(1) << "Denied unauthorized request for raw headers";
     load_flags &= ~net::LOAD_REPORT_RAW_HEADERS;
   }
 
   return load_flags;
 }
 
 }  // namespace content