Changeset 201464 in webkit


Ignore:
Timestamp:
May 27, 2016 1:22:12 PM (8 years ago)
Author:
Chris Dumez
Message:

WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables
​https://bugs.webkit.org/show_bug.cgi?id=158111

Reviewed by Darin Adler.

WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables.
These are often used cross-thread and copying the captured lambda variables can be
dangerous (e.g. we do not want to copy a String after calling isolatedCopy() upon
capture).

Source/JavaScriptCore:

  • runtime/Watchdog.cpp:

(JSC::Watchdog::startTimer):
(JSC::Watchdog::Watchdog): Deleted.
(JSC::Watchdog::setTimeLimit): Deleted.

  • runtime/Watchdog.h:

Source/WebKit2:

  • NetworkProcess/NetworkProcess.cpp:

(WebKit::clearDiskCacheEntries):

  • NetworkProcess/cache/NetworkCache.cpp:

(WebKit::NetworkCache::Cache::clear):

  • NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:

(WebKit::NetworkCache::runTaskInQueue):

  • Platform/IPC/Connection.cpp:

(IPC::Connection::processIncomingMessage):

  • UIProcess/Storage/StorageManager.cpp:

(WebKit::StorageManager::getSessionStorageOrigins):
(WebKit::StorageManager::deleteSessionStorageOrigins):
(WebKit::StorageManager::deleteSessionStorageEntriesForOrigins):
(WebKit::StorageManager::getLocalStorageOrigins):
(WebKit::StorageManager::getLocalStorageOriginDetails):
(WebKit::StorageManager::deleteLocalStorageOriginsModifiedSince):
(WebKit::StorageManager::deleteLocalStorageEntriesForOrigins):

  • UIProcess/Storage/StorageManager.h:

Source/WTF:

This patch introduces a new NoncopyableFunction type that behaves similarly to
std::function but guarantees that the passed-in lambda (and its captured variables)
cannot be copied. This new NoncopyableFunction type is now used for
WorkQueue / RunLoop's dispatch() / dispatchAfter() which are commonly used
cross-thread. This should now allow us to call WorkQueue::dispatch() with a lambda
that captures a String like so:
[str = str.isolatedCopy()]() { }

Also note that even though this is not leveraged in this patch, NoncopyableFunction
would allow us to capture move-only types such as std::unique_ptr as so:
[p = WTFMove(p)]() { }
This does not work if we convert the lambda into an std::function because
std::function requires the lambda to be copyable, NoncopyableFunction does not.

  • wtf/FunctionDispatcher.h:

(WTF::CallableWrapperBase::~CallableWrapperBase):
(WTF::NoncopyableFunction::NoncopyableFunction):
(WTF::NoncopyableFunction::operator()):
(WTF::NoncopyableFunction::operator bool):
(WTF::NoncopyableFunction::operator=):

  • wtf/RunLoop.cpp:

(WTF::RunLoop::performWork):
(WTF::RunLoop::dispatch):

  • wtf/RunLoop.h:
  • wtf/WorkQueue.h:
  • wtf/cocoa/WorkQueueCocoa.cpp:

(WTF::WorkQueue::dispatch):
(WTF::WorkQueue::dispatchAfter):

  • wtf/efl/DispatchQueueWorkItemEfl.h:

(WorkItem::WorkItem):
(TimerWorkItem::create):
(TimerWorkItem::TimerWorkItem):

  • wtf/efl/WorkQueueEfl.cpp:

(WTF::WorkQueue::dispatch):
(WTF::WorkQueue::dispatchAfter):

  • wtf/generic/RunLoopGeneric.cpp:

(WTF::RunLoop::dispatchAfter):

  • wtf/generic/WorkQueueGeneric.cpp:

(WorkQueue::dispatch):
(WorkQueue::dispatchAfter):

  • wtf/glib/RunLoopGLib.cpp:

(WTF::DispatchAfterContext::DispatchAfterContext):
(WTF::RunLoop::dispatchAfter):

  • wtf/win/WorkItemWin.cpp:

(WTF::WorkItemWin::WorkItemWin):
(WTF::WorkItemWin::create):
(WTF::HandleWorkItem::HandleWorkItem):
(WTF::HandleWorkItem::createByAdoptingHandle):

  • wtf/win/WorkItemWin.h:

(WTF::WorkItemWin::function):

  • wtf/win/WorkQueueWin.cpp:

(WTF::WorkQueue::dispatch):
(WTF::WorkQueue::timerCallback):
(WTF::WorkQueue::dispatchAfter):

Tools:

  • WebKitTestRunner/TestController.cpp:

(WTR::TestController::decidePolicyForNavigationAction):

