From b19861226a8e7b128e06055b2d0da79a1aa607e2 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Mon, 21 Mar 2016 22:21:09 +1100 Subject: [PATCH 1/5] Correctly unregister receiver in "join wifi" swap view. Previously the receiver was added but never removed. The result is that once a swap session is cancelled, the receiver still gets broadcasts. --- .../fdroid/views/swap/JoinWifiView.java | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/F-Droid/src/org/fdroid/fdroid/views/swap/JoinWifiView.java b/F-Droid/src/org/fdroid/fdroid/views/swap/JoinWifiView.java index 425488795..29d27aa69 100644 --- a/F-Droid/src/org/fdroid/fdroid/views/swap/JoinWifiView.java +++ b/F-Droid/src/org/fdroid/fdroid/views/swap/JoinWifiView.java @@ -60,19 +60,23 @@ public class JoinWifiView extends RelativeLayout implements SwapWorkflowActivity }); refreshWifiState(); - // TODO: This is effectively swap state management code, shouldn't be isolated to the - // WifiStateChangeService, but should be bundled with the main swap state handling code. LocalBroadcastManager.getInstance(getActivity()).registerReceiver( - new BroadcastReceiver() { - @Override - public void onReceive(Context context, Intent intent) { - refreshWifiState(); - } - }, + onWifiStateChange, new IntentFilter(WifiStateChangeService.BROADCAST) ); } + /** + * Remove relevant listeners/receivers/etc so that they do not receive and process events + * when this view is not in use. + */ + @Override + protected void onDetachedFromWindow() { + super.onDetachedFromWindow(); + + LocalBroadcastManager.getInstance(getActivity()).unregisterReceiver(onWifiStateChange); + } + // TODO: Listen for "Connecting..." state and reflect that in the view too. private void refreshWifiState() { TextView descriptionView = (TextView) findViewById(R.id.text_description); @@ -138,4 +142,11 @@ public class JoinWifiView extends RelativeLayout implements SwapWorkflowActivity public String getToolbarTitle() { return getResources().getString(R.string.swap_join_same_wifi); } + + private final BroadcastReceiver onWifiStateChange = new BroadcastReceiver() { + @Override + public void onReceive(Context context, Intent intent) { + refreshWifiState(); + } + }; } From c29aff21677ae3d43cac7016fd3e3ed2a025d36a Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Mon, 21 Mar 2016 22:22:57 +1100 Subject: [PATCH 2/5] Correctly unregister receiver in "connecting" swap view (Fixes #409) Previously the receiver was added but never removed. The result is that once a swap session is cancelled, the receiver still gets broadcasts. This is what was causing the bug in #409. It was trying to access the `Activity` once it had been closed, and another swap session started with a new activity. --- .../fdroid/fdroid/views/swap/SwapConnecting.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/F-Droid/src/org/fdroid/fdroid/views/swap/SwapConnecting.java b/F-Droid/src/org/fdroid/fdroid/views/swap/SwapConnecting.java index a14c82021..387ffdceb 100644 --- a/F-Droid/src/org/fdroid/fdroid/views/swap/SwapConnecting.java +++ b/F-Droid/src/org/fdroid/fdroid/views/swap/SwapConnecting.java @@ -61,13 +61,24 @@ public class SwapConnecting extends LinearLayout implements SwapWorkflowActivity } }); - // TODO: Unregister correctly, not just when being notified of completion or errors. LocalBroadcastManager.getInstance(getActivity()).registerReceiver( repoUpdateReceiver, new IntentFilter(UpdateService.LOCAL_ACTION_STATUS)); LocalBroadcastManager.getInstance(getActivity()).registerReceiver( prepareSwapReceiver, new IntentFilter(SwapWorkflowActivity.PrepareSwapRepo.ACTION)); } + /** + * Remove relevant listeners/receivers/etc so that they do not receive and process events + * when this view is not in use. + */ + @Override + protected void onDetachedFromWindow() { + super.onDetachedFromWindow(); + + LocalBroadcastManager.getInstance(getActivity()).unregisterReceiver(repoUpdateReceiver); + LocalBroadcastManager.getInstance(getActivity()).unregisterReceiver(prepareSwapReceiver); + } + private final BroadcastReceiver repoUpdateReceiver = new ConnectSwapReceiver(); private final BroadcastReceiver prepareSwapReceiver = new PrepareSwapReceiver(); From cfe2f71e4d73b87b8398a9e4fc7399c3697a7445 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Mon, 21 Mar 2016 22:23:54 +1100 Subject: [PATCH 3/5] Correctly unregister receiver in "qr code" swap view. Previously the receiver was added but never removed. The result is that once a swap session is cancelled, the receiver still gets broadcasts. --- .../fdroid/fdroid/views/swap/WifiQrView.java | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/F-Droid/src/org/fdroid/fdroid/views/swap/WifiQrView.java b/F-Droid/src/org/fdroid/fdroid/views/swap/WifiQrView.java index bb4447348..34fe99e8c 100644 --- a/F-Droid/src/org/fdroid/fdroid/views/swap/WifiQrView.java +++ b/F-Droid/src/org/fdroid/fdroid/views/swap/WifiQrView.java @@ -78,16 +78,19 @@ public class WifiQrView extends ScrollView implements SwapWorkflowActivity.Inner } }); - // TODO: Unregister this receiver properly. LocalBroadcastManager.getInstance(getActivity()).registerReceiver( - new BroadcastReceiver() { - @Override - public void onReceive(Context context, Intent intent) { - setUIFromWifi(); - } - }, - new IntentFilter(WifiStateChangeService.BROADCAST) - ); + onWifiStateChanged, new IntentFilter(WifiStateChangeService.BROADCAST)); + } + + /** + * Remove relevant listeners/receivers/etc so that they do not receive and process events + * when this view is not in use. + */ + @Override + protected void onDetachedFromWindow() { + super.onDetachedFromWindow(); + + LocalBroadcastManager.getInstance(getActivity()).unregisterReceiver(onWifiStateChanged); } @Override @@ -167,4 +170,11 @@ public class WifiQrView extends ScrollView implements SwapWorkflowActivity.Inner } + private BroadcastReceiver onWifiStateChanged = new BroadcastReceiver() { + @Override + public void onReceive(Context context, Intent intent) { + setUIFromWifi(); + } + }; + } From d46efb1d8414d9331f166d7147255752bf35bb2d Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Mon, 21 Mar 2016 22:26:58 +1100 Subject: [PATCH 4/5] Correctly start and stop listening for repo update events during swap. This fixes the following bugs: * `BroadcastReceiver` was never being created due to incorrect guard condition `if (pollForUpdatesReceiver != null)` (should have been `== null`). * Called `unregisterReceiver` rather than `registerReceiver`. * Even if it did work, it didn't make an effort to unregister the receiver. In addition, the creation and listening with the `BroadcastReceiver is now done in a way similar to the other swap views: * Create it as a `final` member variable. * `registerReceiver` when view is inflated. * `unregisterReceiver` when view is detached. --- .../fdroid/views/swap/SwapAppsView.java | 72 ++++++++++--------- 1 file changed, 40 insertions(+), 32 deletions(-) diff --git a/F-Droid/src/org/fdroid/fdroid/views/swap/SwapAppsView.java b/F-Droid/src/org/fdroid/fdroid/views/swap/SwapAppsView.java index 6d955039f..399c8621e 100644 --- a/F-Droid/src/org/fdroid/fdroid/views/swap/SwapAppsView.java +++ b/F-Droid/src/org/fdroid/fdroid/views/swap/SwapAppsView.java @@ -118,10 +118,22 @@ public class SwapAppsView extends ListView implements displayImageOptions = Utils.getImageLoadingOptions().build(); + LocalBroadcastManager.getInstance(getActivity()).registerReceiver( + pollForUpdatesReceiver, new IntentFilter(UpdateService.LOCAL_ACTION_STATUS)); + schedulePollForUpdates(); } - private BroadcastReceiver pollForUpdatesReceiver; + /** + * Remove relevant listeners/receivers/etc so that they do not receive and process events + * when this view is not in use. + */ + @Override + public void onDetachedFromWindow() { + super.onDetachedFromWindow(); + + LocalBroadcastManager.getInstance(getActivity()).unregisterReceiver(pollForUpdatesReceiver); + } private void pollForUpdates() { if (adapter.getCount() > 1 || @@ -132,37 +144,6 @@ public class SwapAppsView extends ListView implements Utils.debugLog(TAG, "Polling swap repo to see if it has any updates."); getActivity().getService().refreshSwap(); - if (pollForUpdatesReceiver != null) { - pollForUpdatesReceiver = new BroadcastReceiver() { - @Override - public void onReceive(Context context, Intent intent) { - int statusCode = intent.getIntExtra(UpdateService.EXTRA_STATUS_CODE, -1); - switch (statusCode) { - case UpdateService.STATUS_COMPLETE_WITH_CHANGES: - Utils.debugLog(TAG, "Swap repo has updates, notifying the list adapter."); - getActivity().runOnUiThread(new Runnable() { - @Override - public void run() { - adapter.notifyDataSetChanged(); - } - }); - LocalBroadcastManager.getInstance(getActivity()).unregisterReceiver(pollForUpdatesReceiver); - break; - - case UpdateService.STATUS_ERROR_GLOBAL: - // TODO: Well, if we can't get the index, we probably can't swapp apps. - // Tell the user somethign helpful? - break; - - case UpdateService.STATUS_COMPLETE_AND_SAME: - schedulePollForUpdates(); - break; - } - } - }; - // TODO: Unregister this properly, not just when successful (see swithc statement above) - LocalBroadcastManager.getInstance(getActivity()).unregisterReceiver(pollForUpdatesReceiver); - } } private void schedulePollForUpdates() { @@ -486,4 +467,31 @@ public class SwapAppsView extends ListView implements } } + private final BroadcastReceiver pollForUpdatesReceiver = new BroadcastReceiver() { + @Override + public void onReceive(Context context, Intent intent) { + int statusCode = intent.getIntExtra(UpdateService.EXTRA_STATUS_CODE, -1); + switch (statusCode) { + case UpdateService.STATUS_COMPLETE_WITH_CHANGES: + Utils.debugLog(TAG, "Swap repo has updates, notifying the list adapter."); + getActivity().runOnUiThread(new Runnable() { + @Override + public void run() { + adapter.notifyDataSetChanged(); + } + }); + break; + + case UpdateService.STATUS_ERROR_GLOBAL: + // TODO: Well, if we can't get the index, we probably can't swapp apps. + // Tell the user something helpful? + break; + + case UpdateService.STATUS_COMPLETE_AND_SAME: + schedulePollForUpdates(); + break; + } + } + }; + } From a7d757cdb2f10efffa0dd1174c5453438cd3c8c4 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Mon, 21 Mar 2016 22:36:15 +1100 Subject: [PATCH 5/5] Remove unused code and fix some typos. The old swap code used to delegate to the `AppDetails` activity when touching an app in the swap view. Now it shows the install button and download feedback inline. The code which used to exist is no longer required. --- .../fdroid/views/swap/SwapAppsView.java | 28 ++----------------- 1 file changed, 2 insertions(+), 26 deletions(-) diff --git a/F-Droid/src/org/fdroid/fdroid/views/swap/SwapAppsView.java b/F-Droid/src/org/fdroid/fdroid/views/swap/SwapAppsView.java index 399c8621e..2598c76a2 100644 --- a/F-Droid/src/org/fdroid/fdroid/views/swap/SwapAppsView.java +++ b/F-Droid/src/org/fdroid/fdroid/views/swap/SwapAppsView.java @@ -8,7 +8,6 @@ import android.content.Intent; import android.content.IntentFilter; import android.database.ContentObserver; import android.database.Cursor; -import android.graphics.drawable.Drawable; import android.net.Uri; import android.os.Build; import android.os.Bundle; @@ -32,7 +31,6 @@ import android.view.MenuInflater; import android.view.MenuItem; import android.view.View; import android.view.ViewGroup; -import android.widget.AdapterView; import android.widget.Button; import android.widget.ImageView; import android.widget.ListView; @@ -110,12 +108,6 @@ public class SwapAppsView extends ListView implements // either reconnect with an existing loader or start a new one getActivity().getSupportLoaderManager().initLoader(LOADER_SWAPABLE_APPS, null, this); - setOnItemClickListener(new OnItemClickListener() { - public void onItemClick(AdapterView parent, View v, int position, long id) { - showAppDetails(position); - } - }); - displayImageOptions = Utils.getImageLoadingOptions().build(); LocalBroadcastManager.getInstance(getActivity()).registerReceiver( @@ -193,12 +185,6 @@ public class SwapAppsView extends ListView implements return getResources().getString(R.string.swap_success); } - private void showAppDetails(int position) { - Cursor c = (Cursor) adapter.getItem(position); - App app = new App(c); - // TODO: Show app details screen. - } - @Override public CursorLoader onCreateLoader(int id, Bundle args) { Uri uri = TextUtils.isEmpty(mCurrentFilterString) @@ -335,10 +321,10 @@ public class SwapAppsView extends ListView implements this.app = app; apkToInstall = null; // Force lazy loading to fetch the correct apk next time. - // NOTE: Instead of continually unregistering and reregistering the observer + // NOTE: Instead of continually unregistering and re-registering the observer // (with a different URI), this could equally be done by only having one // registration in the constructor, and using the ContentObserver.onChange(boolean, URI) - // method and inspecting the URI to see if it maches. However, this was only + // method and inspecting the URI to see if it matches. However, this was only // implemented on API-16, so leaving like this for now. getActivity().getContentResolver().unregisterContentObserver(appObserver); getActivity().getContentResolver().registerContentObserver( @@ -418,9 +404,6 @@ public class SwapAppsView extends ListView implements @Nullable private LayoutInflater inflater; - @Nullable - private Drawable defaultAppIcon; - AppListAdapter(@NonNull Context context, @Nullable Cursor c) { super(context, c, FLAG_REGISTER_CONTENT_OBSERVER); } @@ -433,13 +416,6 @@ public class SwapAppsView extends ListView implements return inflater; } - private Drawable getDefaultAppIcon(Context context) { - if (defaultAppIcon == null) { - defaultAppIcon = context.getResources().getDrawable(android.R.drawable.sym_def_app_icon); - } - return defaultAppIcon; - } - @Override public View newView(Context context, Cursor cursor, ViewGroup parent) { View view = getInflater(context).inflate(R.layout.swap_app_list_item, parent, false);