[PATCH] ole32: Use an object for Regsitered DragDrop windows. (resend)

[Alistair Leslie-Hughes]

The BCG library registers just about every window it creates for Drag and Drop.
The current method creates a file for each window registered for DnD, and in the case
of a large application that uses BCG, 1000's of files can be created.  Reducing the
amount of file an application can use.

The new way, uses an unique guid for each regsitered window and then use that guid
to lookup whether it supports Drag and Drop.

Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com>
---
 dlls/ole32/ole2.c       | 183 ++++++++++++----------------------------
 dlls/winex11.drv/xdnd.c |  68 +++++----------
 2 files changed, 73 insertions(+), 178 deletions(-)

diff --git a/dlls/ole32/ole2.c b/dlls/ole32/ole2.c
index 6653fac342..531085739f 100644
--- a/dlls/ole32/ole2.c
+++ b/dlls/ole32/ole2.c
@@ -114,10 +114,13 @@ static const WCHAR prop_olemenuW[] =
 static const WCHAR prop_oledroptarget[] =
   {'O','l','e','D','r','o','p','T','a','r','g','e','t','I','n','t','e','r','f','a','c','e',0};
 
-/* property to store Marshalled IDropTarget pointer */
+/* property to store Marshalled Atom value */
 static const WCHAR prop_marshalleddroptarget[] =
   {'W','i','n','e','M','a','r','s','h','a','l','l','e','d','D','r','o','p','T','a','r','g','e','t',0};
 
+static const WCHAR prop_marshalledcookie[] =
+  {'W','i','n','e','M','a','r','s','h','a','l','l','e','d','C','o','o','k','i','e',0};
+
 static const WCHAR emptyW[] = { 0 };
 
 /******************************************************************************
@@ -276,9 +279,9 @@ HRESULT WINAPI OleInitializeWOW(DWORD x, DWORD y) {
  * This handle belongs to the process that called RegisterDragDrop.
  * See get_droptarget_local_handle().
  */
