From 34800dbe2e85936ae62b5fb4f2cd02c8786a7a9b Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Thu, 17 Feb 2022 13:17:29 -0500 Subject: [PATCH] alloy: Implement Find() using find_in_page::FindTabHelper (fixes issue #3098, see issue #3047) The find behavior should now match Chrome. --- include/capi/cef_browser_capi.h | 19 +++---- include/capi/cef_find_handler_capi.h | 13 ++--- include/cef_api_hash.h | 8 +-- include/cef_browser.h | 19 +++---- include/cef_find_handler.h | 10 ++-- .../browser/alloy/alloy_browser_host_impl.cc | 33 +++++++----- .../browser/alloy/alloy_browser_host_impl.h | 3 +- .../alloy/browser_platform_delegate_alloy.cc | 51 ++++++++++++------- .../alloy/browser_platform_delegate_alloy.h | 20 ++++++-- libcef/browser/browser_platform_delegate.cc | 3 +- libcef/browser/browser_platform_delegate.h | 3 +- .../chrome/chrome_browser_host_impl.cc | 3 +- .../browser/chrome/chrome_browser_host_impl.h | 3 +- libcef_dll/cpptoc/browser_host_cpptoc.cc | 7 ++- libcef_dll/ctocpp/browser_host_ctocpp.cc | 8 ++- libcef_dll/ctocpp/browser_host_ctocpp.h | 5 +- tests/cefclient/browser/root_window_win.cc | 2 +- 17 files changed, 114 insertions(+), 96 deletions(-) diff --git a/include/capi/cef_browser_capi.h b/include/capi/cef_browser_capi.h index 05c46f93e..02f76452f 100644 --- a/include/capi/cef_browser_capi.h +++ b/include/capi/cef_browser_capi.h @@ -33,7 +33,7 @@ // by hand. See the translator.README.txt file in the tools directory for // more information. // -// $hash=8af93d03e0b2a6b50d7612b145599600285b76d4$ +// $hash=b80e84c0039ab45d5c4562d64b67a84766c0dab3$ // #ifndef CEF_INCLUDE_CAPI_CEF_BROWSER_CAPI_H_ @@ -450,18 +450,15 @@ typedef struct _cef_browser_host_t { struct _cef_pdf_print_callback_t* callback); /// - // Search for |searchText|. |identifier| must be a unique ID and these IDs - // must strictly increase so that newer requests always have greater IDs than - // older requests. If |identifier| is zero or less than the previous ID value - // then it will be automatically assigned a new valid ID. |forward| indicates - // whether to search forward or backward within the page. |matchCase| - // indicates whether the search should be case-sensitive. |findNext| indicates - // whether this is the first request or a follow-up. The cef_find_handler_t - // instance, if any, returned via cef_client_t::GetFindHandler will be called - // to report find results. + // Search for |searchText|. |forward| indicates whether to search forward or + // backward within the page. |matchCase| indicates whether the search should + // be case-sensitive. |findNext| indicates whether this is the first request + // or a follow-up. The search will be restarted if |searchText| or |matchCase| + // change. The search will be stopped if |searchText| is NULL. The + // cef_find_handler_t instance, if any, returned via + // cef_client_t::GetFindHandler will be called to report find results. /// void(CEF_CALLBACK* find)(struct _cef_browser_host_t* self, - int identifier, const cef_string_t* searchText, int forward, int matchCase, diff --git a/include/capi/cef_find_handler_capi.h b/include/capi/cef_find_handler_capi.h index c2f3e08aa..fd8a317f6 100644 --- a/include/capi/cef_find_handler_capi.h +++ b/include/capi/cef_find_handler_capi.h @@ -33,7 +33,7 @@ // by hand. See the translator.README.txt file in the tools directory for // more information. // -// $hash=b1fe7f7db5ab92c6ae64dc1288b6fd47c80f9423$ +// $hash=f2e80b8637b07f19adea666e554269de4627e399$ // #ifndef CEF_INCLUDE_CAPI_CEF_FIND_HANDLER_CAPI_H_ @@ -59,11 +59,12 @@ typedef struct _cef_find_handler_t { /// // Called to report find results returned by cef_browser_host_t::find(). - // |identifer| is the identifier passed to find(), |count| is the number of - // matches currently identified, |selectionRect| is the location of where the - // match was found (in window coordinates), |activeMatchOrdinal| is the - // current position in the search results, and |finalUpdate| is true (1) if - // this is the last find notification. + // |identifer| is a unique incremental identifier for the currently active + // search, |count| is the number of matches currently identified, + // |selectionRect| is the location of where the match was found (in window + // coordinates), |activeMatchOrdinal| is the current position in the search + // results, and |finalUpdate| is true (1) if this is the last find + // notification. /// void(CEF_CALLBACK* on_find_result)(struct _cef_find_handler_t* self, struct _cef_browser_t* browser, diff --git a/include/cef_api_hash.h b/include/cef_api_hash.h index 0309e98e1..0c83d6ea0 100644 --- a/include/cef_api_hash.h +++ b/include/cef_api_hash.h @@ -42,13 +42,13 @@ // way that may cause binary incompatibility with other builds. The universal // hash value will change if any platform is affected whereas the platform hash // values will change only if that particular platform is affected. -#define CEF_API_HASH_UNIVERSAL "6f6a9c0f3b420cd3120cf4f5924cbc91f5095abd" +#define CEF_API_HASH_UNIVERSAL "3aa65c82f41a666c0abb6e73d34b3f06ae98f59c" #if defined(OS_WIN) -#define CEF_API_HASH_PLATFORM "5835e67ed251fec96837b475df44248a286e422f" +#define CEF_API_HASH_PLATFORM "f93f9a5edb7ac1c4d17b687e1ed917466874b68a" #elif defined(OS_MAC) -#define CEF_API_HASH_PLATFORM "b8187d9f99b028d767dfd6a40490190c25a3d901" +#define CEF_API_HASH_PLATFORM "99cebe5e20e0d64918d1ac4981a9c019b6358b69" #elif defined(OS_LINUX) -#define CEF_API_HASH_PLATFORM "d7199dfd396052e1517e55f18ea75d3d61336538" +#define CEF_API_HASH_PLATFORM "a069e985567598da26f25b326dc1431191605d9d" #endif #ifdef __cplusplus diff --git a/include/cef_browser.h b/include/cef_browser.h index 0a31de81c..a05961a9d 100644 --- a/include/cef_browser.h +++ b/include/cef_browser.h @@ -479,19 +479,16 @@ class CefBrowserHost : public virtual CefBaseRefCounted { CefRefPtr callback) = 0; /// - // Search for |searchText|. |identifier| must be a unique ID and these IDs - // must strictly increase so that newer requests always have greater IDs than - // older requests. If |identifier| is zero or less than the previous ID value - // then it will be automatically assigned a new valid ID. |forward| indicates - // whether to search forward or backward within the page. |matchCase| - // indicates whether the search should be case-sensitive. |findNext| indicates - // whether this is the first request or a follow-up. The CefFindHandler - // instance, if any, returned via CefClient::GetFindHandler will be called to - // report find results. + // Search for |searchText|. |forward| indicates whether to search forward or + // backward within the page. |matchCase| indicates whether the search should + // be case-sensitive. |findNext| indicates whether this is the first request + // or a follow-up. The search will be restarted if |searchText| or |matchCase| + // change. The search will be stopped if |searchText| is empty. The + // CefFindHandler instance, if any, returned via CefClient::GetFindHandler + // will be called to report find results. /// /*--cef()--*/ - virtual void Find(int identifier, - const CefString& searchText, + virtual void Find(const CefString& searchText, bool forward, bool matchCase, bool findNext) = 0; diff --git a/include/cef_find_handler.h b/include/cef_find_handler.h index a9cac0a1e..7fde59d86 100644 --- a/include/cef_find_handler.h +++ b/include/cef_find_handler.h @@ -50,11 +50,11 @@ class CefFindHandler : public virtual CefBaseRefCounted { public: /// // Called to report find results returned by CefBrowserHost::Find(). - // |identifer| is the identifier passed to Find(), |count| is the number of - // matches currently identified, |selectionRect| is the location of where the - // match was found (in window coordinates), |activeMatchOrdinal| is the - // current position in the search results, and |finalUpdate| is true if this - // is the last find notification. + // |identifer| is a unique incremental identifier for the currently active + // search, |count| is the number of matches currently identified, + // |selectionRect| is the location of where the match was found (in window + // coordinates), |activeMatchOrdinal| is the current position in the search + // results, and |finalUpdate| is true if this is the last find notification. /// /*--cef()--*/ virtual void OnFindResult(CefRefPtr browser, diff --git a/libcef/browser/alloy/alloy_browser_host_impl.cc b/libcef/browser/alloy/alloy_browser_host_impl.cc index c27dfb4d6..b72ca0eeb 100644 --- a/libcef/browser/alloy/alloy_browser_host_impl.cc +++ b/libcef/browser/alloy/alloy_browser_host_impl.cc @@ -9,6 +9,7 @@ #include #include "libcef/browser/alloy/alloy_browser_context.h" +#include "libcef/browser/alloy/browser_platform_delegate_alloy.h" #include "libcef/browser/audio_capturer.h" #include "libcef/browser/browser_context.h" #include "libcef/browser/browser_info.h" @@ -450,21 +451,19 @@ void AlloyBrowserHostImpl::PrintToPDF(const CefString& path, } } -void AlloyBrowserHostImpl::Find(int identifier, - const CefString& searchText, +void AlloyBrowserHostImpl::Find(const CefString& searchText, bool forward, bool matchCase, bool findNext) { if (!CEF_CURRENTLY_ON_UIT()) { CEF_POST_TASK(CEF_UIT, - base::BindOnce(&AlloyBrowserHostImpl::Find, this, identifier, - searchText, forward, matchCase, findNext)); + base::BindOnce(&AlloyBrowserHostImpl::Find, this, searchText, + forward, matchCase, findNext)); return; } if (platform_delegate_) { - platform_delegate_->Find(identifier, searchText, forward, matchCase, - findNext); + platform_delegate_->Find(searchText, forward, matchCase, findNext); } } @@ -860,13 +859,21 @@ void AlloyBrowserHostImpl::FindReply(content::WebContents* web_contents, const gfx::Rect& selection_rect, int active_match_ordinal, bool final_update) { - if (client_.get()) { - CefRefPtr handler = client_->GetFindHandler(); - if (handler.get()) { - CefRect rect(selection_rect.x(), selection_rect.y(), - selection_rect.width(), selection_rect.height()); - handler->OnFindResult(this, request_id, number_of_matches, rect, - active_match_ordinal, final_update); + auto alloy_delegate = + static_cast(platform_delegate()); + if (alloy_delegate->HandleFindReply(request_id, number_of_matches, + selection_rect, active_match_ordinal, + final_update)) { + if (client_) { + if (auto handler = client_->GetFindHandler()) { + const auto& details = alloy_delegate->last_search_result(); + CefRect rect(details.selection_rect().x(), details.selection_rect().y(), + details.selection_rect().width(), + details.selection_rect().height()); + handler->OnFindResult( + this, details.request_id(), details.number_of_matches(), rect, + details.active_match_ordinal(), details.final_update()); + } } } } diff --git a/libcef/browser/alloy/alloy_browser_host_impl.h b/libcef/browser/alloy/alloy_browser_host_impl.h index 421f42c02..ad7aba0bd 100644 --- a/libcef/browser/alloy/alloy_browser_host_impl.h +++ b/libcef/browser/alloy/alloy_browser_host_impl.h @@ -94,8 +94,7 @@ class AlloyBrowserHostImpl : public CefBrowserHostBase, void PrintToPDF(const CefString& path, const CefPdfPrintSettings& settings, CefRefPtr callback) override; - void Find(int identifier, - const CefString& searchText, + void Find(const CefString& searchText, bool forward, bool matchCase, bool findNext) override; diff --git a/libcef/browser/alloy/browser_platform_delegate_alloy.cc b/libcef/browser/alloy/browser_platform_delegate_alloy.cc index fd863c0a5..c0528d071 100644 --- a/libcef/browser/alloy/browser_platform_delegate_alloy.cc +++ b/libcef/browser/alloy/browser_platform_delegate_alloy.cc @@ -18,6 +18,8 @@ #include "base/logging.h" #include "chrome/browser/printing/print_view_manager.h" #include "chrome/browser/ui/prefs/prefs_tab_helper.h" +#include "components/find_in_page/find_tab_helper.h" +#include "components/find_in_page/find_types.h" #include "components/zoom/zoom_controller.h" #include "content/browser/renderer_host/render_widget_host_impl.h" #include "content/browser/web_contents/web_contents_impl.h" @@ -108,6 +110,7 @@ void CefBrowserPlatformDelegateAlloy::WebContentsCreated( content::WebContents* web_contents, bool owned) { CefBrowserPlatformDelegate::WebContentsCreated(web_contents, owned); + find_in_page::FindTabHelper::CreateForWebContents(web_contents); if (owned) { SetOwnedWebContents(web_contents); @@ -387,37 +390,47 @@ void CefBrowserPlatformDelegateAlloy::PrintToPDF( settings, std::move(pdf_callback)); } -void CefBrowserPlatformDelegateAlloy::Find(int identifier, - const CefString& searchText, +void CefBrowserPlatformDelegateAlloy::Find(const CefString& searchText, bool forward, bool matchCase, bool findNext) { if (!web_contents_) return; - // Every find request must have a unique ID and these IDs must strictly - // increase so that newer requests always have greater IDs than older - // requests. - if (identifier <= find_request_id_counter_) - identifier = ++find_request_id_counter_; - else - find_request_id_counter_ = identifier; - - auto options = blink::mojom::FindOptions::New(); - options->forward = forward; - options->match_case = matchCase; - options->find_match = findNext; - web_contents_->Find(identifier, searchText, std::move(options)); + find_in_page::FindTabHelper::FromWebContents(web_contents_) + ->StartFinding(searchText.ToString16(), forward, matchCase, findNext, + /*run_synchronously_for_testing=*/false); } void CefBrowserPlatformDelegateAlloy::StopFinding(bool clearSelection) { if (!web_contents_) return; - content::StopFindAction action = - clearSelection ? content::STOP_FIND_ACTION_CLEAR_SELECTION - : content::STOP_FIND_ACTION_KEEP_SELECTION; - web_contents_->StopFinding(action); + last_search_result_ = find_in_page::FindNotificationDetails(); + find_in_page::FindTabHelper::FromWebContents(web_contents_) + ->StopFinding(clearSelection ? find_in_page::SelectionAction::kClear + : find_in_page::SelectionAction::kKeep); +} + +bool CefBrowserPlatformDelegateAlloy::HandleFindReply( + int request_id, + int number_of_matches, + const gfx::Rect& selection_rect, + int active_match_ordinal, + bool final_update) { + if (!web_contents_) + return false; + + auto find_in_page = + find_in_page::FindTabHelper::FromWebContents(web_contents_); + + find_in_page->HandleFindReply(request_id, number_of_matches, selection_rect, + active_match_ordinal, final_update); + if (!(find_in_page->find_result() == last_search_result_)) { + last_search_result_ = find_in_page->find_result(); + return true; + } + return false; } base::RepeatingClosure diff --git a/libcef/browser/alloy/browser_platform_delegate_alloy.h b/libcef/browser/alloy/browser_platform_delegate_alloy.h index 6a6a66876..72fed13b2 100644 --- a/libcef/browser/alloy/browser_platform_delegate_alloy.h +++ b/libcef/browser/alloy/browser_platform_delegate_alloy.h @@ -10,6 +10,7 @@ #include "libcef/browser/web_contents_dialog_helper.h" #include "base/memory/weak_ptr.h" +#include "components/find_in_page/find_notification_details.h" #include "content/public/browser/web_contents.h" #include "ui/gfx/geometry/size.h" @@ -58,13 +59,23 @@ class CefBrowserPlatformDelegateAlloy : public CefBrowserPlatformDelegate { void PrintToPDF(const CefString& path, const CefPdfPrintSettings& settings, CefRefPtr callback) override; - void Find(int identifier, - const CefString& searchText, + void Find(const CefString& searchText, bool forward, bool matchCase, bool findNext) override; void StopFinding(bool clearSelection) override; + // Called from AlloyBrowserHostImpl::FindReply(). + bool HandleFindReply(int request_id, + int number_of_matches, + const gfx::Rect& selection_rect, + int active_match_ordinal, + bool final_update); + + const find_in_page::FindNotificationDetails& last_search_result() const { + return last_search_result_; + } + protected: CefBrowserPlatformDelegateAlloy(); @@ -96,8 +107,9 @@ class CefBrowserPlatformDelegateAlloy : public CefBrowserPlatformDelegate { // Used for the print preview dialog. std::unique_ptr web_contents_dialog_helper_; - // Used to provide unique incremental IDs for each find request. - int find_request_id_counter_ = 0; + // The last find result. This object contains details about the number of + // matches, the find selection rectangle, etc. + find_in_page::FindNotificationDetails last_search_result_; // Used when the browser is hosting an extension. extensions::ExtensionHost* extension_host_ = nullptr; diff --git a/libcef/browser/browser_platform_delegate.cc b/libcef/browser/browser_platform_delegate.cc index 8c867e3ef..f6698a05c 100644 --- a/libcef/browser/browser_platform_delegate.cc +++ b/libcef/browser/browser_platform_delegate.cc @@ -381,8 +381,7 @@ void CefBrowserPlatformDelegate::PrintToPDF( NOTIMPLEMENTED(); } -void CefBrowserPlatformDelegate::Find(int identifier, - const CefString& searchText, +void CefBrowserPlatformDelegate::Find(const CefString& searchText, bool forward, bool matchCase, bool findNext) { diff --git a/libcef/browser/browser_platform_delegate.h b/libcef/browser/browser_platform_delegate.h index 2974165b4..495ef2a89 100644 --- a/libcef/browser/browser_platform_delegate.h +++ b/libcef/browser/browser_platform_delegate.h @@ -353,8 +353,7 @@ class CefBrowserPlatformDelegate { virtual void PrintToPDF(const CefString& path, const CefPdfPrintSettings& settings, CefRefPtr callback); - virtual void Find(int identifier, - const CefString& searchText, + virtual void Find(const CefString& searchText, bool forward, bool matchCase, bool findNext); diff --git a/libcef/browser/chrome/chrome_browser_host_impl.cc b/libcef/browser/chrome/chrome_browser_host_impl.cc index 28768107e..e0383d190 100644 --- a/libcef/browser/chrome/chrome_browser_host_impl.cc +++ b/libcef/browser/chrome/chrome_browser_host_impl.cc @@ -215,8 +215,7 @@ void ChromeBrowserHostImpl::PrintToPDF( callback->OnPdfPrintFinished(CefString(), false); } -void ChromeBrowserHostImpl::Find(int identifier, - const CefString& searchText, +void ChromeBrowserHostImpl::Find(const CefString& searchText, bool forward, bool matchCase, bool findNext) { diff --git a/libcef/browser/chrome/chrome_browser_host_impl.h b/libcef/browser/chrome/chrome_browser_host_impl.h index 92b5e64b8..a39795e57 100644 --- a/libcef/browser/chrome/chrome_browser_host_impl.h +++ b/libcef/browser/chrome/chrome_browser_host_impl.h @@ -74,8 +74,7 @@ class ChromeBrowserHostImpl : public CefBrowserHostBase { void PrintToPDF(const CefString& path, const CefPdfPrintSettings& settings, CefRefPtr callback) override; - void Find(int identifier, - const CefString& searchText, + void Find(const CefString& searchText, bool forward, bool matchCase, bool findNext) override; diff --git a/libcef_dll/cpptoc/browser_host_cpptoc.cc b/libcef_dll/cpptoc/browser_host_cpptoc.cc index 0f6eba71b..1dde6415d 100644 --- a/libcef_dll/cpptoc/browser_host_cpptoc.cc +++ b/libcef_dll/cpptoc/browser_host_cpptoc.cc @@ -9,7 +9,7 @@ // implementations. See the translator.README.txt file in the tools directory // for more information. // -// $hash=3e851a9007832ba6543257d90cfd8588e3540500$ +// $hash=a45e96e634e88deb54637d0d570a827d49282150$ // #include "libcef_dll/cpptoc/browser_host_cpptoc.h" @@ -424,7 +424,6 @@ browser_host_print_to_pdf(struct _cef_browser_host_t* self, } void CEF_CALLBACK browser_host_find(struct _cef_browser_host_t* self, - int identifier, const cef_string_t* searchText, int forward, int matchCase, @@ -443,8 +442,8 @@ void CEF_CALLBACK browser_host_find(struct _cef_browser_host_t* self, // Execute CefBrowserHostCppToC::Get(self)->Find( - identifier, CefString(searchText), forward ? true : false, - matchCase ? true : false, findNext ? true : false); + CefString(searchText), forward ? true : false, matchCase ? true : false, + findNext ? true : false); } void CEF_CALLBACK browser_host_stop_finding(struct _cef_browser_host_t* self, diff --git a/libcef_dll/ctocpp/browser_host_ctocpp.cc b/libcef_dll/ctocpp/browser_host_ctocpp.cc index 16cbdb668..a4e10169b 100644 --- a/libcef_dll/ctocpp/browser_host_ctocpp.cc +++ b/libcef_dll/ctocpp/browser_host_ctocpp.cc @@ -9,7 +9,7 @@ // implementations. See the translator.README.txt file in the tools directory // for more information. // -// $hash=80f473fbb920009cc911a60c2ed7517eeccad06e$ +// $hash=2edab12ab1759213ab9a6b7620ea39a74291abc7$ // #include "libcef_dll/ctocpp/browser_host_ctocpp.h" @@ -384,8 +384,7 @@ void CefBrowserHostCToCpp::PrintToPDF(const CefString& path, } NO_SANITIZE("cfi-icall") -void CefBrowserHostCToCpp::Find(int identifier, - const CefString& searchText, +void CefBrowserHostCToCpp::Find(const CefString& searchText, bool forward, bool matchCase, bool findNext) { @@ -403,8 +402,7 @@ void CefBrowserHostCToCpp::Find(int identifier, return; // Execute - _struct->find(_struct, identifier, searchText.GetStruct(), forward, matchCase, - findNext); + _struct->find(_struct, searchText.GetStruct(), forward, matchCase, findNext); } NO_SANITIZE("cfi-icall") diff --git a/libcef_dll/ctocpp/browser_host_ctocpp.h b/libcef_dll/ctocpp/browser_host_ctocpp.h index 0479677f8..8cfd9a7f7 100644 --- a/libcef_dll/ctocpp/browser_host_ctocpp.h +++ b/libcef_dll/ctocpp/browser_host_ctocpp.h @@ -9,7 +9,7 @@ // implementations. See the translator.README.txt file in the tools directory // for more information. // -// $hash=90e6257bcf2ef9af754ac47c5948ab8cc3171e21$ +// $hash=6de4205143b6855e7ccf54da14a0494db0b4aaa3$ // #ifndef CEF_LIBCEF_DLL_CTOCPP_BROWSER_HOST_CTOCPP_H_ @@ -64,8 +64,7 @@ class CefBrowserHostCToCpp : public CefCToCppRefCounted callback) override; - void Find(int identifier, - const CefString& searchText, + void Find(const CefString& searchText, bool forward, bool matchCase, bool findNext) override; diff --git a/tests/cefclient/browser/root_window_win.cc b/tests/cefclient/browser/root_window_win.cc index c0341217c..1cb3e8a3c 100644 --- a/tests/cefclient/browser/root_window_win.cc +++ b/tests/cefclient/browser/root_window_win.cc @@ -839,7 +839,7 @@ void RootWindowWin::OnFindEvent() { find_what_last_ = find_buff_; } - browser->GetHost()->Find(0, find_what, + browser->GetHost()->Find(find_what, (find_state_.Flags & FR_DOWN) ? true : false, match_case, find_next_); if (!find_next_)