Commit 1e6da542 authored by Colin Lee's avatar Colin Lee Committed by Emily Kager
Browse files

For #3238: fixes race condition crash, nav destination unknown

parent 0a3b001a
......@@ -9,6 +9,7 @@ import android.content.Intent
import android.os.Bundle
import android.util.AttributeSet
import android.view.View
import androidx.annotation.IdRes
import androidx.appcompat.app.AppCompatActivity
import androidx.appcompat.widget.Toolbar
import androidx.navigation.fragment.NavHostFragment
......@@ -29,6 +30,7 @@ import mozilla.components.support.ktx.kotlin.toNormalizedUrl
import mozilla.components.support.utils.SafeIntent
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.nav
import org.mozilla.fenix.home.HomeFragmentDirections
import org.mozilla.fenix.library.bookmarks.BookmarkFragmentDirections
import org.mozilla.fenix.library.bookmarks.selectfolder.SelectBookmarkFolderFragmentDirections
......@@ -56,10 +58,12 @@ open class HomeActivity : AppCompatActivity() {
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
browsingModeManager = createBrowsingModeManager()
themeManager = createThemeManager(when (browsingModeManager.isPrivate) {
true -> ThemeManager.Theme.Private
false -> ThemeManager.Theme.Normal
})
themeManager = createThemeManager(
when (browsingModeManager.isPrivate) {
true -> ThemeManager.Theme.Private
false -> ThemeManager.Theme.Normal
}
)
setTheme(themeManager.currentTheme)
ThemeManager.applyStatusBarTheme(window, themeManager, this)
......@@ -183,35 +187,55 @@ open class HomeActivity : AppCompatActivity() {
sessionObserver = subscribeToSessions()
if (navHost.navController.currentDestination?.id == R.id.browserFragment) return
@IdRes var fragmentId: Int? = null
val directions = if (!navHost.navController.popBackStack(R.id.browserFragment, false)) {
when (from) {
BrowserDirection.FromGlobal -> NavGraphDirections.actionGlobalBrowser(customTabSessionId)
BrowserDirection.FromHome ->
BrowserDirection.FromGlobal ->
NavGraphDirections.actionGlobalBrowser(customTabSessionId)
BrowserDirection.FromHome -> {
fragmentId = R.id.homeFragment
HomeFragmentDirections.actionHomeFragmentToBrowserFragment(customTabSessionId)
BrowserDirection.FromSearch ->
}
BrowserDirection.FromSearch -> {
fragmentId = R.id.searchFragment
SearchFragmentDirections.actionSearchFragmentToBrowserFragment(customTabSessionId)
BrowserDirection.FromSettings ->
}
BrowserDirection.FromSettings -> {
fragmentId = R.id.settingsFragment
SettingsFragmentDirections.actionSettingsFragmentToBrowserFragment(customTabSessionId)
BrowserDirection.FromBookmarks ->
}
BrowserDirection.FromBookmarks -> {
fragmentId = R.id.bookmarkFragment
BookmarkFragmentDirections.actionBookmarkFragmentToBrowserFragment(customTabSessionId)
BrowserDirection.FromBookmarksFolderSelect ->
}
BrowserDirection.FromBookmarksFolderSelect -> {
fragmentId = R.id.bookmarkSelectFolderFragment
SelectBookmarkFolderFragmentDirections
.actionBookmarkSelectFolderFragmentToBrowserFragment(customTabSessionId)
BrowserDirection.FromHistory ->
}
BrowserDirection.FromHistory -> {
fragmentId = R.id.historyFragment
HistoryFragmentDirections.actionHistoryFragmentToBrowserFragment(customTabSessionId)
BrowserDirection.FromPair ->
}
BrowserDirection.FromPair -> {
fragmentId = R.id.pairFragment
PairFragmentDirections.actionPairFragmentToBrowserFragment(customTabSessionId)
BrowserDirection.FromTurnOnSync ->
}
BrowserDirection.FromTurnOnSync -> {
fragmentId = R.id.turnOnSyncFragment
TurnOnSyncFragmentDirections.actionTurnOnSyncFragmentToBrowserFragment(customTabSessionId)
BrowserDirection.FromAccountProblem ->
}
BrowserDirection.FromAccountProblem -> {
fragmentId = R.id.turnOnSyncFragment
AccountProblemFragmentDirections.actionAccountProblemFragmentToBrowserFragment(customTabSessionId)
}
}
} else {
null
}
directions?.let {
navHost.navController.navigate(it)
navHost.navController.nav(fragmentId, it)
}
}
......@@ -265,10 +289,12 @@ open class HomeActivity : AppCompatActivity() {
CustomTabBrowsingModeManager()
} else {
DefaultBrowsingModeManager(Settings.getInstance(this).createBrowserModeStorage()) {
themeManager.setTheme(when (it.isPrivate()) {
true -> ThemeManager.Theme.Private
false -> ThemeManager.Theme.Normal
})
themeManager.setTheme(
when (it.isPrivate()) {
true -> ThemeManager.Theme.Private
false -> ThemeManager.Theme.Normal
}
)
}
}
}
......
......@@ -17,7 +17,6 @@ import androidx.appcompat.app.AppCompatActivity
import androidx.coordinatorlayout.widget.CoordinatorLayout
import androidx.fragment.app.Fragment
import androidx.lifecycle.ViewModelProviders
import androidx.navigation.Navigation
import androidx.navigation.fragment.NavHostFragment.findNavController
import androidx.transition.TransitionInflater
import com.google.android.material.snackbar.Snackbar
......@@ -78,6 +77,7 @@ import org.mozilla.fenix.components.toolbar.ToolbarUIView
import org.mozilla.fenix.components.toolbar.ToolbarViewModel
import org.mozilla.fenix.customtabs.CustomTabsIntegration
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.nav
import org.mozilla.fenix.ext.requireComponents
import org.mozilla.fenix.ext.urlToTrimmedHost
import org.mozilla.fenix.home.sessioncontrol.Tab
......@@ -443,13 +443,12 @@ class BrowserFragment : Fragment(), BackHandler, CoroutineScope {
.subscribe {
when (it) {
is SearchAction.ToolbarClicked -> {
Navigation
.findNavController(toolbarComponent.getView())
.navigate(
BrowserFragmentDirections.actionBrowserFragmentToSearchFragment(
getSessionById()?.id
)
nav(
R.id.browserFragment,
BrowserFragmentDirections.actionBrowserFragmentToSearchFragment(
getSessionById()?.id
)
)
requireComponents.analytics.metrics.track(
Event.SearchBarTapped(Event.SearchBarTapped.Source.BROWSER)
......@@ -533,11 +532,11 @@ class BrowserFragment : Fragment(), BackHandler, CoroutineScope {
val found = existing.isNotEmpty() && existing[0].url == session.url
if (found) {
launch(Main) {
Navigation.findNavController(requireActivity(), R.id.container)
.navigate(
BrowserFragmentDirections
.actionBrowserFragmentToBookmarkEditFragment(existing[0].guid)
)
nav(
R.id.browserFragment,
BrowserFragmentDirections
.actionBrowserFragmentToBookmarkEditFragment(existing[0].guid)
)
}
} else {
val guid = components.core.bookmarksStorage
......@@ -558,16 +557,13 @@ class BrowserFragment : Fragment(), BackHandler, CoroutineScope {
)
.setAnchorView(toolbarComponent.uiView.view)
.setAction(getString(R.string.edit_bookmark_snackbar_action)) {
Navigation.findNavController(
requireActivity(),
R.id.container
nav(
R.id.browserFragment,
BrowserFragmentDirections
.actionBrowserFragmentToBookmarkEditFragment(
guid
)
)
.navigate(
BrowserFragmentDirections
.actionBrowserFragmentToBookmarkEditFragment(
guid
)
)
}
.setText(getString(R.string.bookmark_saved_snackbar))
.show()
......@@ -687,10 +683,14 @@ class BrowserFragment : Fragment(), BackHandler, CoroutineScope {
ToolbarMenu.Item.Forward -> sessionUseCases.goForward.invoke(getSessionById())
ToolbarMenu.Item.Reload -> sessionUseCases.reload.invoke(getSessionById())
ToolbarMenu.Item.Stop -> sessionUseCases.stopLoading.invoke(getSessionById())
ToolbarMenu.Item.Settings -> Navigation.findNavController(toolbarComponent.getView())
.navigate(BrowserFragmentDirections.actionBrowserFragmentToSettingsFragment())
ToolbarMenu.Item.Library -> Navigation.findNavController(toolbarComponent.getView())
.navigate(BrowserFragmentDirections.actionBrowserFragmentToLibraryFragment())
ToolbarMenu.Item.Settings -> nav(
R.id.browserFragment,
BrowserFragmentDirections.actionBrowserFragmentToSettingsFragment()
)
ToolbarMenu.Item.Library -> nav(
R.id.browserFragment,
BrowserFragmentDirections.actionBrowserFragmentToLibraryFragment()
)
is ToolbarMenu.Item.RequestDesktop -> sessionUseCases.requestDesktopSite.invoke(action.item.isChecked)
ToolbarMenu.Item.Share -> getSessionById()?.let { session ->
session.url.apply {
......@@ -700,7 +700,7 @@ class BrowserFragment : Fragment(), BackHandler, CoroutineScope {
ToolbarMenu.Item.NewPrivateTab -> {
val directions = BrowserFragmentDirections
.actionBrowserFragmentToSearchFragment(null)
Navigation.findNavController(view!!).navigate(directions)
nav(R.id.browserFragment, directions)
(activity as HomeActivity).browsingModeManager.mode = BrowsingModeManager.Mode.Private
}
ToolbarMenu.Item.FindInPage -> {
......@@ -724,7 +724,7 @@ class BrowserFragment : Fragment(), BackHandler, CoroutineScope {
ToolbarMenu.Item.NewTab -> {
val directions = BrowserFragmentDirections
.actionBrowserFragmentToSearchFragment(null)
Navigation.findNavController(view!!).navigate(directions)
nav(R.id.browserFragment, directions)
(activity as HomeActivity).browsingModeManager.mode =
BrowsingModeManager.Mode.Normal
}
......@@ -759,7 +759,7 @@ class BrowserFragment : Fragment(), BackHandler, CoroutineScope {
viewModel?.snackbarAnchorView = nestedScrollQuickAction
view?.let {
val directions = BrowserFragmentDirections.actionBrowserFragmentToCreateCollectionFragment()
Navigation.findNavController(it).navigate(directions)
nav(R.id.browserFragment, directions)
}
}
}
......@@ -793,7 +793,7 @@ class BrowserFragment : Fragment(), BackHandler, CoroutineScope {
sitePermissions = sitePermissions,
gravity = getAppropriateLayoutGravity()
)
Navigation.findNavController(it).navigate(directions)
nav(R.id.browserFragment, directions)
}
}
}
......@@ -876,7 +876,7 @@ class BrowserFragment : Fragment(), BackHandler, CoroutineScope {
private fun shareUrl(url: String) {
val directions = BrowserFragmentDirections.actionBrowserFragmentToShareFragment(url)
Navigation.findNavController(view!!).navigate(directions)
nav(R.id.browserFragment, directions)
}
companion object {
......
......@@ -23,6 +23,7 @@ import mozilla.components.support.ktx.android.view.hideKeyboard
import org.mozilla.fenix.R
import org.mozilla.fenix.ThemeManager
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.nav
import org.mozilla.fenix.utils.Settings
class ToolbarIntegration(
......@@ -67,8 +68,18 @@ class ToolbarIntegration(
)
.build()
val navController = Navigation.findNavController(toolbar)
if (!navController.popBackStack(R.id.homeFragment, false)) {
navController.navigate(R.id.action_browserFragment_to_homeFragment, null, options, extras)
if (!navController.popBackStack(
R.id.homeFragment,
false
)
) {
navController.nav(
R.id.browserFragment,
R.id.action_browserFragment_to_homeFragment,
null,
options,
extras
)
}
},
isPrivate
......
......@@ -4,10 +4,28 @@
package org.mozilla.fenix.ext
import androidx.annotation.IdRes
import androidx.fragment.app.Fragment
import androidx.navigation.NavDirections
import androidx.navigation.NavOptions
import androidx.navigation.Navigator
import androidx.navigation.fragment.NavHostFragment.findNavController
import org.mozilla.fenix.components.Components
/**
* Get the requireComponents of this application.
*/
val androidx.fragment.app.Fragment.requireComponents: Components
val Fragment.requireComponents: Components
get() = requireContext().components
fun Fragment.nav(@IdRes id: Int?, directions: NavDirections) {
findNavController(this).nav(id, directions)
}
fun Fragment.nav(@IdRes id: Int?, directions: NavDirections, extras: Navigator.Extras) {
findNavController(this).nav(id, directions, extras)
}
fun Fragment.nav(@IdRes id: Int?, directions: NavDirections, options: NavOptions) {
findNavController(this).nav(id, directions, options)
}
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
package org.mozilla.fenix.ext
import android.os.Bundle
import androidx.annotation.IdRes
import androidx.navigation.NavController
import androidx.navigation.NavDirections
import androidx.navigation.NavOptions
import androidx.navigation.Navigator
import io.sentry.Sentry
import org.mozilla.fenix.BuildConfig
fun NavController.nav(@IdRes id: Int?, directions: NavDirections) {
if (id == null || this.currentDestination?.id == id) {
this.navigate(directions)
} else {
recordIdException(this.currentDestination?.id, id)
}
}
fun NavController.nav(@IdRes id: Int?, directions: NavDirections, extras: Navigator.Extras) {
if (id == null || this.currentDestination?.id == id) {
this.navigate(directions, extras)
} else {
recordIdException(this.currentDestination?.id, id)
}
}
fun NavController.nav(@IdRes id: Int?, directions: NavDirections, options: NavOptions) {
if (id == null || this.currentDestination?.id == id) {
this.navigate(directions, options)
} else {
recordIdException(this.currentDestination?.id, id)
}
}
fun NavController.nav(
@IdRes id: Int?,
@IdRes destId: Int,
args: Bundle?,
navOptions: NavOptions?,
extras: Navigator.Extras?
) {
if (id == null || this.currentDestination?.id == id) {
this.navigate(destId, args, navOptions, extras)
} else {
recordIdException(this.currentDestination?.id, id)
}
}
fun recordIdException(actual: Int?, expected: Int?) {
if (!BuildConfig.SENTRY_TOKEN.isNullOrEmpty()) {
Sentry.capture("Fragment id $actual did not match expected $expected")
}
}
......@@ -18,7 +18,6 @@ import androidx.constraintlayout.widget.ConstraintLayout.LayoutParams.PARENT_ID
import androidx.fragment.app.Fragment
import androidx.lifecycle.Observer
import androidx.lifecycle.ViewModelProviders
import androidx.navigation.Navigation
import androidx.navigation.fragment.FragmentNavigator
import androidx.navigation.fragment.NavHostFragment.findNavController
import androidx.transition.TransitionInflater
......@@ -50,6 +49,7 @@ import org.mozilla.fenix.collections.CreateCollectionViewModel
import org.mozilla.fenix.collections.SaveCollectionStep
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.nav
import org.mozilla.fenix.ext.requireComponents
import org.mozilla.fenix.ext.urlToTrimmedHost
import org.mozilla.fenix.home.sessioncontrol.CollectionAction
......@@ -220,7 +220,7 @@ class HomeFragment : Fragment(), CoroutineScope, AccountObserver {
FragmentNavigator.Extras.Builder()
.addSharedElement(toolbar_wrapper, "toolbar_wrapper_transition")
.build()
Navigation.findNavController(it).navigate(directions, extras)
nav(R.id.homeFragment, directions, extras)
requireComponents.analytics.metrics.track(Event.SearchBarTapped(Event.SearchBarTapped.Source.HOME))
}
......@@ -328,7 +328,7 @@ class HomeFragment : Fragment(), CoroutineScope, AccountObserver {
FragmentNavigator.Extras.Builder()
.addSharedElement(action.tabView, "$TAB_ITEM_TRANSITION_NAME${action.sessionId}")
.build()
Navigation.findNavController(action.tabView).navigate(directions, extras)
nav(R.id.homeFragment, directions, extras)
}
is TabAction.Close -> {
if (deleteSessionJob == null) removeTabWithUndo(action.sessionId) else {
......@@ -362,7 +362,7 @@ class HomeFragment : Fragment(), CoroutineScope, AccountObserver {
is TabAction.Add -> {
invokePendingDeleteJobs()
val directions = HomeFragmentDirections.actionHomeFragmentToSearchFragment(null)
Navigation.findNavController(view!!).navigate(directions)
nav(R.id.homeFragment, directions)
}
is TabAction.ShareTabs -> {
invokePendingDeleteJobs()
......@@ -504,14 +504,16 @@ class HomeFragment : Fragment(), CoroutineScope, AccountObserver {
HomeMenu.Item.Settings -> {
invokePendingDeleteJobs()
onboarding.finish()
Navigation.findNavController(homeLayout).navigate(
nav(
R.id.homeFragment,
HomeFragmentDirections.actionHomeFragmentToSettingsFragment()
)
}
HomeMenu.Item.Library -> {
invokePendingDeleteJobs()
onboarding.finish()
Navigation.findNavController(homeLayout).navigate(
nav(
R.id.homeFragment,
HomeFragmentDirections.actionHomeFragmentToLibraryFragment()
)
}
......@@ -673,14 +675,14 @@ class HomeFragment : Fragment(), CoroutineScope, AccountObserver {
view?.let {
val directions = HomeFragmentDirections.actionHomeFragmentToCreateCollectionFragment()
Navigation.findNavController(it).navigate(directions)
nav(R.id.homeFragment, directions)
}
}
private fun share(url: String? = null, tabs: List<ShareTab>? = null) {
val directions =
HomeFragmentDirections.actionHomeFragmentToShareFragment(url = url, tabs = tabs?.toTypedArray())
Navigation.findNavController(view!!).navigate(directions)
nav(R.id.homeFragment, directions)
}
private fun currentMode(): Mode = if (!onboarding.userHasBeenOnboarded()) {
......@@ -696,11 +698,25 @@ class HomeFragment : Fragment(), CoroutineScope, AccountObserver {
Mode.Normal
}
override fun onAuthenticationProblems() { emitAccountChanges() }
override fun onAuthenticated(account: OAuthAccount) { emitAccountChanges() }
override fun onError(error: Exception) { emitAccountChanges() }
override fun onLoggedOut() { emitAccountChanges() }
override fun onProfileUpdated(profile: Profile) { emitAccountChanges() }
override fun onAuthenticationProblems() {
emitAccountChanges()
}
override fun onAuthenticated(account: OAuthAccount) {
emitAccountChanges()
}
override fun onError(error: Exception) {
emitAccountChanges()
}
override fun onLoggedOut() {
emitAccountChanges()
}
override fun onProfileUpdated(profile: Profile) {
emitAccountChanges()
}
companion object {
private const val SHARED_TRANSITION_MS = 200L
......
......@@ -22,6 +22,7 @@ import mozilla.appservices.places.BookmarkRoot
import org.mozilla.fenix.R
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.ext.getColorFromAttr
import org.mozilla.fenix.ext.nav
import org.mozilla.fenix.ext.requireComponents
class LibraryFragment : Fragment() {
......@@ -53,15 +54,16 @@ class LibraryFragment : Fragment() {
libraryHistory.setOnClickListener {
requireComponents.analytics.metrics
.track(Event.LibrarySelectedItem(view.context.getString(R.string.library_history)))
Navigation.findNavController(view)
.navigate(LibraryFragmentDirections.actionLibraryFragmentToHistoryFragment())
nav(R.id.libraryFragment, LibraryFragmentDirections.actionLibraryFragmentToHistoryFragment())
}
libraryBookmarks.setOnClickListener {
requireComponents.analytics.metrics
.track(Event.LibrarySelectedItem(view.context.getString(R.string.library_bookmarks)))
Navigation.findNavController(view)
.navigate(LibraryFragmentDirections.actionLibraryFragmentToBookmarksFragment(BookmarkRoot.Mobile.id))
nav(
R.id.libraryFragment,
LibraryFragmentDirections.actionLibraryFragmentToBookmarksFragment(BookmarkRoot.Mobile.id)
)
}
requireComponents.analytics.metrics.track(Event.LibraryOpened)
......@@ -95,6 +97,6 @@ class LibraryFragment : Fragment() {
toolbar.setBackgroundColor(backgroundColor)
toolbar.setTitleTextColor(foregroundColor)
toolbar.navigationIcon?.colorFilter =
PorterDuffColorFilter(foregroundColor, PorterDuff.Mode.SRC_IN)
PorterDuffColorFilter(foregroundColor, PorterDuff.Mode.SRC_IN)
}
}
......@@ -45,6 +45,7 @@ import org.mozilla.fenix.components.FenixSnackbar
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.components.metrics.MetricController
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.nav
import org.mozilla.fenix.ext.urlToTrimmedHost
import org.mozilla.fenix.mvi.ActionBusFactory
import org.mozilla.fenix.mvi.getAutoDisposeObservable
......@@ -197,18 +198,20 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve
metrics()?.track(Event.OpenedBookmark)
}
is BookmarkAction.Expand -> {
navigation
.navigate(BookmarkFragmentDirections.actionBookmarkFragmentSelf(it.folder.guid))
nav(
R.id.bookmarkFragment,
BookmarkFragmentDirections.actionBookmarkFragmentSelf(it.folder.guid)
)
}
is BookmarkAction.BackPressed -> {
navigation.popBackStack()
}
is BookmarkAction.Edit -> {
navigation
.navigate(
BookmarkFragmentDirections
.actionBookmarkFragmentToBookmarkEditFragment(it.item.guid)
)
nav(
R.id.bookmarkFragment,
BookmarkFragmentDirections
.actionBookmarkFragmentToBookmarkEditFragment(it.item.guid)
)
}
is BookmarkAction.Select -> {
getManagedEmitter<BookmarkChange>().onNext(BookmarkChange.IsSelected(it.item))
......@@ -224,7 +227,8 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve
}
is BookmarkAction.Share -> {
it.item.url?.apply {
navigation.navigate(
nav(
R.id.bookmarkFragment,
BookmarkFragmentDirections.actionBookmarkFragmentToShareFragment(
this,
it.item.title
......@@ -308,18 +312,17 @@ class BookmarkFragment : Fragment(), CoroutineScope, BackHandler, AccountObserve
(activity as HomeActivity).browsingModeManager.mode = BrowsingModeManager.Mode.Normal
(activity as HomeActivity).supportActionBar?.hide()
navigation
.navigate(BookmarkFragmentDirections.actionBookmarkFragmentToHomeFragment())
nav(R.id.bookmarkFragment, BookmarkFragmentDirections.actionBookmarkFragmentToHomeFragment())
metrics()?.track(Event.OpenedBookmarksInNewTabs)
true
}
R.id.edit_bookmark_multi_select -> {
val bookmark = getSelectedBookmarks().first()
navigation
.navigate(
BookmarkFragmentDirections
.actionBookmarkFragmentToBookmarkEditFragment(bookmark.guid)
)
nav(
R.id.bookmarkFragment,
BookmarkFragmentDirections
.actionBookmarkFragmentToBookmarkEditFragment(bookmark.guid)
)
true