0

I have an acitivty which looks for user location (MyLocation class), then with the geopoint or without it it runs an AsyncTask to connect to server and get list of cities from my server. When the list is ready it saves them in ArrayList cities. Once the cities ArrayList is filled I would like it to be saved for good (proof to configuration changes). CityItem implements Parcelable. I save them in onSaveInstanceState and retrieve them onCreate.

Now, everything works fine if the task has completed and the cities list is filled. Then I rotate my device back and forth and Log.i("StartActivity", "Cities list downloaded:"+cities.toString()); gets called.

But if I rotate the device before the geopoint was found (or the task finished - hard to tell because it happens fast), then 

public void gotCities(ArrayList<CityItem> _cities){

    cities = _cities;
    Log.i("StartActivity", "gotCities("+cities.size()+"): "+cities.toString());
}

gets called (and cities are perfectly fine in the log) but when I rotate it once more ArrayList cities appears to be null again.

It appears that if the configuration changed and the savedInstanceState.cities was null, the ArrayList cities is somehow created again and it's not the same ArrayList as the one in gotCities() function.

I'm pretty sure it's something easy but I've been searching for answer for hours and I simply can't do it.

Code of the Activity:

public class StartActivity extends Activity {

public static final String PREFS_NAME = "PrefsFile";

MyLocation myLocationObject = null;
LatLngPoint point = null;
ArrayList<CityItem> cities = null;

FindCityTask task = null;
Activity startActivity;

@Override
public void onCreate(Bundle savedInstanceState) {

if(savedInstanceState!=null) if(savedInstanceState.containsKey("cities")) cities = savedInstanceState.getParcelableArrayList("cities"); if(cities!=null) Log.i("Cities retrieved", cities.toString());

    super.onCreate(savedInstanceState);

    startActivity = this;

        setContentView(R.layout.start);

        //check if the configuration (orientation) has been changed
        NonConfigurationObject nco = (NonConfigurationObject)getLastNonConfigurationInstance();
        if(nco!=null) if(nco.myLocationObject!=null) myLocationObject = nco.myLocationObject;
        if(nco!=null) if(nco.task!=null) task = nco.task;

        if(cities==null){

            Log.i("StartActivity", "Cities list is empty - retrieve them.");

            if(myLocationObject==null){
                getGeopoint();
            }
        } else {
            Log.i("StartActivity", "Cities list downloaded:"+cities.toString());

    }

}

private void getGeopoint(){

    if(isOnline()){ //there is internet connection


        if(myLocationObject==null){

            myLocationObject = new MyLocation();
            //calls function to check user location (returns false if no providers are enabled
            if(!myLocationObject.getLocation(this, locationResult)){ /*TODO handle */Log.i("StartActivity", "Location providers disabled");}

        }


    } else { //not online - show msg

        Log.i("StartActivity", "No internet connection");

    }

}


//waits for user geopoint. then starts FindCityTask 
LocationResult locationResult = new LocationResult(){
    @Override
    public void gotLocation(final Location location){
        if(location!=null){

            // location found
            Log.i("StartActivity", "Received location: "+location.toString());
            point = new LatLngPoint((float)location.getLatitude(), (float)location.getLongitude());

        } else {

            // location not found   
            Log.i("StartActivity", "No location received after 20 seconds");
            point = null;

        }

        //RUN TASK to connect to server to get cities list (even if there's no geopoint)
        task = new FindCityTask(startActivity);
        task.execute(point);

    }
};

public void gotCities(ArrayList<CityItem> _cities){

    cities = _cities;
    Log.i("StartActivity", "gotCities("+cities.size()+"): "+cities.toString());
}

@Override
public void onSaveInstanceState(Bundle savedInstanceState) {
    super.onSaveInstanceState(savedInstanceState);
    Log.i("onSaveInstanceState", "onSaveInstanceState");
    if(cities!=null) savedInstanceState.putParcelableArrayList("cities", cities);
}

@Override
public NonConfigurationObject onRetainNonConfigurationInstance() {

    NonConfigurationObject nco = new NonConfigurationObject();

    if(myLocationObject!=null){
        nco.myLocationObject = myLocationObject;
    }
    if(task!=null){
        nco.task = task;
    }

    return nco;

}

static class NonConfigurationObject{

