[fenix] Closes https://github.com/mozilla-mobile/fenix/issues/18253: Bookmark and History open new tabs in the background (https://github.com/mozilla-mobile/fenix/pull/19275)

app/src/androidTest/java/org/mozilla/fenix/ui/BookmarksTest.kt

@@ -223,8 +223,7 @@ class BookmarksTest {
             IdlingRegistry.getInstance().register(bookmarksListIdlingResource!!)
         }.openThreeDotMenu(defaultWebPage.url) {
         }.clickOpenInNewTab {
-            verifyUrl(defaultWebPage.url.toString())
+            verifyTabTrayIsOpened()
-        }.openTabDrawer {
             verifyNormalModeSelected()
         }
     }
@@ -242,8 +241,7 @@ class BookmarksTest {
             IdlingRegistry.getInstance().register(bookmarksListIdlingResource!!)
         }.openThreeDotMenu(defaultWebPage.url) {
         }.clickOpenInPrivateTab {
-            verifyUrl(defaultWebPage.url.toString())
+            verifyTabTrayIsOpened()
-        }.openTabDrawer {
             verifyPrivateModeSelected()
         }
     }

app/src/androidTest/java/org/mozilla/fenix/ui/HistoryTest.kt

@@ -149,8 +149,7 @@ class HistoryTest {
             IdlingRegistry.getInstance().register(historyListIdlingResource!!)
         }.openThreeDotMenu {
         }.clickOpenInNormalTab {
-            verifyUrl(firstWebPage.url.toString())
+            verifyTabTrayIsOpened()
-        }.openTabDrawer {
             verifyNormalModeSelected()
         }
     }
@@ -170,8 +169,7 @@ class HistoryTest {
             IdlingRegistry.getInstance().register(historyListIdlingResource!!)
         }.openThreeDotMenu {
         }.clickOpenInPrivateTab {
-            verifyUrl(firstWebPage.url.toString())
+            verifyTabTrayIsOpened()
-        }.openTabDrawer {
             verifyPrivateModeSelected()
         }
     }

app/src/androidTest/java/org/mozilla/fenix/ui/robots/TabDrawerRobot.kt

