Fix bugs when opening findbar in new page after a find with no matches

See the new tests for exact repro steps. These issues were caused by
preexisting internal state that wasn't getting properly reset but which
only caused problems in combination with the new `find_match = false`
mode.

Change-Id: Ic4adb54cd748223fac48af6c9eac2ae169c6d09b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2448680Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814384}

chrome/browser/ui/find_bar/find_bar_host_browsertest.cc

@@ -427,6 +427,30 @@ IN_PROC_BROWSER_TEST_F(FindInPageControllerTest, FindInPageSpecialURLs) {
   ASSERT_EQ(first, first_reverse);
 }
 
+// This tests the following bug that used to exist:
+// 1) Do a find that has 0 results
+// 2) Navigate to a new page (on the same domain) that contains the search text.
+// 3) Open the find bar. It will be prepopulated with the previous search text
+// and should show the number of matches for that text. The bug caused it to
+// show 0 matches instead.
+IN_PROC_BROWSER_TEST_F(FindInPageControllerTest, StaleCountAfterNoResults) {
+  WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  ui_test_utils::NavigateToURL(browser(), GetURL("simple.html"));
+  EXPECT_EQ(0, FindInPageASCII(web_contents, "link", kFwd, kIgnoreCase,
+                               nullptr));
+  browser()->GetFindBarController()->EndFindSession(
+      find_in_page::SelectionAction::kKeep, find_in_page::ResultAction::kKeep);
+
+  ui_test_utils::NavigateToURL(browser(), GetURL("link.html"));
+  browser()->GetFindBarController()->Show();
+  ui_test_utils::FindResultWaiter observer(web_contents);
+  observer.Wait();
+  EXPECT_EQ(1, observer.number_of_matches());
+  EXPECT_EQ(0, observer.active_match_ordinal());
+}
+
 // Verifies that comments and meta data are not searchable.
 IN_PROC_BROWSER_TEST_F(FindInPageControllerTest,
                        CommentsAndMetaDataNotSearchable) {

chrome/browser/ui/views/find_bar_views_interactive_uitest.cc

@@ -614,6 +614,52 @@ IN_PROC_BROWSER_TEST_F(FindInPageTest, MAYBE_CtrlEnter) {
   observer.Wait();
 }
 
+// This tests the following bug that used to exist:
+// 1) Do a find that has 0 results. The search text must contain a space.
+// 2) Navigate to a new page (on the same domain) that contains the search text.
+// 3) Open the find bar. It should display 0/N (where N is the number of
+// matches) and have no active-match highlighting. The bug caused it to display
+// 1/N, with the first having active-match highlighting (and the page wouldn't
+// scroll to the match if it was off-screen).
+IN_PROC_BROWSER_TEST_F(FindInPageTest, ActiveMatchAfterNoResults) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  // Make sure Chrome is in the foreground, otherwise sending input
+  // won't do anything and the test will hang.
+  ASSERT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
+
+  ui_test_utils::NavigateToURL(
+      browser(), embedded_test_server()->GetURL("/find_in_page/simple.html"));
+
+  // This bug does not reproduce when using ui_test_utils::FindInPage here;
+  // sending keystrokes like this is required. Also note that the text must
+  // contain a space.
+  browser()->GetFindBarController()->Show();
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+      browser(), ui::VKEY_A, false, false, false, false));
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+      browser(), ui::VKEY_SPACE, false, false, false, false));
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+      browser(), ui::VKEY_L, false, false, false, false));
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+      browser(), ui::VKEY_I, false, false, false, false));
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+      browser(), ui::VKEY_N, false, false, false, false));
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+      browser(), ui::VKEY_K, false, false, false, false));
+  EXPECT_EQ(ASCIIToUTF16("a link"), GetFindBarText());
+
+  browser()->GetFindBarController()->EndFindSession(
+      find_in_page::SelectionAction::kKeep, find_in_page::ResultAction::kKeep);
+
+  ui_test_utils::NavigateToURL(
+      browser(), embedded_test_server()->GetURL("/find_in_page/link.html"));
+
+  browser()->GetFindBarController()->Show();
+  auto details = WaitForFindResult();
+  EXPECT_EQ(1, details.number_of_matches());
+  EXPECT_EQ(0, details.active_match_ordinal());
+}
+
 IN_PROC_BROWSER_TEST_F(FindInPageTest, SelectionDuringFind) {
   ASSERT_TRUE(embedded_test_server()->Start());
   // Make sure Chrome is in the foreground, otherwise sending input

third_party/blink/renderer/core/editing/finder/text_finder.cc

@@ -126,6 +126,17 @@ static void ScrollToVisible(Range* match) {
       const_cast<Node*>(&first_node));
 }
 
+void TextFinder::InitNewSession(const mojom::blink::FindOptions& options) {
+  should_locate_active_rect_ = false;
+  CancelPendingScopingEffort();
+  if (!options.find_match) {
+    // This gets called in FindInternal if a match is found, but FindInternal
+    // doesn't run when find_match is false, so we need to do it here in case
+    // there is a match (to get the scoping effort to look for it).
+    find_task_controller_->ResetLastFindRequestCompletedWithNoMatches();
+  }
+}
+
 bool TextFinder::Find(int identifier,
                       const WebString& search_text,
                       const mojom::blink::FindOptions& options,

third_party/blink/renderer/core/editing/finder/text_finder.h

@@ -56,6 +56,7 @@ class CORE_EXPORT TextFinder final : public GarbageCollected<TextFinder> {
             const mojom::blink::FindOptions& options,
             bool wrap_within_frame,
             bool* active_now = nullptr);
+  void InitNewSession(const mojom::blink::FindOptions&  options);
   void ClearActiveFindMatch();
   void SetFindEndstateFocusAndSelection();
   void StopFindingAndClearSelection();

third_party/blink/renderer/core/frame/find_in_page.cc

@@ -95,11 +95,8 @@ void FindInPage::Find(int request_id,
   bool result = false;
   bool active_now = false;
 
-  if (options->new_session) {
-    // If this is an initial find request, cancel any pending scoping effort
-    // done by the previous find request.
-    EnsureTextFinder().CancelPendingScopingEffort();
-  }
+  if (options->new_session)
+    EnsureTextFinder().InitNewSession(*options);
 
   // Search for an active match only if this frame is focused or if this is an
   // existing session.