jenkins-bot has submitted this change and it was merged.
Change subject: Fix memory leaks (for real!), part 1.
......................................................................
Fix memory leaks (for real!), part 1.
My ongoing search has finally yielded some tangible nuggets.
Exhibit A: Adding a TextWatcher to a TextView, but never removing it! (The
TextWatcher holds a reference to the fragment, thereby leaking it)
Also, now explicitly removing the onItemClick and onItemLongClick
listeners from the ListView, since this seems to lead to a leak in
API <20.
Change-Id: I497ebd37f3059b420622416a66ee5b87a15a2d0c
---
M app/src/main/java/org/wikipedia/history/HistoryFragment.java
M app/src/main/java/org/wikipedia/savedpages/SavedPagesFragment.java
2 files changed, 231 insertions(+), 215 deletions(-)
Approvals:
Niedzielski: Looks good to me, approved
jenkins-bot: Verified
diff --git a/app/src/main/java/org/wikipedia/history/HistoryFragment.java
b/app/src/main/java/org/wikipedia/history/HistoryFragment.java
index 52aea5c..223f6fc 100644
--- a/app/src/main/java/org/wikipedia/history/HistoryFragment.java
+++ b/app/src/main/java/org/wikipedia/history/HistoryFragment.java
@@ -55,6 +55,9 @@
private WikipediaApp app;
private ActionMode actionMode;
+ private HistorySearchTextWatcher textWatcher = new
HistorySearchTextWatcher();
+ private HistoryItemClickListener itemClickListener = new
HistoryItemClickListener();
+ private HistoryItemLongClickListener itemLongClickListener = new
HistoryItemLongClickListener();
private boolean firstRun = true;
@@ -63,20 +66,24 @@
super.onCreate(savedInstanceState);
setRetainInstance(true);
app = WikipediaApp.getInstance();
+ adapter = new HistoryEntryAdapter(getContext(), null, true);
}
@Override
public View onCreateView(LayoutInflater inflater, ViewGroup container,
Bundle savedInstanceState) {
View rootView = inflater.inflate(R.layout.fragment_history, container,
false);
rootView.setPadding(0, getContentTopOffsetPx(getActivity()), 0, 0);
-
historyEntryList = (ListView)
rootView.findViewById(R.id.history_entry_list);
historyEmptyContainer =
rootView.findViewById(R.id.history_empty_container);
historyEmptyTitle = (TextView)
rootView.findViewById(R.id.history_empty_title);
historyEmptyMessage = (TextView)
rootView.findViewById(R.id.history_empty_message);
entryFilter = (EditText)
rootView.findViewById(R.id.history_search_list);
-
app.adjustDrawableToTheme(((ImageView)
rootView.findViewById(R.id.history_empty_image)).getDrawable());
+
+ entryFilter.addTextChangedListener(textWatcher);
+ historyEntryList.setAdapter(adapter);
+ historyEntryList.setOnItemClickListener(itemClickListener);
+ historyEntryList.setOnItemLongClickListener(itemLongClickListener);
return rootView;
}
@@ -84,102 +91,7 @@
public void onActivityCreated(Bundle savedInstanceState) {
super.onActivityCreated(savedInstanceState);
setHasOptionsMenu(true);
- adapter = new HistoryEntryAdapter(getActivity(), null, true);
- historyEntryList.setAdapter(adapter);
historyEntryList.setEmptyView(historyEmptyContainer);
-
- entryFilter.addTextChangedListener(
- new TextWatcher() {
- @Override
- public void beforeTextChanged(CharSequence charSequence,
int i, int i2, int i3) {
- // Do nothing
- }
-
- @Override
- public void onTextChanged(CharSequence charSequence, int
i, int i2, int i3) {
- // Do nothing
- }
-
- @Override
- public void afterTextChanged(Editable editable) {
-
getActivity().getSupportLoaderManager().restartLoader(HISTORY_FRAGMENT_LOADER_ID,
null, HistoryFragment.this);
- if (editable.length() == 0) {
-
historyEmptyTitle.setText(R.string.history_empty_title);
- historyEmptyMessage.setVisibility(View.VISIBLE);
- } else {
-
historyEmptyTitle.setText(getString(R.string.history_search_empty_message));
- historyEmptyMessage.setVisibility(View.GONE);
- }
- }
- });
-
- historyEntryList.setOnItemClickListener(new
AdapterView.OnItemClickListener() {
- @Override
- public void onItemClick(AdapterView<?> parent, View view, int
position, long id) {
- if (actionMode == null) {
- HistoryEntry oldEntry = (HistoryEntry) view.getTag();
- HistoryEntry newEntry = new
HistoryEntry(oldEntry.getTitle(), HistoryEntry.SOURCE_HISTORY);
- ((PageActivity)
getActivity()).loadPage(oldEntry.getTitle(), newEntry);
- }
- }
- });
-
- historyEntryList.setOnItemLongClickListener(new
AdapterView.OnItemLongClickListener() {
- @Override
- public boolean onItemLongClick(AdapterView<?> parent, View view,
int position, long id) {
- if (actionMode != null) {
- return false;
- }
-
historyEntryList.setChoiceMode(AbsListView.CHOICE_MODE_MULTIPLE);
- actionMode =
((AppCompatActivity)getActivity()).startSupportActionMode(new
ActionMode.Callback() {
- private final String actionModeTag = "actionModeHistory";
- @Override
- public boolean onCreateActionMode(ActionMode mode, Menu
menu) {
-
mode.getMenuInflater().inflate(R.menu.menu_history_context, menu);
- return true;
- }
-
- @Override
- public boolean onPrepareActionMode(ActionMode mode, Menu
menu) {
- mode.setTag(actionModeTag);
- return false;
- }
-
- @Override
- public boolean onActionItemClicked(ActionMode mode,
MenuItem item) {
- if (item.getItemId() ==
R.id.menu_delete_selected_history) {
- SparseBooleanArray checkedItems =
historyEntryList.getCheckedItemPositions();
- for (int i = 0; i < checkedItems.size(); i++) {
- if (checkedItems.valueAt(i)) {
-
app.getDatabaseClient(HistoryEntry.class).delete(
-
HistoryEntry.DATABASE_TABLE.fromCursor((Cursor)
adapter.getItem(checkedItems.keyAt(i))),
-
HistoryEntryDatabaseTable.SELECTION_KEYS);
- }
- }
- if (checkedItems.size() ==
historyEntryList.getAdapter().getCount()) {
- entryFilter.setVisibility(View.GONE);
- }
- mode.finish();
- return true;
- } else {
- throw new RuntimeException("Unknown context menu
item clicked");
- }
- }
-
- @Override
- public void onDestroyActionMode(ActionMode mode) {
-
historyEntryList.setChoiceMode(AbsListView.CHOICE_MODE_SINGLE);
- actionMode = null;
- // Clear all selections
- historyEntryList.clearChoices();
- historyEntryList.requestLayout(); // Required to
immediately redraw unchecked states
- }
- });
- historyEntryList.setItemChecked(position, true);
- return true;
- }
- });
-
getActivity().getSupportLoaderManager().initLoader(HISTORY_FRAGMENT_LOADER_ID,
null, this);
getActivity().getSupportLoaderManager().restartLoader(HISTORY_FRAGMENT_LOADER_ID,
null, this);
}
@@ -187,6 +99,10 @@
@Override
public void onDestroyView() {
getActivity().getSupportLoaderManager().destroyLoader(HISTORY_FRAGMENT_LOADER_ID);
+ entryFilter.removeTextChangedListener(textWatcher);
+ historyEntryList.setOnItemClickListener(null);
+ historyEntryList.setOnItemLongClickListener(null);
+ historyEntryList.setAdapter(null);
super.onDestroyView();
}
@@ -331,4 +247,95 @@
return super.onOptionsItemSelected(item);
}
}
+
+ private class HistoryItemClickListener implements
AdapterView.OnItemClickListener {
+ @Override
+ public void onItemClick(AdapterView<?> parent, View view, int
position, long id) {
+ if (actionMode == null) {
+ HistoryEntry oldEntry = (HistoryEntry) view.getTag();
+ HistoryEntry newEntry = new HistoryEntry(oldEntry.getTitle(),
HistoryEntry.SOURCE_HISTORY);
+ ((PageActivity) getActivity()).loadPage(oldEntry.getTitle(),
newEntry);
+ }
+ }
+ }
+
+ private class HistoryItemLongClickListener implements
AdapterView.OnItemLongClickListener {
+ @Override
+ public boolean onItemLongClick(AdapterView<?> parent, View view, int
position, long id) {
+ if (actionMode != null) {
+ return false;
+ }
+ historyEntryList.setChoiceMode(AbsListView.CHOICE_MODE_MULTIPLE);
+ actionMode =
((AppCompatActivity)getActivity()).startSupportActionMode(new
ActionMode.Callback() {
+ private final String actionModeTag = "actionModeHistory";
+ @Override
+ public boolean onCreateActionMode(ActionMode mode, Menu menu) {
+
mode.getMenuInflater().inflate(R.menu.menu_history_context, menu);
+ return true;
+ }
+
+ @Override
+ public boolean onPrepareActionMode(ActionMode mode, Menu menu)
{
+ mode.setTag(actionModeTag);
+ return false;
+ }
+
+ @Override
+ public boolean onActionItemClicked(ActionMode mode, MenuItem
item) {
+ if (item.getItemId() == R.id.menu_delete_selected_history)
{
+ SparseBooleanArray checkedItems =
historyEntryList.getCheckedItemPositions();
+ for (int i = 0; i < checkedItems.size(); i++) {
+ if (checkedItems.valueAt(i)) {
+
app.getDatabaseClient(HistoryEntry.class).delete(
+
HistoryEntry.DATABASE_TABLE.fromCursor((Cursor)
adapter.getItem(checkedItems.keyAt(i))),
+
HistoryEntryDatabaseTable.SELECTION_KEYS);
+ }
+ }
+ if (checkedItems.size() ==
historyEntryList.getAdapter().getCount()) {
+ entryFilter.setVisibility(View.GONE);
+ }
+ mode.finish();
+ return true;
+ } else {
+ throw new RuntimeException("Unknown context menu item
clicked");
+ }
+ }
+
+ @Override
+ public void onDestroyActionMode(ActionMode mode) {
+
historyEntryList.setChoiceMode(AbsListView.CHOICE_MODE_SINGLE);
+ actionMode = null;
+ // Clear all selections
+ historyEntryList.clearChoices();
+ historyEntryList.requestLayout(); // Required to
immediately redraw unchecked states
+ }
+ });
+ historyEntryList.setItemChecked(position, true);
+ return true;
+ }
+ }
+
+ private class HistorySearchTextWatcher implements TextWatcher {
+ @Override
+ public void beforeTextChanged(CharSequence charSequence, int i, int
i2, int i3) {
+ // Do nothing
+ }
+
+ @Override
+ public void onTextChanged(CharSequence charSequence, int i, int i2,
int i3) {
+ // Do nothing
+ }
+
+ @Override
+ public void afterTextChanged(Editable editable) {
+
getActivity().getSupportLoaderManager().restartLoader(HISTORY_FRAGMENT_LOADER_ID,
null, HistoryFragment.this);
+ if (editable.length() == 0) {
+ historyEmptyTitle.setText(R.string.history_empty_title);
+ historyEmptyMessage.setVisibility(View.VISIBLE);
+ } else {
+
historyEmptyTitle.setText(getString(R.string.history_search_empty_message,
editable.toString()));
+ historyEmptyMessage.setVisibility(View.GONE);
+ }
+ }
+ }
}
diff --git a/app/src/main/java/org/wikipedia/savedpages/SavedPagesFragment.java
b/app/src/main/java/org/wikipedia/savedpages/SavedPagesFragment.java
index 736ab2d..64f7973 100644
--- a/app/src/main/java/org/wikipedia/savedpages/SavedPagesFragment.java
+++ b/app/src/main/java/org/wikipedia/savedpages/SavedPagesFragment.java
@@ -61,6 +61,9 @@
private WikipediaApp app;
private ActionMode actionMode;
+ private SavedPageSearchTextWatcher textWatcher = new
SavedPageSearchTextWatcher();
+ private SavedPageItemClickListener itemClickListener = new
SavedPageItemClickListener();
+ private SavedPageItemLongClickListener itemLongClickListener = new
SavedPageItemLongClickListener();
private boolean firstRun = true;
@@ -69,6 +72,7 @@
super.onCreate(savedInstanceState);
setRetainInstance(true);
app = WikipediaApp.getInstance();
+ adapter = new SavedPagesAdapter(getActivity(), null, true);
}
@Override
@@ -82,6 +86,12 @@
savedPagesEmptyMessage = (TextView)
rootView.findViewById(R.id.saved_pages_empty_message);
entryFilter = (EditText)
rootView.findViewById(R.id.saved_pages_search_list);
savedPagesEmptyImage = (ImageView)
rootView.findViewById(R.id.saved_pages_empty_image);
+ app.adjustDrawableToTheme(savedPagesEmptyImage.getDrawable());
+
+ entryFilter.addTextChangedListener(textWatcher);
+ savedPagesList.setAdapter(adapter);
+ savedPagesList.setOnItemClickListener(itemClickListener);
+ savedPagesList.setOnItemLongClickListener(itemLongClickListener);
return rootView;
}
@@ -89,125 +99,7 @@
public void onActivityCreated(Bundle savedInstanceState) {
super.onActivityCreated(savedInstanceState);
setHasOptionsMenu(true);
- adapter = new SavedPagesAdapter(getActivity(), null, true);
- savedPagesList.setAdapter(adapter);
savedPagesList.setEmptyView(savedPagesEmptyContainer);
-
- savedPagesList.setOnItemLongClickListener(new
AdapterView.OnItemLongClickListener() {
- @Override
- public boolean onItemLongClick(AdapterView<?> parent, View view,
int position, long id) {
- if (actionMode != null) {
- return false;
- }
- savedPagesList.setChoiceMode(AbsListView.CHOICE_MODE_MULTIPLE);
- actionMode =
((AppCompatActivity)getActivity()).startSupportActionMode(new
ActionMode.Callback() {
- private final String actionModeTag =
"actionModeSavedPages";
- @Override
- public boolean onCreateActionMode(ActionMode mode, Menu
menu) {
-
mode.getMenuInflater().inflate(R.menu.menu_saved_pages_context, menu);
- return true;
- }
-
- @Override
- public boolean onPrepareActionMode(ActionMode mode, Menu
menu) {
- mode.setTag(actionModeTag);
- return false;
- }
-
- @Override
- public boolean onActionItemClicked(ActionMode mode,
MenuItem item) {
- switch (item.getItemId()) {
- case R.id.menu_refresh_selected_saved_pages:
- refreshSelected();
- actionMode.finish();
- return true;
- case R.id.menu_delete_selected_saved_pages:
- deleteSelected();
- actionMode.finish();
- return true;
- default:
- throw new RuntimeException("Unknown context
menu item clicked");
- }
- }
-
- private void deleteSelected() {
- SparseBooleanArray checkedItems =
savedPagesList.getCheckedItemPositions();
- for (int i = 0; i < checkedItems.size(); i++) {
- if (checkedItems.valueAt(i)) {
- final SavedPage page =
SavedPage.DATABASE_TABLE.fromCursor((Cursor)
adapter.getItem(checkedItems.keyAt(i)));
- new DeleteSavedPageTask(getActivity(), page) {
- @Override
- public void onFinish(Boolean result) {
- if (!isAdded()) {
- return;
- }
- FeedbackUtil.showMessage(getActivity(),
-
R.string.snackbar_saved_page_deleted);
- }
- }.execute();
- }
- }
- if (checkedItems.size() ==
savedPagesList.getAdapter().getCount()) {
- entryFilter.setVisibility(View.GONE);
- }
- }
-
- @Override
- public void onDestroyActionMode(ActionMode mode) {
-
savedPagesList.setChoiceMode(AbsListView.CHOICE_MODE_SINGLE);
- actionMode = null;
- // Clear all selections
- savedPagesList.clearChoices();
- savedPagesList.requestLayout(); // Required to
immediately redraw unchecked states
-
- }
- });
- savedPagesList.setItemChecked(position, true);
- return true;
- }
- });
-
- entryFilter.addTextChangedListener(
- new TextWatcher() {
- @Override
- public void beforeTextChanged(CharSequence charSequence,
int i, int i2, int i3) {
- // Do nothing
- }
-
- @Override
- public void onTextChanged(CharSequence charSequence, int
i, int i2, int i3) {
- // Do nothing
- }
-
- @Override
- public void afterTextChanged(Editable editable) {
-
getActivity().getSupportLoaderManager().restartLoader(SAVED_PAGES_FRAGMENT_LOADER_ID,
null, SavedPagesFragment.this);
- if (editable.length() == 0) {
-
savedPagesEmptyTitle.setText(R.string.saved_pages_empty_title);
- savedPagesEmptyImage.setVisibility(View.VISIBLE);
- savedPagesEmptyMessage.setVisibility(View.VISIBLE);
- } else {
-
savedPagesEmptyTitle.setText(getString(R.string.saved_pages_search_empty_message));
- savedPagesEmptyImage.setVisibility(View.GONE);
- savedPagesEmptyMessage.setVisibility(View.GONE);
- }
- }
- });
-
- savedPagesList.setOnItemClickListener(new
AdapterView.OnItemClickListener() {
- @Override
- public void onItemClick(AdapterView<?> parent, View view, int
position, long id) {
- // We shouldn't do anything if the user is multi-selecting
things
- if (actionMode == null) {
- SavedPage savedPage = (SavedPage) view.getTag();
- HistoryEntry newEntry = new
HistoryEntry(savedPage.getTitle(), HistoryEntry.SOURCE_SAVED_PAGE);
-
((PageActivity)getActivity()).loadPage(savedPage.getTitle(), newEntry);
- }
- }
- });
-
- app.adjustDrawableToTheme(savedPagesEmptyImage.getDrawable());
-
getActivity().getSupportLoaderManager().initLoader(SAVED_PAGES_FRAGMENT_LOADER_ID,
null, this);
getActivity().getSupportLoaderManager().restartLoader(SAVED_PAGES_FRAGMENT_LOADER_ID,
null, this);
}
@@ -215,6 +107,10 @@
@Override
public void onDestroyView() {
getActivity().getSupportLoaderManager().destroyLoader(SAVED_PAGES_FRAGMENT_LOADER_ID);
+ entryFilter.removeTextChangedListener(textWatcher);
+ savedPagesList.setAdapter(null);
+ savedPagesList.setOnItemClickListener(null);
+ savedPagesList.setOnItemLongClickListener(null);
super.onDestroyView();
}
@@ -393,4 +289,117 @@
}
super.onStop();
}
+
+ private class SavedPageItemClickListener implements
AdapterView.OnItemClickListener {
+ @Override
+ public void onItemClick(AdapterView<?> parent, View view, int
position, long id) {
+ // We shouldn't do anything if the user is multi-selecting things
+ if (actionMode == null) {
+ SavedPage savedPage = (SavedPage) view.getTag();
+ HistoryEntry newEntry = new HistoryEntry(savedPage.getTitle(),
HistoryEntry.SOURCE_SAVED_PAGE);
+ ((PageActivity)getActivity()).loadPage(savedPage.getTitle(),
newEntry);
+ }
+ }
+ }
+
+ private class SavedPageItemLongClickListener implements
AdapterView.OnItemLongClickListener {
+ @Override
+ public boolean onItemLongClick(AdapterView<?> parent, View view, int
position, long id) {
+ if (actionMode != null) {
+ return false;
+ }
+ savedPagesList.setChoiceMode(AbsListView.CHOICE_MODE_MULTIPLE);
+ actionMode = ((AppCompatActivity)
getActivity()).startSupportActionMode(new ActionMode.Callback() {
+ private final String actionModeTag = "actionModeSavedPages";
+
+ @Override
+ public boolean onCreateActionMode(ActionMode mode, Menu menu) {
+
mode.getMenuInflater().inflate(R.menu.menu_saved_pages_context, menu);
+ return true;
+ }
+
+ @Override
+ public boolean onPrepareActionMode(ActionMode mode, Menu menu)
{
+ mode.setTag(actionModeTag);
+ return false;
+ }
+
+ @Override
+ public boolean onActionItemClicked(ActionMode mode, MenuItem
item) {
+ switch (item.getItemId()) {
+ case R.id.menu_refresh_selected_saved_pages:
+ refreshSelected();
+ actionMode.finish();
+ return true;
+ case R.id.menu_delete_selected_saved_pages:
+ deleteSelected();
+ actionMode.finish();
+ return true;
+ default:
+ throw new RuntimeException("Unknown context menu
item clicked");
+ }
+ }
+
+ private void deleteSelected() {
+ SparseBooleanArray checkedItems =
savedPagesList.getCheckedItemPositions();
+ for (int i = 0; i < checkedItems.size(); i++) {
+ if (checkedItems.valueAt(i)) {
+ final SavedPage page =
SavedPage.DATABASE_TABLE.fromCursor((Cursor)
adapter.getItem(checkedItems.keyAt(i)));
+ new DeleteSavedPageTask(getActivity(), page) {
+ @Override
+ public void onFinish(Boolean result) {
+ if (!isAdded()) {
+ return;
+ }
+ FeedbackUtil.showMessage(getActivity(),
+
R.string.snackbar_saved_page_deleted);
+ }
+ }.execute();
+ }
+ }
+ if (checkedItems.size() ==
savedPagesList.getAdapter().getCount()) {
+ entryFilter.setVisibility(View.GONE);
+ }
+ }
+
+ @Override
+ public void onDestroyActionMode(ActionMode mode) {
+
savedPagesList.setChoiceMode(AbsListView.CHOICE_MODE_SINGLE);
+ actionMode = null;
+ // Clear all selections
+ savedPagesList.clearChoices();
+ savedPagesList.requestLayout(); // Required to immediately
redraw unchecked states
+
+ }
+ });
+ savedPagesList.setItemChecked(position, true);
+ return true;
+ }
+ }
+
+ private class SavedPageSearchTextWatcher implements TextWatcher {
+ @Override
+ public void beforeTextChanged(CharSequence charSequence, int i, int
i2, int i3) {
+ // Do nothing
+ }
+
+ @Override
+ public void onTextChanged(CharSequence charSequence, int i, int i2,
int i3) {
+ // Do nothing
+ }
+
+ @Override
+ public void afterTextChanged(Editable editable) {
+
getActivity().getSupportLoaderManager().restartLoader(SAVED_PAGES_FRAGMENT_LOADER_ID,
null, SavedPagesFragment.this);
+ if (editable.length() == 0) {
+ savedPagesEmptyTitle.setText(R.string.saved_pages_empty_title);
+ savedPagesEmptyImage.setVisibility(View.VISIBLE);
+ savedPagesEmptyMessage.setVisibility(View.VISIBLE);
+ } else {
+
savedPagesEmptyTitle.setText(getString(R.string.saved_pages_search_empty_message,
editable.toString()));
+ savedPagesEmptyImage.setVisibility(View.GONE);
+ savedPagesEmptyMessage.setVisibility(View.GONE);
+ }
+ }
+ }
}
--
To view, visit https://gerrit.wikimedia.org/r/271554
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I497ebd37f3059b420622416a66ee5b87a15a2d0c
Gerrit-PatchSet: 4
Gerrit-Project: apps/android/wikipedia
Gerrit-Branch: master
Gerrit-Owner: Dbrant <[email protected]>
Gerrit-Reviewer: BearND <[email protected]>
Gerrit-Reviewer: Brion VIBBER <[email protected]>
Gerrit-Reviewer: Dbrant <[email protected]>
Gerrit-Reviewer: Mholloway <[email protected]>
Gerrit-Reviewer: Niedzielski <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
