Fix various issues in RedirectToFileResourceHandler.

content/browser/loader/temporary_file_manager.cc

Index: content/browser/loader/temporary_file_manager.cc
diff --git a/content/browser/loader/temporary_file_manager.cc b/content/browser/loader/temporary_file_manager.cc
new file mode 100644
index 0000000000000000000000000000000000000000..a745a5bf65830cb167dc52f86e20aa0f2ac15c29
--- /dev/null
+++ b/content/browser/loader/temporary_file_manager.cc
@@ -0,0 +1,133 @@
+// Copyright 2013 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_manager.h"
+
+#include "base/files/file_util_proxy.h"
+#include "content/browser/child_process_security_policy_impl.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 RemoveDownloadFileFromChildSecurityPolicy(int child_id,
+                                               const base::FilePath& path) {
+  ChildProcessSecurityPolicyImpl::GetInstance()->RevokeAllPermissionsForFile(
+      child_id, path);
+}
+
+}  // namespace
+
+struct TemporaryFileManager::PendingTemporary {

[mmenke, 2013/12/05 21:34:03]

optional:  Do we even need this class?  Seems like you can just pass them all
directly now.  Doesn't really save us a whole lot, of course.

[davidben, 2013/12/05 22:43:20]

Done. I guess DidCreateTemporaryFile is now kinda huge, but oh well.

+  PendingTemporary(
+      int child_id,
+      int request_id,
+      base::WeakPtr<RedirectToFileResourceHandler> handler)
+      : child_id(child_id),
+        request_id(request_id),
+        handler(handler) {
+  }
+
+  int child_id;
+  int request_id;
+  base::WeakPtr<RedirectToFileResourceHandler> handler;
+};
+
+TemporaryFileManager::TemporaryFileManager() : weak_factory_(this) {
+}
+
+TemporaryFileManager::~TemporaryFileManager() {
+}
+
+void TemporaryFileManager::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()) {
+    map.erase(found);
+  }

[mmenke, 2013/12/05 21:34:03]

Can't all of this just be replaced with:

registered_temp_files_[child_id].erase(request_id)?

[davidben, 2013/12/05 22:43:20]

Hrm, yeah, it can... this just came from the original version. Done.

+
+  // Note that we don't remove the security bits here. This will be done
+  // when all file refs are deleted (see RegisterDownloadedTempFile).
+}
+
+void TemporaryFileManager::UnregisterFilesForChild(int child_id) {
+  registered_temp_files_.erase(child_id);
+}
+
+void TemporaryFileManager::CreateTemporary(
+    int child_id,
+    int request_id,
+    base::WeakPtr<RedirectToFileResourceHandler> handler) {
+  PendingTemporary* pending =
+      new PendingTemporary(child_id, request_id, handler);
+
+  base::FileUtilProxy::CreateTemporary(
+      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE).get(),
+      base::PLATFORM_FILE_ASYNC,
+      base::Bind(&TemporaryFileManager::DidCreateTemporaryFile, AsWeakPtr(),
+                 base::Owned(pending)));
+}
+
+void TemporaryFileManager::DidCreateTemporaryFile(
+    TemporaryFileManager::PendingTemporary* pending,
+    base::PlatformFileError error_code,
+    base::PassPlatformFile file_handle,
+    const base::FilePath& file_path) {
+  if (error_code != base::PLATFORM_FILE_OK) {
+    if (pending->handler) {
+      pending->handler->DidCreateTemporaryFile(
+          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());
+
+  // Check if the handler has since been destroyed. If so, skip the rest.
+  if (!pending->handler)
+    return;
+
+  scoped_ptr<net::FileStream> file_stream(new net::FileStream(
+      file_handle.ReleaseValue(),
+      base::PLATFORM_FILE_WRITE | base::PLATFORM_FILE_ASYNC,
+      NULL));
+
+  RegisterDownloadedTempFile(pending->child_id, pending->request_id,
+                             deletable_file.get());
+  pending->handler->DidCreateTemporaryFile(
+      error_code, file_stream.Pass(), deletable_file);
+}
+
+void TemporaryFileManager::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.

[mmenke, 2013/12/05 21:34:03]

nit:  Don't use "we" in comments.

[davidben, 2013/12/05 22:43:20]

Also copied from the old file, but done. :-P

+  reference->AddFinalReleaseCallback(
+      base::Bind(&RemoveDownloadFileFromChildSecurityPolicy,
+                 child_id));
+}
+
+}  // namespace content