When you search for something, press F3, close the box and press F3 it would only open the Find box and not issue the search. This is because we set the prepopulate string to find_text and not find_text_ (the former is blank on F3). I have also changed the variable name to prevent this kind of confusion in the future.

BUG=28306
TEST=Automated test added.
Review URL: http://codereview.chromium.org/425003

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

4 files changed, 47 insertions, 10 deletions

diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index 48b7cb6..4799e6e 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -1012,16 +1012,16 @@ void TabContents::DidMoveOrResize(ConstrainedWindow* window) {
 #endif
 }
 
-void TabContents::StartFinding(string16 find_text,
+void TabContents::StartFinding(string16 search_string,
                                bool forward_direction,
                                bool case_sensitive) {
-  // If find_text is empty, it means FindNext was pressed with a keyboard
+  // If search_string is empty, it means FindNext was pressed with a keyboard
   // shortcut so unless we have something to search for we return early.
-  if (find_text.empty() && find_text_.empty()) {
+  if (search_string.empty() && find_text_.empty()) {
     if (last_search_prepopulate_text_->empty())
       return;
     // Try whatever we searched for last in any tab.
-    find_text = *last_search_prepopulate_text_;
+    search_string = *last_search_prepopulate_text_;
   }
 
   // Keep track of the previous search.
@@ -1033,21 +1033,20 @@ void TabContents::StartFinding(string16 find_text,
   // because the highlighting has been cleared and we need it to reappear). We
   // therefore treat FindNext after an aborted Find operation as a full fledged
   // Find.
-  bool find_next = (find_text_ == find_text || find_text.empty()) &&
+  bool find_next = (find_text_ == search_string || search_string.empty()) &&
                    (last_search_case_sensitive_ == case_sensitive) &&
                    !find_op_aborted_;
   if (!find_next)
     current_find_request_id_ = find_request_id_counter_++;
 
-  if (!find_text.empty())
-    find_text_ = find_text;
+  if (!search_string.empty())
+    find_text_ = search_string;
   last_search_case_sensitive_ = case_sensitive;
 
   find_op_aborted_ = false;
 
   // Keep track of what the last search was across the tabs.
-  *last_search_prepopulate_text_ = find_text;
-
+  *last_search_prepopulate_text_ = find_text_;
   render_view_host()->StartFinding(current_find_request_id_,
                                    find_text_,
                                    forward_direction,
diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h
index e440a53..33dae8a 100644
--- a/chrome/browser/tab_contents/tab_contents.h
+++ b/chrome/browser/tab_contents/tab_contents.h
@@ -488,7 +488,7 @@ class TabContents : public PageNavigator,
   // function does not block while a search is in progress. The controller will
   // receive the results through the notification mechanism. See Observe(...)
   // for details.
-  void StartFinding(string16 find_text,
+  void StartFinding(string16 search_string,
                     bool forward_direction,
                     bool case_sensitive);
 
diff --git a/chrome/browser/views/find_bar_host_browsertest.cc b/chrome/browser/views/find_bar_host_browsertest.cc
index d254aa04..94a586e 100644
--- a/chrome/browser/views/find_bar_host_browsertest.cc
+++ b/chrome/browser/views/find_bar_host_browsertest.cc
@@ -29,6 +29,7 @@ const std::wstring kMoveIfOver = L"files/find_in_page/move_if_obscuring.html";
 const std::wstring kBitstackCrash = L"files/find_in_page/crash_14491.html";
 const std::wstring kSelectChangesOrdinal =
     L"files/find_in_page/select_changes_ordinal.html";
+const std::wstring kSimple = L"files/find_in_page/simple.html";
 
 const bool kBack = false;
 const bool kFwd = true;
@@ -730,3 +731,32 @@ IN_PROC_BROWSER_TEST_F(FindInPageControllerTest, StayActive) {
   // still responds to browser window resizing.
   ASSERT_TRUE(tab_contents->find_ui_active());
 }
+
+// Make sure F3 works after you FindNext a couple of times and end the Find
+// session. See issue http://crbug.com/28306.
+IN_PROC_BROWSER_TEST_F(FindInPageControllerTest, RestartSearchFromF3) {
+  HTTPTestServer* server = StartHTTPServer();
+
+  // First we navigate to a simple page.
+  GURL url = server->TestServerPageW(kSimple);
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  // Search for 'page'. Should have 1 match.
+  int ordinal = 0;
+  TabContents* tab = browser()->GetSelectedTabContents();
+  EXPECT_EQ(1, FindInPageWchar(tab, L"page", kFwd, kIgnoreCase, &ordinal));
+  EXPECT_EQ(1, ordinal);
+
+  // Simulate what happens when you press F3 for FindNext. Still should show
+  // one match. This cleared the pre-populate string at one point (see bug).
+  EXPECT_EQ(1, ui_test_utils::FindInPage(tab, string16(),
+                                         kFwd, kIgnoreCase, &ordinal));
+  EXPECT_EQ(1, ordinal);
+
+  // End the Find session, thereby making the next F3 start afresh.
+  browser()->GetFindBarController()->EndFindSession();
+
+  // Simulate F3 while Find box is closed. Should have 1 match.
+  EXPECT_EQ(1, FindInPageWchar(tab, L"", kFwd, kIgnoreCase, &ordinal));
+  EXPECT_EQ(1, ordinal);
+}
diff --git a/chrome/test/data/find_in_page/simple.html b/chrome/test/data/find_in_page/simple.html
new file mode 100644
index 0000000..15bf59c
--- /dev/null
+++ b/chrome/test/data/find_in_page/simple.html
@@ -0,0 +1,8 @@
+<html>
+<head>
+  <title>Find in page</title>
+</head>
+<body>
+  This is a page with some text that we use in the browser tests.
+</body>
+</html>
\ No newline at end of file