[Webkit-unassigned] [Bug 138468] [GTK] WebProcess crashes when quickly attempting many DnD operations

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=138468

Mario Sanchez Prada <mario at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Missing null-check in       |[GTK] WebProcess crashes
                   |EventHandler::dragSourceEnd |when quickly attempting
                   |edAt()                      |many DnD operations

--- Comment #3 from Mario Sanchez Prada <mario at webkit.org> ---
I'm renaming this issue after the investigation carried out during the Web Engines Hackfest and a few more experiments I run today, since it seems clear now to me that the problem is not in EventHandler itself, but in how the GTK port is using it.

More specifically, it seems that WebCore is only able to handle one DnD for the main frame of the current page at the same time, since the flow looks something like this, for a single DnD operation:

  1. Once you start dragging and the "drag hysteresis" is exceeded, a DataTransfer object is created and the UI Process is notified 
  2. The startDrag() event is handled in the UI Process by WebKitGTK's DragAndDropHandler, which calls into gtk_drag_begin(webView)
  3. Once you stop dragging (button released), "drag-end" signal is emitted for the WebView, calling DragAndDropHandler::finishDrag()
  4. Retrieved all the necessary info about the drag event, a "DragEnded" message is sent to the WebPage in the web process
  5. From WebPage::dragEnded(), m_page->mainFrame().eventHandler().dragSourceEndedAt(event, (DragOperation)operation) is called
  6. Back in EventHandler, dragSourceEndedAt() is executed, which will call dragState().dataTransfer->setDestinationOperation(operation)
  7. That will work as dragState().dataTransfer (created in step 1) will be still valid for the Frame

What I found is that, when doing DnD very fast over the same element (e.g. a link), WebKit2GTK+ would mix things up as it's very likely that more than one call to PageClientImpl::startDrag() happen before the "drag-end" signal is emitted for the first DnD operation, resulting in EventHandler::dragSourceEndedAt() being called over the wrong DataTransfer element for the first time that the DragEnded message is sent:

As a new DnD operation has been generated by the time the first DragEnded message is received back in the WebProcess, the DataTransfer element in there will be associated to the 2nd DnD operation, not the original 1st one. And this, in the worst case scenario, can lead to the Web process to crash if we are "unlucky enough" to have that DataTransfer object already cleared/freed by the time that second DragEnded message is received, which is not an easy to reproduce event, but can happen (and I became pretty good at crashing it).

So, I guess one way to fix this would be to allow the EventHandler handle different DnD operations in a better way, but not sure if it's worth the effort, since it's an unlikely scenario anyway. The other way I can see to fix it is to handle this situation in a better way in the DragAndDropHandler class of the GTK port, so that (perhaps) an "obsolete" DnD operation for a given WebPage is cancelled if a new one arrives before having dealt with that one first.

Opinions?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20151224/6484c912/attachment.html>