Changeset 173749 in webkit


Ignore:
Timestamp:
Sep 18, 2014 11:51:24 PM (10 years ago)
Author:
Carlos Garcia Campos
Message:

[GTK] Dot not allow to create delete-on-destroy GMainLoopSources
​https://bugs.webkit.org/show_bug.cgi?id=136923

Reviewed by Gustavo Noronha Silva.

Source/WebCore:

  • platform/gtk/GtkDragAndDropHelper.cpp:

(WebCore::GtkDragAndDropHelper::handleDragLeave): Use GMainLoopSource::scheduleAndDeleteOnDestroy().

Source/WebKit2:

  • Platform/gtk/WorkQueueGtk.cpp:

(WorkQueue::dispatch): Use GMainLoopSource::scheduleAndDeleteOnDestroy().
(WorkQueue::dispatchAfter): Use GMainLoopSource::scheduleAfterDelayAndDeleteOnDestroy().

Source/WTF:

We have several asserts to ensure that delete-on-destroy sources
are not misused, like not scheduling socket sources on a
delete-on-destroy GMainLoopSource or not allowing to cancel them
before they have been dispatched. It's better to ensure all those
things at compile time, using static methods to schedule sources
creating a delete-on-destroy GMainLoopSource that is not returned
to the user.

  • wtf/gobject/GMainLoopSource.cpp:

(WTF::GMainLoopSource::create): Private static method to create a
delete-on-destroy GMainLoopSource.
(WTF::GMainLoopSource::cancelWithoutLocking): Return early in case
of delete-on-destroy source, since they can't be cancelled.
(WTF::GMainLoopSource::schedule): Remove assertion to ensure
socket sources are not scheduled on a delete-on-destroy GMainLoopSource.
(WTF::GMainLoopSource::scheduleAndDeleteOnDestroy):
(WTF::GMainLoopSource::scheduleAfterDelayAndDeleteOnDestroy):
(WTF::GMainLoopSource::createAndDeleteOnDestroy): Deleted.

  • wtf/gobject/GMainLoopSource.h:
  • wtf/gtk/MainThreadGtk.cpp:

(WTF::scheduleDispatchFunctionsOnMainThread): Use GMainLoopSource::scheduleAndDeleteOnDestroy()

  • wtf/gtk/RunLoopGtk.cpp:

(WTF::RunLoop::wakeUp): Ditto.

Tools:

  • TestWebKitAPI/Tests/WTF/gobject/GMainLoopSource.cpp:

(TestWebKitAPI::TEST): Use the new API that doesn't allow to use
the source.

