Fix flickering search results when pressing cmd-f with findbar showing

This fixes the issue in two ways:
* At a high level, we no longer run a new find when nothing has changed.
* At a lower level, when both find_match and new_session are false, we
no longer end up in an infinite loop of calls to SendFindRequest.

Bug: 1129756
Change-Id: Ic2fdad11a83f882dff3dc272a5bd7da24cab8e3c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2418252
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809126}

chrome/browser/ui/views/find_bar_views_interactive_uitest.cc

@@ -659,6 +659,13 @@ IN_PROC_BROWSER_TEST_F(FindInPageTest, SelectionDuringFind) {
   content::RunUntilInputProcessed(host);
   EXPECT_EQ(find_request_id, helper->current_find_request_id());
 
+  // Make sure calling Show while the findbar is already showing doesn't result
+  // in a find request. It's wasted work, could cause some flicker in the
+  // results, and was previously triggering another bug that caused an endless
+  // loop of searching and flickering results. See http://crbug.com/1129756
+  find_bar_controller->Show(false /*find_next*/);
+  EXPECT_EQ(find_request_id, helper->current_find_request_id());
+
   // Find the next match and verify the correct match is highlighted (the
   // one after text that was selected).
   find_bar_controller->Show(true /*find_next*/);

components/find_in_page/find_tab_helper.cc

@@ -60,14 +60,19 @@ void FindTabHelper::StartFinding(base::string16 search_string,
       return;
   }
 
-  // Keep track of the previous search.
-  previous_find_text_ = find_text_;
-
   // NB: search_string will be empty when using the FindNext keyboard shortcut.
   bool new_session = (find_text_ != search_string && !search_string.empty()) ||
                      (last_search_case_sensitive_ != case_sensitive) ||
                      find_op_aborted_;
 
+  // Continuing here would just find the same results, potentially causing
+  // some flicker in the highlighting.
+  if (!new_session && !find_match)
+    return;
+
+  // Keep track of the previous search.
+  previous_find_text_ = find_text_;
+
   current_find_request_id_ = find_request_id_counter_++;
   if (new_session)
     current_find_session_id_ = current_find_request_id_;

content/browser/find_request_manager.cc

@@ -759,6 +759,7 @@ void FindRequestManager::UpdateActiveMatchOrdinal() {
 void FindRequestManager::FinalUpdateReceived(int request_id,
                                              RenderFrameHost* rfh) {
   if (!number_of_matches_ ||
+      !current_request_.options->find_match ||
       (active_match_ordinal_ && !pending_active_match_ordinal_) ||
       pending_find_next_reply_) {
     // All the find results for |request_id| are in and ready to report. Note

content/browser/find_request_manager_browsertest.cc

@@ -197,6 +197,29 @@ IN_PROC_BROWSER_TEST_P(FindRequestManagerTest, MAYBE(Basic)) {
   }
 }
 
+IN_PROC_BROWSER_TEST_P(FindRequestManagerTest, FindInPage_Issue615291) {
+  LoadAndWait("/find_in_simple_page.html");
+
+  auto options = blink::mojom::FindOptions::New();
+  options->run_synchronously_for_testing = true;
+  options->find_match = false;
+  Find("result", options->Clone());
+  delegate()->WaitForFinalReply();
+
+  FindResults results = delegate()->GetFindResults();
+  EXPECT_EQ(5, results.number_of_matches);
+  EXPECT_EQ(0, results.active_match_ordinal);
+
+  options->new_session = false;
+  Find("result", options->Clone());
+  // With the issue being tested, this would loop forever and cause the
+  // test to timeout.
+  delegate()->WaitForFinalReply();
+  results = delegate()->GetFindResults();
+  EXPECT_EQ(5, results.number_of_matches);
+  EXPECT_EQ(0, results.active_match_ordinal);
+}
+
 bool ExecuteScriptAndExtractRect(FrameTreeNode* frame,
                                  const std::string& script,
                                  gfx::Rect* out) {