-static inline HANDLE get_droptarget_handle(HWND hwnd)
+static inline ATOM get_droptarget_handle(HWND hwnd)
 {
-    return GetPropW(hwnd, prop_marshalleddroptarget);
+    return HandleToULong(GetPropW(hwnd, prop_marshalleddroptarget));
 }
 
 /*************************************************************
@@ -291,91 +294,6 @@ static inline BOOL is_droptarget(HWND hwnd)
     return get_droptarget_handle(hwnd) != 0;
 }
 
-/*************************************************************
- *           get_droptarget_local_handle
- *
- * Retrieve a handle to the map containing the marshalled IDropTarget.
- * The handle should be closed when finished with.
- */
-static HANDLE get_droptarget_local_handle(HWND hwnd)
-{
-    HANDLE handle, local_handle = 0;
-
-    handle = get_droptarget_handle(hwnd);
-
-    if(handle)
-    {
-        DWORD pid;
-        HANDLE process;
-
-        GetWindowThreadProcessId(hwnd, &pid);
-        process = OpenProcess(PROCESS_DUP_HANDLE, FALSE, pid);
-        if(process)
-        {
-            DuplicateHandle(process, handle, GetCurrentProcess(), &local_handle, 0, FALSE, DUPLICATE_SAME_ACCESS);
-            CloseHandle(process);
-        }
-    }
-    return local_handle;
-}
-
-/***********************************************************************
- *     create_map_from_stream
- *
- * Helper for RegisterDragDrop.  Creates a file mapping object
- * with the contents of the provided stream.  The stream must
- * be a global memory backed stream.
- */
-static HRESULT create_map_from_stream(IStream *stream, HANDLE *map)
-{
-    HGLOBAL hmem;
-    DWORD size;
-    HRESULT hr;
-    void *data;
-
-    hr = GetHGlobalFromStream(stream, &hmem);
-    if(FAILED(hr)) return hr;
-
-    size = GlobalSize(hmem);
-    *map = CreateFileMappingW(INVALID_HANDLE_VALUE, NULL, PAGE_READWRITE, 0, size, NULL);
-    if(!*map) return E_OUTOFMEMORY;
-
-    data = MapViewOfFile(*map, FILE_MAP_WRITE, 0, 0, size);
-    memcpy(data, GlobalLock(hmem), size);
-    GlobalUnlock(hmem);
-    UnmapViewOfFile(data);
-    return S_OK;
-}
-
-/***********************************************************************
- *     create_stream_from_map
- *
- * Creates a stream from the provided map.
- */
-static HRESULT create_stream_from_map(HANDLE map, IStream **stream)
-{
-    HRESULT hr = E_OUTOFMEMORY;
-    HGLOBAL hmem;
-    void *data;
-    MEMORY_BASIC_INFORMATION info;
-
-    data = MapViewOfFile(map, FILE_MAP_READ, 0, 0, 0);
-    if(!data) return hr;
-
-    VirtualQuery(data, &info, sizeof(info));
-    TRACE("size %d\n", (int)info.RegionSize);
-
-    hmem = GlobalAlloc(GMEM_MOVEABLE, info.RegionSize);
-    if(hmem)
-    {
-        memcpy(GlobalLock(hmem), data, info.RegionSize);
-        GlobalUnlock(hmem);
-        hr = CreateStreamOnHGlobal(hmem, TRUE, stream);
-    }
-    UnmapViewOfFile(data);
-    return hr;
-}
-
 /* This is to work around apps which break COM rules by not implementing
  * IDropTarget::QueryInterface().  Windows doesn't expose this because it
  * doesn't call CoMarshallInterface() in RegisterDragDrop().
@@ -528,18 +446,29 @@ static IDropTarget* WrapDropTarget( HWND hwnd )
 static IDropTarget* get_droptarget_pointer(HWND hwnd)
 {
     IDropTarget *droptarget = NULL;
-    HANDLE map;
-    IStream *stream;
+    ATOM atom;
+    WCHAR guid_str[40];
+    GUID guid;
+    HRESULT hr;
+
+    atom = get_droptarget_handle(hwnd);
+    if (!atom) return NULL;
 
-    map = get_droptarget_local_handle(hwnd);
-    if(!map) return NULL;
+    if (GlobalGetAtomNameW(atom, guid_str, ARRAY_SIZE(guid_str)) == 0)
+        return NULL;
 
-    if(SUCCEEDED(create_stream_from_map(map, &stream)))
+    if (CLSIDFromString(guid_str, &guid) != S_OK)
     {
-        CoUnmarshalInterface(stream, &IID_IDropTarget, (void**)&droptarget);
-        IStream_Release(stream);
+        ERR("Failed to convert string %s\n", debugstr_w(guid_str));
+        return NULL;
     }
-    CloseHandle(map);
+
+    hr = CoGetClassObject(&guid, CLSCTX_SERVER, NULL, &IID_IDropTarget, (void**)&droptarget);
+    if (FAILED(hr))
+    {
+        WARN("CoGetClassObject failed (0x%08x)\n", hr);
+    }
+
     return droptarget;
 }
 
@@ -550,9 +479,9 @@ HRESULT WINAPI RegisterDragDrop(HWND hwnd, LPDROPTARGET pDropTarget)
 {
   DWORD pid = 0;
   HRESULT hr;
-  IStream *stream;
-  HANDLE map;
   IDropTarget *wrapper;
+  GUID guid;
+  DWORD cookie;
 
   TRACE("(%p,%p)\n", hwnd, pDropTarget);
 
@@ -590,37 +519,38 @@ HRESULT WINAPI RegisterDragDrop(HWND hwnd, LPDROPTARGET pDropTarget)
    * "OleDropTargetInterface" prop for compatibility with Windows.
    */
 
-  hr = CreateStreamOnHGlobal(NULL, TRUE, &stream);
-  if(FAILED(hr)) return hr;
+  CoCreateGuid(&guid);
 
   /* IDropTarget::QueryInterface() shouldn't be called, some (broken) apps depend on this. */
   wrapper = WrapDropTarget( hwnd );
   if(!wrapper)
   {
-    IStream_Release(stream);
     return E_OUTOFMEMORY;
   }
