Use C++11 lambdas to construct FileThread::Task objects
authorzandobersek@gmail.com <zandobersek@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 7 Jun 2014 13:29:31 +0000 (13:29 +0000)
committerzandobersek@gmail.com <zandobersek@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 7 Jun 2014 13:29:31 +0000 (13:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=133079

Reviewed by Darin Adler.

Have FileThread::Task constructor take in a std::function<void ()>-like object
as the second parameter. Much like with ScriptExecutionContext::Task, the idea here
is to use inlined C++11 lambdas in FileThread::postTask() calls. Because FileThread::Task
also requires an instance pointer, a brace-init list is passed to these calls, passing
in the instance pointer as the first parameter and the C++11 lambda as the second.

FileThread::postTask() now takes in a FileThread::Task rvalue and moves it onto the heap
and into the queue.

In AsyncFileStream.cpp, calls to FileThread::postTask() are updated to use C++11 lambdas.
Uses of MainThreadTask are also replaced by C++11 lambdas. This modernizes the code and
removes a lot of helper functions.

FileThread::Task doesn't use CrossThreadCopier anymore. Instead, we manually produce
thread-safe copies as necessary. At the moment this approach produces another copy when
passing the freshly-copied object into the lambda, but this will be avoidable once we
make C++14 and the lambda capture expression support mandatory.

* fileapi/AsyncFileStream.cpp:
(WebCore::AsyncFileStream::create):
(WebCore::AsyncFileStream::stop):
(WebCore::AsyncFileStream::getSize):
(WebCore::AsyncFileStream::openForRead):
(WebCore::AsyncFileStream::openForWrite):
(WebCore::AsyncFileStream::close):
(WebCore::AsyncFileStream::read):
(WebCore::AsyncFileStream::write):
(WebCore::AsyncFileStream::truncate):
(WebCore::didStart): Deleted.
(WebCore::AsyncFileStream::startOnFileThread): Deleted.
(WebCore::derefProxyOnMainThread): Deleted.
(WebCore::AsyncFileStream::stopOnFileThread): Deleted.
(WebCore::didGetSize): Deleted.
(WebCore::AsyncFileStream::getSizeOnFileThread): Deleted.
(WebCore::didOpen): Deleted.
(WebCore::AsyncFileStream::openForReadOnFileThread): Deleted.
(WebCore::AsyncFileStream::openForWriteOnFileThread): Deleted.
(WebCore::AsyncFileStream::closeOnFileThread): Deleted.
(WebCore::didRead): Deleted.
(WebCore::AsyncFileStream::readOnFileThread): Deleted.
(WebCore::didWrite): Deleted.
(WebCore::AsyncFileStream::writeOnFileThread): Deleted.
(WebCore::didTruncate): Deleted.
(WebCore::AsyncFileStream::truncateOnFileThread): Deleted.
* fileapi/AsyncFileStream.h:
* fileapi/FileThread.cpp:
(WebCore::FileThread::postTask):
* fileapi/FileThread.h:
(WebCore::FileThread::Task::Task):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@169675 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/fileapi/AsyncFileStream.cpp
Source/WebCore/fileapi/AsyncFileStream.h
Source/WebCore/fileapi/FileThread.cpp
Source/WebCore/fileapi/FileThread.h

index 6c45fe5..050d28e 100644 (file)
@@ -1,5 +1,62 @@
 2014-06-07  Zan Dobersek  <zdobersek@igalia.com>
 
+        Use C++11 lambdas to construct FileThread::Task objects
+        https://bugs.webkit.org/show_bug.cgi?id=133079
+
+        Reviewed by Darin Adler.
+
+        Have FileThread::Task constructor take in a std::function<void ()>-like object
+        as the second parameter. Much like with ScriptExecutionContext::Task, the idea here
+        is to use inlined C++11 lambdas in FileThread::postTask() calls. Because FileThread::Task
+        also requires an instance pointer, a brace-init list is passed to these calls, passing
+        in the instance pointer as the first parameter and the C++11 lambda as the second.
+
+        FileThread::postTask() now takes in a FileThread::Task rvalue and moves it onto the heap
+        and into the queue.
+
+        In AsyncFileStream.cpp, calls to FileThread::postTask() are updated to use C++11 lambdas.
+        Uses of MainThreadTask are also replaced by C++11 lambdas. This modernizes the code and
+        removes a lot of helper functions.
+
+        FileThread::Task doesn't use CrossThreadCopier anymore. Instead, we manually produce
+        thread-safe copies as necessary. At the moment this approach produces another copy when
+        passing the freshly-copied object into the lambda, but this will be avoidable once we
+        make C++14 and the lambda capture expression support mandatory.
+
+        * fileapi/AsyncFileStream.cpp:
+        (WebCore::AsyncFileStream::create):
+        (WebCore::AsyncFileStream::stop):
+        (WebCore::AsyncFileStream::getSize):
+        (WebCore::AsyncFileStream::openForRead):
+        (WebCore::AsyncFileStream::openForWrite):
+        (WebCore::AsyncFileStream::close):
+        (WebCore::AsyncFileStream::read):
+        (WebCore::AsyncFileStream::write):
+        (WebCore::AsyncFileStream::truncate):
+        (WebCore::didStart): Deleted.
+        (WebCore::AsyncFileStream::startOnFileThread): Deleted.
+        (WebCore::derefProxyOnMainThread): Deleted.
+        (WebCore::AsyncFileStream::stopOnFileThread): Deleted.
+        (WebCore::didGetSize): Deleted.
+        (WebCore::AsyncFileStream::getSizeOnFileThread): Deleted.
+        (WebCore::didOpen): Deleted.
+        (WebCore::AsyncFileStream::openForReadOnFileThread): Deleted.
+        (WebCore::AsyncFileStream::openForWriteOnFileThread): Deleted.
+        (WebCore::AsyncFileStream::closeOnFileThread): Deleted.
+        (WebCore::didRead): Deleted.
+        (WebCore::AsyncFileStream::readOnFileThread): Deleted.
+        (WebCore::didWrite): Deleted.
+        (WebCore::AsyncFileStream::writeOnFileThread): Deleted.
+        (WebCore::didTruncate): Deleted.
+        (WebCore::AsyncFileStream::truncateOnFileThread): Deleted.
+        * fileapi/AsyncFileStream.h:
+        * fileapi/FileThread.cpp:
+        (WebCore::FileThread::postTask):
+        * fileapi/FileThread.h:
+        (WebCore::FileThread::Task::Task):
+
+2014-06-07  Zan Dobersek  <zdobersek@igalia.com>
+
         Pass C++11 lambdas to callOnMainThread() calls in IconDatabase
         https://bugs.webkit.org/show_bug.cgi?id=133374
 
index db29b0f..5a391f8 100644 (file)
@@ -37,7 +37,6 @@
 #include "FileStream.h"
 #include "FileStreamClient.h"
 #include "FileThread.h"
-#include "MainThreadTask.h"
 #include <wtf/MainThread.h>
 #include <wtf/text/WTFString.h>
 
@@ -77,7 +76,18 @@ PassRefPtr<AsyncFileStream> AsyncFileStream::create(FileStreamClient* client)
     // This is balanced by the deref in derefProxyOnContext below.
     proxy->ref();
 
-    fileThread()->postTask(std::make_unique<FileThread::Task>(proxy.get(), &AsyncFileStream::startOnFileThread));
+    AsyncFileStream* proxyPtr = proxy.get();
+    fileThread()->postTask({ proxyPtr, [=] {
+        // FIXME: It is not correct to check m_client from a secondary thread - stop() could be racing with this check.
+        if (!proxyPtr->client())
+            return;
+
+        proxyPtr->m_stream->start();
+        callOnMainThread([proxyPtr] {
+            if (proxyPtr->client())
+                proxyPtr->client()->didStart();
+        });
+    } });
 
     return proxy.release();
 }
