Bug 1840721 - We should prefer switching to open tabs vs opening a duplicate tab

app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt

@@ -371,6 +371,8 @@ class HomeFragment : Fragment() {
                 activity = activity,
                 navController = findNavController(),
                 appStore = components.appStore,
+                browserStore = components.core.store,
+                selectTabUseCase = components.useCases.tabsUseCases.selectTab,
             ),
             recentVisitsController = DefaultRecentVisitsController(
                 navController = findNavController(),

app/src/main/java/org/mozilla/fenix/home/recentbookmarks/controller/RecentBookmarksController.kt

@@ -6,11 +6,14 @@ package org.mozilla.fenix.home.recentbookmarks.controller
 
 import androidx.navigation.NavController
 import mozilla.appservices.places.BookmarkRoot
+import mozilla.components.browser.state.store.BrowserStore
 import mozilla.components.concept.engine.EngineSession
 import mozilla.components.concept.engine.EngineSession.LoadUrlFlags.Companion.ALLOW_JAVASCRIPT_URL
+import mozilla.components.feature.tabs.TabsUseCases
 import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.GleanMetrics.RecentBookmarks
 import org.mozilla.fenix.HomeActivity
+import org.mozilla.fenix.R
 import org.mozilla.fenix.components.AppStore
 import org.mozilla.fenix.components.appstate.AppAction
 import org.mozilla.fenix.home.HomeFragmentDirections
@@ -46,15 +49,27 @@ class DefaultRecentBookmarksController(
     private val activity: HomeActivity,
     private val navController: NavController,
     private val appStore: AppStore,
+    private val browserStore: BrowserStore,
+    private val selectTabUseCase: TabsUseCases.SelectTabUseCase,
 ) : RecentBookmarksController {
 
     override fun handleBookmarkClicked(bookmark: RecentBookmark) {
-        activity.openToBrowserAndLoad(
+        val bookmarkTab = browserStore.state.tabs.firstOrNull {
-            searchTermOrURL = bookmark.url!!,
+            it.content.url == bookmark.url
-            newTab = true,
+        }
-            from = BrowserDirection.FromHome,
+
-            flags = EngineSession.LoadUrlFlags.select(ALLOW_JAVASCRIPT_URL),
+        if (bookmarkTab != null) {
-        )
+            selectTabUseCase.invoke(bookmarkTab.id)
+            navController.navigate(R.id.browserFragment)
+        } else {
+            activity.openToBrowserAndLoad(
+                searchTermOrURL = bookmark.url!!,
+                newTab = true,
+                from = BrowserDirection.FromHome,
+                flags = EngineSession.LoadUrlFlags.select(ALLOW_JAVASCRIPT_URL),
+            )
+        }
+
         RecentBookmarks.bookmarkClicked.add()
     }
 

app/src/test/java/org/mozilla/fenix/home/recentbookmarks/DefaultRecentBookmarksControllerTest.kt

@@ -11,10 +11,12 @@ import io.mockk.just
 import io.mockk.mockk
 import io.mockk.spyk
 import io.mockk.verify
-import kotlinx.coroutines.ExperimentalCoroutinesApi
 import mozilla.appservices.places.BookmarkRoot
+import mozilla.components.browser.state.state.createTab
+import mozilla.components.browser.state.store.BrowserStore
 import mozilla.components.concept.engine.EngineSession
 import mozilla.components.concept.engine.EngineSession.LoadUrlFlags.Companion.ALLOW_JAVASCRIPT_URL
+import mozilla.components.feature.tabs.TabsUseCases
 import mozilla.components.support.test.robolectric.testContext
 import mozilla.components.support.test.rule.MainCoroutineRule
 import mozilla.components.support.test.rule.runTestOnMain
@@ -28,11 +30,11 @@ import org.junit.runner.RunWith
 import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.GleanMetrics.RecentBookmarks
 import org.mozilla.fenix.HomeActivity
+import org.mozilla.fenix.R
 import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
 import org.mozilla.fenix.home.HomeFragmentDirections
 import org.mozilla.fenix.home.recentbookmarks.controller.DefaultRecentBookmarksController
 
-@OptIn(ExperimentalCoroutinesApi::class)
 @RunWith(FenixRobolectricTestRunner::class)
 class DefaultRecentBookmarksControllerTest {
 
@@ -44,31 +46,53 @@ class DefaultRecentBookmarksControllerTest {
 
     private val activity: HomeActivity = mockk(relaxed = true)
     private val navController: NavController = mockk(relaxUnitFun = true)
+    private val selectTabUseCase: TabsUseCases = mockk(relaxed = true)
+    private val browserStore: BrowserStore = mockk(relaxed = true)
 
     private lateinit var controller: DefaultRecentBookmarksController
 
     @Before
     fun setup() {
         every { activity.openToBrowserAndLoad(any(), any(), any()) } just Runs
+        every { browserStore.state.tabs }.returns(emptyList())
 
         controller = spyk(
             DefaultRecentBookmarksController(
                 activity = activity,
                 navController = navController,
                 appStore = mockk(),
+                browserStore = browserStore,
+                selectTabUseCase = selectTabUseCase.selectTab,
             ),
         )
     }
 
     @Test
-    fun `WHEN a recently saved bookmark is clicked THEN the selected bookmark is opened`() {
+    fun `GIVEN no tabs WHEN a recently saved bookmark is clicked THEN the selected bookmark is opened in a new tab`() {
         assertNull(RecentBookmarks.bookmarkClicked.testGetValue())
 
-        val bookmark = RecentBookmark(
+        val bookmark = RecentBookmark(title = null, url = "https://www.example.com")
-            title = null,
+        controller.handleBookmarkClicked(bookmark)
-            url = "https://www.example.com",
+
-        )
+        verify {
+            activity.openToBrowserAndLoad(
+                searchTermOrURL = bookmark.url!!,
+                newTab = true,
+                flags = EngineSession.LoadUrlFlags.select(ALLOW_JAVASCRIPT_URL),
+                from = BrowserDirection.FromHome,
+            )
+        }
+        assertNotNull(RecentBookmarks.bookmarkClicked.testGetValue())
+    }
+
+    @Test
+    fun `GIVEN no matching tabs WHEN a recently saved bookmark is clicked THEN the selected bookmark is opened in a new tab`() {
+        assertNull(RecentBookmarks.bookmarkClicked.testGetValue())
 
+        val testTab = createTab("https://www.not_example.com")
+        every { browserStore.state.tabs }.returns(listOf(testTab))
+
+        val bookmark = RecentBookmark(title = null, url = "https://www.example.com")
         controller.handleBookmarkClicked(bookmark)
 
         verify {
@@ -82,6 +106,24 @@ class DefaultRecentBookmarksControllerTest {
         assertNotNull(RecentBookmarks.bookmarkClicked.testGetValue())
     }
 
+    @Test
+    fun `GIVEN matching tab WHEN a recently saved bookmark is clicked THEN the existing tab is opened`() {
+        assertNull(RecentBookmarks.bookmarkClicked.testGetValue())
+
+        val testUrl = "https://www.example.com"
+        val testTab = createTab(testUrl)
+        every { browserStore.state.tabs }.returns(listOf(testTab))
+
+        val bookmark = RecentBookmark(title = null, url = testUrl)
+        controller.handleBookmarkClicked(bookmark)
+
+        verify {
+            selectTabUseCase.invoke(testTab.id)
+            navController.navigate(R.id.browserFragment)
+        }
+        assertNotNull(RecentBookmarks.bookmarkClicked.testGetValue())
+    }
+
     @Test
     fun `WHEN show all recently saved bookmark is clicked THEN the bookmarks root is opened`() = runTestOnMain {
         assertNull(RecentBookmarks.showAllBookmarks.testGetValue())