Fix Q_ASSERT when dragging an item over WebEngineView

When dragging something over a WebEngineView we're waiting for the
result of the asynchronous DragTargetDragOver using a RunLoop. The
result will be delivered by a call to updateDragAction.

The RunLoop will call MessagePumpForUIQt::Run which spins a QEventLoop.
The QEventLoop will dispatch more posted QDragMoveEvent objects while
we're handling the current QDragMoveEvent. This triggers a recursion
guard's Q_ASSERT when dragging from a QtQuick item onto a WebEngineView.

When waiting for the DragTargetDragOver result we're not interested in
Qt events. Instead of using a RunLoop, implement a poor man's chromium
event loop and actively wait for updateDragAction being called.

In practice, no more than two iterations of the loop are run until
updateDragAction is called. Therefore the extra CPU and sleep times are
negligible.

Task-number: QTBUG-58920
Change-Id: Icfdf9c680c4c9987ac3dbb41fbc3e1403af0fa9f
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>

3 files changed, 26 insertions, 62 deletions

diff --git a/src/core/web_contents_adapter.cpp b/src/core/web_contents_adapter.cpp
index 373dc7a5a..cff8a025a 100644
--- a/src/core/web_contents_adapter.cpp
+++ b/src/core/web_contents_adapter.cpp
@@ -86,8 +86,8 @@
 #include <QPageLayout>
 #include <QStringList>
 #include <QStyleHints>
-#include <QTimer>
 #include <QVariant>
+#include <QtCore/qelapsedtimer.h>
 #include <QtCore/qmimedata.h>
 #include <QtGui/qaccessible.h>
 #include <QtGui/qdrag.h>
@@ -337,8 +337,6 @@ WebContentsAdapterPrivate::WebContentsAdapterPrivate()
     , nextRequestId(CallbackDirectory::ReservedCallbackIdsEnd)
     , lastFindRequestId(0)
     , currentDropAction(blink::WebDragOperationNone)
-    , inDragUpdateLoop(false)
-    , updateDragCursorMessagePollingTimer(new QTimer)
 {
 }
 
@@ -381,7 +379,6 @@ WebContentsAdapter::WebContentsAdapter(content::WebContents *webContents)
 {
     Q_D(WebContentsAdapter);
     d->webContents.reset(webContents);
-    initUpdateDragCursorMessagePollingTimer();
 }
 
 WebContentsAdapter::~WebContentsAdapter()
@@ -1232,50 +1229,42 @@ Qt::DropAction WebContentsAdapter::updateDragPosition(QDragMoveEvent *e, const Q
     d->lastDragScreenPos = toGfx(screenPos);
     rvh->DragTargetDragOver(d->lastDragClientPos, d->lastDragScreenPos, toWeb(e->possibleActions()),
                             toWeb(e->mouseButtons()) | toWeb(e->keyboardModifiers()));
-
-    base::MessageLoop *currentMessageLoop = base::MessageLoop::current();
-    DCHECK(currentMessageLoop);
-    if (!currentMessageLoop->NestableTasksAllowed()) {
-        // We're already inside a MessageLoop::RunTask call, and scheduled tasks will not be
-        // executed. That means, updateDragAction will never be called, and the RunLoop below will
-        // remain blocked forever.
-        qWarning("WebContentsAdapter::updateDragPosition called from MessageLoop::RunTask.");
-        return Qt::IgnoreAction;
-    }
-
-    // Wait until we get notified via RenderViewHostDelegateView::UpdateDragCursor. This calls
-    // WebContentsAdapter::updateDragAction that will eventually quit the nested loop.
-    base::RunLoop loop;
-    d->inDragUpdateLoop = true;
-    d->dragUpdateLoopQuitClosure = loop.QuitClosure();
-
-    d->updateDragCursorMessagePollingTimer->start();
-    loop.Run();
-    d->updateDragCursorMessagePollingTimer->stop();
-
+    waitForUpdateDragActionCalled();
     return toQt(d->currentDropAction);
 }
 
-void WebContentsAdapter::updateDragAction(int action)
+void WebContentsAdapter::waitForUpdateDragActionCalled()
 {
     Q_D(WebContentsAdapter);
-    d->currentDropAction = static_cast<blink::WebDragOperation>(action);
-    finishDragUpdate();
+    const qint64 timeout = 3000;
+    QElapsedTimer t;
+    t.start();
+    base::MessagePump::Delegate *delegate = base::MessageLoop::current();
+    DCHECK(delegate);
+    d->updateDragActionCalled = false;
+    for (;;) {
+        while (delegate->DoWork() && !d->updateDragActionCalled) {}
+        if (d->updateDragActionCalled)
+            break;
+        if (t.hasExpired(timeout)) {
+            qWarning("WebContentsAdapter::updateDragAction was not called within %d ms.",
+                     static_cast<int>(timeout));
+            return;
+        }
+        base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(1));
+    }
 }
 
