From 568e8c8bc0754e70d587dd9a9ec323fb746a16c2 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 8 Jul 2019 10:10:39 +0200 Subject: [PATCH 1/7] Do not load user icon before Android Pie --- .../java/im/vector/riotx/features/notifications/IconLoader.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/vector/src/main/java/im/vector/riotx/features/notifications/IconLoader.kt b/vector/src/main/java/im/vector/riotx/features/notifications/IconLoader.kt index c606f900..787ac604 100644 --- a/vector/src/main/java/im/vector/riotx/features/notifications/IconLoader.kt +++ b/vector/src/main/java/im/vector/riotx/features/notifications/IconLoader.kt @@ -17,6 +17,7 @@ package im.vector.riotx.features.notifications import android.content.Context +import android.os.Build import android.os.Handler import android.os.HandlerThread import androidx.annotation.WorkerThread @@ -56,9 +57,10 @@ class IconLoader(val context: Context, /** * Get icon of a user. * If already in cache, use it, else load it and call IconLoaderListener.onIconsLoaded() when ready + * Before Android P, this does nothing because the icon won't be used */ fun getUserIcon(path: String?): IconCompat? { - if (path == null) { + if (path == null || Build.VERSION.SDK_INT < Build.VERSION_CODES.P) { return null } From 62a81a556eae61d6d032d700e510b78b6e0d05d1 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 8 Jul 2019 10:26:22 +0200 Subject: [PATCH 2/7] Refresh notification drawer in a background thread. It also fixes the person and room avatar display --- .../features/notifications/BitmapLoader.kt | 72 +++---------------- .../features/notifications/IconLoader.kt | 72 +++---------------- .../NotificationDrawerManager.kt | 34 +++++---- 3 files changed, 42 insertions(+), 136 deletions(-) diff --git a/vector/src/main/java/im/vector/riotx/features/notifications/BitmapLoader.kt b/vector/src/main/java/im/vector/riotx/features/notifications/BitmapLoader.kt index d3506be7..02bf03bf 100644 --- a/vector/src/main/java/im/vector/riotx/features/notifications/BitmapLoader.kt +++ b/vector/src/main/java/im/vector/riotx/features/notifications/BitmapLoader.kt @@ -18,73 +18,38 @@ package im.vector.riotx.features.notifications import android.content.Context import android.graphics.Bitmap -import android.os.Handler -import android.os.HandlerThread import androidx.annotation.WorkerThread import com.bumptech.glide.Glide import com.bumptech.glide.load.DecodeFormat import timber.log.Timber -/** - * FIXME It works, but it does not refresh the notification, when it's already displayed - */ -class BitmapLoader(val context: Context, - val listener: BitmapLoaderListener) { +class BitmapLoader(val context: Context) { /** * Avatar Url -> Icon */ - private val cache = HashMap() - - // URLs to load - private val toLoad = HashSet() + private val cache = HashMap() // Black list of URLs (broken URL, etc.) private val blacklist = HashSet() - private var uiHandler = Handler() - - private val handlerThread: HandlerThread = HandlerThread("BitmapLoader", Thread.MIN_PRIORITY) - private var backgroundHandler: Handler - - init { - handlerThread.start() - backgroundHandler = Handler(handlerThread.looper) - } - /** * Get icon of a room. * If already in cache, use it, else load it and call BitmapLoaderListener.onBitmapsLoaded() when ready */ + @WorkerThread fun getRoomBitmap(path: String?): Bitmap? { if (path == null) { return null } - synchronized(cache) { - if (cache[path] != null) { - return cache[path] - } - - // Add to the queue, if not blacklisted - if (!blacklist.contains(path)) { - if (toLoad.contains(path)) { - // Wait - } else { - toLoad.add(path) - - backgroundHandler.post { - loadRoomBitmap(path) - } - } - } + return cache.getOrPut(path) { + loadRoomBitmap(path) } - - return null } @WorkerThread - private fun loadRoomBitmap(path: String) { + private fun loadRoomBitmap(path: String): Bitmap? { val bitmap = path.let { try { Glide.with(context) @@ -99,26 +64,11 @@ class BitmapLoader(val context: Context, } } - synchronized(cache) { - if (bitmap == null) { - // Add to the blacklist - blacklist.add(path) - } else { - cache[path] = bitmap - } - - toLoad.remove(path) - - if (toLoad.isEmpty()) { - uiHandler.post { - listener.onBitmapsLoaded() - } - } + if (bitmap == null) { + // Add to the blacklist + blacklist.add(path) } - } - - interface BitmapLoaderListener { - fun onBitmapsLoaded() + return bitmap } -} \ No newline at end of file +} diff --git a/vector/src/main/java/im/vector/riotx/features/notifications/IconLoader.kt b/vector/src/main/java/im/vector/riotx/features/notifications/IconLoader.kt index 787ac604..6523ebc0 100644 --- a/vector/src/main/java/im/vector/riotx/features/notifications/IconLoader.kt +++ b/vector/src/main/java/im/vector/riotx/features/notifications/IconLoader.kt @@ -18,8 +18,6 @@ package im.vector.riotx.features.notifications import android.content.Context import android.os.Build -import android.os.Handler -import android.os.HandlerThread import androidx.annotation.WorkerThread import androidx.core.graphics.drawable.IconCompat import com.bumptech.glide.Glide @@ -27,67 +25,34 @@ import com.bumptech.glide.load.DecodeFormat import com.bumptech.glide.request.RequestOptions import timber.log.Timber -/** - * FIXME It works, but it does not refresh the notification, when it's already displayed - */ -class IconLoader(val context: Context, - val listener: IconLoaderListener) { +class IconLoader(val context: Context) { /** * Avatar Url -> Icon */ - private val cache = HashMap() - - // URLs to load - private val toLoad = HashSet() + private val cache = HashMap() // Black list of URLs (broken URL, etc.) private val blacklist = HashSet() - private var uiHandler = Handler() - - private val handlerThread: HandlerThread = HandlerThread("IconLoader", Thread.MIN_PRIORITY) - private var backgroundHandler: Handler - - init { - handlerThread.start() - backgroundHandler = Handler(handlerThread.looper) - } - /** * Get icon of a user. * If already in cache, use it, else load it and call IconLoaderListener.onIconsLoaded() when ready * Before Android P, this does nothing because the icon won't be used */ + @WorkerThread fun getUserIcon(path: String?): IconCompat? { if (path == null || Build.VERSION.SDK_INT < Build.VERSION_CODES.P) { return null } - synchronized(cache) { - if (cache[path] != null) { - return cache[path] - } - - // Add to the queue, if not blacklisted - if (!blacklist.contains(path)) { - if (toLoad.contains(path)) { - // Wait - } else { - toLoad.add(path) - - backgroundHandler.post { - loadUserIcon(path) - } - } - } + return cache.getOrPut(path) { + loadUserIcon(path) } - - return null } @WorkerThread - private fun loadUserIcon(path: String) { + fun loadUserIcon(path: String): IconCompat? { val iconCompat = path.let { try { Glide.with(context) @@ -105,26 +70,11 @@ class IconLoader(val context: Context, } } - synchronized(cache) { - if (iconCompat == null) { - // Add to the blacklist - blacklist.add(path) - } else { - cache[path] = iconCompat - } - - toLoad.remove(path) - - if (toLoad.isEmpty()) { - uiHandler.post { - listener.onIconsLoaded() - } - } + if (iconCompat == null) { + // Add to the blacklist + blacklist.add(path) } - } - - interface IconLoaderListener { - fun onIconsLoaded() + return iconCompat } -} \ No newline at end of file +} diff --git a/vector/src/main/java/im/vector/riotx/features/notifications/NotificationDrawerManager.kt b/vector/src/main/java/im/vector/riotx/features/notifications/NotificationDrawerManager.kt index a704f672..2870b8ba 100644 --- a/vector/src/main/java/im/vector/riotx/features/notifications/NotificationDrawerManager.kt +++ b/vector/src/main/java/im/vector/riotx/features/notifications/NotificationDrawerManager.kt @@ -18,6 +18,9 @@ package im.vector.riotx.features.notifications import android.app.Notification import android.content.Context import android.graphics.Bitmap +import android.os.Handler +import android.os.HandlerThread +import androidx.annotation.WorkerThread import androidx.core.app.NotificationCompat import androidx.core.app.Person import im.vector.matrix.android.api.session.content.ContentUrlResolver @@ -44,6 +47,14 @@ class NotificationDrawerManager @Inject constructor(private val context: Context private val activeSessionHolder: ActiveSessionHolder, private val outdatedDetector: OutdatedEventDetector?) { + private val handlerThread: HandlerThread = HandlerThread("NotificationDrawerManager", Thread.MIN_PRIORITY) + private var backgroundHandler: Handler + + init { + handlerThread.start() + backgroundHandler = Handler(handlerThread.looper) + } + //The first time the notification drawer is refreshed, we force re-render of all notifications private var firstTime = true @@ -53,21 +64,9 @@ class NotificationDrawerManager @Inject constructor(private val context: Context private var currentRoomId: String? = null - private var iconLoader = IconLoader(context, - object : IconLoader.IconLoaderListener { - override fun onIconsLoaded() { - // Force refresh - refreshNotificationDrawer() - } - }) + private var iconLoader = IconLoader(context) - private var bitmapLoader = BitmapLoader(context, - object : BitmapLoader.BitmapLoaderListener { - override fun onBitmapsLoaded() { - // Force refresh - refreshNotificationDrawer() - } - }) + private var bitmapLoader = BitmapLoader(context) /** Should be called as soon as a new event is ready to be displayed. @@ -171,6 +170,13 @@ class NotificationDrawerManager @Inject constructor(private val context: Context fun refreshNotificationDrawer() { + backgroundHandler.post { + refreshNotificationDrawerBg() + } + } + + @WorkerThread + private fun refreshNotificationDrawerBg() { val session = activeSessionHolder.getActiveSession() val user = session.getUser(session.sessionParams.credentials.userId) val myUserDisplayName = user?.displayName ?: "" From 8c872caf78d1495344722af5a30def073aa5f06d Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 8 Jul 2019 10:30:45 +0200 Subject: [PATCH 3/7] Inject IconLoader and BitmapLoader --- .../im/vector/riotx/features/notifications/BitmapLoader.kt | 5 ++++- .../im/vector/riotx/features/notifications/IconLoader.kt | 5 ++++- .../features/notifications/NotificationDrawerManager.kt | 6 ++---- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/vector/src/main/java/im/vector/riotx/features/notifications/BitmapLoader.kt b/vector/src/main/java/im/vector/riotx/features/notifications/BitmapLoader.kt index 02bf03bf..51a025e5 100644 --- a/vector/src/main/java/im/vector/riotx/features/notifications/BitmapLoader.kt +++ b/vector/src/main/java/im/vector/riotx/features/notifications/BitmapLoader.kt @@ -22,8 +22,11 @@ import androidx.annotation.WorkerThread import com.bumptech.glide.Glide import com.bumptech.glide.load.DecodeFormat import timber.log.Timber +import javax.inject.Inject +import javax.inject.Singleton -class BitmapLoader(val context: Context) { +@Singleton +class BitmapLoader @Inject constructor(val context: Context) { /** * Avatar Url -> Icon diff --git a/vector/src/main/java/im/vector/riotx/features/notifications/IconLoader.kt b/vector/src/main/java/im/vector/riotx/features/notifications/IconLoader.kt index 6523ebc0..ef175ca6 100644 --- a/vector/src/main/java/im/vector/riotx/features/notifications/IconLoader.kt +++ b/vector/src/main/java/im/vector/riotx/features/notifications/IconLoader.kt @@ -24,8 +24,11 @@ import com.bumptech.glide.Glide import com.bumptech.glide.load.DecodeFormat import com.bumptech.glide.request.RequestOptions import timber.log.Timber +import javax.inject.Inject +import javax.inject.Singleton -class IconLoader(val context: Context) { +@Singleton +class IconLoader @Inject constructor(val context: Context) { /** * Avatar Url -> Icon diff --git a/vector/src/main/java/im/vector/riotx/features/notifications/NotificationDrawerManager.kt b/vector/src/main/java/im/vector/riotx/features/notifications/NotificationDrawerManager.kt index 2870b8ba..508f60dc 100644 --- a/vector/src/main/java/im/vector/riotx/features/notifications/NotificationDrawerManager.kt +++ b/vector/src/main/java/im/vector/riotx/features/notifications/NotificationDrawerManager.kt @@ -45,6 +45,8 @@ import javax.inject.Singleton @Singleton class NotificationDrawerManager @Inject constructor(private val context: Context, private val activeSessionHolder: ActiveSessionHolder, + private val iconLoader: IconLoader, + private val bitmapLoader: BitmapLoader, private val outdatedDetector: OutdatedEventDetector?) { private val handlerThread: HandlerThread = HandlerThread("NotificationDrawerManager", Thread.MIN_PRIORITY) @@ -64,10 +66,6 @@ class NotificationDrawerManager @Inject constructor(private val context: Context private var currentRoomId: String? = null - private var iconLoader = IconLoader(context) - - private var bitmapLoader = BitmapLoader(context) - /** Should be called as soon as a new event is ready to be displayed. The notification corresponding to this event will not be displayed until From 21357a1ec7661c5cdb89a9e94741da41e84cdaf1 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 8 Jul 2019 10:32:38 +0200 Subject: [PATCH 4/7] private fun --- .../java/im/vector/riotx/features/notifications/IconLoader.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vector/src/main/java/im/vector/riotx/features/notifications/IconLoader.kt b/vector/src/main/java/im/vector/riotx/features/notifications/IconLoader.kt index ef175ca6..205f2b0e 100644 --- a/vector/src/main/java/im/vector/riotx/features/notifications/IconLoader.kt +++ b/vector/src/main/java/im/vector/riotx/features/notifications/IconLoader.kt @@ -55,7 +55,7 @@ class IconLoader @Inject constructor(val context: Context) { } @WorkerThread - fun loadUserIcon(path: String): IconCompat? { + private fun loadUserIcon(path: String): IconCompat? { val iconCompat = path.let { try { Glide.with(context) From 535b41d8180936dc14c56b04bf39f1eb3236d6e8 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 8 Jul 2019 10:49:32 +0200 Subject: [PATCH 5/7] Rename Debouncer to FirstThrottler --- .../main/java/im/vector/riotx/core/extensions/LiveData.kt | 8 ++++---- .../riotx/core/utils/{Debouncer.kt => FirstThrottler.kt} | 5 +++-- .../riotx/features/home/room/list/RoomListFragment.kt | 4 ++-- 3 files changed, 9 insertions(+), 8 deletions(-) rename vector/src/main/java/im/vector/riotx/core/utils/{Debouncer.kt => FirstThrottler.kt} (82%) diff --git a/vector/src/main/java/im/vector/riotx/core/extensions/LiveData.kt b/vector/src/main/java/im/vector/riotx/core/extensions/LiveData.kt index a6ab57fe..a278eab0 100644 --- a/vector/src/main/java/im/vector/riotx/core/extensions/LiveData.kt +++ b/vector/src/main/java/im/vector/riotx/core/extensions/LiveData.kt @@ -19,7 +19,7 @@ package im.vector.riotx.core.extensions import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.LiveData import androidx.lifecycle.Observer -import im.vector.riotx.core.utils.Debouncer +import im.vector.riotx.core.utils.FirstThrottler import im.vector.riotx.core.utils.EventObserver import im.vector.riotx.core.utils.LiveEvent @@ -35,11 +35,11 @@ inline fun LiveData>.observeEvent(owner: LifecycleOwner, crossi this.observe(owner, EventObserver { it.run(observer) }) } -inline fun LiveData>.observeEventDebounced(owner: LifecycleOwner, minimumInterval: Long, crossinline observer: (T) -> Unit) { - val debouncer = Debouncer(minimumInterval) +inline fun LiveData>.observeEventFirstThrottle(owner: LifecycleOwner, minimumInterval: Long, crossinline observer: (T) -> Unit) { + val firstThrottler = FirstThrottler(minimumInterval) this.observe(owner, EventObserver { - if (debouncer.canHandle()) { + if (firstThrottler.canHandle()) { it.run(observer) } }) diff --git a/vector/src/main/java/im/vector/riotx/core/utils/Debouncer.kt b/vector/src/main/java/im/vector/riotx/core/utils/FirstThrottler.kt similarity index 82% rename from vector/src/main/java/im/vector/riotx/core/utils/Debouncer.kt rename to vector/src/main/java/im/vector/riotx/core/utils/FirstThrottler.kt index 60e51ad2..eba0d0d6 100644 --- a/vector/src/main/java/im/vector/riotx/core/utils/Debouncer.kt +++ b/vector/src/main/java/im/vector/riotx/core/utils/FirstThrottler.kt @@ -17,9 +17,10 @@ package im.vector.riotx.core.utils /** - * Simple Debouncer + * Simple ThrottleFirst + * See https://raw.githubusercontent.com/wiki/ReactiveX/RxJava/images/rx-operators/throttleFirst.png */ -class Debouncer(private val minimumInterval: Long = 800) { +class FirstThrottler(private val minimumInterval: Long = 800) { private var lastDate = 0L fun canHandle(): Boolean { diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/list/RoomListFragment.kt b/vector/src/main/java/im/vector/riotx/features/home/room/list/RoomListFragment.kt index 07304ae3..b6e2e24a 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/list/RoomListFragment.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/list/RoomListFragment.kt @@ -33,7 +33,7 @@ import im.vector.riotx.core.di.ScreenComponent import im.vector.riotx.core.epoxy.LayoutManagerStateRestorer import im.vector.riotx.core.error.ErrorFormatter import im.vector.riotx.core.extensions.observeEvent -import im.vector.riotx.core.extensions.observeEventDebounced +import im.vector.riotx.core.extensions.observeEventFirstThrottle import im.vector.riotx.core.platform.OnBackPressed import im.vector.riotx.core.platform.StateView import im.vector.riotx.core.platform.VectorBaseFragment @@ -81,7 +81,7 @@ class RoomListFragment : VectorBaseFragment(), RoomSummaryController.Listener, O setupCreateRoomButton() setupRecyclerView() roomListViewModel.subscribe { renderState(it) } - roomListViewModel.openRoomLiveData.observeEventDebounced(this, 800L) { + roomListViewModel.openRoomLiveData.observeEventFirstThrottle(this, 800L) { navigator.openRoom(requireActivity(), it) } From e90aeff41724891b9340b08e882030bb43519543 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 8 Jul 2019 11:08:23 +0200 Subject: [PATCH 6/7] ThrottleLast the notification drawer manager --- .../notifications/NotificationDrawerManager.kt | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/vector/src/main/java/im/vector/riotx/features/notifications/NotificationDrawerManager.kt b/vector/src/main/java/im/vector/riotx/features/notifications/NotificationDrawerManager.kt index 508f60dc..1e529bc6 100644 --- a/vector/src/main/java/im/vector/riotx/features/notifications/NotificationDrawerManager.kt +++ b/vector/src/main/java/im/vector/riotx/features/notifications/NotificationDrawerManager.kt @@ -168,13 +168,20 @@ class NotificationDrawerManager @Inject constructor(private val context: Context fun refreshNotificationDrawer() { - backgroundHandler.post { - refreshNotificationDrawerBg() - } + // Implement last throttler + Timber.w("refreshNotificationDrawer()") + backgroundHandler.removeCallbacksAndMessages(null) + backgroundHandler.postDelayed( + { + refreshNotificationDrawerBg() + } + , 200) } @WorkerThread private fun refreshNotificationDrawerBg() { + Timber.w("refreshNotificationDrawerBg()") + val session = activeSessionHolder.getActiveSession() val user = session.getUser(session.sessionParams.credentials.userId) val myUserDisplayName = user?.displayName ?: "" From 443fb41d18386b12308517c83a240a18328dd64f Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 8 Jul 2019 11:21:26 +0200 Subject: [PATCH 7/7] Cleanup --- .../riotx/features/notifications/BitmapLoader.kt | 14 ++------------ .../riotx/features/notifications/IconLoader.kt | 14 ++------------ 2 files changed, 4 insertions(+), 24 deletions(-) diff --git a/vector/src/main/java/im/vector/riotx/features/notifications/BitmapLoader.kt b/vector/src/main/java/im/vector/riotx/features/notifications/BitmapLoader.kt index 51a025e5..c3a74bd9 100644 --- a/vector/src/main/java/im/vector/riotx/features/notifications/BitmapLoader.kt +++ b/vector/src/main/java/im/vector/riotx/features/notifications/BitmapLoader.kt @@ -29,13 +29,10 @@ import javax.inject.Singleton class BitmapLoader @Inject constructor(val context: Context) { /** - * Avatar Url -> Icon + * Avatar Url -> Bitmap */ private val cache = HashMap() - // Black list of URLs (broken URL, etc.) - private val blacklist = HashSet() - /** * Get icon of a room. * If already in cache, use it, else load it and call BitmapLoaderListener.onBitmapsLoaded() when ready @@ -53,7 +50,7 @@ class BitmapLoader @Inject constructor(val context: Context) { @WorkerThread private fun loadRoomBitmap(path: String): Bitmap? { - val bitmap = path.let { + return path.let { try { Glide.with(context) .asBitmap() @@ -66,12 +63,5 @@ class BitmapLoader @Inject constructor(val context: Context) { null } } - - if (bitmap == null) { - // Add to the blacklist - blacklist.add(path) - } - - return bitmap } } diff --git a/vector/src/main/java/im/vector/riotx/features/notifications/IconLoader.kt b/vector/src/main/java/im/vector/riotx/features/notifications/IconLoader.kt index 205f2b0e..0c4477f4 100644 --- a/vector/src/main/java/im/vector/riotx/features/notifications/IconLoader.kt +++ b/vector/src/main/java/im/vector/riotx/features/notifications/IconLoader.kt @@ -31,13 +31,10 @@ import javax.inject.Singleton class IconLoader @Inject constructor(val context: Context) { /** - * Avatar Url -> Icon + * Avatar Url -> IconCompat */ private val cache = HashMap() - // Black list of URLs (broken URL, etc.) - private val blacklist = HashSet() - /** * Get icon of a user. * If already in cache, use it, else load it and call IconLoaderListener.onIconsLoaded() when ready @@ -56,7 +53,7 @@ class IconLoader @Inject constructor(val context: Context) { @WorkerThread private fun loadUserIcon(path: String): IconCompat? { - val iconCompat = path.let { + return path.let { try { Glide.with(context) .asBitmap() @@ -72,12 +69,5 @@ class IconLoader @Inject constructor(val context: Context) { IconCompat.createWithBitmap(bitmap) } } - - if (iconCompat == null) { - // Add to the blacklist - blacklist.add(path) - } - - return iconCompat } }