Don't need nullable favicon

Slion · Oct 1, 2023 · bd265fe · bd265fe
1 parent 63ec64c
commit bd265fe
Show file tree

Hide file tree

Showing 8 changed files with 48 additions and 33 deletions.
diff --git a/app/src/main/java/acr/browser/lightning/browser/TabModel.kt b/app/src/main/java/acr/browser/lightning/browser/TabModel.kt
@@ -35,7 +35,7 @@ open class TabModel (
     var title : String,
     var desktopMode: Boolean,
     var darkMode: Boolean,
-    var favicon : Bitmap?,
+    var favicon : Bitmap,
     // Find in page search query
     var searchQuery: String,
     // Define if find in page search was active
@@ -53,7 +53,7 @@ open class TabModel (
                 it.putBoolean(KEY_DARK_MODE, darkMode)
                 it.putString(KEY_SEARCH_QUERY, searchQuery)
                 it.putBoolean(KEY_SEARCH_ACTIVE, searchActive)
-                favicon?.apply {
+                favicon.apply {
                     // Crashlytics was showing our bitmap compression can lead to java.lang.IllegalStateException: Can't compress a recycled bitmap
                     // Therefore we now check if it was recycled before going ahead with compression.
                     // Otherwise we can still proceed without favicon anyway.
@@ -92,10 +92,10 @@ class TabModelFromBundle (
 ): TabModel(
         bundle.getString(URL_KEY)?:"",
         bundle.getString(TAB_TITLE_KEY)?:"",
-        bundle.getBoolean(KEY_DESKTOP_MODE)?:false,
-        bundle.getBoolean(KEY_DARK_MODE)?:false,
-        bundle.getByteArray(TAB_FAVICON_KEY)?.let{BitmapFactory.decodeByteArray(it, 0, it.size)},
+        bundle.getBoolean(KEY_DESKTOP_MODE),
+        bundle.getBoolean(KEY_DARK_MODE),
+        bundle.getByteArray(TAB_FAVICON_KEY).let{BitmapFactory.decodeByteArray(it, 0, it?.size?:0)},
         bundle.getString(KEY_SEARCH_QUERY)?:"",
-        bundle.getBoolean(KEY_SEARCH_ACTIVE)?:false,
+        bundle.getBoolean(KEY_SEARCH_ACTIVE),
         bundle.getBundle(WEB_VIEW_KEY)
 )
diff --git a/app/src/main/java/acr/browser/lightning/browser/activity/BrowserActivity.kt b/app/src/main/java/acr/browser/lightning/browser/activity/BrowserActivity.kt
@@ -210,7 +210,6 @@ abstract class BrowserActivity : ThemedBrowserActivity(), BrowserView, UIControl
     private lateinit var queue: RequestQueue
 
     // Image
-    private var webPageBitmap: Bitmap? = null
     private val backgroundDrawable = ColorDrawable()
     private var incognitoNotification: IncognitoNotification? = null
 
@@ -726,7 +725,6 @@ abstract class BrowserActivity : ThemedBrowserActivity(), BrowserView, UIControl
 
         iBinding.drawerLayout.addDrawerListener(DrawerLocker())
 
-        webPageBitmap = drawable(R.drawable.ic_webpage).toBitmap()
 
         // Show incognito icon in more menu button
         if (isIncognito()) {
@@ -1950,7 +1948,7 @@ abstract class BrowserActivity : ThemedBrowserActivity(), BrowserView, UIControl
                         && currentView.url.isNotBlank()
                         && !currentView.url.isSpecialUrl()) {
                     HistoryEntry(currentView.url, currentView.title).also {
-                        Utils.createShortcut(this, it, currentView.favicon ?: webPageBitmap!!)
+                        Utils.createShortcut(this, it, currentView.favicon)
                         Timber.d("Creating shortcut: ${it.title} ${it.url}")
                     }
                 }

diff --git a/app/src/main/java/acr/browser/lightning/browser/tabs/TabViewHolder.kt b/app/src/main/java/acr/browser/lightning/browser/tabs/TabViewHolder.kt
@@ -27,7 +27,7 @@ class TabViewHolder(
     val iCardView: MaterialCardView = view.findViewById(R.id.tab_item_background)
     // Keep a copy of our tab data to be able to understand what was changed on update
     // TODO: Is that how we should do things?
-    var tab: TabViewState = TabViewState()
+    var tab: TabViewState? = null
 
     init {
         exitButton.setOnClickListener(this)

diff --git a/app/src/main/java/acr/browser/lightning/browser/tabs/TabViewState.kt b/app/src/main/java/acr/browser/lightning/browser/tabs/TabViewState.kt
@@ -3,21 +3,37 @@ package acr.browser.lightning.browser.tabs
 import acr.browser.lightning.view.LightningView
 import android.graphics.Bitmap
 import android.graphics.Color
+import timber.log.Timber
 
 /**
  * @param id The unique id of the tab.
  * @param title The title of the tab.
- * @param favicon The favicon of the tab, may be null.
+ * @param favicon The favicon of the tab.
  * @param isForeground True if the tab is in the foreground, false otherwise.
  */
 data class TabViewState(
         val id: Int = 0,
         val title: String = "",
-        val favicon: Bitmap? = null,
+        val favicon: Bitmap = createDefaultBitmap(),
         val isForeground: Boolean = false,
         val themeColor: Int = Color.TRANSPARENT,
         val isFrozen: Boolean = true
-)
+) {
+    init {
+        // TODO: This is called way too many times from displayTabs() through asTabViewState
+        // Find a way to improve this
+        //Timber.v("init")
+    }
+}
+
+/**
+ * We used a function to be able to log
+ */
+private fun createDefaultBitmap() : Bitmap {
+    Timber.w("createDefaultBitmap - ideally that should never be called")
+    return Bitmap.createBitmap(1,1,Bitmap.Config.ARGB_8888)
+}
+
 
 /**
  * Converts a [LightningView] to a [TabViewState].

diff --git a/app/src/main/java/acr/browser/lightning/browser/tabs/TabsAdapter.kt b/app/src/main/java/acr/browser/lightning/browser/tabs/TabsAdapter.kt
@@ -60,7 +60,7 @@ abstract class TabsAdapter(val uiController: UIController): RecyclerView.Adapter
         super.onViewRecycled(holder)
         // I'm not convinced that's needed
         //(uiController as BrowserActivity).toast("Recycled: " + holder.tab.title)
-        holder.tab = TabViewState()
+        holder.tab = null
     }
 
     /**

diff --git a/app/src/main/java/acr/browser/lightning/browser/tabs/TabsDesktopAdapter.kt b/app/src/main/java/acr/browser/lightning/browser/tabs/TabsDesktopAdapter.kt
@@ -61,21 +61,24 @@ class TabsDesktopAdapter(
         holder.tab = tab.copy();
     }
 
-
+    /**
+     *
+     */
     private fun updateViewHolderFavicon(viewHolder: TabViewHolder, tab: TabViewState) {
         // Apply filter to favicon if needed
-        tab.favicon?.let {
-            val ba = uiController as BrowserActivity
-            if (tab.isForeground) {
-                // Make sure that on light theme with dark tab background because color mode we still inverse favicon color if needed, see github.com
-                viewHolder.favicon.setImageForTheme(it, ColorUtils.calculateLuminance(foregroundTabColor)<0.2)
-            }
-            else {
-                viewHolder.favicon.setImageForTheme(it, ba.useDarkTheme)
-            }
-        } ?: viewHolder.favicon.setImageResource(R.drawable.ic_webpage)
+        val ba = uiController as BrowserActivity
+        if (tab.isForeground) {
+            // Make sure that on light theme with dark tab background because color mode we still inverse favicon color if needed, see github.com
+            viewHolder.favicon.setImageForTheme(tab.favicon, ColorUtils.calculateLuminance(foregroundTabColor)<0.2)
+        }
+        else {
+            viewHolder.favicon.setImageForTheme(tab.favicon, ba.useDarkTheme)
+        }
     }
 
+    /**
+     *
+     */
     private fun updateViewHolderAppearance(viewHolder: TabViewHolder, tab: TabViewState) {
 
         // Just to init our default text color
@@ -89,9 +92,9 @@ class TabsDesktopAdapter(
             viewHolder.txtTitle.setTextColor(newTextColor)
             viewHolder.exitButton.findViewById<ImageView>(R.id.deleteButton).setColorFilter(newTextColor)
             // If we just got to the foreground
-            if (tab.isForeground!=viewHolder.tab.isForeground
+            if (tab.isForeground!=viewHolder.tab?.isForeground
                     // or if our theme color changed
-                    || tab.themeColor!=viewHolder.tab.themeColor
+                    || tab.themeColor!=viewHolder.tab?.themeColor
                     // or if our theme color is different than our UI color, i.e. using favicon color instead of meta theme
                     || tab.themeColor!=uiController.getUiColor()) {
 

diff --git a/app/src/main/java/acr/browser/lightning/browser/tabs/TabsDrawerAdapter.kt b/app/src/main/java/acr/browser/lightning/browser/tabs/TabsDrawerAdapter.kt
@@ -42,12 +42,10 @@ class TabsDrawerAdapter(
         holder.tab = tab.copy();
     }
 
-    private fun updateViewHolderFavicon(viewHolder: TabViewHolder, favicon: Bitmap?, isForeground: Boolean) {
+    private fun updateViewHolderFavicon(viewHolder: TabViewHolder, favicon: Bitmap, isForeground: Boolean) {
         // Apply filter to favicon if needed
-        favicon?.let {
-                val ba = uiController as BrowserActivity
-                viewHolder.favicon.setImageForTheme(it, ba.useDarkTheme)
-        } ?: viewHolder.favicon.setImageResource(R.drawable.ic_webpage)
+        val ba = uiController as BrowserActivity
+        viewHolder.favicon.setImageForTheme(favicon, ba.useDarkTheme)
     }
 
     private fun updateViewHolderBackground(viewHolder: TabViewHolder, isForeground: Boolean) {

diff --git a/app/src/main/java/acr/browser/lightning/view/LightningView.kt b/app/src/main/java/acr/browser/lightning/view/LightningView.kt
@@ -278,7 +278,7 @@ class LightningView(
      *
      * @return a non-null Bitmap with the current favicon.
      */
-    val favicon: Bitmap?
+    val favicon: Bitmap
         get() = titleInfo.getFavicon()
 
     /**