Bug 131465

Summary: Eliminate DragSession structure
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: WebCore Misc.Assignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch benjamin: review+

Alexey Proskuryakov
Reported 2014-04-09 16:54:29 PDT
It is very difficult to figure out which object is responsible for what in dragging. We have many "session"-like object, including DragController, DragData, DragState and DragSession. Plus a DragOperation enum that could also look like a session object at a first glance. DragSession is the worst - it's really just a transient response to NSDragDestination delegate methods. WebKit2 keeps it as a member of WebPageProxy, but that's only because of its asynchronous nature - we can't apply the response until the next delegate call is made.
Attachments
proposed patch (34.20 KB, patch)
2014-04-09 17:03 PDT, Alexey Proskuryakov
benjamin: review+
Alexey Proskuryakov
Comment 1 2014-04-09 17:03:23 PDT
Created attachment 229001 [details] proposed patch
Benjamin Poulain
Comment 2 2014-04-09 17:09:55 PDT
Comment on attachment 229001 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=229001&action=review Looks great. > Source/WebCore/page/DragController.h:65 > + // As computed from the most recent DragData. Not sure the comment is useful. That is pretty much true for all the state of DragController. > Source/WebKit2/UIProcess/WebPageProxy.h:905 > + void resetCurrentDragInformation() { m_currentDragOperation = WebCore::DragOperationNone; m_currentDragIsOverFileInput = false; m_currentDragNumberOfFilesToBeAccepted = 0; } Let's put this in the cpp file or split it over 3 lines. > Source/WebKit2/UIProcess/API/mac/WKView.mm:2251 > + NSInteger numberOfValidItemsForDrop = _data->_page->currentDragNumberOfFilesToBeAccepted(); No need for cast here? > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2783 > + DragOperation resolvedDragOperation; It may be better to create a scope for both cases and define resolvedDragOperation in there.
Alexey Proskuryakov
Comment 3 2014-04-10 10:34:07 PDT
Alexey Proskuryakov
Comment 4 2014-04-10 10:37:46 PDT
Tried to fix Gtk build in <http://trac.webkit.org/r167075>.
Note You need to log in before you can comment on or make changes to this bug.