[Adium-devl] Severe Structural AIChat-Issue in Adium

[Evan Schoenberg]

On Jul 19, 2007, at 12:12 PM, Andreas Monitzer wrote:

> Specifically, the problem is described in the following tickets:
> http://trac.adiumx.com/ticket/1874
> http://trac.adiumx.com/ticket/2549
> http://trac.adiumx.com/ticket/7048
>
> Those are actually all caused by the same issue. It starts with
>
<snip> Thanks much for the good flow explanation of a rather  
complicated block of code to let everyone be up to speed from the get- 
go.  That's a great way to handle this :)

> -[CBPurpleAccount openChat:] (note that we move to libpurple-specific
> stuff now) calls -[SLPurpleAdapter openChat:onAccount:], which in
> turn calls convLookupFromChat().
>
> convLookupFromChat() sets up the necessary information for
> lilbpurple, and calls serv_join_chat (this one doesn't return
> anything). This is all that has to be done from Adium.

It seems like here we have the first problem.  serv_join_chat()  
appears not to be able to return the PurpleConversation, because it  
may not exist for some time to come - at least some of the prpls do  
asynchronous joins and create the conversation on success.  I think  
that, for starters:
  * serv_join_chat() needs some way of indicating an error.  This  
could be a return value or an error-returned-by-reference.
  * I say the latter because I think the return value should be a  
handle which can be used to identify the PurpleConversation once it  
is created.

There is already an integer id used when passing to  
serv_got_joined_chat - it appears this is unique per prpl.  Perhaps  
returning this and making use of this could be a good direction?

> Now, libpurple joins the chat, and calls adiumPurpleConvCreate() on
> success. Note that libpurple doesn't call anything in Adium when it
> doesn't succeed. Since Adium already created the AIChat object, this
> is the cause for bug #1874 (Adium thinks that there's already a
> groupchat with that MUC, so it disallows the join).

I think having already created the AIChat object is fine - the  
problem is that it's not destroyed on error.  As mentioned above,  
serv_join_chat() should return an error if it fails immediately...  
but if there's a failure some other way, we still need to find out.   
What do you think about a serv_failed_joined_chat() or something like  
that which the prpl would call and which would, in turn, let us know  
that the join attempt failed?

> adiumPurpleConvCreate() calls groupChatLookupFromConv(), which in
> turn calls -[CBPurpleAccount chatWithName:identifier:], passing the
> PurpleConversation as the identifier (wrapped in an NSValue).
>
> -[CBPurpleAccount chatWithName:identifier:] calls
> -[AIChatController  
> chatWithName:identifier:onAccount:chatCreationInfo:];
> Since the identifier for our existing AIChat is nil, this method
> doesn't find it, and instead creates a new AIChat object, which is
> then opened by using -[CBPurpleAccount addChat:] in
> adiumPurpleConvCreate().

> After opening the chat, the Chat_DidOpen notification is sent with
> the new AIChat object, which doesn't trigger the notification we sent
> earlier, since that used the original AIChat object. That's the cause
> of bug #2549 and #7048.

I'm a bit confused at this point - are you saying you've found that 2  
AIChat objects are created for every joined group chat?  I think, if  
so, that's a 1.1/trunk regression, though I'm not positive - and in  
that case I may have caused that while working on fixing MSN group  
chats.

Does this case only occur if there has been a previous failure, or  
does it happen every time?

-Evan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: PGP.sig
Type: application/pgp-signature
Size: 186 bytes
Desc: This is a digitally signed message part
URL: <http://adium.im/pipermail/devel_adium.im/attachments/20070726/6f4c3cb3/attachment.sig>