Fix cycling through find results when typing in findbar

Bug: 1142027
Change-Id: Ie2fb4d4631f3d8507b404d6641f89170e35d98e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2496290
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821279}

chrome/browser/ui/find_bar/find_bar_platform_helper_mac.mm

@@ -47,11 +47,16 @@ class FindBarPlatformHelperMac : public FindBarPlatformHelper {
       return;
     }
 
-    [[FindPasteboard sharedInstance]
-        setFindText:base::SysUTF16ToNSString(text)];
+    {
+      base::AutoReset<bool> resetter(&sending_own_notification_, true);
+      [[FindPasteboard sharedInstance]
+          setFindText:base::SysUTF16ToNSString(text)];
+    }
   }
 
  private:
+  bool sending_own_notification_ = false;
+
   void UpdateFindBarControllerFromPasteboard() {
     content::WebContents* active_web_contents =
         find_bar_controller_->web_contents();
@@ -73,8 +78,10 @@ class FindBarPlatformHelperMac : public FindBarPlatformHelper {
       }
     }
 
-    NSString* find_text = [[FindPasteboard sharedInstance] findText];
-    find_bar_controller_->SetText(base::SysNSStringToUTF16(find_text));
+    if (!sending_own_notification_) {
+      NSString* find_text = [[FindPasteboard sharedInstance] findText];
+      find_bar_controller_->SetText(base::SysNSStringToUTF16(find_text));
+    }
   }
 
   id find_pasteboard_notification_observer_;

chrome/browser/ui/views/find_bar_views_interactive_uitest.cc

@@ -121,24 +121,6 @@ class FindInPageTest : public InProcessBrowserTest {
     }
   }
 
-  bool SendKeyPressAndWait(const Browser* browser,
-                           ui::KeyboardCode key,
-                           bool control,
-                           bool shift,
-                           bool alt,
-                           bool command,
-                           int type,
-                           const content::NotificationSource& source) {
-    content::WindowedNotificationObserver observer(type, source);
-
-    if (!ui_test_utils::SendKeyPressSync(browser, key, control, shift, alt,
-                                         command))
-      return false;
-
-    observer.Wait();
-    return !testing::Test::HasFatalFailure();
-  }
-
  private:
   DISALLOW_COPY_AND_ASSIGN(FindInPageTest);
 };
@@ -771,3 +753,29 @@ IN_PROC_BROWSER_TEST_F(FindInPageTest, GlobalEscapeClosesFind) {
   // Find should be closed
   ASSERT_FALSE(IsFindBarVisible());
 }
+
+// See http://crbug.com/1142027
+IN_PROC_BROWSER_TEST_F(FindInPageTest, MatchOrdinalStableWhileTyping) {
+  // 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(), GURL("data:text/html,foo foo foo"));
+  auto* web_contents = browser()->tab_strip_model()->GetActiveWebContents();
+
+  browser()->GetFindBarController()->Show();
+  EXPECT_TRUE(IsViewFocused(browser(), VIEW_ID_FIND_IN_PAGE_TEXT_FIELD));
+
+  ui_test_utils::FindResultWaiter waiter1(web_contents, 1 /*request_offset*/);
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+      browser(), ui::VKEY_F, false, false, false, false));
+  waiter1.Wait();
+  EXPECT_EQ(1, waiter1.active_match_ordinal());
+  EXPECT_EQ(3, waiter1.number_of_matches());
+
+  ui_test_utils::FindResultWaiter waiter2(web_contents, 1 /*request_offset*/);
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+      browser(), ui::VKEY_O, false, false, false, false));
+  waiter2.Wait();
+  EXPECT_EQ(1, waiter2.active_match_ordinal());
+  EXPECT_EQ(3, waiter2.number_of_matches());
+}

chrome/test/base/find_result_waiter.cc

@@ -10,10 +10,12 @@
 
 namespace ui_test_utils {
 
-FindResultWaiter::FindResultWaiter(content::WebContents* parent_tab) {
+FindResultWaiter::FindResultWaiter(content::WebContents* parent_tab,
+                                   int request_offset) {
   find_in_page::FindTabHelper* find_tab_helper =
       find_in_page::FindTabHelper::FromWebContents(parent_tab);
-  current_find_request_id_ = find_tab_helper->current_find_request_id();
+  current_find_request_id_ = find_tab_helper->current_find_request_id() +
+      request_offset;
   observer_.Add(find_tab_helper);
 }
 

chrome/test/base/find_result_waiter.h

@@ -28,12 +28,16 @@ namespace ui_test_utils {
 //   FindInPageWchar();
 //   FindResultWaiter observer(tab);
 //   observer.Wait();
-//
-// Always construct FindResultWaiter AFTER initiating the search. It captures
-// the current search ID in the constructor and waits for it only.
 class FindResultWaiter : public find_in_page::FindResultObserver {
  public:
-  explicit FindResultWaiter(content::WebContents* parent_tab);
+  // |request_offset| will be added to the current find request ID; the
+  // resulting ID will be the only one waited on. Typically, FindResultWaiter
+  // is constructed AFTER initiating a search, with request_offset set to 0. In
+  // such cases you must be sure the find-result callback won't already have
+  // been called. Otherwise, you can construct FindResultWaiter BEFORE starting
+  // a search and set request_offset to 1 (or whatever offset is appropriate).
+  explicit FindResultWaiter(content::WebContents* parent_tab,
+                            int request_offset = 0);
   FindResultWaiter(const FindResultWaiter&) = delete;
   FindResultWaiter& operator=(const FindResultWaiter&) = delete;
   ~FindResultWaiter() override;