From 3251b84e23c68025f9a18d764c3c4ee79b654e6e Mon Sep 17 00:00:00 2001
From: OxygenCobalt <alexyx@tutanota.com>
Date: Mon, 26 Oct 2020 18:39:39 -0600
Subject: [PATCH] Fix memory leak with PlaybackViewModel

Fix a memory leak involving a stray context that PlaybackViewModel would store.
---
 app/src/main/AndroidManifest.xml              |  5 ++--
 .../java/org/oxycblt/auxio/MainFragment.kt    | 11 +++++---
 .../auxio/detail/AlbumDetailFragment.kt       |  4 +--
 .../auxio/detail/ArtistDetailFragment.kt      |  4 +--
 .../auxio/detail/GenreDetailFragment.kt       |  4 +--
 .../oxycblt/auxio/library/LibraryFragment.kt  |  4 +--
 .../auxio/playback/CompactPlaybackFragment.kt |  4 +--
 .../auxio/playback/PlaybackFragment.kt        |  4 +--
 .../oxycblt/auxio/playback/PlaybackService.kt |  7 +++---
 .../auxio/playback/PlaybackViewModel.kt       | 25 +++----------------
 .../auxio/playback/queue/QueueFragment.kt     |  4 +--
 .../org/oxycblt/auxio/songs/SongsFragment.kt  |  4 +--
 app/src/main/res/values/strings.xml           |  6 ++---
 13 files changed, 29 insertions(+), 57 deletions(-)

diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml
index d69adda0c..c4cacb710 100644
--- a/app/src/main/AndroidManifest.xml
+++ b/app/src/main/AndroidManifest.xml
@@ -23,9 +23,10 @@
                 <category android:name="android.intent.category.LAUNCHER" />
             </intent-filter>
         </activity>
-        <service android:name=".playback.PlaybackService"
+        <service
+            android:name=".playback.PlaybackService"
             android:icon="@drawable/ic_launcher_foreground"
-            android:description="@string/description_service_playback"
+            android:description="@string/label_service_playback"
             android:stopWithTask="false" />
     </application>
 </manifest>
\ No newline at end of file
diff --git a/app/src/main/java/org/oxycblt/auxio/MainFragment.kt b/app/src/main/java/org/oxycblt/auxio/MainFragment.kt
index 02f8f9a51..e99ce4f98 100644
--- a/app/src/main/java/org/oxycblt/auxio/MainFragment.kt
+++ b/app/src/main/java/org/oxycblt/auxio/MainFragment.kt
@@ -1,5 +1,6 @@
 package org.oxycblt.auxio
 
+import android.content.Intent
 import android.os.Bundle
 import android.util.Log
 import android.view.LayoutInflater
@@ -15,6 +16,7 @@ import com.google.android.material.tabs.TabLayoutMediator
 import org.oxycblt.auxio.databinding.FragmentMainBinding
 import org.oxycblt.auxio.library.LibraryFragment
 import org.oxycblt.auxio.music.MusicStore
+import org.oxycblt.auxio.playback.PlaybackService
 import org.oxycblt.auxio.playback.PlaybackViewModel
 import org.oxycblt.auxio.songs.SongsFragment
 import org.oxycblt.auxio.theme.accent
