Fix audible alert when opening findbar with prepopulated text

Bug: 1131780
Change-Id: I3556016852284a7c7c2c64c22787747dfb420ba0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2438634Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Eric Lawrence [MSFT] <ericlaw@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#812419}

chrome/browser/ui/find_bar/find_bar_controller.cc

@@ -193,8 +193,11 @@ void FindBarController::OnFindResultAvailable(
   find_in_page::FindTabHelper* find_tab_helper =
       find_in_page::FindTabHelper::FromWebContents(web_contents_);
 
-  // Only "final" results may audibly alert the user.
-  if (!find_tab_helper->find_result().final_update())
+  // Only "final" results may audibly alert the user. Also don't alert when
+  // we're only highlighting results (when first opening the find bar).
+  // See https://crbug.com/1131780
+  if (!find_tab_helper->find_result().final_update() ||
+      !find_tab_helper->should_find_match())
     return;
 
   const base::string16& current_search = find_tab_helper->find_text();
@@ -203,9 +206,8 @@ void FindBarController::OnFindResultAvailable(
   // convention). Alert only once per unique search, and don't alert on
   // backspace.
   if ((find_tab_helper->find_result().number_of_matches() == 0) &&
-      (current_search != find_tab_helper->last_completed_find_text() &&
-       !base::StartsWith(find_tab_helper->previous_find_text(), current_search,
-                         base::CompareCase::SENSITIVE))) {
+      !base::StartsWith(find_tab_helper->last_completed_find_text(),
+                        current_search, base::CompareCase::SENSITIVE)) {
     find_bar_->AudibleAlert();
   }
 

chrome/browser/ui/find_bar/find_bar_host_browsertest.cc

@@ -365,6 +365,40 @@ IN_PROC_BROWSER_TEST_F(FindInPageControllerTest, NoAudibleAlertOnNavigation) {
   EXPECT_EQ(0u, GetFindBarAudibleAlertsForBrowser(browser()));
 }
 
+// See http://crbug.com/1131780
+IN_PROC_BROWSER_TEST_F(FindInPageControllerTest,
+                       AudibleAlertsWithPrepopulatedFind) {
+  ui_test_utils::NavigateToURL(browser(), GetURL(kSimple));
+
+  WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  EXPECT_EQ(0u, GetFindBarAudibleAlertsForBrowser(browser()));
+
+  // Do an initial find so the text will be prepopulated for the next one.
+  // This will produce an audible alert.
+  EXPECT_EQ(
+      0, FindInPageASCII(web_contents, "zzz", kFwd, kIgnoreCase, nullptr));
+  EXPECT_EQ(1u, GetFindBarAudibleAlertsForBrowser(browser()));
+  browser()->GetFindBarController()->EndFindSession(
+      find_in_page::SelectionAction::kKeep, find_in_page::ResultAction::kKeep);
+
+  // Now show the findbar (prepopulated) and ensure there's no alert.
+  browser()->GetFindBarController()->Show(false /*find_next*/);
+  EXPECT_EQ(ASCIIToUTF16("zzz"), GetFindBarText());
+  ui_test_utils::FindResultWaiter observer1(web_contents);
+  observer1.Wait();
+  EXPECT_EQ(0, observer1.number_of_matches());
+  EXPECT_EQ(1u, GetFindBarAudibleAlertsForBrowser(browser()));
+
+  // Now do a find-next and ensure there *is* an alert
+  browser()->GetFindBarController()->Show(true /*find_next*/);
+  EXPECT_EQ(ASCIIToUTF16("zzz"), GetFindBarText());
+  ui_test_utils::FindResultWaiter observer2(web_contents);
+  observer2.Wait();
+  EXPECT_EQ(0, observer2.number_of_matches());
+  EXPECT_EQ(2u, GetFindBarAudibleAlertsForBrowser(browser()));
+}
+
 // Verify search selection coordinates. The data file used is set-up such that
 // the text occurs on the same line, and we verify their positions by verifying
 // their relative positions.

components/find_in_page/find_tab_helper.cc

@@ -82,6 +82,7 @@ void FindTabHelper::StartFinding(base::string16 search_string,
   last_search_case_sensitive_ = case_sensitive;
 
   find_op_aborted_ = false;
+  should_find_match_ = find_match;
 
   // Keep track of what the last search was across the tabs.
   if (delegate_)
@@ -111,6 +112,7 @@ void FindTabHelper::StopFinding(SelectionAction selection_action) {
   last_completed_find_text_.clear();
   find_op_aborted_ = true;
   last_search_result_ = FindNotificationDetails();
+  should_find_match_ = false;
 
   content::StopFindAction action;
   switch (selection_action) {

components/find_in_page/find_tab_helper.h

@@ -81,7 +81,8 @@ class FindTabHelper : public content::WebContentsUserData<FindTabHelper> {
   // Accessor for the previous search we issued.
   base::string16 previous_find_text() const { return previous_find_text_; }
 
-  // Accessor for the latest search for which a final result was reported.
+  // Accessor for the last completed search (i.e., where |find_match| was true
+  // and we got a final_update result).
   base::string16 last_completed_find_text() const {
     return last_completed_find_text_;
   }
@@ -101,6 +102,8 @@ class FindTabHelper : public content::WebContentsUserData<FindTabHelper> {
     return last_search_result_;
   }
 
+  bool should_find_match() const { return should_find_match_; }
+
 #if defined(OS_ANDROID)
   // Selects and zooms to the find result nearest to the point (x,y)
   // defined in find-in-page coordinates.
@@ -156,7 +159,8 @@ class FindTabHelper : public content::WebContentsUserData<FindTabHelper> {
   // The string we searched for before |find_text_|.
   base::string16 previous_find_text_;
 
-  // Used to keep track the last completed search. A single find session can
+  // Used to keep track the last completed search (i.e., where |find_match|
+  // was true and we got a final_update result). A single find session can
   // result in multiple final updates, if the document contents change
   // dynamically. It's a nuisance to notify the user more than once that a
   // search came up empty, and we never want to notify the user that a
@@ -175,6 +179,10 @@ class FindTabHelper : public content::WebContentsUserData<FindTabHelper> {
   // information to build its presentation.
   FindNotificationDetails last_search_result_;
 
+  // The value of the |find_match| option for the active search, or false if
+  // there is no active search.
+  bool should_find_match_ = false;
+
   // The optional delegate that remembers recent search text state.
   Delegate* delegate_ = nullptr;