Bug 1804107 - Fix for BaseBrowserFragment toolbar getter null crash and invalidating actions when modifying toolbar

app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt

@@ -33,7 +33,6 @@ import com.google.android.material.snackbar.Snackbar
 import kotlinx.coroutines.Dispatchers.IO
 import kotlinx.coroutines.Dispatchers.Main
 import kotlinx.coroutines.Job
-import kotlinx.coroutines.flow.collect
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.mapNotNull
 import kotlinx.coroutines.launch
@@ -1400,6 +1399,11 @@ abstract class BaseBrowserFragment :
         binding.swipeRefresh.isEnabled = shouldPullToRefreshBeEnabled(inFullScreen)
     }
 
+    @CallSuper
+    internal open fun onUpdateToolbarForConfigurationChange(toolbar: BrowserToolbarView) {
+        toolbar.dismissMenu()
+    }
+
     /*
      * Dereference these views when the fragment view is destroyed to prevent memory leaks
      */
@@ -1472,7 +1476,9 @@ abstract class BaseBrowserFragment :
     override fun onConfigurationChanged(newConfig: Configuration) {
         super.onConfigurationChanged(newConfig)
 
-        _browserToolbarView?.dismissMenu()
+        _browserToolbarView?.let {
+            onUpdateToolbarForConfigurationChange(it)
+        }
     }
 
     // This method is called in response to native web extension messages from

app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt

@@ -5,7 +5,6 @@
 package org.mozilla.fenix.browser
 
 import android.content.Context
-import android.content.res.Configuration
 import android.os.StrictMode
 import android.view.View
 import android.view.ViewGroup
@@ -32,6 +31,7 @@ import org.mozilla.fenix.GleanMetrics.ReaderMode
 import org.mozilla.fenix.R
 import org.mozilla.fenix.components.FenixSnackbar
 import org.mozilla.fenix.components.TabCollectionStorage
+import org.mozilla.fenix.components.toolbar.BrowserToolbarView
 import org.mozilla.fenix.components.toolbar.ToolbarMenu
 import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.nav
@@ -90,7 +90,7 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
 
         browserToolbarView.view.addNavigationAction(homeAction)
 
-        setScreenSize(isTablet = resources.getBoolean(R.bool.tablet))
+        updateToolbarActions(isTablet = resources.getBoolean(R.bool.tablet))
 
         val readerModeAction =
             BrowserToolbar.ToggleButton(
@@ -170,7 +170,14 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
         }
     }
 
-    private fun setScreenSize(isTablet: Boolean) {
+    override fun onUpdateToolbarForConfigurationChange(toolbar: BrowserToolbarView) {
+        super.onUpdateToolbarForConfigurationChange(toolbar)
+
+        updateToolbarActions(isTablet = resources.getBoolean(R.bool.tablet))
+    }
+
+    @VisibleForTesting
+    internal fun updateToolbarActions(isTablet: Boolean) {
         if (isTablet == this.isTablet) return
 
         if (isTablet) {
@@ -280,6 +287,8 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
         refreshAction?.let {
             browserToolbarView.view.addNavigationAction(it)
         }
+
+        browserToolbarView.view.invalidateActions()
     }
 
     private fun removeTabletActions() {
@@ -292,6 +301,8 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
         refreshAction?.let {
             browserToolbarView.view.removeNavigationAction(it)
         }
+
+        browserToolbarView.view.invalidateActions()
     }
 
     override fun onStart() {
@@ -447,9 +458,4 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
     internal fun updateLastBrowseActivity() {
         requireContext().settings().lastBrowseActivity = System.currentTimeMillis()
     }
-
-    override fun onConfigurationChanged(newConfig: Configuration) {
-        super.onConfigurationChanged(newConfig)
-        setScreenSize(isTablet = resources.getBoolean(R.bool.tablet))
-    }
 }

app/src/test/java/org/mozilla/fenix/browser/BrowserFragmentTest.kt

@@ -312,6 +312,21 @@ class BrowserFragmentTest {
         verify(exactly = 1) { toolbarIntegration.invalidateMenu() }
     }
 
+    @Test
+    fun `WHEN toolbar is initialized THEN onConfigurationChanged sets toolbar actions for size in fragment`() {
+        val browserToolbarView: BrowserToolbarView = mockk(relaxed = true)
+
+        browserFragment._browserToolbarView = null
+        browserFragment.onConfigurationChanged(mockk(relaxed = true))
+        verify(exactly = 0) { browserFragment.onUpdateToolbarForConfigurationChange(any()) }
+        verify(exactly = 0) { browserFragment.updateToolbarActions(any()) }
+
+        browserFragment._browserToolbarView = browserToolbarView
+        browserFragment.onConfigurationChanged(mockk(relaxed = true))
+        verify(exactly = 1) { browserFragment.onUpdateToolbarForConfigurationChange(any()) }
+        verify(exactly = 1) { browserFragment.updateToolbarActions(any()) }
+    }
+
     @Test
     fun `WHEN fragment configuration changed THEN menu is dismissed`() {
         val browserToolbarView: BrowserToolbarView = mockk(relaxed = true)
@@ -325,7 +340,9 @@ class BrowserFragmentTest {
 
     @Test
     fun `WHEN fragment configuration screen size changes between tablet and mobile size THEN tablet action items added and removed`() {
+        val browserToolbarView: BrowserToolbarView = mockk(relaxed = true)
         val browserToolbar: BrowserToolbar = mockk(relaxed = true)
+        browserFragment._browserToolbarView = browserToolbarView
         every { browserFragment.browserToolbarView.view } returns browserToolbar
 
         mockkObject(ThemeManager.Companion)
@@ -348,7 +365,9 @@ class BrowserFragmentTest {
 
     @Test
     fun `WHEN fragment configuration change enables tablet size twice THEN tablet action items are only added once`() {
+        val browserToolbarView: BrowserToolbarView = mockk(relaxed = true)
         val browserToolbar: BrowserToolbar = mockk(relaxed = true)
+        browserFragment._browserToolbarView = browserToolbarView
         every { browserFragment.browserToolbarView.view } returns browserToolbar
 
         mockkObject(ThemeManager.Companion)
@@ -370,7 +389,9 @@ class BrowserFragmentTest {
 
     @Test
     fun `WHEN fragment configuration change sets mobile size twice THEN tablet action items are not added or removed`() {
+        val browserToolbarView: BrowserToolbarView = mockk(relaxed = true)
         val browserToolbar: BrowserToolbar = mockk(relaxed = true)
+        browserFragment._browserToolbarView = browserToolbarView
         every { browserFragment.browserToolbarView.view } returns browserToolbar
 
         mockkObject(ThemeManager.Companion)