From bb9d4a9c9708a568dddf2b1c637ae54ec53caa25 Mon Sep 17 00:00:00 2001 From: Damyan Ivanov Date: Fri, 29 Mar 2019 15:29:07 +0200 Subject: [PATCH] fix observer leak move observers from AccountSummaryFragment and TransactionListFragment to MainActivity; drop them when the activity is destroyed the observers were leaked leading to multiple duplicate change notifications --- .../AccountSummaryFragment.java | 2 - .../ui/activity/MainActivity.java | 120 +++++++++++------- .../profiles/ProfilesRecyclerViewAdapter.java | 18 +-- .../TransactionListFragment.java | 35 ++--- 4 files changed, 96 insertions(+), 79 deletions(-) diff --git a/app/src/main/java/net/ktnx/mobileledger/ui/account_summary/AccountSummaryFragment.java b/app/src/main/java/net/ktnx/mobileledger/ui/account_summary/AccountSummaryFragment.java index 73a75af4..14ec0209 100644 --- a/app/src/main/java/net/ktnx/mobileledger/ui/account_summary/AccountSummaryFragment.java +++ b/app/src/main/java/net/ktnx/mobileledger/ui/account_summary/AccountSummaryFragment.java @@ -167,8 +167,6 @@ public class AccountSummaryFragment extends MobileLedgerListFragment { Data.accounts.addObserver( (o, arg) -> mActivity.runOnUiThread(() -> modelAdapter.notifyDataSetChanged())); - Data.profile.addObserver((o, arg) -> mActivity.runOnUiThread( - AccountSummaryViewModel::scheduleAccountListReload)); update_account_table(); } private void update_account_table() { diff --git a/app/src/main/java/net/ktnx/mobileledger/ui/activity/MainActivity.java b/app/src/main/java/net/ktnx/mobileledger/ui/activity/MainActivity.java index 3e774a04..0e51d857 100644 --- a/app/src/main/java/net/ktnx/mobileledger/ui/activity/MainActivity.java +++ b/app/src/main/java/net/ktnx/mobileledger/ui/activity/MainActivity.java @@ -46,6 +46,7 @@ import net.ktnx.mobileledger.model.LedgerAccount; import net.ktnx.mobileledger.model.MobileLedgerProfile; import net.ktnx.mobileledger.ui.account_summary.AccountSummaryAdapter; import net.ktnx.mobileledger.ui.account_summary.AccountSummaryFragment; +import net.ktnx.mobileledger.ui.account_summary.AccountSummaryViewModel; import net.ktnx.mobileledger.ui.profiles.ProfileDetailFragment; import net.ktnx.mobileledger.ui.profiles.ProfilesRecyclerViewAdapter; import net.ktnx.mobileledger.ui.transaction_list.TransactionListFragment; @@ -58,7 +59,6 @@ import java.lang.ref.WeakReference; import java.text.DateFormat; import java.util.Date; import java.util.List; -import java.util.Observable; import java.util.Observer; import androidx.appcompat.app.ActionBarDrawerToggle; @@ -96,26 +96,14 @@ public class MainActivity extends ProfileThemedActivity { private int mCurrentPage; private String mAccountFilter; private boolean mBackMeansToAccountList = false; + private Observer profileObserver; + private Observer profilesObserver; + private Observer lastUpdateDateObserver; @Override protected void onStart() { super.onStart(); Log.d("flow", "MainActivity.onStart()"); - setupProfile(); - - updateLastUpdateTextFromDB(); - Date lastUpdate = Data.lastUpdateDate.get(); - - long now = new Date().getTime(); - if ((lastUpdate == null) || (now > (lastUpdate.getTime() + (24 * 3600 * 1000)))) { - if (lastUpdate == null) Log.d("db::", "WEB data never fetched. scheduling a fetch"); - else Log.d("db", - String.format("WEB data last fetched at %1.3f and now is %1.3f. re-fetching", - lastUpdate.getTime() / 1000f, now / 1000f)); - - scheduleTransactionListRetrieval(); - } - mViewPager.setCurrentItem(mCurrentPage, false); if (mAccountFilter != null) showTransactionsFragment(mAccountFilter); @@ -128,6 +116,18 @@ public class MainActivity extends ProfileThemedActivity { outState.putString(STATE_ACC_FILTER, TransactionListFragment.accountFilter.get()); } @Override + protected void onDestroy() { + mSectionsPagerAdapter = null; + Data.profile.deleteObserver(profileObserver); + profileObserver = null; + Data.profiles.deleteObserver(profilesObserver); + profilesObserver = null; + Data.lastUpdateDate.deleteObserver(lastUpdateDateObserver); + RecyclerView root = findViewById(R.id.nav_profile_list); + if (root != null) root.setAdapter(null); + super.onDestroy(); + } + @Override protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); Log.d("flow", "MainActivity.onCreate()"); @@ -155,12 +155,12 @@ public class MainActivity extends ProfileThemedActivity { Toolbar toolbar = findViewById(R.id.toolbar); setSupportActionBar(toolbar); - Data.profile.addObserver((o, arg) -> { + profileObserver = (o, arg) -> { MobileLedgerProfile profile = Data.profile.get(); - runOnUiThread(() -> { - if (profile == null) setTitle(R.string.app_name); - else setTitle(profile.getName()); - updateLastUpdateTextFromDB(); + MainActivity.this.runOnUiThread(() -> { + if (profile == null) MainActivity.this.setTitle(R.string.app_name); + else MainActivity.this.setTitle(profile.getName()); + MainActivity.this.updateLastUpdateTextFromDB(); if (profile.isPostingPermitted()) { toolbar.setSubtitle(null); fab.show(); @@ -184,25 +184,35 @@ public class MainActivity extends ProfileThemedActivity { } else mProfileListAdapter.notifyDataSetChanged(); - collapseProfileList(); + MainActivity.this.collapseProfileList(); int newProfileTheme = profile.getThemeId(); if (newProfileTheme != Colors.profileThemeId) { Log.d("profiles", String.format("profile theme %d → %d", Colors.profileThemeId, newProfileTheme)); - profileThemeChanged(); + MainActivity.this.profileThemeChanged(); Colors.profileThemeId = newProfileTheme; } - else - drawer.closeDrawers(); + else drawer.closeDrawers(); + + Log.d("transactions", "requesting list reload"); + TransactionListViewModel.scheduleTransactionListReload(); + + AccountSummaryViewModel.scheduleAccountListReload(); + }); - }); - Data.profiles.addObserver((o, arg) -> { + }; + Data.profile.addObserver(profileObserver); + profilesObserver = (o, arg) -> { findViewById(R.id.nav_profile_list).setMinimumHeight( (int) (getResources().getDimension(R.dimen.thumb_row_height) * Data.profiles.size())); - mProfileListAdapter.notifyDataSetChanged(); - }); + + Log.d("profiles", "profile list changed"); + if (arg == null) mProfileListAdapter.notifyDataSetChanged(); + else mProfileListAdapter.notifyItemChanged((int) arg); + }; + Data.profiles.addObserver(profilesObserver); ActionBarDrawerToggle toggle = new ActionBarDrawerToggle(this, drawer, toolbar, R.string.navigation_drawer_open, @@ -256,10 +266,11 @@ public class MainActivity extends ProfileThemedActivity { mAccountFilter = savedInstanceState.getString(STATE_ACC_FILTER, null); } - Data.lastUpdateDate.addObserver((o, arg) -> { + lastUpdateDateObserver = (o, arg) -> { Log.d("main", "lastUpdateDate changed"); runOnUiThread(this::updateLastUpdateDisplay); - }); + }; + Data.lastUpdateDate.addObserver(lastUpdateDateObserver); updateLastUpdateDisplay(); @@ -275,26 +286,22 @@ public class MainActivity extends ProfileThemedActivity { if (root == null) throw new RuntimeException("Can't get hold on the transaction value view"); - mProfileListAdapter = new ProfilesRecyclerViewAdapter(); + if (mProfileListAdapter == null) mProfileListAdapter = new ProfilesRecyclerViewAdapter(); root.setAdapter(mProfileListAdapter); - mProfileListAdapter.addEditingProfilesObserver(new Observer() { - @Override - public void update(Observable o, Object arg) { - if (mProfileListAdapter.isEditingProfiles()) { - profileListHeadArrow.clearAnimation(); - profileListHeadArrow.setVisibility(View.GONE); - profileListHeadMore.setVisibility(View.GONE); - profileListHeadCancel.setVisibility(View.VISIBLE); - } - else { - profileListHeadArrow.setRotation(180f); - profileListHeadArrow.setVisibility(View.VISIBLE); - profileListHeadCancel.setVisibility(View.GONE); - profileListHeadMore.setVisibility(View.GONE); - profileListHeadMore - .setVisibility(profileListExpanded ? View.VISIBLE : View.GONE); - } + mProfileListAdapter.addEditingProfilesObserver((o, arg) -> { + if (mProfileListAdapter.isEditingProfiles()) { + profileListHeadArrow.clearAnimation(); + profileListHeadArrow.setVisibility(View.GONE); + profileListHeadMore.setVisibility(View.GONE); + profileListHeadCancel.setVisibility(View.VISIBLE); + } + else { + profileListHeadArrow.setRotation(180f); + profileListHeadArrow.setVisibility(View.VISIBLE); + profileListHeadCancel.setVisibility(View.GONE); + profileListHeadMore.setVisibility(View.GONE); + profileListHeadMore.setVisibility(profileListExpanded ? View.VISIBLE : View.GONE); } }); @@ -315,6 +322,21 @@ public class MainActivity extends ProfileThemedActivity { collapseProfileList(); } }); + + setupProfile(); + + updateLastUpdateTextFromDB(); + Date lastUpdate = Data.lastUpdateDate.get(); + + long now = new Date().getTime(); + if ((lastUpdate == null) || (now > (lastUpdate.getTime() + (24 * 3600 * 1000)))) { + if (lastUpdate == null) Log.d("db::", "WEB data never fetched. scheduling a fetch"); + else Log.d("db", + String.format("WEB data last fetched at %1.3f and now is %1.3f. re-fetching", + lastUpdate.getTime() / 1000f, now / 1000f)); + + scheduleTransactionListRetrieval(); + } } private void updateLastUpdateDisplay() { LinearLayout l = findViewById(R.id.transactions_last_update_layout); diff --git a/app/src/main/java/net/ktnx/mobileledger/ui/profiles/ProfilesRecyclerViewAdapter.java b/app/src/main/java/net/ktnx/mobileledger/ui/profiles/ProfilesRecyclerViewAdapter.java index dfffc8f2..eb3d0c85 100644 --- a/app/src/main/java/net/ktnx/mobileledger/ui/profiles/ProfilesRecyclerViewAdapter.java +++ b/app/src/main/java/net/ktnx/mobileledger/ui/profiles/ProfilesRecyclerViewAdapter.java @@ -49,20 +49,10 @@ public class ProfilesRecyclerViewAdapter editProfile(view, profile); }; private ObservableValue editingProfiles = new ObservableValue<>(false); - public void addEditingProfilesObserver(Observer o) { - editingProfiles.addObserver(o); - } - public void deleteEditingProfilesObserver(Observer o) { - editingProfiles.deleteObserver(o); - } private RecyclerView recyclerView; private ItemTouchHelper rearrangeHelper; public ProfilesRecyclerViewAdapter() { - Data.profiles.addObserver((o, arg) -> { - Log.d("profiles", "profile list changed"); - if (arg == null) notifyDataSetChanged(); - else notifyItemChanged((int) arg); - }); + Log.d("flow", "ProfilesRecyclerViewAdapter.new()"); ItemTouchHelper.Callback cb = new ItemTouchHelper.Callback() { @Override @@ -86,6 +76,12 @@ public class ProfilesRecyclerViewAdapter }; rearrangeHelper = new ItemTouchHelper(cb); } + public void addEditingProfilesObserver(Observer o) { + editingProfiles.addObserver(o); + } + public void deleteEditingProfilesObserver(Observer o) { + editingProfiles.deleteObserver(o); + } @Override public void onDetachedFromRecyclerView(@NonNull RecyclerView recyclerView) { rearrangeHelper.attachToRecyclerView(null); diff --git a/app/src/main/java/net/ktnx/mobileledger/ui/transaction_list/TransactionListFragment.java b/app/src/main/java/net/ktnx/mobileledger/ui/transaction_list/TransactionListFragment.java index 82082a90..631205f2 100644 --- a/app/src/main/java/net/ktnx/mobileledger/ui/transaction_list/TransactionListFragment.java +++ b/app/src/main/java/net/ktnx/mobileledger/ui/transaction_list/TransactionListFragment.java @@ -20,11 +20,6 @@ package net.ktnx.mobileledger.ui.transaction_list; import android.content.Context; import android.database.MatrixCursor; import android.os.Bundle; -import androidx.annotation.NonNull; -import androidx.annotation.Nullable; -import com.google.android.material.floatingactionbutton.FloatingActionButton; -import androidx.recyclerview.widget.LinearLayoutManager; -import androidx.recyclerview.widget.RecyclerView; import android.util.Log; import android.view.LayoutInflater; import android.view.Menu; @@ -36,6 +31,8 @@ import android.view.inputmethod.InputMethodManager; import android.widget.AutoCompleteTextView; import android.widget.Toast; +import com.google.android.material.floatingactionbutton.FloatingActionButton; + import net.ktnx.mobileledger.R; import net.ktnx.mobileledger.model.Data; import net.ktnx.mobileledger.ui.MobileLedgerListFragment; @@ -48,6 +45,11 @@ import net.ktnx.mobileledger.utils.ObservableValue; import java.util.Observable; import java.util.Observer; +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; +import androidx.recyclerview.widget.LinearLayoutManager; +import androidx.recyclerview.widget.RecyclerView; + import static android.content.Context.INPUT_METHOD_SERVICE; public class TransactionListFragment extends MobileLedgerListFragment { @@ -58,6 +60,7 @@ public class TransactionListFragment extends MobileLedgerListFragment { private View vAccountFilter; private AutoCompleteTextView accNameFilter; private Observer backgroundTaskCountObserver; + private Observer accountFilterObserver; private static void update(Observable o, Object arg) { } @Override @@ -184,18 +187,16 @@ public class TransactionListFragment extends MobileLedgerListFragment { Globals.hideSoftKeyboard(mActivity); }); - accountFilter.addObserver((o, arg) -> { - String accountName = accountFilter.get(); - modelAdapter.setBoldAccountName(accountName); - setShowOnlyAccountName(accountName); - TransactionListViewModel.scheduleTransactionListReload(); - if (menuTransactionListFilter != null) menuTransactionListFilter.setVisible(false); - }); - - Data.profile.addObserver((o, arg) -> mActivity.runOnUiThread(() -> { - Log.d("transactions", "requesting list reload"); - TransactionListViewModel.scheduleTransactionListReload(); - })); + if (accountFilterObserver == null) { + accountFilterObserver = (o, arg) -> { + String accountName = accountFilter.get(); + modelAdapter.setBoldAccountName(accountName); + setShowOnlyAccountName(accountName); + TransactionListViewModel.scheduleTransactionListReload(); + if (menuTransactionListFilter != null) menuTransactionListFilter.setVisible(false); + }; + accountFilter.addObserver(accountFilterObserver); + } TransactionListViewModel.scheduleTransactionListReload(); TransactionListViewModel.updating.addObserver( -- 2.39.2