Thursday, May 22, 2014

Clicking unclickable list items

One of the UI patterns that improve lists usability is dividing items into sections. The section might be the first letter of the main text on the list item, date formatted and rounded in a specific way or whatever makes sense for your data.

From the technical point of view you can either add the header view to every list item and show and hide them as needed or create the separate view for header and regular list item and register multiple view types in your Adapter. Both options were described in details by +Cyril Mottier in excellent ListView Tips & Tricks #2: Sectioning Your ListView blog post.

If you choose the second approach, you'll have to decide what to return from your Adapter's getItem and getItemId methods for items representing sections. If your sections are not supposed to be clickable, you might implement your Adapter like this:
@Override
public Object getItem(int position) {
  return getItemViewType(position) == TYPE_ITEM
      ? mItems[getCursorPosition(position)]
      : null;
}

@Override
public long getItemId(int position) {
  return getItemViewType(position) == TYPE_ITEM
      ? getCursorPosition(position)
      : 0;
}

@Override
public boolean areAllItemsEnabled() {
  return false;
}

@Override
public boolean isEnabled(int position) {
  return getItemViewType(position) == TYPE_ITEM;
}
And your onListItemClickListener like this:
@Override
public void onListItemClick(ListView l, View v, int position, long id) {
  super.onListItemClick(l, v, position, id);

  // dummy action which uses Object returned from getItem(position)
  Log.d("DMFM", getListAdapter().getItem(position).toString());
}
If you do so, the Android has a nasty surprise for you:
java.lang.NullPointerException
    at org.chalup.dialmformonkey.app.MainFragment.onListItemClick(MainFragment.java:38)
    at android.app.ListFragment$2.onItemClick(ListFragment.java:160)
    at android.widget.AdapterView.performItemClick(AdapterView.java:298)
    at android.widget.AbsListView.performItemClick(AbsListView.java:1100)
    at android.widget.AbsListView$PerformClick.run(AbsListView.java:2749)
    at android.widget.AbsListView$1.run(AbsListView.java:3423)
    at android.os.Handler.handleCallback(Handler.java:725)
    ...
The only way this can happen is getting null from Adapter.getItem(), but this method will be called only for disabled items, right?
@Override
public void onListItemClick(ListView l, View v, int position, long id) {
  super.onListItemClick(l, v, position, id);

  Log.d("DMFM", "Clicked on item " + position + " which is " +
        (getListAdapter().isEnabled(position) 
            ? "enabled"
            : "disabled")
  );

  // dummy action which uses Object returned from getItem(position)
  Log.d("DMFM", getListAdapter().getItem(position).toString());
}
Wrong:
D/DMFM﹕ Clicked on item 4 which is enabled
D/DMFM﹕ Abondance
D/DMFM﹕ Clicked on item 4 which is enabled
D/DMFM﹕ Abondance
D/DMFM﹕ Clicked on item 31 which is enabled
D/DMFM﹕ Aragon
D/DMFM﹕ Clicked on item 31 which is enabled
D/DMFM﹕ Aragon
D/dalvikvm﹕ GC_CONCURRENT freed 138K, 3% free 8825K/9016K, paused 0ms+0ms, total 3ms
D/DMFM﹕ Clicked on item 28 which is disabled
It's very difficult to reproduce this error manually, especially if tapping the list item does something more than writing to logcat, but I investigated this issue, because the stack traces above appeared in crash reports on Google Analytics, so several people managed to click exactly wrong area at the wrong time.

I didn't investigate the issue thoroughly, but it seems there must be some disparity between checking the isEnabled method and getting the item. If I ever dive into ListView code, I'll definitely write about it. If you want to reproduce or investigate the issue yourself, compile this project and run the monkey:
$ adb shell monkey -p org.chalup.dialmformonkey.app -v 500
So what can we do? First option is checking the Adapter.isEnabled() in your onListItemClick listener, which is yet another kind of boilerplate you have to add to your Android code, but it's super easy to add. The other option is going with the first sectioning approach, i.e. putting section as a part of the clickable list item, but it might not work for your use case (for example adapter with multiple item types).

Saturday, May 10, 2014

Android App Widgets issues

This week I spend few days analyzing and fixing various issues of app widget in Base CRM application.

This part of our codebase was created over a year ago during one of our internal hackathons and was released soon after that. Most of the times it worked. Every once in a while we received a weird crash report from Google Analytics, but it never caused much trouble. Recently though we received few complaints from customers. I happened to have few hours available for bug hunting, so I took a dive.

The widget is really a simple todo list backed by ContentProvider. The code looks like it was based on the WeatherWidget from SDK samples. What can possibly go wrong?

Issue #1: gazillions of threads started

