Skip to content

Commit

Permalink
Merge pull request #20197 from wordpress-mobile/issue/20196-main-thre…
Browse files Browse the repository at this point in the history 
…ad-blocking

Fix the blocking issue in main thread in the Notifications tab
  • Loading branch information
Antonis Lilis authored Feb 16, 2024
2 parents e302eb7 + 75b431d commit 0c81a3b
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 69 deletions.
78 changes: 42 additions & 36 deletions ...ess/src/main/java/org/wordpress/android/ui/notifications/NotificationsListFragmentPage.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,11 @@ import androidx.recyclerview.widget.LinearLayoutManager
import androidx.recyclerview.widget.RecyclerView
import androidx.recyclerview.widget.RecyclerView.OnScrollListener
import dagger.hilt.android.AndroidEntryPoint
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import org.greenrobot.eventbus.EventBus
import org.greenrobot.eventbus.Subscribe
import org.greenrobot.eventbus.ThreadMode.MAIN
Expand Down Expand Up @@ -77,7 +80,6 @@ class NotificationsListFragmentPage : ViewPagerFragment(R.layout.notifications_l
private lateinit var notesAdapter: NotesAdapter
private var swipeToRefreshHelper: SwipeToRefreshHelper? = null
private var isAnimatingOutNewNotificationsBar = false
private var shouldRefreshNotifications = false
private var tabPosition = 0
private val viewModel: NotificationsListViewModel by viewModels()

Expand Down Expand Up @@ -105,7 +107,6 @@ class NotificationsListFragmentPage : ViewPagerFragment(R.layout.notifications_l
@Suppress("DEPRECATION", "OVERRIDE_DEPRECATION")
override fun onActivityResult(requestCode: Int, resultCode: Int, data: Intent?) {
if (requestCode == RequestCodes.NOTE_DETAIL) {
shouldRefreshNotifications = false
if (resultCode == Activity.RESULT_OK) {
val noteId = data?.getStringExtra(NotificationsListFragment.NOTE_MODERATE_ID_EXTRA)
val newStatus = data?.getStringExtra(NotificationsListFragment.NOTE_MODERATE_STATUS_EXTRA)
Expand All @@ -119,7 +120,6 @@ class NotificationsListFragmentPage : ViewPagerFragment(R.layout.notifications_l
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
(requireActivity().application as WordPress).component().inject(this)
shouldRefreshNotifications = true
}

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
Expand All @@ -139,7 +139,7 @@ class NotificationsListFragmentPage : ViewPagerFragment(R.layout.notifications_l
notificationsList.adapter = notesAdapter
swipeToRefreshHelper = WPSwipeToRefreshHelper.buildSwipeToRefreshHelper(notificationsRefresh) {
hideNewNotificationsBar()
fetchNotesFromRemote()
fetchRemoteNotes()
}
layoutNewNotificatons.visibility = View.GONE
layoutNewNotificatons.setOnClickListener { onScrollToTop() }
Expand All @@ -148,11 +148,18 @@ class NotificationsListFragmentPage : ViewPagerFragment(R.layout.notifications_l
viewModel.updatedNote.observe(viewLifecycleOwner) {
notesAdapter.updateNote(it)
}

if (tabPosition == All.ordinal) {
swipeToRefreshHelper?.isRefreshing = true
fetchRemoteNotes()
} else {
notesAdapter.reloadLocalNotes()
}
}

override fun onDestroyView() {
super.onDestroyView()
notesAdapter.cancelReloadNotesTask()
notesAdapter.cancelReloadLocalNotes()
swipeToRefreshHelper = null
binding?.notificationsList?.adapter = null
binding?.notificationsList?.removeCallbacks(showNewUnseenNotificationsRunnable)
Expand All @@ -175,11 +182,6 @@ class NotificationsListFragmentPage : ViewPagerFragment(R.layout.notifications_l
}
}

override fun onPause() {
super.onPause()
shouldRefreshNotifications = true
}

override fun getScrollableViewForUniqueIdProvision(): View? {
return binding?.notificationsList
}
Expand All @@ -188,12 +190,6 @@ class NotificationsListFragmentPage : ViewPagerFragment(R.layout.notifications_l
super.onResume()
binding?.hideNewNotificationsBar()
EventBus.getDefault().post(NotificationsUnseenStatus(false))
if (accountStore.hasAccessToken()) {
notesAdapter.reloadNotesFromDBAsync()
if (shouldRefreshNotifications) {
fetchNotesFromRemote()
}
}
}

override fun onSaveInstanceState(outState: Bundle) {
Expand Down Expand Up @@ -265,11 +261,15 @@ class NotificationsListFragmentPage : ViewPagerFragment(R.layout.notifications_l
private fun NotificationsListFragmentPageBinding.clearPendingNotificationsItemsOnUI() {
hideNewNotificationsBar()
EventBus.getDefault().post(NotificationsUnseenStatus(false))
NotificationsActions.updateNotesSeenTimestamp()
Thread { gcmMessageHandler.removeAllNotifications(activity) }.start()
lifecycleScope.launch {
withContext(Dispatchers.IO) {
NotificationsActions.updateNotesSeenTimestamp()
gcmMessageHandler.removeAllNotifications(activity)
}
}
}

private fun fetchNotesFromRemote() {
private fun fetchRemoteNotes() {
if (!isAdded) {
return
}
Expand Down Expand Up @@ -422,12 +422,14 @@ class NotificationsListFragmentPage : ViewPagerFragment(R.layout.notifications_l
}
}

private fun updateNote(noteId: String, status: CommentStatus) {
val note = NotificationsTable.getNoteById(noteId)
if (note != null) {
note.localStatus = status.toString()
NotificationsTable.saveNote(note)
EventBus.getDefault().post(NotificationsChanged())
private fun updateNote(noteId: String, status: CommentStatus) = lifecycleScope.launch {
withContext(Dispatchers.IO) {
val note = NotificationsTable.getNoteById(noteId)
if (note != null) {
note.localStatus = status.toString()
NotificationsTable.saveNote(note)
EventBus.getDefault().post(NotificationsChanged())
}
}
}

Expand Down Expand Up @@ -456,27 +458,31 @@ class NotificationsListFragmentPage : ViewPagerFragment(R.layout.notifications_l

@Subscribe(sticky = true, threadMode = MAIN)
fun onEventMainThread(event: NoteLikeOrModerationStatusChanged) {
NotificationsActions.downloadNoteAndUpdateDB(
event.noteId,
{
EventBus.getDefault()
.removeStickyEvent(
lifecycleScope.launch {
withContext(Dispatchers.IO) {
NotificationsActions.downloadNoteAndUpdateDB(
event.noteId,
{
EventBus.getDefault()
.removeStickyEvent(
NoteLikeOrModerationStatusChanged::class.java
)
}
) {
EventBus.getDefault().removeStickyEvent(
NoteLikeOrModerationStatusChanged::class.java
)
}
}
) {
EventBus.getDefault().removeStickyEvent(
NoteLikeOrModerationStatusChanged::class.java
)
}
}

@Subscribe(threadMode = MAIN)
@Subscribe(sticky = true, threadMode = MAIN)
fun onEventMainThread(event: NotificationsChanged) {
if (!isAdded) {
return
}
notesAdapter.reloadNotesFromDBAsync()
notesAdapter.reloadLocalNotes()
if (event.hasUnseenNotes) {
binding?.showNewUnseenNotificationsUI()
}
Expand Down
68 changes: 35 additions & 33 deletions WordPress/src/main/java/org/wordpress/android/ui/notifications/adapters/NotesAdapter.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@

package org.wordpress.android.ui.notifications.adapters

import android.annotation.SuppressLint
import android.content.Context
import android.content.res.ColorStateList
import android.os.AsyncTask
import android.text.Spanned
import android.text.TextUtils
import android.view.LayoutInflater
Expand All @@ -20,11 +18,14 @@ import androidx.core.text.BidiFormatter
import androidx.core.view.ViewCompat
import androidx.core.view.isVisible
import androidx.core.widget.ImageViewCompat
import androidx.recyclerview.widget.DiffUtil
import androidx.recyclerview.widget.RecyclerView
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import org.wordpress.android.R
import org.wordpress.android.WordPress
import org.wordpress.android.datasets.NotificationsTable
Expand Down Expand Up @@ -56,6 +57,7 @@ class NotesAdapter(
private val textIndentSize: Int
private val dataLoadedListener: DataLoadedListener
private val onLoadMoreListener: OnLoadMoreListener?
private val coroutineScope = CoroutineScope(Dispatchers.IO)
val filteredNotes = ArrayList<Note>()

@Inject
Expand Down Expand Up @@ -84,7 +86,7 @@ class NotesAdapter(

var currentFilter = FILTERS.FILTER_ALL
private set
private var reloadNotesFromDBTask: ReloadNotesFromDBTask? = null
private var reloadLocalNotesJob: Job? = null

interface DataLoadedListener {
fun onDataLoaded(itemsCount: Int)
Expand All @@ -99,13 +101,25 @@ class NotesAdapter(
currentFilter = newFilter
}

@SuppressLint("NotifyDataSetChanged")
fun addAll(notes: List<Note>) {
try {
filteredNotes.clear()
filteredNotes.addAll(buildFilteredNotesList(notes, currentFilter))
} finally {
notifyDataSetChanged()
/**
* Add notes to the adapter and notify the change
*/
fun addAll(notes: List<Note>) = coroutineScope.launch {
val newNotes = buildFilteredNotesList(notes, currentFilter)
val result = DiffUtil.calculateDiff(object : DiffUtil.Callback() {
override fun getOldListSize(): Int = filteredNotes.size
override fun getNewListSize(): Int = newNotes.size
override fun areItemsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean =
filteredNotes[oldItemPosition].id == newNotes[newItemPosition].id

override fun areContentsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean =
filteredNotes[oldItemPosition].json.toString() == newNotes[newItemPosition].json.toString()
})

filteredNotes.clear()
filteredNotes.addAll(newNotes)
withContext(Dispatchers.Main) {
result.dispatchUpdatesTo(this@NotesAdapter)
dataLoadedListener.onDataLoaded(itemCount)
}
}
Expand All @@ -121,7 +135,6 @@ class NotesAdapter(

private fun isValidPosition(position: Int): Boolean = position >= 0 && position < filteredNotes.size


override fun getItemCount(): Int = filteredNotes.size

private val Note.timeGroup
Expand Down Expand Up @@ -278,17 +291,19 @@ class NotesAdapter(
onNoteClickListener = mNoteClickListener
}

fun cancelReloadNotesTask() {
if (reloadNotesFromDBTask != null && reloadNotesFromDBTask!!.status != AsyncTask.Status.FINISHED) {
reloadNotesFromDBTask!!.cancel(true)
reloadNotesFromDBTask = null
}
fun cancelReloadLocalNotes() {
reloadLocalNotesJob?.cancel()
}

fun reloadNotesFromDBAsync() {
cancelReloadNotesTask()
reloadNotesFromDBTask = ReloadNotesFromDBTask()
reloadNotesFromDBTask!!.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR)
/**
* Reload the notes from local database and update the adapter
*/
fun reloadLocalNotes() {
cancelReloadLocalNotes()
reloadLocalNotesJob = coroutineScope.launch {
val notes = NotificationsTable.getLatestNotes()
addAll(notes)
}
}

/**
Expand All @@ -302,17 +317,6 @@ class NotesAdapter(
}
}

@SuppressLint("StaticFieldLeak")
private inner class ReloadNotesFromDBTask : AsyncTask<Void?, Void?, ArrayList<Note>>() {
override fun doInBackground(vararg voids: Void?): ArrayList<Note> {
return NotificationsTable.getLatestNotes()
}

override fun onPostExecute(notes: ArrayList<Note>) {
addAll(notes)
}
}

inner class NoteViewHolder(view: View) : RecyclerView.ViewHolder(view) {
val contentView: View
val headerText: TextView
Expand All @@ -330,8 +334,6 @@ class NotesAdapter(
val unreadNotificationView: View
val actionIcon: ImageView

val coroutineScope = CoroutineScope(Dispatchers.Main)

init {
contentView = checkNotNull(view.findViewById(R.id.note_content_container))
headerText = checkNotNull(view.findViewById(R.id.header_text))
Expand Down

0 comments on commit 0c81a3b

Please sign in to comment.