Bug 136923 - [GTK] Dot not allow to create delete-on-destroy GMainLoopSources
Summary: [GTK] Dot not allow to create delete-on-destroy GMainLoopSources
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2014-09-18 08:09 PDT by Carlos Garcia Campos
Modified: 2014-09-18 23:51 PDT (History)
2 users (show)

See Also:


Attachments
Patch (16.40 KB, patch)
2014-09-18 08:17 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Patch (16.39 KB, patch)
2014-09-18 08:51 PDT, Carlos Garcia Campos
gustavo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2014-09-18 08:09:37 PDT
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 would be better if we can ensure all those things at compile time, so instead of having  static method to create a delete-on-destroy source with all the API available, we could add static methods to schedule sources creating a delete-on-destroy GMainLoopSource that is not returned to the user.
Comment 1 Carlos Garcia Campos 2014-09-18 08:17:06 PDT
Created attachment 238309 [details]
Patch
Comment 2 Carlos Garcia Campos 2014-09-18 08:51:32 PDT
Created attachment 238310 [details]
Patch
Comment 3 Gustavo Noronha (kov) 2014-09-18 13:08:47 PDT
Comment on attachment 238310 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=238310&action=review

> Source/WTF/wtf/gobject/GMainLoopSource.cpp:36
> -GMainLoopSource& GMainLoopSource::createAndDeleteOnDestroy()
> +GMainLoopSource& GMainLoopSource::create()

I think this name should be kept, otherwise it may lead people to believe that this is the default or only way of creating, like on other objects, it gets confusing.

> Source/WTF/wtf/gobject/GMainLoopSource.cpp:-85
> -    // Delete-on-destroy GMainLoopSource objects can only be cancelled when there's callback either scheduled
> -    // or in the middle of dispatch. At that point cancellation will have no effect.
> -    ASSERT(m_deleteOnDestroy != DeleteOnDestroy || (m_status == Ready && !m_context.source));

Heh, this was a bit weird.
Comment 4 Carlos Garcia Campos 2014-09-18 23:34:04 PDT
(In reply to comment #3)
> (From update of attachment 238310 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=238310&action=review
> 
> > Source/WTF/wtf/gobject/GMainLoopSource.cpp:36
> > -GMainLoopSource& GMainLoopSource::createAndDeleteOnDestroy()
> > +GMainLoopSource& GMainLoopSource::create()
> 
> I think this name should be kept, otherwise it may lead people to believe that this is the default or only way of creating, like on other objects, it gets confusing.

But I made it private for that reason, so it can only be created inside the object.

> > Source/WTF/wtf/gobject/GMainLoopSource.cpp:-85
> > -    // Delete-on-destroy GMainLoopSource objects can only be cancelled when there's callback either scheduled
> > -    // or in the middle of dispatch. At that point cancellation will have no effect.
> > -    ASSERT(m_deleteOnDestroy != DeleteOnDestroy || (m_status == Ready && !m_context.source));
> 
> Heh, this was a bit weird.

Yes.
Comment 5 Carlos Garcia Campos 2014-09-18 23:51:40 PDT
Committed r173749: <http://trac.webkit.org/changeset/173749>