Fix 12729: Find box doesn't unregister Esc accelerator.

This regressed recently during porting. When
comparing to see who is registered we need to 
be comparing against a class that implements
FocusChangeListener (and registers as such
with AddFocusChangeListener). In this case,
host_ does not implement that interface and 
doesn't register as a listener, so it will 
never be registered. We should be comparing
using 'this', not 'host_'.

I've fixed this and added a browser_test to 
catch this in the future.

BUG=12729
TEST=Open Find, press Esc, press Esc again 
and the Find box should not briefly appear
and hide again. Also, make sure Esc works 
to cancel navigations.

Review URL: http://codereview.chromium.org/115832

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

chrome/browser/views/find_bar_win.cc

@@ -35,7 +35,7 @@ bool FindBarWin::disable_animations_during_testing_ = false;
 #if defined(OS_WIN)
 class FindBarWin::Host : public views::WidgetWin {
  public:
-  Host(FindBarWin* find_bar) : find_bar_(find_bar) {
+  explicit Host(FindBarWin* find_bar) : find_bar_(find_bar) {
     // Don't let WidgetWin manage our lifetime. We want our lifetime to
     // coincide with TabContents.
     set_delete_on_destroy(false);
@@ -55,7 +55,9 @@ class FindBarWin::Host : public views::WidgetWin {
 #else
 class FindBarWin::Host : public views::WidgetGtk {
  public:
-  Host(FindBarWin* find_bar) : WidgetGtk(TYPE_CHILD), find_bar_(find_bar) {
+  explicit Host(FindBarWin* find_bar)
+    : WidgetGtk(TYPE_CHILD),
+      find_bar_(find_bar) {
     // Don't let WidgetWin manage our lifetime. We want our lifetime to
     // coincide with TabContents.
     set_delete_on_destroy(false);
@@ -439,7 +441,7 @@ bool FindBarWin::GetFindBarWindowInfo(gfx::Point* position,
       !::IsWindow(host_->GetNativeView())) {
 #else
       false) {
-      // TODO: figure out linux side.
+      // TODO(sky): figure out linux side.
 #endif
     *position = gfx::Point();
     *fully_visible = false;
@@ -623,7 +625,7 @@ void FindBarWin::UnregisterEscAccelerator() {
   views::Accelerator escape(VK_ESCAPE, false, false, false);
   views::AcceleratorTarget* current_target =
       focus_manager_->GetTargetForAccelerator(escape);
-  if (current_target == host_.get())
+  if (current_target == this)
     focus_manager_->RegisterAccelerator(escape, old_accel_target_for_esc_);
 #else
   NOTIMPLEMENTED();

chrome/browser/views/find_bar_win_browsertest.cc

@@ -13,6 +13,7 @@
 #include "chrome/common/notification_service.h"
 #include "chrome/test/in_process_browser_test.h"
 #include "chrome/test/ui_test_utils.h"
+#include "views/focus/focus_manager.h"
 
 const std::wstring kSimplePage = L"404_is_enough_for_us.html";
 const std::wstring kFramePage = L"files/find_in_page/frames.html";
@@ -519,7 +520,7 @@ IN_PROC_BROWSER_TEST_F(FindInPageControllerTest,
                        FindNextInNewTabUsesPrepopulate) {
   HTTPTestServer* server = StartHTTPServer();
 
-  // First we navigate to our special focus tracking page.
+  // First we navigate to any page.
   GURL url = server->TestServerPageW(kSimplePage);
   ui_test_utils::NavigateToURL(browser(), url);
 
@@ -546,3 +547,37 @@ IN_PROC_BROWSER_TEST_F(FindInPageControllerTest,
   EXPECT_EQ(0, FindInPage(std::wstring(), FWD, IGNORE_CASE, &ordinal));
   EXPECT_EQ(0, ordinal);
 }
+
+// Make sure Find box grabs the Esc accelerator and restores it again.
+IN_PROC_BROWSER_TEST_F(FindInPageControllerTest, AcceleratorRestoring) {
+  HTTPTestServer* server = StartHTTPServer();
+
+  // First we navigate to any page.
+  GURL url = server->TestServerPageW(kSimplePage);
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  views::FocusManager* focus_manager = views::FocusManager::GetFocusManager(
+      browser()->window()->GetNativeHandle());
+
+  // See where Escape is registered.
+  views::Accelerator escape(VK_ESCAPE, false, false, false);
+  views::AcceleratorTarget* old_target =
+      focus_manager->GetTargetForAccelerator(escape);
+  EXPECT_TRUE(old_target != NULL);
+
+  // Open the Find box.
+  browser()->ShowFindBar();
+
+  // Our Find bar should be the new target.
+  views::AcceleratorTarget* new_target =
+      focus_manager->GetTargetForAccelerator(escape);
+
+  EXPECT_TRUE(new_target != NULL);
+  EXPECT_NE(new_target, old_target);
+
+  // Close the Find box.
+  browser()->find_bar()->EndFindSession();
+
+  // The accelerator for Escape should be back to what it was before.
+  EXPECT_EQ(old_target, focus_manager->GetTargetForAccelerator(escape));
+}