From b919faf0129ec76b34c85b0574e197949ccdaaff Mon Sep 17 00:00:00 2001
From: Samuel Tardieu <sam@rfc1149.net>
Date: Thu, 20 Feb 2014 09:09:59 +0100
Subject: refactoring: makes unsubscribing from image loading easier

This is related to #3618.
---
 main/src/cgeo/geocaching/CacheDetailActivity.java |  8 +++---
 main/src/cgeo/geocaching/ImagesActivity.java      | 16 ++++++------
 main/src/cgeo/geocaching/ui/ImagesList.java       | 32 ++++++++++++++++++-----
 3 files changed, 39 insertions(+), 17 deletions(-)

(limited to 'main/src')

diff --git a/main/src/cgeo/geocaching/CacheDetailActivity.java b/main/src/cgeo/geocaching/CacheDetailActivity.java
index 52214f4..b905a46 100644
--- a/main/src/cgeo/geocaching/CacheDetailActivity.java
+++ b/main/src/cgeo/geocaching/CacheDetailActivity.java
@@ -61,9 +61,10 @@ import rx.Observable;
 import rx.Observable.OnSubscribe;
 import rx.Observer;
 import rx.Subscriber;
+import rx.Subscription;
 import rx.android.observables.AndroidObservable;
-import rx.schedulers.Schedulers;
 import rx.functions.Action1;
+import rx.schedulers.Schedulers;
 
 import android.R.color;
 import android.app.AlertDialog;
@@ -168,6 +169,7 @@ public class CacheDetailActivity extends AbstractViewPagerActivity<CacheDetailAc
     private TextView cacheDistanceView;
 
     protected ImagesList imagesList;
+    private Subscription imagesSubscription;
     /**
      * waypoint selected in context menu. This variable will be gone when the waypoint context menu is a fragment.
      */
@@ -324,7 +326,7 @@ public class CacheDetailActivity extends AbstractViewPagerActivity<CacheDetailAc
     @Override
     public void onDestroy() {
         if (imagesList != null) {
-            imagesList.removeAllViews();
+            imagesSubscription.unsubscribe();
         }
         super.onDestroy();
     }
@@ -702,7 +704,7 @@ public class CacheDetailActivity extends AbstractViewPagerActivity<CacheDetailAc
             return;
         }
         imagesList = new ImagesList(this, cache.getGeocode());
