Bug 55773 - make ServicePoints actually work
Summary: make ServicePoints actually work
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: mission-control (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: Telepathy bugs list
URL:
Whiteboard:
Keywords: patch
Depends on:
Blocks: 54879 59162
  Show dependency treegraph
 
Reported: 2012-10-08 18:45 UTC by Simon McVittie
Modified: 2013-09-05 15:13 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Require GLib 2.32, for GHashTable set APIs (add, contains) (1.21 KB, patch)
2012-10-08 18:46 UTC, Simon McVittie
Details | Splinter Review
McdConnection: build up our sets of emergency numbers over time (6.78 KB, patch)
2012-10-08 18:46 UTC, Simon McVittie
Details | Splinter Review
Add a regression test for service-points (matched by identifier) (11.57 KB, patch)
2012-10-08 18:46 UTC, Simon McVittie
Details | Splinter Review

Description Simon McVittie 2012-10-08 18:45:35 UTC
+++ This bug was initially created as a clone of Bug #54879 +++

On Bug #54879, Jonny noticed that the service-point code in MC wasn't robust against having more than one distinct service point, and was completely untested.

His patch for that simultaneously fixed ServicePoints and switched to non-deprecated APIs and ported them to next, which is unwelcome from the point of view of "make master work first".

Things we need to do:

1) Test and fix service points

2) Switch from deprecated to non-deprecated APIs

3) Make it work on next

This bug is just (1).
Comment 1 Simon McVittie 2012-10-08 18:46:10 UTC
Created attachment 68272 [details] [review]
Require GLib 2.32, for GHashTable set APIs (add,  contains)
Comment 2 Simon McVittie 2012-10-08 18:46:30 UTC
Created attachment 68273 [details] [review]
McdConnection: build up our sets of emergency numbers  over time

To facilitate this, use nicer data structures.
 
Most importantly, this means we don't critical whenever we get more
than one distinct service point, as Jonny noticed while porting to
Telepathy 1.0. It might slightly defeat the intention of the
ServicePointsChanged signal, but in practice the list is probably
never going to shrink, and if it does, it's probably wise to
keep considering its old members to be an emergency number (the effect
that that has is that plugins aren't allowed to influence channel
requests).
 
Also, don't leak the sets of emergency numbers when finalized.
Comment 3 Simon McVittie 2012-10-08 18:46:54 UTC
Created attachment 68274 [details] [review]
Add a regression test for service-points (matched by  identifier)

The side-effect they have within MC is that plugins aren't allowed to
delay or reject channel requests; we ought to test that, really.
Comment 4 Marco Barisione 2013-09-05 13:25:02 UTC
All the patches look fine.
Comment 5 Simon McVittie 2013-09-05 15:13:43 UTC
Fixed in git for 5.15.1, thanks


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.