@@ -86,146 +96,96 @@ AsyncFileStream::~AsyncFileStream()
 {
 }
 
-static void didStart(AsyncFileStream* proxy)
-{
-    if (proxy->client())
-        proxy->client()->didStart();
-}
-
-void AsyncFileStream::startOnFileThread()
-{
-    // FIXME: It is not correct to check m_client from a secondary thread - stop() could be racing with this check.
-    if (!m_client)
-        return;
-    m_stream->start();
-    callOnMainThread(MainThreadTask(didStart, AllowCrossThreadAccess(this)));
-}
-
 void AsyncFileStream::stop()
 {
     // Clear the client so that we won't be invoking callbacks on the client.
     setClient(0);
 
     fileThread()->unscheduleTasks(m_stream.get());
-    fileThread()->postTask(std::make_unique<FileThread::Task>(this, &AsyncFileStream::stopOnFileThread));
-}
-
-static void derefProxyOnMainThread(AsyncFileStream* proxy)
-{
-    ASSERT(proxy->hasOneRef());
-    proxy->deref();
-}
-
-void AsyncFileStream::stopOnFileThread()
-{
-    m_stream->stop();
-    callOnMainThread(MainThreadTask(derefProxyOnMainThread, AllowCrossThreadAccess(this)));
-}
-
-static void didGetSize(AsyncFileStream* proxy, long long size)
-{
-    if (proxy->client())
-        proxy->client()->didGetSize(size);
+    fileThread()->postTask({ this, [this] {
+        m_stream->stop();
+        callOnMainThread([this] {
+            ASSERT(hasOneRef());
+            deref();
+        });
+    } });
 }
 
 void AsyncFileStream::getSize(const String& path, double expectedModificationTime)
 {
-    fileThread()->postTask(std::make_unique<FileThread::Task>(this, &AsyncFileStream::getSizeOnFileThread, path, expectedModificationTime));
-}
-
-void AsyncFileStream::getSizeOnFileThread(const String& path, double expectedModificationTime)
-{
-    long long size = m_stream->getSize(path, expectedModificationTime);
-    callOnMainThread(MainThreadTask(didGetSize, AllowCrossThreadAccess(this), size));
-}
-
-static void didOpen(AsyncFileStream* proxy, bool success)
-{
-    if (proxy->client())
-        proxy->client()->didOpen(success);
+    String pathCopy = path.isolatedCopy();
+    fileThread()->postTask({ this, [this, pathCopy, expectedModificationTime] {
+        long long size = m_stream->getSize(pathCopy, expectedModificationTime);
+        callOnMainThread([this, size] {
+            if (client())
+                client()->didGetSize(size);
+        });
+    } });
 }
 
 void AsyncFileStream::openForRead(const String& path, long long offset, long long length)
 {
-    fileThread()->postTask(std::make_unique<FileThread::Task>(this, &AsyncFileStream::openForReadOnFileThread, path, offset, length));
-}
-
-void AsyncFileStream::openForReadOnFileThread(const String& path, long long offset, long long length)
-{
-    bool success = m_stream->openForRead(path, offset, length);
-    callOnMainThread(MainThreadTask(didOpen, AllowCrossThreadAccess(this), success));
+    String pathCopy = path.isolatedCopy();
+    fileThread()->postTask({ this, [this, pathCopy, offset, length] {
+        bool success = m_stream->openForRead(pathCopy, offset, length);
+        callOnMainThread([this, success] {
+            if (client())
+                client()->didOpen(success);
+        });
+    } });
 }
 
 void AsyncFileStream::openForWrite(const String& path)
 {
-    fileThread()->postTask(std::make_unique<FileThread::Task>(this, &AsyncFileStream::openForWriteOnFileThread, path));
-}
-
-void AsyncFileStream::openForWriteOnFileThread(const String& path)
-{
-    bool success = m_stream->openForWrite(path);
-    callOnMainThread(MainThreadTask(didOpen, AllowCrossThreadAccess(this), success));
+    String pathCopy = path.isolatedCopy();
+    fileThread()->postTask({ this, [this, pathCopy] {
+        bool success = m_stream->openForWrite(pathCopy);
+        callOnMainThread([this, success] {
+            if (client())
+                client()->didOpen(success);
+        });
+    } });
 }
 
 void AsyncFileStream::close()
 {
-    fileThread()->postTask(std::make_unique<FileThread::Task>(this, &AsyncFileStream::closeOnFileThread));
-}
-
-void AsyncFileStream::closeOnFileThread()
-{
-    m_stream->close();
-}
-
-static void didRead(AsyncFileStream* proxy, int bytesRead)
-{
-    if (proxy->client())
-        proxy->client()->didRead(bytesRead);
+    fileThread()->postTask({this, [this] {
+        m_stream->close();
+    } });
 }
 
 void AsyncFileStream::read(char* buffer, int length)
 {
-    fileThread()->postTask(std::make_unique<FileThread::Task>(this, &AsyncFileStream::readOnFileThread, AllowCrossThreadAccess(buffer), length));
-}
-
-void AsyncFileStream::readOnFileThread(char* buffer, int length)
-{
-    int bytesRead = m_stream->read(buffer, length);
-    callOnMainThread(MainThreadTask(didRead, AllowCrossThreadAccess(this), bytesRead));
-}
-
-static void didWrite(AsyncFileStream* proxy, int bytesWritten)
-{
-    if (proxy->client())
-        proxy->client()->didWrite(bytesWritten);
+    fileThread()->postTask({ this, [this, buffer, length] {
+        int bytesRead = m_stream->read(buffer, length);
+        callOnMainThread([this, bytesRead] {
+            if (client())
+                client()->didRead(bytesRead);
+        });
+    } });
 }
 
 void AsyncFileStream::write(const URL& blobURL, long long position, int length)
 {
-    fileThread()->postTask(std::make_unique<FileThread::Task>(this, &AsyncFileStream::writeOnFileThread, blobURL, position, length));
-}
-
-void AsyncFileStream::writeOnFileThread(const URL& blobURL, long long position, int length)
-{
-    int bytesWritten = m_stream->write(blobURL, position, length);
-    callOnMainThread(MainThreadTask(didWrite, AllowCrossThreadAccess(this), bytesWritten));
-}
-
-static void didTruncate(AsyncFileStream* proxy, bool success)
-{
-    if (proxy->client())
-        proxy->client()->didTruncate(success);
+    URL blobURLCopy = blobURL.copy();
+    fileThread()->postTask({ this, [this, blobURLCopy, position, length] {
+        int bytesWritten = m_stream->write(blobURLCopy, position, length);
+        callOnMainThread([this, bytesWritten] {
+            if (client())
+                client()->didWrite(bytesWritten);
+        });
+    } });
 }
 
 void AsyncFileStream::truncate(long long position)
 {
-    fileThread()->postTask(std::make_unique<FileThread::Task>(this, &AsyncFileStream::truncateOnFileThread, position));
-}
-
-void AsyncFileStream::truncateOnFileThread(long long position)
-{
-    bool success = m_stream->truncate(position);
-    callOnMainThread(MainThreadTask(didTruncate, AllowCrossThreadAccess(this), success));
+    fileThread()->postTask({ this, [this, position] {
+        bool success = m_stream->truncate(position);
+        callOnMainThread([this, success] {
+            if (client())
+                client()->didTruncate(success);
+        });
+    } });
 }
 
 } // namespace WebCore
