No disconnection on mobile network disconnection and other state confusion.

[GoogleCodeExporter]

At least on the same devices for which issue 8 (Gtalksms freezes after turning 
off mobile data connection) was a problem...

* Connect via 3g network (ie, with wifi disabled)
* Enable wifi - 3g network is dropped.
* GtalkSMS hasn't dropped the 3g connection so still thinks it is connected - 
but fails to respond to chat requests (as the connection is dead).  It should 
have disconnected then reconnected on the wifi network.
* Explicitly stopping GTalkSMS does work as expected.

The problem is our handing of the NETWORK_CHANGED action - we no longer attempt 
to close an existing connection when the network has become not available.  In 
*most* cases, this isn't a problem - the XmppManager's connection listener sees 
an error state as the network drops, so the right thing happens.  Sadly though, 
on the devices where this bug exists, we do *not* see an error condition - so 
the new handling of NETWORK_CHANGED leaves the (now dead) connection around, 
but it never receives any messages.

So in short, we need to disconnect when the network is not available.  Also 
note one other bit of confusion around the xmpp state in the retry handler.  
While we are waiting for a retry, the state is set to CONNECTING (which it 
isn't - it is waiting) and once we have given up on retries the state is left 
in WAITING_TO_CONNECT (which it isn't - it has given up.)  This causes some 
problems when handling, for example, the TOGGLE action - it does nothing when 
in the state CONNECTING, so pressing the widget does nothing.

I've spent some time on this and can't make it work correctly in all cases 
using the current scheme.  I think we really need to rework the state 
management - I'm thinking along the lines of:

* The xmpp manager has 2 states - the "desired" state and the actual connection 
state.

* The states CONNECTING and DISCONNECTING remain as "temporary" states - ie, 
the actual state will never be one of these values except *during* a call to 
start() or stop().

* We drop the WAITING_FOR_CONNECTION state - this is implied by having a 
desired state of CONNECTED, but an actual state of DISCONNECTED.

The devil is in the detail, but I'm out of time for today...

Original issue reported on code.google.com by skippy.hammond@gmail.com on 4 Jan 2011 at 8:08

[GoogleCodeExporter]

I've yet another patch for state management :)  With the following patch, the 
service is more "reactive" to external state changes - ie, we don't trust what 
we *think* the state is, we use what the connection state actually is.  The 
actual connection state may change behind our back - eg, as a network error is 
handled, or as a scheduled retry finally connects.

This patch also makes things more thread-safe - the worker thread is now 
responsible for handling incoming chat messages - previously onMessageReceived 
was called on the main thread leading to the possibility of the worker thread 
handling state changes *while* a message was being sent.

It is fairly large, but my testing shows it works perfectly ;)  A limitation 
though is that changes to connection preferences will likely not be picked up 
after a connection is established - we probably just need a property-change 
listener, but I thought I'd leave that out of this...

FWIW, this bug also fixes the new bug re the "GTalkSMS stopped" toast message 
not being removed (as this message is now only ever generated by the main 
thread, not the worker thread...

Original comment by skippy.hammond@gmail.com on 5 Jan 2011 at 7:59

Attachments:

• disconnect-no-network.patch

[GoogleCodeExporter]

Bugger - just saw the app crash with that patch when a call was received - I'll 
check it out...

Original comment by skippy.hammond@gmail.com on 5 Jan 2011 at 9:54

[GoogleCodeExporter]

That's not your patch, on market someone has the same error on the 1.6:

java.lang.IllegalArgumentException: URI: 
content://com.android.contacts/phone_lookup/, calling user: 
com.googlecode.gtalksms, calling package:com.googlecode.gtalksms
at 
android.database.DatabaseUtils.readExceptionFromParcel(DatabaseUtils.java:144)
at 
android.database.DatabaseUtils.readExceptionFromParcel(DatabaseUtils.java:114)
at 
android.content.ContentProviderProxy.bulkQueryInternal(ContentProviderNative.jav
a:330)
at android.content.ContentProviderProxy.query(ContentProviderNative.java:366)
at android.content.ContentResolver.query(ContentResolver.java:250)
at 
com.googlecode.gtalksms.data.contacts.ContactsManager.getContactName(ContactsMan
ager.java:32)
at 
com.googlecode.gtalksms.receivers.PhoneCallListener.onCallStateChanged(PhoneCall
Listener.java:30)
at 
android.telephony.PhoneStateListener$2.handleMessage(PhoneStateListener.java:319
)
at android.os.Handler.dispatchMessage(Handler.java:99)
at android.os.Looper.loop(Looper.java:123)
at android.os.HandlerThread.run(HandlerThread.java:60)

Original comment by Florent....@gmail.com on 5 Jan 2011 at 9:57

[GoogleCodeExporter]

Original comment by Florent....@gmail.com on 5 Jan 2011 at 9:19

• Changed state: Started

[GoogleCodeExporter]

Original comment by Florent....@gmail.com on 5 Jan 2011 at 9:36

• Changed state: Fixed

[GoogleCodeExporter]

Original comment by fschm...@gmail.com on 25 Aug 2011 at 7:51

• Changed state: Deployed