diff --git a/src/core/web_contents_adapter.cpp b/src/core/web_contents_adapter.cpp index 404af74ff8c03af675af91c4bf9e5fc425415a9f..c765c31ab3d7cfdc6173aec2577fc5cea2b457e9 100644 --- a/src/core/web_contents_adapter.cpp +++ b/src/core/web_contents_adapter.cpp @@ -305,13 +305,15 @@ public: scoped_ptr<WebContentsDelegateQt> webContentsDelegate; scoped_ptr<QtRenderViewObserverHost> renderViewObserverHost; WebContentsAdapterClient *adapterClient; - quint64 lastRequestId; + quint64 nextRequestId; + int lastFindRequestId; }; WebContentsAdapterPrivate::WebContentsAdapterPrivate() // This has to be the first thing we create, and the last we destroy. : engineContext(WebEngineContext::current()) - , lastRequestId(0) + , nextRequestId(1) + , lastFindRequestId(0) { } @@ -645,28 +647,37 @@ quint64 WebContentsAdapter::runJavaScriptCallbackResult(const QString &javaScrip Q_D(WebContentsAdapter); content::RenderViewHost *rvh = d->webContents->GetRenderViewHost(); Q_ASSERT(rvh); - content::RenderFrameHost::JavaScriptResultCallback callback = base::Bind(&callbackOnEvaluateJS, d->adapterClient, ++d->lastRequestId); + content::RenderFrameHost::JavaScriptResultCallback callback = base::Bind(&callbackOnEvaluateJS, d->adapterClient, d->nextRequestId++); rvh->GetMainFrame()->ExecuteJavaScript(toString16(javaScript), callback); - return d->lastRequestId; + return d->nextRequestId; } quint64 WebContentsAdapter::fetchDocumentMarkup() { Q_D(WebContentsAdapter); - d->renderViewObserverHost->fetchDocumentMarkup(++d->lastRequestId); - return d->lastRequestId; + d->renderViewObserverHost->fetchDocumentMarkup(d->nextRequestId++); + return d->nextRequestId; } quint64 WebContentsAdapter::fetchDocumentInnerText() { Q_D(WebContentsAdapter); - d->renderViewObserverHost->fetchDocumentInnerText(++d->lastRequestId); - return d->lastRequestId; + d->renderViewObserverHost->fetchDocumentInnerText(d->nextRequestId++); + return d->nextRequestId; } quint64 WebContentsAdapter::findText(const QString &subString, bool caseSensitively, bool findBackward) { Q_D(WebContentsAdapter); + if (d->lastFindRequestId > d->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. + d->adapterClient->didFindText(d->lastFindRequestId, 0); + } + blink::WebFindOptions options; options.forward = !findBackward; options.matchCase = caseSensitively; @@ -675,8 +686,9 @@ quint64 WebContentsAdapter::findText(const QString &subString, bool caseSensitiv // 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 = ++d->lastRequestId & 0x7fffffff; + int shrunkRequestId = d->nextRequestId++ & 0x7fffffff; d->webContents->Find(shrunkRequestId, toString16(subString), options); + d->lastFindRequestId = shrunkRequestId; return shrunkRequestId; } diff --git a/src/core/web_contents_delegate_qt.cpp b/src/core/web_contents_delegate_qt.cpp index b3c1cd70b5c84f37720227dbc80de95a759629ca..08f11cf5c2c0969279a7c05969a835d855de322a 100644 --- a/src/core/web_contents_delegate_qt.cpp +++ b/src/core/web_contents_delegate_qt.cpp @@ -76,6 +76,7 @@ static WebContentsAdapterClient::JavaScriptConsoleMessageLevel mapToJavascriptCo WebContentsDelegateQt::WebContentsDelegateQt(content::WebContents *webContents, WebContentsAdapterClient *adapterClient) : m_viewClient(adapterClient) + , m_lastReceivedFindReply(0) { webContents->SetDelegate(this); Observe(webContents); @@ -239,8 +240,10 @@ void WebContentsDelegateQt::FindReply(content::WebContents *source, int request_ Q_UNUSED(source) Q_UNUSED(selection_rect) Q_UNUSED(active_match_ordinal) - if (final_update) + if (final_update) { + m_lastReceivedFindReply = request_id; m_viewClient->didFindText(request_id, number_of_matches); + } } void WebContentsDelegateQt::RequestMediaAccessPermission(content::WebContents *web_contents, const content::MediaStreamRequest &request, const content::MediaResponseCallback &callback) diff --git a/src/core/web_contents_delegate_qt.h b/src/core/web_contents_delegate_qt.h index e4bc143c14f72d21c6de8b3e432d9a23af6ce6de..8e1761a0a10b61611623ab1c680520bc7ae721b4 100644 --- a/src/core/web_contents_delegate_qt.h +++ b/src/core/web_contents_delegate_qt.h @@ -65,6 +65,7 @@ public: WebContentsDelegateQt(content::WebContents*, WebContentsAdapterClient *adapterClient); QString lastSearchedString() const { return m_lastSearchedString; } void setLastSearchedString(const QString &s) { m_lastSearchedString = s; } + int lastReceivedFindReply() const { return m_lastReceivedFindReply; } virtual content::WebContents *OpenURLFromTab(content::WebContents *source, const content::OpenURLParams ¶ms) Q_DECL_OVERRIDE; virtual void NavigationStateChanged(const content::WebContents* source, unsigned changed_flags) Q_DECL_OVERRIDE; @@ -92,6 +93,7 @@ public: private: WebContentsAdapterClient *m_viewClient; QString m_lastSearchedString; + int m_lastReceivedFindReply; }; #endif // WEB_CONTENTS_DELEGATE_QT_H diff --git a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp index ca3740715eae46d0511820f6ea330c082ef612ea..70fbba128935102bb16b4edce28a89d10282d103 100644 --- a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp +++ b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp @@ -175,6 +175,7 @@ private Q_SLOTS: void testStopScheduledPageRefresh(); void findText(); void findTextResult(); + void findTextSuccessiveShouldCallAllCallbacks(); void supportedContentType(); // [Qt] tst_QWebEnginePage::infiniteLoopJS() timeouts with DFG JIT // https://bugs.webkit.org/show_bug.cgi?id=79040 @@ -3254,6 +3255,29 @@ void tst_QWebEnginePage::findTextResult() QCOMPARE(findTextSync(m_page, ""), false); } +void tst_QWebEnginePage::findTextSuccessiveShouldCallAllCallbacks() +{ + CallbackSpy<bool> spy1; + CallbackSpy<bool> spy2; + CallbackSpy<bool> spy3; + CallbackSpy<bool> spy4; + CallbackSpy<bool> spy5; + QSignalSpy loadSpy(m_view, SIGNAL(loadFinished(bool))); + m_view->setHtml(QString("<html><head></head><body><div>abcdefg abcdefg abcdefg abcdefg abcdefg</div></body></html>")); + QTRY_COMPARE(loadSpy.count(), 1); + m_page->findText("abcde", 0, spy1.ref()); + m_page->findText("abcd", 0, spy2.ref()); + m_page->findText("abc", 0, spy3.ref()); + m_page->findText("ab", 0, spy4.ref()); + m_page->findText("a", 0, spy5.ref()); + spy5.waitForResult(); + QVERIFY(spy1.wasCalled()); + QVERIFY(spy2.wasCalled()); + QVERIFY(spy3.wasCalled()); + QVERIFY(spy4.wasCalled()); + QVERIFY(spy5.wasCalled()); +} + static QString getMimeTypeForExtension(const QString &ext) { QMimeType mimeType = QMimeDatabase().mimeTypeForFile(QStringLiteral("filename.") + ext.toLower(), QMimeDatabase::MatchExtension);