新米Android開発者が見落としがちな3つのポイント

こんにちは、投稿推進部の吉田(@101kaz)です。Androidアプリの投稿周りの開発を担当しています。
去年クックパッドに入社したことをきっかけに、本格的にAndroid開発をするようになりました。今回は私のような開発をはじめて日が浅い人が見落としがちな「非同期処理時のNPE(NullPointerException)」と「Activity破棄に関する問題」と「ProGuardの設定忘れ」について実際の遭遇した事例をベースに紹介します。

非同期処理コールバック時のNPE

ある時Fragmentから非同期処理を行い、コールバック内でFragmentの内のviewにアクセスするコードを書きました。

@Override
public void onActivityCreated(Bundle savedInstanceState) {
    ApiClient.getRecipes(new ApiClient.Callback() {
                @Override
                public void onSuccess(List<Recipe> recipes) {
                  View loadingView = getView()
                          .findViewById(R.id.loading_view);
                  loadingView.setVisibility(View.GONE);
                }
                @Override
                public void onFailure() {
                }
    });
}

このコードは問題なさそうに見えますが、getView()がnullを返す可能性があり、その場合findViewById()でNPEが発生します。これは、非同期処理のコールバックが返ってくる前にFragmentが画面の回転などの理由で既にデタッチされ、Viewが破棄される可能性があるためです。そのためコールバックの処理を行う前にまず自分の状態を確認し、すでにデタッチされているようであれば処理を終了しなければなりません。

if(isDetached() || getActivity() == null) {
    return;
}

オフィス内のネットワーク環境ではAPIサーバーからのレスポンスが数十msほどで返ってきます。そのため、コールバックが返ってくるより先にFragmentがデタッチされることは少ないので開発時に見落としがちですが、このままアプリがリリースされてしまうと多くのユーザーの方の手元でアプリがクラッシュする可能性があるので注意が必要です。

非同期処理時のNPEを防ぐにはコールバックの冒頭でデタッチされてないか(もしくはアクティビティが既に終了していないか)チェックする事を習慣づけることが重要です。とはいえ、忘れてしまうこともあるので可能であれば仕組みで解決したいところです。クックパッドではPull Request上で、botが投稿するチェックリストを開発者自身がチェックすることでミスを事前に防ぐ試みをしています。

また、非同期処理を行うライブラリ側にチェックを委譲する方法もあります。
RxAndroidというライブラリでは、bindActivity()、bindFragment()というメソッドがそれぞれ提供されています。
これらを利用すると、Activityが既に終了していたり、フラグメントが既にデタッチされている場合にそれ以上の処理が実行されないようにすることが可能です。
RxAndroidでhello worldをトーストするサンプルは以下のようになります。このサンプルでは、Fragmentがデタッチされているとcall()は実行されません。

Observable<String> observable = Observable.just("hello world")
Observable<String> bindedObservable = AndroidObservable.bindFragment(this, observable);
bindedObservable.subscribe(new Action1<String>() {
    @Override
    public void call(String s) {
        //Fragmentがデタッチされている場合呼び出されない
        Toast.makeText(context, s, Toast.LENGTH_SHORT).show();
    }
});

Activity破棄に伴うインスタンス変数の永続化

「写真撮影後に失敗した旨のメッセージが表示され、画像が取り込めない事がある。」というバグ報告があり調査したところ、以下の様なコードがありました。ここでは、カメラアプリからSelectPhotoFragmentに戻ってきて、onActivityResultで画像ファイルにアクセスしようとしています。

public class SelectPhotoFragment extends Fragment {
    private Uri savedPhotoUri;
    // ....

    private void launchCamera() {
        savedPhotoUri = Uri.fromFile(tempFile);
        Intent intent = new Intent(MediaStore.ACTION_IMAGE_CAPTURE);
        intent.putExtra(MediaStore.EXTRA_OUTPUT, savedPhotoUri);
        this.startActivityForResult(intent, REQUEST_CODE_TAKE_PICTURE);
    }

