Create a new FileSystemOperationRunner for each FileAPIMessageFilter

BUG=262918
TEST=FileAPIMessageFilter.{CloseChannelWithInflightRequest,MultipleFilters}

Review URL: https://chromiumcodereview.appspot.com/19705017

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@213374 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 83 insertions, 6 deletions

diff --git a/content/browser/fileapi/fileapi_message_filter.cc b/content/browser/fileapi/fileapi_message_filter.cc
index 312e04d..1fbb338 100644
--- a/content/browser/fileapi/fileapi_message_filter.cc
+++ b/content/browser/fileapi/fileapi_message_filter.cc
@@ -90,6 +90,7 @@ FileAPIMessageFilter::FileAPIMessageFilter(
 }
 
 void FileAPIMessageFilter::OnChannelConnected(int32 peer_pid) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   BrowserMessageFilter::OnChannelConnected(peer_pid);
 
   if (request_context_getter_.get()) {
@@ -98,9 +99,12 @@ void FileAPIMessageFilter::OnChannelConnected(int32 peer_pid) {
     request_context_getter_ = NULL;
     DCHECK(request_context_);
   }
+
+  operation_runner_ = context_->CreateFileSystemOperationRunner();
 }
 
 void FileAPIMessageFilter::OnChannelClosing() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   BrowserMessageFilter::OnChannelClosing();
 
   // Unregister all the blob URLs that are previously registered in this
@@ -128,7 +132,7 @@ void FileAPIMessageFilter::OnChannelClosing() {
   }
 
   on_close_callbacks_.Clear();
-  operation_runner()->Shutdown();
+  operation_runner_.reset();
   operations_.clear();
 }
 
@@ -738,8 +742,4 @@ bool FileAPIMessageFilter::HasPermissionsForFile(
                                               permissions, error);
 }
 
-fileapi::FileSystemOperationRunner* FileAPIMessageFilter::operation_runner() {
-  return context_->operation_runner();
-}
-
 }  // namespace content
diff --git a/content/browser/fileapi/fileapi_message_filter.h b/content/browser/fileapi/fileapi_message_filter.h
index 9d7f8bb..ed3b8d8 100644
--- a/content/browser/fileapi/fileapi_message_filter.h
+++ b/content/browser/fileapi/fileapi_message_filter.h
@@ -32,6 +32,7 @@ class Time;
 namespace fileapi {
 class FileSystemURL;
 class FileSystemContext;
+class FileSystemOperationRunner;
 struct DirectoryEntry;
 }
 
@@ -169,7 +170,9 @@ class CONTENT_EXPORT FileAPIMessageFilter : public BrowserMessageFilter {
                              int permissions,
                              base::PlatformFileError* error);
 
-  fileapi::FileSystemOperationRunner* operation_runner();
+  fileapi::FileSystemOperationRunner* operation_runner() {
+    return operation_runner_.get();
+  }
 
   int process_id_;
 
@@ -187,6 +190,8 @@ class CONTENT_EXPORT FileAPIMessageFilter : public BrowserMessageFilter {
 
   scoped_refptr<ChromeBlobStorageContext> blob_storage_context_;
 
+  scoped_ptr<fileapi::FileSystemOperationRunner> operation_runner_;
+
   // Keep track of blob URLs registered in this process. Need to unregister
   // all of them when the renderer process dies.
   base::hash_set<std::string> blob_urls_;
diff --git a/content/browser/fileapi/fileapi_message_filter_unittest.cc b/content/browser/fileapi/fileapi_message_filter_unittest.cc
index 4266941..a290454 100644
--- a/content/browser/fileapi/fileapi_message_filter_unittest.cc
+++ b/content/browser/fileapi/fileapi_message_filter_unittest.cc
@@ -9,7 +9,9 @@
 
 #include "base/memory/ref_counted.h"
 #include "base/message_loop/message_loop.h"
+#include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/fileapi/chrome_blob_storage_context.h"
+#include "content/common/fileapi/file_system_messages.h"
 #include "content/common/fileapi/webblob_messages.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/common_param_traits.h"
@@ -34,6 +36,15 @@ class FileAPIMessageFilterTest : public testing::Test {
   virtual void SetUp() OVERRIDE {
     file_system_context_ =
         fileapi::CreateFileSystemContextForTesting(NULL, base::FilePath());
+
+    std::vector<fileapi::FileSystemType> types;
+    file_system_context_->GetFileSystemTypes(&types);
+    for (size_t i = 0; i < types.size(); ++i) {
+      ChildProcessSecurityPolicyImpl::GetInstance()->
+          RegisterFileSystemPermissionPolicy(
+              types[i],
+              fileapi::FileSystemContext::GetPermissionPolicy(types[i]));
+    }
   }
 
   base::MessageLoop message_loop_;
@@ -88,4 +99,65 @@ TEST_F(FileAPIMessageFilterTest, BuildEmptyBlob) {
   EXPECT_TRUE(controller->GetBlobDataFromUrl(kDifferentUrl) == NULL);
 }
 
+TEST_F(FileAPIMessageFilterTest, CloseChannelWithInflightRequest) {
+  scoped_refptr<FileAPIMessageFilter> filter(
+      new FileAPIMessageFilter(
+          0 /* process_id */,
+          browser_context_.GetRequestContext(),
+          file_system_context_.get(),
+          ChromeBlobStorageContext::GetFor(&browser_context_)));
+  filter->OnChannelConnected(0);
+
+  // Complete initialization.
+  message_loop_.RunUntilIdle();
+
+  IPC::ChannelProxy::MessageFilter* casted_filter =
+      static_cast<IPC::ChannelProxy::MessageFilter*>(filter.get());
+
+  int request_id = 0;
+  const GURL kUrl("filesystem:http://example.com/temporary/foo");
+  FileSystemHostMsg_ReadMetadata read_metadata(request_id++, kUrl);
+  EXPECT_TRUE(casted_filter->OnMessageReceived(read_metadata));
+
+  // Close the filter while it has inflight request.
+  filter->OnChannelClosing();
+
+  // This shouldn't cause DCHECK failure.
+  message_loop_.RunUntilIdle();
+}
+
+TEST_F(FileAPIMessageFilterTest, MultipleFilters) {
+  scoped_refptr<FileAPIMessageFilter> filter1(
+      new FileAPIMessageFilter(
+          0 /* process_id */,
+          browser_context_.GetRequestContext(),
+          file_system_context_.get(),
+          ChromeBlobStorageContext::GetFor(&browser_context_)));
+  scoped_refptr<FileAPIMessageFilter> filter2(
+      new FileAPIMessageFilter(
+          1 /* process_id */,
+          browser_context_.GetRequestContext(),
+          file_system_context_.get(),
+          ChromeBlobStorageContext::GetFor(&browser_context_)));
+  filter1->OnChannelConnected(0);
+  filter2->OnChannelConnected(1);
+
+  // Complete initialization.
+  message_loop_.RunUntilIdle();
+
+  IPC::ChannelProxy::MessageFilter* casted_filter =
+      static_cast<IPC::ChannelProxy::MessageFilter*>(filter1.get());
+
+  int request_id = 0;
+  const GURL kUrl("filesystem:http://example.com/temporary/foo");
+  FileSystemHostMsg_ReadMetadata read_metadata(request_id++, kUrl);
+  EXPECT_TRUE(casted_filter->OnMessageReceived(read_metadata));
+
+  // Close the other filter before the request for filter1 is processed.
+  filter2->OnChannelClosing();
+
+  // This shouldn't cause DCHECK failure.
+  message_loop_.RunUntilIdle();
+}
+
 }  // namespace fileapi