[fenix] Fixes history visual bugs

app/src/main/java/org/mozilla/fenix/library/history/HistoryAdapter.kt

@@ -10,6 +10,7 @@ import android.view.LayoutInflater
 import android.view.ViewGroup
 import androidx.recyclerview.widget.RecyclerView
 import io.reactivex.Observer
+import kotlinx.coroutines.Job
 import org.mozilla.fenix.R
 import org.mozilla.fenix.library.history.viewholders.HistoryDeleteButtonViewHolder
 import org.mozilla.fenix.library.history.viewholders.HistoryHeaderViewHolder
@@ -96,6 +97,7 @@ class HistoryAdapter(
 ) : RecyclerView.Adapter<RecyclerView.ViewHolder>() {
     private var historyList: HistoryList = HistoryList(emptyList())
     private var mode: HistoryState.Mode = HistoryState.Mode.Normal
+    private lateinit var job: Job
 
     fun updateData(items: List<HistoryItem>, mode: HistoryState.Mode) {
         this.historyList = HistoryList(items)
@@ -119,12 +121,11 @@ class HistoryAdapter(
         return when (viewType) {
             HistoryDeleteButtonViewHolder.LAYOUT_ID -> HistoryDeleteButtonViewHolder(view, actionEmitter)
             HistoryHeaderViewHolder.LAYOUT_ID -> HistoryHeaderViewHolder(view)
-            HistoryListItemViewHolder.LAYOUT_ID -> HistoryListItemViewHolder(view, actionEmitter)
+            HistoryListItemViewHolder.LAYOUT_ID -> HistoryListItemViewHolder(view, actionEmitter, job)
             else -> throw IllegalStateException()
         }
     }
 
-
     override fun onBindViewHolder(holder: RecyclerView.ViewHolder, position: Int) {
         when (holder) {
             is HistoryDeleteButtonViewHolder -> holder.bind(mode)
@@ -136,6 +137,16 @@ class HistoryAdapter(
             is HistoryListItemViewHolder -> (historyList.items[position] as AdapterItem.Item).also {
                 holder.bind(it.item, mode)
             }
-       }
+        }
+    }
+
+    override fun onAttachedToRecyclerView(recyclerView: RecyclerView) {
+        super.onAttachedToRecyclerView(recyclerView)
+        job = Job()
+    }
+
+    override fun onDetachedFromRecyclerView(recyclerView: RecyclerView) {
+        super.onDetachedFromRecyclerView(recyclerView)
+        job.cancel()
     }
 }

app/src/main/java/org/mozilla/fenix/library/history/viewholders/HistoryDeleteButtonViewHolder.kt

@@ -17,11 +17,11 @@ class HistoryDeleteButtonViewHolder(
     private val actionEmitter: Observer<HistoryAction>
 ) : RecyclerView.ViewHolder(view) {
     private var mode: HistoryState.Mode? = null
-    val delete_history_button_text = view.delete_history_button_text
+    private val textView = view.delete_history_button_text
-    val delete_history_button = view.delete_history_button
+    private val buttonView = view.delete_history_button
 
     init {
-        delete_history_button.setOnClickListener {
+        buttonView.setOnClickListener {
             mode?.also {
                 val action = when (it) {
                     is HistoryState.Mode.Normal -> HistoryAction.Delete.All
@@ -36,19 +36,19 @@ class HistoryDeleteButtonViewHolder(
         val mode = mode
 
         val text = if (mode is HistoryState.Mode.Editing && mode.selectedItems.isNotEmpty()) {
-            delete_history_button_text.context.resources.getString(
+            textView.context.resources.getString(
                 R.string.history_delete_some,
                 mode.selectedItems.size
             )
         } else {
-            delete_history_button_text.context.resources.getString(R.string.history_delete_all)
+            textView.context.resources.getString(R.string.history_delete_all)
         }
 
-        delete_history_button.contentDescription = text
+        buttonView.contentDescription = text
-        delete_history_button_text.text = text
+        textView.text = text
     }
 
     companion object {
         const val LAYOUT_ID = R.layout.delete_history_button
     }
-}
+}

app/src/main/java/org/mozilla/fenix/library/history/viewholders/HistoryHeaderViewHolder.kt

@@ -21,4 +21,4 @@ class HistoryHeaderViewHolder(
     companion object {
         const val LAYOUT_ID = R.layout.history_header
     }
-}
+}

app/src/main/java/org/mozilla/fenix/library/history/viewholders/HistoryListItemViewHolder.kt

@@ -9,17 +9,28 @@ import android.widget.CompoundButton
 import androidx.recyclerview.widget.RecyclerView
 import io.reactivex.Observer
 import kotlinx.android.synthetic.main.history_list_item.view.*
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.Job
+import kotlinx.coroutines.launch
+import mozilla.components.browser.icons.IconRequest
 import mozilla.components.browser.menu.BrowserMenu
 import org.mozilla.fenix.R
+import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.library.history.HistoryAction
 import org.mozilla.fenix.library.history.HistoryItem
 import org.mozilla.fenix.library.history.HistoryItemMenu
 import org.mozilla.fenix.library.history.HistoryState
+import kotlin.coroutines.CoroutineContext
 
 class HistoryListItemViewHolder(
     view: View,
-    private val actionEmitter: Observer<HistoryAction>
+    private val actionEmitter: Observer<HistoryAction>,
-) : RecyclerView.ViewHolder(view) {
+    val job: Job
+) : RecyclerView.ViewHolder(view), CoroutineScope {
+
+    override val coroutineContext: CoroutineContext
+        get() = Dispatchers.IO + job
 
     private val checkbox = view.should_remove_checkbox
     private val favicon = view.history_favicon
@@ -99,6 +110,8 @@ class HistoryListItemViewHolder(
             }
             checkbox.setOnCheckedChangeListener(checkListener)
         }
+
+        updateFavIcon(item.url)
     }
 
     private fun setupMenu() {
@@ -111,7 +124,17 @@ class HistoryListItemViewHolder(
         }
     }
 
+    private fun updateFavIcon(url: String) {
+        launch(Dispatchers.IO) {
+            val bitmap = favicon.context.components.utils.icons
+                .loadIcon(IconRequest(url)).await().bitmap
+            launch(Dispatchers.Main) {
+                favicon.setImageBitmap(bitmap)
+            }
+        }
+    }
+
     companion object {
         const val LAYOUT_ID = R.layout.history_list_item
     }
-}
+}

app/src/main/res/layout/history_list_item.xml

@@ -52,7 +52,7 @@
         android:textAlignment="viewStart"
         android:textColor="?attr/historyURLColor"
         android:textSize="12sp"
-        app:layout_constraintBottom_toBottomOf="parent"
+        android:layout_marginTop="4dp"
         app:layout_constraintEnd_toStartOf="@id/history_item_overflow"
         app:layout_constraintStart_toEndOf="@id/history_favicon"
         app:layout_constraintTop_toBottomOf="@id/history_title" />
@@ -62,6 +62,7 @@
         android:layout_width="0dp"
         android:layout_height="wrap_content"
         android:layout_marginStart="16dp"
+        android:layout_marginEnd="16dp"
         android:ellipsize="end"
         android:singleLine="true"
         android:textAlignment="viewStart"
@@ -69,5 +70,5 @@
         android:textSize="18sp"
         app:layout_constraintEnd_toStartOf="@id/history_item_overflow"
         app:layout_constraintStart_toEndOf="@id/history_favicon"
-        app:layout_constraintTop_toTopOf="@id/history_item_overflow" />
+        app:layout_constraintTop_toTopOf="@id/history_favicon" />
 </androidx.constraintlayout.widget.ConstraintLayout>