index 8eebfe0..b60bc60 100644 (file)
@@ -65,19 +65,7 @@ public:
 private:
     AsyncFileStream(FileStreamClient*);
 
-    // Called on File thread.
-    void startOnFileThread();
-    void stopOnFileThread();
-    void getSizeOnFileThread(const String& path, double expectedModificationTime);
-    void openForReadOnFileThread(const String& path, long long offset, long long length);
-    void openForWriteOnFileThread(const String& path);
-    void closeOnFileThread();
-    void readOnFileThread(char* buffer, int length);
-    void writeOnFileThread(const URL& blobURL, long long position, int length);
-    void truncateOnFileThread(long long position);
-
     RefPtr<FileStream> m_stream;
-
     FileStreamClient* m_client;
 };
 
index 2b31ca1..e80ae9f 100644 (file)
@@ -62,9 +62,9 @@ void FileThread::stop()
     m_queue.kill();
 }
 
-void FileThread::postTask(std::unique_ptr<Task> task)
+void FileThread::postTask(Task&& task)
 {
-    m_queue.append(std::move(task));
+    m_queue.append(std::make_unique<FileThread::Task>(std::move(task)));
 }
 
 class SameInstancePredicate {
index 7fdc04d..a44b046 100644 (file)
@@ -31,7 +31,6 @@
 #ifndef FileThread_h
 #define FileThread_h
 
-#include "CrossThreadCopier.h"
 #include <functional>
 #include <wtf/MessageQueue.h>
 #include <wtf/PassRefPtr.h>
@@ -56,13 +55,19 @@ public:
     class Task {
         WTF_MAKE_NONCOPYABLE(Task);
     public:
-        template<typename T, typename Method, typename... Parameters>
-        Task(T* instance, Method method, const Parameters&... parameters)
-            : m_task(std::bind(method, instance, typename CrossThreadCopier<Parameters>::Type(CrossThreadCopier<Parameters>::copy(parameters))...))
+        template<typename T, typename U, typename = typename std::enable_if<!std::is_base_of<Task, U>::value && std::is_convertible<U, std::function<void ()>>::value>::type>
+        Task(T* instance, U method)
+            : m_task(std::move(method))
             , m_instance(instance)
         {
         }
 
+        Task(Task&& other)
+            : m_task(std::move(other.m_task))
+            , m_instance(other.m_instance)
+        {
+        }
+
         void performTask()
         {
             m_task();
@@ -74,7 +79,7 @@ public:
         void* m_instance;
     };
 
-    void postTask(std::unique_ptr<Task>);
+    void postTask(Task&&);
 
     void unscheduleTasks(const void* instance);