From 55a4c28542c6dc9e4a4edc0aab7043feef2ab0d2 Mon Sep 17 00:00:00 2001 From: Peter Varga Date: Thu, 11 Jul 2019 13:15:39 +0200 Subject: Refactor findText handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move most of the findText logic to the QtWebEngineCore::FindTextHelper class. This change also separates findText callbacks in the new class for getting rid of the request ID conversion and make it easier to remove them in Qt6. Task-number: QTBUG-50420 Change-Id: I348cedd0f90a49f9b360165c46319aeed2c236c0 Reviewed-by: Jüri Valdmann --- src/core/core_chromium.pri | 2 + src/core/find_text_helper.cpp | 165 +++++++++++++++++++++ src/core/find_text_helper.h | 99 +++++++++++++ src/core/web_contents_adapter.cpp | 50 +------ src/core/web_contents_adapter.h | 5 +- src/core/web_contents_adapter_client.h | 1 - src/core/web_contents_delegate_qt.cpp | 19 +-- src/core/web_contents_delegate_qt.h | 9 +- src/webengine/api/qquickwebengineview.cpp | 28 +--- src/webengine/api/qquickwebengineview_p_p.h | 1 - src/webenginewidgets/api/qwebenginepage.cpp | 22 +-- src/webenginewidgets/api/qwebenginepage_p.h | 1 - .../widgets/qwebenginepage/tst_qwebenginepage.cpp | 23 +++ 13 files changed, 319 insertions(+), 106 deletions(-) create mode 100644 src/core/find_text_helper.cpp create mode 100644 src/core/find_text_helper.h diff --git a/src/core/core_chromium.pri b/src/core/core_chromium.pri index bc39f8e15..810ec9a0f 100644 --- a/src/core/core_chromium.pri +++ b/src/core/core_chromium.pri @@ -73,6 +73,7 @@ SOURCES = \ download_manager_delegate_qt.cpp \ favicon_manager.cpp \ file_picker_controller.cpp \ + find_text_helper.cpp \ javascript_dialog_controller.cpp \ javascript_dialog_manager_qt.cpp \ login_delegate_qt.cpp \ @@ -180,6 +181,7 @@ HEADERS = \ download_manager_delegate_qt.h \ favicon_manager.h \ file_picker_controller.h \ + find_text_helper.h \ global_descriptors_qt.h \ javascript_dialog_controller_p.h \ javascript_dialog_controller.h \ diff --git a/src/core/find_text_helper.cpp b/src/core/find_text_helper.cpp new file mode 100644 index 000000000..5fb56dacc --- /dev/null +++ b/src/core/find_text_helper.cpp @@ -0,0 +1,165 @@ +/**************************************************************************** +** +** Copyright (C) 2019 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of the QtWebEngine module of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:LGPL$ +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/contact-us. +** +** GNU Lesser General Public License Usage +** Alternatively, this file may be used under the terms of the GNU Lesser +** General Public License version 3 as published by the Free Software +** Foundation and appearing in the file LICENSE.LGPL3 included in the +** packaging of this file. Please review the following information to +** ensure the GNU Lesser General Public License version 3 requirements +** will be met: https://www.gnu.org/licenses/lgpl-3.0.html. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 2.0 or (at your option) the GNU General +** Public license version 3 or any later version approved by the KDE Free +** Qt Foundation. The licenses are as published by the Free Software +** Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3 +** included in the packaging of this file. Please review the following +** information to ensure the GNU General Public License requirements will +** be met: https://www.gnu.org/licenses/gpl-2.0.html and +** https://www.gnu.org/licenses/gpl-3.0.html. +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +#include "find_text_helper.h" +#include "type_conversion.h" + +#include "content/public/browser/web_contents.h" +#include "third_party/blink/public/mojom/frame/find_in_page.mojom.h" + +namespace QtWebEngineCore { + +// static +int FindTextHelper::m_findRequestIdCounter = -1; + +FindTextHelper::FindTextHelper(content::WebContents *webContents) + : m_webContents(webContents) + , m_currentFindRequestId(m_findRequestIdCounter++) + , m_lastCompletedFindRequestId(m_currentFindRequestId) +{ +} + +FindTextHelper::~FindTextHelper() +{ + if (isFindTextInProgress()) + stopFinding(); +} + +void FindTextHelper::startFinding(const QString &findText, bool caseSensitively, bool findBackward, const QWebEngineCallback resultCallback) +{ + if (findText.isEmpty()) { + stopFinding(); + m_widgetCallbacks.invokeEmpty(resultCallback); + return; + } + + startFinding(findText, caseSensitively, findBackward); + m_widgetCallbacks.registerCallback(m_currentFindRequestId, resultCallback); +} + +void FindTextHelper::startFinding(const QString &findText, bool caseSensitively, bool findBackward, const QJSValue &resultCallback) +{ + if (findText.isEmpty()) { + stopFinding(); + if (!resultCallback.isUndefined()) { + QJSValueList args; + args.append(QJSValue(0)); + const_cast(resultCallback).call(args); + } + return; + } + + startFinding(findText, caseSensitively, findBackward); + if (!resultCallback.isUndefined()) + m_quickCallbacks.insert(m_currentFindRequestId, resultCallback); +} + +void FindTextHelper::startFinding(const QString &findText, bool caseSensitively, bool findBackward) +{ + if (findText.isEmpty()) { + stopFinding(); + return; + } + + if (m_currentFindRequestId > m_lastCompletedFindRequestId) { + // There are cases where the render process will overwrite a previous request + // with the new search and we'll have a dangling callback, leaving the application + // waiting for it forever. + // Assume that any unfinished find has been unsuccessful when a new one is started + // to cover that case. + invokeResultCallback(m_currentFindRequestId, 0); + } + + blink::mojom::FindOptionsPtr options = blink::mojom::FindOptions::New(); + options->forward = !findBackward; + options->match_case = caseSensitively; + options->find_next = findText == m_previousFindText; + m_previousFindText = findText; + + m_currentFindRequestId = m_findRequestIdCounter++; + m_webContents->Find(m_currentFindRequestId, toString16(findText), std::move(options)); +} + +void FindTextHelper::stopFinding() +{ + m_lastCompletedFindRequestId = m_currentFindRequestId; + m_previousFindText = QString(); + m_webContents->StopFinding(content::STOP_FIND_ACTION_KEEP_SELECTION); +} + +bool FindTextHelper::isFindTextInProgress() const +{ + return m_currentFindRequestId != m_lastCompletedFindRequestId; +} + +void FindTextHelper::handleFindReply(content::WebContents *source, int requestId, int numberOfMatches, + const gfx::Rect &selectionRect, int activeMatchOrdinal, bool finalUpdate) +{ + Q_UNUSED(selectionRect); + Q_UNUSED(activeMatchOrdinal); + + Q_ASSERT(source == m_webContents); + + if (!finalUpdate || requestId <= m_lastCompletedFindRequestId) + return; + + Q_ASSERT(m_currentFindRequestId == requestId); + m_lastCompletedFindRequestId = requestId; + invokeResultCallback(requestId, numberOfMatches); +} + +void FindTextHelper::handleLoadCommitted() +{ + // Make sure that we don't set the findNext WebFindOptions on a new frame. + m_previousFindText = QString(); +} + +void FindTextHelper::invokeResultCallback(int requestId, int numberOfMatches) +{ + if (m_quickCallbacks.contains(requestId)) { + QJSValue resultCallback = m_quickCallbacks.take(requestId); + QJSValueList args; + args.append(QJSValue(numberOfMatches)); + resultCallback.call(args); + } else { + m_widgetCallbacks.invoke(requestId, numberOfMatches > 0); + } +} + +} // namespace QtWebEngineCore diff --git a/src/core/find_text_helper.h b/src/core/find_text_helper.h new file mode 100644 index 000000000..17e76ecc7 --- /dev/null +++ b/src/core/find_text_helper.h @@ -0,0 +1,99 @@ +/**************************************************************************** +** +** Copyright (C) 2019 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of the QtWebEngine module of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:LGPL$ +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/contact-us. +** +** GNU Lesser General Public License Usage +** Alternatively, this file may be used under the terms of the GNU Lesser +** General Public License version 3 as published by the Free Software +** Foundation and appearing in the file LICENSE.LGPL3 included in the +** packaging of this file. Please review the following information to +** ensure the GNU Lesser General Public License version 3 requirements +** will be met: https://www.gnu.org/licenses/lgpl-3.0.html. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 2.0 or (at your option) the GNU General +** Public license version 3 or any later version approved by the KDE Free +** Qt Foundation. The licenses are as published by the Free Software +** Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3 +** included in the packaging of this file. Please review the following +** information to ensure the GNU General Public License requirements will +** be met: https://www.gnu.org/licenses/gpl-2.0.html and +** https://www.gnu.org/licenses/gpl-3.0.html. +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +// +// W A R N I N G +// ------------- +// +// This file is not part of the Qt API. It exists purely as an +// implementation detail. This header file may change from version to +// version without notice, or even be removed. +// +// We mean it. +// + +#ifndef FIND_TEXT_HELPER_H +#define FIND_TEXT_HELPER_H + +#include "qtwebenginecoreglobal_p.h" + +#include "qwebenginecallback_p.h" +#include + +namespace content { +class WebContents; +} + +namespace gfx { +class Rect; +} + +namespace QtWebEngineCore { + +class Q_WEBENGINECORE_PRIVATE_EXPORT FindTextHelper { +public: + FindTextHelper(content::WebContents *webContents); + ~FindTextHelper(); + + void startFinding(const QString &findText, bool caseSensitively, bool findBackward, const QWebEngineCallback resultCallback); + void startFinding(const QString &findText, bool caseSensitively, bool findBackward, const QJSValue &resultCallback); + void startFinding(const QString &findText, bool caseSensitively, bool findBackward); + void stopFinding(); + bool isFindTextInProgress() const; + void handleFindReply(content::WebContents *source, int requestId, int numberOfMatches, const gfx::Rect &selectionRect, int activeMatchOrdinal, bool finalUpdate); + void handleLoadCommitted(); + +private: + void invokeResultCallback(int requestId, int numberOfMatches); + + content::WebContents *m_webContents; + + static int m_findRequestIdCounter; + int m_currentFindRequestId; + int m_lastCompletedFindRequestId; + + QString m_previousFindText; + + QMap m_quickCallbacks; + CallbackDirectory m_widgetCallbacks; +}; + +} // namespace QtWebEngineCore + +#endif // FIND_TEXT_HELPER_H diff --git a/src/core/web_contents_adapter.cpp b/src/core/web_contents_adapter.cpp index 2831ec159..85e63c5a4 100644 --- a/src/core/web_contents_adapter.cpp +++ b/src/core/web_contents_adapter.cpp @@ -86,7 +86,6 @@ #include "content/public/common/web_preferences.h" #include "content/public/common/webrtc_ip_handling_policy.h" #include "extensions/buildflags/buildflags.h" -#include "third_party/blink/public/mojom/frame/find_in_page.mojom.h" #include "third_party/blink/public/web/web_media_player_action.h" #include "printing/buildflags/buildflags.h" #include "ui/base/clipboard/clipboard.h" @@ -416,7 +415,6 @@ WebContentsAdapter::WebContentsAdapter() #endif , m_adapterClient(nullptr) , m_nextRequestId(CallbackDirectory::ReservedCallbackIdsEnd) - , m_lastFindRequestId(0) , m_currentDropAction(blink::kWebDragOperationNone) , m_devToolsFrontend(nullptr) { @@ -433,7 +431,6 @@ WebContentsAdapter::WebContentsAdapter(std::unique_ptr web #endif , m_adapterClient(nullptr) , m_nextRequestId(CallbackDirectory::ReservedCallbackIdsEnd) - , m_lastFindRequestId(0) , m_currentDropAction(blink::kWebDragOperationNone) , m_devToolsFrontend(nullptr) { @@ -1046,41 +1043,6 @@ quint64 WebContentsAdapter::fetchDocumentInnerText() return m_nextRequestId++; } -quint64 WebContentsAdapter::findText(const QString &subString, bool caseSensitively, bool findBackward) -{ - CHECK_INITIALIZED(0); - if (m_lastFindRequestId > m_webContentsDelegate->lastReceivedFindReply()) { - // There are cases where the render process will overwrite a previous request - // with the new search and we'll have a dangling callback, leaving the application - // waiting for it forever. - // Assume that any unfinished find has been unsuccessful when a new one is started - // to cover that case. - m_webContentsDelegate->setLastReceivedFindReply(m_lastFindRequestId); - m_adapterClient->didFindText(m_lastFindRequestId, 0); - } - - blink::mojom::FindOptionsPtr options = blink::mojom::FindOptions::New(); - options->forward = !findBackward; - options->match_case = caseSensitively; - options->find_next = subString == m_webContentsDelegate->lastSearchedString(); - m_webContentsDelegate->setLastSearchedString(subString); - - // Find already allows a request ID as input, but only as an int. - // Use the same counter but mod it to MAX_INT, this keeps the same likeliness of request ID clashing. - int shrunkRequestId = m_nextRequestId++ & 0x7fffffff; - m_webContents->Find(shrunkRequestId, toString16(subString), std::move(options)); - m_lastFindRequestId = shrunkRequestId; - return shrunkRequestId; -} - -void WebContentsAdapter::stopFinding() -{ - CHECK_INITIALIZED(); - m_webContentsDelegate->setLastReceivedFindReply(m_lastFindRequestId); - m_webContentsDelegate->setLastSearchedString(QString()); - m_webContents->StopFinding(content::STOP_FIND_ACTION_KEEP_SELECTION); -} - void WebContentsAdapter::updateWebPreferences(const content::WebPreferences & webPreferences) { CHECK_INITIALIZED(); @@ -1696,12 +1658,6 @@ void WebContentsAdapter::focusIfNecessary() m_webContents->Focus(); } -bool WebContentsAdapter::isFindTextInProgress() const -{ - CHECK_INITIALIZED(false); - return m_lastFindRequestId != m_webContentsDelegate->lastReceivedFindReply(); -} - bool WebContentsAdapter::hasFocusedFrame() const { CHECK_INITIALIZED(false); @@ -1745,6 +1701,12 @@ FaviconManager *WebContentsAdapter::faviconManager() return m_webContentsDelegate->faviconManager(); } +FindTextHelper *WebContentsAdapter::findTextHelper() +{ + CHECK_INITIALIZED(nullptr); + return m_webContentsDelegate->findTextHelper(); +} + void WebContentsAdapter::viewSource() { CHECK_INITIALIZED(); diff --git a/src/core/web_contents_adapter.h b/src/core/web_contents_adapter.h index baf9d241c..11f8f9cb1 100644 --- a/src/core/web_contents_adapter.h +++ b/src/core/web_contents_adapter.h @@ -85,6 +85,7 @@ namespace QtWebEngineCore { class DevToolsFrontendQt; class FaviconManager; +class FindTextHelper; class MessagePassingInterface; class ProfileQt; class RenderViewObserverHostQt; @@ -158,8 +159,6 @@ public: quint64 runJavaScriptCallbackResult(const QString &javaScript, quint32 worldId); quint64 fetchDocumentMarkup(); quint64 fetchDocumentInnerText(); - quint64 findText(const QString &subString, bool caseSensitively, bool findBackward); - void stopFinding(); void updateWebPreferences(const content::WebPreferences &webPreferences); void download(const QUrl &url, const QString &suggestedFileName, const QUrl &referrerUrl = QUrl(), @@ -205,6 +204,7 @@ public: void setWebChannel(QWebChannel *, uint worldId); #endif FaviconManager *faviconManager(); + FindTextHelper *findTextHelper(); QPointF lastScrollOffset() const; QSizeF lastContentsSize() const; @@ -263,7 +263,6 @@ private: #endif WebContentsAdapterClient *m_adapterClient; quint64 m_nextRequestId; - int m_lastFindRequestId; std::unique_ptr m_currentDropData; uint m_currentDropAction; bool m_updateDragActionCalled; diff --git a/src/core/web_contents_adapter_client.h b/src/core/web_contents_adapter_client.h index d53568215..4743c1ed7 100644 --- a/src/core/web_contents_adapter_client.h +++ b/src/core/web_contents_adapter_client.h @@ -461,7 +461,6 @@ public: virtual void didRunJavaScript(quint64 requestId, const QVariant& result) = 0; virtual void didFetchDocumentMarkup(quint64 requestId, const QString& result) = 0; virtual void didFetchDocumentInnerText(quint64 requestId, const QString& result) = 0; - virtual void didFindText(quint64 requestId, int matchCount) = 0; virtual void didPrintPage(quint64 requestId, QSharedPointer) = 0; virtual void didPrintPageToPdf(const QString &filePath, bool success) = 0; virtual bool passOnFocus(bool reverse) = 0; diff --git a/src/core/web_contents_delegate_qt.cpp b/src/core/web_contents_delegate_qt.cpp index f4812f9db..e3015d5f6 100644 --- a/src/core/web_contents_delegate_qt.cpp +++ b/src/core/web_contents_delegate_qt.cpp @@ -102,8 +102,8 @@ static WebContentsAdapterClient::JavaScriptConsoleMessageLevel mapToJavascriptCo WebContentsDelegateQt::WebContentsDelegateQt(content::WebContents *webContents, WebContentsAdapterClient *adapterClient) : m_viewClient(adapterClient) - , m_lastReceivedFindReply(0) , m_faviconManager(new FaviconManager(webContents, adapterClient)) + , m_findTextHelper(new FindTextHelper(webContents)) , m_lastLoadProgress(-1) , m_loadingState(determineLoadingState(webContents)) , m_didStartLoadingSeen(m_loadingState == LoadingState::Loading) @@ -350,9 +350,7 @@ void WebContentsDelegateQt::EmitLoadFinished(bool success, const QUrl &url, bool void WebContentsDelegateQt::EmitLoadCommitted() { - // Make sure that we don't set the findNext WebFindOptions on a new frame. - m_lastSearchedString = QString(); - + m_findTextHelper->handleLoadCommitted(); m_viewClient->loadCommitted(); m_viewClient->updateNavigationActions(); } @@ -577,13 +575,7 @@ bool WebContentsDelegateQt::DidAddMessageToConsole(content::WebContents *source, void WebContentsDelegateQt::FindReply(content::WebContents *source, int request_id, int number_of_matches, const gfx::Rect& selection_rect, int active_match_ordinal, bool final_update) { - Q_UNUSED(source) - Q_UNUSED(selection_rect) - Q_UNUSED(active_match_ordinal) - if (final_update && request_id > m_lastReceivedFindReply) { - m_lastReceivedFindReply = request_id; - m_viewClient->didFindText(request_id, number_of_matches); - } + m_findTextHelper->handleFindReply(source, request_id, number_of_matches, selection_rect, active_match_ordinal, final_update); } void WebContentsDelegateQt::RequestMediaAccessPermission(content::WebContents *web_contents, const content::MediaStreamRequest &request, content::MediaResponseCallback callback) @@ -792,6 +784,11 @@ FaviconManager *WebContentsDelegateQt::faviconManager() return m_faviconManager.data(); } +FindTextHelper *WebContentsDelegateQt::findTextHelper() +{ + return m_findTextHelper.data(); +} + WebEngineSettings *WebContentsDelegateQt::webEngineSettings() const { return m_viewClient->webEngineSettings(); } diff --git a/src/core/web_contents_delegate_qt.h b/src/core/web_contents_delegate_qt.h index 00b715c30..f1d5ed76c 100644 --- a/src/core/web_contents_delegate_qt.h +++ b/src/core/web_contents_delegate_qt.h @@ -50,6 +50,7 @@ #include "color_chooser_controller.h" #include "favicon_manager.h" +#include "find_text_helper.h" #include "javascript_dialog_manager_qt.h" #include @@ -112,10 +113,6 @@ class WebContentsDelegateQt : public content::WebContentsDelegate public: WebContentsDelegateQt(content::WebContents*, WebContentsAdapterClient *adapterClient); ~WebContentsDelegateQt(); - QString lastSearchedString() const { return m_lastSearchedString; } - void setLastSearchedString(const QString &s) { m_lastSearchedString = s; } - int lastReceivedFindReply() const { return m_lastReceivedFindReply; } - void setLastReceivedFindReply(int id) { m_lastReceivedFindReply = id; } QUrl url() const { return m_url; } QString title() const { return m_title; } @@ -178,6 +175,7 @@ public: void requestUserNotificationPermission(const QUrl &requestingOrigin); void launchExternalURL(const QUrl &url, ui::PageTransition page_transition, bool is_main_frame, bool has_user_gesture); FaviconManager *faviconManager(); + FindTextHelper *findTextHelper(); void setSavePageInfo(const SavePageInfo &spi) { m_savePageInfo = spi; } const SavePageInfo &savePageInfo() { return m_savePageInfo; } @@ -213,10 +211,9 @@ private: int &streamCount(blink::MediaStreamType type); WebContentsAdapterClient *m_viewClient; - QString m_lastSearchedString; - int m_lastReceivedFindReply; QVector m_loadingErrorFrameList; QScopedPointer m_faviconManager; + QScopedPointer m_findTextHelper; SavePageInfo m_savePageInfo; QSharedPointer m_filePickerController; QUrl m_initialTargetUrl; diff --git a/src/webengine/api/qquickwebengineview.cpp b/src/webengine/api/qquickwebengineview.cpp index 30283dc03..f340ebd33 100644 --- a/src/webengine/api/qquickwebengineview.cpp +++ b/src/webengine/api/qquickwebengineview.cpp @@ -43,6 +43,7 @@ #include "profile_adapter.h" #include "certificate_error_controller.h" #include "file_picker_controller.h" +#include "find_text_helper.h" #include "javascript_dialog_controller.h" #include "touch_selection_menu_controller.h" @@ -164,7 +165,6 @@ QQuickWebEngineViewPrivate::~QQuickWebEngineViewPrivate() { Q_ASSERT(m_profileInitialized); m_profile->d_ptr->removeWebContentsAdapterClient(this); - adapter->stopFinding(); if (faviconProvider) faviconProvider->detach(q_ptr); // q_ptr->d_ptr might be null due to destroy() @@ -273,8 +273,8 @@ void QQuickWebEngineViewPrivate::navigationRequested(int navigationType, const Q Q_EMIT q->navigationRequested(&navigationRequest); navigationRequestAction = navigationRequest.action(); - if ((navigationRequestAction == WebContentsAdapterClient::AcceptRequest) && adapter->isFindTextInProgress()) - adapter->stopFinding(); + if ((navigationRequestAction == WebContentsAdapterClient::AcceptRequest) && adapter->findTextHelper()->isFindTextInProgress()) + adapter->findTextHelper()->stopFinding(); } void QQuickWebEngineViewPrivate::javascriptDialog(QSharedPointer dialog) @@ -1185,14 +1185,6 @@ void QQuickWebEngineViewPrivate::didRunJavaScript(quint64 requestId, const QVari callback.call(args); } -void QQuickWebEngineViewPrivate::didFindText(quint64 requestId, int matchCount) -{ - QJSValue callback = m_callbacks.take(requestId); - QJSValueList args; - args.append(QJSValue(matchCount)); - callback.call(args); -} - void QQuickWebEngineViewPrivate::didPrintPage(quint64 requestId, QSharedPointer result) { Q_Q(QQuickWebEngineView); @@ -1464,18 +1456,8 @@ void QQuickWebEngineView::findText(const QString &subString, FindFlags options, Q_D(QQuickWebEngineView); if (!d->adapter->isInitialized()) return; - if (subString.isEmpty()) { - d->adapter->stopFinding(); - if (!callback.isUndefined()) { - QJSValueList args; - args.append(QJSValue(0)); - const_cast(callback).call(args); - } - } else { - quint64 requestId = d->adapter->findText(subString, options & FindCaseSensitively, options & FindBackward); - if (!callback.isUndefined()) - d->m_callbacks.insert(requestId, callback); - } + + d->adapter->findTextHelper()->startFinding(subString, options & FindCaseSensitively, options & FindBackward, callback); } QQuickWebEngineHistory *QQuickWebEngineView::navigationHistory() const diff --git a/src/webengine/api/qquickwebengineview_p_p.h b/src/webengine/api/qquickwebengineview_p_p.h index e0f2595ec..7a1916d82 100644 --- a/src/webengine/api/qquickwebengineview_p_p.h +++ b/src/webengine/api/qquickwebengineview_p_p.h @@ -133,7 +133,6 @@ public: void didRunJavaScript(quint64, const QVariant&) override; void didFetchDocumentMarkup(quint64, const QString&) override { } void didFetchDocumentInnerText(quint64, const QString&) override { } - void didFindText(quint64, int) override; void didPrintPage(quint64 requestId, QSharedPointer) override; void didPrintPageToPdf(const QString &filePath, bool success) override; bool passOnFocus(bool reverse) override; diff --git a/src/webenginewidgets/api/qwebenginepage.cpp b/src/webenginewidgets/api/qwebenginepage.cpp index e990170eb..29566f021 100644 --- a/src/webenginewidgets/api/qwebenginepage.cpp +++ b/src/webenginewidgets/api/qwebenginepage.cpp @@ -45,6 +45,7 @@ #include "certificate_error_controller.h" #include "color_chooser_controller.h" #include "favicon_manager.h" +#include "find_text_helper.h" #include "file_picker_controller.h" #include "javascript_dialog_controller.h" #if QT_CONFIG(webengine_printing_and_pdf) @@ -420,11 +421,6 @@ void QWebEnginePagePrivate::didFetchDocumentInnerText(quint64 requestId, const Q m_callbacks.invoke(requestId, result); } -void QWebEnginePagePrivate::didFindText(quint64 requestId, int matchCount) -{ - m_callbacks.invoke(requestId, matchCount > 0); -} - void QWebEnginePagePrivate::didPrintPage(quint64 requestId, QSharedPointer result) { #if QT_CONFIG(webengine_printing_and_pdf) @@ -963,7 +959,6 @@ QWebEnginePage::~QWebEnginePage() if (d_ptr) { // d_ptr might be exceptionally null if profile adapter got deleted first setDevToolsPage(nullptr); - d_ptr->adapter->stopFinding(); QWebEnginePagePrivate::bindPageAndView(this, nullptr); QWebEnginePagePrivate::bindPageAndWidget(this, nullptr); } @@ -1592,16 +1587,11 @@ void QWebEnginePage::findText(const QString &subString, FindFlags options, const { Q_D(QWebEnginePage); if (!d->adapter->isInitialized()) { - d->m_callbacks.invokeEmpty(resultCallback); + QtWebEngineCore::CallbackDirectory().invokeEmpty(resultCallback); return; } - if (subString.isEmpty()) { - d->adapter->stopFinding(); - d->m_callbacks.invokeEmpty(resultCallback); - } else { - quint64 requestId = d->adapter->findText(subString, options & FindCaseSensitively, options & FindBackward); - d->m_callbacks.registerCallback(requestId, resultCallback); - } + + d->adapter->findTextHelper()->startFinding(subString, options & FindCaseSensitively, options & FindBackward, resultCallback); } /*! @@ -1652,8 +1642,8 @@ void QWebEnginePagePrivate::navigationRequested(int navigationType, const QUrl & { Q_Q(QWebEnginePage); bool accepted = q->acceptNavigationRequest(url, static_cast(navigationType), isMainFrame); - if (accepted && adapter->isFindTextInProgress()) - adapter->stopFinding(); + if (accepted && adapter->findTextHelper()->isFindTextInProgress()) + adapter->findTextHelper()->stopFinding(); navigationRequestAction = accepted ? WebContentsAdapterClient::AcceptRequest : WebContentsAdapterClient::IgnoreRequest; } diff --git a/src/webenginewidgets/api/qwebenginepage_p.h b/src/webenginewidgets/api/qwebenginepage_p.h index acf95a265..a8cde8199 100644 --- a/src/webenginewidgets/api/qwebenginepage_p.h +++ b/src/webenginewidgets/api/qwebenginepage_p.h @@ -127,7 +127,6 @@ public: void didRunJavaScript(quint64 requestId, const QVariant& result) override; void didFetchDocumentMarkup(quint64 requestId, const QString& result) override; void didFetchDocumentInnerText(quint64 requestId, const QString& result) override; - void didFindText(quint64 requestId, int matchCount) override; void didPrintPage(quint64 requestId, QSharedPointer result) override; void didPrintPageToPdf(const QString &filePath, bool success) override; bool passOnFocus(bool reverse) override; diff --git a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp index 713feca6d..7e9815298 100644 --- a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp +++ b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp @@ -127,6 +127,7 @@ private Q_SLOTS: void findText(); void findTextResult(); void findTextSuccessiveShouldCallAllCallbacks(); + void findTextCalledOnMatch(); void deleteQWebEngineViewTwice(); void loadSignalsOrder_data(); void loadSignalsOrder(); @@ -1024,6 +1025,28 @@ void tst_QWebEnginePage::findTextSuccessiveShouldCallAllCallbacks() QVERIFY(spy5.wasCalled()); } +void tst_QWebEnginePage::findTextCalledOnMatch() +{ + QSignalSpy loadSpy(m_view->page(), &QWebEnginePage::loadFinished); + + // findText will abort in blink if the view has an empty size. + m_view->resize(800, 600); + m_view->show(); + m_view->setHtml(QString("
foo bar
")); + QTRY_COMPARE(loadSpy.count(), 1); + + bool callbackCalled = false; + m_view->page()->findText("foo", 0, [this, &callbackCalled](bool found) { + QVERIFY(found); + + m_view->page()->findText("bar", 0, [&callbackCalled](bool found) { + QVERIFY(found); + callbackCalled = true; + }); + }); + QTRY_VERIFY(callbackCalled); +} + static QWindow *findNewTopLevelWindow(const QWindowList &oldTopLevelWindows) { const auto tlws = QGuiApplication::topLevelWindows(); -- cgit v1.2.1