Bug 24474 - [fixed 5.2, pending 5.3] Observers not invoked for channels requested "behind MC's back"
Summary: [fixed 5.2, pending 5.3] Observers not invoked for channels requested "behind...
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: mission-control (show other bugs)
Version: unspecified
Hardware: Other All
: high major
Assignee: Simon McVittie
QA Contact: Telepathy bugs list
URL: http://git.collabora.co.uk/?p=user/sm...
Whiteboard:
Keywords: patch
Depends on: 24120
Blocks:
  Show dependency treegraph
 
Reported: 2009-10-12 06:31 UTC by Simon McVittie
Modified: 2009-11-02 06:57 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Simon McVittie 2009-10-12 06:31:30 UTC
MC doesn't currently call ObserveChannels for channels that were requested by a direct call to Connection methods - only for channels that either were requested via MC, or were not requested (i.e. are incoming). This isn't what was intended by telepathy-spec, and significantly weakens the guarantee that observers see all new channels.
Comment 1 Simon McVittie 2009-10-12 08:43:58 UTC
52-always-observe-new-channels (referenced as the URL for this bug) fixes this bug for 5.2.

always-observe-new-channels is a reimplementation for master; I do not recommend merging this one.

always-observe-after-refactor is a reimplementation on top of dispatcher-refactor-part4.
Comment 2 Simon McVittie 2009-10-13 12:33:30 UTC
Fixed in 5.2. I recommend fixing this in 5.3 by merging dispatcher-refactor-part4 first: accordingly, blocking by Bug #24120.
Comment 3 Sjoerd Simons 2009-11-02 04:00:46 UTC
+    /* channels that we will only observe should not need approval - so at
+     * least one must be false */
+    g_return_val_if_fail (!(observe_only && needs_approval), NULL);

Are you trying to say observe_only => !needs_approval, because the assertion is a non-obvious way of writing that and the comment doesn't make it very clear either


Assuming this still ignores channels on connections that were established behind mc5's back, the branch looks fine. (apart from probably an improved comment)
Comment 4 Simon McVittie 2009-11-02 06:46:51 UTC
Is this better?

     /* possible-handlers is only allowed to be NULL if we're only observing */
     g_return_val_if_fail (possible_handlers != NULL || observe_only, NULL);
-    /* channels that we will only observe should not need approval - so at
-     * least one must be false */
-    g_return_val_if_fail (!(observe_only && needs_approval), NULL);
+
+    /* If we're only observing, then the channels were requested "behind MC's
+     * back", so they can't need approval (i.e. observe_only implies
+     * !needs_approval) */
+    g_return_val_if_fail (!observe_only || !needs_approval, NULL);

Comment 5 Simon McVittie 2009-11-02 06:57:16 UTC
Reviewed by Sjoerd on IRC and fixed in git.


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.