Issue mozilla-mobile#21291: SearchDialogFragment: Get URL from clipbo…

…ard once and not for every state update
jonalmeida · Sep 17, 2021 · 6ac10d5 · 6ac10d5
1 parent fc18fd2
commit 6ac10d5
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 7 deletions.
diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt b/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt
@@ -44,6 +44,7 @@ import mozilla.components.concept.storage.HistoryStorage
 import mozilla.components.feature.qr.QrFeature
 import mozilla.components.lib.state.ext.consumeFlow
 import mozilla.components.lib.state.ext.consumeFrom
+import mozilla.components.support.base.coroutines.Dispatchers
 import mozilla.components.support.base.feature.UserInteractionHandler
 import mozilla.components.support.base.feature.ViewBoundFeatureWrapper
 import mozilla.components.support.ktx.android.content.getColorFromAttr
@@ -349,7 +350,7 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler {
             if (it.url != it.query) firstUpdate = false
             binding.awesomeBar.visibility = if (shouldShowAwesomebar(it)) View.VISIBLE else View.INVISIBLE
             updateSearchSuggestionsHintVisibility(it)
-            updateClipboardSuggestion(it, requireContext().components.clipboardHandler.url)
+            updateClipboardSuggestion(it)
             updateToolbarContentDescription(it)
             updateSearchShortcutsIcon(it)
             toolbarView.update(it)
@@ -374,6 +375,22 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler {
         }
     }
 
+    override fun onResume() {
+        super.onResume()
+
+        view?.post {
+            // We delay querying the clipboard by posting this code to the main thread message queue,
+            // because ClipboardManager will return null if the does app not have input focus yet.
+            lifecycleScope.launch(Dispatchers.Cached) {
+                store.dispatch(
+                    SearchFragmentAction.UpdateClipboardUrl(
+                        requireContext().components.clipboardHandler.url
+                    )
+                )
+            }
+        }
+    }
+
     override fun onPause() {
         super.onPause()
         view?.hideKeyboard()
@@ -585,10 +602,10 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler {
 
     private fun isSpeechAvailable(): Boolean = speechIntent.resolveActivity(requireContext().packageManager) != null
 
-    private fun updateClipboardSuggestion(searchState: SearchFragmentState, clipboardUrl: String?) {
+    private fun updateClipboardSuggestion(searchState: SearchFragmentState) {
         val shouldShowView = searchState.showClipboardSuggestions &&
             searchState.query.isEmpty() &&
-            !clipboardUrl.isNullOrEmpty() && !searchState.showSearchShortcuts
+            !searchState.clipboardUrl.isNullOrEmpty() && !searchState.showSearchShortcuts
 
         binding.fillLinkFromClipboard.isVisible = shouldShowView
         binding.fillLinkDivider.isVisible = shouldShowView
@@ -598,13 +615,13 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler {
         binding.clipboardTitle.isVisible = shouldShowView
         binding.linkIcon.isVisible = shouldShowView
 
-        binding.clipboardUrl.text = clipboardUrl
+        binding.clipboardUrl.text = searchState.clipboardUrl
 
         binding.fillLinkFromClipboard.contentDescription =
             "${binding.clipboardTitle.text}, ${binding.clipboardUrl.text}."
 
-        if (clipboardUrl != null && !((activity as HomeActivity).browsingModeManager.mode.isPrivate)) {
-            requireComponents.core.engine.speculativeConnect(clipboardUrl)
+        if (searchState.clipboardUrl != null && !((activity as HomeActivity).browsingModeManager.mode.isPrivate)) {
+            requireComponents.core.engine.speculativeConnect(searchState.clipboardUrl)
         }
     }
 

diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt b/app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt
@@ -61,6 +61,7 @@ sealed class SearchEngineSource {
  * @property showHistorySuggestions Whether or not to show history suggestions in the AwesomeBar
  * @property showBookmarkSuggestions Whether or not to show the bookmark suggestion in the AwesomeBar
  * @property pastedText The text pasted from the long press toolbar menu
+ * @property clipboardUrl The URL in the clipboard of the user - if there's any; otherwise `null`.
  */
 data class SearchFragmentState(
     val query: String,
@@ -79,7 +80,8 @@ data class SearchFragmentState(
     val showSyncedTabsSuggestions: Boolean,
     val tabId: String?,
     val pastedText: String? = null,
-    val searchAccessPoint: Event.PerformedSearch.SearchAccessPoint?
+    val searchAccessPoint: Event.PerformedSearch.SearchAccessPoint?,
+    val clipboardUrl: String? = null
 ) : State
 
 fun createInitialSearchFragmentState(
@@ -129,6 +131,7 @@ sealed class SearchFragmentAction : Action {
     data class ShowSearchShortcutEnginePicker(val show: Boolean) : SearchFragmentAction()
     data class AllowSearchSuggestionsInPrivateModePrompt(val show: Boolean) : SearchFragmentAction()
     data class UpdateQuery(val query: String) : SearchFragmentAction()
+    data class UpdateClipboardUrl(val url: String?) : SearchFragmentAction()
 
     /**
      * Updates the local `SearchFragmentState` from the global `SearchState` in `BrowserStore`.
@@ -169,5 +172,10 @@ private fun searchStateReducer(state: SearchFragmentState, action: SearchFragmen
                 }
             )
         }
+        is SearchFragmentAction.UpdateClipboardUrl -> {
+            state.copy(
+                clipboardUrl = action.url
+            )
+        }
     }
 }
diff --git a/app/src/test/java/org/mozilla/fenix/search/SearchFragmentStoreTest.kt b/app/src/test/java/org/mozilla/fenix/search/SearchFragmentStoreTest.kt
@@ -16,6 +16,7 @@ import mozilla.components.browser.state.state.BrowserState
 import mozilla.components.browser.state.state.ContentState
 import mozilla.components.browser.state.state.SearchState
 import mozilla.components.browser.state.state.TabSessionState
+import mozilla.components.support.test.ext.joinBlocking
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertNotNull
@@ -199,6 +200,23 @@ class SearchFragmentStoreTest {
         assertFalse(store.state.showSearchSuggestionsHint)
     }
 
+    @Test
+    fun updatingClipboardUrl() {
+        val initialState = emptyDefaultState()
+        val store = SearchFragmentStore(initialState)
+
+        assertNull(store.state.clipboardUrl)
+
+        store.dispatch(
+            SearchFragmentAction.UpdateClipboardUrl("https://www.mozilla.org")
+        ).joinBlocking()
+
+        assertEquals(
+            "https://www.mozilla.org",
+            store.state.clipboardUrl
+        )
+    }
+
     @Test
     fun `Updating SearchFragmentState from SearchState`() = runBlocking {
         val store = SearchFragmentStore(