Dimi Paun wrote:
>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?
>
>
AFAIK using standard list lists or not is up to developer.
Personally I don't like Wine's implementation... I don't have
any argument why - I just feel better not using it :-)
>
>
>> /* 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;
> };
>
>
I can't see why union is worse... But instead of union it can
always be a list, if we don't care about memory. I didn't do it
this way because it was better for my earlier versions, but now
I may change it.
>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.
>
>
>
OK, I thought it's clean enough, but I can change it.
Jacek
