Skip to content

Commit

Permalink
prevent MainActivity from leaking through the DrawerImageLoader singl…
Browse files Browse the repository at this point in the history
…eton (#4153)

Another fix for a memory leak. This one is not as big as #4150, but
still worth fixing for memory constrained devices imo.
The `DrawerImageLoader` implementation (a global singleton) references a
member of the `MainActivity`, causing the whole activity to leak.

This weird construct was introduced in #1989 to fix a crash, but I think
since we migrated to coroutines it is no longer necessary because all
calls get correctly cancelled. I tried reproducing the crash but could
not, so I'm pretty sure it is fine. I would appreciate it if someone
else could try it as well though.

(The crash could be reproduced on slow internet, when
`onFetchUserInfoSuccess` was called while the activity was being
destroyed, causing Glide to crash the app because it can't use destroyed
activities. `onFetchUserInfoSuccess` is now no longer called in this
case because it is inside a `lifecycleScope.launch` block.)
  • Loading branch information
connyduck authored Dec 10, 2023
1 parent ee3760f commit 75c42cb
Showing 1 changed file with 15 additions and 12 deletions.
27 changes: 15 additions & 12 deletions app/src/main/java/com/keylesspalace/tusky/MainActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ import androidx.preference.PreferenceManager
import androidx.viewpager2.widget.MarginPageTransformer
import at.connyduck.calladapter.networkresult.fold
import com.bumptech.glide.Glide
import com.bumptech.glide.RequestManager
import com.bumptech.glide.load.resource.bitmap.RoundedCorners
import com.bumptech.glide.request.target.CustomTarget
import com.bumptech.glide.request.target.FixedSizeDrawable
Expand Down Expand Up @@ -174,8 +173,6 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje

private val preferences by unsafeLazy { PreferenceManager.getDefaultSharedPreferences(this) }

private lateinit var glide: RequestManager

// We need to know if the emoji pack has been changed
private var selectedEmojiPack: String? = null

Expand All @@ -187,7 +184,6 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje

private var directMessageTab: TabLayout.Tab? = null

@Suppress("DEPRECATION")
@SuppressLint("RestrictedApi")
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
Expand Down Expand Up @@ -266,8 +262,6 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje
window.statusBarColor = Color.TRANSPARENT // don't draw a status bar, the DrawerLayout and the MaterialDrawerLayout have their own
setContentView(binding.root)

glide = Glide.with(this)

binding.composeButton.setOnClickListener {
val composeIntent = Intent(applicationContext, ComposeActivity::class.java)
startActivity(composeIntent)
Expand Down Expand Up @@ -562,19 +556,21 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje
DrawerImageLoader.init(object : AbstractDrawerImageLoader() {
override fun set(imageView: ImageView, uri: Uri, placeholder: Drawable, tag: String?) {
if (animateAvatars) {
glide.load(uri)
Glide.with(imageView)
.load(uri)
.placeholder(placeholder)
.into(imageView)
} else {
glide.asBitmap()
Glide.with(imageView)
.asBitmap()
.load(uri)
.placeholder(placeholder)
.into(imageView)
}
}

override fun cancel(imageView: ImageView) {
glide.clear(imageView)
Glide.with(imageView).clear(imageView)
}

override fun placeholder(ctx: Context, tag: String?): Drawable {
Expand Down Expand Up @@ -979,7 +975,8 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje
}

private fun onFetchUserInfoSuccess(me: Account) {
glide.asBitmap()
Glide.with(header.accountHeaderBackground)
.asBitmap()
.load(me.header)
.into(header.accountHeaderBackground)

Expand Down Expand Up @@ -1021,7 +1018,10 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje
val navIconSize = resources.getDimensionPixelSize(R.dimen.avatar_toolbar_nav_icon_size)

if (animateAvatars) {
glide.asDrawable().load(avatarUrl).transform(RoundedCorners(resources.getDimensionPixelSize(R.dimen.avatar_radius_36dp)))
Glide.with(this)
.asDrawable()
.load(avatarUrl)
.transform(RoundedCorners(resources.getDimensionPixelSize(R.dimen.avatar_radius_36dp)))
.apply {
if (showPlaceholder) placeholder(R.drawable.avatar_default)
}
Expand All @@ -1048,7 +1048,10 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje
}
})
} else {
glide.asBitmap().load(avatarUrl).transform(RoundedCorners(resources.getDimensionPixelSize(R.dimen.avatar_radius_36dp)))
Glide.with(this)
.asBitmap()
.load(avatarUrl)
.transform(RoundedCorners(resources.getDimensionPixelSize(R.dimen.avatar_radius_36dp)))
.apply {
if (showPlaceholder) placeholder(R.drawable.avatar_default)
}
Expand Down

0 comments on commit 75c42cb

Please sign in to comment.