As I said on bug 38568: > in retrospect, I think the console API should have been on a channel So here comes a branch which makes that change. It's quite disappointing how much boilerplate you need to implement a TpChannelManager. Pretty much the only code in there specific to the console is the call to g_object_new (GABBLE_TYPE_CONSOLE_CHANNEL, ...). I also fixed some bugs I found in the plugin loader. Some of this code has been lying around a branch for ages.
+ /* Not reffing this: the connection owns all channel managers, so it + * must outlive us. Taking a reference leads to a cycle. + */ + self->plugin_connection = g_value_get_object (value); Weak ref, or drop it on DISCONNECTED state? +gabble_console_channel_manager_dispose ( + GObject *object) +{ + GabbleConsoleChannelManager *self = GABBLE_CONSOLE_CHANNEL_MANAGER (object); + TpBaseChannel *channel; + + while ((channel = g_queue_peek_head (&self->console_channels)) != NULL) + { + tp_base_channel_close (channel); _destroy instead? (Not that it really matters for these particular channels.) + # We only prepare the channel so that ::invalidated will be emitted + # when it closes. Is this really necessary? I would expect that channels would invalidate anyway.
(In reply to comment #0) > It's quite disappointing how much boilerplate you need to implement a > TpChannelManager. I think we had vague plans to implement base classes for "channel manager based on a map { handle => channel }" and "channel manager based on a list of anonymous channels" at one point. (Neither of those works beyond trivial cases, though - you're getting the worst case here, because your channel manager is really trivial.)
(In reply to comment #1) > + /* Not reffing this: the connection owns all channel managers, so it > + * must outlive us. Taking a reference leads to a cycle. > + */ > + self->plugin_connection = g_value_get_object (value); > > Weak ref, or drop it on DISCONNECTED state? Pushed b88fa16 which makes it use a weak ref. > +gabble_console_channel_manager_dispose ( > + GObject *object) > +{ > + GabbleConsoleChannelManager *self = GABBLE_CONSOLE_CHANNEL_MANAGER > (object); > + TpBaseChannel *channel; > + > + while ((channel = g_queue_peek_head (&self->console_channels)) != NULL) > + { > + tp_base_channel_close (channel); > > _destroy instead? (Not that it really matters for these particular channels.) There's no such method. (tp_base_channel_destroyed() is to be used by the subclass itself.) > + # We only prepare the channel so that ::invalidated will be emitted > + # when it closes. > > Is this really necessary? I would expect that channels would invalidate > anyway. You're right. Pushed 37eef77 which does away with that.
I've also pushed 1108a8a which will avoid confusing build problems as and when more plugins are added.
> +extern int debug; Extern symbols in plugins with short names seem like a recipe for disaster. I changed it to gabble_console_debug. Also, there was a missing import in the regression test (assertNotEquals). Other than that, all good! Fixed in git for 0.19.0.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.