    MyLocation myLocationObject;
    FindCityTask task;

}

gotCities() method is called from AsyncTask onPostExecute:

@Override
protected void onPostExecute(Void result) {
    if(this.activity!=null){
        ((StartActivity) activity).gotCities(cities);   
    }
}
Michał Klimczak
  • 12,674
  • 8
  • 66
  • 99

2 Answers2

0

When orientation cnange hits, your activity is stopped, destroyed and created anew, and the only callback which is guaranted to be called is onPause(). The same happens when screen lock kick in - it forces your activity in portrait mode.

I recommend you to read about android activity lifecycle. Rule of thumb is:

  • onCreate() is for initalizing interface objects and services
  • onResume() is called when your activity comes on top and is about to be presented to user
  • onPause() when it loses focus and is not presented to use anymore.

Since obtaining location takes long time, it is better to move it away from activity to background service, ( start it in onCreate() if necessary ) and decouple its lifecycle from activity. Service can pass results to activity via broadcast messages, or java method calls

And look into your onSaveInstanceState() - first you are calling super.onSaveInstanceState(), and then you modify bundle to include your data. This way they are never saved.

Konstantin Pribluda
  • 12,329
  • 1
  • 30
  • 35
  • But I have callback from onSaveInstanceState so it is called. Location obtaining is moved to a TimerTask (MyLocation takes care of this and sends the geopoint to locationResult). I found a solution just a second ago, but I wonder if it's ok. – Michał Klimczak Mar 10 '12 at 13:06
  • 1
    Static field is OK. It lives as long as your app JVM and is not affected by activity lifecycle. – Konstantin Pribluda Mar 10 '12 at 13:54
  • Thanks. But I still feel I'm missing something. I worry that the cities variable was simply leaked. And I wonder where did I let it happen. – Michał Klimczak Mar 10 '12 at 13:56
  • Paradoxically it does. I even changed it to be this way (super.onSaveInstanceState() on top of the method) because someone here on SO suggested it in a similar post. But I already found out the solution and wrote the answer, but I have to wait a few hours before I answer my own question (SO restrictions). It's about attaching the reference for activity to the task, so the right activity gets updated onPostExecute(). In my code the task holded reference to the previous activity (the one before configuration change) in some cases. – Michał Klimczak Mar 10 '12 at 17:28
  • after rereading documentation I think that proper sequence would be to modify bundle with your data, and call through to superclass afterwards - no point in storing your data in already persisted bundle – Konstantin Pribluda Mar 10 '12 at 18:43
  • http://stackoverflow.com/questions/151777/how-do-i-save-an-android-applications-state - here you can see this upvoted (50) comment on the accepted answer that says to do it the other way. To be honest I didn't test it thoroughly, but to me both approaches worked well (he mentions Droid X i test on Desire). – Michał Klimczak Mar 10 '12 at 19:31
0

I finally got it. It's the AsyncTask which pointed to wrong (the one before configuration change) activity. The trick is to attach to the task the reference to activity (and to do it in the right moment).

So the first thing to do is to put these functions in the AsyncTask:

void attach(Activity activity){
    this.activity = activity;
}

void detach(){
    this.activity = null;
}

When the task is first called we should attach the activity to it. Then onRetainNonConfigurationInstance() detach it.

@Override    
public NonConfigurationObject onRetainNonConfigurationInstance() {

    //normally it would return only the task, but i have to return another object
    //hence the NonConfigurationObject which holds reference to both the AsyncTask and MyLocation
    //(as seen in the original question)

    NonConfigurationObject nco = new NonConfigurationObject();

    if(task!=null){
        task.detach();
        nco.task = task;
    }

    return nco;

}

And finally attach it again when we call getLastNonConfigurationInstance() in onCreate().

        //check if the configuration (orientation) has been changed
        NonConfigurationObject nco = (NonConfigurationObject)getLastNonConfigurationInstance();

        if(nco!=null){ //not created for the first time

            Log.i("StartActivity", "NCO: "+nco.toString());
            task = nco.task;
            if(task!=null){ //nco can be present but task still null
                task.attach(this);
            } else {
                task = new FindCityTask(this);
            }

        } else {
            Log.i("StartActivity", "NCO: null");
            task = new FindCityTask(this);
        }

Please share your thoughts on this solution if you have any. I'll modify the question so it better suits the problem.

Michał Klimczak
  • 12,674
  • 8
  • 66
  • 99