Re: Added XEmbed embedder support

[Dimi Paun]

On Mon, 2005-06-20 at 00:05 +0200, Jacek Caban wrote:
> Hello.
>
> This patch adds support for XEmbed embedder
> It implements most of it (remaining todos
> are listed in comments of xembed.c).

Cool stuff, I'm really excited about this work. 
Not to mention that this might prove useful outside
of the MSHTML context.

I will not comment on the fundamentals of
the approach, that's more Alexandre's alley, but
just a few style nits.
> +struct xembed_list
> +{
> - struct xembed_list *next;
> - HWND hwnd;
>
> +};

What about using the standard list from wine/list.h?
> /* DIB Section sync state */
> @@ -590,6 +601,8 @@ enum x11drv_atoms
>      XATOM_XdndSelection,
>      XATOM_XdndTarget,
>      XATOM_XdndTypeList,
> - XATOM__XEMBED,
> - XATOM__XEMBED_INFO,
> XATOM_WCF_DIB,
> XATOM_image_gif,
> XATOM_text_html,
>
> @@ -649,6 +662,10 @@ struct x11drv_win_data
>      struct dce *dce;            /* DCE for CS_OWNDC or CS_CLASSDC windows */
>      HBITMAP     hWMIconBitmap;
>      HBITMAP     hWMIconMask;
> - union {
> -
>    struct xembed_list *list;
> -
>    ULONG_PTR cnt;
> - } xembed;
>
> };

Do we really need the union? We're not that hard pressed for
memory. What about just

@@ -649,6 +662,10 @@ struct x11drv_win_data
     struct dce *dce;            /* DCE for CS_OWNDC or CS_CLASSDC windows */
     HBITMAP     hWMIconBitmap;
     HBITMAP     hWMIconMask;
+    struct xembed_list *list;
+    ULONG_PTR cnt;
 };

Also, instead of having explicit tests like these:
> - if(data->xembed.cnt)

Maybe we can have a 
  BOOL win_has_embedded_children(struct x11drv_win_data *data)

function to make it more explicit what we're testing for.