Fix issue 5079: Incorrect "Active match ordinal" count during Find-in-page

I introduced a regression in my reimplemenation of Find-in-page. The active match ordinal in Find-in-page (also known as "the 7" in "7 of 9") would be just a little off on pages with frames. 

Problem A: When you search for something in gmail, for example, the ordinal could start off slightly negative or be 0. I wasn't checking the last_match_count_ of a frame for negative numbers before adding it to the total (it starts off as -1 and remains that way if the frame is not deemed to be worthy of being scoped, i.e. if it is hidden).

Problem B: On pages with multiple matches spread across multiple frames the ordinal would not be subtracted correctly after pressing F3 and Shift-F3 to go back to the frame you were on. We shouldn't be increasing/decreasing the active_match_index for a given frame when FindNext/FindPrevious causes us to jump between frames. We should instead reset it.

I added two tests to catch this in the future. They test ordinal values as you use Find in page (including combinations of frames/no-frames & FindNext/FindPrevious).

Oh, and I also removed some traces that were supposed to expose why a test was flaky, but it turns out to have been something unrelated to the test.
Review URL: http://codereview.chromium.org/13130

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@6369 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 144 insertions, 62 deletions

diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
index b51eb54..146883b 100644
--- a/chrome/browser/automation/automation_provider.cc
+++ b/chrome/browser/automation/automation_provider.cc
@@ -410,7 +410,8 @@ class FindInPageNotificationObserver : public NotificationObserver {
                                  int32 routing_id)
       : automation_(automation),
         parent_tab_(parent_tab),