Take a look at the code of WeatherWidgetProvider:
public WeatherWidgetProvider() {
  // Start the worker thread
  sWorkerThread = new HandlerThread("WeatherWidgetProvider-worker");
  sWorkerThread.start();
  sWorkerQueue = new Handler(sWorkerThread.getLooper());
}
The WeatherWidgetProvider is an AppWidgetProvider implementation, which extends a regular BroadcastReceiver. It means that for every action a new instance of WeatherWidgetProvider is created, and the current implementation spawns new thread which is never closed.

The sample author obviously intended to create only one worker thread - the sWorkerThread is the static - but forgot to do the null check before creating a new thread. So let's fix it:
public WeatherWidgetProvider() {
  if (sWorkerThread == null) {
    // Start the worker thread
    sWorkerThread = new HandlerThread("WeatherWidgetProvider-worker");
    sWorkerThread.start();
    sWorkerQueue = new Handler(sWorkerThread.getLooper());
  }
}

Issue #2: no refresh after application update

The widget shows data from the same ContentProvider as the main app, so when the user creates a task inside in the main app and then goes back to homescreen, the task should be displayed on the widget. To achieve this we did the same thing the WeatherWidget sample does - we register the ContentObserver in onEnabled callback of AppWidgetProvider:

@Override
public void onEnabled(Context context) {
  final ContentResolver r = context.getContentResolver();
  if (sDataObserver == null) {
    final AppWidgetManager mgr = AppWidgetManager.getInstance(context);
    final ComponentName cn = new ComponentName(context, WeatherWidgetProvider.class);
    sDataObserver = new WeatherDataProviderObserver(mgr, cn, sWorkerQueue);
    r.registerContentObserver(WeatherDataProvider.CONTENT_URI, true, sDataObserver);
  }
}

The onEnabled callback is called when the first instance of the widget is added to homescreen, so the code looks fine. Unfortunately the callback is not called at your process startup. So if your app is updated and the process is restarted, the ContentObserver won't be registered. The same thing happens if your app crashes or is stopped by the OS to free resources.

To solve this you have to register the ContentObserver in few more places. I have added registration to onCreate callback in RemoteViewsFactory and the onReceive part which handles our custom actions in AppWidgetProvider.

WeatherWidget sample does one more thing wrong: the ContentObserver is never unregistered and the worker thread is never stopped. The correct place to do this is onDisabled callback in AppWidgetProvider.

Issue #3: CursorOutOfBoundsException crash

Ever since we introduced the tasks widget, we've occasionally received the crash reports indicating that the RemoteViewsFactory requested elements outside of [0, getCount) range:

05-10 15:22:50.559  13781-13795/org.chalup.widgetfail.widget E/AndroidRuntime﹕ FATAL EXCEPTION: Binder_2
    Process: org.chalup.widgetfail.widget, PID: 13781
    android.database.CursorIndexOutOfBoundsException: Index 1 requested, with a size of 1

The reproduction steps for this issue are quite complicated:
  • Tap the task on the widget to mark it was completed. Internally we set the PENDING_DONE flag, so the task is marked as done, but is still displayed on the list, so the user can tap it again and reset the flag.
  • Trigger the sync
  • SyncAdapter syncs the Task to our backend. The task is marked as DONE in our database, which triggers the ContentObserver registered by the widget.
  • ContentObserver triggers onDataSetChanged callback in RemoteViewsFactory, which then calls getCount and getViewAt
  • In some rare cases getViewAt with position == result of getCount is called
It looks like some kind of a race condition or another threading issue in Android code which populates the app widgets. I tried synchronizing the RemoteViewsFactory methods, but it didn't help. The getViewAt have to return a valid RemoteViews, so I fixed it up by returning the loading view when element outside of valid range is requested:

@Override
public synchronized RemoteViews getViewAt(int position) {
  if (position >= mCursor.getCount()) {
    return getLoadingView();
  } else {
    mCursor.moveToPosition(position);

    // ...
  }
}

Issue #4: no refresh when "Don't keep activities" setting is enabled

User can click on the tasks displayed on the widget to go to the edit screen. The activity is closed when user saves or discards changes and the homescreen with the widget is shown again. Changing the task triggers the ContentObserver, the onDataSetChanged is called on all active RemoteViewsFactories, but sometimes other callbacks (getCount, getViewAt, etc.) are not called.

It turns out this happens when the Homescreen activity is recreated because of low memory condition. To easily reproduce this issue you can check the "Don't keep activities" in developers settings.

I do not have a solution or workaround for this issue unfortunately. I'll file a bug report and hope for the best.

Recap

There are mutliple issues with the WeatherWidget sample and some issues with the system services responsible for populating app widgets with content. I've created a simple project which reproduces the issues #3 and #4 and shows the correct way of registering ContentObserver for your widget. The sources are available on Github.