Implement Worker-MainThread bridge for FileSystem API in Chrome

content/child/fileapi/file_system_dispatcher.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.
 
 #include "content/child/fileapi/file_system_dispatcher.h"
 
 #include "base/callback.h"
 #include "base/file_util.h"
+#include "base/message_loop/message_loop_proxy.h"
 #include "base/process.h"
 #include "content/child/child_thread.h"
 #include "content/common/fileapi/file_system_messages.h"
 
 namespace content {
 
+namespace {
+
+void DidReceiveSnapshotFile(
+    int request_id,
+    base::MessageLoopProxy* message_loop) {

[michaeln, 2013/07/24 00:54:08]

You might be able to use the ThreadSafeSender here instead of manually
loop->PostTask'ing in this function.

[kinuko, 2013/07/24 09:29:09]

Done.

+  if (!message_loop->RunsTasksOnCurrentThread()) {
+    message_loop->PostTask(FROM_HERE,
+                           base::Bind(&DidReceiveSnapshotFile, request_id,
+                                      make_scoped_refptr(message_loop)));
+    return;
+  }
+  if (ChildThread::current()) {
+    ChildThread::current()->Send(
+        new FileSystemHostMsg_DidReceiveSnapshotFile(request_id));
+  }
+}
+
+}  // namespace
+
 class FileSystemDispatcher::CallbackDispatcher {
  public:
   typedef CallbackDispatcher self;
   typedef FileSystemDispatcher::StatusCallback StatusCallback;
   typedef FileSystemDispatcher::MetadataCallback MetadataCallback;
   typedef FileSystemDispatcher::ReadDirectoryCallback ReadDirectoryCallback;
   typedef FileSystemDispatcher::OpenFileSystemCallback OpenFileSystemCallback;
   typedef FileSystemDispatcher::WriteCallback WriteCallback;
   typedef FileSystemDispatcher::OpenFileCallback OpenFileCallback;
 
@@ skipping 56 matching lines @@
     error_callback_.Run(error_code);
   }
 
   void DidReadMetadata(
       const base::PlatformFileInfo& file_info) {
     metadata_callback_.Run(file_info);
   }
 
   void DidCreateSnapshotFile(
       const base::PlatformFileInfo& file_info,
-      const base::FilePath& platform_path) {
-    snapshot_callback_.Run(file_info, platform_path);
+      const base::FilePath& platform_path,
+      const base::Closure& did_receive_snapshot) {
+    snapshot_callback_.Run(file_info, platform_path, did_receive_snapshot);

[michaeln, 2013/07/24 01:38:58]

What method/function is snapshot_callback_ this wired to? I see,
CallbacksWrapper::DidCreateSnapshotFile.

Alternatively, instead of passing a did_receive_snapshot Closure to be invoked
by that method, maybe alter that method to directly call...
  sender->Send(new FileSystemHostMsg_DidReceiveSnapshotFile(request_id));
...i guess you'd also have to pass in the request_id to that method in place of
this closure.

The closure indirection is harder to follow fwiw.

[kinuko, 2013/07/24 09:29:09]

I thought returning a closure is clearer about what is expected, but it might be
also true that adding more indirection makes things harder to follow... ok,
done.

   }
 
   void DidReadDirectory(
       const std::vector<fileapi::DirectoryEntry>& entries,
       bool has_more) {
     directory_callback_.Run(entries, has_more);
   }
 
   void DidOpenFileSystem(const std::string& name,
                          const GURL& root) {
@@ skipping 29 matching lines @@
 FileSystemDispatcher::FileSystemDispatcher() {
 }
 
 FileSystemDispatcher::~FileSystemDispatcher() {
   // Make sure we fire all the remaining callbacks.
   for (IDMap<CallbackDispatcher, IDMapOwnPointer>::iterator
            iter(&dispatchers_); !iter.IsAtEnd(); iter.Advance()) {
     int request_id = iter.GetCurrentKey();
     CallbackDispatcher* dispatcher = iter.GetCurrentValue();
     DCHECK(dispatcher);
     dispatcher->DidFail(base::PLATFORM_FILE_ERROR_ABORT);

[michaeln, 2013/07/24 00:54:08]

O/T for this CL, but similarly we're shutting down if we're destructing (right),
so I wonder about not attempting to propagate DidFail() back to script at this
point.

[kinuko, 2013/07/24 09:29:09]

Current API contract expects it to be called in the end, so let me keep this as
is for now.

     dispatchers_.Remove(request_id);
   }
 }
 
 bool FileSystemDispatcher::OnMessageReceived(const IPC::Message& msg) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(FileSystemDispatcher, msg)
     IPC_MESSAGE_HANDLER(FileSystemMsg_DidOpenFileSystem, OnDidOpenFileSystem)
     IPC_MESSAGE_HANDLER(FileSystemMsg_DidSucceed, OnDidSucceed)
     IPC_MESSAGE_HANDLER(FileSystemMsg_DidReadDirectory, OnDidReadDirectory)
@@ skipping 203 matching lines @@
   DCHECK(dispatcher);
   dispatcher->DidReadMetadata(file_info);
   dispatchers_.Remove(request_id);
 }
 
 void FileSystemDispatcher::OnDidCreateSnapshotFile(
     int request_id, const base::PlatformFileInfo& file_info,
     const base::FilePath& platform_path) {
   CallbackDispatcher* dispatcher = dispatchers_.Lookup(request_id);
   DCHECK(dispatcher);
-  dispatcher->DidCreateSnapshotFile(file_info, platform_path);
+  dispatcher->DidCreateSnapshotFile(
+      file_info, platform_path,
+      base::Bind(&DidReceiveSnapshotFile, request_id,
+                 base::MessageLoopProxy::current()));
   dispatchers_.Remove(request_id);
-  ChildThread::current()->Send(
-      new FileSystemHostMsg_DidReceiveSnapshotFile(request_id));
 }
 
 void FileSystemDispatcher::OnDidReadDirectory(
     int request_id,
     const std::vector<fileapi::DirectoryEntry>& entries,
     bool has_more) {
   CallbackDispatcher* dispatcher = dispatchers_.Lookup(request_id);
   DCHECK(dispatcher);
   dispatcher->DidReadDirectory(entries, has_more);
   dispatchers_.Remove(request_id);
@@ skipping 23 matching lines @@
     quota::QuotaLimitType quota_policy) {
   CallbackDispatcher* dispatcher = dispatchers_.Lookup(request_id);
   DCHECK(dispatcher);
   dispatcher->DidOpenFile(IPC::PlatformFileForTransitToPlatformFile(file),
                           file_open_id,
                           quota_policy);
   dispatchers_.Remove(request_id);
 }
 
 }  // namespace content