[PATCH 4/6] mshtml: Delegate event construction to functions by type.

[Jacek Caban]

On 5/23/22 19:38, Gabriel Ivăncescu wrote:
> On 23/05/2022 19:06, Jacek Caban wrote:
>> Hi Gabriel,
>>
>> On 5/23/22 17:22, Gabriel Ivăncescu wrote:
>>> Instead of hardcoding it, which will be necessary as the amount of 
>>> event
>>> types implemented grows.
>>>
>>> Signed-off-by: Gabriel Ivăncescu<gabrielopcode at gmail.com>
>>> ---
>>>   dlls/mshtml/htmlevent.c | 33 ++++++++++++++++++++++++---------
>>>   1 file changed, 24 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/dlls/mshtml/htmlevent.c b/dlls/mshtml/htmlevent.c
>>> index f28dd63..c25b85f 100644
>>> --- a/dlls/mshtml/htmlevent.c
>>> +++ b/dlls/mshtml/htmlevent.c
>>> @@ -2373,6 +2373,26 @@ static BOOL check_event_iface(nsIDOMEvent 
>>> *event, REFIID riid)
>>>       return TRUE;
>>>   }
>>> +static DOMEvent *message_event_ctor(nsIDOMEvent *nsevent, 
>>> dispex_static_data_t **dispex_data)
>>> +{
>>> +    DOMMessageEvent *message_event = 
>>> heap_alloc_zero(sizeof(*message_event));
>>> +    if(!message_event)
>>> +        return NULL;
>>> +
>>> +    message_event->IDOMMessageEvent_iface.lpVtbl = 
>>> &DOMMessageEventVtbl;
>>> +    message_event->event.query_interface = 
>>> DOMMessageEvent_query_interface;
>>> +    message_event->event.destroy = DOMMessageEvent_destroy;
>>> +    *dispex_data = &DOMMessageEvent_dispex;
>>> +    return &message_event->event;
>>> +}
>>> +
>>> +static const struct {
>>> +    DOMEvent* (*proc)(nsIDOMEvent*,dispex_static_data_t**);
>>> +    compat_mode_t min_mode;
>>> +} event_type_ctors[ARRAY_SIZE(event_types)] = {
>>> +    [EVENT_TYPE_MESSAGE]        = { message_event_ctor, 
>>> COMPAT_MODE_QUIRKS },
>>> +};
>>
>>
>> While we may need something more generic at some point, this seems 
>> this patch series just uses it just for two event types and it's not 
>> clear we want to go in that direction. The way we handle 
>> DOMCustomEvent is probably better in cases where it's possible. At 
>> this point you leave out of generic mechanism, meaning that we will 
>> prefer to not use the new mechanism anyway. I'd suggest to wait for 
>> more problematic cases before having something like that.
>>
>
> Do you mean checking for Gecko's interface? AFAIK there is no 
> nsIDOMProgressEvent (it seems to have been removed?) so I can't do that.


Yes, I know, nsIDOMProgressEvent was removed from Gecko as part of 
upstream de-XPCOM efforts, but we could potentially bring it back if 
needed. And we may need it to properly implement IDOMMessageEvent at 
some point. Once we do, we will probably want this to not use event 
id-based constructor and rely on Gecko to provide that information instead.


> So what should I do? Add it inline like for Message Event? I heavily 
> dislike hardcoding checks but... 


As above, having an array of two constructors covering only part of the 
problem that we potentially would like to get rid of in favour of 
another mechanism does not seem like an improvement. Please keep it simple.


Jacek