-  hr = CoMarshalInterface(stream, &IID_IDropTarget, (IUnknown*)wrapper, MSHCTX_LOCAL, NULL, MSHLFLAGS_TABLESTRONG);
+
+  hr = CoRegisterClassObject(&guid, (IUnknown*)wrapper, CLSCTX_SERVER, REGCLS_MULTIPLEUSE,
+          &cookie);
+  if (FAILED(hr))
+      ERR("Failed to register class 0x%08x\n", hr);
   IDropTarget_Release(wrapper);
 
   if(SUCCEEDED(hr))
   {
-    hr = create_map_from_stream(stream, &map);
-    if(SUCCEEDED(hr))
-    {
-      IDropTarget_AddRef(pDropTarget);
-      SetPropW(hwnd, prop_oledroptarget, pDropTarget);
-      SetPropW(hwnd, prop_marshalleddroptarget, map);
-    }
-    else
-    {
-      LARGE_INTEGER zero;
-      zero.QuadPart = 0;
-      IStream_Seek(stream, zero, STREAM_SEEK_SET, NULL);
-      CoReleaseMarshalData(stream);
-    }
+    WCHAR *guid_str;
+    ATOM atom;
+
+    IDropTarget_AddRef(pDropTarget);
+    SetPropW(hwnd, prop_oledroptarget, pDropTarget);
+
+    StringFromCLSID(&guid, &guid_str);
+
+    atom = GlobalAddAtomW(guid_str);
+
+    SetPropW(hwnd, prop_marshalleddroptarget, ULongToHandle(atom));
+    SetPropW(hwnd, prop_marshalledcookie, ULongToHandle(cookie));
+
+    CoTaskMemFree(guid_str);
   }
-  IStream_Release(stream);
 
   return hr;
 }
@@ -630,10 +560,9 @@ HRESULT WINAPI RegisterDragDrop(HWND hwnd, LPDROPTARGET pDropTarget)
  */
 HRESULT WINAPI RevokeDragDrop(HWND hwnd)
 {
-  HANDLE map;
-  IStream *stream;
+  ATOM atom;
   IDropTarget *drop_target;
-  HRESULT hr;
+  DWORD cookie;
 
   TRACE("(%p)\n", hwnd);
 
@@ -644,24 +573,20 @@ HRESULT WINAPI RevokeDragDrop(HWND hwnd)
   }
 
   /* no registration data */
-  if (!(map = get_droptarget_handle(hwnd)))
+  if (!(atom = get_droptarget_handle(hwnd)))
     return DRAGDROP_E_NOTREGISTERED;
 
   drop_target = GetPropW(hwnd, prop_oledroptarget);
   if(drop_target) IDropTarget_Release(drop_target);
+  cookie = HandleToULong(GetPropW(hwnd, prop_marshalledcookie));
 
   RemovePropW(hwnd, prop_oledroptarget);
   RemovePropW(hwnd, prop_marshalleddroptarget);
 
-  hr = create_stream_from_map(map, &stream);
-  if(SUCCEEDED(hr))
-  {
-      CoReleaseMarshalData(stream);
-      IStream_Release(stream);
-  }
-  CloseHandle(map);
+  CoRevokeClassObject(cookie);
+  GlobalDeleteAtom(atom);
 
