Dbrant has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/338366 )
Change subject: Improve symmetry of setting/nulling callbacks on feed views.
......................................................................
Improve symmetry of setting/nulling callbacks on feed views.
So, here's the thing:
When feed views get destroyed, we need to explicitly null out the
"callback" field that they contain. Otherwise, it's known to cause memory
leak(s) on certain APIs.
So far, we've been using the onViewAttachedToWindow() method to give the
callback to the view, and similarly using onViewDetachedFromWindow() to
set the callback to null, which seems to work well in preventing memory
leaks.
I've noticed, however, that in certain feed views, we have *nested*
RecyclerViews, and in the adapters of those nested RecyclerViews, we don't
actually use the attached/detached-from-window logic for setting and
nulling the callback (instead we just set the callback in onBindViewHolder).
My guess is that the latest version of appcompat has exposed this
inconsistency, in the form of race condition between the parent and nested
RecyclerAdapters in setting the callback.
This patch updates our "child" RecyclerAdapters to follow the same pattern
of setting and nulling the callback based on the attached/detached calls.
Bug: T158347
Change-Id: Ieb5461dbee0627df5252c51c54817627eedcf4a5
---
M
app/src/main/java/org/wikipedia/feed/becauseyouread/BecauseYouReadCardView.java
M app/src/main/java/org/wikipedia/feed/mostread/MostReadCardView.java
M app/src/main/java/org/wikipedia/feed/news/NewsListCardView.java
M app/src/main/java/org/wikipedia/feed/view/HorizontalScrollingListCardView.java
M app/src/main/java/org/wikipedia/feed/view/ListCardRecyclerAdapter.java
5 files changed, 42 insertions(+), 3 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/apps/android/wikipedia
refs/changes/66/338366/1
diff --git
a/app/src/main/java/org/wikipedia/feed/becauseyouread/BecauseYouReadCardView.java
b/app/src/main/java/org/wikipedia/feed/becauseyouread/BecauseYouReadCardView.java
index 724cd41..1701aaa 100644
---
a/app/src/main/java/org/wikipedia/feed/becauseyouread/BecauseYouReadCardView.java
+++
b/app/src/main/java/org/wikipedia/feed/becauseyouread/BecauseYouReadCardView.java
@@ -2,6 +2,7 @@
import android.content.Context;
import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
import android.support.annotation.VisibleForTesting;
import android.view.View;
@@ -75,11 +76,14 @@
super(items);
}
+ @Nullable @Override protected ListCardItemView.Callback callback() {
+ return getCallback();
+ }
+
@Override
public void onBindViewHolder(DefaultViewHolder<ListCardItemView>
holder, int i) {
BecauseYouReadItemCard card = item(i);
holder.getView().setHistoryEntry(new
HistoryEntry(card.pageTitle(), HistoryEntry.SOURCE_FEED_BECAUSE_YOU_READ));
- holder.getView().setCallback(getCallback());
}
}
}
diff --git
a/app/src/main/java/org/wikipedia/feed/mostread/MostReadCardView.java
b/app/src/main/java/org/wikipedia/feed/mostread/MostReadCardView.java
index 40008bf..d4c7be0 100644
--- a/app/src/main/java/org/wikipedia/feed/mostread/MostReadCardView.java
+++ b/app/src/main/java/org/wikipedia/feed/mostread/MostReadCardView.java
@@ -2,6 +2,7 @@
import android.content.Context;
import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
import org.wikipedia.R;
import org.wikipedia.feed.view.CardHeaderView;
@@ -42,11 +43,14 @@
super(items);
}
+ @Nullable @Override protected ListCardItemView.Callback callback() {
+ return getCallback();
+ }
+
@Override
public void onBindViewHolder(DefaultViewHolder<ListCardItemView>
holder, int position) {
MostReadItemCard card = item(position);
holder.getView().setHistoryEntry(new
HistoryEntry(card.pageTitle(), HistoryEntry.SOURCE_FEED_MOST_READ));
- holder.getView().setCallback(getCallback());
}
}
}
diff --git a/app/src/main/java/org/wikipedia/feed/news/NewsListCardView.java
b/app/src/main/java/org/wikipedia/feed/news/NewsListCardView.java
index b113044..c800659 100644
--- a/app/src/main/java/org/wikipedia/feed/news/NewsListCardView.java
+++ b/app/src/main/java/org/wikipedia/feed/news/NewsListCardView.java
@@ -2,10 +2,12 @@
import android.content.Context;
import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
import android.view.View;
import org.wikipedia.R;
import org.wikipedia.feed.view.CardHeaderView;
+import org.wikipedia.feed.view.FeedAdapter;
import org.wikipedia.feed.view.HorizontalScrollingListCardItemView;
import org.wikipedia.feed.view.HorizontalScrollingListCardView;
import org.wikipedia.util.DateUtil;
@@ -46,12 +48,15 @@
super(items);
}
+ @Nullable @Override protected FeedAdapter.Callback callback() {
+ return getCallback();
+ }
+
@Override
public void
onBindViewHolder(DefaultViewHolder<HorizontalScrollingListCardItemView> holder,
int i) {
final NewsItemCard card = item(i);
holder.getView().setText(card.text());
holder.getView().setImage(card.image());
- holder.getView().setCallback(getCallback());
holder.getView().setOnClickListener(new OnClickListener() {
@Override
public void onClick(View view) {
diff --git
a/app/src/main/java/org/wikipedia/feed/view/HorizontalScrollingListCardView.java
b/app/src/main/java/org/wikipedia/feed/view/HorizontalScrollingListCardView.java
index 12c554e..84b657e 100644
---
a/app/src/main/java/org/wikipedia/feed/view/HorizontalScrollingListCardView.java
+++
b/app/src/main/java/org/wikipedia/feed/view/HorizontalScrollingListCardView.java
@@ -2,6 +2,7 @@
import android.content.Context;
import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
import android.support.v7.widget.LinearLayoutManager;
import android.support.v7.widget.RecyclerView;
import android.view.ViewGroup;
@@ -44,9 +45,21 @@
super(items);
}
+ @Nullable protected abstract FeedAdapter.Callback callback();
+
@Override public
DefaultViewHolder<HorizontalScrollingListCardItemView>
onCreateViewHolder(ViewGroup parent,
int viewType) {
return new DefaultViewHolder<>(new
HorizontalScrollingListCardItemView(parent.getContext()));
}
+
+ @Override public void
onViewAttachedToWindow(DefaultViewHolder<HorizontalScrollingListCardItemView>
holder) {
+ super.onViewAttachedToWindow(holder);
+ holder.getView().setCallback(callback());
+ }
+
+ @Override public void
onViewDetachedFromWindow(DefaultViewHolder<HorizontalScrollingListCardItemView>
holder) {
+ holder.getView().setCallback(null);
+ super.onViewDetachedFromWindow(holder);
+ }
}
}
diff --git
a/app/src/main/java/org/wikipedia/feed/view/ListCardRecyclerAdapter.java
b/app/src/main/java/org/wikipedia/feed/view/ListCardRecyclerAdapter.java
index 83e5a44..a4be1a3 100644
--- a/app/src/main/java/org/wikipedia/feed/view/ListCardRecyclerAdapter.java
+++ b/app/src/main/java/org/wikipedia/feed/view/ListCardRecyclerAdapter.java
@@ -1,6 +1,7 @@
package org.wikipedia.feed.view;
import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
import android.view.ViewGroup;
import org.wikipedia.views.DefaultRecyclerAdapter;
@@ -14,8 +15,20 @@
super(items);
}
+ @Nullable protected abstract ListCardItemView.Callback callback();
+
@Override public DefaultViewHolder<ListCardItemView>
onCreateViewHolder(ViewGroup parent,
int viewType) {
return new DefaultViewHolder<>(new
ListCardItemView(parent.getContext()));
}
+
+ @Override public void
onViewAttachedToWindow(DefaultViewHolder<ListCardItemView> holder) {
+ super.onViewAttachedToWindow(holder);
+ holder.getView().setCallback(callback());
+ }
+
+ @Override public void
onViewDetachedFromWindow(DefaultViewHolder<ListCardItemView> holder) {
+ holder.getView().setCallback(null);
+ super.onViewDetachedFromWindow(holder);
+ }
}
--
To view, visit https://gerrit.wikimedia.org/r/338366
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ieb5461dbee0627df5252c51c54817627eedcf4a5
Gerrit-PatchSet: 1
Gerrit-Project: apps/android/wikipedia
Gerrit-Branch: master
Gerrit-Owner: Dbrant <dbr...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
