[Bug 69430] Make NewChannels, etc., singular?

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=69430

--- Comment #45 from Simon McVittie <simon.mcvittie at collabora.co.uk> ---
> - tp_tests_add_channel_to_ptr_array

Can that function be removed entirely?

+ g_object_notify ((GObject *) self, "channel");
+
+ if (g_hash_table_lookup (self->priv->immutable_properties,
+ TP_PROP_CHANNEL_DISPATCH_OPERATION_CHANNEL) == NULL)
+ {
+ g_hash_table_insert (self->priv->immutable_properties,
+ g_strdup (TP_PROP_CHANNEL_DISPATCH_OPERATION_CHANNEL),
+ tp_g_value_slice_new_boxed (DBUS_TYPE_G_OBJECT_PATH, path));
+ }
+
+ if (g_hash_table_lookup (self->priv->immutable_properties,
+ TP_PROP_CHANNEL_DISPATCH_OPERATION_CHANNEL_PROPERTIES) == NULL)
+ {
+ g_hash_table_insert (self->priv->immutable_properties,
+ g_strdup (TP_PROP_CHANNEL_DISPATCH_OPERATION_CHANNEL_PROPERTIES),
+ tp_g_value_slice_new_boxed (
+ TP_HASH_TYPE_STRING_VARIANT_MAP, properties));
+ }
+}

Shouldn't we be updating internal state *before* emitting signals?

> Handler.HandleWith::User_Action_Time: use a signed type

I was surprised how small this patch is, but the high-level API is signed
already, so it seems correct.

> RequestableChannelClasses has been moved to Connection

I think you still need to update the client API to match?

(In reply to comment #38)
> I didn't change the TpChannelManager::new-channels: API yet. How would you
> re-design it?

As I'm sure you already know:

The high-level API should be tp_channel_manager_emit_new_channel(), possibly
replacing the GSList with a different data structure.

tp_channel_manager_emit_new_channels() should just go away, if it hasn't
already.

::new-channels should be renamed to ::new-channel and have signature:

new-channel (TpChannelManager *self, TpExportableChannel *channel,
    something<something> requests)

The thing I'm not sure about, and probably the thing you're not sure about, is
the type of the requests. We should do something that is introspectable, if
that isn't hideously painful.

#gnome-hackers suggests that GList<TpChannelManagerRequest *>, where
TpChannelManagerRequest is opaque, would be the best thing to have in a signal
- apparently GList<boxed> and GList<GObject> have regression test coverage,
which is a good way to make sure they still work.

How about this?

* Turn TpBaseConnection's ChannelRequest into TpChannelManagerRequest

* Make it refcounted, or maybe even make it into a trivial GObject
  (there's no reason why a channel manager should need to ref it,
  but introspected code will do it anyway)

* TpChannelManagerRequestFunc's 2nd argument becomes a TpChannelManagerRequest*

* tp_channel_manager_create_channel, ..._ensure_channel,
  _request_already_satisfied, _emit_request_failed[_printf], and the
  corresponding vfuncs should take that type too

* tp_channel_manager_request_channel (and its vfunc) should be removed
  altogether

* Write some silly Python that subclasses TpChannelManager and
  TpExportableChannel (you might need to temporarily remove
  "TpExportableChannel requires TpSvcChannel")

* Verify that the chosen type for ::new-channel can work

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.