Merge pull request #8200 from wordpress-mobile/issue/8199-memory-leaks

Issue/8199 memory leaks
wordpress-mobile · Aug 17, 2018 · f8077f7 · f8077f7
2 parents 1b522bc + 0a9a471
commit f8077f7
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 30 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
@@ -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
@@ -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,9 @@
  * 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;
 
     public ReaderLikingUsersView(Context context) {
         this(context, null);
@@ -46,39 +45,32 @@ public ReaderLikingUsersView(Context context, AttributeSet attrs) {
         mLikeAvatarSz = context.getResources().getDimensionPixelSize(R.dimen.avatar_sz_small);
     }
 
-    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<String> 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);
+        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;
@@ -116,4 +108,38 @@ private void showLikingAvatars(final ArrayList<String> avatarUrls) {
             index++;
         }
     }
+
+    private static class LoadAvatarsTask extends AsyncTask<ReaderPost, Void, ArrayList<String>> {
+        private final WeakReference<ReaderLikingUsersView> 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<String> 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<String> avatars) {
+            super.onPostExecute(avatars);
+            ReaderLikingUsersView view = mViewReference.get();
+            if (view != null && avatars != null && !isCancelled()) {
+                view.mLoadAvatarsTask = null;
+                view.showLikingAvatars(avatars);
+            }
+        }
+    }
 }
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
@@ -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) {