Adding an interactive UI test to catch when switching between two tabs (both with FindInPage open) is trashing the Esc handler and causing a crash.

I originally tried an automated_ui test, but was unable to get it to work since Focus changes are involved.

BUG=1303709



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

7 files changed, 175 insertions, 12 deletions

diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
index 5636302..5c10d68 100644
--- a/chrome/browser/automation/automation_provider.cc
+++ b/chrome/browser/automation/automation_provider.cc
@@ -775,6 +775,8 @@ void AutomationProvider::OnMessageReceived(const IPC::Message& message) {
                         AutocompleteEditGetMatches)
     IPC_MESSAGE_HANDLER(AutomationMsg_ConstrainedWindowBoundsRequest,
                         GetConstrainedWindowBounds)
+    IPC_MESSAGE_HANDLER(AutomationMsg_OpenFindInPageRequest,
+                        HandleOpenFindInPageRequest)
   IPC_END_MESSAGE_MAP()
 }
 
@@ -1096,7 +1098,6 @@ void AutomationProvider::GetWindowHWND(const IPC::Message& message,
 void AutomationProvider::ExecuteBrowserCommand(const IPC::Message& message,
                                                int handle,
                                                int command) {
-
   bool success = false;
   if (browser_tracker_->ContainsHandle(handle)) {
     Browser* browser = browser_tracker_->GetResource(handle);
@@ -1694,6 +1695,18 @@ void AutomationProvider::HandleFindInPageRequest(
       false);  // Not a FindNext operation.
 }
 
+void AutomationProvider::HandleOpenFindInPageRequest(
+    const IPC::Message& message, int handle) {
+  if (tab_tracker_->ContainsHandle(handle)) {
+    NavigationController* tab = tab_tracker_->GetResource(handle);
+    Browser* browser = Browser::GetBrowserForController(tab, NULL);
+    if (tab->active_contents()->AsWebContents()) {
+      WebContents* web_contents = tab->active_contents()->AsWebContents();
+      web_contents->OpenFindInPageWindow(*browser);
+    }
+  }
+}
+
 void AutomationProvider::HandleInspectElementRequest(
     const IPC::Message& message, int handle, int x, int y) {
   if (!tab_tracker_->ContainsHandle(handle)) {
diff --git a/chrome/browser/automation/automation_provider.h b/chrome/browser/automation/automation_provider.h
index 85171d3..35827c8 100644
--- a/chrome/browser/automation/automation_provider.h
+++ b/chrome/browser/automation/automation_provider.h
@@ -33,8 +33,8 @@
 // the BrowserProcess, and in particular the NotificationService that's
 // hung off of it.
 
-#ifndef CHROME_BROWSER_AUTOMATION_AUTOMATION_PROVIDER_H__
-#define CHROME_BROWSER_AUTOMATION_AUTOMATION_PROVIDER_H__
+#ifndef CHROME_BROWSER_AUTOMATION_AUTOMATION_PROVIDER_H_
+#define CHROME_BROWSER_AUTOMATION_AUTOMATION_PROVIDER_H_
 
 #include <map>
 #include <string>
@@ -58,7 +58,7 @@ class AutomationProvider : public base::RefCounted<AutomationProvider>,
                            public IPC::Channel::Listener,
                            public IPC::Message::Sender {
  public:
-  AutomationProvider(Profile* profile);
+  explicit AutomationProvider(Profile* profile);
   virtual ~AutomationProvider();
 
   // Establishes a connection to an automation client, if present.
@@ -217,6 +217,10 @@ class AutomationProvider : public base::RefCounted<AutomationProvider>,
                                int forward,
                                int match_case);
 
+  // Responds to requests to open the FindInPage window.
+  void HandleOpenFindInPageRequest(const IPC::Message& message,
+                                   int handle);
+
   // Responds to InspectElement request
   void HandleInspectElementRequest(const IPC::Message& message,
                                    int handle,
@@ -353,10 +357,10 @@ class AutomationProvider : public base::RefCounted<AutomationProvider>,
 };
 
 // When life started, the AutomationProvider class was a singleton and was meant
-// only for UI tests. It had specific behavior (like for example, when the channel
-// was shut down. it closed all open Browsers). The new AutomationProvider serves
-// other purposes than just UI testing. This class is meant to provide the OLD
-// functionality for backward compatibility
+// only for UI tests. It had specific behavior (like for example, when the
+// channel was shut down. it closed all open Browsers). The new
+// AutomationProvider serves other purposes than just UI testing. This class is
+// meant to provide the OLD functionality for backward compatibility
 class TestingAutomationProvider : public AutomationProvider,
                                   public BrowserList::Observer,
                                   public NotificationObserver {
@@ -379,4 +383,4 @@ class TestingAutomationProvider : public AutomationProvider,
                        const NotificationSource& source,
                        const NotificationDetails& details);
 };
-#endif  // CHROME_BROWSER_AUTOMATION_AUTOMATION_PROVIDER_H__
+#endif  // CHROME_BROWSER_AUTOMATION_AUTOMATION_PROVIDER_H_
diff --git a/chrome/browser/find_in_page_controller_interactive_uitest.cc b/chrome/browser/find_in_page_controller_interactive_uitest.cc
new file mode 100644
index 0000000..d1f1847
--- /dev/null
+++ b/chrome/browser/find_in_page_controller_interactive_uitest.cc
@@ -0,0 +1,121 @@
+// Copyright 2008, Google Inc.
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//    * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//    * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//    * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+
+#include "chrome/browser/view_ids.h"
+#include "chrome/views/view.h"
+#include "chrome/test/automation/browser_proxy.h"
+#include "chrome/test/automation/window_proxy.h"
+#include "chrome/test/automation/tab_proxy.h"
+#include "chrome/test/ui/ui_test.h"
+#include "net/url_request/url_request_unittest.h"
+
+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 wchar_t kSimplePage[] = L"404_is_enough_for_us.html";
+
+class FindInPageTest : public UITest {
+ public:
+  FindInPageTest() {
+    show_window_ = true;
+    dom_automation_enabled_ = true;
+  }
+};
+
+// Activate a tab by clicking on it.  Returns true if the call was successful
+// (meaning the messages were correctly sent, but does not guarantee the tab
+// has been changed).
+bool ActivateTabByClick(AutomationProxy* automation,
+                        WindowProxy* browser_window,
+                        int tab_index) {
+  // Click on the tab.
+  gfx::Rect bounds;
+
+  if (!browser_window->GetViewBounds(VIEW_ID_TAB_0 + tab_index, &bounds, true))
+    return false;
+
+  POINT click(bounds.CenterPoint().ToPOINT());
+  if (!browser_window->SimulateOSClick(click,
+                                       ChromeViews::Event::EF_LEFT_BUTTON_DOWN))
+    return false;
+
+  // Wait a bit to let the click be processed.
+  ::Sleep(kActionDelayMs);
+
+  return true;
+}
+
+}  // namespace
+
+TEST_F(FindInPageTest, CrashEscHandlers) {
+  TestServer server(kDocRoot);
+
+  scoped_ptr<WindowProxy> window(automation()->GetActiveWindow());
+  EXPECT_TRUE(window.get() != NULL);
+
+  scoped_ptr<BrowserProxy> browser(automation()->
+      GetBrowserForWindow(window.get()));
+
+  // First we navigate to our test page (tab A).
+  GURL url = server.TestServerPageW(kSimplePage);
+  scoped_ptr<TabProxy> tabA(GetActiveTab());
+  EXPECT_NE(AUTOMATION_MSG_NAVIGATION_ERROR, tabA->NavigateToURL(url));
+
+  EXPECT_TRUE(tabA->OpenFindInPage());
+
+  // Open another tab (tab B).
+  EXPECT_TRUE(browser->AppendTab(url));
+  scoped_ptr<TabProxy> tabB(GetActiveTab());
+
+  EXPECT_TRUE(tabB->OpenFindInPage());
+
+  // Select tab A.
+  EXPECT_TRUE(ActivateTabByClick(automation(), window.get(), 0));
+
+  // Close tab B.
+  EXPECT_TRUE(tabB->Close(true));
+
+  // Click on the location bar so that Find box loses focus.
+  gfx::Rect bounds;
+  EXPECT_TRUE(window->GetViewBounds(VIEW_ID_LOCATION_BAR, &bounds, false));
+  POINT click(bounds.CenterPoint().ToPOINT());
+  EXPECT_TRUE(window->SimulateOSClick(click,
+      ChromeViews::Event::EF_LEFT_BUTTON_DOWN));
+  ::Sleep(kActionDelayMs);
+  int focused_view_id;
+  EXPECT_TRUE(window->GetFocusedViewID(&focused_view_id));
+  EXPECT_EQ(VIEW_ID_LOCATION_BAR, focused_view_id);
+
+  // This used to crash until bug 1303709 was fixed.
+  EXPECT_TRUE(window->SimulateOSKeyPress(VK_ESCAPE, 0));
+  ::Sleep(kActionDelayMs);
+}
diff --git a/chrome/test/automation/automation_messages_internal.h b/chrome/test/automation/automation_messages_internal.h
index f6efda3..bbd1782 100644
--- a/chrome/test/automation/automation_messages_internal.h
+++ b/chrome/test/automation/automation_messages_internal.h
@@ -732,4 +732,8 @@ IPC_BEGIN_MESSAGES(Automation, 0)
   IPC_MESSAGE_ROUTED1(AutomationMsg_WindowExecuteCommandResponse,
                       bool /* success flag */)
 
+  // This message opens the Find window within a tab corresponding to the
+  // supplied tab handle. 
+  IPC_MESSAGE_ROUTED1(AutomationMsg_OpenFindInPageRequest,
+                      int /* tab_handle */)
 IPC_END_MESSAGES(Automation)
diff --git a/chrome/test/automation/tab_proxy.cc b/chrome/test/automation/tab_proxy.cc
index 4fb4432..2cca9a4 100644
--- a/chrome/test/automation/tab_proxy.cc
+++ b/chrome/test/automation/tab_proxy.cc
@@ -92,6 +92,15 @@ bool TabProxy::IsShelfVisible(bool* is_visible) {
   return true;
 }
 
+bool TabProxy::OpenFindInPage() {
+  if (!is_valid())
+    return false;
+
+  return sender_->Send(
+      new AutomationMsg_OpenFindInPageRequest(0, handle_));
+  // This message expects no response.
+}
+
 int TabProxy::FindInPage(const std::wstring& search_string,
                          FindInPageDirection forward,
                          FindInPageCase match_case) {
diff --git a/chrome/test/automation/tab_proxy.h b/chrome/test/automation/tab_proxy.h
index 712ab9c..baa5a0f 100644
--- a/chrome/test/automation/tab_proxy.h
+++ b/chrome/test/automation/tab_proxy.h
@@ -27,8 +27,8 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-#ifndef CHROME_TEST_AUTOMATION_TAB_PROXY_H__
-#define CHROME_TEST_AUTOMATION_TAB_PROXY_H__
+#ifndef CHROME_TEST_AUTOMATION_TAB_PROXY_H_
+#define CHROME_TEST_AUTOMATION_TAB_PROXY_H_
 
 #include <wtypes.h>
 #include <string>
@@ -188,6 +188,10 @@ class TabProxy : public AutomationResourceProxy {
   // unchanged.
   bool IsShelfVisible(bool* is_visible);
 
+  // Opens the FindInPage box. Note: If you just want to search within a tab
+  // you don't need to call this function, just use FindInPage(...) directly.
+  bool OpenFindInPage();
+
   // Starts a search within the current tab. The parameter 'search_string'
   // specifies what string to search for, 'forward' specifies whether to search
   // in forward direction, and 'match_case' specifies case sensitivity
@@ -263,4 +267,4 @@ class TabProxy : public AutomationResourceProxy {
   DISALLOW_EVIL_CONSTRUCTORS(TabProxy);
 };
 
-#endif  // CHROME_TEST_AUTOMATION_TAB_PROXY_H__
+#endif  // CHROME_TEST_AUTOMATION_TAB_PROXY_H_
diff --git a/chrome/test/interactive_ui/interactive_ui.vcproj b/chrome/test/interactive_ui/interactive_ui.vcproj
index 56c649f..b29342a 100644
--- a/chrome/test/interactive_ui/interactive_ui.vcproj
+++ b/chrome/test/interactive_ui/interactive_ui.vcproj
@@ -180,6 +180,14 @@
 			</File>
 		</Filter>
 		<Filter
+			Name="TestFindInPage"
+			>
+			<File
+				RelativePath="..\..\browser\find_in_page_controller_interactive_uitest.cc"
+				>
+			</File>
+		</Filter>
+		<Filter
 			Name="TestFocus"
 			>
 			<File