Adding a UI test to catch the crash described in issue 1341577.This test is disabled, and will be turned on once we fix the issue.I added to TabProxy the ability to do FindNext, which was necessary to reproduce the crash, and changed the automation IPC to take a FindInPageRequest struct, which makes it identical to the IPC we pass to render_view.BUG=1341577

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

3 files changed, 65 insertions, 30 deletions

diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
index 5451a82..2112e27 100644
--- a/chrome/browser/automation/automation_provider.cc
+++ b/chrome/browser/automation/automation_provider.cc
@@ -178,7 +178,7 @@ class NavigationControllerRestoredObserver : public NotificationObserver {
   NavigationController* controller_;
   const int routing_id_;
 
-  DISALLOW_EVIL_CONSTRUCTORS(NavigationControllerRestoredObserver);
+  DISALLOW_COPY_AND_ASSIGN(NavigationControllerRestoredObserver);
 };
 
 
@@ -576,7 +576,7 @@ AutomationProvider::AutomationProvider(Profile* profile)
 AutomationProvider::~AutomationProvider() {
   // Make sure that any outstanding NotificationObservers also get destroyed.
   ObserverList<NotificationObserver>::Iterator it(notification_observer_list_);
-  NotificationObserver* observer; 
+  NotificationObserver* observer;
   while ((observer = it.GetNext()) != NULL)
     delete observer;
 }
@@ -749,6 +749,8 @@ void AutomationProvider::OnMessageReceived(const IPC::Message& message) {
                         HandleOpenFindInPageRequest)
     IPC_MESSAGE_HANDLER(AutomationMsg_HandleMessageFromExternalHost,
                         OnMessageFromExternalHost)
+    IPC_MESSAGE_HANDLER(AutomationMsg_FindRequest,
+                        HandleFindRequest)
   IPC_END_MESSAGE_MAP()
 }
 