-  return hr;
+  return S_OK;
 }
 
 /***********************************************************************
diff --git a/dlls/winex11.drv/xdnd.c b/dlls/winex11.drv/xdnd.c
index 0cd2ad8892..0e61ecd15d 100644
--- a/dlls/winex11.drv/xdnd.c
+++ b/dlls/winex11.drv/xdnd.c
@@ -85,69 +85,39 @@ 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)
+static ATOM get_droptarget_local_handle(HWND hwnd)
 {
     static const WCHAR prop_marshalleddroptarget[] =
         {'W','i','n','e','M','a','r','s','h','a','l','l','e','d','D','r','o','p','T','a','r','g','e','t',0};
-    HANDLE handle;
-    HANDLE local_handle = 0;
-
-    handle = GetPropW(hwnd, prop_marshalleddroptarget);
-    if (handle)
-    {
-        DWORD pid;
-        HANDLE process;
-
-        GetWindowThreadProcessId(hwnd, &pid);
-        process = OpenProcess(PROCESS_DUP_HANDLE, FALSE, pid);
-        if (process)
-        {
-            DuplicateHandle(process, handle, GetCurrentProcess(), &local_handle, 0, FALSE, DUPLICATE_SAME_ACCESS);
-            CloseHandle(process);
-        }
-    }
-    return local_handle;
+    return HandleToULong(GetPropW(hwnd, prop_marshalleddroptarget));
 }
 
-static HRESULT create_stream_from_map(HANDLE map, IStream **stream)
+static IDropTarget* get_droptarget_pointer(HWND hwnd)
 {
-    HRESULT hr = E_OUTOFMEMORY;
-    HGLOBAL hmem;
-    void *data;
-    MEMORY_BASIC_INFORMATION info;
+    IDropTarget *droptarget = NULL;
+    ATOM atom;
+    WCHAR guid_str[40];
+    GUID guid;
+    HRESULT hr;
 
-    data = MapViewOfFile(map, FILE_MAP_READ, 0, 0, 0);
-    if(!data) return hr;
+    atom = get_droptarget_local_handle(hwnd);
+    if (!atom) return NULL;
 
-    VirtualQuery(data, &info, sizeof(info));
-    TRACE("size %d\n", (int)info.RegionSize);
+    if (GlobalGetAtomNameW(atom, guid_str, ARRAY_SIZE(guid_str)) == 0)
+        return NULL;
 
-    hmem = GlobalAlloc(GMEM_MOVEABLE, info.RegionSize);
-    if(hmem)
+    if (CLSIDFromString(guid_str, &guid) != S_OK)
     {
-        memcpy(GlobalLock(hmem), data, info.RegionSize);
-        GlobalUnlock(hmem);
-        hr = CreateStreamOnHGlobal(hmem, TRUE, stream);
+        ERR("Failed to convert string %s\n", debugstr_w(guid_str));
+        return NULL;
     }
-    UnmapViewOfFile(data);
-    return hr;
-}
-
-static IDropTarget* get_droptarget_pointer(HWND hwnd)
-{
-    IDropTarget *droptarget = NULL;
-    HANDLE map;
-    IStream *stream;
 
-    map = get_droptarget_local_handle(hwnd);
-    if(!map) return NULL;
-
-    if(SUCCEEDED(create_stream_from_map(map, &stream)))
+    hr = CoGetClassObject(&guid, CLSCTX_SERVER, NULL, &IID_IDropTarget, (void**)&droptarget);
+    if (FAILED(hr))
     {
-        CoUnmarshalInterface(stream, &IID_IDropTarget, (void**)&droptarget);
-        IStream_Release(stream);
+        ERR("CoGetClassObject failed (0x%08x)\n", hr);
     }
-    CloseHandle(map);
+
     return droptarget;
 }
 
-- 
2.26.2

[Huw Davies]

On Wed, Jun 03, 2020 at 06:25:13PM +1000, Alistair Leslie-Hughes wrote:
> The BCG library registers just about every window it creates for Drag and Drop.
> The current method creates a file for each window registered for DnD, and in the case
> of a large application that uses BCG, 1000's of files can be created.  Reducing the
> amount of file an application can use.

Sorry this has taken so long to review.
> The new way, uses an unique guid for each regsitered window and then use that guid
> to lookup whether it supports Drag and Drop.

So this will start a new local server thread for each registration,
which doesn't sound like an improvement.  Possible other options
include allocating a larger memory block which could be shared by a
number of registrations or using the running object table.

Also, please try to update the existing comments when changing the
implementation, this patch left several comments that were no longer
correct.

Huw.