@@ -80,6 +80,7 @@ class TabDrawerRobot {
     fun verifySelectTabsButton() = assertSelectTabsButton()
     fun verifyTabTrayOverflowMenu(visibility: Boolean) = assertTabTrayOverflowButton(visibility)
 
+    fun verifyTabTrayIsOpened() = assertTabTrayDoesExist()
     fun verifyTabTrayIsClosed() = assertTabTrayDoesNotExist()
     fun verifyHalfExpandedRatio() = assertMinisculeHalfExpandedRatio()
     fun verifyBehaviorState(expectedState: Int) = assertBehaviorState(expectedState)
@@ -421,6 +422,11 @@ private fun assertTabTrayOverflowButton(visible: Boolean) =
     onView(withId(R.id.tab_tray_overflow))
         .check(matches(withEffectiveVisibility(visibleOrGone(visible))))
 
+private fun assertTabTrayDoesExist() {
+    onView(withId(R.id.tab_wrapper))
+        .check(matches(withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE)))
+}
+
 private fun assertTabTrayDoesNotExist() {
     onView(withId(R.id.tab_wrapper))
         .check(doesNotExist())

app/src/androidTest/java/org/mozilla/fenix/ui/robots/ThreeDotMenuBookmarksRobot.kt

@@ -38,18 +38,18 @@ class ThreeDotMenuBookmarksRobot {
             return BookmarksRobot.Transition()
         }
 
-        fun clickOpenInNewTab(interact: BrowserRobot.() -> Unit): BrowserRobot.Transition {
+        fun clickOpenInNewTab(interact: TabDrawerRobot.() -> Unit): TabDrawerRobot.Transition {
             openInNewTabButton().click()
 
-            BrowserRobot().interact()
+            TabDrawerRobot().interact()
-            return BrowserRobot.Transition()
+            return TabDrawerRobot.Transition()
         }
 
-        fun clickOpenInPrivateTab(interact: BrowserRobot.() -> Unit): BrowserRobot.Transition {
+        fun clickOpenInPrivateTab(interact: TabDrawerRobot.() -> Unit): TabDrawerRobot.Transition {
             openInPrivateTabButton().click()
 
-            BrowserRobot().interact()
+            TabDrawerRobot().interact()
-            return BrowserRobot.Transition()
+            return TabDrawerRobot.Transition()
         }
 
         fun clickDelete(interact: BookmarksRobot.() -> Unit): BookmarksRobot.Transition {

app/src/androidTest/java/org/mozilla/fenix/ui/robots/ThreeDotMenuHistoryItemRobot.kt

@@ -41,16 +41,16 @@ class ThreeDotMenuHistoryItemRobot {
             return LibrarySubMenusMultipleSelectionToolbarRobot.Transition()
         }
 
-        fun clickOpenInNormalTab(interact: BrowserRobot.() -> Unit): BrowserRobot.Transition {
+        fun clickOpenInNormalTab(interact: TabDrawerRobot.() -> Unit): TabDrawerRobot.Transition {
             openInNewNormalTabButton().click()
-            BrowserRobot().interact()
+            TabDrawerRobot().interact()
-            return BrowserRobot.Transition()
+            return TabDrawerRobot.Transition()
         }
 
-        fun clickOpenInPrivateTab(interact: BrowserRobot.() -> Unit): BrowserRobot.Transition {
+        fun clickOpenInPrivateTab(interact: TabDrawerRobot.() -> Unit): TabDrawerRobot.Transition {
             openInNewPrivateTabButton().click()
-            BrowserRobot().interact()
+            TabDrawerRobot().interact()
-            return BrowserRobot.Transition()
+            return TabDrawerRobot.Transition()
         }
 
         fun clickDelete(interact: HistoryRobot.() -> Unit): HistoryRobot.Transition {

app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt

@@ -14,6 +14,7 @@ import kotlinx.coroutines.launch
 import mozilla.appservices.places.BookmarkRoot
 import mozilla.components.concept.engine.prompt.ShareData
 import mozilla.components.concept.storage.BookmarkNode
+import mozilla.components.feature.tabs.TabsUseCases
 import mozilla.components.service.fxa.sync.SyncReason
 import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.HomeActivity
@@ -55,11 +56,13 @@ class DefaultBookmarkController(
     private val scope: CoroutineScope,
     private val store: BookmarkFragmentStore,
     private val sharedViewModel: BookmarksSharedViewModel,
+    private val tabsUseCases: TabsUseCases?,
     private val loadBookmarkNode: suspend (String) -> BookmarkNode?,
     private val showSnackbar: (String) -> Unit,
     private val deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit,
     private val deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit,
-    private val invokePendingDeletion: () -> Unit
+    private val invokePendingDeletion: () -> Unit,
+    private val showTabTray: () -> Unit
 ) : BookmarkController {
 
     private val resources: Resources = activity.resources
@@ -70,7 +73,7 @@ class DefaultBookmarkController(
     }
 
     override fun handleBookmarkTapped(item: BookmarkNode) {
-        openInNewTab(item.url!!, true, BrowserDirection.FromBookmarks, activity.browsingModeManager.mode)
+        openInNewTabAndShow(item.url!!, true, BrowserDirection.FromBookmarks, activity.browsingModeManager.mode)
     }
 
     override fun handleBookmarkExpand(folder: BookmarkNode) {
@@ -126,7 +129,8 @@ class DefaultBookmarkController(
     }
 
     override fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode) {
-        openInNewTab(item.url!!, true, BrowserDirection.FromBookmarks, mode)
+        openInNewTab(item.url!!, mode)
+        showTabTray()
     }
 
     override fun handleBookmarkDeletion(nodes: Set<BookmarkNode>, eventType: Event) {
@@ -169,7 +173,7 @@ class DefaultBookmarkController(
         }
     }
 
-    private fun openInNewTab(
+    private fun openInNewTabAndShow(
         searchTermOrURL: String,
         newTab: Boolean,
         from: BrowserDirection,
@@ -182,6 +186,18 @@ class DefaultBookmarkController(
         }
     }
 
+    private fun openInNewTab(
+        url: String,
+        mode: BrowsingMode
+    ) {
+        invokePendingDeletion.invoke()
+        activity.browsingModeManager.mode = BrowsingMode.fromBoolean(mode == BrowsingMode.Private)
+        tabsUseCases?.let { tabsUseCases ->
+            val addTab = if (mode == BrowsingMode.Private) tabsUseCases.addPrivateTab else tabsUseCases.addTab
+            addTab.invoke(url)
+        }
+    }
+
     private fun navigateToGivenDirection(directions: NavDirections) {
         invokePendingDeletion.invoke()
         navController.nav(R.id.bookmarkFragment, directions)

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

@@ -92,11 +92,13 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
                 scope = viewLifecycleOwner.lifecycleScope,
                 store = bookmarkStore,
                 sharedViewModel = sharedViewModel,
+                tabsUseCases = activity?.components?.useCases?.tabsUseCases,
                 loadBookmarkNode = ::loadBookmarkNode,
                 showSnackbar = ::showSnackBarWithText,
                 deleteBookmarkNodes = ::deleteMulti,
                 deleteBookmarkFolder = ::showRemoveFolderDialog,
-                invokePendingDeletion = ::invokePendingDeletion
+                invokePendingDeletion = ::invokePendingDeletion,
+                showTabTray = ::showTabTray
             ),
             metrics = metrics!!
         )
@@ -243,7 +245,7 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
 
     private fun showTabTray() {
         invokePendingDeletion()
-        navigateToBookmarkFragment(BookmarkFragmentDirections.actionGlobalTabTrayDialogFragment())
+        navigateToBookmarkFragment(BookmarkFragmentDirections.actionGlobalTabsTrayFragment())
     }
 
     private fun navigateToBookmarkFragment(directions: NavDirections) {

app/src/main/java/org/mozilla/fenix/library/history/HistoryController.kt

@@ -21,7 +21,8 @@ import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph
 
 @Suppress("TooManyFunctions")
 interface HistoryController {
-    fun handleOpen(item: HistoryItem, mode: BrowsingMode? = null)
+    fun handleOpen(item: HistoryItem)
+    fun handleOpenInNewTab(item: HistoryItem, mode: BrowsingMode)
     fun handleSelect(item: HistoryItem)
     fun handleDeselect(item: HistoryItem)
     fun handleBackPressed(): Boolean
@@ -42,15 +43,20 @@ class DefaultHistoryController(
     private val snackbar: FenixSnackbar,
     private val clipboardManager: ClipboardManager,
     private val scope: CoroutineScope,
-    private val openToBrowser: (item: HistoryItem, mode: BrowsingMode?) -> Unit,
+    private val openToBrowser: (item: HistoryItem) -> Unit,
+    private val openInNewTab: (item: HistoryItem, mode: BrowsingMode) -> Unit,
     private val displayDeleteAll: () -> Unit,
     private val invalidateOptionsMenu: () -> Unit,
     private val deleteHistoryItems: (Set<HistoryItem>) -> Unit,
     private val syncHistory: suspend () -> Unit,
     private val metrics: MetricController
 ) : HistoryController {
-    override fun handleOpen(item: HistoryItem, mode: BrowsingMode?) {
+    override fun handleOpen(item: HistoryItem) {
-        openToBrowser(item, mode)
+        openToBrowser(item)
+    }
+
+    override fun handleOpenInNewTab(item: HistoryItem, mode: BrowsingMode) {
+        openInNewTab(item, mode)
     }
 
     override fun handleSelect(item: HistoryItem) {

app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt

@@ -80,22 +80,23 @@ class HistoryFragment : LibraryPageFragment<HistoryItem>(), UserInteractionHandl
             )
         }
         val historyController: HistoryController = DefaultHistoryController(
-            historyStore,
+            store = historyStore,
-            findNavController(),
+            navController = findNavController(),
-            resources,
+            resources = resources,
-            FenixSnackbar.make(
+            snackbar = FenixSnackbar.make(
                 view = view,
                 duration = FenixSnackbar.LENGTH_LONG,
                 isDisplayedWithBrowserToolbar = false
             ),
-            activity?.getSystemService(CLIPBOARD_SERVICE) as ClipboardManager,
+            clipboardManager = activity?.getSystemService(CLIPBOARD_SERVICE) as ClipboardManager,
-            lifecycleScope,
+            scope = lifecycleScope,
-            ::openItem,
+            openToBrowser = ::openItem,
-            ::displayDeleteAllDialog,
+            openInNewTab = ::openItemAndShowTray,
-            ::invalidateOptionsMenu,
+            displayDeleteAll = ::displayDeleteAllDialog,
-            ::deleteHistoryItems,
+            invalidateOptionsMenu = ::invalidateOptionsMenu,
-            ::syncHistory,
+            deleteHistoryItems = ::deleteHistoryItems,
-            requireComponents.analytics.metrics
+            syncHistory = ::syncHistory,
+            metrics = requireComponents.analytics.metrics
         )
         historyInteractor = HistoryInteractor(
             historyController
@@ -216,7 +217,7 @@ class HistoryFragment : LibraryPageFragment<HistoryItem>(), UserInteractionHandl
         invokePendingDeletion()
         findNavController().nav(
             R.id.historyFragment,
-            HistoryFragmentDirections.actionGlobalTabTrayDialogFragment()
+            HistoryFragmentDirections.actionGlobalTabsTrayFragment()
         )
     }
 
@@ -247,14 +248,8 @@ class HistoryFragment : LibraryPageFragment<HistoryItem>(), UserInteractionHandl
         _historyView = null
     }
 
-    private fun openItem(item: HistoryItem, mode: BrowsingMode? = null) {
+    private fun openItem(item: HistoryItem) {
-        when (mode?.isPrivate) {
+        requireComponents.analytics.metrics.track(Event.HistoryItemOpened)
-            true -> requireComponents.analytics.metrics.track(Event.HistoryOpenedInPrivateTab)
-            false -> requireComponents.analytics.metrics.track(Event.HistoryOpenedInNewTab)
-            null -> requireComponents.analytics.metrics.track(Event.HistoryItemOpened)
-        }
-
-        mode?.let { (activity as HomeActivity).browsingModeManager.mode = it }
 
         (activity as HomeActivity).openToBrowserAndLoad(
             searchTermOrURL = item.url,
@@ -263,6 +258,22 @@ class HistoryFragment : LibraryPageFragment<HistoryItem>(), UserInteractionHandl
         )
     }
 
+    private fun openItemAndShowTray(item: HistoryItem, mode: BrowsingMode) {
+        when (mode.isPrivate) {
+            true -> requireComponents.analytics.metrics.track(Event.HistoryOpenedInPrivateTab)
+            false -> requireComponents.analytics.metrics.track(Event.HistoryOpenedInNewTab)
+        }
+
+        val homeActivity = activity as HomeActivity
+        homeActivity.browsingModeManager.mode = mode
+        homeActivity.components.useCases.tabsUseCases.let { tabsUseCases ->
+            val addTab = if (mode == BrowsingMode.Private) tabsUseCases.addPrivateTab else tabsUseCases.addTab
+            addTab.invoke(item.url)
+        }
+
+        showTabTray()
+    }
+
     private fun displayDeleteAllDialog() {
         activity?.let { activity ->
             AlertDialog.Builder(activity).apply {

app/src/main/java/org/mozilla/fenix/library/history/HistoryInteractor.kt

@@ -43,11 +43,11 @@ class HistoryInteractor(
     }
 
     override fun onOpenInNormalTab(item: HistoryItem) {
-        historyController.handleOpen(item, BrowsingMode.Normal)
+        historyController.handleOpenInNewTab(item, BrowsingMode.Normal)
     }
 
     override fun onOpenInPrivateTab(item: HistoryItem) {
-        historyController.handleOpen(item, BrowsingMode.Private)
+        historyController.handleOpenInNewTab(item, BrowsingMode.Private)
     }
 
     override fun onDeleteAll() {

app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt

@@ -27,6 +27,7 @@ import kotlinx.coroutines.test.TestCoroutineScope
 import mozilla.appservices.places.BookmarkRoot
 import mozilla.components.concept.storage.BookmarkNode
 import mozilla.components.concept.storage.BookmarkNodeType
+import mozilla.components.feature.tabs.TabsUseCases
 import org.junit.After
 import org.junit.Assert.assertEquals
 import org.junit.Before
@@ -54,14 +55,18 @@ class BookmarkControllerTest {
     private val clipboardManager: ClipboardManager = mockk(relaxUnitFun = true)
     private val navController: NavController = mockk(relaxed = true)
     private val sharedViewModel: BookmarksSharedViewModel = mockk()
+    private val tabsUseCases: TabsUseCases = mockk()
     private val loadBookmarkNode: suspend (String) -> BookmarkNode? = mockk(relaxed = true)
     private val showSnackbar: (String) -> Unit = mockk(relaxed = true)
     private val deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit = mockk(relaxed = true)
     private val deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit = mockk(relaxed = true)
     private val invokePendingDeletion: () -> Unit = mockk(relaxed = true)
+    private val showTabTray: () -> Unit = mockk(relaxed = true)
 
     private val homeActivity: HomeActivity = mockk(relaxed = true)
     private val services: Services = mockk(relaxed = true)
+    private val addNewTabUseCase: TabsUseCases.AddNewTabUseCase = mockk(relaxed = true)
+    private val addNewPrivateTabUseCase: TabsUseCases.AddNewPrivateTabUseCase = mockk(relaxed = true)
 
     private val item =
         BookmarkNode(BookmarkNodeType.ITEM, "456", "123", 0, "Mozilla", "http://mozilla.org", null)
@@ -100,6 +105,8 @@ class BookmarkControllerTest {
         }
         every { bookmarkStore.dispatch(any()) } returns mockk()
         every { sharedViewModel.selectedFolder = any() } just runs
+        every { tabsUseCases.addTab } returns addNewTabUseCase
+        every { tabsUseCases.addPrivateTab } returns addNewPrivateTabUseCase
 
         controller = DefaultBookmarkController(
             activity = homeActivity,
@@ -108,11 +115,13 @@ class BookmarkControllerTest {
             scope = scope,
             store = bookmarkStore,
             sharedViewModel = sharedViewModel,
+            tabsUseCases = tabsUseCases,
             loadBookmarkNode = loadBookmarkNode,
             showSnackbar = showSnackbar,
             deleteBookmarkNodes = deleteBookmarkNodes,
             deleteBookmarkFolder = deleteBookmarkFolder,
-            invokePendingDeletion = invokePendingDeletion
+            invokePendingDeletion = invokePendingDeletion,
+            showTabTray = showTabTray
         )
     }
 
@@ -267,6 +276,16 @@ class BookmarkControllerTest {
         }
     }
 
+    @Test
+    fun `handleBookmarkTapped should open the bookmark`() {
+        controller.handleBookmarkTapped(item)
+
+        verifyOrder {
+            invokePendingDeletion.invoke()
+            homeActivity.openToBrowserAndLoad(item.url!!, true, BrowserDirection.FromBookmarks)
+        }
+    }
+
     @Test
     fun `handleOpeningBookmark should open the bookmark a new 'Normal' tab`() {
         controller.handleOpeningBookmark(item, BrowsingMode.Normal)
@@ -274,7 +293,9 @@ class BookmarkControllerTest {
         verifyOrder {
             invokePendingDeletion.invoke()
             homeActivity.browsingModeManager.mode = BrowsingMode.Normal
-            homeActivity.openToBrowserAndLoad(item.url!!, true, BrowserDirection.FromBookmarks)
+            tabsUseCases.addTab
+            addNewTabUseCase.invoke(item.url!!)
+            showTabTray
         }
     }
 
@@ -285,7 +306,9 @@ class BookmarkControllerTest {
         verifyOrder {
             invokePendingDeletion.invoke()
             homeActivity.browsingModeManager.mode = BrowsingMode.Private
-            homeActivity.openToBrowserAndLoad(item.url!!, true, BrowserDirection.FromBookmarks)
+            tabsUseCases.addPrivateTab
+            addNewPrivateTabUseCase.invoke(item.url!!)
+            showTabTray
         }
     }
 

app/src/test/java/org/mozilla/fenix/library/history/HistoryControllerTest.kt

@@ -41,7 +41,8 @@ class HistoryControllerTest {
     private val resources: Resources = mockk(relaxed = true)
     private val snackbar: FenixSnackbar = mockk(relaxed = true)
     private val clipboardManager: ClipboardManager = mockk(relaxed = true)
-    private val openInBrowser: (HistoryItem, BrowsingMode?) -> Unit = mockk(relaxed = true)
+    private val openInBrowser: (HistoryItem) -> Unit = mockk(relaxed = true)
+    private val openAndShowTray: (HistoryItem, BrowsingMode) -> Unit = mockk(relaxed = true)
     private val displayDeleteAll: () -> Unit = mockk(relaxed = true)
     private val invalidateOptionsMenu: () -> Unit = mockk(relaxed = true)
     private val deleteHistoryItems: (Set<HistoryItem>) -> Unit = mockk(relaxed = true)
@@ -55,6 +56,7 @@ class HistoryControllerTest {
         clipboardManager,
         scope,
         openInBrowser,
+        openAndShowTray,
         displayDeleteAll,
         invalidateOptionsMenu,
         deleteHistoryItems,
@@ -77,25 +79,25 @@ class HistoryControllerTest {
         controller.handleOpen(historyItem)
 
         verify {
-            openInBrowser(historyItem, null)
+            openInBrowser(historyItem)
         }
     }
 
     @Test
     fun onOpenItemInNormalMode() {
-        controller.handleOpen(historyItem, BrowsingMode.Normal)
+        controller.handleOpenInNewTab(historyItem, BrowsingMode.Normal)
 
         verify {
-            openInBrowser(historyItem, BrowsingMode.Normal)
+            openAndShowTray(historyItem, BrowsingMode.Normal)
         }
     }
 
     @Test
     fun onOpenItemInPrivateMode() {
-        controller.handleOpen(historyItem, BrowsingMode.Private)
+        controller.handleOpenInNewTab(historyItem, BrowsingMode.Private)
 
         verify {
-            openInBrowser(historyItem, BrowsingMode.Private)
+            openAndShowTray(historyItem, BrowsingMode.Private)
         }
     }
 

app/src/test/java/org/mozilla/fenix/library/history/HistoryInteractorTest.kt

@@ -89,7 +89,7 @@ class HistoryInteractorTest {
         interactor.onOpenInNormalTab(historyItem)
 
         verifyAll {
-            controller.handleOpen(historyItem, BrowsingMode.Normal)
+            controller.handleOpenInNewTab(historyItem, BrowsingMode.Normal)
         }
     }
 
@@ -98,7 +98,7 @@ class HistoryInteractorTest {
         interactor.onOpenInPrivateTab(historyItem)
 
         verifyAll {
-            controller.handleOpen(historyItem, BrowsingMode.Private)
+            controller.handleOpenInNewTab(historyItem, BrowsingMode.Private)
         }
     }