Addressed code review comments

Author: Isira-Seneviratne

app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java

@@ -1473,6 +1473,7 @@ public void showLoading() {
         CoilUtils.dispose(binding.detailThumbnailImageView);
         CoilUtils.dispose(binding.detailSubChannelThumbnailView);
         CoilUtils.dispose(binding.overlayThumbnail);
+        CoilUtils.dispose(binding.detailUploaderThumbnailView);
 
         binding.detailThumbnailImageView.setImageBitmap(null);
         binding.detailSubChannelThumbnailView.setImageBitmap(null);

app/src/main/java/org/schabi/newpipe/local/feed/notifications/NotificationHelper.kt

@@ -76,8 +76,7 @@ class NotificationHelper(val context: Context) {
 
         summaryBuilder.setLargeIcon(avatarIcon)
 
-        // Show individual stream notifications, set channel icon only if there is actually
-        // one
+        // Show individual stream notifications, set channel icon only if there is actually one
         showStreamNotifications(newStreams, data.serviceId, avatarIcon)
         // Show summary notification
         manager.notify(data.pseudoId, summaryBuilder.build())

app/src/main/java/org/schabi/newpipe/player/Player.java

@@ -192,6 +192,8 @@ public final class Player implements PlaybackListener, Listener {
     private MediaItemTag currentMetadata;
     @Nullable
     private Bitmap currentThumbnail;
+    @Nullable
+    private coil.request.Disposable thumbnailDisposable;
 
     /*//////////////////////////////////////////////////////////////////////////
     // Player
@@ -772,6 +774,11 @@ private void loadCurrentThumbnail(final List<Image> thumbnails) {
                     + thumbnails.size() + "]");
         }
 
+        // Cancel any ongoing image loading
+        if (thumbnailDisposable != null) {
+            thumbnailDisposable.dispose();
+        }
+
         // Unset currentThumbnail, since it is now outdated. This ensures it is not used in media
         // session metadata while the new thumbnail is being loaded by Coil.
         onThumbnailLoaded(null);
@@ -780,7 +787,7 @@ private void loadCurrentThumbnail(final List<Image> thumbnails) {
         }
 
         // scale down the notification thumbnail for performance
-        final var target = new Target() {
+        final var thumbnailTarget = new Target() {
             @Override
             public void onError(@Nullable final Drawable error) {
                 Log.e(TAG, "Thumbnail - onError() called");
@@ -805,7 +812,8 @@ public void onSuccess(@NonNull final Drawable result) {
                         result.getIntrinsicHeight(), null));
             }
         };
-        CoilHelper.INSTANCE.loadScaledDownThumbnail(context, thumbnails, target);
+        thumbnailDisposable = CoilHelper.INSTANCE
+                .loadScaledDownThumbnail(context, thumbnails, thumbnailTarget);
     }
 
     private void onThumbnailLoaded(@Nullable final Bitmap bitmap) {

app/src/main/java/org/schabi/newpipe/settings/ContentSettingsFragment.java

@@ -42,12 +42,8 @@ public void onCreatePreferences(final Bundle savedInstanceState, final String ro
                     ImageStrategy.setPreferredImageQuality(PreferredImageQuality
                             .fromPreferenceKey(requireContext(), (String) newValue));
                     final var loader = Coil.imageLoader(preference.getContext());
-                    if (loader.getMemoryCache() != null) {
-                        loader.getMemoryCache().clear();
-                    }
-                    if (loader.getDiskCache() != null) {
-                        loader.getDiskCache().clear();
-                    }
+                    loader.getMemoryCache().clear();
+                    loader.getDiskCache().clear();
                     Toast.makeText(preference.getContext(),
                             R.string.thumbnail_cache_wipe_complete_notice, Toast.LENGTH_SHORT)
                             .show();

app/src/main/java/org/schabi/newpipe/util/external_communication/ShareUtils.java

@@ -10,6 +10,7 @@
 import android.content.pm.PackageManager;
 import android.content.pm.ResolveInfo;
 import android.graphics.Bitmap;
+import android.graphics.BitmapFactory;
 import android.net.Uri;
 import android.os.Build;
 import android.text.TextUtils;
@@ -24,13 +25,16 @@
 import org.schabi.newpipe.BuildConfig;
 import org.schabi.newpipe.R;
 import org.schabi.newpipe.extractor.Image;
-import org.schabi.newpipe.util.image.CoilHelper;
 import org.schabi.newpipe.util.image.ImageStrategy;
 
-import java.io.File;
-import java.io.FileOutputStream;
+import java.nio.file.Files;
+import java.util.Collections;
 import java.util.List;
 
+import coil.Coil;
+import coil.disk.DiskCache;
+import coil.memory.MemoryCache;
+
 public final class ShareUtils {
     private static final String TAG = ShareUtils.class.getSimpleName();
 
@@ -335,7 +339,13 @@ public static void copyToClipboard(@NonNull final Context context, final String
     }
 
     /**
-     * Generate a {@link ClipData} with the image of the content shared.
+     * Generate a {@link ClipData} with the image of the content shared, if it's in the app cache.
+     *
+     * <p>
+     * In order not to worry about network issues (timeouts, DNS issues, low connection speed, ...)
+     * when sharing a content, only images in the {@link MemoryCache} or {@link DiskCache}
+     * used by the Coil library are used as preview images. If the thumbnail image is not in the
+     * cache, no {@link ClipData} will be generated and {@code null} will be returned.
      *
      * <p>
      * In order to display the image in the content preview of the Android share sheet, an URI of
@@ -351,11 +361,6 @@ public static void copyToClipboard(@NonNull final Context context, final String
      * </p>
      *
      * <p>
-     * This method will call {@link CoilHelper#loadBitmapBlocking(Context, String)} to get the
-     * thumbnail of the content.
-     * </p>
-     *
-     * <p>
      * Using the result of this method when sharing has only an effect on the system share sheet (if
      * OEMs didn't change Android system standard behavior) on Android API 29 and higher.
      * </p>
@@ -369,33 +374,46 @@ private static ClipData generateClipDataForImagePreview(
             @NonNull final Context context,
             @NonNull final String thumbnailUrl) {
         try {
-            final var bitmap = CoilHelper.INSTANCE.loadBitmapBlocking(context, thumbnailUrl);
-            if (bitmap == null) {
-                return null;
-            }
-
             // Save the image in memory to the application's cache because we need a URI to the
             // image to generate a ClipData which will show the share sheet, and so an image file
             final Context applicationContext = context.getApplicationContext();
-            final String appFolder = applicationContext.getCacheDir().getAbsolutePath();
-            final File thumbnailPreviewFile = new File(appFolder
-                    + "/android_share_sheet_image_preview.jpg");
+            final var loader = Coil.imageLoader(context);
+            final var value = loader.getMemoryCache()
+                    .get(new MemoryCache.Key(thumbnailUrl, Collections.emptyMap()));
+
+            final Bitmap cachedBitmap;
+            if (value != null) {
+                cachedBitmap = value.getBitmap();
+            } else {
+                try (var snapshot = loader.getDiskCache().openSnapshot(thumbnailUrl)) {
+                    if (snapshot != null) {
+                        cachedBitmap = BitmapFactory.decodeFile(snapshot.getData().toString());
+                    } else {
+                        cachedBitmap = null;
+                    }
+                }
+            }
+
+            if (cachedBitmap == null) {
+                return null;
+            }
 
-            // Any existing file will be overwritten with FileOutputStream
-            final FileOutputStream fileOutputStream = new FileOutputStream(thumbnailPreviewFile);
-            bitmap.compress(Bitmap.CompressFormat.JPEG, 90, fileOutputStream);
-            fileOutputStream.close();
+            final var path = applicationContext.getCacheDir().toPath()
+                    .resolve("android_share_sheet_image_preview.jpg");
+            // Any existing file will be overwritten
+            try (var outputStream = Files.newOutputStream(path)) {
+                cachedBitmap.compress(Bitmap.CompressFormat.JPEG, 90, outputStream);
+            }
 
             final ClipData clipData = ClipData.newUri(applicationContext.getContentResolver(), "",
                         FileProvider.getUriForFile(applicationContext,
                                 BuildConfig.APPLICATION_ID + ".provider",
-                                thumbnailPreviewFile));
+                                path.toFile()));
 
             if (DEBUG) {
                 Log.d(TAG, "ClipData successfully generated for Android share sheet: " + clipData);
             }
             return clipData;
-
         } catch (final Exception e) {
             Log.w(TAG, "Error when setting preview image for share sheet", e);
             return null;

app/src/main/java/org/schabi/newpipe/util/image/CoilHelper.kt

@@ -8,6 +8,7 @@ import androidx.annotation.DrawableRes
 import androidx.core.graphics.drawable.toBitmapOrNull
 import coil.executeBlocking
 import coil.imageLoader
+import coil.request.Disposable
 import coil.request.ImageRequest
 import coil.size.Size
 import coil.target.Target
@@ -47,7 +48,7 @@ object CoilHelper {
         loadImageDefault(target, url, R.drawable.placeholder_thumbnail_video)
     }
 
-    fun loadScaledDownThumbnail(context: Context, images: List<Image>, target: Target) {
+    fun loadScaledDownThumbnail(context: Context, images: List<Image>, target: Target): Disposable {
         val url = ImageStrategy.choosePreferredImage(images)
         val request = getImageRequest(context, url, R.drawable.placeholder_thumbnail_video)
             .target(target)
@@ -79,7 +80,7 @@ object CoilHelper {
             })
             .build()
 
-        context.imageLoader.enqueue(request)
+        return context.imageLoader.enqueue(request)
     }
 
     fun loadDetailsThumbnail(target: ImageView, images: List<Image>) {
@@ -133,6 +134,8 @@ object CoilHelper {
         return ImageRequest.Builder(context)
             .data(takenUrl)
             .error(placeholderResId)
+            .memoryCacheKey(takenUrl)
+            .diskCacheKey(takenUrl)
             .apply {
                 if (takenUrl != null || showPlaceholderWhileLoading) {
                     placeholder(placeholderResId)