Location:
trunk
Files:
26 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r201462 r201464  
     12016-05-27  Chris Dumez  <cdumez@apple.com>
     2
     3        WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables
     4        https://bugs.webkit.org/show_bug.cgi?id=158111
     5
     6        Reviewed by Darin Adler.
     7
     8        WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables.
     9        These are often used cross-thread and copying the captured lambda variables can be
     10        dangerous (e.g. we do not want to copy a String after calling isolatedCopy() upon
     11        capture).
     12
     13        * runtime/Watchdog.cpp:
     14        (JSC::Watchdog::startTimer):
     15        (JSC::Watchdog::Watchdog): Deleted.
     16        (JSC::Watchdog::setTimeLimit): Deleted.
     17        * runtime/Watchdog.h:
     18
    1192016-05-27  Konstantin Tokarev  <annulen@yandex.ru>
    220

  • trunk/Source/JavaScriptCore/runtime/Watchdog.cpp

    r193636 r201464  
    5151    , m_timerQueue(WorkQueue::create("jsc.watchdog.queue", WorkQueue::Type::Serial, WorkQueue::QOS::Utility))
    5252{
    53     m_timerHandler = [this] {
    54         {
    55             LockHolder locker(m_lock);
    56             this->m_timerDidFire = true;
    57         }
    58         this->deref();
    59     };
    6053}
    6154
    … …  
    178171    m_wallClockDeadline = wallClockDeadline;
    179172
    180     m_timerQueue->dispatchAfter(std::chrono::nanoseconds(timeLimit), m_timerHandler);
     173    m_timerQueue->dispatchAfter(std::chrono::nanoseconds(timeLimit), [this] {
     174        {
     175            LockHolder locker(m_lock);
     176            m_timerDidFire = true;
     177        }
     178        deref();
     179    });
    181180}
    182181

  • trunk/Source/JavaScriptCore/runtime/Watchdog.h

    r193636 r201464  
    9292
    9393    Ref<WorkQueue> m_timerQueue;
    94     std::function<void ()> m_timerHandler;
    9594
    9695    friend class LLIntOffsetsExtractor;

  • trunk/Source/WTF/ChangeLog

    r201433 r201464  
     12016-05-27  Chris Dumez  <cdumez@apple.com>
     2
     3        WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables
     4        https://bugs.webkit.org/show_bug.cgi?id=158111
     5
     6        Reviewed by Darin Adler.
     7
     8        WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables.
     9        These are often used cross-thread and copying the captured lambda variables can be
     10        dangerous (e.g. we do not want to copy a String after calling isolatedCopy() upon
     11        capture).
     12
     13        This patch introduces a new NoncopyableFunction type that behaves similarly to
     14        std::function but guarantees that the passed-in lambda (and its captured variables)
     15        cannot be copied. This new NoncopyableFunction type is now used for
     16        WorkQueue / RunLoop's dispatch() / dispatchAfter() which are commonly used
     17        cross-thread. This should now allow us to call WorkQueue::dispatch() with a lambda
     18        that captures a String like so:
     19        [str = str.isolatedCopy()]() { }
     20
     21        Also note that even though this is not leveraged in this patch, NoncopyableFunction
     22        would allow us to capture move-only types such as std::unique_ptr as so:
     23        [p = WTFMove(p)]() { }
     24        This does not work if we convert the lambda into an std::function because
     25        std::function requires the lambda to be copyable, NoncopyableFunction does not.
     26
     27        * wtf/FunctionDispatcher.h:
     28        (WTF::CallableWrapperBase::~CallableWrapperBase):
     29        (WTF::NoncopyableFunction::NoncopyableFunction):
     30        (WTF::NoncopyableFunction::operator()):
     31        (WTF::NoncopyableFunction::operator bool):
     32        (WTF::NoncopyableFunction::operator=):
     33        * wtf/RunLoop.cpp:
     34        (WTF::RunLoop::performWork):
     35        (WTF::RunLoop::dispatch):
     36        * wtf/RunLoop.h:
     37        * wtf/WorkQueue.h:
     38        * wtf/cocoa/WorkQueueCocoa.cpp:
     39        (WTF::WorkQueue::dispatch):
     40        (WTF::WorkQueue::dispatchAfter):
     41        * wtf/efl/DispatchQueueWorkItemEfl.h:
     42        (WorkItem::WorkItem):
     43        (TimerWorkItem::create):
     44        (TimerWorkItem::TimerWorkItem):
     45        * wtf/efl/WorkQueueEfl.cpp:
     46        (WTF::WorkQueue::dispatch):
     47        (WTF::WorkQueue::dispatchAfter):
     48        * wtf/generic/RunLoopGeneric.cpp:
     49        (WTF::RunLoop::dispatchAfter):
     50        * wtf/generic/WorkQueueGeneric.cpp:
     51        (WorkQueue::dispatch):
     52        (WorkQueue::dispatchAfter):
     53        * wtf/glib/RunLoopGLib.cpp:
     54        (WTF::DispatchAfterContext::DispatchAfterContext):
     55        (WTF::RunLoop::dispatchAfter):
     56        * wtf/win/WorkItemWin.cpp:
     57        (WTF::WorkItemWin::WorkItemWin):
     58        (WTF::WorkItemWin::create):
     59        (WTF::HandleWorkItem::HandleWorkItem):
     60        (WTF::HandleWorkItem::createByAdoptingHandle):
     61        * wtf/win/WorkItemWin.h:
     62        (WTF::WorkItemWin::function):
     63        * wtf/win/WorkQueueWin.cpp:
     64        (WTF::WorkQueue::dispatch):
     65        (WTF::WorkQueue::timerCallback):
     66        (WTF::WorkQueue::dispatchAfter):
     67
    1682016-05-26  Filip Pizlo  <fpizlo@apple.com>
    269

  • trunk/Source/WTF/wtf/FunctionDispatcher.h

    r174864 r201464  
    3232namespace WTF {
    3333
     34// FIXME: Move this to its own header (e.g. Functional.h).
     35// FIXME: We could make this templated to support other lambdas than void() and make this more reusable.
     36class NoncopyableFunction {
     37public:
     38    NoncopyableFunction() = default;
     39
     40    template<typename CallableType, class = typename std::enable_if<std::is_rvalue_reference<CallableType&&>::value>::type>
     41    NoncopyableFunction(CallableType&& callable)
     42        : m_callableWrapper(std::make_unique<CallableWrapper<CallableType>>(WTFMove(callable)))
     43    {
     44    }
     45
     46    void operator()() const
     47    {
     48        if (m_callableWrapper)
     49            m_callableWrapper->call();
     50    }
     51
     52    explicit operator bool() const { return !!m_callableWrapper; }
     53
     54    template<typename CallableType, class = typename std::enable_if<std::is_rvalue_reference<CallableType&&>::value>::type>
     55    NoncopyableFunction& operator=(CallableType&& callable)
     56    {
     57        m_callableWrapper = std::make_unique<CallableWrapper<CallableType>>(WTFMove(callable));
     58        return *this;
     59    }
     60
     61private:
     62    class CallableWrapperBase {
     63        WTF_MAKE_FAST_ALLOCATED;
     64    public:
     65        virtual ~CallableWrapperBase() { }
     66
     67        virtual void call() = 0;
     68    };
     69
     70    template<typename CallableType>
     71    class CallableWrapper final : public CallableWrapperBase {
     72    public:
     73        explicit CallableWrapper(CallableType&& callable)
     74            : m_callable(WTFMove(callable))
     75        {
     76        }
     77
     78        CallableWrapper(const CallableWrapper&) = delete;
     79        CallableWrapper& operator=(const CallableWrapper&) = delete;
     80
     81        void call() final { m_callable(); }
     82
     83    private:
     84        CallableType m_callable;
     85    };
     86
     87    std::unique_ptr<CallableWrapperBase> m_callableWrapper;
     88};
     89
    3490// FunctionDispatcher is an abstract representation of something that functions can be
    3591// dispatched to. This can for example be a run loop or a work queue.
    … …  
    3995    WTF_EXPORT_PRIVATE virtual ~FunctionDispatcher();
    4096
    41     virtual void dispatch(std::function<void ()>) = 0;
     97    virtual void dispatch(NoncopyableFunction&&) = 0;
    4298
    4399protected:
    … …  
    48104
    49105using WTF::FunctionDispatcher;
     106using WTF::NoncopyableFunction;
    50107
    51108#endif // FunctionDispatcher_h

  • trunk/Source/WTF/wtf/RunLoop.cpp

    r194819 r201464  
    9292    size_t functionsToHandle = 0;
    9393    {
    94         std::function<void()> function;
     94        NoncopyableFunction function;
    9595        {
    9696            MutexLocker locker(m_functionQueueLock);
    … …  
    107107
    108108    for (size_t functionsHandled = 1; functionsHandled < functionsToHandle; ++functionsHandled) {
    109         std::function<void()> function;
     109        NoncopyableFunction function;
    110110        {
    111111            MutexLocker locker(m_functionQueueLock);
    … …  
    124124}
    125125
    126 void RunLoop::dispatch(std::function<void ()> function)
     126void RunLoop::dispatch(NoncopyableFunction&& function)
    127127{
    128128    {

  • trunk/Source/WTF/wtf/RunLoop.h

    r199713 r201464  
    6060    ~RunLoop();
    6161
    62     void dispatch(std::function<void()>) override;
     62    void dispatch(NoncopyableFunction&&) override;
    6363
    6464    WTF_EXPORT_PRIVATE static void run();
    … …  
    8080
    8181#if USE(GLIB_EVENT_LOOP) || USE(GENERIC_EVENT_LOOP)
    82     WTF_EXPORT_PRIVATE void dispatchAfter(std::chrono::nanoseconds, std::function<void()>);
     82    WTF_EXPORT_PRIVATE void dispatchAfter(std::chrono::nanoseconds, NoncopyableFunction&&);
    8383#endif
    8484
    … …  
    156156
    157157    Mutex m_functionQueueLock;
    158     Deque<std::function<void ()>> m_functionQueue;
     158    Deque<NoncopyableFunction> m_functionQueue;
    159159
    160160#if USE(WINDOWS_EVENT_LOOP)

  • trunk/Source/WTF/wtf/WorkQueue.h

    r199713 r201464  
    6969        Background
    7070    };
    71    
     71
    7272    WTF_EXPORT_PRIVATE static Ref<WorkQueue> create(const char* name, Type = Type::Serial, QOS = QOS::Default);
    7373    virtual ~WorkQueue();
    7474
    75     WTF_EXPORT_PRIVATE void dispatch(std::function<void ()>) override;
    76     WTF_EXPORT_PRIVATE void dispatchAfter(std::chrono::nanoseconds, std::function<void ()>);
     75    WTF_EXPORT_PRIVATE void dispatch(NoncopyableFunction&&) override;
     76    WTF_EXPORT_PRIVATE void dispatchAfter(std::chrono::nanoseconds, NoncopyableFunction&&);
    7777
    7878    WTF_EXPORT_PRIVATE static void concurrentApply(size_t iterations, const std::function<void (size_t index)>&);

  • trunk/Source/WTF/wtf/cocoa/WorkQueueCocoa.cpp

    r188981 r201464  
    2929namespace WTF {
    3030
    31 void WorkQueue::dispatch(std::function<void ()> function)
     31void WorkQueue::dispatch(NoncopyableFunction&& function)
    3232{
    3333    ref();
     34    auto* functionPtr = new NoncopyableFunction(WTFMove(function));
    3435    dispatch_async(m_dispatchQueue, ^{
    35         function();
     36        (*functionPtr)();
     37        delete functionPtr;
    3638        deref();
    3739    });
    3840}
    3941
    40 void WorkQueue::dispatchAfter(std::chrono::nanoseconds duration, std::function<void ()> function)
     42void WorkQueue::dispatchAfter(std::chrono::nanoseconds duration, NoncopyableFunction&& function)
    4143{
    4244    ref();
     45    auto* functionPtr = new NoncopyableFunction(WTFMove(function));
    4346    dispatch_after(dispatch_time(DISPATCH_TIME_NOW, duration.count()), m_dispatchQueue, ^{
    44         function();
     47        (*functionPtr)();
     48        delete functionPtr;
    4549        deref();
    4650    });

  • trunk/Source/WTF/wtf/efl/DispatchQueueWorkItemEfl.h

    r194496 r201464  
    2929#include <wtf/Assertions.h>
    3030#include <wtf/CurrentTime.h>
     31#include <wtf/FunctionDispatcher.h>
    3132#include <wtf/RefCounted.h>
    3233#include <wtf/WorkQueue.h>
    … …  
    3435class WorkItem {
    3536public:
    36     WorkItem(PassRefPtr<WorkQueue> workQueue, std::function<void ()> function)
    37         : m_workQueue(workQueue)
     37    WorkItem(Ref<WorkQueue>&& workQueue, NoncopyableFunction&& function)
     38        : m_workQueue(WTFMove(workQueue))
    3839        , m_function(WTFMove(function))
    3940    {
    … …  
    4344
    4445private:
    45     RefPtr<WorkQueue> m_workQueue;
    46     std::function<void ()> m_function;
     46    Ref<WorkQueue> m_workQueue;
     47    NoncopyableFunction m_function;
    4748};
    4849
    4950class TimerWorkItem : public WorkItem {
    5051public:
    51     static std::unique_ptr<TimerWorkItem> create(PassRefPtr<WorkQueue> workQueue, std::function<void ()> function, std::chrono::nanoseconds delayNanoSeconds)
     52    static std::unique_ptr<TimerWorkItem> create(Ref<WorkQueue>&& workQueue, NoncopyableFunction&& function, std::chrono::nanoseconds delayNanoSeconds)
    5253    {
    5354        ASSERT(delayNanoSeconds.count() >= 0);
    54         return std::unique_ptr<TimerWorkItem>(new TimerWorkItem(workQueue, WTFMove(function), monotonicallyIncreasingTime() * 1000000000.0 + delayNanoSeconds.count()));
     55        return std::unique_ptr<TimerWorkItem>(new TimerWorkItem(WTFMove(workQueue), WTFMove(function), monotonicallyIncreasingTime() * 1000000000.0 + delayNanoSeconds.count()));
    5556    }
    5657    double expirationTimeNanoSeconds() const { return m_expirationTimeNanoSeconds; }
    … …  
    5859
    5960protected:
    60     TimerWorkItem(PassRefPtr<WorkQueue> workQueue, std::function<void ()> function, double expirationTimeNanoSeconds)
    61         : WorkItem(workQueue, WTFMove(function))
     61    TimerWorkItem(Ref<WorkQueue>&& workQueue, NoncopyableFunction&& function, double expirationTimeNanoSeconds)
     62        : WorkItem(WTFMove(workQueue), WTFMove(function))
    6263        , m_expirationTimeNanoSeconds(expirationTimeNanoSeconds)
    6364    {

  • trunk/Source/WTF/wtf/efl/WorkQueueEfl.cpp

    r195094 r201464  
    5555}
    5656
    57 void WorkQueue::dispatch(std::function<void ()> function)
     57void WorkQueue::dispatch(NoncopyableFunction&& function)
    5858{
    5959    if (!m_dispatchQueue)
    6060        return;
    6161
    62     m_dispatchQueue->dispatch(std::make_unique<WorkItem>(this, WTFMove(function)));
     62    m_dispatchQueue->dispatch(std::make_unique<WorkItem>(*this, WTFMove(function)));
    6363}
    6464
    65 void WorkQueue::dispatchAfter(std::chrono::nanoseconds duration, std::function<void ()> function)
     65void WorkQueue::dispatchAfter(std::chrono::nanoseconds duration, NoncopyableFunction&& function)
    6666{
    6767    if (!m_dispatchQueue)
    6868        return;
    6969
    70     m_dispatchQueue->dispatch(TimerWorkItem::create(this, WTFMove(function), duration));
     70    m_dispatchQueue->dispatch(TimerWorkItem::create(*this, WTFMove(function), duration));
    7171}
    7272

  • trunk/Source/WTF/wtf/generic/RunLoopGeneric.cpp

    r199694 r201464  
    240240}
    241241
    242 void RunLoop::dispatchAfter(std::chrono::nanoseconds delay, std::function<void()> function)
     242void RunLoop::dispatchAfter(std::chrono::nanoseconds delay, NoncopyableFunction&& function)
    243243{
    244244    LockHolder locker(m_loopLock);
    245245    bool repeating = false;
    246     schedule(locker, TimerBase::ScheduledTask::create(function, delay.count() / 1000.0 / 1000.0 / 1000.0, repeating));
     246    schedule(locker, TimerBase::ScheduledTask::create(WTFMove(function), delay.count() / 1000.0 / 1000.0 / 1000.0, repeating));
    247247    wakeUp(locker);
    248248}

  • trunk/Source/WTF/wtf/generic/WorkQueueGeneric.cpp

    r199694 r201464  
    8282}
    8383
    84 void WorkQueue::dispatch(std::function<void()> function)
     84void WorkQueue::dispatch(NoncopyableFunction&& function)
    8585{
    8686    RefPtr<WorkQueue> protect(this);
    87     m_runLoop->dispatch([protect, function] {
     87    m_runLoop->dispatch([protect, function = WTFMove(function)] {
    8888        function();
    8989    });
    9090}
    9191
    92 void WorkQueue::dispatchAfter(std::chrono::nanoseconds delay, std::function<void()> function)
     92void WorkQueue::dispatchAfter(std::chrono::nanoseconds delay, NoncopyableFunction&& function)
    9393{
    9494    RefPtr<WorkQueue> protect(this);
    95     m_runLoop->dispatchAfter(delay, [protect, function] {
     95    m_runLoop->dispatchAfter(delay, [protect, function = WTFMove(function)] {
    9696        function();
    9797    });

  • trunk/Source/WTF/wtf/glib/RunLoopGLib.cpp

    r199713 r201464  
    125125    WTF_MAKE_FAST_ALLOCATED;
    126126public:
    127     DispatchAfterContext(std::function<void()>&& function)
     127    DispatchAfterContext(NoncopyableFunction&& function)
    128128        : m_function(WTFMove(function))
    129129    {
    … …  
    136136
    137137private:
    138     std::function<void()> m_function;
     138    NoncopyableFunction m_function;
    139139};
    140140
    141 void RunLoop::dispatchAfter(std::chrono::nanoseconds duration, std::function<void()> function)
     141void RunLoop::dispatchAfter(std::chrono::nanoseconds duration, NoncopyableFunction&& function)
    142142{
    143143    GRefPtr<GSource> source = adoptGRef(g_timeout_source_new(std::chrono::duration_cast<std::chrono::milliseconds>(duration).count()));

  • trunk/Source/WTF/wtf/win/WorkItemWin.cpp

    r181220 r201464  
    3333namespace WTF {
    3434
    35 WorkItemWin::WorkItemWin(std::function<void()> function, WorkQueue* queue)
    36     : m_function(function)
     35WorkItemWin::WorkItemWin(NoncopyableFunction&& function, WorkQueue* queue)
     36    : m_function(WTFMove(function))
    3737    , m_queue(queue)
    3838{
    3939}
    4040
    41 RefPtr<WorkItemWin> WorkItemWin::create(std::function<void()> function, WorkQueue* queue)
     41RefPtr<WorkItemWin> WorkItemWin::create(NoncopyableFunction&& function, WorkQueue* queue)
    4242{
    43     return adoptRef(new WorkItemWin(function, queue));
     43    return adoptRef(new WorkItemWin(WTFMove(function), queue));
    4444}
    4545
    … …  
    4848}
    4949
    50 HandleWorkItem::HandleWorkItem(HANDLE handle, const std::function<void()>& function, WorkQueue* queue)
    51     : WorkItemWin(function, queue)
     50HandleWorkItem::HandleWorkItem(HANDLE handle, NoncopyableFunction&& function, WorkQueue* queue)
     51    : WorkItemWin(WTFMove(function), queue)
    5252    , m_handle(handle)
    5353    , m_waitHandle(0)
    … …  
    5656}
    5757
    58 RefPtr<HandleWorkItem> HandleWorkItem::createByAdoptingHandle(HANDLE handle, const std::function<void()>& function, WorkQueue* queue)
     58RefPtr<HandleWorkItem> HandleWorkItem::createByAdoptingHandle(HANDLE handle, NoncopyableFunction&& function, WorkQueue* queue)
    5959{
    60     return adoptRef(new HandleWorkItem(handle, function, queue));
     60    return adoptRef(new HandleWorkItem(handle, WTFMove(function), queue));
    6161}
    6262

  • trunk/Source/WTF/wtf/win/WorkItemWin.h

    r181220 r201464  
    3030#include <Windows.h>
    3131#include <functional>
     32#include <wtf/FunctionDispatcher.h>
    3233#include <wtf/RefPtr.h>
    3334#include <wtf/ThreadSafeRefCounted.h>
    … …  
    3940class WorkItemWin : public ThreadSafeRefCounted<WorkItemWin> {
    4041public:
    41     static RefPtr<WorkItemWin> create(std::function<void()>, WorkQueue*);
     42    static RefPtr<WorkItemWin> create(NoncopyableFunction&&, WorkQueue*);
    4243    virtual ~WorkItemWin();
    4344
    44     std::function<void()>& function() { return m_function; }
     45    NoncopyableFunction& function() { return m_function; }
    4546    WorkQueue* queue() const { return m_queue.get(); }
    4647
    4748protected:
    48     WorkItemWin(std::function<void()>, WorkQueue*);
     49    WorkItemWin(NoncopyableFunction&&, WorkQueue*);
    4950
    5051private:
    51     std::function<void()> m_function;
     52    NoncopyableFunction m_function;
    5253    RefPtr<WorkQueue> m_queue;
    5354};
    … …  
    5556class HandleWorkItem : public WorkItemWin {
    5657public:
    57     static RefPtr<HandleWorkItem> createByAdoptingHandle(HANDLE, const std::function<void()>&, WorkQueue*);
     58    static RefPtr<HandleWorkItem> createByAdoptingHandle(HANDLE, NoncopyableFunction&&, WorkQueue*);
    5859    virtual ~HandleWorkItem();
    5960
    … …  
    6263
    6364private:
    64     HandleWorkItem(HANDLE, const std::function<void()>&, WorkQueue*);
     65    HandleWorkItem(HANDLE, NoncopyableFunction&&, WorkQueue*);
    6566
    6667    HANDLE m_handle;

  • trunk/Source/WTF/wtf/win/WorkQueueWin.cpp

    r188415 r201464  
    130130}
    131131
    132 void WorkQueue::dispatch(std::function<void()> function)
     132void WorkQueue::dispatch(NoncopyableFunction&& function)
    133133{
    134134    MutexLocker locker(m_workItemQueueLock);
    135135    ref();
    136     m_workItemQueue.append(WorkItemWin::create(function, this));
     136    m_workItemQueue.append(WorkItemWin::create(WTFMove(function), this));
    137137
    138138    // Spawn a work thread to perform the work we just added. As an optimization, we avoid
    … …  
    150150
    151151    WorkQueue* queue;
    152     std::function<void()> function;
     152    NoncopyableFunction function;
    153153    Mutex timerMutex;
    154154    HANDLE timer;
    … …  
    170170    RefPtr<TimerContext> timerContext = adoptRef(static_cast<TimerContext*>(context));
    171171
    172     timerContext->queue->dispatch(timerContext->function);
     172    timerContext->queue->dispatch(WTFMove(timerContext->function));
    173173
    174174    MutexLocker lock(timerContext->timerMutex);
    … …  
    181181}
    182182
    183 void WorkQueue::dispatchAfter(std::chrono::nanoseconds duration, std::function<void()> function)
     183void WorkQueue::dispatchAfter(std::chrono::nanoseconds duration, NoncopyableFunction&& function)
    184184{
    185185    ASSERT(m_timerQueue);
    … …  
    188188    RefPtr<TimerContext> context = TimerContext::create();
    189189    context->queue = this;
    190     context->function = function;
     190    context->function = WTFMove(function);
    191191
    192192    {

  • trunk/Source/WebKit2/ChangeLog

    r201457 r201464  
     12016-05-27  Chris Dumez  <cdumez@apple.com>
     2
     3        WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables
     4        https://bugs.webkit.org/show_bug.cgi?id=158111
     5
     6        Reviewed by Darin Adler.
     7
     8        WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables.
     9        These are often used cross-thread and copying the captured lambda variables can be
     10        dangerous (e.g. we do not want to copy a String after calling isolatedCopy() upon
     11        capture).
     12
     13        * NetworkProcess/NetworkProcess.cpp:
     14        (WebKit::clearDiskCacheEntries):
     15        * NetworkProcess/cache/NetworkCache.cpp:
     16        (WebKit::NetworkCache::Cache::clear):
     17        * NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:
     18        (WebKit::NetworkCache::runTaskInQueue):
     19        * Platform/IPC/Connection.cpp:
     20        (IPC::Connection::processIncomingMessage):
     21        * UIProcess/Storage/StorageManager.cpp:
     22        (WebKit::StorageManager::getSessionStorageOrigins):
     23        (WebKit::StorageManager::deleteSessionStorageOrigins):
     24        (WebKit::StorageManager::deleteSessionStorageEntriesForOrigins):
     25        (WebKit::StorageManager::getLocalStorageOrigins):
     26        (WebKit::StorageManager::getLocalStorageOriginDetails):
     27        (WebKit::StorageManager::deleteLocalStorageOriginsModifiedSince):
     28        (WebKit::StorageManager::deleteLocalStorageEntriesForOrigins):
     29        * UIProcess/Storage/StorageManager.h:
     30
    1312016-05-27  Alex Christensen  <achristensen@webkit.org>
    232

  • trunk/Source/WebKit2/NetworkProcess/NetworkProcess.cpp

    r201333 r201464  
    422422}
    423423
    424 static void clearDiskCacheEntries(const Vector<SecurityOriginData>& origins, std::function<void ()> completionHandler)
     424static void clearDiskCacheEntries(const Vector<SecurityOriginData>& origins, std::function<void ()>&& completionHandler)
    425425{
    426426#if ENABLE(NETWORK_CACHE)
    … …  
    433433        auto* cacheKeysToDelete = new Vector<NetworkCache::Key>;
    434434
    435         NetworkCache::singleton().traverse([completionHandler, originsToDelete, cacheKeysToDelete](auto* traversalEntry) {
     435        NetworkCache::singleton().traverse([completionHandler = WTFMove(completionHandler), originsToDelete, cacheKeysToDelete](auto* traversalEntry) mutable {
    436436            if (traversalEntry) {
    437437                if (originsToDelete->contains(SecurityOrigin::create(traversalEntry->entry.response().url())))
    … …  
    447447            delete cacheKeysToDelete;
    448448
    449             RunLoop::main().dispatch(completionHandler);
     449            RunLoop::main().dispatch(WTFMove(completionHandler));
    450450            return;
    451451        });

  • trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp

    r201354 r201464  
    633633
    634634    if (!m_storage) {
    635         RunLoop::main().dispatch(completionHandler);
     635        RunLoop::main().dispatch(WTFMove(completionHandler));
    636636        return;
    637637    }

  • trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp

    r199875 r201464  
    7070}
    7171
    72 static inline void runTaskInQueue(std::function<void ()> task, WorkQueue* queue)
     72static inline void runTaskInQueue(NoncopyableFunction&& task, WorkQueue* queue)
    7373{
    7474    if (queue) {
    75         queue->dispatch(task);
     75        queue->dispatch(WTFMove(task));
    7676        return;
    7777    }

  • trunk/Source/WebKit2/Platform/IPC/Connection.cpp

    r201246 r201464  
    682682
    683683        for (auto& callback : m_incomingSyncMessageCallbacks.values())
    684             m_incomingSyncMessageCallbackQueue->dispatch(callback);
     684            m_incomingSyncMessageCallbackQueue->dispatch(WTFMove(callback));
    685685
    686686        m_incomingSyncMessageCallbacks.clear();

  • trunk/Source/WebKit2/UIProcess/Storage/StorageManager.cpp

    r194496 r201464  
    554554}
    555555
    556 void StorageManager::getSessionStorageOrigins(std::function<void (HashSet<RefPtr<WebCore::SecurityOrigin>>&&)> completionHandler)
    557 {
    558     RefPtr<StorageManager> storageManager(this);
    559 
    560     m_queue->dispatch([storageManager, completionHandler] {
     556void StorageManager::getSessionStorageOrigins(std::function<void (HashSet<RefPtr<WebCore::SecurityOrigin>>&&)>&& completionHandler)
     557{
     558    RefPtr<StorageManager> storageManager(this);
     559
     560    m_queue->dispatch([storageManager, completionHandler = WTFMove(completionHandler)]() mutable {
    561561        HashSet<RefPtr<SecurityOrigin>> origins;
    562562
    … …  
    566566        }
    567567
    568         RunLoop::main().dispatch([origins, completionHandler]() mutable {
     568        RunLoop::main().dispatch([origins, completionHandler = WTFMove(completionHandler)]() mutable {
    569569            completionHandler(WTFMove(origins));
    570570        });
    … …  
    572572}
    573573
    574 void StorageManager::deleteSessionStorageOrigins(std::function<void ()> completionHandler)
    575 {
    576     RefPtr<StorageManager> storageManager(this);
    577 
    578     m_queue->dispatch([storageManager, completionHandler] {
     574void StorageManager::deleteSessionStorageOrigins(std::function<void ()>&& completionHandler)
     575{
     576    RefPtr<StorageManager> storageManager(this);
     577
     578    m_queue->dispatch([storageManager, completionHandler = WTFMove(completionHandler)]() mutable {
    579579        for (auto& sessionStorageNamespace : storageManager->m_sessionStorageNamespaces.values())
    580580            sessionStorageNamespace->clearAllStorageAreas();
    581581
    582         RunLoop::main().dispatch(completionHandler);
    583     });
    584 }
    585 
    586 void StorageManager::deleteSessionStorageEntriesForOrigins(const Vector<RefPtr<WebCore::SecurityOrigin>>& origins, std::function<void ()> completionHandler)
     582        RunLoop::main().dispatch(WTFMove(completionHandler));
     583    });
     584}
     585
     586void StorageManager::deleteSessionStorageEntriesForOrigins(const Vector<RefPtr<WebCore::SecurityOrigin>>& origins, std::function<void ()>&& completionHandler)
    587587{
    588588    Vector<RefPtr<WebCore::SecurityOrigin>> copiedOrigins;
    … …  
    593593
    594594    RefPtr<StorageManager> storageManager(this);
    595     m_queue->dispatch([storageManager, copiedOrigins, completionHandler] {
     595    m_queue->dispatch([storageManager, copiedOrigins, completionHandler = WTFMove(completionHandler)]() mutable {
    596596        for (auto& origin : copiedOrigins) {
    597597            for (auto& sessionStorageNamespace : storageManager->m_sessionStorageNamespaces.values())
    … …  
    599599        }
    600600
    601         RunLoop::main().dispatch(completionHandler);
    602     });
    603 }
    604 
    605 void StorageManager::getLocalStorageOrigins(std::function<void (HashSet<RefPtr<WebCore::SecurityOrigin>>&&)> completionHandler)
    606 {
    607     RefPtr<StorageManager> storageManager(this);
    608 
    609     m_queue->dispatch([storageManager, completionHandler] {
     601        RunLoop::main().dispatch(WTFMove(completionHandler));
     602    });
     603}
     604
     605void StorageManager::getLocalStorageOrigins(std::function<void (HashSet<RefPtr<WebCore::SecurityOrigin>>&&)>&& completionHandler)
     606{
     607    RefPtr<StorageManager> storageManager(this);
     608
     609    m_queue->dispatch([storageManager, completionHandler = WTFMove(completionHandler)]() mutable {
    610610        HashSet<RefPtr<SecurityOrigin>> origins;
    611611
    … …  
    618618        }
    619619
    620         RunLoop::main().dispatch([origins, completionHandler]() mutable {
     620        RunLoop::main().dispatch([origins, completionHandler = WTFMove(completionHandler)]() mutable {
    621621            completionHandler(WTFMove(origins));
    622622        });
    … …  
    624624}
    625625
    626 void StorageManager::getLocalStorageOriginDetails(std::function<void (Vector<LocalStorageDatabaseTracker::OriginDetails>)> completionHandler)
    627 {
    628     RefPtr<StorageManager> storageManager(this);
    629 
    630     m_queue->dispatch([storageManager, completionHandler] {
     626void StorageManager::getLocalStorageOriginDetails(std::function<void (Vector<LocalStorageDatabaseTracker::OriginDetails>)>&& completionHandler)
     627{
     628    RefPtr<StorageManager> storageManager(this);
     629
     630    m_queue->dispatch([storageManager, completionHandler = WTFMove(completionHandler)]() mutable {
    631631        auto originDetails = storageManager->m_localStorageDatabaseTracker->originDetails();
    632632
    633         RunLoop::main().dispatch([originDetails, completionHandler]() mutable {
     633        RunLoop::main().dispatch([originDetails, completionHandler = WTFMove(completionHandler)]() mutable {
    634634            completionHandler(WTFMove(originDetails));
    635635        });
    … …  
    653653}
    654654
    655 void StorageManager::deleteLocalStorageOriginsModifiedSince(std::chrono::system_clock::time_point time, std::function<void ()> completionHandler)
    656 {
    657     RefPtr<StorageManager> storageManager(this);
    658 
    659     m_queue->dispatch([storageManager, time, completionHandler] {
     655void StorageManager::deleteLocalStorageOriginsModifiedSince(std::chrono::system_clock::time_point time, std::function<void ()>&& completionHandler)
     656{
     657    RefPtr<StorageManager> storageManager(this);
     658
     659    m_queue->dispatch([storageManager, time, completionHandler = WTFMove(completionHandler)]() mutable {
    660660        auto deletedOrigins = storageManager->m_localStorageDatabaseTracker->deleteDatabasesModifiedSince(time);
    661661
    … …  
    668668            transientLocalStorageNamespace->clearAllStorageAreas();
    669669
    670         RunLoop::main().dispatch(completionHandler);
    671     });
    672 }
    673 
    674 void StorageManager::deleteLocalStorageEntriesForOrigins(const Vector<RefPtr<WebCore::SecurityOrigin>>& origins, std::function<void ()> completionHandler)
     670        RunLoop::main().dispatch(WTFMove(completionHandler));
     671    });
     672}
     673
     674void StorageManager::deleteLocalStorageEntriesForOrigins(const Vector<RefPtr<WebCore::SecurityOrigin>>& origins, std::function<void ()>&& completionHandler)
    675675{
    676676    Vector<RefPtr<WebCore::SecurityOrigin>> copiedOrigins;
    … …  
    681681
    682682    RefPtr<StorageManager> storageManager(this);
    683     m_queue->dispatch([storageManager, copiedOrigins, completionHandler] {
     683    m_queue->dispatch([storageManager, copiedOrigins, completionHandler = WTFMove(completionHandler)]() mutable {
    684684        for (auto& origin : copiedOrigins) {
    685685            for (auto& localStorageNamespace : storageManager->m_localStorageNamespaces.values())
    … …  
    692692        }
    693693
    694         RunLoop::main().dispatch(completionHandler);
     694        RunLoop::main().dispatch(WTFMove(completionHandler));
    695695    });
    696696}

  • trunk/Source/WebKit2/UIProcess/Storage/StorageManager.h

    r197563 r201464  
    6060    void applicationWillTerminate();
    6161
    62     void getSessionStorageOrigins(std::function<void (HashSet<RefPtr<WebCore::SecurityOrigin>>&&)> completionHandler);
    63     void deleteSessionStorageOrigins(std::function<void ()> completionHandler);
    64     void deleteSessionStorageEntriesForOrigins(const Vector<RefPtr<WebCore::SecurityOrigin>>&, std::function<void ()> completionHandler);
     62    void getSessionStorageOrigins(std::function<void (HashSet<RefPtr<WebCore::SecurityOrigin>>&&)>&& completionHandler);
     63    void deleteSessionStorageOrigins(std::function<void ()>&& completionHandler);
     64    void deleteSessionStorageEntriesForOrigins(const Vector<RefPtr<WebCore::SecurityOrigin>>&, std::function<void ()>&& completionHandler);
    6565
    66     void getLocalStorageOrigins(std::function<void (HashSet<RefPtr<WebCore::SecurityOrigin>>&&)> completionHandler);
     66    void getLocalStorageOrigins(std::function<void (HashSet<RefPtr<WebCore::SecurityOrigin>>&&)>&& completionHandler);
    6767    void deleteLocalStorageEntriesForOrigin(const WebCore::SecurityOrigin&);
    6868
    69     void deleteLocalStorageOriginsModifiedSince(std::chrono::system_clock::time_point, std::function<void ()> completionHandler);
    70     void deleteLocalStorageEntriesForOrigins(const Vector<RefPtr<WebCore::SecurityOrigin>>&, std::function<void ()> completionHandler);
     69    void deleteLocalStorageOriginsModifiedSince(std::chrono::system_clock::time_point, std::function<void ()>&& completionHandler);
     70    void deleteLocalStorageEntriesForOrigins(const Vector<RefPtr<WebCore::SecurityOrigin>>&, std::function<void ()>&& completionHandler);
    7171
    72     void getLocalStorageOriginDetails(std::function<void (Vector<LocalStorageDatabaseTracker::OriginDetails>)> completionHandler);
     72    void getLocalStorageOriginDetails(std::function<void (Vector<LocalStorageDatabaseTracker::OriginDetails>)>&& completionHandler);
    7373
    7474private:

  • trunk/Tools/ChangeLog

    r201461 r201464  
     12016-05-27  Chris Dumez  <cdumez@apple.com>
     2
     3        WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables
     4        https://bugs.webkit.org/show_bug.cgi?id=158111
     5
     6        Reviewed by Darin Adler.
     7
     8        WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables.
     9        These are often used cross-thread and copying the captured lambda variables can be
     10        dangerous (e.g. we do not want to copy a String after calling isolatedCopy() upon
     11        capture).
     12
     13        * WebKitTestRunner/TestController.cpp:
     14        (WTR::TestController::decidePolicyForNavigationAction):
     15
    1162016-05-27  Brady Eidson  <beidson@apple.com>
    217

  • trunk/Tools/WebKitTestRunner/TestController.cpp

    r200945 r201464  
    20232023    WKRetainPtr<WKFramePolicyListenerRef> retainedListener { listener };
    20242024    const bool shouldIgnore { m_policyDelegateEnabled && !m_policyDelegatePermissive };
    2025     std::function<void()> decisionFunction = [shouldIgnore, retainedListener]() {
     2025    auto decisionFunction = [shouldIgnore, retainedListener]() {
    20262026        if (shouldIgnore)
    20272027            WKFramePolicyListenerIgnore(retainedListener.get());
    … …  
    20312031
    20322032    if (m_shouldDecideNavigationPolicyAfterDelay)
    2033         RunLoop::main().dispatch(decisionFunction);
     2033        RunLoop::main().dispatch(WTFMove(decisionFunction));
    20342034    else
    20352035        decisionFunction();
Note: See TracChangeset for help on using the changeset viewer.