[Bug 27833] TpChannelDispatchOperation high-level methods and CORE feature

[bugzilla-daemon at freedesktop.org]

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

--- Comment #11 from Guillaume Desmottes <guillaume.desmottes at collabora.co.uk> 2010-05-07 02:16:19 PDT ---
(In reply to comment #8)
> > +maybe_set_connection (TpChannelDispatchOperation *self,
> [...]
> > +  g_object_notify ((GObject *) self, "connection");
> > +
> > +  if (g_hash_table_lookup (self->priv->immutable_properties,
> > +        TP_PROP_CHANNEL_DISPATCH_OPERATION_CONNECTION) != NULL)
> > +    return;
> 
> This doesn't look right. How about checking whether the path is the same one we
> already had before notifying? Same for account and possible-handlers.

Hum, in that case should I recreate the priv->connetion as well? Didn't we
already lost if the initial properties were actually wrong?

> > +tp_channel_dispatch_operation_set_property (GObject *object,
> ...
> > +      self->priv->connection = g_value_dup_object (value);
> (etc.)
> 
> This isn't right - the properties are marked as read-only. Also, unless they're
> construct-only, you can overwrite (and leak) an earlier non-NULL pointer.

Right. I removed the setter for those properties.

> > +get_dispatch_operation_prop_cb (TpProxy *proxy,
> 
> The CDO should be invalidated if its D-Bus properties aren't sane.

Done.
Does that include if the properties are not the same as the ones when received
in tp_channel_dispatch_operation_new?

> > +  if (self->priv->preparing_core)
> > +    return;   /* already running */
> 
> You don't seem to ever set this to true?

Oooops. Fixed.

> > +   * TpChannelDispatchOperation::channel-lost: (skip)
> 
> Why do you (skip) this? It seems just as useful to Python/Javascript.

Copy/paste mistake. Fixed.

> > + * tp_channel_dispatch_operation_handle_with_async:
> ...
> > + * If successful, this method will cause the #TpProxy::invalidated signal
> > + * to be emitted.
> 
> Specify which code it'll be? (TP_DBUS_ERROR_OBJECT_REMOVED, I think)

Indeed. Done.

> > +  g_assert (!tp_strdiff (tp_proxy_get_object_path (channel),
> > +        tp_proxy_get_object_path (test->text_chan_2)));
> 
> Better as g_assert_cmpstr (X, !=, Y); for suitable X and Y.

done.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.