Support synchronous mode for FileSystem/FileWriter API

content/child/fileapi/webfilesystem_impl.cc

Index: content/child/fileapi/webfilesystem_impl.cc
diff --git a/content/child/fileapi/webfilesystem_impl.cc b/content/child/fileapi/webfilesystem_impl.cc
index 090bf7d79740e3aed851a4d40ce564b622b45aea..552a8c5c8ee6be5bd7f6011cb0d4b3620f716b2b 100644
--- a/content/child/fileapi/webfilesystem_impl.cc
+++ b/content/child/fileapi/webfilesystem_impl.cc
@@ -9,6 +9,7 @@
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop_proxy.h"
+#include "base/synchronization/waitable_event.h"
 #include "base/threading/thread_local.h"
 #include "content/child/child_thread.h"
 #include "content/child/fileapi/file_system_dispatcher.h"
@@ -94,6 +95,35 @@ class CallbacksMap : public WorkerTaskRunner::Observer {
   DISALLOW_COPY_AND_ASSIGN(CallbacksMap);
 };
 
+class WaitableCallbackResults {
+ public:
+  static WaitableCallbackResults* MaybeCreate(
+      WebKit::WebFileSystemCallbacks* callbacks) {
+    if (callbacks->shouldBlockUntilCompletion())

[michaeln, 2013/07/31 23:34:03]

Looks like this all assumes that a call that originates on the main thread will
never block until completion (there is no logic to delete 'this' in that case).
Is that right?

Maybe add a DCHECK here for that?

[kinuko, 2013/08/01 06:12:26]

Done.

+      return new WaitableCallbackResults;
+    return NULL;
+  }
+  ~WaitableCallbackResults() {}
+
+  void SetResultsAndSignal(const base::Closure& results_closure) {
+    results_closure_ = results_closure;
+    event_->Signal();
+  }
+
+  void WaitAndRun() {
+    event_->Wait();
+    DCHECK(!results_closure_.is_null());
+    results_closure_.Run();
+  }
+
+ private:
+  WaitableCallbackResults() : event_(new base::WaitableEvent(false, false)) {}

[michaeln, 2013/07/31 23:34:03]

would manual reset make more sense since this is a one time event?

[kinuko, 2013/08/01 06:12:26]

Done.

+
+  base::WaitableEvent* event_;
+  base::Closure results_closure_;
+  DISALLOW_COPY_AND_ASSIGN(WaitableCallbackResults);
+};
+
 void DidReceiveSnapshotFile(int request_id) {
   if (ChildThread::current())
     ChildThread::current()->Send(
@@ -107,12 +137,17 @@ int CurrentWorkerId() {
 template <typename Method, typename Params>
 void CallDispatcherOnMainThread(
     base::MessageLoopProxy* loop,
-    Method method, const Params& params) {
+    Method method, const Params& params,
+    WaitableCallbackResults* waitable_results) {
   if (!loop->RunsTasksOnCurrentThread()) {
     loop->PostTask(FROM_HERE,
                    base::Bind(&CallDispatcherOnMainThread<Method, Params>,
-                              make_scoped_refptr(loop), method, params));
-    return;
+                              make_scoped_refptr(loop), method, params,
+                              waitable_results));

[michaeln, 2013/07/31 23:34:03]

maybe bind NULL here for waitable_results since it's not used on the other code
path immediately below

[kinuko, 2013/08/01 06:12:26]

Done. Also added DCHECK(!waitable_results) in the main-thread code path.

+    if (!waitable_results)
+      return;
+    waitable_results->WaitAndRun();
+    delete waitable_results;
   }
   if (!ChildThread::current() ||
       !ChildThread::current()->file_system_dispatcher())
@@ -123,51 +158,67 @@ void CallDispatcherOnMainThread(
 }
 
 template <typename Method, typename Params>
-void CallbackFileSystemCallbacks(
-    int thread_id, int callbacks_id,
-    Method method, const Params& params) {
-  if (thread_id != CurrentWorkerId()) {
-    WorkerTaskRunner::Instance()->PostTask(
-        thread_id,
-        base::Bind(&CallbackFileSystemCallbacks<Method, Params>,
-                   thread_id, callbacks_id, method, params));
-    return;
-  }
+void RunCallbacks(int callbacks_id, Method method, const Params& params) {
   if (!CallbacksMap::Get())
     return;
-
   WebFileSystemCallbacks* callbacks =
       CallbacksMap::Get()->GetAndUnregisterCallbacks(callbacks_id);
   DCHECK(callbacks);
   DispatchToMethod(callbacks, method, params);
 }
 
+void DispatchResultsClosure(int thread_id, int callbacks_id,
+                            WaitableCallbackResults* waitable_results,
+                            const base::Closure& results_closure) {
+  if (thread_id != CurrentWorkerId()) {
+    if (waitable_results) {
+      waitable_results->SetResultsAndSignal(results_closure);
+      return;
+    }
+    WorkerTaskRunner::Instance()->PostTask(thread_id, results_closure);
+    return;
+  }
+  results_closure.Run();
+}
+
+template <typename Method, typename Params>
+void CallbackFileSystemCallbacks(
+    int thread_id, int callbacks_id,
+    WaitableCallbackResults* waitable_results,
+    Method method, const Params& params) {
+  DispatchResultsClosure(
+      thread_id, callbacks_id, waitable_results,
+      base::Bind(&RunCallbacks<Method, Params>, callbacks_id, method, params));
+}
+
 void StatusCallbackAdapter(int thread_id, int callbacks_id,
+                           WaitableCallbackResults* waitable_results,
                            base::PlatformFileError error) {
   if (error == base::PLATFORM_FILE_OK) {
     CallbackFileSystemCallbacks(
-        thread_id, callbacks_id,
+        thread_id, callbacks_id, waitable_results,
         &WebFileSystemCallbacks::didSucceed, MakeTuple());
   } else {
     CallbackFileSystemCallbacks(
-        thread_id, callbacks_id,
+        thread_id, callbacks_id, waitable_results,
         &WebFileSystemCallbacks::didFail,
         MakeTuple(fileapi::PlatformFileErrorToWebFileError(error)));
   }
 }
 
 void ReadMetadataCallbackAdapter(int thread_id, int callbacks_id,
+                                 WaitableCallbackResults* waitable_results,
                                  const base::PlatformFileInfo& file_info) {
   WebFileInfo web_file_info;
   webkit_glue::PlatformFileInfoToWebFileInfo(file_info, &web_file_info);
   CallbackFileSystemCallbacks(
-      thread_id, callbacks_id,
+      thread_id, callbacks_id, waitable_results,
       &WebFileSystemCallbacks::didReadMetadata,
       MakeTuple(web_file_info));
 }
 
 void ReadDirectoryCallbackAdapater(
-    int thread_id, int callbacks_id,
+    int thread_id, int callbacks_id, WaitableCallbackResults* waitable_results,
     const std::vector<fileapi::DirectoryEntry>& entries,
     bool has_more) {
   WebVector<WebFileSystemEntry> file_system_entries(entries.size());
@@ -177,27 +228,17 @@ void ReadDirectoryCallbackAdapater(
     file_system_entries[i].isDirectory = entries[i].is_directory;
   }
   CallbackFileSystemCallbacks(
-      thread_id, callbacks_id,
+      thread_id, callbacks_id, waitable_results,
       &WebFileSystemCallbacks::didReadDirectory,
       MakeTuple(file_system_entries, has_more));
 }
 
-void CreateFileWriterCallbackAdapter(
-    int thread_id, int callbacks_id,
-    base::MessageLoopProxy* main_thread_loop,
+void DidCreateFileWriter(
+    int callbacks_id,
     const GURL& path,
     WebKit::WebFileWriterClient* client,
+    base::MessageLoopProxy* main_thread_loop,
     const base::PlatformFileInfo& file_info) {
-  if (thread_id != CurrentWorkerId()) {
-    WorkerTaskRunner::Instance()->PostTask(
-        thread_id,
-        base::Bind(&CreateFileWriterCallbackAdapter,
-                    thread_id, callbacks_id,
-                    make_scoped_refptr(main_thread_loop),
-                    path, client, file_info));
-    return;
-  }
-
   if (!CallbacksMap::Get())
     return;
 
@@ -213,22 +254,25 @@ void CreateFileWriterCallbackAdapter(
       new WebFileWriterImpl(path, client, main_thread_loop), file_info.size);
 }
 
-void CreateSnapshotFileCallbackAdapter(
+void CreateFileWriterCallbackAdapter(
     int thread_id, int callbacks_id,
+    WaitableCallbackResults* waitable_results,
+    base::MessageLoopProxy* main_thread_loop,
+    const GURL& path,
+    WebKit::WebFileWriterClient* client,
+    const base::PlatformFileInfo& file_info) {
+  DispatchResultsClosure(
+      thread_id, callbacks_id, waitable_results,
+      base::Bind(&DidCreateFileWriter, callbacks_id, path, client,
+                 make_scoped_refptr(main_thread_loop), file_info));
+}
+
+void DidCreateSnapshotFile(
+    int callbacks_id,
     base::MessageLoopProxy* main_thread_loop,
     const base::PlatformFileInfo& file_info,
     const base::FilePath& platform_path,
     int request_id) {
-  if (thread_id != CurrentWorkerId()) {
-    WorkerTaskRunner::Instance()->PostTask(
-        thread_id,
-        base::Bind(&CreateSnapshotFileCallbackAdapter,
-                    thread_id, callbacks_id,
-                    make_scoped_refptr(main_thread_loop),
-                    file_info, platform_path, request_id));
-    return;
-  }
-
   if (!CallbacksMap::Get())
     return;
 
@@ -247,6 +291,20 @@ void CreateSnapshotFileCallbackAdapter(
       FROM_HERE, base::Bind(&DidReceiveSnapshotFile, request_id));
 }
 
+void CreateSnapshotFileCallbackAdapter(
+    int thread_id, int callbacks_id,
+    WaitableCallbackResults* waitable_results,
+    base::MessageLoopProxy* main_thread_loop,
+    const base::PlatformFileInfo& file_info,
+    const base::FilePath& platform_path,
+    int request_id) {
+  DispatchResultsClosure(
+      thread_id, callbacks_id, waitable_results,
+      base::Bind(&DidCreateSnapshotFile, callbacks_id,
+                 make_scoped_refptr(main_thread_loop),
+                 file_info, platform_path, request_id));
+}
+
 }  // namespace
 
 WebFileSystemImpl::~WebFileSystemImpl() {
@@ -261,12 +319,15 @@ void WebFileSystemImpl::move(
     const WebKit::WebURL& dest_path,
     WebKit::WebFileSystemCallbacks* callbacks) {
   int callbacks_id = CallbacksMap::GetOrCreate()->RegisterCallbacks(callbacks);
+  WaitableCallbackResults* waitable_results =
+      WaitableCallbackResults::MaybeCreate(callbacks);
   CallDispatcherOnMainThread(
       main_thread_loop_.get(),
       &FileSystemDispatcher::Move,
       MakeTuple(GURL(src_path), GURL(dest_path),
                 base::Bind(&StatusCallbackAdapter,
-                           CurrentWorkerId(), callbacks_id)));
+                           CurrentWorkerId(), callbacks_id, waitable_results)),

[michaeln, 2013/07/31 23:51:27]

Using base::Unretained(waitable_results) for the callback closure here, and
using scoped_ptr.Pass() for the last param to CallDispatcherOnMainThread (just
below)... might make ownership more clear.

[kinuko, 2013/08/01 06:12:26]

Partially done.

Pass() clears up before we pass the value to bind, so we need another ptr
variable... instead changed code to pass waitable_results with make_scoped_ptr
to CallDispatcherOnMainThread.  Also added base::Unretained to make it clearer.

+      waitable_results);
 }
 
 void WebFileSystemImpl::copy(
@@ -274,50 +335,62 @@ void WebFileSystemImpl::copy(
     const WebKit::WebURL& dest_path,
     WebKit::WebFileSystemCallbacks* callbacks) {
   int callbacks_id = CallbacksMap::GetOrCreate()->RegisterCallbacks(callbacks);
+  WaitableCallbackResults* waitable_results =
+      WaitableCallbackResults::MaybeCreate(callbacks);
   CallDispatcherOnMainThread(
       main_thread_loop_.get(),
       &FileSystemDispatcher::Copy,
       MakeTuple(GURL(src_path), GURL(dest_path),
                 base::Bind(&StatusCallbackAdapter,
-                           CurrentWorkerId(), callbacks_id)));
+                           CurrentWorkerId(), callbacks_id, waitable_results)),
+      waitable_results);
 }
 
 void WebFileSystemImpl::remove(
     const WebKit::WebURL& path,
     WebKit::WebFileSystemCallbacks* callbacks) {
   int callbacks_id = CallbacksMap::GetOrCreate()->RegisterCallbacks(callbacks);
+  WaitableCallbackResults* waitable_results =
+      WaitableCallbackResults::MaybeCreate(callbacks);
   CallDispatcherOnMainThread(
       main_thread_loop_.get(),
       &FileSystemDispatcher::Remove,
       MakeTuple(GURL(path), false /* recursive */,
                 base::Bind(&StatusCallbackAdapter,
-                           CurrentWorkerId(), callbacks_id)));
+                           CurrentWorkerId(), callbacks_id, waitable_results)),
+      waitable_results);
 }
 
 void WebFileSystemImpl::removeRecursively(
     const WebKit::WebURL& path,
     WebKit::WebFileSystemCallbacks* callbacks) {
   int callbacks_id = CallbacksMap::GetOrCreate()->RegisterCallbacks(callbacks);
+  WaitableCallbackResults* waitable_results =
+      WaitableCallbackResults::MaybeCreate(callbacks);
   CallDispatcherOnMainThread(
       main_thread_loop_.get(),
       &FileSystemDispatcher::Remove,
       MakeTuple(GURL(path), true /* recursive */,
                 base::Bind(&StatusCallbackAdapter,
-                           CurrentWorkerId(), callbacks_id)));
+                           CurrentWorkerId(), callbacks_id, waitable_results)),
+      waitable_results);
 }
 
 void WebFileSystemImpl::readMetadata(
     const WebKit::WebURL& path,
     WebKit::WebFileSystemCallbacks* callbacks) {
   int callbacks_id = CallbacksMap::GetOrCreate()->RegisterCallbacks(callbacks);
+  WaitableCallbackResults* waitable_results =
+      WaitableCallbackResults::MaybeCreate(callbacks);
   CallDispatcherOnMainThread(
       main_thread_loop_.get(),
       &FileSystemDispatcher::ReadMetadata,
       MakeTuple(GURL(path),
                 base::Bind(&ReadMetadataCallbackAdapter,
-                           CurrentWorkerId(), callbacks_id),
+                           CurrentWorkerId(), callbacks_id, waitable_results),
                 base::Bind(&StatusCallbackAdapter,
-                           CurrentWorkerId(), callbacks_id)));
+                           CurrentWorkerId(), callbacks_id, waitable_results)),
+      waitable_results);
 }
 
 void WebFileSystemImpl::createFile(
@@ -325,12 +398,15 @@ void WebFileSystemImpl::createFile(
     bool exclusive,
     WebKit::WebFileSystemCallbacks* callbacks) {
   int callbacks_id = CallbacksMap::GetOrCreate()->RegisterCallbacks(callbacks);
+  WaitableCallbackResults* waitable_results =
+      WaitableCallbackResults::MaybeCreate(callbacks);
   CallDispatcherOnMainThread(
       main_thread_loop_.get(),
       &FileSystemDispatcher::CreateFile,
       MakeTuple(GURL(path), exclusive,
                 base::Bind(&StatusCallbackAdapter,
-                           CurrentWorkerId(), callbacks_id)));
+                           CurrentWorkerId(), callbacks_id, waitable_results)),
+      waitable_results);
 }
 
 void WebFileSystemImpl::createDirectory(
@@ -338,50 +414,62 @@ void WebFileSystemImpl::createDirectory(
     bool exclusive,
     WebKit::WebFileSystemCallbacks* callbacks) {
   int callbacks_id = CallbacksMap::GetOrCreate()->RegisterCallbacks(callbacks);
+  WaitableCallbackResults* waitable_results =
+      WaitableCallbackResults::MaybeCreate(callbacks);
   CallDispatcherOnMainThread(
       main_thread_loop_.get(),
       &FileSystemDispatcher::CreateDirectory,
       MakeTuple(GURL(path), exclusive, false /* recursive */,
                 base::Bind(&StatusCallbackAdapter,
-                           CurrentWorkerId(), callbacks_id)));
+                           CurrentWorkerId(), callbacks_id, waitable_results)),
+      waitable_results);
 }
 
 void WebFileSystemImpl::fileExists(
     const WebKit::WebURL& path,
     WebKit::WebFileSystemCallbacks* callbacks) {
   int callbacks_id = CallbacksMap::GetOrCreate()->RegisterCallbacks(callbacks);
+  WaitableCallbackResults* waitable_results =
+      WaitableCallbackResults::MaybeCreate(callbacks);
   CallDispatcherOnMainThread(
       main_thread_loop_.get(),
       &FileSystemDispatcher::Exists,
       MakeTuple(GURL(path), false /* directory */,
                 base::Bind(&StatusCallbackAdapter,
-                           CurrentWorkerId(), callbacks_id)));
+                           CurrentWorkerId(), callbacks_id, waitable_results)),
+      waitable_results);
 }
 
 void WebFileSystemImpl::directoryExists(
     const WebKit::WebURL& path,
     WebKit::WebFileSystemCallbacks* callbacks) {
   int callbacks_id = CallbacksMap::GetOrCreate()->RegisterCallbacks(callbacks);
+  WaitableCallbackResults* waitable_results =
+      WaitableCallbackResults::MaybeCreate(callbacks);
   CallDispatcherOnMainThread(
       main_thread_loop_.get(),
       &FileSystemDispatcher::Exists,
       MakeTuple(GURL(path), true /* directory */,
                 base::Bind(&StatusCallbackAdapter,
-                           CurrentWorkerId(), callbacks_id)));
+                           CurrentWorkerId(), callbacks_id, waitable_results)),
+      waitable_results);
 }
 
 void WebFileSystemImpl::readDirectory(
     const WebKit::WebURL& path,
     WebKit::WebFileSystemCallbacks* callbacks) {
   int callbacks_id = CallbacksMap::GetOrCreate()->RegisterCallbacks(callbacks);
+  WaitableCallbackResults* waitable_results =
+      WaitableCallbackResults::MaybeCreate(callbacks);
   CallDispatcherOnMainThread(
       main_thread_loop_.get(),
       &FileSystemDispatcher::ReadDirectory,
       MakeTuple(GURL(path),
                 base::Bind(&ReadDirectoryCallbackAdapater,
-                           CurrentWorkerId(), callbacks_id),
+                           CurrentWorkerId(), callbacks_id, waitable_results),
                 base::Bind(&StatusCallbackAdapter,
-                           CurrentWorkerId(), callbacks_id)));
+                           CurrentWorkerId(), callbacks_id, waitable_results)),
+      waitable_results);
 }
 
 WebKit::WebFileWriter* WebFileSystemImpl::createFileWriter(
@@ -394,30 +482,37 @@ void WebFileSystemImpl::createFileWriter(
     WebKit::WebFileWriterClient* client,
     WebKit::WebFileSystemCallbacks* callbacks) {
   int callbacks_id = CallbacksMap::GetOrCreate()->RegisterCallbacks(callbacks);
+  WaitableCallbackResults* waitable_results =
+      WaitableCallbackResults::MaybeCreate(callbacks);
   CallDispatcherOnMainThread(
       main_thread_loop_.get(),
       &FileSystemDispatcher::ReadMetadata,
       MakeTuple(GURL(path),
                 base::Bind(&CreateFileWriterCallbackAdapter,
-                           CurrentWorkerId(), callbacks_id, main_thread_loop_,
+                           CurrentWorkerId(), callbacks_id, waitable_results,
+                           main_thread_loop_,
                            GURL(path), client),
                 base::Bind(&StatusCallbackAdapter,
-                           CurrentWorkerId(), callbacks_id)));
+                           CurrentWorkerId(), callbacks_id, waitable_results)),
+      waitable_results);
 }
 
 void WebFileSystemImpl::createSnapshotFileAndReadMetadata(
     const WebKit::WebURL& path,
     WebKit::WebFileSystemCallbacks* callbacks) {
   int callbacks_id = CallbacksMap::GetOrCreate()->RegisterCallbacks(callbacks);
+  WaitableCallbackResults* waitable_results =
+      WaitableCallbackResults::MaybeCreate(callbacks);
   CallDispatcherOnMainThread(
       main_thread_loop_.get(),
       &FileSystemDispatcher::CreateSnapshotFile,
       MakeTuple(GURL(path),
                 base::Bind(&CreateSnapshotFileCallbackAdapter,
-                           CurrentWorkerId(), callbacks_id,
+                           CurrentWorkerId(), callbacks_id, waitable_results,
                            main_thread_loop_),
                 base::Bind(&StatusCallbackAdapter,
-                           CurrentWorkerId(), callbacks_id)));
+                           CurrentWorkerId(), callbacks_id, waitable_results)),
+      waitable_results);
 }
 
 }  // namespace content