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

1 files changed, 13 insertions, 4 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;
   }