From 730be154e7918d3adb331fc087d2e7d11078c9ee Mon Sep 17 00:00:00 2001 From: malinajirka Date: Thu, 16 Aug 2018 10:24:25 +0200 Subject: [PATCH 1/4] Fix crash when loading images from a leaked context(reader post detail) --- .../ui/reader/ReaderPostDetailFragment.java | 2 +- .../reader/views/ReaderLikingUsersView.java | 90 ++++++++++++------- 2 files changed, 61 insertions(+), 31 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderPostDetailFragment.java b/WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderPostDetailFragment.java index 364fc33c2662..bad0bdf9838f 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderPostDetailFragment.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderPostDetailFragment.java @@ -935,7 +935,7 @@ public void onClick(View view) { mLikingUsersDivider.setVisibility(View.VISIBLE); mLikingUsersLabel.setVisibility(View.VISIBLE); mLikingUsersView.setVisibility(View.VISIBLE); - mLikingUsersView.showLikingUsers(mPost); + mLikingUsersView.showLikingUsers(mPost, mAccountStore.getAccount().getUserId()); } private boolean showPhotoViewer(String imageUrl, View sourceView, int startX, int startY) { diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/views/ReaderLikingUsersView.java b/WordPress/src/main/java/org/wordpress/android/ui/reader/views/ReaderLikingUsersView.java index 4cb0a7351937..493c0750fe10 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/views/ReaderLikingUsersView.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/views/ReaderLikingUsersView.java @@ -1,7 +1,7 @@ package org.wordpress.android.ui.reader.views; import android.content.Context; -import android.os.Handler; +import android.os.AsyncTask; import android.util.AttributeSet; import android.view.Gravity; import android.view.LayoutInflater; @@ -12,13 +12,13 @@ import org.wordpress.android.WordPress; import org.wordpress.android.datasets.ReaderLikeTable; import org.wordpress.android.datasets.ReaderUserTable; -import org.wordpress.android.fluxc.store.AccountStore; import org.wordpress.android.models.ReaderPost; import org.wordpress.android.models.ReaderUserIdList; import org.wordpress.android.util.StringUtils; import org.wordpress.android.util.image.ImageManager; import org.wordpress.android.util.image.ImageType; +import java.lang.ref.WeakReference; import java.util.ArrayList; import javax.inject.Inject; @@ -27,10 +27,11 @@ * LinearLayout which shows liking users - used by ReaderPostDetailFragment */ public class ReaderLikingUsersView extends LinearLayout { - private final int mLikeAvatarSz; - - @Inject AccountStore mAccountStore; @Inject ImageManager mImageManager; + private LoadAvatarsTask mLoadAvatarsTask; + private final int mLikeAvatarSz; + private final int mMarginAvatar; + private final int mMarginReader; public ReaderLikingUsersView(Context context) { this(context, null); @@ -44,43 +45,36 @@ public ReaderLikingUsersView(Context context, AttributeSet attrs) { setGravity(Gravity.CENTER_VERTICAL); mLikeAvatarSz = context.getResources().getDimensionPixelSize(R.dimen.avatar_sz_small); + mMarginAvatar = context.getResources().getDimensionPixelSize(R.dimen.margin_extra_small); + mMarginReader = context.getResources().getDimensionPixelSize(R.dimen.reader_detail_margin); } - public void showLikingUsers(final ReaderPost post) { + public void showLikingUsers(final ReaderPost post, final long currentUserId) { if (post == null) { return; } - final Handler handler = new Handler(); - new Thread() { - @Override - public void run() { - // get avatar URLs of liking users up to the max, sized to fit - int maxAvatars = getMaxAvatars(); - ReaderUserIdList avatarIds = ReaderLikeTable.getLikesForPost(post); - // TODO: Probably a bad idea to have mAccountStore.getAccount().getUserId() here, - // a view should not read the account state - final ArrayList avatars = ReaderUserTable.getAvatarUrls(avatarIds, maxAvatars, mLikeAvatarSz, - mAccountStore.getAccount().getUserId()); - - handler.post(new Runnable() { - @Override - public void run() { - showLikingAvatars(avatars); - } - }); - } - }.start(); + if (mLoadAvatarsTask != null) { + mLoadAvatarsTask.cancel(false); + } + mLoadAvatarsTask = new LoadAvatarsTask(this, currentUserId, mLikeAvatarSz, getMaxAvatars()); + mLoadAvatarsTask.execute(post); + } + + @Override + protected void onDetachedFromWindow() { + super.onDetachedFromWindow(); + if (mLoadAvatarsTask != null) { + mLoadAvatarsTask.cancel(false); + } } /* * returns count of avatars that can fit the current space */ private int getMaxAvatars() { - int marginAvatar = getResources().getDimensionPixelSize(R.dimen.margin_extra_small); - int marginReader = getResources().getDimensionPixelSize(R.dimen.reader_detail_margin); - int likeAvatarSizeWithMargin = mLikeAvatarSz + (marginAvatar * 2); - int spaceForAvatars = getWidth() - (marginReader * 2); + int likeAvatarSizeWithMargin = mLikeAvatarSz + (mMarginAvatar * 2); + int spaceForAvatars = getWidth() - (mMarginReader * 2); return spaceForAvatars / likeAvatarSizeWithMargin; } @@ -116,4 +110,40 @@ private void showLikingAvatars(final ArrayList avatarUrls) { index++; } } + + private static class LoadAvatarsTask extends AsyncTask> { + private final WeakReference mViewReference; + private final long mCurrentUserId; + private final int mLikeAvatarSize; + private final int mMaxAvatars; + + LoadAvatarsTask(ReaderLikingUsersView view, long currentUserId, int likeAvatarSz, + int maxAvatars) { + mViewReference = new WeakReference<>(view); + mCurrentUserId = currentUserId; + mLikeAvatarSize = likeAvatarSz; + mMaxAvatars = maxAvatars; + } + + @Override + protected ArrayList doInBackground(ReaderPost... posts) { + if (posts.length != 1 || posts[0] == null) { + return null; + } + ReaderPost post = posts[0]; + ReaderUserIdList avatarIds = ReaderLikeTable.getLikesForPost(post); + return ReaderUserTable.getAvatarUrls(avatarIds, mMaxAvatars, mLikeAvatarSize, + mCurrentUserId); + } + + @Override + protected void onPostExecute(ArrayList avatars) { + super.onPostExecute(avatars); + ReaderLikingUsersView view = mViewReference.get(); + if (view != null && avatars != null && !isCancelled()) { + view.mLoadAvatarsTask = null; + view.showLikingAvatars(avatars); + } + } + } } From d18f513c7c44fc395b53a34e1d634d13124e459e Mon Sep 17 00:00:00 2001 From: malinajirka Date: Thu, 16 Aug 2018 10:30:46 +0200 Subject: [PATCH 2/4] Fix crash when loading images from a leaked context(ReaderSiteHeaderView) --- .../ui/reader/views/ReaderSiteHeaderView.java | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/views/ReaderSiteHeaderView.java b/WordPress/src/main/java/org/wordpress/android/ui/reader/views/ReaderSiteHeaderView.java index a5c2a210f873..d462fd5303fc 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/views/ReaderSiteHeaderView.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/views/ReaderSiteHeaderView.java @@ -38,6 +38,7 @@ public interface OnBlogInfoLoadedListener { void onBlogInfoLoaded(ReaderBlog blogInfo); } + private boolean mAttached; private long mBlogId; private long mFeedId; private ReaderFollowButton mFollowButton; @@ -63,6 +64,16 @@ public ReaderSiteHeaderView(Context context, AttributeSet attrs, int defStyleAtt initView(context); } + @Override protected void onAttachedToWindow() { + super.onAttachedToWindow(); + mAttached = true; + } + + @Override protected void onDetachedFromWindow() { + super.onDetachedFromWindow(); + mAttached = false; + } + private void initView(Context context) { View view = inflate(context, R.layout.reader_site_header_view, this); mFollowButton = view.findViewById(R.id.follow_button); @@ -99,7 +110,9 @@ public void loadBlogInfo(long blogId, long feedId) { ReaderActions.UpdateBlogInfoListener listener = new ReaderActions.UpdateBlogInfoListener() { @Override public void onResult(ReaderBlog serverBlogInfo) { - showBlogInfo(serverBlogInfo); + if (mAttached) { + showBlogInfo(serverBlogInfo); + } } }; if (mFeedId != 0) { From 90557afa04075c6aabf462bc6783ad558ede0b4c Mon Sep 17 00:00:00 2001 From: malinajirka Date: Thu, 16 Aug 2018 21:35:49 +0200 Subject: [PATCH 3/4] Fix formatting --- .../android/ui/reader/views/ReaderLikingUsersView.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/views/ReaderLikingUsersView.java b/WordPress/src/main/java/org/wordpress/android/ui/reader/views/ReaderLikingUsersView.java index 493c0750fe10..7c9040ff0645 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/views/ReaderLikingUsersView.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/views/ReaderLikingUsersView.java @@ -117,8 +117,7 @@ private static class LoadAvatarsTask extends AsyncTask(view); mCurrentUserId = currentUserId; mLikeAvatarSize = likeAvatarSz; @@ -132,8 +131,7 @@ protected ArrayList doInBackground(ReaderPost... posts) { } ReaderPost post = posts[0]; ReaderUserIdList avatarIds = ReaderLikeTable.getLikesForPost(post); - return ReaderUserTable.getAvatarUrls(avatarIds, mMaxAvatars, mLikeAvatarSize, - mCurrentUserId); + return ReaderUserTable.getAvatarUrls(avatarIds, mMaxAvatars, mLikeAvatarSize, mCurrentUserId); } @Override From 0a9a471598b947341c5a6672d52036af4e4c8a20 Mon Sep 17 00:00:00 2001 From: malinajirka Date: Fri, 17 Aug 2018 07:07:42 +0200 Subject: [PATCH 4/4] Convert fields to local variables --- .../android/ui/reader/views/ReaderLikingUsersView.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/views/ReaderLikingUsersView.java b/WordPress/src/main/java/org/wordpress/android/ui/reader/views/ReaderLikingUsersView.java index 7c9040ff0645..6f302ca73185 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/views/ReaderLikingUsersView.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/views/ReaderLikingUsersView.java @@ -30,8 +30,6 @@ public class ReaderLikingUsersView extends LinearLayout { @Inject ImageManager mImageManager; private LoadAvatarsTask mLoadAvatarsTask; private final int mLikeAvatarSz; - private final int mMarginAvatar; - private final int mMarginReader; public ReaderLikingUsersView(Context context) { this(context, null); @@ -45,8 +43,6 @@ public ReaderLikingUsersView(Context context, AttributeSet attrs) { setGravity(Gravity.CENTER_VERTICAL); mLikeAvatarSz = context.getResources().getDimensionPixelSize(R.dimen.avatar_sz_small); - mMarginAvatar = context.getResources().getDimensionPixelSize(R.dimen.margin_extra_small); - mMarginReader = context.getResources().getDimensionPixelSize(R.dimen.reader_detail_margin); } public void showLikingUsers(final ReaderPost post, final long currentUserId) { @@ -73,8 +69,10 @@ protected void onDetachedFromWindow() { * returns count of avatars that can fit the current space */ private int getMaxAvatars() { - int likeAvatarSizeWithMargin = mLikeAvatarSz + (mMarginAvatar * 2); - int spaceForAvatars = getWidth() - (mMarginReader * 2); + final int marginAvatar = getResources().getDimensionPixelSize(R.dimen.margin_extra_small); + final int marginReader = getResources().getDimensionPixelSize(R.dimen.reader_detail_margin); + int likeAvatarSizeWithMargin = mLikeAvatarSz + (marginAvatar * 2); + int spaceForAvatars = getWidth() - (marginReader * 2); return spaceForAvatars / likeAvatarSizeWithMargin; }