@@ -1161,7 +1163,7 @@ class MouseEventTask : public Task {
   POINT point_;
   int flags_;
 
-  DISALLOW_EVIL_CONSTRUCTORS(MouseEventTask);
+  DISALLOW_COPY_AND_ASSIGN(MouseEventTask);
 };
 
 void AutomationProvider::ScheduleMouseEvent(ChromeViews::View* view,
@@ -1187,7 +1189,7 @@ class InvokeTaskLaterTask : public Task {
  private:
   Task* task_;
 
-  DISALLOW_EVIL_CONSTRUCTORS(InvokeTaskLaterTask);
+  DISALLOW_COPY_AND_ASSIGN(InvokeTaskLaterTask);
 };
 
 // This task sends a WindowDragResponse message with the appropriate
@@ -1208,7 +1210,7 @@ class WindowDragResponseTask : public Task {
   AutomationProvider* provider_;
   int routing_id_;
 
-  DISALLOW_EVIL_CONSTRUCTORS(WindowDragResponseTask);
+  DISALLOW_COPY_AND_ASSIGN(WindowDragResponseTask);
 };
 
 void AutomationProvider::WindowSimulateClick(const IPC::Message& message,
@@ -1658,6 +1660,14 @@ void AutomationProvider::GetConstrainedWindowBounds(const IPC::Message& message,
 void AutomationProvider::HandleFindInPageRequest(
     const IPC::Message& message, int handle, const std::wstring& find_request,
     int forward, int match_case) {
+  NOTREACHED() << "This function has been deprecated."
+    << "Please use HandleFindRequest instead.";
+  Send(new AutomationMsg_FindInPageResponse(message.routing_id(), -1));
+  return;
+}
+
+void AutomationProvider::HandleFindRequest(const IPC::Message& message,
+    int handle, const FindInPageRequest& request) {
   if (!tab_tracker_->ContainsHandle(handle)) {
     Send(new AutomationMsg_FindInPageResponse(message.routing_id(), -1));
     return;
@@ -1677,11 +1687,10 @@ void AutomationProvider::HandleFindInPageRequest(
     tab_contents->AsWebContents()->OpenFindInPageWindow(*browser);
   }
 
-  // The explicit comparison to TRUE avoids a warning (C4800).
   tab_contents->StartFinding(
       FindInPageNotificationObserver::kFindInPageRequestId,
-      find_request, forward == TRUE, match_case == TRUE,
-      false);  // Not a FindNext operation.
+      request.search_string, request.forward, request.match_case,
+      request.find_next);
 }
 
 void AutomationProvider::HandleOpenFindInPageRequest(
diff --git a/chrome/browser/automation/automation_provider.h b/chrome/browser/automation/automation_provider.h
index 6d40253..0b08627 100644
--- a/chrome/browser/automation/automation_provider.h
+++ b/chrome/browser/automation/automation_provider.h
@@ -185,14 +185,19 @@ class AutomationProvider : public base::RefCounted<AutomationProvider>,
   void GetConstrainedWindowBounds(const IPC::Message& message,
                                   int handle);
 
-  // Responds to the FindInPage request, retrieves the search query parameters,
-  // launches an observer to listen for results and issues a StartFind request.
+  // This function has been deprecated, please use HandleFindRequest.
   void HandleFindInPageRequest(const IPC::Message& message,
                                int handle,
                                const std::wstring& find_request,
                                int forward,
                                int match_case);
 
+  // Responds to the FindInPage request, retrieves the search query parameters,
+  // launches an observer to listen for results and issues a StartFind request.
+  void HandleFindRequest(const IPC::Message& message,
+                         int handle,
+                         const FindInPageRequest& request);
+
   // Responds to requests to open the FindInPage window.
   void HandleOpenFindInPageRequest(const IPC::Message& message,
                                    int handle);
@@ -332,7 +337,7 @@ class AutomationProvider : public base::RefCounted<AutomationProvider>,
 
   Profile* profile_;
 
-  DISALLOW_EVIL_CONSTRUCTORS(AutomationProvider);
+  DISALLOW_COPY_AND_ASSIGN(AutomationProvider);
 };
 
 // When life started, the AutomationProvider class was a singleton and was meant
diff --git a/chrome/browser/find_in_page_controller_uitest.cc b/chrome/browser/find_in_page_controller_uitest.cc
index 13dee13..d8e722a 100644
--- a/chrome/browser/find_in_page_controller_uitest.cc
+++ b/chrome/browser/find_in_page_controller_uitest.cc
@@ -16,6 +16,7 @@ class FindInPageControllerTest : public UITest {
 
 const std::wstring kFramePage = L"files/find_in_page/frames.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";
 
 // This test loads a page with frames and starts FindInPage requests
 TEST_F(FindInPageControllerTest, FindInPageFrames) {
@@ -27,39 +28,40 @@ TEST_F(FindInPageControllerTest, FindInPageFrames) {
   ASSERT_TRUE(tab->NavigateToURL(url));
 
   // Try incremental search (mimicking user typing in).
-  EXPECT_EQ(18, tab->FindInPage(L"g",       FWD, IGNORE_CASE));
-  EXPECT_EQ(11, tab->FindInPage(L"go",      FWD, IGNORE_CASE));
-  EXPECT_EQ(04, tab->FindInPage(L"goo",     FWD, IGNORE_CASE));
-  EXPECT_EQ(03, tab->FindInPage(L"goog",    FWD, IGNORE_CASE));
-  EXPECT_EQ(02, tab->FindInPage(L"googl",   FWD, IGNORE_CASE));
-  EXPECT_EQ(01, tab->FindInPage(L"google",  FWD, IGNORE_CASE));
-  EXPECT_EQ(00, tab->FindInPage(L"google!", FWD, IGNORE_CASE));
+  EXPECT_EQ(18, tab->FindInPage(L"g",       FWD, IGNORE_CASE, false));
+  EXPECT_EQ(11, tab->FindInPage(L"go",      FWD, IGNORE_CASE, false));
+  EXPECT_EQ(04, tab->FindInPage(L"goo",     FWD, IGNORE_CASE, false));
+  EXPECT_EQ(03, tab->FindInPage(L"goog",    FWD, IGNORE_CASE, false));
+  EXPECT_EQ(02, tab->FindInPage(L"googl",   FWD, IGNORE_CASE, false));
+  EXPECT_EQ(01, tab->FindInPage(L"google",  FWD, IGNORE_CASE, false));
+  EXPECT_EQ(00, tab->FindInPage(L"google!", FWD, IGNORE_CASE, false));
 
   // Negative test (no matches should be found).
-  EXPECT_EQ(0, tab->FindInPage(L"Non-existing string", FWD, IGNORE_CASE));
+  EXPECT_EQ(0, tab->FindInPage(L"Non-existing string", FWD, IGNORE_CASE,
+                               false));
 
   // 'horse' only exists in the three right frames.
-  EXPECT_EQ(3, tab->FindInPage(L"horse", FWD, IGNORE_CASE));
+  EXPECT_EQ(3, tab->FindInPage(L"horse", FWD, IGNORE_CASE, false));
 
   // 'cat' only exists in the first frame.
-  EXPECT_EQ(1, tab->FindInPage(L"cat", FWD, IGNORE_CASE));
+  EXPECT_EQ(1, tab->FindInPage(L"cat", FWD, IGNORE_CASE, false));
 
   // Try searching again, should still come up with 1 match.
-  EXPECT_EQ(1, tab->FindInPage(L"cat", FWD, IGNORE_CASE));
+  EXPECT_EQ(1, tab->FindInPage(L"cat", FWD, IGNORE_CASE, false));
 
   // Try searching backwards, ignoring case, should still come up with 1 match.
-  EXPECT_EQ(1, tab->FindInPage(L"CAT", BACK, IGNORE_CASE));
+  EXPECT_EQ(1, tab->FindInPage(L"CAT", BACK, IGNORE_CASE, false));
 
   // Try case sensitive, should NOT find it.
-  EXPECT_EQ(0, tab->FindInPage(L"CAT", FWD, CASE_SENSITIVE));
+  EXPECT_EQ(0, tab->FindInPage(L"CAT", FWD, CASE_SENSITIVE, false));
 
   // Try again case sensitive, but this time with right case.
-  EXPECT_EQ(1, tab->FindInPage(L"dog", FWD, CASE_SENSITIVE));
+  EXPECT_EQ(1, tab->FindInPage(L"dog", FWD, CASE_SENSITIVE, false));
 
   // Try non-Latin characters ('Hreggvidur' with 'eth' for 'd' in left frame).
-  EXPECT_EQ(1, tab->FindInPage(L"Hreggvi\u00F0ur", FWD, IGNORE_CASE));
-  EXPECT_EQ(1, tab->FindInPage(L"Hreggvi\u00F0ur", FWD, CASE_SENSITIVE));
-  EXPECT_EQ(0, tab->FindInPage(L"hreggvi\u00F0ur", FWD, CASE_SENSITIVE));
+  EXPECT_EQ(1, tab->FindInPage(L"Hreggvi\u00F0ur", FWD, IGNORE_CASE, false));
+  EXPECT_EQ(1, tab->FindInPage(L"Hreggvi\u00F0ur", FWD, CASE_SENSITIVE, false));
+  EXPECT_EQ(0, tab->FindInPage(L"hreggvi\u00F0ur", FWD, CASE_SENSITIVE, false));
 }
 
 // Load a page with no selectable text and make sure we don't crash.
@@ -70,7 +72,26 @@ TEST_F(FindInPageControllerTest, FindUnSelectableText) {
   scoped_ptr<TabProxy> tab(GetActiveTab());
   ASSERT_TRUE(tab->NavigateToURL(url));
 
-  EXPECT_EQ(0, tab->FindInPage(L"text",                FWD, IGNORE_CASE));
-  EXPECT_EQ(0, tab->FindInPage(L"Non-existing string", FWD, IGNORE_CASE));
+  EXPECT_EQ(0, tab->FindInPage(L"text", FWD, IGNORE_CASE, false));
+  EXPECT_EQ(0, tab->FindInPage(L"Non-existing string", FWD, IGNORE_CASE,
+                               false));
 }
 
+// Try to reproduce the crash seen in issue 1341577.
+TEST_F(FindInPageControllerTest, DISABLED_FindCrash_Issue1341577) {
+  TestServer server(L"chrome/test/data");
+
+  GURL url = server.TestServerPageW(kCrashPage);
+  scoped_ptr<TabProxy> tab(GetActiveTab());
+  ASSERT_TRUE(tab->NavigateToURL(url));
+
+  // This would crash the tab. These must be the first two find requests issued
+  // against the frame, otherwise an active frame pointer is set and it wont
+  // produce the crash.
+  EXPECT_EQ(1, tab->FindInPage(L"\u0D4C", FWD, IGNORE_CASE, false));
+  EXPECT_EQ(1, tab->FindInPage(L"\u0D4C", FWD, IGNORE_CASE, true));  // FindNext
+
+  // This should work fine.
+  EXPECT_EQ(1, tab->FindInPage(L"\u0D24\u0D46", FWD, IGNORE_CASE, false));
+  EXPECT_EQ(0, tab->FindInPage(L"nostring", FWD, IGNORE_CASE, false));
+}