Bug 66085

+ /* 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.