From: Damjan Jovanovic Subject: [PATCH 3/7] winex11.drv: fetch DND data only when requested to by the receiving application (try 3) Message-Id: Date: Sat, 18 Jul 2015 10:42:53 +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 only accesses data that the application needs. Also eliminates an unnecessary critical section. Try 2 and 3: no changes Damjan Jovanovic --- dlls/winex11.drv/clipboard.c | 14 ++ dlls/winex11.drv/x11drv.h | 1 + dlls/winex11.drv/xdnd.c | 324 ++++++++++++++++++------------------------- 3 files changed, 149 insertions(+), 190 deletions(-) diff --git a/dlls/winex11.drv/clipboard.c b/dlls/winex11.drv/clipboard.c index eb5c567..1aaf0e6 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_WindowsFormatToX11Atom * * Returns the X11 atom equivalent to the given windows format. diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index ad5b766..7135c73 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -244,6 +244,7 @@ extern void X11DRV_XDND_SelectionRequest(XSelectionRequestEvent *event) DECLSPEC extern HANDLE X11DRV_CLIPBOARD_ImportSelection(Display *d, Atom target, Window w, Atom prop, UINT *windowsFormat) DECLSPEC_HIDDEN; extern HANDLE X11DRV_CLIPBOARD_ExportSelection(UINT windowsFormat, HGLOBAL hGlobal, Atom *atom, DWORD *cBytes) DECLSPEC_HIDDEN; extern Atom X11DRV_CLIPBOARD_WindowsFormatToX11Atom(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 87fc641..5ec6b6b 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,21 +67,9 @@ 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 void 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) @@ -198,8 +182,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", @@ -214,6 +196,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. */ @@ -222,39 +206,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("XDNDEnter atom %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); } /************************************************************************** @@ -272,11 +278,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) @@ -378,6 +386,10 @@ void X11DRV_XDND_DropEvent( HWND hWnd, XClientMessageEvent *event ) TRACE("\n"); + XDNDDropSourceWindow = event->data.l[0]; + XDNDDropTargetWindow = event->window; + XDNDTimestamp = event->data.l[2]; + /* If we have a HDROP type we send a WM_ACCEPTFILES.*/ if (GetWindowLongW( hWnd, GWL_EXSTYLE ) & WS_EX_ACCEPTFILES) X11DRV_XDND_SendDropFiles( hWnd ); @@ -452,116 +464,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_SendDropFiles */ static void X11DRV_XDND_SendDropFiles(HWND hwnd) { - LPXDNDDATA current = NULL; + DWORD i; BOOL found = FALSE; - EnterCriticalSection(&xdnd_cs); - /* Find CF_HDROP type if any */ - LIST_FOR_EACH_ENTRY(current, &xdndData, XDNDDATA, entry) + for (i = 0; i < XDNDTypeListSize; i++) { - if (current->cf_win == CF_HDROP) + UINT windowsFormat = X11DRV_CLIPBOARD_X11AtomToWindowsFormat(XDNDTypeList[i]); + if (windowsFormat == CF_HDROP) { found = TRUE; break; @@ -570,26 +484,33 @@ static void X11DRV_XDND_SendDropFiles(HWND hwnd) if (found) { - HGLOBAL dropHandle = GlobalAlloc(GMEM_FIXED, GlobalSize(current->contents)); + HRESULT hr; + FORMATETC formatEtc; + STGMEDIUM stgMedium; + + formatEtc.cfFormat = CF_HDROP; + formatEtc.dwAspect = DVASPECT_CONTENT; + formatEtc.ptd = NULL; + formatEtc.tymed = TYMED_HGLOBAL; - if (dropHandle) + 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)) - GlobalFree(dropHandle); + GlobalUnlock(stgMedium.u.hGlobal); + + if (!PostMessageW(hwnd, WM_DROPFILES, (WPARAM)stgMedium.u.hGlobal, 0)) + GlobalFree(stgMedium.u.hGlobal); } + else + ERR("fetching CF_HDROP data for WM_DROPFILES failed, hr=0x%08x\n", hr); } - - LeaveCriticalSection(&xdnd_cs); } @@ -598,27 +519,15 @@ static void 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); } @@ -708,20 +617,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; } } } @@ -740,7 +675,7 @@ static HRESULT WINAPI XDNDDATAOBJECT_QueryGetData(IDataObject *dataObject, FORMATETC *formatEtc) { char formatDesc[1024]; - XDNDDATA *current; + int i; TRACE("(%p, %p={.tymed=0x%x, .dwAspect=%d, .cfFormat=%d}\n", dataObject, formatEtc, formatEtc->tymed, formatEtc->dwAspect, formatEtc->cfFormat); @@ -757,9 +692,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; @@ -791,7 +727,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); @@ -802,21 +739,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);