[fenix] Obtain searchTerms from previous page in tab's history

Co-authored-by: Christian Sadilek <christian.sadilek@gmail.com>

app/src/main/java/org/mozilla/fenix/historymetadata/HistoryMetadataMiddleware.kt

@@ -14,6 +14,7 @@ import mozilla.components.browser.state.selector.findTab
 import mozilla.components.browser.state.selector.selectedNormalTab
 import mozilla.components.browser.state.state.BrowserState
 import mozilla.components.browser.state.state.TabSessionState
+import mozilla.components.feature.search.ext.parseSearchTerms
 import mozilla.components.lib.state.Middleware
 import mozilla.components.lib.state.MiddlewareContext
 import mozilla.components.lib.state.Store
@@ -66,12 +67,10 @@ class HistoryMetadataMiddleware(
             is ContentAction.UpdateLoadingStateAction -> {
                 context.state.findNormalTab(action.sessionId)?.let { tab ->
                     val selectedTab = tab.id == context.state.selectedTabId
-                    if (tab.content.loading && !action.loading) {
+                    if (!tab.content.loading && action.loading && selectedTab) {
-                        // When a page stops loading we record its metadata
-                        createHistoryMetadata(context, tab)
-                    } else if (!tab.content.loading && action.loading && selectedTab) {
                         // When a page starts loading (e.g. user navigated away by
-                        // clicking on a link) we update metadata
+                        // clicking on a link) we update metadata for the selected
+                        // (i.e. previous) url of this tab.
                         updateHistoryMetadata(tab)
                     }
                 }
@@ -80,10 +79,25 @@ class HistoryMetadataMiddleware(
 
         next(action)
 
-        // Post process actions
+        // Post process actions. At this point, tab state will be up-to-date and will possess any
+        // changes introduced by the action. These handlers rely on up-to-date tab state, which
+        // is why they're in the "post" section.
         when (action) {
-            // We're handling this after processing the action because we want the tab
+            // NB: sometimes this fires multiple times after the page finished loading.
-            // state to contain the updated media session state.
+            is ContentAction.UpdateHistoryStateAction -> {
+                context.state.findNormalTab(action.sessionId)?.let { tab ->
+                    // When history state is ready, we can record metadata for this page.
+                    val knownHistoryMetadata = tab.historyMetadata
+                    val metadataPresentForUrl = knownHistoryMetadata != null &&
+                            knownHistoryMetadata.url == tab.content.url
+                    // Record metadata for tab if there is no metadata present, or if url of the
+                    // tab changes since we last recorded metadata.
+                    if (!metadataPresentForUrl) {
+                        createHistoryMetadata(context, tab)
+                    }
+                }
+            }
+            // NB: this could be called bunch of times in quick succession.
             is MediaSessionAction.UpdateMediaMetadataAction -> {
                 context.state.findNormalTab(action.tabId)?.let { tab ->
                     createHistoryMetadata(context, tab)
@@ -93,7 +107,22 @@ class HistoryMetadataMiddleware(
     }
 
     private fun createHistoryMetadata(context: MiddlewareContext<BrowserState, BrowserAction>, tab: TabSessionState) {
-        val key = historyMetadataService.createMetadata(tab, tab.getParent(context.store))
+        val tabParent = tab.getParent(context.store)
+        val previousUrlIndex = tab.content.history.currentIndex - 1
+
+        // Obtain search terms and referrer url either from tab parent, or from the history stack.
+        val (searchTerm, referrerUrl) = when {
+            tabParent != null -> {
+                tabParent.content.searchTerms.takeUnless { it.isEmpty() } to tabParent.content.url
+            }
+            previousUrlIndex >= 0 -> {
+                val previousUrl = tab.content.history.items[previousUrlIndex].uri
+                context.state.search.parseSearchTerms(previousUrl) to previousUrl
+            }
+            else -> null to null
+        }
+
+        val key = historyMetadataService.createMetadata(tab, searchTerm, referrerUrl)
         context.dispatch(HistoryMetadataAction.SetHistoryMetadataKeyAction(tab.id, key))
     }
 

app/src/main/java/org/mozilla/fenix/historymetadata/HistoryMetadataService.kt

@@ -23,9 +23,14 @@ interface HistoryMetadataService {
      * Creates a history metadata record for the provided tab.
      *
      * @param tab the [TabSessionState] to record metadata for.
-     * @param parent the parent [TabSessionState] for search and domain grouping purposes.
+     * @param searchTerms Search terms associated with this metadata.
+     * @param referrerUrl Referrer url associated with this metadata.
      */
-    fun createMetadata(tab: TabSessionState, parent: TabSessionState? = null): HistoryMetadataKey
+    fun createMetadata(
+        tab: TabSessionState,
+        searchTerms: String? = null,
+        referrerUrl: String? = null
+    ): HistoryMetadataKey
 
     /**
      * Updates the history metadata corresponding to the provided tab.
@@ -51,14 +56,14 @@ class DefaultHistoryMetadataService(
 
     private val logger = Logger("DefaultHistoryMetadataService")
 
-    override fun createMetadata(tab: TabSessionState, parent: TabSessionState?): HistoryMetadataKey {
+    override fun createMetadata(tab: TabSessionState, searchTerms: String?, referrerUrl: String?): HistoryMetadataKey {
         logger.debug("Creating metadata for tab ${tab.id}")
 
         val existingMetadata = tab.historyMetadata
         val metadataKey = if (existingMetadata != null && existingMetadata.url == tab.content.url) {
             existingMetadata
         } else {
-            tab.toHistoryMetadataKey(parent)
+            tab.toHistoryMetadataKey(searchTerms, referrerUrl)
         }
 
         val documentTypeObservation = HistoryMetadataObservation.DocumentTypeObservation(
@@ -95,9 +100,9 @@ class DefaultHistoryMetadataService(
     }
 }
 
-fun TabSessionState.toHistoryMetadataKey(parent: TabSessionState? = null): HistoryMetadataKey =
+fun TabSessionState.toHistoryMetadataKey(searchTerms: String?, referrerUrl: String?): HistoryMetadataKey =
     HistoryMetadataKey(
         url = content.url,
-        referrerUrl = parent?.content?.url,
+        referrerUrl = referrerUrl,
-        searchTerm = parent?.content?.searchTerms.takeUnless { it.isNullOrEmpty() }
+        searchTerm = searchTerms
     )

app/src/test/java/org/mozilla/fenix/historymetadata/HistoryMetadataMiddlewareTest.kt

@@ -12,20 +12,27 @@ import io.mockk.verify
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import mozilla.components.browser.state.action.ContentAction
 import mozilla.components.browser.state.action.MediaSessionAction
+import mozilla.components.browser.state.action.SearchAction
 import mozilla.components.browser.state.action.TabListAction
 import mozilla.components.browser.state.engine.EngineMiddleware
+import mozilla.components.browser.state.search.SearchEngine
 import mozilla.components.browser.state.selector.findTab
 import mozilla.components.browser.state.state.BrowserState
 import mozilla.components.browser.state.state.TabSessionState
 import mozilla.components.browser.state.state.createTab
 import mozilla.components.browser.state.store.BrowserStore
+import mozilla.components.concept.engine.history.HistoryItem
 import mozilla.components.concept.storage.HistoryMetadataKey
 import mozilla.components.support.test.ext.joinBlocking
+import mozilla.components.support.test.mock
 import org.junit.Assert.assertEquals
 import org.junit.Before
 import org.junit.Test
+import org.junit.runner.RunWith
+import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
 
 @ExperimentalCoroutinesApi
+@RunWith(FenixRobolectricTestRunner::class)
 class HistoryMetadataMiddlewareTest {
 
     private lateinit var store: BrowserStore
@@ -43,22 +50,81 @@ class HistoryMetadataMiddlewareTest {
     }
 
     @Test
-    fun `GIVEN normal tab WHEN loading completed THEN meta data is recorded`() {
+    fun `GIVEN normal tab WHEN history is updated THEN meta data is also recorded`() {
         val tab = createTab("https://mozilla.org")
 
         val expectedKey = HistoryMetadataKey(url = tab.content.url)
         every { service.createMetadata(any(), any()) } returns expectedKey
 
         store.dispatch(TabListAction.AddTabAction(tab)).joinBlocking()
-        store.dispatch(ContentAction.UpdateLoadingStateAction(tab.id, true)).joinBlocking()
         verify { service wasNot Called }
 
-        store.dispatch(ContentAction.UpdateLoadingStateAction(tab.id, false)).joinBlocking()
+        store.dispatch(ContentAction.UpdateHistoryStateAction(tab.id, emptyList(), currentIndex = 0)).joinBlocking()
         val capturedTab = slot<TabSessionState>()
-        verify { service.createMetadata(capture(capturedTab), null) }
+        verify(exactly = 1) { service.createMetadata(capture(capturedTab)) }
+
+        // Not recording if url didn't change.
+        store.dispatch(ContentAction.UpdateHistoryStateAction(tab.id, emptyList(), currentIndex = 0)).joinBlocking()
+        verify(exactly = 1) { service.createMetadata(capture(capturedTab)) }
 
         assertEquals(tab.id, capturedTab.captured.id)
         assertEquals(expectedKey, store.state.findTab(tab.id)?.historyMetadata)
+
+        // Now, test that we'll record metadata for the same tab after url is changed.
+        store.dispatch(ContentAction.UpdateUrlAction(tab.id, "https://firefox.com")).joinBlocking()
+        store.dispatch(ContentAction.UpdateHistoryStateAction(tab.id, emptyList(), currentIndex = 0)).joinBlocking()
+        verify(exactly = 2) { service.createMetadata(capture(capturedTab)) }
+
+        assertEquals(tab.id, capturedTab.captured.id)
+        assertEquals(expectedKey, store.state.findTab(tab.id)?.historyMetadata)
+    }
+
+    @Test
+    fun `GIVEN normal tab has parent WHEN history metadata is recorded THEN search terms and referrer url are provided`() {
+        val parentTab = createTab("https://google.com?q=mozilla+website", searchTerms = "mozilla website")
+        val tab = createTab("https://mozilla.org", parent = parentTab)
+        store.dispatch(TabListAction.AddTabAction(parentTab)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(tab)).joinBlocking()
+
+        store.dispatch(ContentAction.UpdateHistoryStateAction(tab.id, emptyList(), currentIndex = 0)).joinBlocking()
+        verify {
+            service.createMetadata(any(), eq("mozilla website"), eq("https://google.com?q=mozilla+website"))
+        }
+    }
+
+    @Test
+    fun `GIVEN normal tab has no parent WHEN history metadata is recorded THEN search terms and referrer url are provided`() {
+        val tab = createTab("https://mozilla.org")
+        store.dispatch(TabListAction.AddTabAction(tab)).joinBlocking()
+        store.dispatch(SearchAction.SetSearchEnginesAction(
+            regionSearchEngines = listOf(
+                SearchEngine(
+                    id = "google",
+                    name = "Google",
+                    icon = mock(),
+                    type = SearchEngine.Type.BUNDLED,
+                    resultUrls = listOf("https://google.com?q={searchTerms}")
+                )
+            ),
+            userSelectedSearchEngineId = null,
+            userSelectedSearchEngineName = null,
+            regionDefaultSearchEngineId = "google",
+            customSearchEngines = emptyList(),
+            hiddenSearchEngines = emptyList(),
+            additionalAvailableSearchEngines = emptyList(),
+            additionalSearchEngines = emptyList(),
+            regionSearchEnginesOrder = listOf("google")
+        )).joinBlocking()
+
+        val historyState = listOf(
+            HistoryItem("firefox", "https://google.com?q=mozilla+website"),
+            HistoryItem("mozilla", "https://mozilla.org")
+        )
+        store.dispatch(ContentAction.UpdateHistoryStateAction(tab.id, historyState, currentIndex = 1)).joinBlocking()
+
+        verify {
+            service.createMetadata(any(), eq("mozilla website"), eq("https://google.com?q=mozilla+website"))
+        }
     }
 
     @Test
@@ -69,10 +135,9 @@ class HistoryMetadataMiddlewareTest {
         every { service.createMetadata(any(), any()) } returns expectedKey
 
         store.dispatch(TabListAction.AddTabAction(tab)).joinBlocking()
-        store.dispatch(ContentAction.UpdateLoadingStateAction(tab.id, true)).joinBlocking()
         verify { service wasNot Called }
 
-        store.dispatch(ContentAction.UpdateLoadingStateAction(tab.id, false)).joinBlocking()
+        store.dispatch(ContentAction.UpdateHistoryStateAction(tab.id, emptyList(), currentIndex = 0)).joinBlocking()
         verify { service wasNot Called }
     }
 
@@ -130,7 +195,7 @@ class HistoryMetadataMiddlewareTest {
 
         store.dispatch(MediaSessionAction.UpdateMediaMetadataAction(tab.id, mockk())).joinBlocking()
         val capturedTab = slot<TabSessionState>()
-        verify { service.createMetadata(capture(capturedTab), null) }
+        verify { service.createMetadata(capture(capturedTab)) }
 
         assertEquals(tab.id, capturedTab.captured.id)
         assertEquals(expectedKey, store.state.findTab(tab.id)?.historyMetadata)

app/src/test/java/org/mozilla/fenix/historymetadata/HistoryMetadataServiceTest.kt

@@ -42,10 +42,10 @@ class HistoryMetadataServiceTest {
     fun `GIVEN a regular page WHEN metadata is created THEN a regular document type observation is recorded`() {
         val parent = createTab("https://mozilla.org")
         val tab = createTab("https://blog.mozilla.org", parent = parent)
-        service.createMetadata(tab, parent)
+        service.createMetadata(tab, searchTerms = "hello", referrerUrl = parent.content.url)
         testDispatcher.advanceUntilIdle()
 
-        val expectedKey = HistoryMetadataKey(url = tab.content.url, referrerUrl = parent.content.url)
+        val expectedKey = HistoryMetadataKey(url = tab.content.url, searchTerm = "hello", referrerUrl = parent.content.url)
         val expectedObservation = HistoryMetadataObservation.DocumentTypeObservation(documentType = DocumentType.Regular)
         coVerify { storage.noteHistoryMetadataObservation(expectedKey, expectedObservation) }
     }

buildSrc/src/main/java/AndroidComponents.kt

@@ -3,5 +3,5 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 object AndroidComponents {
-    const val VERSION = "91.0.20210617143333"
+    const val VERSION = "91.0.20210617193910"
 }