Wednesday, September 26, 2012

Android sticky broadcast perils


I might have called this blog post "Android sticky broadcasts considered harmful", but "Considered harmful" texts are considered harmful, so I opted for a less linkbaity title.

I was working on a bug in a legacy code which used sticky broadcasts for publishing the sync service state (BTW: if you thought "Hello, ContentResolver.isSyncActive?" you should subscribe to my blog feed - I'm planning to write about the problems with sync state methods in ContentResolver). The bug manifested as a minor UI issue - sometimes the UI indicated that sync is in progress, even in situations when the sync could not be in progress, for example because the network connection was down. The QA team found an easy reproduction steps for it: start sync and reinstall the app before the sync finishes.

I found much more troubling reproduction steps: start sync, uninstall app while sync is in progress and then install it again. The difference between those steps and the ones found by QA is the fact that I'm performing uninstall, which means that all information about my app should be wiped from the system.

I dug deeper and found out two scary facts:

  • Sticky broadcasts are not connected in any way to app package, which means they are not removed on uninstallation. They are removed on phone restart, but that's not a scenario you should rely upon.
  • Any application with BROADCAST_STICKY permission may alter your sticky broadcasts.


As long as you don't use sync status for anything more complicated than showing a spinner in Action Bar, you might get away with only minor UI issues, but if you want to use it for business logic, you're entering the world of pain.

The sticky broadcasts might be garbage left over by the old version of your app or some other app if you've chosen the action string poorly (protip: if you really have to use sticky broadcasts, include your app's package name in action string). Even if you ignore the latter case (and the very unlikely scenario of other app maliciously tinkering with your app's intents), I consider the former case to be a deal breaker - each sticky broadcast is an additional state which has to be migrated between app versions and app installations which further increases cognitive load of programming, which is high enough already.

Let's summarize this blog post: remove BROADCAST_STICKY permission from your app's AndroidManifest.xml, test the app, fix all crashes from SecurityExceptions and never look back.

Wednesday, September 19, 2012

Android heterogeneous Adapters gotcha

Unless you were writing your Android apps under some kind of digital rock, you heard about Mark Murphy a.k.a. commonsguy. Today I'd like to write about a gotcha related to heterogeneous Adapters in general, which recently bit me in the rear when I used (misused?) one of Mark's Android components - MergeAdapter.

As you can read on the project's site, "MergeAdapter accepts a mix of Adapters and Views and presents them as one contiguous whole to whatever ListView it is poured into". This means of course that this is a heterogeneous adapter, i.e. the one which returns integer > 1 from getViewTypeCount(). The implementation of this method is pretty straightforward - it just iterates through the list of adapters it consists of and returns the sum of getViewTypeCount()s :
@Override
public int getViewTypeCount() {
  int total=0;

  for (PieceState piece : pieces.getRawPieces()) {
    total+=piece.adapter.getViewTypeCount();
  }

  return(Math.max(total, 1)); // needed for
                              // setListAdapter() before
                              // content add'
}
Everything is fine and dandy if you use the code like this:
@Override
public void onCreate(Bundle icicle) {
  super.onCreate(icicle);
  setContentView(R.layout.main);

  MergeAdapter adapter = new MergeAdapter();
  adapter.addView(someView);
  adapter.addAdapter(someAdapter);

  setListAdapter(adapter);
}
But sometimes you might want to attach the MergeAdapter to ListView and add fill it later (the real scenario for this case is adding stuff in onLoadFinished callback, I'm using contrived example for sake of simplicity):
@Override
public void onCreate(Bundle icicle) {
  super.onCreate(icicle);
  setContentView(R.layout.main);

  MergeAdapter adapter = new MergeAdapter();
  adapter.addAdapter(someAdapter);

  setListAdapter(adapter);

  adapter.addAdapter(someOtherAdapter);
}
This code will work as long as the adapter's contents fit on one screen, but if you start scrolling the list and the item recycling kicks in your app will crash with ClassCastException from your adapters' getView(). If by some chance you use the same IDs for the Views of the same type the app won't crash, but your items probably won't look exactly as they should. Either way, you won't be happy.

The root cause is the undocumented fact that the getViewTypeCount() is called only once after attaching it with ListView.setAdapter(). In the example above, the MergeAdapter contains only one item type when the setAdapter() is called, getViewTypeCount() will return 1, and adding the second adapter with another item type won't change this.

Why doesn't this crash right away? The ListView will call getView() in correct adapters, but then it will try to reuse items created by one adapter for items in the second adapter, because it assumes there is only one view type (getViewTypeCount() returned 1).

So what's the lesson here? Do not change the MergeAdapter in loader callbacks, either construct it before setAdapter() call (for example add empty CursorAdapters and call CursorAdapter.changeCursor() later) or postpone the setAdapter() call until you load all the parts. The more general rule is that you may not calculate the number of item types from the actual data, for example the following code won't work:
public class MyCursorAdapter extends CursorAdapter {
  // Implementation of bindView, newView, etc. skipped

  private int mCalculatedItemTypeCount;

  @Override
  public int getViewTypeCount() {
    return Math.max(mCalculatedItemTypeCount, 1);
  }

  @Override
  public void changeCursor(Cursor cursor) {
    mCalculatedItemTypeCount = /* some calculations */;
    super.changeCursor(cursor);
  }
}

Wednesday, September 12, 2012

Screen orientation and QT applications on Symbian Belle

Let's take a break from Android gotchas and do some mobile necrophilia, i.e. let's talk about Symbian.

Recently I received an email from Nokians saying that they are testing Nokia Store content with new firmware update and my app won't work after update. After few quick email exchanges we narrowed down the problem to screen orientation locking code I wrote about some time ago. It turns out that things can be done much simpler:

Window {
    Rectangle {
        id: root
        anchors.fill: parent

        Component.onCompleted: {
            screen.allowedOrientations = Screen.Landscape
        }

        // more stuff
    }
}
int main(int argc, char *argv[])
{
    QScopedPointer<QApplication> app(createApplication(argc, argv));

    QmlApplicationViewer viewer;
    viewer.setMainQmlFile(QLatin1String("qml/nupagadi/GameArea.qml"));
    viewer.setOrientation(QmlApplicationViewer::ScreenOrientationLockLandscape);
    viewer.setResizeMode(QDeclarativeView::SizeRootObjectToView);
    viewer.showExpanded();

    return app->exec();
}
Less code, no need to comment it as a gotcha/workaround, and it's supposedly futureproof.

I'm very positively surprised with Nokians' approach, responsiveness and this whole experience. Of course I wouldn't be me if I didn't bitch a little bit, namely: why did I have this problem in the first place? I mean, locking the screen orientation is not a rocket science and should be well documented. It should, but unfortunately it's not, like so many things about QML.

Wednesday, September 5, 2012

Android SharedPreferences gotcha

I have another gotcha for you. Can you tell what's wrong with the following code?

public class MyFragment extends Fragment implements OnSharedPreferenceChangeListener {
  private TextView mInfo;
  private SharedPreferences mPreferences;

  public static final String INFO_SP_KEY = "info";
  
  @Override
  public View onCreateView(LayoutInflater inflater, ViewGroup container,
      Bundle savedInstanceState) {
    return inflater.inflate(R.layout.my_fragment, container, false);
  }

  @Override
  public void onViewCreated(View view, Bundle savedInstanceState) {
    super.onViewCreated(view, savedInstanceState);
    mInfo = (TextView) view.findViewById(R.id.info);
  }
  
  @Override
  public void onSharedPreferenceChanged(SharedPreferences sharedPreferences, String key) {
    if (key.equals(INFO_SP_KEY)) {
      updateInfo();
    }
  }

  @Override
  public void onActivityCreated(Bundle savedInstanceState) {
    super.onActivityCreated(savedInstanceState);
    mPreferences = PreferenceManager.getDefaultSharedPreferences(getActivity());
  }

  @Override
  public void onPause() {
    super.onPause();
    mPreferences.unregisterOnSharedPreferenceChangeListener(this);
  }

  @Override
  public void onResume() {
    super.onResume();
    mPreferences.registerOnSharedPreferenceChangeListener(this);
    updateInfo();
  }

  protected void updateInfo() {
    mInfo.setText(getString(R.string.info_text, mPreferences.getInt(INFO_SP_KEY, 0)));
  }
}
At first glance everything looks fine and in most cases it will work fine as well. However, if you a) set android:minSdkVersion to 8 or lower and b) change the shared preference from another thread (IntentService, SyncAdapter, etc.), you'll get the following crash:

09-05 07:16:58.993: E/AndroidRuntime(403): android.view.ViewRoot$CalledFromWrongThreadException: Only the original thread that created a view hierarchy can touch its views.
09-05 07:16:58.993: E/AndroidRuntime(403): at android.view.ViewRoot.checkThread(ViewRoot.java:2802)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.view.ViewRoot.requestLayout(ViewRoot.java:594)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.view.View.requestLayout(View.java:8125)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.view.View.requestLayout(View.java:8125)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.view.View.requestLayout(View.java:8125)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.view.View.requestLayout(View.java:8125)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.view.View.requestLayout(View.java:8125)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.view.View.requestLayout(View.java:8125)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.view.View.requestLayout(View.java:8125)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.widget.TextView.checkForRelayout(TextView.java:5378)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.widget.TextView.setText(TextView.java:2688)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.widget.TextView.setText(TextView.java:2556)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.widget.TextView.setText(TextView.java:2531)
09-05 07:16:58.993: E/AndroidRuntime(403): at com.porcupineprogrammer.sharedpreferencesgotcha.BaseFragment.updateButtonText(BaseFragment.java:65)
09-05 07:16:58.993: E/AndroidRuntime(403): at com.porcupineprogrammer.sharedpreferencesgotcha.WrongFragment.onSharedPreferenceChanged(WrongFragment.java:12)
09-05 07:16:58.993: E/AndroidRuntime(403): at android.app.ContextImpl$SharedPreferencesImpl$EditorImpl.commit(ContextImpl.java:2830)
09-05 07:16:58.993: E/AndroidRuntime(403): at com.porcupineprogrammer.sharedpreferencesgotcha.BaseFragment$1$1.run(BaseFragment.java:36)
09-05 07:16:58.993: E/AndroidRuntime(403): at java.lang.Thread.run(Thread.java:1096)

Fortunately the obvious crashlog is obvious and you can solve this issue in about 5 seconds, by wrapping the UI action in Activity.runOnUiThread() method. Morbidly curious may track the root cause of this issue in GrepCode. Tl;dr: before Gingerbread the listeners are notified in the same thread as the SharedPreferences.commit() caller, in later releases commit() ensures the notifications are performed in UI thread.

Code of the sample application that demonstrates this issue is available on my github.