Location:
trunk
Files:
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WTF/ChangeLog

    r173736 r173749  
     12014-09-18  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        [GTK] Dot not allow to create delete-on-destroy GMainLoopSources
     4        https://bugs.webkit.org/show_bug.cgi?id=136923
     5
     6        Reviewed by Gustavo Noronha Silva.
     7
     8        We have several asserts to ensure that delete-on-destroy sources
     9        are not misused, like not scheduling socket sources on a
     10        delete-on-destroy GMainLoopSource or not allowing to cancel them
     11        before they have been dispatched. It's better to ensure all those
     12        things at compile time, using static methods to schedule sources
     13        creating a delete-on-destroy GMainLoopSource that is not returned
     14        to the user.
     15
     16        * wtf/gobject/GMainLoopSource.cpp:
     17        (WTF::GMainLoopSource::create): Private static method to create a
     18        delete-on-destroy GMainLoopSource.
     19        (WTF::GMainLoopSource::cancelWithoutLocking): Return early in case
     20        of delete-on-destroy source, since they can't be cancelled.
     21        (WTF::GMainLoopSource::schedule): Remove assertion to ensure
     22        socket sources are not scheduled on a delete-on-destroy GMainLoopSource.
     23        (WTF::GMainLoopSource::scheduleAndDeleteOnDestroy):
     24        (WTF::GMainLoopSource::scheduleAfterDelayAndDeleteOnDestroy):
     25        (WTF::GMainLoopSource::createAndDeleteOnDestroy): Deleted.
     26        * wtf/gobject/GMainLoopSource.h:
     27        * wtf/gtk/MainThreadGtk.cpp:
     28        (WTF::scheduleDispatchFunctionsOnMainThread): Use GMainLoopSource::scheduleAndDeleteOnDestroy()
     29        * wtf/gtk/RunLoopGtk.cpp:
     30        (WTF::RunLoop::wakeUp): Ditto.
     31
    1322014-09-18  Joseph Pecoraro  <pecoraro@apple.com>
    233

  • trunk/Source/WTF/wtf/gobject/GMainLoopSource.cpp

    r173720 r173749  
    3434namespace WTF {
    3535
    36 GMainLoopSource& GMainLoopSource::createAndDeleteOnDestroy()
     36GMainLoopSource& GMainLoopSource::create()
    3737{
    3838    return *new GMainLoopSource(DeleteOnDestroy);
    … …  
    7777void GMainLoopSource::cancelWithoutLocking()
    7878{
     79    // Delete-on-destroy GMainLoopSource objects can't be cancelled.
     80    if (m_deleteOnDestroy == DeleteOnDestroy)
     81        return;
     82
    7983    // A valid context should only be present if GMainLoopSource is in the Scheduled or Dispatching state.
    8084    ASSERT(!m_context.source || m_status == Scheduled || m_status == Dispatching);
    8185    // The general cancellable object should only be present if we're currently dispatching this GMainLoopSource.
    8286    ASSERT(!m_cancellable || m_status == Dispatching);
    83     // Delete-on-destroy GMainLoopSource objects can only be cancelled when there's callback either scheduled
    84     // or in the middle of dispatch. At that point cancellation will have no effect.
    85     ASSERT(m_deleteOnDestroy != DeleteOnDestroy || (m_status == Ready && !m_context.source));
    8687
    8788    m_status = Ready;
    … …  
    154155    cancelWithoutLocking();
    155156
    156     // Don't allow scheduling GIOCondition callbacks on delete-on-destroy GMainLoopSources.
    157     ASSERT(m_deleteOnDestroy == DoNotDeleteOnDestroy);
    158 
    159157    ASSERT(!m_context.source);
    160158    GCancellable* socketCancellable = g_cancellable_new();
    … …  
    258256    };
    259257    scheduleTimeoutSource(name, reinterpret_cast<GSourceFunc>(boolSourceCallback), priority, context);
     258}
     259
     260void GMainLoopSource::scheduleAndDeleteOnDestroy(const char* name, std::function<void()> function, int priority, std::function<void()> destroyFunction, GMainContext* context)
     261{
     262    create().schedule(name, function, priority, destroyFunction, context);
     263}
     264
     265void GMainLoopSource::scheduleAndDeleteOnDestroy(const char* name, std::function<bool()> function, int priority, std::function<void()> destroyFunction, GMainContext* context)
     266{
     267    create().schedule(name, function, priority, destroyFunction, context);
     268}
     269
     270void GMainLoopSource::scheduleAfterDelayAndDeleteOnDestroy(const char* name, std::function<void()> function, std::chrono::milliseconds delay, int priority, std::function<void()> destroyFunction, GMainContext* context)
     271{
     272    create().scheduleAfterDelay(name, function, delay, priority, destroyFunction, context);
     273}
     274
     275void GMainLoopSource::scheduleAfterDelayAndDeleteOnDestroy(const char* name, std::function<bool()> function, std::chrono::milliseconds delay, int priority, std::function<void()> destroyFunction, GMainContext* context)
     276{
     277    create().scheduleAfterDelay(name, function, delay, priority, destroyFunction, context);
     278}
     279
     280void GMainLoopSource::scheduleAfterDelayAndDeleteOnDestroy(const char* name, std::function<void()> function, std::chrono::seconds delay, int priority, std::function<void()> destroyFunction, GMainContext* context)
     281{
     282    create().scheduleAfterDelay(name, function, delay, priority, destroyFunction, context);
     283}
     284
     285void GMainLoopSource::scheduleAfterDelayAndDeleteOnDestroy(const char* name, std::function<bool()> function, std::chrono::seconds delay, int priority, std::function<void()> destroyFunction, GMainContext* context)
     286{
     287    create().scheduleAfterDelay(name, function, delay, priority, destroyFunction, context);
    260288}
    261289

  • trunk/Source/WTF/wtf/gobject/GMainLoopSource.h

    r173720 r173749  
    4141    WTF_MAKE_FAST_ALLOCATED;
    4242public:
    43     static GMainLoopSource& createAndDeleteOnDestroy();
    44 
    4543    WTF_EXPORT_PRIVATE GMainLoopSource();
    4644    WTF_EXPORT_PRIVATE ~GMainLoopSource();
    … …  
    6159    WTF_EXPORT_PRIVATE void cancel();
    6260
     61    static void scheduleAndDeleteOnDestroy(const char* name, std::function<void()>, int priority = G_PRIORITY_DEFAULT, std::function<void()> destroyFunction = nullptr, GMainContext* = nullptr);
     62    static void scheduleAndDeleteOnDestroy(const char* name, std::function<bool()>, int priority = G_PRIORITY_DEFAULT, std::function<void()> destroyFunction = nullptr, GMainContext* = nullptr);
     63    static void scheduleAfterDelayAndDeleteOnDestroy(const char* name, std::function<void()>, std::chrono::milliseconds, int priority = G_PRIORITY_DEFAULT, std::function<void()> destroyFunction = nullptr, GMainContext* = nullptr);
     64    static void scheduleAfterDelayAndDeleteOnDestroy(const char* name, std::function<bool()>, std::chrono::milliseconds, int priority = G_PRIORITY_DEFAULT, std::function<void()> destroyFunction = nullptr, GMainContext* = nullptr);
     65    static void scheduleAfterDelayAndDeleteOnDestroy(const char* name, std::function<void()>, std::chrono::seconds, int priority = G_PRIORITY_DEFAULT, std::function<void()> destroyFunction = nullptr, GMainContext* = nullptr);
     66    static void scheduleAfterDelayAndDeleteOnDestroy(const char* name, std::function<bool()>, std::chrono::seconds, int priority = G_PRIORITY_DEFAULT, std::function<void()> destroyFunction = nullptr, GMainContext* = nullptr);
     67
    6368private:
     69    static GMainLoopSource& create();
     70
    6471    enum DeleteOnDestroyType { DeleteOnDestroy, DoNotDeleteOnDestroy };
    6572    GMainLoopSource(DeleteOnDestroyType);

  • trunk/Source/WTF/wtf/gtk/MainThreadGtk.cpp

    r166916 r173749  
    4141void scheduleDispatchFunctionsOnMainThread()
    4242{
    43     GMainLoopSource::createAndDeleteOnDestroy().schedule("[WebKit] dispatchFunctionsFromMainThread", std::function<void()>(dispatchFunctionsFromMainThread));
     43    GMainLoopSource::scheduleAndDeleteOnDestroy("[WebKit] dispatchFunctionsFromMainThread", std::function<void()>(dispatchFunctionsFromMainThread));
    4444}
    4545

  • trunk/Source/WTF/wtf/gtk/RunLoopGtk.cpp

    r169373 r173749  
    101101{
    102102    RefPtr<RunLoop> runLoop(this);
    103     GMainLoopSource::createAndDeleteOnDestroy().schedule("[WebKit] RunLoop work", std::function<void()>([runLoop] {
     103    GMainLoopSource::scheduleAndDeleteOnDestroy("[WebKit] RunLoop work", std::function<void()>([runLoop] {
    104104        runLoop->performWork();
    105105    }), G_PRIORITY_DEFAULT, nullptr, m_runLoopContext.get());

  • trunk/Source/WebCore/ChangeLog

    r173746 r173749  
     12014-09-18  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        [GTK] Dot not allow to create delete-on-destroy GMainLoopSources
     4        https://bugs.webkit.org/show_bug.cgi?id=136923
     5
     6        Reviewed by Gustavo Noronha Silva.
     7
     8        * platform/gtk/GtkDragAndDropHelper.cpp:
     9        (WebCore::GtkDragAndDropHelper::handleDragLeave): Use GMainLoopSource::scheduleAndDeleteOnDestroy().
     10
    1112014-09-18  Roger Fong  <roger_fong@apple.com>
    212

  • trunk/Source/WebCore/platform/gtk/GtkDragAndDropHelper.cpp

    r166916 r173749  
    100100    // those for drag-drop, so schedule them to happen asynchronously here.
    101101    context->exitedCallback = exitedCallback;
    102     GMainLoopSource::createAndDeleteOnDestroy().schedule("[WebKit] handleDragLeaveLater", std::function<void()>(std::bind(&GtkDragAndDropHelper::handleDragLeaveLater, this, context)));
     102    GMainLoopSource::scheduleAndDeleteOnDestroy("[WebKit] handleDragLeaveLater", std::function<void()>(std::bind(&GtkDragAndDropHelper::handleDragLeaveLater, this, context)));
    103103}
    104104

  • trunk/Source/WebKit2/ChangeLog

    r173748 r173749  
     12014-09-18  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        [GTK] Dot not allow to create delete-on-destroy GMainLoopSources
     4        https://bugs.webkit.org/show_bug.cgi?id=136923
     5
     6        Reviewed by Gustavo Noronha Silva.
     7
     8        * Platform/gtk/WorkQueueGtk.cpp:
     9        (WorkQueue::dispatch): Use GMainLoopSource::scheduleAndDeleteOnDestroy().
     10        (WorkQueue::dispatchAfter): Use GMainLoopSource::scheduleAfterDelayAndDeleteOnDestroy().
     11
    1122014-09-18  Ryuan Choi  <ryuan.choi@gmail.com>
    213

  • trunk/Source/WebKit2/Platform/gtk/WorkQueueGtk.cpp

    r170774 r173749  
    109109{
    110110    ref();
    111     GMainLoopSource::createAndDeleteOnDestroy().schedule("[WebKit] WorkQueue::dispatch", WTF::move(function), G_PRIORITY_DEFAULT,
     111    GMainLoopSource::scheduleAndDeleteOnDestroy("[WebKit] WorkQueue::dispatch", WTF::move(function), G_PRIORITY_DEFAULT,
    112112        [this] { deref(); }, m_eventContext.get());
    113113}
    … …  
    116116{
    117117    ref();
    118     GMainLoopSource::createAndDeleteOnDestroy().scheduleAfterDelay("[WebKit] WorkQueue::dispatchAfter", WTF::move(function),
     118    GMainLoopSource::scheduleAfterDelayAndDeleteOnDestroy("[WebKit] WorkQueue::dispatchAfter", WTF::move(function),
    119119        std::chrono::duration_cast<std::chrono::milliseconds>(duration), G_PRIORITY_DEFAULT, [this] { deref(); }, m_eventContext.get());
    120120}

  • trunk/Tools/ChangeLog

    r173725 r173749  
     12014-09-18  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        [GTK] Dot not allow to create delete-on-destroy GMainLoopSources
     4        https://bugs.webkit.org/show_bug.cgi?id=136923
     5
     6        Reviewed by Gustavo Noronha Silva.
     7
     8        * TestWebKitAPI/Tests/WTF/gobject/GMainLoopSource.cpp:
     9        (TestWebKitAPI::TEST): Use the new API that doesn't allow to use
     10        the source.
     11
    1122014-09-18  Csaba Osztrogonác  <ossy@webkit.org>
    213

  • trunk/Tools/TestWebKitAPI/Tests/WTF/gobject/GMainLoopSource.cpp

    r173720 r173749  
    425425    // Testing the delete-on-destroy sources is very limited. There's no good way
    426426    // of testing that the GMainLoopSource objects are deleted when their GSource
    427     // is destroyed, and the socket callbacks shouldn't be scheduled on these types
    428     // of GMainLoopSources (as we aggressively assert to prevent that).
     427    // is destroyed.
    429428
    430429    struct TestingContext {
    … …  
    437436        TestingContext context;
    438437
    439         // We take a reference to the GMainLoopSource just to perform additional
    440         // tests on its status. We shouldn't use the reference after the main loop
    441         // exists since at that point the GMainLoopSource will be destroyed and
    442         // the reference pointing to an invalid piece of memory.
    443         GMainLoopSource& source = GMainLoopSource::createAndDeleteOnDestroy();
    444         EXPECT_TRUE(!source.isActive());
    445         source.schedule("[Test] DeleteOnDestroy",
    446             [&] {
    447                 EXPECT_TRUE(source.isActive() && !source.isScheduled());
     438        GMainLoopSource::scheduleAndDeleteOnDestroy("[Test] DeleteOnDestroy",
     439            [&] {
    448440                context.callbackCallCount++;
    449441            }, G_PRIORITY_DEFAULT,
    450442            [&] {
    451                 EXPECT_TRUE(!source.isActive());
    452443                EXPECT_FALSE(context.destroyCallbackCalled);
    453444                context.destroyCallbackCalled = true;
    454445            });
    455         EXPECT_TRUE(source.isScheduled());
    456446
    457447        context.test.delayedFinish();
    … …  
    464454        TestingContext context;
    465455
    466         // As in the previous scope, we need a reference to the GMainLoopSource.
    467         GMainLoopSource& source = GMainLoopSource::createAndDeleteOnDestroy();
    468         EXPECT_TRUE(!source.isActive());
    469         source.schedule("[Test] DeleteOnDestroy",
     456        GMainLoopSource::scheduleAndDeleteOnDestroy("[Test] DeleteOnDestroy",
    470457            std::function<bool ()>([&] {
    471                 EXPECT_TRUE(source.isActive() && !source.isScheduled());
    472458                context.callbackCallCount++;
    473459                return context.callbackCallCount != 3;
    474460            }), G_PRIORITY_DEFAULT,
    475461            [&] {
    476                 EXPECT_TRUE(!source.isActive());
    477462                EXPECT_FALSE(context.destroyCallbackCalled);
    478463                context.destroyCallbackCalled = true;
    479464            });
    480         EXPECT_TRUE(source.isScheduled());
    481465
    482466        context.test.delayedFinish();
Note: See TracChangeset for help on using the changeset viewer.