From 47e065442edc108d4bb38f9daaa7cdb3fff26b49 Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Thu, 12 Mar 2015 15:45:39 +0100 Subject: [PATCH 1/5] Do not manually call onChange() (fix NPE) If the AppDetails activity has been destroyed by the system during an application installation/remove, it is recreated once it should be displayed again (this behavior can be forced by enabling "Don't keep activities" in Android developer options). In onCreate(), it passes its instance of myInstallerCallback to an Installer. In onActivityResult() (which is called before onResume()), this installer calls a method (onSuccess() or onError()) on this callback. The implementation of these methods (the anonymous inner class assigned to myInstallerCallback) dereference myAppObserver, which is still null, because it will be initialized in onResume(), so a NullPointerException is thrown. However, the problem is not only that myAppObserver.onChange() is called when myAppObserver is null, but that it should not be called manually at all: it is a ContentObserver, so it is automatically called when registered to the content resolver. As a consequence, this callback was called twice. Removing these calls fix both problems. Should fix issue #135 https://gitlab.com/fdroid/fdroidclient/issues/135 --- F-Droid/src/org/fdroid/fdroid/AppDetails.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/F-Droid/src/org/fdroid/fdroid/AppDetails.java b/F-Droid/src/org/fdroid/fdroid/AppDetails.java index 8199391bb..915b029f0 100644 --- a/F-Droid/src/org/fdroid/fdroid/AppDetails.java +++ b/F-Droid/src/org/fdroid/fdroid/AppDetails.java @@ -924,7 +924,6 @@ public class AppDetails extends ActionBarActivity implements ProgressListener, A } setSupportProgressBarIndeterminateVisibility(false); - myAppObserver.onChange(); } }); } @@ -936,7 +935,6 @@ public class AppDetails extends ActionBarActivity implements ProgressListener, A @Override public void run() { setSupportProgressBarIndeterminateVisibility(false); - myAppObserver.onChange(); } }); } else { @@ -944,7 +942,6 @@ public class AppDetails extends ActionBarActivity implements ProgressListener, A @Override public void run() { setSupportProgressBarIndeterminateVisibility(false); - myAppObserver.onChange(); Log.e(TAG, "Installer aborted with errorCode: " + errorCode); From 4e2ab4c04858bbe47b1a5812359a33038da6d07d Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Fri, 13 Mar 2015 10:18:26 +0100 Subject: [PATCH 2/5] Call super.onPause() first super.onPause() should always be called first: http://developer.android.com/training/basics/activity-lifecycle/pausing.html#Pause --- F-Droid/src/org/fdroid/fdroid/AppDetails.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/F-Droid/src/org/fdroid/fdroid/AppDetails.java b/F-Droid/src/org/fdroid/fdroid/AppDetails.java index 915b029f0..07b5fd079 100644 --- a/F-Droid/src/org/fdroid/fdroid/AppDetails.java +++ b/F-Droid/src/org/fdroid/fdroid/AppDetails.java @@ -523,6 +523,7 @@ public class AppDetails extends ActionBarActivity implements ProgressListener, A @Override protected void onPause() { + super.onPause(); if (myAppObserver != null) { getContentResolver().unregisterContentObserver(myAppObserver); } @@ -537,8 +538,6 @@ public class AppDetails extends ActionBarActivity implements ProgressListener, A } removeProgressDialog(); - - super.onPause(); } public void setIgnoreUpdates(String appId, boolean ignoreAll, int ignoreVersionCode) { From cda80f5de654b913906a53b5d1d2e3d6669b5139 Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Fri, 13 Mar 2015 10:22:06 +0100 Subject: [PATCH 3/5] Register ContentObserver when activity is started The ContentObserver was registered only when the activity was in resumed state. However, in started but paused state (when the activity is visible but not in focus), we still want to receive these notifications to update the view. Therefore, register it on start and unregister it on stop. As a consequence, myAppObserver will be non-null during onActivityResult(). --- F-Droid/src/org/fdroid/fdroid/AppDetails.java | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/F-Droid/src/org/fdroid/fdroid/AppDetails.java b/F-Droid/src/org/fdroid/fdroid/AppDetails.java index 07b5fd079..5be3f9e23 100644 --- a/F-Droid/src/org/fdroid/fdroid/AppDetails.java +++ b/F-Droid/src/org/fdroid/fdroid/AppDetails.java @@ -468,15 +468,19 @@ public class AppDetails extends ActionBarActivity implements ProgressListener, A private String mInstalledSigID; @Override - protected void onResume() { - super.onResume(); - + protected void onStart() { + super.onStart(); // register observer to know when install status changes myAppObserver = new AppObserver(new Handler()); getContentResolver().registerContentObserver( AppProvider.getContentUri(app.id), true, myAppObserver); + } + + @Override + protected void onResume() { + super.onResume(); if (downloadHandler != null) { if (downloadHandler.isComplete()) { downloadCompleteInstallApk(); @@ -521,12 +525,14 @@ public class AppDetails extends ActionBarActivity implements ProgressListener, A } } + protected void onStop() { + super.onStop(); + getContentResolver().unregisterContentObserver(myAppObserver); + } + @Override protected void onPause() { super.onPause(); - if (myAppObserver != null) { - getContentResolver().unregisterContentObserver(myAppObserver); - } if (app != null && (app.ignoreAllUpdates != startingIgnoreAll || app.ignoreThisUpdate != startingIgnoreThis)) { Log.d(TAG, "Updating 'ignore updates', as it has changed since we started the activity..."); From 578084ff96eb45024ab5acacce6538567351c188 Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Fri, 13 Mar 2015 10:26:42 +0100 Subject: [PATCH 4/5] Revert "Do not manually call onChange() (fix NPE)" This reverts commit 47e065442edc108d4bb38f9daaa7cdb3fff26b49. Now that the ContentObserver is created when activity is started (even if not resumed), then it will be non-null during onActivityResult(). Therefore, the calls to onChange() will not lead to NullPointerException anymore. The reason why we want to manually call onChange() is that the ContentObserver notifications may happen several seconds later: https://gitlab.com/fdroid/fdroidclient/merge_requests/58#note_948719 --- F-Droid/src/org/fdroid/fdroid/AppDetails.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/F-Droid/src/org/fdroid/fdroid/AppDetails.java b/F-Droid/src/org/fdroid/fdroid/AppDetails.java index 5be3f9e23..106115846 100644 --- a/F-Droid/src/org/fdroid/fdroid/AppDetails.java +++ b/F-Droid/src/org/fdroid/fdroid/AppDetails.java @@ -929,6 +929,7 @@ public class AppDetails extends ActionBarActivity implements ProgressListener, A } setSupportProgressBarIndeterminateVisibility(false); + myAppObserver.onChange(); } }); } @@ -940,6 +941,7 @@ public class AppDetails extends ActionBarActivity implements ProgressListener, A @Override public void run() { setSupportProgressBarIndeterminateVisibility(false); + myAppObserver.onChange(); } }); } else { @@ -947,6 +949,7 @@ public class AppDetails extends ActionBarActivity implements ProgressListener, A @Override public void run() { setSupportProgressBarIndeterminateVisibility(false); + myAppObserver.onChange(); Log.e(TAG, "Installer aborted with errorCode: " + errorCode); From eef1e0a406c2f5879a62be281334878cc17afc22 Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Fri, 13 Mar 2015 10:37:57 +0100 Subject: [PATCH 5/5] Externalize application changed actions When we receive notifications indicating that the app has changed, the App object needs to be changed and the view updated. These notifications can be received from two sources: - the ContentObserver; - onActivityResult(). Thus, the implementation should not be related to the ContentObserver (in theory, we might want to keep only the onActivityResult() notification). Therefore, move it to a separate method in AppDetails. This also preventively avoids bugs when the ContentObserver is null. --- F-Droid/src/org/fdroid/fdroid/AppDetails.java | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/F-Droid/src/org/fdroid/fdroid/AppDetails.java b/F-Droid/src/org/fdroid/fdroid/AppDetails.java index 106115846..1f51f12d4 100644 --- a/F-Droid/src/org/fdroid/fdroid/AppDetails.java +++ b/F-Droid/src/org/fdroid/fdroid/AppDetails.java @@ -144,18 +144,9 @@ public class AppDetails extends ActionBarActivity implements ProgressListener, A @Override public void onChange(boolean selfChange, Uri uri) { - onChange(); + onAppChanged(); } - public void onChange() { - if (!reset(app.id)) { - AppDetails.this.finish(); - return; - } - - refreshApkList(); - supportInvalidateOptionsMenu(); - } } class ApkListAdapter extends ArrayAdapter { @@ -546,6 +537,16 @@ public class AppDetails extends ActionBarActivity implements ProgressListener, A removeProgressDialog(); } + private void onAppChanged() { + if (!reset(app.id)) { + AppDetails.this.finish(); + return; + } + + refreshApkList(); + supportInvalidateOptionsMenu(); + } + public void setIgnoreUpdates(String appId, boolean ignoreAll, int ignoreVersionCode) { Uri uri = AppProvider.getContentUri(appId); @@ -929,7 +930,7 @@ public class AppDetails extends ActionBarActivity implements ProgressListener, A } setSupportProgressBarIndeterminateVisibility(false); - myAppObserver.onChange(); + onAppChanged(); } }); } @@ -941,7 +942,7 @@ public class AppDetails extends ActionBarActivity implements ProgressListener, A @Override public void run() { setSupportProgressBarIndeterminateVisibility(false); - myAppObserver.onChange(); + onAppChanged(); } }); } else { @@ -949,7 +950,7 @@ public class AppDetails extends ActionBarActivity implements ProgressListener, A @Override public void run() { setSupportProgressBarIndeterminateVisibility(false); - myAppObserver.onChange(); + onAppChanged(); Log.e(TAG, "Installer aborted with errorCode: " + errorCode);