From: Damjan Jovanovic Subject: [PATCH] winex11.drv: fetch DND data only when requested to by the receiving application Message-Id: Date: Thu, 13 Aug 2015 21:04:59 +0200 Only fetches targets from the drag source when requested to by the drop target. This is more correct as it avoids fetching dangerous targets with side effects such as DELETE, prevents a potential slowdown in DragEnter as all the data is fetched en-masse and similarly reduces memory usage, and only accesses data that the application needs. Also eliminates an unnecessary critical section. Damjan Jovanovic --- dlls/winex11.drv/clipboard.c | 14 ++ dlls/winex11.drv/x11drv.h | 1 + dlls/winex11.drv/xdnd.c | 350 +++++++++++++++++-------------------------- 3 files changed, 150 insertions(+), 215 deletions(-) diff --git a/dlls/winex11.drv/clipboard.c b/dlls/winex11.drv/clipboard.c index 6c6b3b9..e8285b8 100644 --- a/dlls/winex11.drv/clipboard.c +++ b/dlls/winex11.drv/clipboard.c @@ -1694,6 +1694,20 @@ HANDLE X11DRV_CLIPBOARD_ImportSelection(Display *d, Atom target, Window w, Atom /************************************************************************** + * X11DRV_CLIPBOARD_X11AtomToWindowsFormat + * + * Returns the Windows format equivalent to the given X11 atom. + */ +UINT X11DRV_CLIPBOARD_X11AtomToWindowsFormat(Atom x11atom) +{ + WINE_CLIPFORMAT *clipFormat = X11DRV_CLIPBOARD_LookupProperty(NULL, x11atom); + if (clipFormat) + return clipFormat->wFormatID; + return 0; +} + + +/************************************************************************** X11DRV_CLIPBOARD_ExportClipboardData * * Generic export clipboard data routine. diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index cb4b0bb..d03f73b 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -237,6 +237,7 @@ extern void X11DRV_XDND_PositionEvent( HWND hWnd, XClientMessageEvent *event ) D extern void X11DRV_XDND_DropEvent( HWND hWnd, XClientMessageEvent *event ) DECLSPEC_HIDDEN; extern void X11DRV_XDND_LeaveEvent( HWND hWnd, XClientMessageEvent *event ) DECLSPEC_HIDDEN; extern HANDLE X11DRV_CLIPBOARD_ImportSelection(Display *d, Atom target, Window w, Atom prop, UINT *windowsFormat) DECLSPEC_HIDDEN; +extern UINT X11DRV_CLIPBOARD_X11AtomToWindowsFormat(Atom x11atom) DECLSPEC_HIDDEN; /************************************************************************** * X11 GDI driver diff --git a/dlls/winex11.drv/xdnd.c b/dlls/winex11.drv/xdnd.c index 72758a0..f671dce 100644 --- a/dlls/winex11.drv/xdnd.c +++ b/dlls/winex11.drv/xdnd.c @@ -53,16 +53,12 @@ WINE_DEFAULT_DEBUG_CHANNEL(xdnd); #define SELECTION_RETRIES 500 /* wait for .1 seconds */ #define SELECTION_WAIT 1000 /* us */ -typedef struct tagXDNDDATA -{ - int cf_win; - Atom cf_xdnd; - HANDLE contents; - struct list entry; -} XDNDDATA, *LPXDNDDATA; - -static struct list xdndData = LIST_INIT(xdndData); +static Window XDNDDropSourceWindow = None; +static Window XDNDDropTargetWindow = None; +static Atom *XDNDTypeList = NULL; +static unsigned long XDNDTypeListSize = 0; static POINT XDNDxy = { 0, 0 }; +static Time XDNDTimestamp; static IDataObject XDNDDataObject; static BOOL XDNDAccepted = FALSE; static DWORD XDNDDropEffect = DROPEFFECT_NONE; @@ -71,22 +67,10 @@ static HWND XDNDLastTargetWnd; /* might be an ancestor of XDNDLastTargetWnd */ static HWND XDNDLastDropTargetWnd; -static void X11DRV_XDND_InsertXDNDData(int property, int format, HANDLE contents); -static void X11DRV_XDND_ResolveProperty(Display *display, Window xwin, Time tm, - Atom *types, unsigned long count); static BOOL X11DRV_XDND_HasHDROP(void); static HRESULT X11DRV_XDND_SendDropFiles(HWND hwnd); static void X11DRV_XDND_FreeDragDropOp(void); -static CRITICAL_SECTION xdnd_cs; -static CRITICAL_SECTION_DEBUG critsect_debug = -{ - 0, 0, &xdnd_cs, - { &critsect_debug.ProcessLocksList, &critsect_debug.ProcessLocksList }, - 0, 0, { (DWORD_PTR)(__FILE__ ": xdnd_cs") } -}; -static CRITICAL_SECTION xdnd_cs = { &critsect_debug, -1, 0, 0, 0, 0 }; - /* Based on functions in dlls/ole32/ole2.c */ static HANDLE get_droptarget_local_handle(HWND hwnd) @@ -199,8 +183,6 @@ static long X11DRV_XDND_DROPEFFECTToXdndAction(DWORD effect) void X11DRV_XDND_EnterEvent( HWND hWnd, XClientMessageEvent *event ) { int version; - Atom *xdndtypes; - unsigned long count = 0; version = (event->data.l[1] & 0xFF000000) >> 24; TRACE("ver(%d) check-XdndTypeList(%ld) data=%ld,%ld,%ld,%ld,%ld\n", @@ -215,6 +197,8 @@ void X11DRV_XDND_EnterEvent( HWND hWnd, XClientMessageEvent *event ) } XDNDAccepted = FALSE; + XDNDDropSourceWindow = event->data.l[0]; + XDNDDropTargetWindow = event->window; /* If the source supports more than 3 data types we retrieve * the entire list. */ @@ -223,39 +207,61 @@ void X11DRV_XDND_EnterEvent( HWND hWnd, XClientMessageEvent *event ) Atom acttype; int actfmt; unsigned long bytesret; + Atom *xdndtypes = NULL; /* Request supported formats from source window */ XGetWindowProperty(event->display, event->data.l[0], x11drv_atom(XdndTypeList), - 0, 65535, FALSE, AnyPropertyType, &acttype, &actfmt, &count, + 0, 65535, FALSE, AnyPropertyType, &acttype, &actfmt, &XDNDTypeListSize, &bytesret, (unsigned char**)&xdndtypes); + if (xdndtypes) + { + XDNDTypeList = HeapAlloc(GetProcessHeap(), 0, sizeof(Atom)*XDNDTypeListSize); + if (XDNDTypeList) + memcpy(XDNDTypeList, xdndtypes, sizeof(Atom)*XDNDTypeListSize); + else + { + ERR("out of memory for XDNDTypeList\n"); + XDNDTypeListSize = 0; + } + XFree(xdndtypes); + } + else + { + ERR("could not read XdndTypeList property from X11 window 0x%lx\n", XDNDDropSourceWindow); + XDNDTypeListSize = 0; + } } else { - count = 3; - xdndtypes = (Atom*) &event->data.l[2]; + int i; + XDNDTypeListSize = 3; + XDNDTypeList = HeapAlloc(GetProcessHeap(), 0, sizeof(Atom)*XDNDTypeListSize); + if (XDNDTypeList) + { + for (i = 0; i < 3; i++) + XDNDTypeList[i] = event->data.l[2 + i]; + } + else + { + ERR("out of memory for XDNDTypeList\n"); + XDNDTypeListSize = 0; + } } if (TRACE_ON(xdnd)) { unsigned int i; - for (i = 0; i < count; i++) + for (i = 0; i < XDNDTypeListSize; i++) { - if (xdndtypes[i] != 0) + if (XDNDTypeList[i] != 0) { - char * pn = XGetAtomName(event->display, xdndtypes[i]); - TRACE("XDNDEnterAtom %ld: %s\n", xdndtypes[i], pn); + char * pn = XGetAtomName(event->display, XDNDTypeList[i]); + TRACE("XDNDEnterAtom %ld: %s\n", XDNDTypeList[i], pn); XFree(pn); } } } - - /* Do a one-time data read and cache results */ - X11DRV_XDND_ResolveProperty(event->display, event->window, - event->data.l[1], xdndtypes, count); - - if (event->data.l[1] & 1) - XFree(xdndtypes); } /************************************************************************** @@ -273,11 +279,13 @@ void X11DRV_XDND_PositionEvent( HWND hWnd, XClientMessageEvent *event ) HWND targetWindow; HRESULT hr; + XDNDDropSourceWindow = event->data.l[0]; + XDNDDropTargetWindow = event->window; XDNDxy = root_to_virtual_screen( event->data.l[2] >> 16, event->data.l[2] & 0xFFFF ); targetWindow = WindowFromPoint(XDNDxy); - pointl.x = XDNDxy.x; pointl.y = XDNDxy.y; + XDNDTimestamp = event->data.l[3]; effect = X11DRV_XDND_XdndActionToDROPEFFECT(event->data.l[4]); if (!XDNDAccepted || XDNDLastTargetWnd != targetWindow) @@ -386,6 +394,10 @@ void X11DRV_XDND_DropEvent( HWND hWnd, XClientMessageEvent *event ) TRACE("\n"); + XDNDDropSourceWindow = event->data.l[0]; + XDNDDropTargetWindow = event->window; + XDNDTimestamp = event->data.l[2]; + /* Notify OLE of Drop */ dropTarget = get_droptarget_pointer(XDNDLastDropTargetWnd); if (dropTarget) @@ -472,125 +484,18 @@ void X11DRV_XDND_LeaveEvent( HWND hWnd, XClientMessageEvent *event ) /************************************************************************** - * X11DRV_XDND_ResolveProperty - * - * Resolve all MIME types to windows clipboard formats. All data is cached. - */ -static void X11DRV_XDND_ResolveProperty(Display *display, Window xwin, Time tm, - Atom *types, unsigned long count) -{ - unsigned int i, j; - BOOL res; - XEvent xe; - XDNDDATA *current, *next; - BOOL haveHDROP = FALSE; - - TRACE("count(%ld)\n", count); - - X11DRV_XDND_FreeDragDropOp(); /* Clear previously cached data */ - - for (i = 0; i < count; i++) - { - HANDLE contents; - UINT windowsFormat; - - TRACE("requesting atom %ld from xwin %ld\n", types[i], xwin); - - if (types[i] == 0) - continue; - - XConvertSelection(display, x11drv_atom(XdndSelection), types[i], - x11drv_atom(XdndTarget), xwin, /*tm*/CurrentTime); - - /* - * Wait for SelectionNotify - */ - for (j = 0; j < SELECTION_RETRIES; j++) - { - res = XCheckTypedWindowEvent(display, xwin, SelectionNotify, &xe); - if (res && xe.xselection.selection == x11drv_atom(XdndSelection)) break; - - usleep(SELECTION_WAIT); - } - - if (xe.xselection.property == None) - continue; - - contents = X11DRV_CLIPBOARD_ImportSelection(display, types[i], xwin, x11drv_atom(XdndTarget), &windowsFormat); - if (contents) - X11DRV_XDND_InsertXDNDData(types[i], windowsFormat, contents); - } - - /* On Windows when there is a CF_HDROP, there are no other CF_ formats. - * foobar2000 relies on this (spaces -> %20's without it). - */ - LIST_FOR_EACH_ENTRY(current, &xdndData, XDNDDATA, entry) - { - if (current->cf_win == CF_HDROP) - { - haveHDROP = TRUE; - break; - } - } - if (haveHDROP) - { - LIST_FOR_EACH_ENTRY_SAFE(current, next, &xdndData, XDNDDATA, entry) - { - if (current->cf_win != CF_HDROP && current->cf_win < CF_MAX) - { - list_remove(¤t->entry); - GlobalFree(current->contents); - HeapFree(GetProcessHeap(), 0, current); - } - } - } -} - - -/************************************************************************** - * X11DRV_XDND_InsertXDNDData - * - * Cache available XDND property - */ -static void X11DRV_XDND_InsertXDNDData(int property, int format, HANDLE contents) -{ - LPXDNDDATA current = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(XDNDDATA)); - - if (current) - { - EnterCriticalSection(&xdnd_cs); - current->cf_xdnd = property; - current->cf_win = format; - current->contents = contents; - list_add_tail(&xdndData, ¤t->entry); - LeaveCriticalSection(&xdnd_cs); - } -} - - -/************************************************************************** * X11DRV_XDND_HasHDROP */ static BOOL X11DRV_XDND_HasHDROP(void) { - LPXDNDDATA current = NULL; - BOOL found = FALSE; - - EnterCriticalSection(&xdnd_cs); - - /* Find CF_HDROP type if any */ - LIST_FOR_EACH_ENTRY(current, &xdndData, XDNDDATA, entry) + DWORD i; + for (i = 0; i < XDNDTypeListSize; i++) { - if (current->cf_win == CF_HDROP) - { - found = TRUE; - break; - } + UINT windowsFormat = X11DRV_CLIPBOARD_X11AtomToWindowsFormat(XDNDTypeList[i]); + if (windowsFormat == CF_HDROP) + return TRUE; } - - LeaveCriticalSection(&xdnd_cs); - - return found; + return FALSE; } /************************************************************************** @@ -599,50 +504,42 @@ static BOOL X11DRV_XDND_HasHDROP(void) static HRESULT X11DRV_XDND_SendDropFiles(HWND hwnd) { HRESULT hr; - LPXDNDDATA current = NULL; - BOOL found = FALSE; - EnterCriticalSection(&xdnd_cs); - - LIST_FOR_EACH_ENTRY(current, &xdndData, XDNDDATA, entry) - { - if (current->cf_win == CF_HDROP) - { - found = TRUE; - break; - } - } - if (found) + if (X11DRV_XDND_HasHDROP()) { - HGLOBAL dropHandle = GlobalAlloc(GMEM_FIXED, GlobalSize(current->contents)); - if (dropHandle) + FORMATETC formatEtc; + STGMEDIUM stgMedium; + + formatEtc.cfFormat = CF_HDROP; + formatEtc.dwAspect = DVASPECT_CONTENT; + formatEtc.ptd = NULL; + formatEtc.tymed = TYMED_HGLOBAL; + + hr = IDataObject_GetData(&XDNDDataObject, &formatEtc, &stgMedium); + if (SUCCEEDED(hr)) { - DROPFILES *lpDrop = GlobalLock(dropHandle); - memcpy(lpDrop, GlobalLock(current->contents), GlobalSize(current->contents)); - GlobalUnlock(current->contents); + DROPFILES *lpDrop = GlobalLock(stgMedium.u.hGlobal); lpDrop->pt.x = XDNDxy.x; lpDrop->pt.y = XDNDxy.y; lpDrop->fNC = !ScreenToClient(hwnd, &lpDrop->pt); TRACE("Sending WM_DROPFILES: hWnd=0x%p, fNC=%d, x=%d, y=%d, files=%p(%s)\n", hwnd, lpDrop->fNC, lpDrop->pt.x, lpDrop->pt.y, ((char*)lpDrop) + lpDrop->pFiles, debugstr_w((WCHAR*)(((char*)lpDrop) + lpDrop->pFiles))); - GlobalUnlock(dropHandle); - if (PostMessageW(hwnd, WM_DROPFILES, (WPARAM)dropHandle, 0)) + GlobalUnlock(stgMedium.u.hGlobal); + if (PostMessageW(hwnd, WM_DROPFILES, (WPARAM)stgMedium.u.hGlobal, 0)) hr = S_OK; else { hr = HRESULT_FROM_WIN32(GetLastError()); - GlobalFree(dropHandle); + GlobalFree(stgMedium.u.hGlobal); } } else - hr = HRESULT_FROM_WIN32(GetLastError()); + ERR("fetching CF_HDROP data for WM_DROPFILES failed, hr=0x%08x\n", hr); } else hr = E_FAIL; - LeaveCriticalSection(&xdnd_cs); - return hr; } @@ -652,27 +549,15 @@ static HRESULT X11DRV_XDND_SendDropFiles(HWND hwnd) */ static void X11DRV_XDND_FreeDragDropOp(void) { - LPXDNDDATA next; - LPXDNDDATA current; - TRACE("\n"); - EnterCriticalSection(&xdnd_cs); - - /** Free data cache */ - LIST_FOR_EACH_ENTRY_SAFE(current, next, &xdndData, XDNDDATA, entry) - { - list_remove(¤t->entry); - GlobalFree(current->contents); - HeapFree(GetProcessHeap(), 0, current); - } - + HeapFree(GetProcessHeap(), 0, XDNDTypeList); + XDNDTypeList = NULL; + XDNDTypeListSize = 0; XDNDxy.x = XDNDxy.y = 0; XDNDLastTargetWnd = NULL; XDNDLastDropTargetWnd = NULL; XDNDAccepted = FALSE; - - LeaveCriticalSection(&xdnd_cs); } @@ -762,20 +647,46 @@ static HRESULT WINAPI XDNDDATAOBJECT_GetData(IDataObject *dataObject, hr = IDataObject_QueryGetData(dataObject, formatEtc); if (SUCCEEDED(hr)) { - XDNDDATA *current; - LIST_FOR_EACH_ENTRY(current, &xdndData, XDNDDATA, entry) + XEvent xe; + DWORD i; + + for (i = 0; i < XDNDTypeListSize; i++) { - if (current->cf_win == formatEtc->cfFormat) + UINT windowsFormat = X11DRV_CLIPBOARD_X11AtomToWindowsFormat(XDNDTypeList[i]); + if (windowsFormat != 0 && windowsFormat == formatEtc->cfFormat) { - pMedium->tymed = TYMED_HGLOBAL; - pMedium->u.hGlobal = GlobalAlloc(GMEM_FIXED | GMEM_ZEROINIT, GlobalSize(current->contents)); - if (pMedium->u.hGlobal == NULL) + int j; + + XConvertSelection(thread_display(), x11drv_atom(XdndSelection), XDNDTypeList[i], + x11drv_atom(XdndTarget), XDNDDropTargetWindow, XDNDTimestamp); + + /* + * Wait for SelectionNotify + */ + for (j = 0; j < SELECTION_RETRIES; j++) + { + Bool res = XCheckTypedWindowEvent(thread_display(), XDNDDropTargetWindow, SelectionNotify, &xe); + if (res && xe.xselection.selection == x11drv_atom(XdndSelection)) break; + + usleep(SELECTION_WAIT); + } + + if (xe.xselection.property == None) + { + ERR("No SelectionNotify event received\n"); + return E_FAIL; + } + + pMedium->u.hGlobal = X11DRV_CLIPBOARD_ImportSelection(thread_display(), XDNDTypeList[i], + XDNDDropTargetWindow, x11drv_atom(XdndTarget), &windowsFormat); + if (pMedium->u.hGlobal) + { + pMedium->tymed = TYMED_HGLOBAL; + pMedium->pUnkForRelease = 0; + return S_OK; + } + else return E_OUTOFMEMORY; - memcpy(GlobalLock(pMedium->u.hGlobal), GlobalLock(current->contents), GlobalSize(current->contents)); - GlobalUnlock(pMedium->u.hGlobal); - GlobalUnlock(current->contents); - pMedium->pUnkForRelease = 0; - return S_OK; } } } @@ -794,7 +705,7 @@ static HRESULT WINAPI XDNDDATAOBJECT_QueryGetData(IDataObject *dataObject, FORMATETC *formatEtc) { char formatDesc[1024]; - XDNDDATA *current; + DWORD i; TRACE("(%p, %p={.tymed=0x%x, .dwAspect=%d, .cfFormat=%d}\n", dataObject, formatEtc, formatEtc->tymed, formatEtc->dwAspect, formatEtc->cfFormat); @@ -811,9 +722,10 @@ static HRESULT WINAPI XDNDDATAOBJECT_QueryGetData(IDataObject *dataObject, return E_NOTIMPL; } - LIST_FOR_EACH_ENTRY(current, &xdndData, XDNDDATA, entry) + for (i = 0; i < XDNDTypeListSize; i++) { - if (current->cf_win == formatEtc->cfFormat) + UINT windowsFormat = X11DRV_CLIPBOARD_X11AtomToWindowsFormat(XDNDTypeList[i]); + if (windowsFormat != 0 && windowsFormat == formatEtc->cfFormat) { TRACE("application found %s\n", formatDesc); return S_OK; @@ -845,7 +757,8 @@ static HRESULT WINAPI XDNDDATAOBJECT_EnumFormatEtc(IDataObject *dataObject, DWORD dwDirection, IEnumFORMATETC **ppEnumFormatEtc) { - DWORD count; + DWORD count = 0; + DWORD i; FORMATETC *formats; TRACE("(%p, %u, %p)\n", dataObject, dwDirection, ppEnumFormatEtc); @@ -856,21 +769,28 @@ static HRESULT WINAPI XDNDDATAOBJECT_EnumFormatEtc(IDataObject *dataObject, return E_NOTIMPL; } - count = list_count(&xdndData); + for (i = 0; i < XDNDTypeListSize; i++) + { + UINT windowsFormat = X11DRV_CLIPBOARD_X11AtomToWindowsFormat(XDNDTypeList[i]); + if (windowsFormat != 0) + ++count; + } formats = HeapAlloc(GetProcessHeap(), 0, count * sizeof(FORMATETC)); if (formats) { - XDNDDATA *current; - DWORD i = 0; + DWORD next = 0; HRESULT hr; - LIST_FOR_EACH_ENTRY(current, &xdndData, XDNDDATA, entry) + for (i = 0; i < XDNDTypeListSize; i++) { - formats[i].cfFormat = current->cf_win; - formats[i].ptd = NULL; - formats[i].dwAspect = DVASPECT_CONTENT; - formats[i].lindex = -1; - formats[i].tymed = TYMED_HGLOBAL; - i++; + UINT windowsFormat = X11DRV_CLIPBOARD_X11AtomToWindowsFormat(XDNDTypeList[i]); + if (windowsFormat == 0) + continue; + formats[next].cfFormat = windowsFormat; + formats[next].ptd = NULL; + formats[next].dwAspect = DVASPECT_CONTENT; + formats[next].lindex = -1; + formats[next].tymed = TYMED_HGLOBAL; + next++; } hr = SHCreateStdEnumFmtEtc(count, formats, ppEnumFormatEtc); HeapFree(GetProcessHeap(), 0, formats);