@@ -23,9 +25,7 @@ import org.oxycblt.auxio.theme.getTransparentAccent
 import org.oxycblt.auxio.theme.toColor
 
 class MainFragment : Fragment() {
-    private val playbackModel: PlaybackViewModel by activityViewModels {
-        PlaybackViewModel.Factory(requireContext())
-    }
+    private val playbackModel: PlaybackViewModel by activityViewModels()
 
     private val shownFragments = listOf(0, 1)
     private val tabIcons = listOf(
@@ -105,6 +105,11 @@ class MainFragment : Fragment() {
             }
         }
 
+        // Start the playback service.
+        Intent(requireContext(), PlaybackService::class.java).also {
+            requireContext().startService(it)
+        }
+
         Log.d(this::class.simpleName, "Fragment Created.")
 
         return binding.root
diff --git a/app/src/main/java/org/oxycblt/auxio/detail/AlbumDetailFragment.kt b/app/src/main/java/org/oxycblt/auxio/detail/AlbumDetailFragment.kt
index 35e4a377a..a6b433793 100644
--- a/app/src/main/java/org/oxycblt/auxio/detail/AlbumDetailFragment.kt
+++ b/app/src/main/java/org/oxycblt/auxio/detail/AlbumDetailFragment.kt
@@ -22,9 +22,7 @@ class AlbumDetailFragment : Fragment() {
 
     private val args: AlbumDetailFragmentArgs by navArgs()
     private val detailModel: DetailViewModel by activityViewModels()
-    private val playbackModel: PlaybackViewModel by activityViewModels {
-        PlaybackViewModel.Factory(requireContext())
-    }
+    private val playbackModel: PlaybackViewModel by activityViewModels()
 
     override fun onCreateView(
         inflater: LayoutInflater,
diff --git a/app/src/main/java/org/oxycblt/auxio/detail/ArtistDetailFragment.kt b/app/src/main/java/org/oxycblt/auxio/detail/ArtistDetailFragment.kt
index be6f26c5e..b9f9473eb 100644
--- a/app/src/main/java/org/oxycblt/auxio/detail/ArtistDetailFragment.kt
+++ b/app/src/main/java/org/oxycblt/auxio/detail/ArtistDetailFragment.kt
@@ -20,9 +20,7 @@ import org.oxycblt.auxio.theme.disable
 class ArtistDetailFragment : Fragment() {
     private val args: ArtistDetailFragmentArgs by navArgs()
     private val detailModel: DetailViewModel by activityViewModels()
-    private val playbackModel: PlaybackViewModel by activityViewModels {
-        PlaybackViewModel.Factory(requireContext())
-    }
+    private val playbackModel: PlaybackViewModel by activityViewModels()
 
     override fun onCreateView(
         inflater: LayoutInflater,
diff --git a/app/src/main/java/org/oxycblt/auxio/detail/GenreDetailFragment.kt b/app/src/main/java/org/oxycblt/auxio/detail/GenreDetailFragment.kt
index e3c528796..b2d0539f4 100644
--- a/app/src/main/java/org/oxycblt/auxio/detail/GenreDetailFragment.kt
+++ b/app/src/main/java/org/oxycblt/auxio/detail/GenreDetailFragment.kt
@@ -21,9 +21,7 @@ class GenreDetailFragment : Fragment() {
 
     private val args: GenreDetailFragmentArgs by navArgs()
     private val detailModel: DetailViewModel by activityViewModels()
-    private val playbackModel: PlaybackViewModel by activityViewModels {
-        PlaybackViewModel.Factory(requireContext())
-    }
+    private val playbackModel: PlaybackViewModel by activityViewModels()
 
     override fun onCreateView(
         inflater: LayoutInflater,
diff --git a/app/src/main/java/org/oxycblt/auxio/library/LibraryFragment.kt b/app/src/main/java/org/oxycblt/auxio/library/LibraryFragment.kt
index a2dad5034..9efda2282 100644
--- a/app/src/main/java/org/oxycblt/auxio/library/LibraryFragment.kt
+++ b/app/src/main/java/org/oxycblt/auxio/library/LibraryFragment.kt
@@ -35,9 +35,7 @@ import org.oxycblt.auxio.theme.resolveAttr
 class LibraryFragment : Fragment(), SearchView.OnQueryTextListener {
 
     private val libraryModel: LibraryViewModel by activityViewModels()
-    private val playbackModel: PlaybackViewModel by activityViewModels {
-        PlaybackViewModel.Factory(requireContext())
-    }
+    private val playbackModel: PlaybackViewModel by activityViewModels()
 
     override fun onCreateView(
         inflater: LayoutInflater,
diff --git a/app/src/main/java/org/oxycblt/auxio/playback/CompactPlaybackFragment.kt b/app/src/main/java/org/oxycblt/auxio/playback/CompactPlaybackFragment.kt
index 256faf423..65e215af4 100644
--- a/app/src/main/java/org/oxycblt/auxio/playback/CompactPlaybackFragment.kt
+++ b/app/src/main/java/org/oxycblt/auxio/playback/CompactPlaybackFragment.kt
@@ -16,9 +16,7 @@ import org.oxycblt.auxio.databinding.FragmentCompactPlaybackBinding
 import org.oxycblt.auxio.music.MusicStore
 
 class CompactPlaybackFragment : Fragment() {
-    private val playbackModel: PlaybackViewModel by activityViewModels {
-        PlaybackViewModel.Factory(requireContext())
-    }
+    private val playbackModel: PlaybackViewModel by activityViewModels()
 
     override fun onCreateView(
         inflater: LayoutInflater,
diff --git a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackFragment.kt b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackFragment.kt
index 68ecef44f..5b398cee1 100644
--- a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackFragment.kt
+++ b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackFragment.kt
@@ -22,9 +22,7 @@ import org.oxycblt.auxio.theme.toColor
 
 // TODO: Add a swipe-to-next-track function using a ViewPager
 class PlaybackFragment : Fragment(), SeekBar.OnSeekBarChangeListener {
-    private val playbackModel: PlaybackViewModel by activityViewModels {
-        PlaybackViewModel.Factory(requireActivity().application)
-    }
+    private val playbackModel: PlaybackViewModel by activityViewModels()
 
     // TODO: Implement nav to artists/albums
     override fun onCreateView(
diff --git a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackService.kt b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackService.kt
index a326598c3..670e719ee 100644
--- a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackService.kt
+++ b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackService.kt
@@ -139,12 +139,13 @@ class PlaybackService : Service(), Player.EventListener, PlaybackStateCallback {
     }
 
     private fun createNotification(): Notification {
-        val notificationManager = getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager
+        val notificationManager =
+            getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager
 
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
             val channel = NotificationChannel(
                 CHANNEL_ID,
-                getString(R.string.description_playback),
+                getString(R.string.label_notif_playback),
                 NotificationManager.IMPORTANCE_DEFAULT
             )
             notificationManager.createNotificationChannel(channel)
@@ -156,7 +157,7 @@ class PlaybackService : Service(), Player.EventListener, PlaybackStateCallback {
         )
             .setSmallIcon(R.drawable.ic_song)
             .setContentTitle(getString(R.string.app_name))
-            .setContentText(getString(R.string.description_playback))
+            .setContentText(getString(R.string.label_is_playing))
             .setPriority(NotificationCompat.PRIORITY_DEFAULT)
             .setChannelId(CHANNEL_ID)
             .build()
diff --git a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt
index b4fb6a574..13050f02e 100644
--- a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt
+++ b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt
@@ -1,13 +1,10 @@
 package org.oxycblt.auxio.playback
 
-import android.content.Context
-import android.content.Intent
 import android.util.Log
 import androidx.lifecycle.LiveData
 import androidx.lifecycle.MutableLiveData
 import androidx.lifecycle.Transformations
 import androidx.lifecycle.ViewModel
-import androidx.lifecycle.ViewModelProvider
 import org.oxycblt.auxio.music.Album
 import org.oxycblt.auxio.music.Artist
 import org.oxycblt.auxio.music.Genre
@@ -21,7 +18,7 @@ import org.oxycblt.auxio.playback.state.PlaybackStateManager
 // TODO: Implement Looping Modes
 // TODO: Implement User Queue
 // TODO: Implement Persistence through Bundles/Databases/Idk
-class PlaybackViewModel(private val context: Context) : ViewModel(), PlaybackStateCallback {
+class PlaybackViewModel() : ViewModel(), PlaybackStateCallback {
     // Playback
     private val mSong = MutableLiveData<Song>()
     val song: LiveData<Song> get() = mSong
@@ -66,13 +63,6 @@ class PlaybackViewModel(private val context: Context) : ViewModel(), PlaybackSta
     private val playbackManager = PlaybackStateManager.getInstance()
 
     init {
-        // Start the service from the ViewModel.
-        // Yes, I know ViewModels aren't supposed to deal with this stuff but for some
-        // reason it only works here.
-        Intent(context, PlaybackService::class.java).also {
-            context.startService(it)
-        }
-
         playbackManager.addCallback(this)
 
         // If the PlaybackViewModel was cleared [signified by the PlaybackStateManager having a
@@ -233,6 +223,8 @@ class PlaybackViewModel(private val context: Context) : ViewModel(), PlaybackSta
     }
 
     private fun restorePlaybackState() {
+        Log.d(this::class.simpleName, "Attempting to restore playback state.")
+
         mSong.value = playbackManager.song
         mPosition.value = playbackManager.position
         mQueue.value = playbackManager.queue
@@ -240,15 +232,4 @@ class PlaybackViewModel(private val context: Context) : ViewModel(), PlaybackSta
         mIsPlaying.value = playbackManager.isPlaying
         mIsShuffling.value = playbackManager.isShuffling
     }
-
-    class Factory(private val context: Context) : ViewModelProvider.Factory {
-        @Suppress("unchecked_cast")
-        override fun <T : ViewModel?> create(modelClass: Class<T>): T {
-            if (modelClass.isAssignableFrom(PlaybackViewModel::class.java)) {
-                return PlaybackViewModel(context) as T
-            }
-
-            throw IllegalArgumentException("Unknown ViewModel class.")
-        }
-    }
 }
diff --git a/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueFragment.kt b/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueFragment.kt
index 4ebdd988d..5c29ad60f 100644
--- a/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueFragment.kt
+++ b/app/src/main/java/org/oxycblt/auxio/playback/queue/QueueFragment.kt
@@ -16,9 +16,7 @@ import org.oxycblt.auxio.theme.applyDivider
 import org.oxycblt.auxio.theme.toColor
 
 class QueueFragment : BottomSheetDialogFragment() {
-    private val playbackModel: PlaybackViewModel by activityViewModels {
-        PlaybackViewModel.Factory(requireActivity().application)
-    }
+    private val playbackModel: PlaybackViewModel by activityViewModels()
 
     override fun getTheme(): Int = R.style.Theme_BottomSheetFix
 
diff --git a/app/src/main/java/org/oxycblt/auxio/songs/SongsFragment.kt b/app/src/main/java/org/oxycblt/auxio/songs/SongsFragment.kt
index 4decf70f2..ca979eceb 100644
--- a/app/src/main/java/org/oxycblt/auxio/songs/SongsFragment.kt
+++ b/app/src/main/java/org/oxycblt/auxio/songs/SongsFragment.kt
@@ -15,9 +15,7 @@ import org.oxycblt.auxio.playback.state.PlaybackMode
 import org.oxycblt.auxio.theme.applyDivider
 
 class SongsFragment : Fragment() {
-    private val playbackModel: PlaybackViewModel by activityViewModels {
-        PlaybackViewModel.Factory(requireContext())
-    }
+    private val playbackModel: PlaybackViewModel by activityViewModels()
 
     override fun onCreateView(
         inflater: LayoutInflater,
diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml
index 0a0c72017..eaa2ed087 100644
--- a/app/src/main/res/values/strings.xml
+++ b/app/src/main/res/values/strings.xml
@@ -27,6 +27,9 @@
     <string name="label_play">Play</string>
     <string name="label_queue">Queue</string>
     <string name="label_queue_add">Add to queue</string>
+    <string name="label_notif_playback">Music Playback</string>
+    <string name="label_service_playback">The music playback service for Auxio.</string>
+    <string name="label_is_playing">Auxio is playing music</string>
 
     <!-- Hint Namespace | EditText Hints -->
     <string name="hint_search_library">Search Library…</string>
@@ -47,9 +50,6 @@
     <string name="description_skip_prev">Skip to last song</string>
     <string name="description_shuffle_on">Turn shuffle on</string>
     <string name="description_shuffle_off">Turn shuffle off</string>
-    <string name="description_service_playback">The music playback service for Auxio</string>
-    <string name="description_notif_playback">Music Playback</string>
-    <string name="description_playback">Auxio is playing music</string>
 
     <!-- Placeholder Namespace | Placeholder values -->
     <string name="placeholder_genre">Unknown Genre</string>