diff options
| -rw-r--r-- | chrome/browser/automation/automation_provider.cc | 25 | ||||
| -rw-r--r-- | chrome/browser/automation/automation_provider.h | 11 | ||||
| -rw-r--r-- | chrome/browser/find_in_page_controller_uitest.cc | 59 | ||||
| -rw-r--r-- | chrome/common/ipc_message_utils.h | 25 | ||||
| -rw-r--r-- | chrome/common/render_messages.h | 49 | ||||
| -rw-r--r-- | chrome/common/render_messages_internal.h | 3 | ||||
| -rw-r--r-- | chrome/test/automation/automation_messages_internal.h | 16 | ||||
| -rw-r--r-- | chrome/test/automation/tab_proxy.cc | 13 | ||||
| -rw-r--r-- | chrome/test/automation/tab_proxy.h | 13 | ||||
| -rw-r--r-- | chrome/test/data/find_in_page/crash_1341577.html | 8 |
10 files changed, 143 insertions, 79 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)); +} diff --git a/chrome/common/ipc_message_utils.h b/chrome/common/ipc_message_utils.h index dfc3614..0c6ef8e 100644 --- a/chrome/common/ipc_message_utils.h +++ b/chrome/common/ipc_message_utils.h @@ -15,6 +15,7 @@ #include "chrome/common/thumbnail_score.h" #include "webkit/glue/cache_manager.h" #include "webkit/glue/console_message_level.h" +#include "webkit/glue/find_in_page_request.h" #include "webkit/glue/window_open_disposition.h" // Forward declarations. @@ -1188,6 +1189,30 @@ class MessageWithReply : public SyncMessage { } }; +// Traits for ViewMsg_FindInPageMsg_Request structure to pack/unpack. +template <> +struct ParamTraits<FindInPageRequest> { + typedef FindInPageRequest param_type; + static void Write(Message* m, const param_type& p) { + WriteParam(m, p.request_id); + WriteParam(m, p.search_string); + WriteParam(m, p.forward); + WriteParam(m, p.match_case); + WriteParam(m, p.find_next); + } + static bool Read(const Message* m, void** iter, param_type* p) { + return + ReadParam(m, iter, &p->request_id) && + ReadParam(m, iter, &p->search_string) && + ReadParam(m, iter, &p->forward) && + ReadParam(m, iter, &p->match_case) && + ReadParam(m, iter, &p->find_next); + } + static void Log(const param_type& p, std::wstring* l) { + l->append(L"<FindInPageRequest>"); + } +}; + //----------------------------------------------------------------------------- } // namespace IPC diff --git a/chrome/common/render_messages.h b/chrome/common/render_messages.h index 98eb857..a47b05b 100644 --- a/chrome/common/render_messages.h +++ b/chrome/common/render_messages.h @@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef CHROME_COMMON_RENDER_MESSAGES_H__ -#define CHROME_COMMON_RENDER_MESSAGES_H__ +#ifndef CHROME_COMMON_RENDER_MESSAGES_H_ +#define CHROME_COMMON_RENDER_MESSAGES_H_ #include <string> #include <vector> @@ -284,8 +284,8 @@ struct ViewHostMsg_Resource_Request { }; // Parameters for a resource response header. -struct ViewMsg_Resource_ResponseHead : - webkit_glue::ResourceLoaderBridge::ResponseInfo { +struct ViewMsg_Resource_ResponseHead + : webkit_glue::ResourceLoaderBridge::ResponseInfo { // The response status. URLRequestStatus status; @@ -418,7 +418,7 @@ struct ParamTraits<ResourceType::Type> { } static void Log(const param_type& p, std::wstring* l) { std::wstring type; - switch(p) { + switch (p) { case ResourceType::MAIN_FRAME: type = L"MAIN_FRAME"; break; @@ -455,7 +455,7 @@ struct ParamTraits<FilterPolicy::Type> { } static void Log(const param_type& p, std::wstring* l) { std::wstring type; - switch(p) { + switch (p) { case FilterPolicy::DONT_FILTER: type = L"DONT_FILTER"; break; @@ -489,7 +489,7 @@ struct ParamTraits<ContextNode::Type> { } static void Log(const param_type& p, std::wstring* l) { std::wstring type; - switch(p) { + switch (p) { case WebInputEvent::MOUSE_DOWN: type = L"MOUSE_DOWN"; break; @@ -538,7 +538,7 @@ struct ParamTraits<WebInputEvent::Type> { } static void Log(const param_type& p, std::wstring* l) { std::wstring event; - switch(p) { + switch (p) { case ContextNode::NONE: event = L"NONE"; break; @@ -590,7 +590,7 @@ struct ParamTraits<ViewHostMsg_ImeControl> { } static void Log(const param_type& p, std::wstring* l) { std::wstring control; - switch(p) { + switch (p) { case IME_DISABLE: control = L"IME_DISABLE"; break; @@ -980,30 +980,6 @@ struct ParamTraits<ViewMsg_UploadFile_Params> { } }; -// Traits for ViewMsg_FindInPageMsg_Request structure to pack/unpack. -template <> -struct ParamTraits<FindInPageRequest> { - typedef FindInPageRequest param_type; - static void Write(Message* m, const param_type& p) { - WriteParam(m, p.request_id); - WriteParam(m, p.search_string); - WriteParam(m, p.forward); - WriteParam(m, p.match_case); - WriteParam(m, p.find_next); - } - static bool Read(const Message* m, void** iter, param_type* p) { - return - ReadParam(m, iter, &p->request_id) && - ReadParam(m, iter, &p->search_string) && - ReadParam(m, iter, &p->forward) && - ReadParam(m, iter, &p->match_case) && - ReadParam(m, iter, &p->find_next); - } - static void Log(const param_type& p, std::wstring* l) { - l->append(L"<FindInPageRequest>"); - } -}; - // Traits for net::UploadData::Element. template <> struct ParamTraits<net::UploadData::Element> { @@ -1106,7 +1082,7 @@ struct ParamTraits<NavigationGesture> { } static void Log(const param_type& p, std::wstring* l) { std::wstring event; - switch(p) { + switch (p) { case NavigationGestureUser: event = L"GESTURE_USER"; break; @@ -1192,7 +1168,7 @@ struct ParamTraits<URLRequestStatus> { } static void Log(const param_type& p, std::wstring* l) { std::wstring status; - switch(p.status()) { + switch (p.status()) { case URLRequestStatus::SUCCESS: status = L"SUCCESS"; break; @@ -1530,5 +1506,4 @@ struct ParamTraits<WebDropData> { } // namespace IPC -#endif // CHROME_COMMON_RENDER_MESSAGES_H__ - +#endif // CHROME_COMMON_RENDER_MESSAGES_H_ diff --git a/chrome/common/render_messages_internal.h b/chrome/common/render_messages_internal.h index 29599f1..973c6da 100644 --- a/chrome/common/render_messages_internal.h +++ b/chrome/common/render_messages_internal.h @@ -13,7 +13,6 @@ #include "base/shared_memory.h" #include "chrome/common/ipc_message_macros.h" #include "webkit/glue/dom_operations.h" -#include "webkit/glue/find_in_page_request.h" #include "webkit/glue/console_message_level.h" #include "webkit/glue/context_node_types.h" #include "webkit/glue/webcursor.h" @@ -712,7 +711,7 @@ IPC_BEGIN_MESSAGES(ViewHost, 2) // A message for an external host. // |receiver| can be a receiving script and |message| is any - // arbitrary string that makes sense to the receiver. For + // arbitrary string that makes sense to the receiver. For // example, a user of automation can use it to execute a script // in the form of javascript:receiver("message"); IPC_MESSAGE_ROUTED2(ViewHostMsg_ForwardMessageToExternalHost, diff --git a/chrome/test/automation/automation_messages_internal.h b/chrome/test/automation/automation_messages_internal.h index c04aa65..ca8553a 100644 --- a/chrome/test/automation/automation_messages_internal.h +++ b/chrome/test/automation/automation_messages_internal.h @@ -16,6 +16,7 @@ #include "chrome/common/navigation_types.h" #include "chrome/test/automation/autocomplete_edit_proxy.h" #include "googleurl/src/gurl.h" +#include "webkit/glue/find_in_page_request.h" // NOTE: All IPC messages have either a routing_id of 0 (for asynchronous // messages), or one that's been assigned by the proxy (for calls @@ -309,6 +310,10 @@ IPC_BEGIN_MESSAGES(Automation, 0) // direction (1=forward, 0=back), 'match_case' specifies case sensitivity // (1=case sensitive, 0=case insensitive). If an error occurs, matches_found // will be -1. + // + // NOTE: This message has been deprecated, please use the new message + // AutomationMsg_FindRequest below. + // IPC_MESSAGE_ROUTED4(AutomationMsg_FindInPageRequest, int, /* tab_handle */ std::wstring, /* find_request */ @@ -708,7 +713,7 @@ IPC_BEGIN_MESSAGES(Automation, 0) bool /* success flag */) // This message opens the Find window within a tab corresponding to the - // supplied tab handle. + // supplied tab handle. IPC_MESSAGE_ROUTED1(AutomationMsg_OpenFindInPageRequest, int /* tab_handle */) @@ -725,5 +730,14 @@ IPC_BEGIN_MESSAGES(Automation, 0) std::string /* receiver*/, std::string /* message*/) + // This message starts a find within a tab corresponding to the supplied + // tab handle. The parameter |request| specifies what to search for. + // If an error occurs, |matches_found| will be -1 (see response message + // AutomationMsg_FindInPageResponse). + // + IPC_MESSAGE_ROUTED2(AutomationMsg_FindRequest, + int, /* tab_handle */ + FindInPageRequest /* request */) + IPC_END_MESSAGES(Automation) diff --git a/chrome/test/automation/tab_proxy.cc b/chrome/test/automation/tab_proxy.cc index 2226714..e3529b5 100644 --- a/chrome/test/automation/tab_proxy.cc +++ b/chrome/test/automation/tab_proxy.cc @@ -78,14 +78,21 @@ bool TabProxy::OpenFindInPage() { int TabProxy::FindInPage(const std::wstring& search_string, FindInPageDirection forward, - FindInPageCase match_case) { + FindInPageCase match_case, + bool find_next) { if (!is_valid()) return -1; + FindInPageRequest request = {0}; + request.search_string = search_string; + request.find_next = find_next; + // The explicit comparison to TRUE avoids a warning (C4800). + request.match_case = match_case == TRUE; + request.forward = forward == TRUE; + IPC::Message* response = NULL; bool succeeded = sender_->SendAndWaitForResponse( - new AutomationMsg_FindInPageRequest(0, handle_, search_string, - forward, match_case), + new AutomationMsg_FindRequest(0, handle_, request), &response, AutomationMsg_FindInPageResponse::ID); if (!succeeded) diff --git a/chrome/test/automation/tab_proxy.h b/chrome/test/automation/tab_proxy.h index 3b3bf4f..c1c156b 100644 --- a/chrome/test/automation/tab_proxy.h +++ b/chrome/test/automation/tab_proxy.h @@ -167,12 +167,13 @@ class TabProxy : public AutomationResourceProxy { // 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 - // (true=case sensitive). A return value of -1 indicates failure. + // 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 + // (true=case sensitive). |find_next| specifies whether this is a new search + // or a continuation of the old one. A return value of -1 indicates failure. int FindInPage(const std::wstring& search_string, FindInPageDirection forward, - FindInPageCase match_case); + FindInPageCase match_case, bool find_next); bool GetCookies(const GURL& url, std::string* cookies); bool GetCookieByName(const GURL& url, @@ -246,7 +247,7 @@ class TabProxy : public AutomationResourceProxy { const std::string& message); private: - DISALLOW_EVIL_CONSTRUCTORS(TabProxy); + DISALLOW_COPY_AND_ASSIGN(TabProxy); }; #endif // CHROME_TEST_AUTOMATION_TAB_PROXY_H_ diff --git a/chrome/test/data/find_in_page/crash_1341577.html b/chrome/test/data/find_in_page/crash_1341577.html new file mode 100644 index 0000000..2969109 --- /dev/null +++ b/chrome/test/data/find_in_page/crash_1341577.html @@ -0,0 +1,8 @@ +<html xmlns='http://www.w3.org/1999/xhtml' xmlns:data='http://www.google.com/2005/gml/data'> +<head> + <meta content='text/html; charset=UTF-8' http-equiv='Content-Type'/> +</head> +<body> +തെ പൌരന +</body> +</html>
\ No newline at end of file |