Closes mozilla-mobile#17441: NPE when invalidating toolbar in respons…

…e to reader changes
fork-maintainers · Jan 13, 2021 · f0e10a7 · f0e10a7
1 parent 44d9c30
commit f0e10a7
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 7 deletions.
diff --git a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt
@@ -146,7 +146,9 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler,
     protected val browserInteractor: BrowserToolbarViewInteractor
         get() = _browserInteractor!!
 
-    private var _browserToolbarView: BrowserToolbarView? = null
+    @VisibleForTesting
+    @Suppress("VariableNaming")
+    internal var _browserToolbarView: BrowserToolbarView? = null
     @VisibleForTesting
     internal val browserToolbarView: BrowserToolbarView
         get() = _browserToolbarView!!
@@ -1297,4 +1299,17 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler,
             .actionGlobalSitePermissionsManagePhoneFeature(PhoneFeature.AUTOPLAY_AUDIBLE)
         findNavController().navigate(directions)
     }
+
+    // This method is called in response to native web extension messages from
+    // content scripts (e.g the reader view extension). By the time these
+    // messages are processed the fragment/view may no longer be attached.
+    internal fun safeInvalidateBrowserToolbarView() {
+        runIfFragmentIsAttached {
+            val toolbarView = _browserToolbarView
+            if (toolbarView != null) {
+                toolbarView.view.invalidateActions()
+                toolbarView.toolbarIntegration.invalidateMenu()
+            }
+        }
+    }
 }
diff --git a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt
@@ -30,7 +30,6 @@ import mozilla.components.feature.tabs.WindowFeature
 import mozilla.components.support.base.feature.UserInteractionHandler
 import mozilla.components.support.base.feature.ViewBoundFeatureWrapper
 import org.mozilla.fenix.R
-import org.mozilla.fenix.addons.runIfFragmentIsAttached
 import org.mozilla.fenix.components.FenixSnackbar
 import org.mozilla.fenix.components.TabCollectionStorage
 import org.mozilla.fenix.components.metrics.Event
@@ -122,11 +121,7 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
 
                         readerModeAvailable = available
                         readerModeAction.setSelected(active)
-
-                        runIfFragmentIsAttached {
-                            browserToolbarView.view.invalidateActions()
-                            browserToolbarView.toolbarIntegration.invalidateMenu()
-                        }
+                        safeInvalidateBrowserToolbarView()
                     }
                 },
                 owner = this,

diff --git a/app/src/test/java/org/mozilla/fenix/browser/BrowserFragmentTest.kt b/app/src/test/java/org/mozilla/fenix/browser/BrowserFragmentTest.kt
@@ -25,9 +25,11 @@ import mozilla.components.browser.state.state.SessionState
 import mozilla.components.browser.state.state.TabSessionState
 import mozilla.components.browser.state.state.createTab
 import mozilla.components.browser.state.store.BrowserStore
+import mozilla.components.browser.toolbar.BrowserToolbar
 import mozilla.components.support.test.ext.joinBlocking
 import mozilla.components.support.test.rule.MainCoroutineRule
 import org.junit.After
+import org.junit.Assert.fail
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
@@ -36,11 +38,13 @@ import org.mozilla.fenix.FenixApplication
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
 import org.mozilla.fenix.components.toolbar.BrowserToolbarView
+import org.mozilla.fenix.components.toolbar.ToolbarIntegration
 import org.mozilla.fenix.ext.application
 import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.settings
 import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
 import org.mozilla.fenix.onboarding.FenixOnboarding
+import java.lang.Exception
 
 @ExperimentalCoroutinesApi
 @RunWith(FenixRobolectricTestRunner::class)
@@ -305,6 +309,47 @@ class BrowserFragmentTest {
         assert(!browserFragment.shouldPullToRefreshBeEnabled())
     }
 
+    @Test
+    fun `WHEN fragment is not attached THEN toolbar invalidation does nothing`() {
+        val browserToolbarView: BrowserToolbarView = mockk(relaxed = true)
+        val browserToolbar: BrowserToolbar = mockk(relaxed = true)
+        val toolbarIntegration: ToolbarIntegration = mockk(relaxed = true)
+        every { browserToolbarView.view } returns browserToolbar
+        every { browserToolbarView.toolbarIntegration } returns toolbarIntegration
+        every { browserFragment.context } returns null
+        browserFragment._browserToolbarView = browserToolbarView
+        browserFragment.safeInvalidateBrowserToolbarView()
+
+        verify(exactly = 0) { browserToolbar.invalidateActions() }
+        verify(exactly = 0) { toolbarIntegration.invalidateMenu() }
+    }
+
+    @Test
+    @Suppress("TooGenericExceptionCaught")
+    fun `WHEN fragment is attached and toolbar view is null THEN toolbar invalidation is safe`() {
+        every { browserFragment.context } returns mockk(relaxed = true)
+        try {
+            browserFragment.safeInvalidateBrowserToolbarView()
+        } catch (e: Exception) {
+            fail("Exception thrown when invalidating toolbar")
+        }
+    }
+
+    @Test
+    fun `WHEN fragment and view are attached THEN toolbar invalidation is triggered`() {
+        val browserToolbarView: BrowserToolbarView = mockk(relaxed = true)
+        val browserToolbar: BrowserToolbar = mockk(relaxed = true)
+        val toolbarIntegration: ToolbarIntegration = mockk(relaxed = true)
+        every { browserToolbarView.view } returns browserToolbar
+        every { browserToolbarView.toolbarIntegration } returns toolbarIntegration
+        every { browserFragment.context } returns mockk(relaxed = true)
+        browserFragment._browserToolbarView = browserToolbarView
+        browserFragment.safeInvalidateBrowserToolbarView()
+
+        verify(exactly = 1) { browserToolbar.invalidateActions() }
+        verify(exactly = 1) { toolbarIntegration.invalidateMenu() }
+    }
+
     private fun addAndSelectTab(tab: TabSessionState) {
         store.dispatch(TabListAction.AddTabAction(tab)).joinBlocking()
         store.dispatch(TabListAction.SelectTabAction(tab.id)).joinBlocking()