-void WebContentsAdapter::finishDragUpdate()
+void WebContentsAdapter::updateDragAction(int action)
 {
     Q_D(WebContentsAdapter);
-    if (d->inDragUpdateLoop) {
-        d->dragUpdateLoopQuitClosure.Run();
-        d->inDragUpdateLoop = false;
-    }
+    d->updateDragActionCalled = true;
+    d->currentDropAction = static_cast<blink::WebDragOperation>(action);
 }
 
 void WebContentsAdapter::endDragging(const QPoint &clientPos, const QPoint &screenPos)
 {
     Q_D(WebContentsAdapter);
-    finishDragUpdate();
     content::RenderViewHost *rvh = d->webContents->GetRenderViewHost();
     rvh->FilterDropData(d->currentDropData.get());
     d->lastDragClientPos = toGfx(clientPos);
@@ -1287,32 +1276,11 @@ void WebContentsAdapter::endDragging(const QPoint &clientPos, const QPoint &scre
 void WebContentsAdapter::leaveDrag()
 {
     Q_D(WebContentsAdapter);
-    finishDragUpdate();
     content::RenderViewHost *rvh = d->webContents->GetRenderViewHost();
     rvh->DragTargetDragLeave();
     d->currentDropData.reset();
 }
 
-void WebContentsAdapter::initUpdateDragCursorMessagePollingTimer()
-{
-    Q_D(WebContentsAdapter);
-    // Poll for drag cursor updated message 60 times per second. In practice, the timer is fired
-    // at most twice, after which it is stopped.
-    d->updateDragCursorMessagePollingTimer->setInterval(16);
-    d->updateDragCursorMessagePollingTimer->setSingleShot(false);
-
-    QObject::connect(d->updateDragCursorMessagePollingTimer.data(), &QTimer::timeout, [](){
-        base::MessagePump::Delegate *delegate = base::MessageLoop::current();
-        DCHECK(delegate);
-
-        // Execute Chromium tasks if there are any present. Specifically we are interested to handle
-        // the RenderViewHostImpl::OnUpdateDragCursor message, that gets sent from the render
-        // process.
-        while (delegate->DoWork()) {}
-    });
-}
-
-
 void WebContentsAdapter::replaceMisspelling(const QString &word)
 {
 #if defined(ENABLE_SPELLCHECK)
diff --git a/src/core/web_contents_adapter.h b/src/core/web_contents_adapter.h
index 5b2f9a942..803b1eb57 100644
--- a/src/core/web_contents_adapter.h
+++ b/src/core/web_contents_adapter.h
@@ -167,10 +167,8 @@ public:
     void enterDrag(QDragEnterEvent *e, const QPoint &screenPos);
     Qt::DropAction updateDragPosition(QDragMoveEvent *e, const QPoint &screenPos);
     void updateDragAction(int action);
-    void finishDragUpdate();
     void endDragging(const QPoint &clientPos, const QPoint &screenPos);
     void leaveDrag();
-    void initUpdateDragCursorMessagePollingTimer();
     void printToPDF(const QPageLayout&, const QString&);
     quint64 printToPDFCallbackResult(const QPageLayout &, const bool colorMode = true);
 
@@ -186,8 +184,9 @@ public:
 private:
     Q_DISABLE_COPY(WebContentsAdapter)
     Q_DECLARE_PRIVATE(WebContentsAdapter)
-    QScopedPointer<WebContentsAdapterPrivate> d_ptr;
+    void waitForUpdateDragActionCalled();
 
+    QScopedPointer<WebContentsAdapterPrivate> d_ptr;
 };
 
 } // namespace QtWebEngineCore
diff --git a/src/core/web_contents_adapter_p.h b/src/core/web_contents_adapter_p.h
index 9503b4401..f24070523 100644
--- a/src/core/web_contents_adapter_p.h
+++ b/src/core/web_contents_adapter_p.h
@@ -61,7 +61,6 @@
 #include <QScopedPointer>
 #include <QSharedPointer>
 
-QT_FORWARD_DECLARE_CLASS(QTimer)
 QT_FORWARD_DECLARE_CLASS(QWebChannel)
 
 class WebEngineContext;
@@ -96,11 +95,9 @@ public:
     int lastFindRequestId;
     std::unique_ptr<content::DropData> currentDropData;
     blink::WebDragOperation currentDropAction;
-    bool inDragUpdateLoop;
+    bool updateDragActionCalled;
     gfx::Point lastDragClientPos;
     gfx::Point lastDragScreenPos;
-    base::Closure dragUpdateLoopQuitClosure;
-    QScopedPointer<QTimer> updateDragCursorMessagePollingTimer;
 };
 
 } // namespace QtWebEngineCore