diff --git a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt index 708b834e75..f06f37d824 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -144,7 +144,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!! @@ -1295,4 +1297,17 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, browserToolbarView.setScrollFlags(enabled) } } + + // 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 index f6ba9f5057..bf8bd8fc28 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt @@ -40,7 +40,6 @@ import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.navigateSafe import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.settings -import org.mozilla.fenix.ext.runIfFragmentIsAttached import org.mozilla.fenix.shortcut.PwaOnboardingObserver import org.mozilla.fenix.trackingprotection.TrackingProtectionOverlay @@ -125,11 +124,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 index 846569d683..b3482825a4 100644 --- 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) @@ -301,6 +305,47 @@ class BrowserFragmentTest { assert(!browserFragment.shouldPullToRefreshBeEnabled(true)) } + @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()