Fix 10573: Dismissing Find-in page doesn't set focus

to the link found.

We no longer use the selection controller to
highlight the active match. Before this change,
the focus would not be set if the user had changed
the selection. After this change, the focus will
be set unless the user has selected something on
the page.

I also wrote an in-browser unit test for this to
catch this regression in the future, but it is
disabled due to problem with running multiple
in-process browser tests in a row (teardown
problem).

BUG=10573
TEST=Covered by in process browser test now, see
bug for repro steps.

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

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

2 files changed, 65 insertions, 61 deletions

diff --git a/chrome/browser/browser_focus_uitest.cc b/chrome/browser/browser_focus_uitest.cc
index 77d67b5..a19e072 100644
--- a/chrome/browser/browser_focus_uitest.cc
+++ b/chrome/browser/browser_focus_uitest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
@@ -6,16 +6,10 @@
 #include "base/ref_counted.h"
 #include "chrome/browser/automation/ui_controls.h"
 #include "chrome/browser/browser.h"
-#include "chrome/browser/dom_operation_notification_details.h"
 #include "chrome/browser/tab_contents/web_contents.h"
 #include "chrome/browser/view_ids.h"
 #include "chrome/browser/views/frame/browser_view.h"
 #include "chrome/browser/views/location_bar_view.h"
-#include "chrome/common/notification_details.h"
-#include "chrome/common/notification_observer.h"
-#include "chrome/common/notification_service.h"
-#include "chrome/common/notification_source.h"
-#include "chrome/common/notification_type.h"
 #include "chrome/views/focus/focus_manager.h"
 #include "chrome/views/view.h"
 #include "chrome/views/window/window.h"
@@ -40,55 +34,6 @@ class BrowserFocusTest : public InProcessBrowserTest {
   }
 };
 