-        routing_id_(routing_id) {
+        routing_id_(routing_id),
+        active_match_ordinal_(-1) {
     NotificationService::current()->
         AddObserver(this, NOTIFY_FIND_RESULT_AVAILABLE,
                     Source<TabContents>(parent_tab_));
@@ -431,8 +432,13 @@ class FindInPageNotificationObserver : public NotificationObserver {
     if (type == NOTIFY_FIND_RESULT_AVAILABLE) {
       Details<FindNotificationDetails> find_details(details);
       if (find_details->request_id() == kFindInPageRequestId) {
+        // We get multiple responses and one of those will contain the ordinal.
+        // This message comes to us before the final update is sent.
+        if (find_details->active_match_ordinal() > -1)
+          active_match_ordinal_ = find_details->active_match_ordinal();
         if (find_details->final_update()) {
-          automation_->Send(new AutomationMsg_FindInPageResponse(routing_id_,
+          automation_->Send(new AutomationMsg_FindInPageResponse2(routing_id_,
+              active_match_ordinal_,
               find_details->number_of_matches()));
         } else {
           DLOG(INFO) << "Ignoring, since we only care about the final message";
@@ -455,6 +461,9 @@ class FindInPageNotificationObserver : public NotificationObserver {
   AutomationProvider* automation_;
   TabContents* parent_tab_;
   int32 routing_id_;
+  // We will at some point (before final update) be notified of the ordinal and
+  // we need to preserve it so we can send it later.
+  int active_match_ordinal_;
 };
 
 const int FindInPageNotificationObserver::kFindInPageRequestId = -1;
@@ -1712,14 +1721,14 @@ void AutomationProvider::HandleFindInPageRequest(
     int forward, int match_case) {
   NOTREACHED() << "This function has been deprecated."
     << "Please use HandleFindRequest instead.";
-  Send(new AutomationMsg_FindInPageResponse(message.routing_id(), -1));
+  Send(new AutomationMsg_FindInPageResponse2(message.routing_id(), -1, -1));
   return;
 }
 
 void AutomationProvider::HandleFindRequest(const IPC::Message& message,
     int handle, const FindInPageRequest& request) {
   if (!tab_tracker_->ContainsHandle(handle)) {
-    Send(new AutomationMsg_FindInPageResponse(message.routing_id(), -1));
+    Send(new AutomationMsg_FindInPageResponse2(message.routing_id(), -1, -1));
     return;
   }
 
diff --git a/chrome/browser/views/find_bar_win_uitest.cc b/chrome/browser/views/find_bar_win_uitest.cc
index 7844a55..1ce49f5 100644
--- a/chrome/browser/views/find_bar_win_uitest.cc
+++ b/chrome/browser/views/find_bar_win_uitest.cc
@@ -18,6 +18,7 @@ class FindInPageControllerTest : public UITest {
 };
 
 const std::wstring kFramePage = L"files/find_in_page/frames.html";
+const std::wstring kFrameData = L"files/find_in_page/framedata_general.html";
 const std::wstring kUserSelectPage = L"files/find_in_page/user-select.html";
 const std::wstring kCrashPage = L"files/find_in_page/crash_1341577.html";
 const std::wstring kTooFewMatchesPage = L"files/find_in_page/bug_1155639.html";
@@ -33,40 +34,123 @@ TEST_F(FindInPageControllerTest, FindInPageFrames) {
   WaitUntilTabCount(1);
 
   // Try incremental search (mimicking user typing in).
-  EXPECT_EQ(18, tab->FindInPage(L"g",       FWD, IGNORE_CASE, false));
-  EXPECT_EQ(11, tab->FindInPage(L"go",      FWD, IGNORE_CASE, false));
-  EXPECT_EQ(04, tab->FindInPage(L"goo",     FWD, IGNORE_CASE, false));
-  EXPECT_EQ(03, tab->FindInPage(L"goog",    FWD, IGNORE_CASE, false));
-  EXPECT_EQ(02, tab->FindInPage(L"googl",   FWD, IGNORE_CASE, false));
-  EXPECT_EQ(01, tab->FindInPage(L"google",  FWD, IGNORE_CASE, false));
-  EXPECT_EQ(00, tab->FindInPage(L"google!", FWD, IGNORE_CASE, false));
+  EXPECT_EQ(18, tab->FindInPage(L"g",       FWD, IGNORE_CASE, false, NULL));
+  EXPECT_EQ(11, tab->FindInPage(L"go",      FWD, IGNORE_CASE, false, NULL));
+  EXPECT_EQ(04, tab->FindInPage(L"goo",     FWD, IGNORE_CASE, false, NULL));
+  EXPECT_EQ(03, tab->FindInPage(L"goog",    FWD, IGNORE_CASE, false, NULL));
+  EXPECT_EQ(02, tab->FindInPage(L"googl",   FWD, IGNORE_CASE, false, NULL));
+  EXPECT_EQ(01, tab->FindInPage(L"google",  FWD, IGNORE_CASE, false, NULL));
+  EXPECT_EQ(00, tab->FindInPage(L"google!", FWD, IGNORE_CASE, false, NULL));
 
   // Negative test (no matches should be found).
   EXPECT_EQ(0, tab->FindInPage(L"Non-existing string", FWD, IGNORE_CASE,
-                               false));
+                               false, NULL));
 
   // 'horse' only exists in the three right frames.
-  EXPECT_EQ(3, tab->FindInPage(L"horse", FWD, IGNORE_CASE, false));
+  EXPECT_EQ(3, tab->FindInPage(L"horse", FWD, IGNORE_CASE, false, NULL));
 
   // 'cat' only exists in the first frame.
-  EXPECT_EQ(1, tab->FindInPage(L"cat", FWD, IGNORE_CASE, false));
+  EXPECT_EQ(1, tab->FindInPage(L"cat", FWD, IGNORE_CASE, false, NULL));
 
   // Try searching again, should still come up with 1 match.
-  EXPECT_EQ(1, tab->FindInPage(L"cat", FWD, IGNORE_CASE, false));
+  EXPECT_EQ(1, tab->FindInPage(L"cat", FWD, IGNORE_CASE, false, NULL));
 
   // Try searching backwards, ignoring case, should still come up with 1 match.
-  EXPECT_EQ(1, tab->FindInPage(L"CAT", BACK, IGNORE_CASE, false));
+  EXPECT_EQ(1, tab->FindInPage(L"CAT", BACK, IGNORE_CASE, false, NULL));
 
   // Try case sensitive, should NOT find it.
-  EXPECT_EQ(0, tab->FindInPage(L"CAT", FWD, CASE_SENSITIVE, false));
+  EXPECT_EQ(0, tab->FindInPage(L"CAT", FWD, CASE_SENSITIVE, false, NULL));
 
   // Try again case sensitive, but this time with right case.
-  EXPECT_EQ(1, tab->FindInPage(L"dog", FWD, CASE_SENSITIVE, false));
+  EXPECT_EQ(1, tab->FindInPage(L"dog", FWD, CASE_SENSITIVE, false, NULL));
 
   // Try non-Latin characters ('Hreggvidur' with 'eth' for 'd' in left frame).
-  EXPECT_EQ(1, tab->FindInPage(L"Hreggvi\u00F0ur", FWD, IGNORE_CASE, false));
-  EXPECT_EQ(1, tab->FindInPage(L"Hreggvi\u00F0ur", FWD, CASE_SENSITIVE, false));
-  EXPECT_EQ(0, tab->FindInPage(L"hreggvi\u00F0ur", FWD, CASE_SENSITIVE, false));
+  EXPECT_EQ(1, tab->FindInPage(L"Hreggvi\u00F0ur", FWD, IGNORE_CASE,
+                               false, NULL));
+  EXPECT_EQ(1, tab->FindInPage(L"Hreggvi\u00F0ur", FWD, CASE_SENSITIVE,
+                               false, NULL));
+  EXPECT_EQ(0, tab->FindInPage(L"hreggvi\u00F0ur", FWD, CASE_SENSITIVE,
+                               false, NULL));
+}
+
+// This test loads a single-frame page and makes sure the ordinal returned makes
+// sense as we FindNext over all the items.
+TEST_F(FindInPageControllerTest, FindInPageOrdinal) {
+  TestServer server(L"chrome/test/data");
+
+  // First we navigate to our frames page.
+  GURL url = server.TestServerPageW(kFrameData);
+  scoped_ptr<TabProxy> tab(GetActiveTab());
+  ASSERT_TRUE(tab->NavigateToURL(url));
+  WaitUntilTabCount(1);
+
+  // Search for 'o', which should make the first item active and return
+  // '1 in 3' (1st ordinal of a total of 3 matches).
+  int ordinal = 0;
+  EXPECT_EQ(3, tab->FindInPage(L"o", FWD, IGNORE_CASE, false, &ordinal));
+  EXPECT_EQ(1, ordinal);
+  // FindNext returns -1 for match count because it doesn't bother with
+  // recounting the number of matches. We don't care about the match count
+  // anyway in this case, we just want to check the ordinal.
+  EXPECT_EQ(-1, tab->FindInPage(L"o", FWD, IGNORE_CASE, true, &ordinal));
+  EXPECT_EQ(2, ordinal);
+  EXPECT_EQ(-1, tab->FindInPage(L"o", FWD, IGNORE_CASE, true, &ordinal));
+  EXPECT_EQ(3, ordinal);
+  // Go back one match.
+  EXPECT_EQ(-1, tab->FindInPage(L"o", BACK, IGNORE_CASE, true, &ordinal));
+  EXPECT_EQ(2, ordinal);
+  EXPECT_EQ(-1, tab->FindInPage(L"o", FWD, IGNORE_CASE, true, &ordinal));
+  EXPECT_EQ(3, ordinal);
+  // This should wrap to the top.
+  EXPECT_EQ(-1, tab->FindInPage(L"o", FWD, IGNORE_CASE, true, &ordinal));
+  EXPECT_EQ(1, ordinal);
+  // This should go back to the end.
+  EXPECT_EQ(-1, tab->FindInPage(L"o", BACK, IGNORE_CASE, true, &ordinal));
+  EXPECT_EQ(3, ordinal);
+}
+
+// This test loads a page with frames and makes sure the ordinal returned makes
+// sense.
+TEST_F(FindInPageControllerTest, FindInPageMultiFramesOrdinal) {
+  TestServer server(L"chrome/test/data");
+
+  // First we navigate to our frames page.
+  GURL url = server.TestServerPageW(kFramePage);
+  scoped_ptr<TabProxy> tab(GetActiveTab());
+  ASSERT_TRUE(tab->NavigateToURL(url));
+  WaitUntilTabCount(1);
+
+  // Search for 'a', which should make the first item active and return
+  // '1 in 7' (1st ordinal of a total of 7 matches).
+  int ordinal = 0;
+  EXPECT_EQ(7, tab->FindInPage(L"a", FWD, IGNORE_CASE, false, &ordinal));
+  EXPECT_EQ(1, ordinal);
+  // FindNext returns -1 for match count because it doesn't bother with
+  // recounting the number of matches. We don't care about the match count
+  // anyway in this case, we just want to check the ordinal.
+  EXPECT_EQ(-1, tab->FindInPage(L"a", FWD, IGNORE_CASE, true, &ordinal));
+  EXPECT_EQ(2, ordinal);
+  EXPECT_EQ(-1, tab->FindInPage(L"a", FWD, IGNORE_CASE, true, &ordinal));
+  EXPECT_EQ(3, ordinal);
+  EXPECT_EQ(-1, tab->FindInPage(L"a", FWD, IGNORE_CASE, true, &ordinal));
+  EXPECT_EQ(4, ordinal);
+  // Go back one, which should go back one frame.
+  EXPECT_EQ(-1, tab->FindInPage(L"a", BACK, IGNORE_CASE, true, &ordinal));
+  EXPECT_EQ(3, ordinal);
+  EXPECT_EQ(-1, tab->FindInPage(L"a", FWD, IGNORE_CASE, true, &ordinal));
+  EXPECT_EQ(4, ordinal);
+  EXPECT_EQ(-1, tab->FindInPage(L"a", FWD, IGNORE_CASE, true, &ordinal));
+  EXPECT_EQ(5, ordinal);
+  EXPECT_EQ(-1, tab->FindInPage(L"a", FWD, IGNORE_CASE, true, &ordinal));
+  EXPECT_EQ(6, ordinal);
+  EXPECT_EQ(-1, tab->FindInPage(L"a", FWD, IGNORE_CASE, true, &ordinal));
+  EXPECT_EQ(7, ordinal);
+  // Now we should wrap back to frame 1.
+  EXPECT_EQ(-1, tab->FindInPage(L"a", FWD, IGNORE_CASE, true, &ordinal));
+  EXPECT_EQ(1, ordinal);
+  // Now we should wrap back to frame last frame.
+  EXPECT_EQ(-1, tab->FindInPage(L"a", BACK, IGNORE_CASE, true, &ordinal));
+  EXPECT_EQ(7, ordinal);
 }
 
 // Load a page with no selectable text and make sure we don't crash.
@@ -78,9 +162,9 @@ TEST_F(FindInPageControllerTest, FindUnSelectableText) {
   ASSERT_TRUE(tab->NavigateToURL(url));
   WaitUntilTabCount(1);
 
-  EXPECT_EQ(0, tab->FindInPage(L"text", FWD, IGNORE_CASE, false));
+  EXPECT_EQ(0, tab->FindInPage(L"text", FWD, IGNORE_CASE, false, NULL));
   EXPECT_EQ(0, tab->FindInPage(L"Non-existing string", FWD, IGNORE_CASE,
-                               false));
+                               false, NULL));
 }
 
 // Try to reproduce the crash seen in issue 1341577.
@@ -95,15 +179,15 @@ TEST_F(FindInPageControllerTest, FindCrash_Issue1341577) {
   // This would crash the tab. These must be the first two find requests issued
   // against the frame, otherwise an active frame pointer is set and it wont
   // produce the crash.
-  EXPECT_EQ(1, tab->FindInPage(L"\u0D4C", FWD, IGNORE_CASE, false));
+  EXPECT_EQ(1, tab->FindInPage(L"\u0D4C", FWD, IGNORE_CASE, false, NULL));
   // FindNext returns -1 for match count because it doesn't bother with
   // recounting the number of matches. We don't care about the match count
   // anyway in this case, we just want to make sure it doesn't crash.
-  EXPECT_EQ(-1, tab->FindInPage(L"\u0D4C", FWD, IGNORE_CASE, true));
+  EXPECT_EQ(-1, tab->FindInPage(L"\u0D4C", FWD, IGNORE_CASE, true, NULL));
 
   // This should work fine.
-  EXPECT_EQ(1, tab->FindInPage(L"\u0D24\u0D46", FWD, IGNORE_CASE, false));
-  EXPECT_EQ(0, tab->FindInPage(L"nostring", FWD, IGNORE_CASE, false));
+  EXPECT_EQ(1, tab->FindInPage(L"\u0D24\u0D46", FWD, IGNORE_CASE, false, NULL));
+  EXPECT_EQ(0, tab->FindInPage(L"nostring", FWD, IGNORE_CASE, false, NULL));
 }
 
 // Test to make sure Find does the right thing when restarting from a timeout.
@@ -124,88 +208,65 @@ TEST_F(FindInPageControllerTest, FindEnoughMatches_Issue1155639) {
 
   // This string appears 5 times at the bottom of a long page. If Find restarts
   // properly after a timeout, it will find 5 matches, not just 1.
-  EXPECT_EQ(5, tab->FindInPage(L"008.xml", FWD, IGNORE_CASE, false));
+  EXPECT_EQ(5, tab->FindInPage(L"008.xml", FWD, IGNORE_CASE, false, NULL));
 }
 
 // The find window should not change its location just because we open and close
 // a new tab.
 TEST_F(FindInPageControllerTest, FindMovesOnTabClose_Issue1343052) {
-  fprintf(stderr, "Starting FindMovesOnTabClose_Issue1343052\n");
   TestServer server(L"chrome/test/data");
 
-  fprintf(stderr, "TestServerPageW\n");
   GURL url = server.TestServerPageW(kFramePage);
-  fprintf(stderr, "GetActiveTab A\n");
   scoped_ptr<TabProxy> tabA(GetActiveTab());
-  fprintf(stderr, "Navigate A\n");
   ASSERT_TRUE(tabA->NavigateToURL(url));
-  fprintf(stderr, "WaitUntilTabCount(1) for A\n");
   WaitUntilTabCount(1);
 
-  fprintf(stderr, "GetBrowserWindow(0)\n");
   scoped_ptr<BrowserProxy> browser(automation()->GetBrowserWindow(0));
   ASSERT_TRUE(browser.get() != NULL);
 
   // Toggle the bookmark bar state.
-  fprintf(stderr, "ApplyAccelerator bookmark bar\n");
   browser->ApplyAccelerator(IDC_SHOW_BOOKMARKS_BAR);
-  fprintf(stderr, "WaitForBookmarkVisibility\n");
   EXPECT_TRUE(WaitForBookmarkBarVisibilityChange(browser.get(), true));
 
   // Open the Find window and wait for it to animate.
-  fprintf(stderr, "OpenFindInPage in A\n");
   EXPECT_TRUE(tabA->OpenFindInPage());
-  fprintf(stderr, "WaitForWindowFullyVisible in A\n");
   EXPECT_TRUE(WaitForFindWindowFullyVisible(tabA.get()));
 
   // Find its location.
   int x = -1, y = -1;
-  fprintf(stderr, "GetFindWindowLocation in A\n");
   EXPECT_TRUE(tabA->GetFindWindowLocation(&x, &y));
 
   // Open another tab (tab B).
-  fprintf(stderr, "AppendTab B\n");
   EXPECT_TRUE(browser->AppendTab(url));
-  fprintf(stderr, "GetActiveTab B\n");
   scoped_ptr<TabProxy> tabB(GetActiveTab());
 
   // Close tab B.
-  fprintf(stderr, "Tab Close B\n");
   EXPECT_TRUE(tabB->Close(true));
 
   // See if the Find window has moved.
   int new_x = -1, new_y = -1;
-  fprintf(stderr, "GetFindWindowLocation in A\n");
   EXPECT_TRUE(tabA->GetFindWindowLocation(&new_x, &new_y));
 
   EXPECT_EQ(x, new_x);
   EXPECT_EQ(y, new_y);
 
   // Now reset the bookmarks bar state and try the same again.
-  fprintf(stderr, "ApplyAccelerator BookmarksBar\n");
   browser->ApplyAccelerator(IDC_SHOW_BOOKMARKS_BAR);
-  fprintf(stderr, "WaitForBookmarkBarVisibilityChange\n");
   EXPECT_TRUE(WaitForBookmarkBarVisibilityChange(browser.get(), false));
 
   // Bookmark bar has moved, reset our coordinates.
-  fprintf(stderr, "GetFindWindowLocation again\n");
   EXPECT_TRUE(tabA->GetFindWindowLocation(&x, &y));
 
   // Open another tab (tab C).
-  fprintf(stderr, "Append tab C\n");
   EXPECT_TRUE(browser->AppendTab(url));
-  fprintf(stderr, "GetActiveTab C\n");
   scoped_ptr<TabProxy> tabC(GetActiveTab());
 
   // Close it.
-  fprintf(stderr, "Close tab C\n");
   EXPECT_TRUE(tabC->Close(true));
 
   // See if the Find window has moved.
-  fprintf(stderr, "GetFindWindowLocation yet again\n");
   EXPECT_TRUE(tabA->GetFindWindowLocation(&new_x, &new_y));
 
   EXPECT_EQ(x, new_x);
   EXPECT_EQ(y, new_y);
-  fprintf(stderr, "Done!\n");
 }
diff --git a/chrome/test/automation/automation_messages_internal.h b/chrome/test/automation/automation_messages_internal.h
index 3eb8498..b71457d 100644
--- a/chrome/test/automation/automation_messages_internal.h
+++ b/chrome/test/automation/automation_messages_internal.h
@@ -311,15 +311,15 @@ IPC_BEGIN_MESSAGES(Automation, 0)
   // (1=case sensitive, 0=case insensitive). If an error occurs, matches_found
   // will be -1.
   //
-  // NOTE: This message has been deprecated, please use the new message
-  // AutomationMsg_FindRequest below.
+  // NOTE: These two messages have been deprecated, please use the new messages
+  // AutomationMsg_FindRequest and AutomationMsg_FindInPageResponse2 below.
   //
-  IPC_MESSAGE_ROUTED4(AutomationMsg_FindInPageRequest,
+  IPC_MESSAGE_ROUTED4(AutomationMsg_FindInPageRequest,   // DEPRECATED.
                       int, /* tab_handle */
                       std::wstring, /* find_request */
                       int, /* forward */
                       int /* match_case */)
-  IPC_MESSAGE_ROUTED1(AutomationMsg_FindInPageResponse,
+  IPC_MESSAGE_ROUTED1(AutomationMsg_FindInPageResponse,  // DEPRECATED.
                       int /* matches_found */)
 
   // This message sends a inspect element request for a given tab. The response
@@ -733,7 +733,7 @@ IPC_BEGIN_MESSAGES(Automation, 0)
   // This message starts a find within a tab corresponding to the supplied
   // tab handle. The parameter |request| specifies what to search for.
   // If an error occurs, |matches_found| will be -1 (see response message
-  // AutomationMsg_FindInPageResponse).
+  // AutomationMsg_FindInPageResponse2).
   //
   IPC_MESSAGE_ROUTED2(AutomationMsg_FindRequest,
                       int, /* tab_handle */
@@ -807,4 +807,9 @@ IPC_BEGIN_MESSAGES(Automation, 0)
   IPC_MESSAGE_ROUTED1(AutomationMsg_ShowingAppModalDialogResponse,
                       bool /* showing dialog */)
 
+  // Returns the ordinal and the number of matches found as a response to
+  // a AutomationMsg_FindRequest.
+  IPC_MESSAGE_ROUTED2(AutomationMsg_FindInPageResponse2,
+                      int /* active_ordinal */,
+                      int /* matches_found */)
 IPC_END_MESSAGES(Automation)
diff --git a/chrome/test/automation/tab_proxy.cc b/chrome/test/automation/tab_proxy.cc
index 7668c19..0b87689 100644
--- a/chrome/test/automation/tab_proxy.cc
+++ b/chrome/test/automation/tab_proxy.cc
@@ -121,7 +121,8 @@ bool TabProxy::GetFindWindowLocation(int* x, int* y) {
 int TabProxy::FindInPage(const std::wstring& search_string,
                          FindInPageDirection forward,
                          FindInPageCase match_case,
-                         bool find_next) {
+                         bool find_next,
+                         int* active_ordinal) {
   if (!is_valid())
     return -1;
 
@@ -136,14 +137,18 @@ int TabProxy::FindInPage(const std::wstring& search_string,
   bool succeeded = sender_->SendAndWaitForResponse(
       new AutomationMsg_FindRequest(0, handle_, request),
       &response,
-      AutomationMsg_FindInPageResponse::ID);
+      AutomationMsg_FindInPageResponse2::ID);
   if (!succeeded)
     return -1;
 
   void* iter = NULL;
+  int ordinal;
   int matches_found;
-  AutomationMsg_FindInPageResponse::Read(response, &matches_found);
-
+  response->ReadInt(&iter, &ordinal);
+  response->ReadInt(&iter, &matches_found);
+  if (active_ordinal)
+    *active_ordinal = ordinal;
+  delete response;
   return matches_found;
 }
 
diff --git a/chrome/test/automation/tab_proxy.h b/chrome/test/automation/tab_proxy.h
index a13e6a5..07e3617 100644
--- a/chrome/test/automation/tab_proxy.h
+++ b/chrome/test/automation/tab_proxy.h
@@ -178,9 +178,11 @@ class TabProxy : public AutomationResourceProxy {
   // specifies what string to search for, |forward| specifies whether to search
   // in forward direction, and |match_case| specifies case sensitivity
   // (true=case sensitive). |find_next| specifies whether this is a new search
-  // or a continuation of the old one. A return value of -1 indicates failure.
+  // or a continuation of the old one. |ordinal| is an optional parameter that
+  // returns the ordinal of the active match (also known as "the 7" part of
+  // "7 of 9"). A return value of -1 indicates failure.
   int FindInPage(const std::wstring& search_string, FindInPageDirection forward,
-                 FindInPageCase match_case, bool find_next);
+                 FindInPageCase match_case, bool find_next, int* ordinal);
 
   bool GetCookies(const GURL& url, std::string* cookies);
   bool GetCookieByName(const GURL& url,