Skip to content

Commit 9400f18

Browse files
committed
Bug 2003511 - Cancel loading bookmarks if weve navigated from the Bookmarks Fragment
[try](https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=174243) Pull request: #34
1 parent 3d99435 commit 9400f18

4 files changed

Lines changed: 18 additions & 1 deletion

File tree

mobile/android/fenix/app/src/main/java/org/mozilla/fenix/bookmarks/BookmarkFragment.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ class BookmarkFragment : Fragment() {
111111
BookmarksSyncMiddleware(requireComponents.backgroundServices.syncStore, lifecycleScope),
112112
BrowserToolbarSyncToBookmarksMiddleware(toolbarStore, lifecycleScope),
113113
BookmarksMiddleware(
114+
lifecycleScope = lifecycleScope,
114115
bookmarksStorage = requireContext().bookmarkStorage,
115116
clipboardManager = requireActivity().getSystemService(),
116117
addNewTabUseCase = requireComponents.useCases.tabsUseCases.addTab,

mobile/android/fenix/app/src/main/java/org/mozilla/fenix/bookmarks/BookmarksMiddleware.kt

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import androidx.navigation.NavController
1010
import kotlinx.coroutines.CoroutineDispatcher
1111
import kotlinx.coroutines.CoroutineScope
1212
import kotlinx.coroutines.Dispatchers
13+
import kotlinx.coroutines.ensureActive
1314
import kotlinx.coroutines.launch
1415
import kotlinx.coroutines.withContext
1516
import mozilla.appservices.places.BookmarkRoot
@@ -51,6 +52,7 @@ private const val WARN_OPEN_ALL_SIZE = 15
5152
* @param reportResultGlobally Invoked when an error occurs that needs to be reported even if the
5253
* feature goes out of scope.
5354
* @param ioDispatcher Coroutine dispatcher for IO operations.
55+
* @param lifecycleScope lifecycle bound CoroutineScope scope used to cancel jobs when leaving bookmarks.
5456
*/
5557
@Suppress("LongParameterList", "LargeClass")
5658
internal class BookmarksMiddleware(
@@ -73,9 +75,10 @@ internal class BookmarksMiddleware(
7375
private val lastSavedFolderCache: LastSavedFolderCache,
7476
private val reportResultGlobally: (BookmarksGlobalResultReport) -> Unit,
7577
private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO,
78+
lifecycleScope: CoroutineScope,
7679
) : Middleware<BookmarksState, BookmarksAction> {
7780

78-
private val scope = CoroutineScope(ioDispatcher)
81+
private val scope = CoroutineScope(lifecycleScope.coroutineContext + ioDispatcher)
7982

8083
@Suppress("LongMethod", "CognitiveComplexMethod", "CyclomaticComplexMethod")
8184
override fun invoke(
@@ -278,6 +281,10 @@ internal class BookmarksMiddleware(
278281
}
279282
// list screen cases
280283
preReductionState.selectedItems.isNotEmpty() -> { /* noop */ }
284+
// User is clicking back before we've loaded anything
285+
preReductionState.currentFolder.guid.isEmpty() -> {
286+
exitBookmarks()
287+
}
281288
preReductionState.currentFolder.guid != BookmarkRoot.Mobile.id -> {
282289
scope.launch {
283290
val parentFolderGuid = withContext(ioDispatcher) {
@@ -516,6 +523,8 @@ internal class BookmarksMiddleware(
516523
private fun Store<BookmarksState, BookmarksAction>.tryDispatchLoadFor(guid: String) =
517524
scope.launch {
518525
bookmarksStorage.getTree(guid).getOrNull()?.let { rootNode ->
526+
ensureActive()
527+
519528
val folder = BookmarkItem.Folder(
520529
guid = guid,
521530
title = resolveFolderTitle(rootNode),

mobile/android/fenix/app/src/main/java/org/mozilla/fenix/bookmarks/EditBookmarkFragment.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import androidx.compose.ui.platform.ComposeView
1212
import androidx.compose.ui.platform.ViewCompositionStrategy
1313
import androidx.core.content.getSystemService
1414
import androidx.fragment.app.Fragment
15+
import androidx.lifecycle.lifecycleScope
1516
import androidx.navigation.NavHostController
1617
import androidx.navigation.fragment.findNavController
1718
import androidx.navigation.fragment.navArgs
@@ -111,6 +112,7 @@ class EditBookmarkFragment : Fragment(R.layout.fragment_edit_bookmark) {
111112
AppAction.BookmarkAction.BookmarkOperationResultReported(it),
112113
)
113114
},
115+
lifecycleScope = lifecycleScope,
114116
),
115117
),
116118
bookmarkToLoad = args.guidToEdit,

mobile/android/fenix/app/src/test/java/org/mozilla/fenix/bookmarks/BookmarksMiddlewareTest.kt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ import androidx.navigation.NavBackStackEntry
99
import androidx.navigation.NavController
1010
import androidx.navigation.NavDestination
1111
import androidx.test.ext.junit.runners.AndroidJUnit4
12+
import kotlinx.coroutines.CoroutineScope
1213
import kotlinx.coroutines.ExperimentalCoroutinesApi
14+
import kotlinx.coroutines.test.StandardTestDispatcher
1315
import kotlinx.coroutines.test.advanceUntilIdle
1416
import kotlinx.coroutines.test.runTest
1517
import mozilla.appservices.places.BookmarkRoot
@@ -50,6 +52,8 @@ class BookmarksMiddlewareTest {
5052
@get:Rule
5153
val coroutineRule = MainCoroutineRule()
5254

55+
private val testDispatcher = StandardTestDispatcher()
56+
5357
private lateinit var bookmarksStorage: BookmarksStorage
5458
private lateinit var clipboardManager: ClipboardManager
5559
private lateinit var addNewTabUseCase: TabsUseCases.AddNewTabUseCase
@@ -2064,6 +2068,7 @@ class BookmarksMiddlewareTest {
20642068
saveBookmarkSortOrder = saveSortOrder,
20652069
lastSavedFolderCache = lastSavedFolderCache,
20662070
reportResultGlobally = reportResultGlobally,
2071+
lifecycleScope = CoroutineScope(testDispatcher),
20672072
)
20682073

20692074
private fun BookmarksMiddleware.makeStore(

0 commit comments

Comments
 (0)