From 7421d33c3ae832ebe31a24b280bb4be566a55fa3 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Mon, 7 Mar 2016 22:44:17 +1100 Subject: [PATCH 1/2] Expand null check to include `isAdded()` check Also, don't call `getActivity()` in the separate thread. Instead, use the `Activity` which we have already checked and ensured is not null. --- .../fdroid/views/fragments/AvailableAppsFragment.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/F-Droid/src/org/fdroid/fdroid/views/fragments/AvailableAppsFragment.java b/F-Droid/src/org/fdroid/fdroid/views/fragments/AvailableAppsFragment.java index f0b595f08..ad75ec25c 100644 --- a/F-Droid/src/org/fdroid/fdroid/views/fragments/AvailableAppsFragment.java +++ b/F-Droid/src/org/fdroid/fdroid/views/fragments/AvailableAppsFragment.java @@ -82,18 +82,14 @@ public class AvailableAppsFragment extends AppListFragment implements // me that "Only the original thread that created a view // hierarchy can touch its views." final Activity activity = getActivity(); - // this nullguard is temporary, this Fragment really needs to merged into the Activity - if (activity == null) { + if (!isAdded() || adapter == null || activity == null) { return; } activity.runOnUiThread(new Runnable() { @Override public void run() { - if (adapter == null) { - return; - } adapter.clear(); - categories = AppProvider.Helper.categories(getActivity()); + categories = AppProvider.Helper.categories(activity); ArrayAdapterCompat.addAll(adapter, translateCategories(categories)); } }); From 0f64f2c18138fc7ab728bb4d56cc7461488cb03a Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Mon, 7 Mar 2016 22:46:51 +1100 Subject: [PATCH 2/2] Clarify threading when loading categories Previously, it was not explicit that the `onCreate` happened to be invoked in the UI thread. Now it is, due to passing `new Handler(Looper.getMainLooper())`. Also, the categories are now loaded in a background task, and then the UI is updated on the UI thread. --- .../fragments/AvailableAppsFragment.java | 30 ++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/F-Droid/src/org/fdroid/fdroid/views/fragments/AvailableAppsFragment.java b/F-Droid/src/org/fdroid/fdroid/views/fragments/AvailableAppsFragment.java index ad75ec25c..7b4750dc5 100644 --- a/F-Droid/src/org/fdroid/fdroid/views/fragments/AvailableAppsFragment.java +++ b/F-Droid/src/org/fdroid/fdroid/views/fragments/AvailableAppsFragment.java @@ -7,7 +7,10 @@ import android.content.res.Resources; import android.database.ContentObserver; import android.database.Cursor; import android.net.Uri; +import android.os.AsyncTask; import android.os.Bundle; +import android.os.Handler; +import android.os.Looper; import android.support.annotation.Nullable; import android.support.v4.app.LoaderManager; import android.view.LayoutInflater; @@ -72,27 +75,34 @@ public class AvailableAppsFragment extends AppListFragment implements private final ArrayAdapter adapter; CategoryObserver(ArrayAdapter adapter) { - super(null); + // Using Looper.getMainLooper() ensures that the onChange method is run on the main thread. + super(new Handler(Looper.getMainLooper())); this.adapter = adapter; } @Override public void onChange(boolean selfChange) { - // Wanted to just do this update here, but android tells - // me that "Only the original thread that created a view - // hierarchy can touch its views." final Activity activity = getActivity(); if (!isAdded() || adapter == null || activity == null) { return; } - activity.runOnUiThread(new Runnable() { + + // Because onChange is always invoked on the main thread (see constructor), we want to + // run the database query on a background thread. Afterwards, the UI is updated + // on a foreground thread. + new AsyncTask>() { @Override - public void run() { - adapter.clear(); - categories = AppProvider.Helper.categories(activity); - ArrayAdapterCompat.addAll(adapter, translateCategories(categories)); + protected List doInBackground(Void... params) { + return AppProvider.Helper.categories(activity); } - }); + + @Override + protected void onPostExecute(List loadedCategories) { + adapter.clear(); + categories = loadedCategories; + ArrayAdapterCompat.addAll(adapter, translateCategories(loadedCategories)); + } + }.execute(); } @Override