Fix: Clearing search 'foo', closing and reopening Find

should not prepopulate with 'foo'.

We need to reset the remembered values when the user
clears the search term from the Find box to prevent
them from showing up on close+Reopen and on close+F3.

Added test to cover these cases.

This change also makes sure we don't clear the Find
string when we don't need to, which was happening on
close of the Find box and on new tab, causing the
fix not to work.

BUG=42639
TEST=Covered by new automated test.
Review URL: http://codereview.chromium.org/2322006

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

7 files changed, 110 insertions, 10 deletions

diff --git a/chrome/browser/find_bar_controller.cc b/chrome/browser/find_bar_controller.cc
index 954cab9..8443cf6 100644
--- a/chrome/browser/find_bar_controller.cc
+++ b/chrome/browser/find_bar_controller.cc
@@ -47,7 +47,9 @@ void FindBarController::EndFindSession(SelectionAction action) {
     // for now, so that we can abort the scoping effort and clear all the
     // tickmarks and highlighting.
     tab_contents_->StopFinding(action);
-    find_bar_->ClearResults(tab_contents_->find_result());
+
+    if (action != kKeepSelection)
+      find_bar_->ClearResults(tab_contents_->find_result());
 
     // When we get dismissed we restore the focus to where it belongs.
     find_bar_->RestoreSavedFocus();
diff --git a/chrome/browser/find_bar_state.h b/chrome/browser/find_bar_state.h
index 026caac..b350e30 100644
--- a/chrome/browser/find_bar_state.h
+++ b/chrome/browser/find_bar_state.h
@@ -22,7 +22,7 @@ class FindBarState {
     return last_prepopulate_text_;
   }
 
-  void set_last_prepopulate_text(const string16 text) {
+  void set_last_prepopulate_text(const string16& text) {
     last_prepopulate_text_ = text;
   }
 
diff --git a/chrome/browser/gtk/find_bar_gtk.cc b/chrome/browser/gtk/find_bar_gtk.cc
index b7beac2..909799a 100644
--- a/chrome/browser/gtk/find_bar_gtk.cc
+++ b/chrome/browser/gtk/find_bar_gtk.cc
@@ -12,6 +12,7 @@
 #include "base/string_util.h"
 #include "chrome/browser/browser.h"
 #include "chrome/browser/find_bar_controller.h"
+#include "chrome/browser/find_bar_state.h"
 #include "chrome/browser/gtk/browser_window_gtk.h"
 #include "chrome/browser/gtk/cairo_cached_surface.h"
 #include "chrome/browser/gtk/custom_button.h"
@@ -557,6 +558,12 @@ void FindBarGtk::FindEntryTextInContents(bool forward_search) {
     tab_contents->StopFinding(FindBarController::kClearSelection);
     UpdateUIForFindResult(find_bar_controller_->tab_contents()->find_result(),
                           string16());
+
+    // Clearing the text box should also clear the prepopulate state so that
+    // when we close and reopen the Find box it doesn't show the search we
+    // just deleted.
+    FindBarState* find_bar_state = browser_->profile()->GetFindBarState();
+    find_bar_state->set_last_prepopulate_text(string16());
   }
 }
 
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index 8e2fff5..d3e993d 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -1245,12 +1245,16 @@ void TabContents::StartFinding(string16 search_string,
 
 void TabContents::StopFinding(
     FindBarController::SelectionAction selection_action) {
-  // When the action is kClearAction, it means the find string has been cleared
-  // by the user, but the UI has not been dismissed.
-  if (selection_action != FindBarController::kClearSelection)
+  if (selection_action == FindBarController::kClearSelection) {
+    // kClearSelection means the find string has been cleared by the user, but
+    // the UI has not been dismissed. In that case we want to clear the
+    // previously remembered search (http://crbug.com/42639).
+    previous_find_text_ = string16();
+  } else {
     find_ui_active_ = false;
-  if (!find_text_.empty())
-    previous_find_text_ = find_text_;
+    if (!find_text_.empty())
+      previous_find_text_ = find_text_;
+  }
   find_text_.clear();
   find_op_aborted_ = true;
   last_search_result_ = FindNotificationDetails();
diff --git a/chrome/browser/views/find_bar_host.cc b/chrome/browser/views/find_bar_host.cc
index 5ee4d44..5ff60ad 100644
--- a/chrome/browser/views/find_bar_host.cc
+++ b/chrome/browser/views/find_bar_host.cc
@@ -229,7 +229,8 @@ FindBarTesting* FindBarHost::GetFindBarTesting() {
 
 void FindBarHost::UpdateUIForFindResult(const FindNotificationDetails& result,
                                        const string16& find_text) {
-  find_bar_view()->UpdateForResult(result, find_text);
+  if (!find_text.empty())
+    find_bar_view()->UpdateForResult(result, find_text);
 
   // We now need to check if the window is obscuring the search results.
   if (!result.selection_rect().IsEmpty())
diff --git a/chrome/browser/views/find_bar_host_interactive_uitest.cc b/chrome/browser/views/find_bar_host_interactive_uitest.cc
index 2933ad39..6e9bfbe 100644
--- a/chrome/browser/views/find_bar_host_interactive_uitest.cc
+++ b/chrome/browser/views/find_bar_host_interactive_uitest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -23,7 +23,7 @@ namespace {
 // The delay waited after sending an OS simulated event.
 static const int kActionDelayMs = 500;
 static const wchar_t kDocRoot[] = L"chrome/test/data";
-static const char kSimplePage[] = "404_is_enough_for_us.html";
+static const char kSimplePage[] = "files/find_in_page/simple.html";
 
 class FindInPageTest : public InProcessBrowserTest {
  public:
@@ -76,6 +76,12 @@ class FindInPageTest : public InProcessBrowserTest {
     return -1;
 #endif
   }
+
+  string16 GetFindBarText() {
+    FindBarTesting* find_bar =
+        browser()->GetFindBarController()->find_bar()->GetFindBarTesting();
+    return find_bar->GetFindText();
+  }
 };
 
 }  // namespace
@@ -159,3 +165,72 @@ IN_PROC_BROWSER_TEST_F(FindInPageTest, FocusRestore) {
       FindBarController::kKeepSelection);
   EXPECT_EQ(VIEW_ID_LOCATION_BAR, GetFocusedViewID());
 }
+
+// This tests that whenever you clear values from the Find box and close it that
+// it respects that and doesn't show you the last search, as reported in bug:
+// http://crbug.com/40121.
+IN_PROC_BROWSER_TEST_F(FindInPageTest, PrepopulateRespectBlank) {
+#if defined(OS_MACOSX)
+  // FindInPage on Mac doesn't use prepopulated values. Search there is global.
+  return;
+#endif
+
+  HTTPTestServer* server = StartHTTPServer();
+  ASSERT_TRUE(server);
+
+  // First we navigate to any page.
+  GURL url = server->TestServerPage(kSimplePage);
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  gfx::NativeWindow window = browser()->window()->GetNativeHandle();
+
+  // Show the Find bar.
+  browser()->GetFindBarController()->Show();
+
+  // Search for "a".
+  ui_controls::SendKeyPressNotifyWhenDone(window, base::VKEY_A,
+      false, false, false, false,  // No modifiers.
+      new MessageLoop::QuitTask());
+  ui_test_utils::RunMessageLoop();
+
+  // We should find "a" here.
+  EXPECT_EQ(ASCIIToUTF16("a"), GetFindBarText());
+
+  // Delete "a".
+  ui_controls::SendKeyPressNotifyWhenDone(window, base::VKEY_BACK,
+      false, false, false, false,  // No modifiers.
+      new MessageLoop::QuitTask());
+  ui_test_utils::RunMessageLoop();
+
+  // Validate we have cleared the text.
+  EXPECT_EQ(string16(), GetFindBarText());
+
+  // Close the Find box.
+  ui_controls::SendKeyPressNotifyWhenDone(window, base::VKEY_ESCAPE,
+      false, false, false, false,  // No modifiers.
+      new MessageLoop::QuitTask());
+  ui_test_utils::RunMessageLoop();
+
+  // Show the Find bar.
+  browser()->GetFindBarController()->Show();
+
+  // After the Find box has been reopened, it should not have been prepopulated
+  // with "a" again.
+  EXPECT_EQ(string16(), GetFindBarText());
+
+  // Close the Find box.
+  ui_controls::SendKeyPressNotifyWhenDone(window, base::VKEY_ESCAPE,
+      false, false, false, false,  // No modifiers.
+      new MessageLoop::QuitTask());
+  ui_test_utils::RunMessageLoop();
+
+  // Press F3 to trigger FindNext.
+  ui_controls::SendKeyPressNotifyWhenDone(window, base::VKEY_F3,
+      false, false, false, false,  // No modifiers.
+      new MessageLoop::QuitTask());
+  ui_test_utils::RunMessageLoop();
+
+  // After the Find box has been reopened, it should still have no prepopulate
+  // value.
+  EXPECT_EQ(string16(), GetFindBarText());
+}
diff --git a/chrome/browser/views/find_bar_view.cc b/chrome/browser/views/find_bar_view.cc
index c1f063e..1b78057 100644
--- a/chrome/browser/views/find_bar_view.cc
+++ b/chrome/browser/views/find_bar_view.cc
@@ -11,6 +11,8 @@
 #include "base/string_util.h"
 #include "chrome/browser/browser_theme_provider.h"
 #include "chrome/browser/find_bar_controller.h"
+#include "chrome/browser/find_bar_state.h"
+#include "chrome/browser/profile.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/views/find_bar_host.h"
 #include "chrome/browser/view_ids.h"
@@ -447,6 +449,15 @@ void FindBarView::ContentsChanged(views::Textfield* sender,
   } else {
     controller->tab_contents()->StopFinding(FindBarController::kClearSelection);
     UpdateForResult(controller->tab_contents()->find_result(), string16());
+
+    // Clearing the text box should clear the prepopulate state so that when
+    // we close and reopen the Find box it doesn't show the search we just
+    // deleted. We can't do this on ChromeOS yet because we get ContentsChanged
+    // sent for a lot more things than just the user nulling out the search
+    // terms. See http://crbug.com/45372.
+    FindBarState* find_bar_state =
+        controller->tab_contents()->profile()->GetFindBarState();
+    find_bar_state->set_last_prepopulate_text(string16());
   }
 }