-class JavaScriptRunner : public NotificationObserver {
- public:
-  JavaScriptRunner(WebContents* web_contents,
-                   const std::wstring& frame_xpath,
-                   const std::wstring& jscript)
-      : web_contents_(web_contents),
-        frame_xpath_(frame_xpath),
-        jscript_(jscript) {
-    NotificationService::current()->
-        AddObserver(this, NotificationType::DOM_OPERATION_RESPONSE,
-                    Source<WebContents>(web_contents));
-  }
-
-  virtual void Observe(NotificationType type,
-                       const NotificationSource& source,
-                       const NotificationDetails& details) {
-    Details<DomOperationNotificationDetails> dom_op_details(details);
-    result_ = dom_op_details->json();
-    // The Jasonified response has quotes, remove them.
-    if (result_.length() > 1 && result_[0] == '"')
-      result_ = result_.substr(1, result_.length() - 2);
-
-    NotificationService::current()->
-        RemoveObserver(this, NotificationType::DOM_OPERATION_RESPONSE,
-                       Source<WebContents>(web_contents_));
-    MessageLoop::current()->PostTask(FROM_HERE, new MessageLoop::QuitTask());
-  }
-
-  std::string Run() {
-    // The DOMAutomationController requires an automation ID, eventhough we are
-    // not using it in this case.
-    web_contents_->render_view_host()->ExecuteJavascriptInWebFrame(
-        frame_xpath_, L"window.domAutomationController.setAutomationId(0);");
-
-    web_contents_->render_view_host()->ExecuteJavascriptInWebFrame(frame_xpath_,
-                                                                   jscript_);
-    ui_test_utils::RunMessageLoop();
-    return result_;
-  }
-
- private:
-  WebContents* web_contents_;
-  std::wstring frame_xpath_;
-  std::wstring jscript_;
-  std::string result_;
-
-  DISALLOW_COPY_AND_ASSIGN(JavaScriptRunner);
-};
-
 }  // namespace
 
 IN_PROC_BROWSER_TEST_F(BrowserFocusTest, DISABLED_BrowsersRememberFocus) {
@@ -319,7 +264,7 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, FocusTraversal) {
     // Now let's press tab to move the focus.
     for (int j = 0; j < 7; ++j) {
       // Let's make sure the focus is on the expected element in the page.
-      JavaScriptRunner js_runner(
+      ui_test_utils::JavaScriptRunner js_runner(
           browser()->GetSelectedTabContents()->AsWebContents(),
           L"",
           L"window.domAutomationController.send(getFocusedElement());");
@@ -330,7 +275,7 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, FocusTraversal) {
                                               new MessageLoop::QuitTask());
       ui_test_utils::RunMessageLoop();
       // Ideally, we wouldn't sleep here and instead would use the event
-      // processed ack nofification from the renderer.  I am reluctant to create
+      // processed ack notification from the renderer.  I am reluctant to create
       // a new notification/callback for that purpose just for this test.
       ::Sleep(kActionDelayMs);
     }
@@ -355,7 +300,7 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, FocusTraversal) {
       ::Sleep(kActionDelayMs);
 
       // Let's make sure the focus is on the expected element in the page.
-      JavaScriptRunner js_runner(
+      ui_test_utils::JavaScriptRunner js_runner(
           browser()->GetSelectedTabContents()->AsWebContents(),
           L"",
           L"window.domAutomationController.send(getFocusedElement());");
diff --git a/chrome/browser/views/find_bar_win_unittest.cc b/chrome/browser/views/find_bar_win_unittest.cc
index 097e4f3..4d30d7c 100644
--- a/chrome/browser/views/find_bar_win_unittest.cc
+++ b/chrome/browser/views/find_bar_win_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
@@ -19,6 +19,7 @@ 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";
+const std::wstring kEndState = L"files/find_in_page/end_state.html";
 
 class FindInPageNotificationObserver : public NotificationObserver {
  public:
@@ -78,6 +79,11 @@ typedef enum FindInPageDirection { BACK = 0, FWD = 1 };
 typedef enum FindInPageCase { IGNORE_CASE = 0, CASE_SENSITIVE = 1 };
 
 class FindInPageControllerTest : public InProcessBrowserTest {
+ public:
+  FindInPageControllerTest() {
+    EnableDOMAutomation();
+  }
+
  protected:
   int FindInPage(const std::wstring& search_string,
                  FindInPageDirection forward,
@@ -98,7 +104,7 @@ class FindInPageControllerTest : public InProcessBrowserTest {
   }
 };
 
-// This test loads a page with frames and starts FindInPage requests
+// This test loads a page with frames and starts FindInPage requests.
 IN_PROC_BROWSER_TEST_F(FindInPageControllerTest, FindInPageFrames) {
   HTTPTestServer* server = StartHTTPServer();
 
@@ -141,3 +147,56 @@ IN_PROC_BROWSER_TEST_F(FindInPageControllerTest, FindInPageFrames) {
   EXPECT_EQ(1, FindInPage(L"Hreggvi\u00F0ur", FWD, CASE_SENSITIVE, false));
   EXPECT_EQ(0, FindInPage(L"hreggvi\u00F0ur", FWD, CASE_SENSITIVE, false));
 }
+
+std::string FocusedOnPage(WebContents* web_contents) {
+  ui_test_utils::JavaScriptRunner js_runner(
+      web_contents,
+      L"",
+      L"window.domAutomationController.send(getFocusedElement());");
+  return js_runner.Run();
+}
+
+// This tests the FindInPage end-state, in other words: what is focused when you
+// close the Find box (ie. if you find within a link the link should be
+// focused).
+// TODO(jcampan): This test needs to be enabled once Jay fixes the issues with
+// running two InProc browser tests that both start a web server (crashes).
+IN_PROC_BROWSER_TEST_F(FindInPageControllerTest, DISABLED_FindInPageEndState) {
+  HTTPTestServer* server = StartHTTPServer();
+
+  // First we navigate to our special focus tracking page.
+  GURL url = server->TestServerPageW(kEndState);
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  WebContents* web_contents =
+      browser()->GetSelectedTabContents()->AsWebContents();
+  ASSERT_TRUE(NULL != web_contents);
+
+  // Verify that nothing has focus.
+  ASSERT_STREQ("{nothing focused}", FocusedOnPage(web_contents).c_str());
+
+  // Search for a text that exists within a link on the page.
+  EXPECT_EQ(1, FindInPage(L"nk", FWD, IGNORE_CASE, false));
+
+  // End the find session, which should set focus to the link.
+  web_contents->StopFinding(false);
+
+  // Verify that the link is focused.
+  EXPECT_STREQ("link1", FocusedOnPage(web_contents).c_str());
+
+  // Search for a text that exists within a link on the page.
+  EXPECT_EQ(1, FindInPage(L"Google", FWD, IGNORE_CASE, false));
+
+  // Move the selection to link 1, after searching.
+  ui_test_utils::JavaScriptRunner js_runner(
+      web_contents,
+      L"",
+      L"window.domAutomationController.send(selectLink1());");
+  js_runner.Run();
+
+  // End the find session.
+  web_contents->StopFinding(false);
+
+  // Verify that link2 is not focused.
+  EXPECT_STREQ("", FocusedOnPage(web_contents).c_str());
+}