-        imagesList.loadImages(imageView, cache.getImages(), false);
+        imagesSubscription = imagesList.loadImages(imageView, cache.getImages(), false);
     }
 
     public static void startActivity(final Context context, final String geocode) {
diff --git a/main/src/cgeo/geocaching/ImagesActivity.java b/main/src/cgeo/geocaching/ImagesActivity.java
index 12ee00a..3da1ade 100644
--- a/main/src/cgeo/geocaching/ImagesActivity.java
+++ b/main/src/cgeo/geocaching/ImagesActivity.java
@@ -6,6 +6,7 @@ import cgeo.geocaching.ui.ImagesList;
 import cgeo.geocaching.ui.ImagesList.ImageType;
 
 import org.apache.commons.collections4.CollectionUtils;
+import rx.Subscription;
 
 import android.content.Context;
 import android.content.Intent;
@@ -22,9 +23,9 @@ public class ImagesActivity extends AbstractActivity {
 
     private boolean offline;
     private ArrayList<Image> imageNames;
-    private ImagesList imagesList;
     private ImageType imgType = ImageType.SpoilerImages;
-    private String geocode;
+    private ImagesList imagesList;
+    private Subscription subscription;
 
     @Override
     public void onCreate(Bundle savedInstanceState) {
@@ -45,13 +46,13 @@ public class ImagesActivity extends AbstractActivity {
             return;
         }
 
-        this.geocode = geocode;
-
         // init
         setTheme();
         setContentView(R.layout.images_activity);
         setTitle(res.getString(imgType.getTitle()));
 
+        imagesList = new ImagesList(this, geocode);
+
         imageNames = extras.getParcelableArrayList(Intents.EXTRA_IMAGES);
         if (CollectionUtils.isEmpty(imageNames)) {
             showToast(res.getString(R.string.warn_load_images));
@@ -61,20 +62,19 @@ public class ImagesActivity extends AbstractActivity {
 
         offline = DataStore.isOffline(geocode, null) && (imgType == ImageType.SpoilerImages
                 || Settings.isStoreLogImages());
+
     }
 
     @Override
     public void onStart() {
         super.onStart();
-        imagesList = new ImagesList(this, geocode);
-        imagesList.loadImages(findViewById(R.id.spoiler_list), imageNames, offline);
+        subscription = imagesList.loadImages(findViewById(R.id.spoiler_list), imageNames, offline);
     }
 
     @Override
     public void onStop() {
         // Reclaim native memory faster than the finalizers would
-        imagesList.removeAllViews();
-        imagesList = null;
+        subscription.unsubscribe();
         super.onStop();
     }
 
diff --git a/main/src/cgeo/geocaching/ui/ImagesList.java b/main/src/cgeo/geocaching/ui/ImagesList.java
index d2e7993..dcce969 100644
--- a/main/src/cgeo/geocaching/ui/ImagesList.java
+++ b/main/src/cgeo/geocaching/ui/ImagesList.java
@@ -9,9 +9,12 @@ import cgeo.geocaching.utils.Log;
 
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.StringUtils;
+import rx.Subscription;
 import rx.android.observables.AndroidObservable;
-import rx.subscriptions.CompositeSubscription;
+import rx.functions.Action0;
 import rx.functions.Action1;
+import rx.subscriptions.CompositeSubscription;
+import rx.subscriptions.Subscriptions;
 
 import android.app.Activity;
 import android.content.Intent;
@@ -42,7 +45,6 @@ public class ImagesList {
 
     private BitmapDrawable currentDrawable;
     private Image currentImage;
-    private CompositeSubscription subscriptions = new CompositeSubscription();
 
     public enum ImageType {
         LogImages(R.string.cache_log_images_title),
@@ -76,7 +78,24 @@ public class ImagesList {
         inflater = activity.getLayoutInflater();
     }
 
-    public void loadImages(final View parentView, final List<Image> images, final boolean offline) {
+    /**
+     * Load images into a view.
+     *
+     * @param parentView a view to load the images into
+     * @param images the images to load
+     * @param offline <tt>true</tt> if the images must be stored for offline use
+     * @return a subscription which, when unsubscribed, interrupts the loading and clears up resources
+     */
+    public Subscription loadImages(final View parentView, final List<Image> images, final boolean offline) {
+        // Start with a fresh subscription because of this method can be called several times if the
+        // englobing activity is stopped/restarted.
+        final CompositeSubscription subscriptions = new CompositeSubscription(Subscriptions.create(new Action0() {
+            @Override
+            public void call() {
+                removeAllViews();
+            }
+        }));
+
         imagesView = (LinearLayout) parentView.findViewById(R.id.spoiler_list);
 
         final HtmlImage imgGetter = new HtmlImage(geocode, true, offline ? StoredList.STANDARD_LIST_ID : StoredList.TEMPORARY_LIST_ID, false);
@@ -108,6 +127,8 @@ public class ImagesList {
             rowView.addView(imageView);
             imagesView.addView(rowView);
         }
+
+        return subscriptions;
     }
 
     private void display(final ImageView imageView, final BitmapDrawable image, final Image img, final LinearLayout view) {
@@ -138,14 +159,13 @@ public class ImagesList {
         }
     }
 
-    public void removeAllViews() {
+    private void removeAllViews() {
         for (final Bitmap b : bitmaps) {
             b.recycle();
         }
         bitmaps.clear();
+        images.clear();
 
-        // Stop loading images if some are still in progress
-        subscriptions.unsubscribe();
         imagesView.removeAllViews();
     }
 
-- 
cgit v1.1