[PATCH 5/6] mshtml: Use an event mask and list in XMLHttpReqEventListener.

[Jacek Caban]

Hi Gabriel,

On 5/23/22 19:41, Gabriel Ivăncescu wrote:
> On 23/05/2022 19:09, Jacek Caban wrote:
>> Hi Gabriel,
>>
>> On 5/23/22 17:22, Gabriel Ivăncescu wrote:
>>> Instead of hardcoding each event, since there will be plenty more.
>>
>>
>> What's an example of those missing ones?
>>
>
> It's not in this patch series, but IE10+ modes will additionally need:
>
> abort, error, loadstart, loadend, progress
>
>>> Signed-off-by: Gabriel Ivăncescu<gabrielopcode at gmail.com>
>>> ---
>>>   dlls/mshtml/xmlhttprequest.c | 47 
>>> +++++++++++++++++++-----------------
>>>   1 file changed, 25 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/dlls/mshtml/xmlhttprequest.c 
>>> b/dlls/mshtml/xmlhttprequest.c
>>> index b9f6f1f..baef36e 100644
>>> --- a/dlls/mshtml/xmlhttprequest.c
>>> +++ b/dlls/mshtml/xmlhttprequest.c
>>> @@ -94,12 +94,23 @@ static HRESULT return_nscstr(nsresult nsres, 
>>> nsACString *nscstr, BSTR *p)
>>>       return S_OK;
>>>   }
>>> +#define EVENTS_LIST \
>>> +    X(readystatechange) \
>>> +    X(load)
>>> +
>>> +#undef X
>>> +#define X(event) EVENT_##event,
>>> +enum { EVENTS_LIST };
>>> +#undef X
>>> +#define X(event) L"" #event,
>>> +static const WCHAR *events[] = { EVENTS_LIST };
>>> +#undef X
>>
>>
>> You could avoid all those preprocessor directives by simply using an 
>> array of structs.
>>
>
> The problem is the first one is an enum, while second one is actual 
> data, so I can't use a struct to unify this. I wanted to keep it 
> easily in sync, but I can of course separate them without preprocessor 
> if you prefer.


It doesn't sound too bad and you may always use something like C_ASSERT 
to make sure we don't forget to update one of its parts.


Generally, this could use more generic event code. Reasons for 
XMLHttpReqEventListener existance are mostly historical, we could for 
example have a more generic version of Gecko event listener that could 
be also used instead of htmlevent_listener. I left it separated while 
changing the code for IE9+ events mostly because it was simple enough. 
If this is going to need more duplication in the future, I think we 
should revisit that. That said, I'm generally fine with the approach for 
now, but I wouldn't plan on extending that much further in this direction.


Jacek