1227973 - Dynamic registration of Protocols

Status: ASSIGNED

(Chat Core :: General, enhancement)

Description

[Martin Giger [:freaktechnik]]

Attachment: dynamic-accounts-v1.patch

I've been developing a protocol extension that's bootstrapped (actually SDK based, but that needs more work elsewhere) and decided to make it possible to register and unregister protocols for bootstrapped extensions.

This patch adds observers for protocol registrations and unregistrations in the relevant places. It swaps out the previous protocol and protocol account implementation of an imAccount in both cases and updates the accounts status (connecting it if autoconnect is enabled etc.).

I haven't got around writing tests for it yet, but I promise to do that next. Current tests passed for me.
I've also attached a minimal bootstrapped protocol, based on the jsTestProtocol.

There are a few bits that I'm not completely sure of:
  - It might be nicer to emit a custom observer notification from imCore when protocols are registered and unregistered.
  - prpl-jabber currently gets picked up by the dynamic registration, not sure why.
  - There is currently an error with the /protocol prpl, which I think is somehow related to the purple protocols, not sure there. It doesn't seem to affect any functionality (I don't build the purple extension).

Comment 1

[Martin Giger [:freaktechnik]]

Attachment: test-provider.xpi

Comment 2

[Martin Giger [:freaktechnik]]

Soi the prpl-jabber thing might not be an issue, or it might be one. It's from the forcePurle option for prpl-jabber and I don't have libpurple built. But that doesn't mean it won't break libpurple jabber (I'll have to try that).

Comment 3

[Martin Giger [:freaktechnik]]

I've noticed one issue with this patch: it does not update the File menu when a protocol is removed. Though that's probably in theory not a related bug.

Comment 4

[Martin Giger [:freaktechnik]]

Attachment: dynamic-accounts-v2.patch

I restructured the code so it mostly lives in a method of imAccount called "_swapProtocol". This method is also used in the contructor of the imAccount, since mostly the same things are run.

I would like to eliminate the third arguemnt, however with the current handling of how the protocol is removed it is not guaranteed, that the protocol has already been removed from the cache in imCore. One way to do that would be to check if a protocol is not unloaded before returning it in the imCore. I'm not sure if there's an efficient way to do that.

Further it'd be nice if accounts could be properly disconnected when a protocol is removed, given the prplAccount hasn't been nuked yet. However this would require waiting for the prplAccount to declare the imAccount as disconnected and I'm not sure how to do that nicely.