Bug 42932 - Call: Content.Removed has not reason, but Call.ContentRemoved has one
Summary: Call: Content.Removed has not reason, but Call.ContentRemoved has one
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-spec (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
Whiteboard: Call, review+
Keywords: patch
Depends on:
Reported: 2011-11-14 13:59 UTC by Olivier Crête
Modified: 2011-12-23 08:33 UTC (History)
0 users

See Also:
i915 platform:
i915 features:

Just remove Content.Removed, it's pointlessly duplicated (1.53 KB, patch)
2011-12-14 13:23 UTC, Olivier Crête
Details | Splinter Review

Description Olivier Crête 2011-11-14 13:59:52 UTC
This signal is duplicated. I'd just remove the one on Content.

Otherwise, I would add a reason on Content.Removed.
Comment 1 Olivier Crête 2011-12-14 13:23:30 UTC
Created attachment 54435 [details] [review]
Just remove Content.Removed, it's pointlessly duplicated

Lets just remove the duplicated signal
Comment 2 Simon McVittie 2011-12-15 03:10:10 UTC
(In reply to comment #1)
> Lets just remove the duplicated signal

For the record, the reason for the duplicated signal was that if you only have a client-side proxy object for a Content and not for its parent Channel, you can't tell when the Content has gone away. I'd be happy for that to be resolved by "don't do that, then", though.

If the Content is implicitly considered to have gone away when the Channel closes (which I suspect it ought to) or when the Connection disconnects (which it certainly should), then TpCallContent needs to get a TpCallChannel and a TpConnection in its constructor anyway, to be able to connect to the appropriate invalidation signals.
Comment 3 Olivier Crête 2011-12-15 20:16:15 UTC
Currently, in tp-glib, the TpCallContent proxy is created and destroyed by TpCallChannel.. but I see the point.. That said, it's probably easier to add signals later than remove them.
Comment 4 Xavier Claessens 2011-12-16 04:19:30 UTC
Note that TpCallContent's constructor is internal API, used only by TpCallChannel. It already gives the TpConnection but not the TpCallChannel. But that can be changed even without changing API. So if connection or channel goes away, and user keeps itself a ref to the content (why would it do that?) then it currently rely on the fact that the CM-side will also remove all contents from the bus to get the proxy invalidated (which TpBaseCallChannel does).

As a general rule, I'm in favor to remove things that we can add back later without breaking API rather than keeping useless stuff that we can't remover later.
Comment 5 Jonny Lamb 2011-12-23 01:22:14 UTC
Simon brings a good point to the table, but I say go for it.

Merge away.
Comment 6 Olivier Crête 2011-12-23 08:33:01 UTC
commit 554e9a3c20cb3564e72bd3aa4e94e58977b9292f
Author: Olivier Crête <olivier.crete@collabora.com>
Date:   Wed Dec 14 16:21:28 2011 -0500

    Remove the duplicated Removed() signal on the Call.Content

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.