commit 4ddb5c36783a0fd317a3fde8649d07d24bd21d02
parent fa568b5dac7cda3d781bdb479763b448b4067db7
Author: rmalicdem <rmalicdem@mozilla.com>
Date: Tue, 18 Nov 2025 01:49:26 +0000
Bug 1996692 - Optimize bookmark check logic in toolbar customization r=android-reviewers,Roger
Differential Revision: https://phabricator.services.mozilla.com/D272339
Diffstat:
3 files changed, 53 insertions(+), 110 deletions(-)
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/TabPreview.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/browser/TabPreview.kt
@@ -514,13 +514,6 @@ class TabPreview @JvmOverloads constructor(
}
private suspend fun buildComposableToolbarBrowserEndActions(tab: TabSessionState?): List<Action> {
- val isBookmarked = tab?.content?.url?.let { url ->
- context.components.core.bookmarksStorage
- .getBookmarksWithUrl(url)
- .getOrDefault(emptyList())
- .isNotEmpty()
- } ?: false
-
val settings = context.settings()
val isWideWindow = context.isWideWindow()
val isTallWindow = context.isTallWindow()
@@ -529,7 +522,7 @@ class TabPreview @JvmOverloads constructor(
val useCustomPrimary = settings.shouldShowToolbarCustomization
val primarySlotAction = ShortcutType.fromValue(settings.toolbarSimpleShortcutKey)
- ?.toToolbarAction(isBookmarked).takeIf { useCustomPrimary } ?: ToolbarAction.NewTab
+ ?.toToolbarAction(tab).takeIf { useCustomPrimary } ?: ToolbarAction.NewTab
return listOf(
ToolbarActionConfig(primarySlotAction) {
@@ -553,11 +546,6 @@ class TabPreview @JvmOverloads constructor(
}
private suspend fun buildNavigationActions(tab: TabSessionState): List<Action> {
- val isBookmarked = context.components.core.bookmarksStorage
- .getBookmarksWithUrl(tab.content.url)
- .getOrDefault(listOf())
- .isNotEmpty()
-
val settings = context.settings()
val isWideWindow = context.isWideWindow()
val isTallWindow = context.isTallWindow()
@@ -565,7 +553,7 @@ class TabPreview @JvmOverloads constructor(
val useCustomPrimary = settings.shouldShowToolbarCustomization
val primarySlotAction = ShortcutType.fromValue(settings.toolbarExpandedShortcutKey)
- ?.toToolbarAction(isBookmarked).takeIf { useCustomPrimary } ?: getBookmarkAction(isBookmarked)
+ ?.toToolbarAction(tab).takeIf { useCustomPrimary } ?: getBookmarkAction(tab)
return listOf(
ToolbarActionConfig(primarySlotAction) { shouldUseExpandedToolbar && isTallWindow && !isWideWindow },
@@ -610,20 +598,23 @@ class TabPreview @JvmOverloads constructor(
false -> old()
}
- companion object {
- private fun getBookmarkAction(isBookmarked: Boolean): ToolbarAction =
- when (isBookmarked) {
- true -> ToolbarAction.EditBookmark
- false -> ToolbarAction.Bookmark
- }
+ private suspend fun getBookmarkAction(tab: TabSessionState?): ToolbarAction {
+ val isBookmarked = tab?.content?.url?.let { url ->
+ context.components.core.bookmarksStorage
+ .getBookmarksWithUrl(url)
+ .getOrDefault(emptyList())
+ .isNotEmpty()
+ } ?: return ToolbarAction.Bookmark
- private fun ShortcutType.toToolbarAction(isBookmarked: Boolean) = when (this) {
- ShortcutType.NEW_TAB -> ToolbarAction.NewTab
- ShortcutType.SHARE -> ToolbarAction.Share
- ShortcutType.BOOKMARK -> getBookmarkAction(isBookmarked)
- ShortcutType.TRANSLATE -> ToolbarAction.Translate
- ShortcutType.HOMEPAGE -> ToolbarAction.Homepage
- ShortcutType.BACK -> ToolbarAction.Back
- }
+ return if (isBookmarked) ToolbarAction.EditBookmark else ToolbarAction.Bookmark
+ }
+
+ private suspend fun ShortcutType.toToolbarAction(tab: TabSessionState?) = when (this) {
+ ShortcutType.NEW_TAB -> ToolbarAction.NewTab
+ ShortcutType.SHARE -> ToolbarAction.Share
+ ShortcutType.BOOKMARK -> getBookmarkAction(tab)
+ ShortcutType.TRANSLATE -> ToolbarAction.Translate
+ ShortcutType.HOMEPAGE -> ToolbarAction.Homepage
+ ShortcutType.BACK -> ToolbarAction.Back
}
}
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMiddleware.kt
@@ -687,18 +687,9 @@ class BrowserToolbarMiddleware(
)
private suspend fun updateEndBrowserActions(context: MiddlewareContext<BrowserToolbarState, BrowserToolbarAction>) {
- val url = browserStore.state.selectedTab?.content?.url
- val isBookmarked = if (url != null) {
- withContext(ioDispatcher) {
- bookmarksStorage.getBookmarksWithUrl(url).getOrDefault(listOf()).isNotEmpty()
- }
- } else {
- false
- }
-
context.dispatch(
BrowserActionsEndUpdated(
- buildEndBrowserActions(isBookmarked),
+ buildEndBrowserActions(),
),
)
}
@@ -770,14 +761,14 @@ class BrowserToolbarMiddleware(
}
}
- private fun buildEndBrowserActions(isBookmarked: Boolean): List<Action> {
+ private suspend fun buildEndBrowserActions(): List<Action> {
val isWideWindow = environment?.fragment?.isWideWindow() == true
val isTallWindow = environment?.fragment?.isTallWindow() == true
val tabStripEnabled = settings.isTabStripEnabled
val shouldUseExpandedToolbar = settings.shouldUseExpandedToolbar
val useCustomPrimary = settings.shouldShowToolbarCustomization
val primarySlotAction = ShortcutType.fromValue(settings.toolbarSimpleShortcutKey)
- ?.toToolbarAction(isBookmarked).takeIf { useCustomPrimary } ?: ToolbarAction.NewTab
+ ?.toToolbarAction().takeIf { useCustomPrimary } ?: ToolbarAction.NewTab
val configs = listOf(
ToolbarActionConfig(primarySlotAction) {
@@ -811,14 +802,14 @@ class BrowserToolbarMiddleware(
* - The navigation bar is hidden. (even If user enabled it)
* - The toolbar redesign customization option is also hidden.
*/
- private fun buildNavigationActions(isBookmarked: Boolean): List<Action> {
+ private suspend fun buildNavigationActions(): List<Action> {
val environment = environment ?: return emptyList()
val isWideWindow = environment.fragment.isWideWindow()
val isTallWindow = environment.fragment.isTallWindow()
val shouldUseExpandedToolbar = settings.shouldUseExpandedToolbar
val useCustomPrimary = settings.shouldShowToolbarCustomization
val primarySlotAction = ShortcutType.fromValue(settings.toolbarExpandedShortcutKey)
- ?.toToolbarAction(isBookmarked).takeIf { useCustomPrimary } ?: getBookmarkAction(isBookmarked)
+ ?.toToolbarAction().takeIf { useCustomPrimary } ?: getBookmarkAction()
return listOf(
ToolbarActionConfig(primarySlotAction) { shouldUseExpandedToolbar && isTallWindow && !isWideWindow },
@@ -834,18 +825,9 @@ class BrowserToolbarMiddleware(
}
private suspend fun updateNavigationActions(context: MiddlewareContext<BrowserToolbarState, BrowserToolbarAction>) {
- val url = browserStore.state.selectedTab?.content?.url
- val isBookmarked = if (url != null) {
- withContext(ioDispatcher) {
- bookmarksStorage.getBookmarksWithUrl(url).getOrDefault(listOf()).isNotEmpty()
- }
- } else {
- false
- }
-
context.dispatch(
NavigationActionsUpdated(
- buildNavigationActions(isBookmarked),
+ buildNavigationActions(),
),
)
}
@@ -1316,23 +1298,21 @@ class BrowserToolbarMiddleware(
Source.NavigationBar -> MetricsUtils.BookmarkAction.Source.BROWSER_NAVBAR
}
- companion object {
- @VisibleForTesting
- internal fun getBookmarkAction(isBookmarked: Boolean): ToolbarAction =
- when (isBookmarked) {
- true -> ToolbarAction.EditBookmark
- false -> ToolbarAction.Bookmark
- }
-
- @VisibleForTesting
- @JvmStatic
- internal fun ShortcutType.toToolbarAction(isBookmarked: Boolean = false) = when (this) {
- ShortcutType.NEW_TAB -> ToolbarAction.NewTab
- ShortcutType.SHARE -> ToolbarAction.Share
- ShortcutType.BOOKMARK -> getBookmarkAction(isBookmarked)
- ShortcutType.TRANSLATE -> ToolbarAction.Translate
- ShortcutType.HOMEPAGE -> ToolbarAction.Homepage
- ShortcutType.BACK -> ToolbarAction.Back
+ private suspend fun getBookmarkAction(): ToolbarAction {
+ val url = browserStore.state.selectedTab?.content?.url ?: return ToolbarAction.Bookmark
+ val isBookmarked = withContext(ioDispatcher) {
+ bookmarksStorage.getBookmarksWithUrl(url).getOrDefault(emptyList()).isNotEmpty()
}
+ return if (isBookmarked) ToolbarAction.EditBookmark else ToolbarAction.Bookmark
+ }
+
+ @VisibleForTesting
+ internal suspend fun ShortcutType.toToolbarAction() = when (this) {
+ ShortcutType.NEW_TAB -> ToolbarAction.NewTab
+ ShortcutType.SHARE -> ToolbarAction.Share
+ ShortcutType.BOOKMARK -> getBookmarkAction()
+ ShortcutType.TRANSLATE -> ToolbarAction.Translate
+ ShortcutType.HOMEPAGE -> ToolbarAction.Homepage
+ ShortcutType.BACK -> ToolbarAction.Back
}
}
diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMiddlewareTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMiddlewareTest.kt
@@ -142,7 +142,6 @@ import org.mozilla.fenix.components.menu.MenuAccessPoint
import org.mozilla.fenix.components.search.BOOKMARKS_SEARCH_ENGINE_ID
import org.mozilla.fenix.components.search.HISTORY_SEARCH_ENGINE_ID
import org.mozilla.fenix.components.search.TABS_SEARCH_ENGINE_ID
-import org.mozilla.fenix.components.toolbar.BrowserToolbarMiddleware.Companion.toToolbarAction
import org.mozilla.fenix.components.toolbar.BrowserToolbarMiddleware.ToolbarAction
import org.mozilla.fenix.components.toolbar.DisplayActions.AddBookmarkClicked
import org.mozilla.fenix.components.toolbar.DisplayActions.EditBookmarkClicked
@@ -3200,47 +3199,20 @@ class BrowserToolbarMiddlewareTest {
}
@Test
- fun `toToolbarAction maps ShortcutType to ToolbarAction`() {
- assertEquals(
- ToolbarAction.NewTab,
- ShortcutType.NEW_TAB.toToolbarAction(),
- )
- assertEquals(
- ToolbarAction.Share,
- ShortcutType.SHARE.toToolbarAction(),
- )
- assertEquals(
- ToolbarAction.Bookmark,
- ShortcutType.BOOKMARK.toToolbarAction(false),
- )
- assertEquals(
- ToolbarAction.EditBookmark,
- ShortcutType.BOOKMARK.toToolbarAction(true),
- )
- assertEquals(
- ToolbarAction.Translate,
- ShortcutType.TRANSLATE.toToolbarAction(),
- )
- assertEquals(
- ToolbarAction.Homepage,
- ShortcutType.HOMEPAGE.toToolbarAction(),
- )
- assertEquals(
- ToolbarAction.Back,
- ShortcutType.BACK.toToolbarAction(),
- )
- }
+ fun `ShortcutType toToolbarAction maps shortcuts`() = runTest {
+ val middleware = buildMiddleware()
- @Test
- fun `getBookmarkAction returns correct action based on isBookmarked flag`() {
- assertEquals(
- ToolbarAction.Bookmark,
- BrowserToolbarMiddleware.getBookmarkAction(isBookmarked = false),
- )
- assertEquals(
- ToolbarAction.EditBookmark,
- BrowserToolbarMiddleware.getBookmarkAction(isBookmarked = true),
- )
+ val newTab = with(middleware) { ShortcutType.NEW_TAB.toToolbarAction() }
+ val share = with(middleware) { ShortcutType.SHARE.toToolbarAction() }
+ val translate = with(middleware) { ShortcutType.TRANSLATE.toToolbarAction() }
+ val homepage = with(middleware) { ShortcutType.HOMEPAGE.toToolbarAction() }
+ val back = with(middleware) { ShortcutType.BACK.toToolbarAction() }
+
+ assertEquals(ToolbarAction.NewTab, newTab)
+ assertEquals(ToolbarAction.Share, share)
+ assertEquals(ToolbarAction.Translate, translate)
+ assertEquals(ToolbarAction.Homepage, homepage)
+ assertEquals(ToolbarAction.Back, back)
}
private fun assertEqualsTabCounterButton(expected: TabCounterAction, actual: TabCounterAction) {
