From de14962e3f181d69a9638a8fc0a4ab01d1fc1689 Mon Sep 17 00:00:00 2001 From: Tiger Oakes Date: Mon, 19 Aug 2019 11:28:33 -0400 Subject: [PATCH] Extract quick action sheet observer code (#4368) --- .../fenix/browser/BaseBrowserFragment.kt | 1 - .../mozilla/fenix/browser/BrowserFragment.kt | 81 +++++-------------- .../QuickActionSheetSessionObserver.kt | 68 ++++++++++++++++ .../QuickActionSheetSessionObserverTest.kt | 72 +++++++++++++++++ 4 files changed, 158 insertions(+), 64 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/quickactionsheet/QuickActionSheetSessionObserver.kt create mode 100644 app/src/test/java/org/mozilla/fenix/quickactionsheet/QuickActionSheetSessionObserverTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt index 663f41107..720a1ff75 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -365,7 +365,6 @@ abstract class BaseBrowserFragment : Fragment(), BackHandler, SessionManager.Obs @CallSuper override fun onSessionSelected(session: Session) { - super.onSessionSelected(session) if (!browserInitialized) { // Initializing a new coroutineScope to avoid ConcurrentModificationException in ObserverRegistry // This will be removed when ObserverRegistry is deprecated by browser-state. diff --git a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt index e957b7d63..31ef0f788 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt @@ -22,7 +22,6 @@ import kotlinx.android.synthetic.main.fragment_browser.view.* import kotlinx.coroutines.Dispatchers.IO import kotlinx.coroutines.Dispatchers.Main import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.Job import kotlinx.coroutines.ObsoleteCoroutinesApi import kotlinx.coroutines.launch import kotlinx.coroutines.withContext @@ -49,9 +48,8 @@ import org.mozilla.fenix.home.sessioncontrol.SessionControlChange import org.mozilla.fenix.home.sessioncontrol.TabCollection import org.mozilla.fenix.mvi.getManagedEmitter import org.mozilla.fenix.quickactionsheet.DefaultQuickActionSheetController +import org.mozilla.fenix.quickactionsheet.QuickActionSheetSessionObserver import org.mozilla.fenix.quickactionsheet.QuickActionSheetView -import java.net.MalformedURLException -import java.net.URL /** * Fragment used for browsing the web within the main app and external apps. @@ -61,11 +59,11 @@ import java.net.URL @Suppress("TooManyFunctions") class BrowserFragment : BaseBrowserFragment(), BackHandler { private lateinit var quickActionSheetView: QuickActionSheetView + private var quickActionSheetSessionObserver: QuickActionSheetSessionObserver? = null private val readerViewFeature = ViewBoundFeatureWrapper() private val thumbnailsFeature = ViewBoundFeatureWrapper() private val customTabsIntegration = ViewBoundFeatureWrapper() - private var findBookmarkJob: Job? = null override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) @@ -93,7 +91,7 @@ class BrowserFragment : BaseBrowserFragment(), BackHandler { override fun initializeUI(view: View): Session? { val sessionManager = requireComponents.core.sessionManager - return super.initializeUI(view)?.also { session -> + return super.initializeUI(view)?.also { quickActionSheetView = QuickActionSheetView(view.nestedScrollQuickAction, browserInteractor) @@ -154,8 +152,6 @@ class BrowserFragment : BaseBrowserFragment(), BackHandler { themeReaderViewControlsForPrivateMode(view.readerViewControlsBar) } - updateBookmarkState(session) - consumeFrom(browserStore) { quickActionSheetView.update(it) browserToolbarView.update(it) @@ -165,8 +161,14 @@ class BrowserFragment : BaseBrowserFragment(), BackHandler { override fun onStart() { super.onStart() - subscribeToSession() subscribeToTabCollections() + quickActionSheetSessionObserver = QuickActionSheetSessionObserver( + lifecycleScope, + requireComponents, + dispatch = { action -> browserStore.dispatch(action) } + ).also { observer -> + getSessionById()?.register(observer, this, autoPause = true) + } } override fun onResume() { @@ -306,56 +308,9 @@ class BrowserFragment : BaseBrowserFragment(), BackHandler { }) } - private fun subscribeToSession() { - val observer = object : Session.Observer { - override fun onLoadingStateChanged(session: Session, loading: Boolean) { - if (!loading) { - updateBookmarkState(session) - browserStore.dispatch(QuickActionSheetAction.BounceNeededChange) - } - } - - override fun onUrlChanged(session: Session, url: String) { - updateBookmarkState(session) - updateAppLinksState(session) - } - } - getSessionById()?.register(observer, this, autoPause = true) - } - override fun onSessionSelected(session: Session) { super.onSessionSelected(session) - updateBookmarkState(session) - } - - private suspend fun findBookmarkedURL(session: Session?): Boolean { - return withContext(IO) { - session?.let { - try { - val url = URL(it.url).toString() - val list = requireComponents.core.bookmarksStorage.getBookmarksWithUrl(url) - list.isNotEmpty() && list[0].url == url - } catch (e: MalformedURLException) { - false - } - } ?: false - } - } - - private fun updateBookmarkState(session: Session) { - findBookmarkJob?.cancel() - findBookmarkJob = lifecycleScope.launch(IO) { - val found = findBookmarkedURL(session) - withContext(Main) { - browserStore.dispatch(QuickActionSheetAction.BookmarkedStateChange(found)) - } - } - } - - private fun updateAppLinksState(session: Session) { - val url = session.url - val appLinks = requireComponents.useCases.appLinksUseCases.appLinkRedirect - browserStore.dispatch(QuickActionSheetAction.AppLinkStateChange(appLinks.invoke(url).hasExternalApp())) + quickActionSheetSessionObserver?.updateBookmarkState(session) } private val collectionStorageObserver = object : TabCollectionStorage.Observer { @@ -366,14 +321,14 @@ class BrowserFragment : BaseBrowserFragment(), BackHandler { override fun onTabsAdded(tabCollection: TabCollection, sessions: List) { showTabSavedToCollectionSnackbar() } - } - private fun showTabSavedToCollectionSnackbar() { - view?.let { view -> - FenixSnackbar.make(view, Snackbar.LENGTH_SHORT) - .setText(view.context.getString(R.string.create_collection_tab_saved)) - .setAnchorView(browserToolbarView.view) - .show() + private fun showTabSavedToCollectionSnackbar() { + view?.let { view -> + FenixSnackbar.make(view, Snackbar.LENGTH_SHORT) + .setText(view.context.getString(R.string.create_collection_tab_saved)) + .setAnchorView(browserToolbarView.view) + .show() + } } } diff --git a/app/src/main/java/org/mozilla/fenix/quickactionsheet/QuickActionSheetSessionObserver.kt b/app/src/main/java/org/mozilla/fenix/quickactionsheet/QuickActionSheetSessionObserver.kt new file mode 100644 index 000000000..e1818cc15 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/quickactionsheet/QuickActionSheetSessionObserver.kt @@ -0,0 +1,68 @@ +/* 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.quickactionsheet + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers.IO +import kotlinx.coroutines.Dispatchers.Main +import kotlinx.coroutines.Job +import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext +import mozilla.components.browser.session.Session +import org.mozilla.fenix.components.Components +import org.mozilla.fenix.components.toolbar.QuickActionSheetAction +import java.net.MalformedURLException +import java.net.URL + +class QuickActionSheetSessionObserver( + private val parentScope: CoroutineScope, + private val components: Components, + private val dispatch: (QuickActionSheetAction) -> Unit +) : Session.Observer { + + private var findBookmarkJob: Job? = null + + override fun onLoadingStateChanged(session: Session, loading: Boolean) { + if (!loading) { + updateBookmarkState(session) + dispatch(QuickActionSheetAction.BounceNeededChange) + } + } + + override fun onUrlChanged(session: Session, url: String) { + updateBookmarkState(session) + updateAppLinksState(session) + } + + /** + * Launches job to update the bookmark button on the quick action sheet. + */ + fun updateBookmarkState(session: Session) { + findBookmarkJob?.cancel() + findBookmarkJob = parentScope.launch(Main) { + val found = findBookmarkedURL(session) + dispatch(QuickActionSheetAction.BookmarkedStateChange(found)) + } + } + + /** + * Updates the app link button on the quick action sheet. + */ + private fun updateAppLinksState(session: Session) { + val url = session.url + val appLinks = components.useCases.appLinksUseCases.appLinkRedirect + dispatch(QuickActionSheetAction.AppLinkStateChange(appLinks(url).hasExternalApp())) + } + + private suspend fun findBookmarkedURL(session: Session): Boolean = withContext(IO) { + try { + val url = URL(session.url).toString() + val list = components.core.bookmarksStorage.getBookmarksWithUrl(url) + list.isNotEmpty() && list[0].url == url + } catch (e: MalformedURLException) { + false + } + } +} diff --git a/app/src/test/java/org/mozilla/fenix/quickactionsheet/QuickActionSheetSessionObserverTest.kt b/app/src/test/java/org/mozilla/fenix/quickactionsheet/QuickActionSheetSessionObserverTest.kt new file mode 100644 index 000000000..78776ea0c --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/quickactionsheet/QuickActionSheetSessionObserverTest.kt @@ -0,0 +1,72 @@ +/* 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.quickactionsheet + +import io.mockk.Runs +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.spyk +import io.mockk.verify +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.ObsoleteCoroutinesApi +import mozilla.components.browser.session.Session +import mozilla.components.feature.app.links.AppLinkRedirect +import mozilla.components.feature.app.links.AppLinksUseCases +import org.junit.Before +import org.junit.Test +import org.mozilla.fenix.components.Components +import org.mozilla.fenix.components.toolbar.BrowserStore +import org.mozilla.fenix.components.toolbar.QuickActionSheetAction + +@ExperimentalCoroutinesApi +@ObsoleteCoroutinesApi +class QuickActionSheetSessionObserverTest { + + private lateinit var components: Components + private lateinit var appLinkRedirect: AppLinksUseCases.GetAppLinkRedirect + private lateinit var store: BrowserStore + private lateinit var dispatch: (QuickActionSheetAction) -> Unit + + @Before + fun setup() { + components = mockk(relaxed = true) + appLinkRedirect = mockk(relaxed = true) + store = mockk(relaxed = true) + dispatch = { store.dispatch(it) } + + every { components.useCases.appLinksUseCases.appLinkRedirect } returns appLinkRedirect + } + + @Test + fun `onLoadingStateChanged dispatches BounceNeededChange and updates bookmark button`() { + val session: Session = mockk() + val observer = spyk(QuickActionSheetSessionObserver(mockk(), components, dispatch)) + every { observer.updateBookmarkState(session) } just Runs + + observer.onLoadingStateChanged(session, true) + verify(exactly = 0) { store.dispatch(QuickActionSheetAction.BounceNeededChange) } + + observer.onLoadingStateChanged(session, false) + verify { observer.updateBookmarkState(session) } + verify { store.dispatch(QuickActionSheetAction.BounceNeededChange) } + } + + @Test + fun `onUrlChanged updates bookmark and app link buttons`() { + val url = "https://example.com" + val session: Session = mockk() + every { session.url } returns url + + val observer = spyk(QuickActionSheetSessionObserver(mockk(), components, dispatch)) + every { observer.updateBookmarkState(session) } just Runs + every { appLinkRedirect.invoke(url) } returns AppLinkRedirect(mockk(), "", false) + + observer.onUrlChanged(session, "") + verify { observer.updateBookmarkState(session) } + verify { appLinkRedirect.invoke("https://example.com") } + verify { store.dispatch(QuickActionSheetAction.AppLinkStateChange(true)) } + } +}