The branch in URL implements ContactsChangedWithId plus a mechanism to create contacts even if everything fails but we are using ImmortalHandles and have the contact id and handle.

It also adds some TODOs to the places where we could use the "id trick" implemented in ContactManager but we have no way to know the contact id with current spec.

You could add one more thing - an optimization which we can do thanks to this work: if we have ImmortalHandles AND contactsForHandles is called with no features (incl. factory features) AND we have injected ids for all of those handles, satisfy the request without doing the GetContactAttributes call (i.e. no D-Bus traffic!). To avoid the contact build round-trip was Simon's original intention in adding the ids everywhere anyway.

Note that while the above optimization doesn't help any when other features are asked for, as the ContactFactory was only recently added, currently the majority of applications first do a contact request with no features (mostly internally in Channel), and then upgrade the contacts (themselves). With the optimization, we could avoid the first round-trip, while the second one would still be made, bringing this usage pattern to the same level of performance as using the ContactFactory with all of the features.

I have to ask: rather than making all these extensive mods to Channel internals, why not just injectIDs() when you see the ID, and have the queue logic remain as-is (set-based, not HandleIdentifierMap). In that scheme, the contact manager would keep the injected IDs during the queue processing. Ditto for text. No need to change now exclusively for code clarity purposes, though, but:

As the IDs are now in all kinds of queues - the group change queue, the text message queue, the tube contact queue - the IDs get injected only when the queue gets processed, not when we first see them. Therefore, other requests for contacts with the same handles (but without having knowledge of the ID themselves, which, as you've discovered, still happens in multiple occasions) can't use the IDs during that time period. That said, such contact requests aren't very common. Consider for yourself whether this change would be worth it.

One trivial suggestion: as immortal handles imply the ID of a given handle shouldn't change, please warn in injectIDs if we're overwriting an ID we already had for a handle with a different one. This could help demystify bugs where either we or the service confuses handles with each other.

Branch updated with suggested changes.

I rebased the branch to keep a clean history and also to rebase against current upstream/master.

One thing that I am not sure about the impl is whether the methods ConnectionLowlevel::hasContactId/contactId should be public or not. I let them private for now as we can make them public later if someone has a reason or we find a need later.

OK now, except change injectContactsIDs -> injectContactIDs. "Contacts identifiers" is not a proper English plural of "Contact identifier".

I'd perhaps yet add a injectContactIDs overload which only takes a single handle and id for it, and make the plural one use it internally, as you seem to be constructing single-member {handle -> id} maps on multiple occasions.

Fixed the remaining issues and merged upstream. It will be in 0.5.11.