[Android] Storages adapter: prevent desync

[Geoffrey Métais]

vlc-android | branch: master | Geoffrey Métais <geoffrey.metais at gmail.com> | Thu Oct 17 14:40:50 2019 +0200| [3acf91bed62e60c8dac0aba34d307baa7f52e361] | committer: Geoffrey Métais

Storages adapter: prevent desync

coroutines dispatching could cause race conditions

> https://code.videolan.org/videolan/vlc-android/commit/3acf91bed62e60c8dac0aba34d307baa7f52e361
---

 .../videolan/vlc/gui/browser/BaseBrowserAdapter.kt |  7 +++---
 .../vlc/gui/browser/StorageBrowserAdapter.kt       | 28 ++++++++++++----------
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/vlc-android/src/org/videolan/vlc/gui/browser/BaseBrowserAdapter.kt b/vlc-android/src/org/videolan/vlc/gui/browser/BaseBrowserAdapter.kt
index 324cee4ac..d36d5ce53 100644
--- a/vlc-android/src/org/videolan/vlc/gui/browser/BaseBrowserAdapter.kt
+++ b/vlc-android/src/org/videolan/vlc/gui/browser/BaseBrowserAdapter.kt
@@ -34,6 +34,7 @@ import android.widget.TextView
 import androidx.databinding.ViewDataBinding
 import androidx.recyclerview.widget.RecyclerView
 import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.Job
 import kotlinx.coroutines.ObsoleteCoroutinesApi
 import org.videolan.libvlc.util.AndroidUtil
 import org.videolan.medialibrary.interfaces.media.AbstractMediaWrapper
@@ -52,6 +53,7 @@ import org.videolan.vlc.gui.helpers.SelectorViewHolder
 import org.videolan.vlc.gui.helpers.enableMarqueeEffect
 import org.videolan.vlc.util.AndroidDevices
 import org.videolan.vlc.util.Settings
+import org.videolan.vlc.util.Settings.init
 import org.videolan.vlc.util.UPDATE_SELECTION
 import java.util.*
 
@@ -174,9 +176,7 @@ open class BaseBrowserAdapter() : DiffUtilAdapter<MediaLibraryItem, BaseBrowserA
 
         open fun onImageClick(v: View) {}
 
-        open fun onLongClick(v: View): Boolean {
-            return false
-        }
+        open fun onLongClick(v: View) = false
 
         open fun onCheckBoxClick(v: View) {}
 
@@ -187,6 +187,7 @@ open class BaseBrowserAdapter() : DiffUtilAdapter<MediaLibraryItem, BaseBrowserA
     @TargetApi(Build.VERSION_CODES.M)
     internal inner class MediaViewHolder(binding: BrowserItemBinding) : ViewHolder<BrowserItemBinding>(binding), View.OnFocusChangeListener, MarqueeViewHolder {
         override val titleView: TextView? = binding.title
+        var job : Job? = null
 
         init {
             binding.holder = this
diff --git a/vlc-android/src/org/videolan/vlc/gui/browser/StorageBrowserAdapter.kt b/vlc-android/src/org/videolan/vlc/gui/browser/StorageBrowserAdapter.kt
index f35db498b..01b728181 100644
--- a/vlc-android/src/org/videolan/vlc/gui/browser/StorageBrowserAdapter.kt
+++ b/vlc-android/src/org/videolan/vlc/gui/browser/StorageBrowserAdapter.kt
@@ -27,16 +27,12 @@ import android.content.Context
 import android.net.Uri
 import android.view.View
 import androidx.databinding.ViewDataBinding
-import kotlinx.coroutines.ExperimentalCoroutinesApi
-import kotlinx.coroutines.Job
-import kotlinx.coroutines.ObsoleteCoroutinesApi
-import kotlinx.coroutines.launch
+import kotlinx.coroutines.*
 import org.videolan.medialibrary.interfaces.AbstractMedialibrary
 import org.videolan.medialibrary.interfaces.media.AbstractMediaWrapper
 import org.videolan.medialibrary.media.MediaLibraryItem
 import org.videolan.medialibrary.media.Storage
 import org.videolan.vlc.MediaParsingService
-import org.videolan.vlc.VLCApplication
 import org.videolan.vlc.gui.helpers.ThreeStatesCheckbox
 import org.videolan.vlc.repository.DirectoryRepository
 import org.videolan.vlc.util.containsPath
@@ -47,22 +43,21 @@ internal class StorageBrowserAdapter(fragment: StorageBrowserFragment) : BaseBro
 
     private var mediaDirsLocation: MutableList<String> = mutableListOf()
     private lateinit var customDirsLocation: List<String>
-    private var job : Job? = null
+    private var updateJob : Job? = null
 
     init {
         updateMediaDirs(fragment.requireContext())
     }
 
     override fun onBindViewHolder(holder: ViewHolder<ViewDataBinding>, position: Int) {
-        launch {
-            val vh = holder as BaseBrowserAdapter.MediaViewHolder
+        val vh = holder as MediaViewHolder
+        vh.job = launch(start = CoroutineStart.UNDISPATCHED) {
             var storage = getItem(position)
-
             if (storage.itemType == MediaLibraryItem.TYPE_MEDIA) storage = Storage((storage as AbstractMediaWrapper).uri)
             var storagePath = (storage as Storage).uri.path ?: ""
             if (!storagePath.endsWith("/")) storagePath += "/"
             vh.binding.item = storage
-            job?.join()
+            updateJob?.join()
             val hasContextMenu = customDirsLocation.contains(storagePath)
             val checked = (fragment as StorageBrowserFragment).scannedDirectory || mediaDirsLocation.containsPath(storagePath)
             vh.binding.hasContextMenu = hasContextMenu
@@ -72,7 +67,16 @@ internal class StorageBrowserAdapter(fragment: StorageBrowserFragment) : BaseBro
                 else -> vh.binding.browserCheckbox.state = ThreeStatesCheckbox.STATE_UNCHECKED
             }
             vh.binding.checkEnabled = !(fragment as StorageBrowserFragment).scannedDirectory
+            vh.job = null
+        }
+    }
+
+    override fun onViewRecycled(holder: ViewHolder<ViewDataBinding>) {
+        (holder as MediaViewHolder).apply {
+            job?.cancel()
+            job = null
         }
+        super.onViewRecycled(holder)
     }
 
     private fun hasDiscoveredChildren(path: String): Boolean {
@@ -82,7 +86,7 @@ internal class StorageBrowserAdapter(fragment: StorageBrowserFragment) : BaseBro
 
     suspend fun updateListState(context: Context) {
         updateMediaDirs(context)
-        job?.join()
+        updateJob?.join()
         notifyItemRangeChanged(0, itemCount)
     }
 
@@ -101,7 +105,7 @@ internal class StorageBrowserAdapter(fragment: StorageBrowserFragment) : BaseBro
             mediaDirsLocation.add(Uri.decode(if (it.startsWith("file://")) it.substring(7) else it))
         }
 
-        job = launch {
+        updateJob = launch {
             customDirsLocation = DirectoryRepository.getInstance(context).getCustomDirectories().map { it.path }
         }
     }