    @Override
    public void onActivityResult(int requestCode, int resultCode, Intent data) {
        super.onActivityResult(requestCode, resultCode, data);
        if (savedPhotoUri != null) {
            listener.onSelect(photoUri);
        } else {
            ToastUtils.show(getActivity(), "写真撮影に失敗しました");
            return;
        }
    }
}

このバグは、SelectPhotoFragmentがバックグラウンドにまわった際に破棄され、savedPhotoUriの参照も消えてしまったことが原因でした。 AndroidのシステムはActivityがバックグラウンドにまわるとメモリの空き容量などの都合で順番に破棄していきます。破棄のタイミングは環境や機種の性能によって異なるので、これも開発時には見落としがちになります。 Activityの破棄に起因するバグを防ぐためには、適切にインスタンス変数をBundleに格納して状態の永続化を行う必要があります。

// Activityの場合
    @Override
    protected void onCreate(Bundle savedInstanceState) {
        if (savedInstanceState != null) {
            if (savedInstanceState.containsKey(SAVED_PHOTO_URI_KEY)) {
                savedPhotoUri = savedInstanceState.getParcelable(SAVED_PHOTO_URI_KEY);
            }
        }
    }
    
    @Override
    public void onSaveInstanceState(Bundle outState) {
        super.onSaveInstanceState(outState);
        outState.putParcelable(SAVED_PHOTO_URI_KEY, savedPhotoUri);
    }


// Fragmentの場合
    @Override
    public void onViewStateRestored(Bundle savedInstanceState) {
        if (savedInstanceState != null) {
            if (savedInstanceState.containsKey(SAVED_PHOTO_URI_KEY)) {
                savedPhotoUri = savedInstanceState.getParcelable(SAVED_PHOTO_URI_KEY);
            }
        }
    }

    @Override
    public void onSaveInstanceState(Bundle outState) {
        super.onSaveInstanceState(outState);
        outState.putParcelable(SAVED_PHOTO_URI_KEY, savedPhotoUri);
    }

また、Activityの破棄に起因する問題を未然に防ぐために、開発者向けオプションにあるアクティビティを保持しない設定を有効にした状態で開発する方法も有効です。

さらにIcepickというライブラリを使うと、煩雑な状態の永続化をアノテーションを使って手軽に管理することが出来るようになります。

 @Icicle String username;

  @Override public void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    Icepick.restoreInstanceState(this, savedInstanceState);
  }

  @Override public void onSaveInstanceState(Bundle outState) {
    super.onSaveInstanceState(outState);
    Icepick.saveInstanceState(this, outState);
  }

ProGuardの設定忘れ

ProGuardとは、アプリの最適化と難読化を行うツールです。
クックパッドのAndroidアプリもリリースビルド時にはProGuardを利用しますが、ProGuardを有効にするとリフレクションを使った実装が正しく動作しなくなります。これはProGuardが参照のない箇所を削除したり、メソッド名や変数名を書き換えてしまうことが原因です。
リフレクションを正しく機能させるためには、該当範囲をProGuardの対象から外す必要があるのですが、デバッグビルドでは通常Proguardを有効にしないので、ProGuardに関する問題も開発時に見落としがちです。
アプリ開発で直接リフレクションを使う機会は少ないですが、JavascriptInterfaceなどAPIの裏側でリフレクションが使われいるケースや、利用しているライブラリの中で使われている可能性もあるのでこれも注意が必要です。

-keepclassmembers public class com.cookpad.android.sample {
    *;
}

上記のように書くと、com.cookpad.android.sample以下の全てのメンバ変数とメソッドがProguardの対象から外れます。

まとめ

開発時に発生しにくいバグは、どうしても見落とされてリリースされやすいです。
ライブラリの導入などでそもそも問題が起こりえない体制を整えることがベストですが、
全てはカバーすることは難しいので、QA期間を設けたり、botを活用したり、コードレビューなど様々な方法を組み合わせることによって、クラッシュしにくいアプリ開発をしていきたいですね。