[fenix] Move tabs out of search group if direct load occurs

This regressed in our previous fix that made sure child tabs don't
mistakenly get moved out of the group if their parent is navigated
away, or in case the child tabs are redirected.

However, when a subsequent load occurs in any tab in the group the
search terms need to be cleared and the tab removed from the group
to prevent false positives.

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

@@ -168,7 +168,7 @@ class HistoryMetadataMiddleware(
             // web content i.e., they followed a link, not if the user navigated directly via
             // toolbar.
             !directLoadTriggered && previousUrlIndex >= 0 -> {
-                // Once a tab is within the search group, only direct navigation event can change that.
+                // Once a tab is within the search group, only a direct load event (via the toolbar) can change that.
                 val (searchTerms, referrerUrl) = if (tabMetadataHasSearchTerms) {
                     tab.historyMetadata?.searchTerm to tab.historyMetadata?.referrerUrl
                 } else {
@@ -184,15 +184,15 @@ class HistoryMetadataMiddleware(
             }
             // In certain redirect cases, we won't have a previous url in the history stack of the tab,
             // but will have the search terms already set on the tab from having gone through this logic
-            // for the redirecting url.
-            // In that case, we leave this tab within the search group it's already in.
-            tabMetadataHasSearchTerms -> {
+            // for the redirecting url. So we leave this tab within the search group it's already in
+            // unless a new direct load (via the toolbar) was triggered.
+            tabMetadataHasSearchTerms && !(directLoadTriggered && previousUrlIndex >= 0) -> {
                 tab.historyMetadata?.searchTerm to tab.historyMetadata?.referrerUrl
             }
             // We had no search terms, no history stack, and no parent.
-            // For example, this would be a search results page itself.
-            // For now, the original search results page is not part of the search group.
-            // See https://github.com/mozilla-mobile/fenix/issues/21659.
+            // This would be the case for any page loaded directly via the toolbar including
+            // a search results page itself. For now, the original search results page is not
+            // part of the search group: https://github.com/mozilla-mobile/fenix/issues/21659.
             else -> null to null
         }
 

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

@@ -149,7 +149,7 @@ class HistoryMetadataMiddlewareTest {
             assertEquals(2, this.count())
         }
 
-        // Parent navigates away. Search terms are reset.
+        // Parent navigates away.
         store.dispatch(ContentAction.UpdateUrlAction(parentTab.id, "https://firefox.com")).joinBlocking()
         store.dispatch(ContentAction.UpdateSearchTermsAction(parentTab.id, "")).joinBlocking()
         store.dispatch(ContentAction.UpdateHistoryStateAction(parentTab.id, listOf(HistoryItem("Google - mozilla website", "https://google.com?q=mozilla+website"), HistoryItem("Firefox", "https://firefox.com")), 1)).joinBlocking()
@@ -186,6 +186,62 @@ class HistoryMetadataMiddlewareTest {
         }
     }
 
+    @Test
+    fun `GIVEN tab with search terms WHEN subsequent direct load occurs THEN search terms are not retained`() {
+        service = TestingMetadataService()
+        middleware = HistoryMetadataMiddleware(service)
+        store = BrowserStore(
+            middleware = listOf(middleware) + EngineMiddleware.create(engine = mockk()),
+            initialState = BrowserState()
+        )
+        setupGoogleSearchEngine()
+
+        val parentTab = createTab("https://google.com?q=mozilla+website", searchTerms = "mozilla website")
+        val tab = createTab("https://google.com?url=https://mozilla.org", parent = parentTab)
+        store.dispatch(TabListAction.AddTabAction(parentTab, select = true)).joinBlocking()
+        store.dispatch(TabListAction.AddTabAction(tab)).joinBlocking()
+
+        with((service as TestingMetadataService).createdMetadata) {
+            assertEquals(2, this.count())
+            assertEquals("https://google.com?q=mozilla+website", this[0].url)
+            assertNull(this[0].searchTerm)
+            assertNull(this[0].referrerUrl)
+
+            assertEquals("https://google.com?url=https://mozilla.org", this[1].url)
+            assertEquals("mozilla website", this[1].searchTerm)
+            assertEquals("https://google.com?q=mozilla+website", this[1].referrerUrl)
+        }
+
+        // Both tabs load.
+        store.dispatch(ContentAction.UpdateHistoryStateAction(parentTab.id, listOf(HistoryItem("Google - mozilla website", "https://google.com?q=mozilla+website")), 0)).joinBlocking()
+        store.dispatch(ContentAction.UpdateHistoryStateAction(tab.id, listOf(HistoryItem("", "https://google.com?url=mozilla+website")), currentIndex = 0)).joinBlocking()
+        with((service as TestingMetadataService).createdMetadata) {
+            assertEquals(2, this.count())
+        }
+
+        // Direct load occurs on child tab. Search terms should be cleared.
+        store.dispatch(EngineAction.LoadUrlAction(tab.id, "https://firefox.com")).joinBlocking()
+        store.dispatch(ContentAction.UpdateUrlAction(tab.id, "https://firefox.com")).joinBlocking()
+        store.dispatch(ContentAction.UpdateHistoryStateAction(tab.id, listOf(HistoryItem("", "https://google.com?url=mozilla+website"), HistoryItem("Firefox", "https://firefox.com")), 1)).joinBlocking()
+        with((service as TestingMetadataService).createdMetadata) {
+            assertEquals(3, this.count())
+            assertEquals("https://firefox.com", this[2].url)
+            assertNull(this[2].searchTerm)
+            assertNull(this[2].referrerUrl)
+        }
+
+        // Direct load occurs on parent tab. Search terms should be cleared.
+        store.dispatch(EngineAction.LoadUrlAction(parentTab.id, "https://firefox.com")).joinBlocking()
+        store.dispatch(ContentAction.UpdateUrlAction(parentTab.id, "https://firefox.com")).joinBlocking()
+        store.dispatch(ContentAction.UpdateHistoryStateAction(parentTab.id, listOf(HistoryItem("Google - mozilla website", "https://google.com?q=mozilla+website"), HistoryItem("Firefox", "https://firefox.com")), 1)).joinBlocking()
+        with((service as TestingMetadataService).createdMetadata) {
+            assertEquals(4, this.count())
+            assertEquals("https://firefox.com", this[3].url)
+            assertNull(this[3].searchTerm)
+            assertNull(this[3].referrerUrl)
+        }
+    }
+
     @Test
     fun `GIVEN normal tab has parent WHEN url is the same THEN nothing happens`() {
         val parentTab = createTab("https://mozilla.org", searchTerms = "mozilla website")