[PATCH v2 0/5] MR2098: mshtml: Implement synchronous XMLHttpRequests.
wine-gecko does actually support synchronous XMLHttpRequests, even if they are deprecated, but unfortunately it is very broken compared to IE or all the other major browsers (more details [here](https://bugzilla.mozilla.org/show_bug.cgi?id=697151)). Thankfully, it still provides us with the event message loop, which is enough to fix it on mshtml side and act like IE. For sync XHRs, send() is supposed to block all script code, except for notifications sent to the sync XHR. This is because when send() blocks, no other piece of code in the script should execute other than the sync XHR handlers. Unfortunately, that's not the case in Gecko's broken implementation, which can execute handlers as if they were APCs while it's "blocked" in send(). Note that it doesn't actually block, though, because we still process the message loop (non-event related tasks such as navigation, paints, and other stuff), and that's normal. Gecko doesn't block everything related to script events, only some things on the document and timers, but that's far from enough and not even enough for our purposes since we use our own timer tasks. And not even message events are blocked, even though we *also* use our own message event dispatchers (but it doesn't block Gecko ones either). So what we have to do is we need to track all the timers and events dispatched that result in script handlers being executed, while "blocking" inside of a sync XHR send(), and queue them up to defer them to be dispatched later, after send() unblocks. But this is easier said that done. There are many corner cases to consider here, for example: * Events dispatched *synchronously* from within a sync XHR handler or other piece of code called from it. * Nested sync XHR send() calls (called during handler of another sync XHR). * Async XHRs having their events dispatched during a blocking sync XHR send() are complicated. `readyStateChange` for example needs to have the async XHR's readyState at the time it was sent, **not** at the time it was actually dispatched, since we're going to delay it and dispatch it later. It's similar with other XHR states, such as responseText (which can be partial). * Aborts of async XHRs during such handlers. These patches hopefully should address all the issues and, on a high level, work like this: * Track the `readyState` and `responseText` length manually, so we can override / force them to specific values. * "Snapshot" the async XHR at the time we encounter an event for it, and queue this event with such information. When later dispatching this event (after being deferred), we temporarily set the state of the async XHR to the snapshot. * To deal with nested event dispatches, keep track of a "dispatch depth" (everytime we dispatch an event we increase the depth, and when it returns we decrease it). * For sync XHRs, we note the depth when send() is called, and defer events at that depth, since they're dispatched during the "blocked" message loop and need to be delayed (except for sync XHR events, which is a special case since it must be dispatched synchronously). When it returns, we restore the previous blocking depth. -- v2: mshtml: Send all readystatechange events for synchronous XHRs in IE9 mshtml: Implement synchronous XMLHttpRequest. mshtml: Track responseText's length in XHRs and report it manually. mshtml: Track readyState in XHRs and report it manually. mshtml: Pass optional args to XMLHttpRequest.open() correctly. https://gitlab.winehq.org/wine/wine/-/merge_requests/2098
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/xmlhttprequest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/mshtml/xmlhttprequest.c b/dlls/mshtml/xmlhttprequest.c index b4269bbe8d1..04ee66af6d9 100644 --- a/dlls/mshtml/xmlhttprequest.c +++ b/dlls/mshtml/xmlhttprequest.c @@ -594,8 +594,8 @@ static HRESULT WINAPI HTMLXMLHttpRequest_open(IHTMLXMLHttpRequest *iface, BSTR b return hres; } - nsres = nsIXMLHttpRequest_Open(This->nsxhr, &method, &url, TRUE, - &user, &password, 0); + nsres = nsIXMLHttpRequest_Open(This->nsxhr, &method, &url, !!V_BOOL(&varAsync), + &user, &password, 1 + (V_VT(&varPassword) != VT_EMPTY ? 2 : V_VT(&varUser) != VT_EMPTY)); nsACString_Finish(&method); nsACString_Finish(&url); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2098
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> So it can be forced to specific values during synchronous XHRs. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/xmlhttprequest.c | 75 +++++++++++++++++------------------- 1 file changed, 35 insertions(+), 40 deletions(-) diff --git a/dlls/mshtml/xmlhttprequest.c b/dlls/mshtml/xmlhttprequest.c index 04ee66af6d9..0efe4c3ac85 100644 --- a/dlls/mshtml/xmlhttprequest.c +++ b/dlls/mshtml/xmlhttprequest.c @@ -138,6 +138,7 @@ struct HTMLXMLHttpRequest { IWineXMLHttpRequestPrivate IWineXMLHttpRequestPrivate_iface; IProvideClassInfo2 IProvideClassInfo2_iface; LONG ref; + LONG readyState; response_type_t response_type; nsIXMLHttpRequest *nsxhr; XMLHttpReqEventListener *event_listener; @@ -224,6 +225,7 @@ static nsrefcnt NSAPI XMLHttpReqEventListener_Release(nsIDOMEventListener *iface static nsresult NSAPI XMLHttpReqEventListener_HandleEvent(nsIDOMEventListener *iface, nsIDOMEvent *nsevent) { XMLHttpReqEventListener *This = impl_from_nsIDOMEventListener(iface); + UINT16 readyState; DOMEvent *event; HRESULT hres; @@ -232,6 +234,9 @@ static nsresult NSAPI XMLHttpReqEventListener_HandleEvent(nsIDOMEventListener *i if(!This->xhr) return NS_OK; + if(NS_SUCCEEDED(nsIXMLHttpRequest_GetReadyState(This->xhr->nsxhr, &readyState))) + This->xhr->readyState = readyState; + hres = create_event_from_nsevent(nsevent, dispex_compat_mode(&This->xhr->event_target.dispex), &event); if(SUCCEEDED(hres) ){ dispatch_event(&This->xhr->event_target, event); @@ -344,19 +349,12 @@ static HRESULT WINAPI HTMLXMLHttpRequest_Invoke(IHTMLXMLHttpRequest *iface, DISP static HRESULT WINAPI HTMLXMLHttpRequest_get_readyState(IHTMLXMLHttpRequest *iface, LONG *p) { HTMLXMLHttpRequest *This = impl_from_IHTMLXMLHttpRequest(iface); - UINT16 val; - nsresult nsres; TRACE("(%p)->(%p)\n", This, p); if(!p) return E_POINTER; - nsres = nsIXMLHttpRequest_GetReadyState(This->nsxhr, &val); - if(NS_FAILED(nsres)) { - ERR("nsIXMLHttpRequest_GetReadyState failed: %08lx\n", nsres); - return E_FAIL; - } - *p = val; + *p = This->readyState; return S_OK; } @@ -378,6 +376,11 @@ static HRESULT WINAPI HTMLXMLHttpRequest_get_responseText(IHTMLXMLHttpRequest *i if(!p) return E_POINTER; + if(This->readyState < 3) { + *p = NULL; + return S_OK; + } + nsAString_Init(&nsstr, NULL); nsres = nsIXMLHttpRequest_GetResponseText(This->nsxhr, &nsstr); return return_nsstr(nsres, &nsstr, p); @@ -394,6 +397,11 @@ static HRESULT WINAPI HTMLXMLHttpRequest_get_responseXML(IHTMLXMLHttpRequest *if TRACE("(%p)->(%p)\n", This, p); + if(This->readyState < 4) { + *p = NULL; + return S_OK; + } + if(dispex_compat_mode(&This->event_target.dispex) >= COMPAT_MODE_IE10) { nsIDOMDocument *nsdoc; nsresult nsres; @@ -448,6 +456,11 @@ static HRESULT WINAPI HTMLXMLHttpRequest_get_status(IHTMLXMLHttpRequest *iface, if(!p) return E_POINTER; + if(This->readyState < 2) { + *p = 0; + return E_FAIL; + } + nsres = nsIXMLHttpRequest_GetStatus(This->nsxhr, &val); if(NS_FAILED(nsres)) { ERR("nsIXMLHttpRequest_GetStatus failed: %08lx\n", nsres); @@ -465,19 +478,13 @@ static HRESULT WINAPI HTMLXMLHttpRequest_get_statusText(IHTMLXMLHttpRequest *ifa HTMLXMLHttpRequest *This = impl_from_IHTMLXMLHttpRequest(iface); nsACString nscstr; nsresult nsres; - HRESULT hres; - LONG state; TRACE("(%p)->(%p)\n", This, p); if(!p) return E_POINTER; - hres = IHTMLXMLHttpRequest_get_readyState(iface, &state); - if(FAILED(hres)) - return hres; - - if(state < 2) { + if(This->readyState < 2) { *p = NULL; return E_FAIL; } @@ -518,6 +525,8 @@ static HRESULT WINAPI HTMLXMLHttpRequest_abort(IHTMLXMLHttpRequest *iface) return E_FAIL; } + /* This state change is not broadcast by Gecko, so set it manually */ + This->readyState = 0; return S_OK; } @@ -657,19 +666,13 @@ static HRESULT WINAPI HTMLXMLHttpRequest_getAllResponseHeaders(IHTMLXMLHttpReque HTMLXMLHttpRequest *This = impl_from_IHTMLXMLHttpRequest(iface); nsACString nscstr; nsresult nsres; - HRESULT hres; - LONG state; TRACE("(%p)->(%p)\n", This, p); if(!p) return E_POINTER; - hres = IHTMLXMLHttpRequest_get_readyState(iface, &state); - if(FAILED(hres)) - return hres; - - if(state < 2) { + if(This->readyState < 2) { *p = NULL; return E_FAIL; } @@ -685,8 +688,6 @@ static HRESULT WINAPI HTMLXMLHttpRequest_getResponseHeader(IHTMLXMLHttpRequest * nsACString header, ret; char *cstr; nsresult nsres; - HRESULT hres; - LONG state; TRACE("(%p)->(%s %p)\n", This, debugstr_w(bstrHeader), p); if(!p) @@ -694,11 +695,7 @@ static HRESULT WINAPI HTMLXMLHttpRequest_getResponseHeader(IHTMLXMLHttpRequest * if(!bstrHeader) return E_INVALIDARG; - hres = IHTMLXMLHttpRequest_get_readyState(iface, &state); - if(FAILED(hres)) - return hres; - - if(state < 2) { + if(This->readyState < 2) { *p = NULL; return E_FAIL; } @@ -939,8 +936,6 @@ static HRESULT WINAPI HTMLXMLHttpRequest_private_get_response(IWineXMLHttpReques { HTMLXMLHttpRequest *This = impl_from_IWineXMLHttpRequestPrivate(iface); HRESULT hres = S_OK; - nsresult nsres; - UINT16 state; TRACE("(%p)->(%p)\n", This, p); @@ -958,8 +953,7 @@ static HRESULT WINAPI HTMLXMLHttpRequest_private_get_response(IWineXMLHttpReques case response_type_arraybuf: case response_type_blob: - nsres = nsIXMLHttpRequest_GetReadyState(This->nsxhr, &state); - if(NS_FAILED(nsres) || state < 4) { + if(This->readyState < 4) { V_VT(p) = VT_EMPTY; break; } @@ -986,17 +980,11 @@ static HRESULT WINAPI HTMLXMLHttpRequest_private_put_responseType(IWineXMLHttpRe HTMLXMLHttpRequest *This = impl_from_IWineXMLHttpRequestPrivate(iface); nsAString nsstr; nsresult nsres; - HRESULT hres; unsigned i; - LONG state; TRACE("(%p)->(%s)\n", This, debugstr_w(v)); - hres = IHTMLXMLHttpRequest_get_readyState(&This->IHTMLXMLHttpRequest_iface, &state); - if(FAILED(hres)) - return hres; - - if(state < 1 || state > 2) { + if(This->readyState < 1 || This->readyState > 2) { /* FIXME: Return InvalidStateError */ return E_FAIL; } @@ -1309,6 +1297,9 @@ static void HTMLXMLHttpRequest_bind_event(DispatchEx *dispex, eventid_t eid) This->event_listener->events_mask = 0; } + if(This->event_listener->events_mask & (1 << i)) + return; + nsres = nsIXMLHttpRequest_QueryInterface(This->nsxhr, &IID_nsIDOMEventTarget, (void**)&nstarget); assert(nsres == NS_OK); @@ -1489,6 +1480,10 @@ static HRESULT WINAPI HTMLXMLHttpRequestFactory_create(IHTMLXMLHttpRequestFactor &HTMLXMLHttpRequest_dispex, This->window->doc->document_mode); ret->ref = 1; + /* Always register these handlers because we need them to track state */ + HTMLXMLHttpRequest_bind_event(&ret->event_target.dispex, EVENTID_READYSTATECHANGE); + HTMLXMLHttpRequest_bind_event(&ret->event_target.dispex, EVENTID_PROGRESS); + *p = &ret->IHTMLXMLHttpRequest_iface; return S_OK; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2098
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/xmlhttprequest.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/dlls/mshtml/xmlhttprequest.c b/dlls/mshtml/xmlhttprequest.c index 0efe4c3ac85..f4082bc8e3d 100644 --- a/dlls/mshtml/xmlhttprequest.c +++ b/dlls/mshtml/xmlhttprequest.c @@ -139,6 +139,7 @@ struct HTMLXMLHttpRequest { IProvideClassInfo2 IProvideClassInfo2_iface; LONG ref; LONG readyState; + size_t responseText_length; response_type_t response_type; nsIXMLHttpRequest *nsxhr; XMLHttpReqEventListener *event_listener; @@ -225,7 +226,9 @@ static nsrefcnt NSAPI XMLHttpReqEventListener_Release(nsIDOMEventListener *iface static nsresult NSAPI XMLHttpReqEventListener_HandleEvent(nsIDOMEventListener *iface, nsIDOMEvent *nsevent) { XMLHttpReqEventListener *This = impl_from_nsIDOMEventListener(iface); + const PRUnichar *text; UINT16 readyState; + nsAString nsstr; DOMEvent *event; HRESULT hres; @@ -234,9 +237,21 @@ static nsresult NSAPI XMLHttpReqEventListener_HandleEvent(nsIDOMEventListener *i if(!This->xhr) return NS_OK; - if(NS_SUCCEEDED(nsIXMLHttpRequest_GetReadyState(This->xhr->nsxhr, &readyState))) + if(NS_SUCCEEDED(nsIXMLHttpRequest_GetReadyState(This->xhr->nsxhr, &readyState))) { This->xhr->readyState = readyState; + if(This->xhr->readyState >= 3) { + nsAString_Init(&nsstr, NULL); + if(NS_SUCCEEDED(nsIXMLHttpRequest_GetResponseText(This->xhr->nsxhr, &nsstr))) { + /* Avoid recalculating from the beginning, since it can't be shorter */ + nsAString_GetData(&nsstr, &text); + if(text && text[0]) + This->xhr->responseText_length += wcslen(text + This->xhr->responseText_length); + nsAString_Finish(&nsstr); + } + } + } + hres = create_event_from_nsevent(nsevent, dispex_compat_mode(&This->xhr->event_target.dispex), &event); if(SUCCEEDED(hres) ){ dispatch_event(&This->xhr->event_target, event); @@ -368,6 +383,8 @@ static HRESULT WINAPI HTMLXMLHttpRequest_get_responseBody(IHTMLXMLHttpRequest *i static HRESULT WINAPI HTMLXMLHttpRequest_get_responseText(IHTMLXMLHttpRequest *iface, BSTR *p) { HTMLXMLHttpRequest *This = impl_from_IHTMLXMLHttpRequest(iface); + const PRUnichar *text; + HRESULT hres = S_OK; nsAString nsstr; nsresult nsres; @@ -383,7 +400,17 @@ static HRESULT WINAPI HTMLXMLHttpRequest_get_responseText(IHTMLXMLHttpRequest *i nsAString_Init(&nsstr, NULL); nsres = nsIXMLHttpRequest_GetResponseText(This->nsxhr, &nsstr); - return return_nsstr(nsres, &nsstr, p); + if(NS_FAILED(nsres)) + hres = map_nsresult(nsres); + else { + nsAString_GetData(&nsstr, &text); + if(!text || !text[0]) + *p = NULL; + else if(!(*p = SysAllocStringLen(text, This->responseText_length))) + hres = E_OUTOFMEMORY; + nsAString_Finish(&nsstr); + } + return hres; } static HRESULT WINAPI HTMLXMLHttpRequest_get_responseXML(IHTMLXMLHttpRequest *iface, IDispatch **p) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2098
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- The todo_wine that are added are not new failures, they were just never checked before (it bailed out early since open() failed). --- dlls/mshtml/htmlevent.c | 35 ++++++++ dlls/mshtml/htmlevent.h | 6 ++ dlls/mshtml/mshtml_private.h | 8 ++ dlls/mshtml/task.c | 57 ++++++++++++- dlls/mshtml/tests/script.c | 24 +++++- dlls/mshtml/tests/xhr.js | 92 ++++++++++++++++++++ dlls/mshtml/tests/xmlhttprequest.c | 18 ++-- dlls/mshtml/xmlhttprequest.c | 129 +++++++++++++++++++++++++---- 8 files changed, 335 insertions(+), 34 deletions(-) diff --git a/dlls/mshtml/htmlevent.c b/dlls/mshtml/htmlevent.c index bcde9f38bba..1514213fb0a 100644 --- a/dlls/mshtml/htmlevent.c +++ b/dlls/mshtml/htmlevent.c @@ -3603,6 +3603,7 @@ static HRESULT dispatch_event_object(EventTarget *event_target, DOMEvent *event, const event_target_vtbl_t *vtbl, *target_vtbl; HTMLEventObj *event_obj_ref = NULL; IHTMLEventObj *prev_event = NULL; + thread_data_t *thread_data; EventTarget *iter; HRESULT hres; @@ -3618,6 +3619,39 @@ static HRESULT dispatch_event_object(EventTarget *event_target, DOMEvent *event, return E_FAIL; } + if(!(thread_data = get_thread_data(TRUE))) { + ERR("no memory to get thread data\n"); + return E_OUTOFMEMORY; + } + + if(thread_data->event_dispatch_depth == thread_data->blocking_event_dispatch_depth) { + struct queued_event *queued_event = malloc(sizeof(*queued_event)); + + if(!queued_event) { + ERR("no memory to queue event\n"); + return E_OUTOFMEMORY; + } + + queued_event->event = event; + queued_event->event_target = event_target; + IDOMEvent_AddRef(&event->IDOMEvent_iface); + IEventTarget_AddRef(&event_target->IEventTarget_iface); + + /* While queuing an event this way would not work correctly with their default behavior + * in Gecko (preventDefault() can't be called because we need to *delay* the default, + * rather than prevent it completely), Gecko does suppress events reaching the document + * during the sync XHR event loop, so all the element/node events should be safe. There + * are other events not delayed by Gecko though, but I think those don't have defaults. + * + * Async XHR events should be safe since they don't have defaults, as well as Message or + * Storage events (since we don't use Gecko's). If we find an event that has defaults on + * Gecko's side and isn't delayed by Gecko, we need to figure out a way to handle it... + */ + list_add_tail(&thread_data->queued_events_list, &queued_event->entry); + return S_OK; + } + thread_data->event_dispatch_depth++; + iter = event_target; IEventTarget_AddRef(&event_target->IEventTarget_iface); @@ -3717,6 +3751,7 @@ static HRESULT dispatch_event_object(EventTarget *event_target, DOMEvent *event, if(target_chain != target_chain_buf) free(target_chain); + thread_data->event_dispatch_depth--; return S_OK; } diff --git a/dlls/mshtml/htmlevent.h b/dlls/mshtml/htmlevent.h index 54cc5b5932a..c903e965906 100644 --- a/dlls/mshtml/htmlevent.h +++ b/dlls/mshtml/htmlevent.h @@ -99,6 +99,12 @@ typedef struct DOMEvent { BOOL no_event_obj; } DOMEvent; +struct queued_event { + struct list entry; + EventTarget *event_target; + DOMEvent *event; +}; + const WCHAR *get_event_name(eventid_t) DECLSPEC_HIDDEN; void check_event_attr(HTMLDocumentNode*,nsIDOMElement*) DECLSPEC_HIDDEN; void release_event_target(EventTarget*) DECLSPEC_HIDDEN; diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index bec982a75fc..59afd6d99ca 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -1293,12 +1293,20 @@ typedef struct { struct list task_list; struct list timer_list; struct wine_rb_tree session_storage_map; + + /* See sync_xhr_send() */ + unsigned int event_dispatch_depth; + unsigned int blocking_event_dispatch_depth; + struct list queued_events_list; + struct list queued_xhr_events_list; } thread_data_t; thread_data_t *get_thread_data(BOOL) DECLSPEC_HIDDEN; HWND get_thread_hwnd(void) DECLSPEC_HIDDEN; int session_storage_map_cmp(const void*,const struct wine_rb_entry*) DECLSPEC_HIDDEN; void destroy_session_storage(thread_data_t*) DECLSPEC_HIDDEN; +void process_queued_events(void) DECLSPEC_HIDDEN; +nsresult sync_xhr_send(nsIXMLHttpRequest*,nsIVariant*) DECLSPEC_HIDDEN; LONG get_task_target_magic(void) DECLSPEC_HIDDEN; HRESULT push_task(task_t*,task_proc_t,task_proc_t,LONG) DECLSPEC_HIDDEN; diff --git a/dlls/mshtml/task.c b/dlls/mshtml/task.c index 2987b4bf19d..0985cb7b7b3 100644 --- a/dlls/mshtml/task.c +++ b/dlls/mshtml/task.c @@ -289,7 +289,7 @@ static LRESULT process_timer(void) thread_data = get_thread_data(FALSE); assert(thread_data != NULL); - if(list_empty(&thread_data->timer_list)) { + if(list_empty(&thread_data->timer_list) || thread_data->blocking_event_dispatch_depth != ~0) { KillTimer(thread_data->thread_hwnd, TIMER_ID); return 0; } @@ -324,7 +324,7 @@ static LRESULT process_timer(void) call_timer_disp(disp, timer_type); IDispatch_Release(disp); - }while(!list_empty(&thread_data->timer_list)); + }while(!list_empty(&thread_data->timer_list) && thread_data->blocking_event_dispatch_depth == ~0); KillTimer(thread_data->thread_hwnd, TIMER_ID); return 0; @@ -334,6 +334,8 @@ static LRESULT WINAPI hidden_proc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lPa { switch(msg) { case WM_PROCESSTASK: + process_queued_events(); + while(1) { task_t *task = pop_task(); if(!task) @@ -417,6 +419,9 @@ thread_data_t *get_thread_data(BOOL create) list_init(&thread_data->task_list); list_init(&thread_data->timer_list); wine_rb_init(&thread_data->session_storage_map, session_storage_map_cmp); + thread_data->blocking_event_dispatch_depth = ~0; + list_init(&thread_data->queued_events_list); + list_init(&thread_data->queued_xhr_events_list); } return thread_data; @@ -432,3 +437,51 @@ ULONGLONG get_time_stamp(void) GetSystemTimeAsFileTime(&time); return (((ULONGLONG)time.dwHighDateTime << 32) + time.dwLowDateTime) / 10000 - time_epoch; } + +nsresult sync_xhr_send(nsIXMLHttpRequest *nsxhr, nsIVariant *nsbody) +{ + thread_data_t *thread_data = get_thread_data(TRUE); + unsigned int prev_blocking_depth; + nsresult nsres; + + if(!thread_data) + return NS_ERROR_OUT_OF_MEMORY; + + /* Note: Starting with Gecko 30.0 (Firefox 30.0 / Thunderbird 30.0 / SeaMonkey 2.27), + * synchronous requests on the main thread have been deprecated due to the negative + * effects to the user experience. However, they still work. The larger issue is that + * it is broken because it still dispatches async XHR and some other events, while all + * other major browsers don't, including IE, so we have to filter them out during Send. + * + * They will need to be queued and dispatched later, after Send returns, otherwise it + * breaks JavaScript single-threaded expectations (js code will switch from blocking in + * Send to executing some event handler, then returning back to Send, messing its state). + * + * Of course we can't just delay dispatching the events, because the state won't match + * for each event later on to what it's supposed to be (most notably, XHR's readyState). + * So unfortunately we have to hardcode and snapshot the XHR state for async events... + * + * For details (and bunch of problems to consider) see: https://bugzil.la/697151 + */ + prev_blocking_depth = thread_data->blocking_event_dispatch_depth; + thread_data->blocking_event_dispatch_depth = thread_data->event_dispatch_depth; + + nsres = nsIXMLHttpRequest_Send(nsxhr, nsbody); + + thread_data->blocking_event_dispatch_depth = prev_blocking_depth; + + if(prev_blocking_depth != ~0) + return nsres; + + /* Make sure blocked events and timers are processed */ + if(!list_empty(&thread_data->queued_events_list) || !list_empty(&thread_data->queued_xhr_events_list)) + PostMessageW(thread_data->thread_hwnd, WM_PROCESSTASK, 0, 0); + + if(!list_empty(&thread_data->timer_list)) { + task_timer_t *timer = LIST_ENTRY(list_head(&thread_data->timer_list), task_timer_t, entry); + DWORD tc = GetTickCount(); + + SetTimer(thread_data->thread_hwnd, TIMER_ID, timer->time > tc ? timer->time - tc : 0, NULL); + } + return nsres; +} diff --git a/dlls/mshtml/tests/script.c b/dlls/mshtml/tests/script.c index 2bcd588f51f..4f475885ad5 100644 --- a/dlls/mshtml/tests/script.c +++ b/dlls/mshtml/tests/script.c @@ -3675,6 +3675,7 @@ typedef struct { IInternetProtocolSink *sink; BINDINFO bind_info; + HANDLE delay_event; BSTR content_type; IStream *stream; char *data; @@ -3685,12 +3686,17 @@ typedef struct { IUri *uri; } ProtocolHandler; +static ProtocolHandler *delay_with_signal_handler; + static DWORD WINAPI delay_proc(void *arg) { PROTOCOLDATA protocol_data = {PI_FORCE_ASYNC}; ProtocolHandler *protocol_handler = arg; - Sleep(protocol_handler->delay); + if(protocol_handler->delay_event) + WaitForSingleObject(protocol_handler->delay_event, INFINITE); + else + Sleep(protocol_handler->delay); protocol_handler->delay = -1; IInternetProtocolSink_Switch(protocol_handler->sink, &protocol_data); return 0; @@ -3735,7 +3741,7 @@ static void report_data(ProtocolHandler *This) IHttpNegotiate_Release(http_negotiate); - if(This->delay) { + if(This->delay || This->delay_event) { IInternetProtocolEx_AddRef(&This->IInternetProtocolEx_iface); QueueUserWorkItem(delay_proc, This, 0); return; @@ -4061,8 +4067,20 @@ static HRESULT WINAPI ProtocolEx_StartEx(IInternetProtocolEx *iface, IUri *uri, hres = IUri_GetQuery(uri, &query); if(SUCCEEDED(hres)) { - if(!lstrcmpW(query, L"?delay")) + if(!wcscmp(query, L"?delay")) This->delay = 1000; + else if(!wcscmp(query, L"?delay_with_signal")) { + if(delay_with_signal_handler) { + ProtocolHandler *delayed = delay_with_signal_handler; + delay_with_signal_handler = NULL; + SetEvent(delayed->delay_event); + This->delay = 30; + }else { + delay_with_signal_handler = This; + This->delay_event = CreateEventW(NULL, FALSE, FALSE, NULL); + ok(This->delay_event != NULL, "CreateEvent failed: %08lx\n", GetLastError()); + } + } else if(!wcsncmp(query, L"?content-type=", sizeof("?content-type=")-1)) This->content_type = SysAllocString(query + sizeof("?content-type=")-1); SysFreeString(query); diff --git a/dlls/mshtml/tests/xhr.js b/dlls/mshtml/tests/xhr.js index 2b07ee2f188..c6d8b400af0 100644 --- a/dlls/mshtml/tests/xhr.js +++ b/dlls/mshtml/tests/xhr.js @@ -76,6 +76,97 @@ function test_xhr() { xhr.send(xml); } +function test_sync_xhr() { + var async_xhr, async_xhr2, sync_xhr, sync_xhr_in_async, sync_xhr_nested, a = [ 0 ]; + + document.ondblclick = function() { a.push("dblclick"); }; + window.addEventListener("message", function(e) { a.push("msg" + e.data); }); + window.postMessage("1", "*"); + window.setTimeout(function() { a.push("timeout"); }, 0); + window.postMessage("2", "*"); + a.push(1); + + async_xhr = new XMLHttpRequest(); + async_xhr.open("POST", "echo.php", true); + async_xhr.onreadystatechange = function() { + if(async_xhr.readyState < 3) + return; + a.push("async_xhr(" + async_xhr.readyState + ")"); + ok(async_xhr2.readyState === 1, "async_xhr2.readyState = " + async_xhr2.readyState); + if(async_xhr.readyState == 4) { + window.setTimeout(function() { a.push("timeout_async"); }, 0); + window.postMessage("_async", "*"); + + sync_xhr_in_async = new XMLHttpRequest(); + sync_xhr_in_async.open("POST", "echo.php", false); + sync_xhr_in_async.onreadystatechange = function() { if(sync_xhr_in_async.readyState == 4) a.push("sync_xhr_in_async"); }; + sync_xhr_in_async.setRequestHeader("X-Test", "True"); + sync_xhr_in_async.send("sync_in_async"); + } + }; + async_xhr.addEventListener("click", function() { a.push("click"); }); + async_xhr.setRequestHeader("X-Test", "True"); + async_xhr.send("1234"); + a.push(2); + + async_xhr2 = new XMLHttpRequest(); + async_xhr2.open("POST", "echo.php?delay_with_signal", true); + async_xhr2.onreadystatechange = function() { + if(async_xhr2.readyState < 3) + return; + a.push("async_xhr2(" + async_xhr2.readyState + ")"); + ok(async_xhr.readyState === 4, "async_xhr.readyState = " + async_xhr.readyState); + }; + async_xhr2.setRequestHeader("X-Test", "True"); + async_xhr2.send("foobar"); + a.push(3); + + sync_xhr = new XMLHttpRequest(); + sync_xhr.open("POST", "echo.php?delay_with_signal", false); + sync_xhr.onreadystatechange = function() { + a.push("sync_xhr(" + sync_xhr.readyState + ")"); + ok(async_xhr.readyState === 1, "async_xhr.readyState in sync_xhr handler = " + async_xhr.readyState); + ok(async_xhr2.readyState === 1, "async_xhr2.readyState in sync_xhr handler = " + async_xhr2.readyState); + if(sync_xhr.readyState < 4) + return; + window.setTimeout(function() { a.push("timeout_sync"); }, 0); + window.postMessage("_sync", "*"); + + sync_xhr_nested = new XMLHttpRequest(); + sync_xhr_nested.open("POST", "echo.php", false); + sync_xhr_nested.onreadystatechange = function() { + a.push("nested(" + sync_xhr_nested.readyState + ")"); + if(sync_xhr_nested.readyState == 4) { + window.setTimeout(function() { a.push("timeout_nested"); }, 0); + window.postMessage("_nested", "*"); + + var e = document.createEvent("Event"); + e.initEvent("click", true, false); + async_xhr.dispatchEvent(e); + document.fireEvent("ondblclick", document.createEventObject()); + } + }; + sync_xhr_nested.setRequestHeader("X-Test", "True"); + sync_xhr_nested.send("nested"); + }; + sync_xhr.setRequestHeader("X-Test", "True"); + sync_xhr.send("abcd"); + a.push(4); + + window.setTimeout(function() { + var r = a.join(","); + todo_wine_if(document.documentMode < 10 || document.documentMode >= 11). + ok(r === "0,1,2,3," + (document.documentMode < 10 ? "sync_xhr(1),sync_xhr(2),sync_xhr(3)," : "") + + "sync_xhr(4)," + (document.documentMode < 10 ? "nested(1),nested(2),nested(3)," : "") + + "nested(4),click," + (document.documentMode < 11 ? "dblclick," : "") + + "4,async_xhr(3),async_xhr(4),sync_xhr_in_async,async_xhr2(3),async_xhr2(4)," + + "msg1,msg2,msg_sync,msg_nested,msg_async,timeout,timeout_sync,timeout_nested", + "unexpected order: " + r); + document.ondblclick = null; + next_test(); + }); +} + function test_content_types() { var xhr = new XMLHttpRequest(), types, i = 0, override = false; var v = document.documentMode; @@ -291,6 +382,7 @@ function test_response() { var tests = [ test_xhr, + test_sync_xhr, test_content_types, test_abort, test_timeout, diff --git a/dlls/mshtml/tests/xmlhttprequest.c b/dlls/mshtml/tests/xmlhttprequest.c index a3ec54ffef6..2415ac8eb0f 100644 --- a/dlls/mshtml/tests/xmlhttprequest.c +++ b/dlls/mshtml/tests/xmlhttprequest.c @@ -673,18 +673,12 @@ static void test_sync_xhr(IHTMLDocument2 *doc, const WCHAR *xml_url, const WCHAR SET_EXPECT(xmlhttprequest_onreadystatechange_opened); hres = IHTMLXMLHttpRequest_open(xhr, method, url, vbool, vempty, vempty); - todo_wine ok(hres == S_OK, "open failed: %08lx\n", hres); /* Gecko 30+ only supports async */ - todo_wine CHECK_CALLED(xmlhttprequest_onreadystatechange_opened); + ok(hres == S_OK, "open failed: %08lx\n", hres); + CHECK_CALLED(xmlhttprequest_onreadystatechange_opened); SysFreeString(method); SysFreeString(url); - if(FAILED(hres)) { - IHTMLXMLHttpRequest_Release(xhr); - xhr = NULL; - return; - } - text = (BSTR)0xdeadbeef; hres = IHTMLXMLHttpRequest_getAllResponseHeaders(xhr, &text); ok(hres == E_FAIL, "got %08lx\n", hres); @@ -718,11 +712,11 @@ static void test_sync_xhr(IHTMLDocument2 *doc, const WCHAR *xml_url, const WCHAR loading_cnt = 0; hres = IHTMLXMLHttpRequest_send(xhr, vempty); ok(hres == S_OK, "send failed: %08lx\n", hres); - CHECK_CALLED(xmlhttprequest_onreadystatechange_opened); - CHECK_CALLED(xmlhttprequest_onreadystatechange_headers_received); - CHECK_CALLED(xmlhttprequest_onreadystatechange_loading); + todo_wine CHECK_CALLED(xmlhttprequest_onreadystatechange_opened); + todo_wine CHECK_CALLED(xmlhttprequest_onreadystatechange_headers_received); + todo_wine CHECK_CALLED(xmlhttprequest_onreadystatechange_loading); CHECK_CALLED(xmlhttprequest_onreadystatechange_done); - ok(loading_cnt == 1, "loading_cnt = %d\n", loading_cnt); + todo_wine ok(loading_cnt == 1, "loading_cnt = %d\n", loading_cnt); text = NULL; hres = IHTMLXMLHttpRequest_getResponseHeader(xhr, content_type, &text); diff --git a/dlls/mshtml/xmlhttprequest.c b/dlls/mshtml/xmlhttprequest.c index f4082bc8e3d..b932e9df0a7 100644 --- a/dlls/mshtml/xmlhttprequest.c +++ b/dlls/mshtml/xmlhttprequest.c @@ -141,10 +141,56 @@ struct HTMLXMLHttpRequest { LONG readyState; size_t responseText_length; response_type_t response_type; + BOOL synchronous; nsIXMLHttpRequest *nsxhr; XMLHttpReqEventListener *event_listener; }; +struct queued_xhr_event { + struct queued_event header; + size_t responseText_length; + LONG readyState; +}; + +void process_queued_events(void) +{ + thread_data_t *thread_data = get_thread_data(FALSE); + struct queued_event *queued_event; + struct list *head; + + /* Another sync XHR send() can be called during a handler, so re-check the head after + every dispatch, while making sure async XHR events are always dispatched first */ + while(thread_data->blocking_event_dispatch_depth == ~0) { + BOOL skip_dispatch = FALSE; + + if(!(head = list_head(&thread_data->queued_xhr_events_list))) { + if(!(head = list_head(&thread_data->queued_events_list))) + break; + } else { + struct queued_xhr_event *xhr_event = LIST_ENTRY(head, struct queued_xhr_event, header.entry); + HTMLXMLHttpRequest *xhr = CONTAINING_RECORD(xhr_event->header.event_target, HTMLXMLHttpRequest, event_target); + + /* Skip aborted XHRs */ + if(xhr->readyState == 0 && xhr_event->readyState != 1) + skip_dispatch = TRUE; + else { + xhr->readyState = xhr_event->readyState; + xhr->responseText_length = xhr_event->responseText_length; + } + } + + queued_event = LIST_ENTRY(head, struct queued_event, entry); + list_remove(head); + + if(!skip_dispatch) + dispatch_event(queued_event->event_target, queued_event->event); + + IEventTarget_Release(&queued_event->event_target->IEventTarget_iface); + IDOMEvent_Release(&queued_event->event->IDOMEvent_iface); + free(queued_event); + } +} + static void detach_xhr_event_listener(XMLHttpReqEventListener *event_listener) { nsIDOMEventTarget *event_target; @@ -226,27 +272,33 @@ static nsrefcnt NSAPI XMLHttpReqEventListener_Release(nsIDOMEventListener *iface static nsresult NSAPI XMLHttpReqEventListener_HandleEvent(nsIDOMEventListener *iface, nsIDOMEvent *nsevent) { XMLHttpReqEventListener *This = impl_from_nsIDOMEventListener(iface); + thread_data_t *thread_data; + size_t responseText_length; const PRUnichar *text; - UINT16 readyState; nsAString nsstr; DOMEvent *event; + LONG readyState; HRESULT hres; + UINT16 val; TRACE("(%p)\n", This); if(!This->xhr) return NS_OK; - if(NS_SUCCEEDED(nsIXMLHttpRequest_GetReadyState(This->xhr->nsxhr, &readyState))) { - This->xhr->readyState = readyState; + readyState = This->xhr->readyState; + responseText_length = This->xhr->responseText_length; - if(This->xhr->readyState >= 3) { + if(NS_SUCCEEDED(nsIXMLHttpRequest_GetReadyState(This->xhr->nsxhr, &val))) { + readyState = val; + + if(readyState >= 3) { nsAString_Init(&nsstr, NULL); if(NS_SUCCEEDED(nsIXMLHttpRequest_GetResponseText(This->xhr->nsxhr, &nsstr))) { /* Avoid recalculating from the beginning, since it can't be shorter */ nsAString_GetData(&nsstr, &text); if(text && text[0]) - This->xhr->responseText_length += wcslen(text + This->xhr->responseText_length); + responseText_length += wcslen(text + responseText_length); nsAString_Finish(&nsstr); } } @@ -254,9 +306,52 @@ static nsresult NSAPI XMLHttpReqEventListener_HandleEvent(nsIDOMEventListener *i hres = create_event_from_nsevent(nsevent, dispex_compat_mode(&This->xhr->event_target.dispex), &event); if(SUCCEEDED(hres) ){ + if(!(thread_data = get_thread_data(FALSE)) || thread_data->blocking_event_dispatch_depth == ~0) + thread_data = NULL; + else { + if(!This->xhr->synchronous) { + struct queued_xhr_event *queued_event = malloc(sizeof(*queued_event)); + + if(!queued_event) + return NS_ERROR_OUT_OF_MEMORY; + + queued_event->header.event = event; + queued_event->header.event_target = &This->xhr->event_target; + IHTMLXMLHttpRequest_AddRef(&This->xhr->IHTMLXMLHttpRequest_iface); + + /* Save snapshot of XHR state (see sync_xhr_send() comments) */ + queued_event->readyState = readyState; + queued_event->responseText_length = responseText_length; + + list_add_tail(&thread_data->queued_xhr_events_list, &queued_event->header.entry); + return NS_OK; + } + + /* Workaround weird Gecko behavior with nested sync XHRs, where it sends readyState changes + for OPENED (or possibly other states than DONE), unlike IE10+ and non-nested sync XHRs... */ + if(readyState < 4 && event->event_id == EVENTID_READYSTATECHANGE) { + IDOMEvent_Release(&event->IDOMEvent_iface); + goto update_state; + } + + /* Temporarily increase the dispatch depth to allow this event to be dispatched */ + thread_data->event_dispatch_depth++; + } + + This->xhr->readyState = readyState; + This->xhr->responseText_length = responseText_length; + dispatch_event(&This->xhr->event_target, event); IDOMEvent_Release(&event->IDOMEvent_iface); + if(thread_data) + thread_data->event_dispatch_depth--; + + return NS_OK; } + +update_state: + This->xhr->readyState = readyState; + This->xhr->responseText_length = responseText_length; return NS_OK; } @@ -598,15 +693,6 @@ static HRESULT WINAPI HTMLXMLHttpRequest_open(IHTMLXMLHttpRequest *iface, BSTR b } } - /* Note: Starting with Gecko 30.0 (Firefox 30.0 / Thunderbird 30.0 / SeaMonkey 2.27), - * synchronous requests on the main thread have been deprecated due to the negative - * effects to the user experience. - */ - if(!V_BOOL(&varAsync)) { - FIXME("Synchronous request is not supported yet\n"); - return E_FAIL; - } - hres = variant_to_nsastr(varUser, &user); if(FAILED(hres)) return hres; @@ -630,6 +716,9 @@ static HRESULT WINAPI HTMLXMLHttpRequest_open(IHTMLXMLHttpRequest *iface, BSTR b return hres; } + /* Set this here, Gecko dispatches nested sync XHR readyState changes for OPENED (see HandleEvent) */ + This->synchronous = !V_BOOL(&varAsync); + nsres = nsIXMLHttpRequest_Open(This->nsxhr, &method, &url, !!V_BOOL(&varAsync), &user, &password, 1 + (V_VT(&varPassword) != VT_EMPTY ? 2 : V_VT(&varUser) != VT_EMPTY)); @@ -640,6 +729,7 @@ static HRESULT WINAPI HTMLXMLHttpRequest_open(IHTMLXMLHttpRequest *iface, BSTR b if(NS_FAILED(nsres)) { ERR("nsIXMLHttpRequest_Open failed: %08lx\n", nsres); + This->synchronous = FALSE; return E_FAIL; } @@ -676,13 +766,18 @@ static HRESULT WINAPI HTMLXMLHttpRequest_send(IHTMLXMLHttpRequest *iface, VARIAN return E_NOTIMPL; } - if(NS_SUCCEEDED(nsres)) - nsres = nsIXMLHttpRequest_Send(This->nsxhr, (nsIVariant*)nsbody); + if(NS_SUCCEEDED(nsres)) { + if(This->synchronous) + nsres = sync_xhr_send(This->nsxhr, (nsIVariant*)nsbody); + else + nsres = nsIXMLHttpRequest_Send(This->nsxhr, (nsIVariant*)nsbody); + } + if(nsbody) nsIWritableVariant_Release(nsbody); if(NS_FAILED(nsres)) { ERR("nsIXMLHttpRequest_Send failed: %08lx\n", nsres); - return E_FAIL; + return map_nsresult(nsres); } return S_OK; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2098
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/tests/xhr.js | 2 +- dlls/mshtml/tests/xmlhttprequest.c | 8 ++++---- dlls/mshtml/xmlhttprequest.c | 33 +++++++++++++++++++++++++++++- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/dlls/mshtml/tests/xhr.js b/dlls/mshtml/tests/xhr.js index c6d8b400af0..7799480e14a 100644 --- a/dlls/mshtml/tests/xhr.js +++ b/dlls/mshtml/tests/xhr.js @@ -155,7 +155,7 @@ function test_sync_xhr() { window.setTimeout(function() { var r = a.join(","); - todo_wine_if(document.documentMode < 10 || document.documentMode >= 11). + todo_wine_if(document.documentMode >= 11). ok(r === "0,1,2,3," + (document.documentMode < 10 ? "sync_xhr(1),sync_xhr(2),sync_xhr(3)," : "") + "sync_xhr(4)," + (document.documentMode < 10 ? "nested(1),nested(2),nested(3)," : "") + "nested(4),click," + (document.documentMode < 11 ? "dblclick," : "") + diff --git a/dlls/mshtml/tests/xmlhttprequest.c b/dlls/mshtml/tests/xmlhttprequest.c index 2415ac8eb0f..6226461a502 100644 --- a/dlls/mshtml/tests/xmlhttprequest.c +++ b/dlls/mshtml/tests/xmlhttprequest.c @@ -712,11 +712,11 @@ static void test_sync_xhr(IHTMLDocument2 *doc, const WCHAR *xml_url, const WCHAR loading_cnt = 0; hres = IHTMLXMLHttpRequest_send(xhr, vempty); ok(hres == S_OK, "send failed: %08lx\n", hres); - todo_wine CHECK_CALLED(xmlhttprequest_onreadystatechange_opened); - todo_wine CHECK_CALLED(xmlhttprequest_onreadystatechange_headers_received); - todo_wine CHECK_CALLED(xmlhttprequest_onreadystatechange_loading); + CHECK_CALLED(xmlhttprequest_onreadystatechange_opened); + CHECK_CALLED(xmlhttprequest_onreadystatechange_headers_received); + CHECK_CALLED(xmlhttprequest_onreadystatechange_loading); CHECK_CALLED(xmlhttprequest_onreadystatechange_done); - todo_wine ok(loading_cnt == 1, "loading_cnt = %d\n", loading_cnt); + ok(loading_cnt == 1, "loading_cnt = %d\n", loading_cnt); text = NULL; hres = IHTMLXMLHttpRequest_getResponseHeader(xhr, content_type, &text); diff --git a/dlls/mshtml/xmlhttprequest.c b/dlls/mshtml/xmlhttprequest.c index b932e9df0a7..39fc4526817 100644 --- a/dlls/mshtml/xmlhttprequest.c +++ b/dlls/mshtml/xmlhttprequest.c @@ -142,6 +142,7 @@ struct HTMLXMLHttpRequest { size_t responseText_length; response_type_t response_type; BOOL synchronous; + HTMLInnerWindow *window; nsIXMLHttpRequest *nsxhr; XMLHttpReqEventListener *event_listener; }; @@ -274,6 +275,7 @@ static nsresult NSAPI XMLHttpReqEventListener_HandleEvent(nsIDOMEventListener *i XMLHttpReqEventListener *This = impl_from_nsIDOMEventListener(iface); thread_data_t *thread_data; size_t responseText_length; + compat_mode_t compat_mode; const PRUnichar *text; nsAString nsstr; DOMEvent *event; @@ -304,7 +306,8 @@ static nsresult NSAPI XMLHttpReqEventListener_HandleEvent(nsIDOMEventListener *i } } - hres = create_event_from_nsevent(nsevent, dispex_compat_mode(&This->xhr->event_target.dispex), &event); + compat_mode = dispex_compat_mode(&This->xhr->event_target.dispex); + hres = create_event_from_nsevent(nsevent, compat_mode, &event); if(SUCCEEDED(hres) ){ if(!(thread_data = get_thread_data(FALSE)) || thread_data->blocking_event_dispatch_depth == ~0) thread_data = NULL; @@ -336,6 +339,31 @@ static nsresult NSAPI XMLHttpReqEventListener_HandleEvent(nsIDOMEventListener *i /* Temporarily increase the dispatch depth to allow this event to be dispatched */ thread_data->event_dispatch_depth++; + + /* IE10+ only send readystatechange event when it is DONE for sync XHRs, but older modes + send all the others here, including OPENED state change (even if it was opened earlier). */ + if(compat_mode < COMPAT_MODE_IE10 && This->xhr->readyState < 4 && ( + event->event_id == EVENTID_READYSTATECHANGE || event->event_id == EVENTID_PROGRESS || event->event_id == EVENTID_LOADSTART)) { + DOMEvent *readystatechange_event; + unsigned i; + + for(i = 1; i < 4; i++) { + hres = create_document_event(This->xhr->window->doc, EVENTID_READYSTATECHANGE, &readystatechange_event); + if(FAILED(hres)) + break; + + This->xhr->readyState = i; + dispatch_event(&This->xhr->event_target, readystatechange_event); + IDOMEvent_Release(&readystatechange_event->IDOMEvent_iface); + + /* Check if it was aborted (or somehow changed) in the handler */ + if(This->xhr->readyState != i) { + IDOMEvent_Release(&event->IDOMEvent_iface); + thread_data->event_dispatch_depth--; + return NS_OK; + } + } + } } This->xhr->readyState = readyState; @@ -415,6 +443,7 @@ static ULONG WINAPI HTMLXMLHttpRequest_Release(IHTMLXMLHttpRequest *iface) if(!ref) { if(This->event_listener) detach_xhr_event_listener(This->event_listener); + IHTMLWindow2_Release(&This->window->base.IHTMLWindow2_iface); release_event_target(&This->event_target); release_dispex(&This->event_target.dispex); nsIXMLHttpRequest_Release(This->nsxhr); @@ -1593,6 +1622,8 @@ static HRESULT WINAPI HTMLXMLHttpRequestFactory_create(IHTMLXMLHttpRequestFactor return E_OUTOFMEMORY; } ret->nsxhr = nsxhr; + ret->window = This->window; + IHTMLWindow2_AddRef(&This->window->base.IHTMLWindow2_iface); ret->IHTMLXMLHttpRequest_iface.lpVtbl = &HTMLXMLHttpRequestVtbl; ret->IHTMLXMLHttpRequest2_iface.lpVtbl = &HTMLXMLHttpRequest2Vtbl; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2098
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=129132 Your paranoid android. === w10pro64 (64 bit report) === mshtml: htmldoc.c:351: Test failed: expected Exec_SETTITLE
Jacek Caban (@jacek) commented about dlls/mshtml/htmlevent.c:
+ + if(!queued_event) { + ERR("no memory to queue event\n"); + return E_OUTOFMEMORY; + } + + queued_event->event = event; + queued_event->event_target = event_target; + IDOMEvent_AddRef(&event->IDOMEvent_iface); + IEventTarget_AddRef(&event_target->IEventTarget_iface); + + /* While queuing an event this way would not work correctly with their default behavior + * in Gecko (preventDefault() can't be called because we need to *delay* the default, + * rather than prevent it completely), Gecko does suppress events reaching the document + * during the sync XHR event loop, so all the element/node events should be safe. There + * are other events not delayed by Gecko though, but I think those don't have defaults. What events are not delayed? I'm trying to understand why we need to worry about events initiated by Gecko.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2098#note_23190
On Tue Feb 7 01:50:24 2023 +0000, Jacek Caban wrote:
What events are not delayed? I'm trying to understand why we need to worry about events initiated by Gecko. From what I can see, Gecko only suppresses events that reach the document. So, all nodes (and thus elements) linked to the document. The ones unlinked won't generate any events, anyway, so that's fine.
It also suppresses timeouts (those are on the window), but nothing else. However this is not important because we don't use Gecko's timeouts anyway. Other events that are *not* linked to the document and aren't delayed by Gecko: - Async XHR events (the biggest issue)→but these don't have defaults on Gecko AFAIK, so should be fine. - Message events (posted to window)→we don't use Gecko's anyway. - Storage events, but again, we don't use Gecko's anyway, so it's fine. - Possibly other such similar events? That said, such events would need to be sent/posted by Gecko before the event spin loop or during it to pose a problem. *And* they'd need to have defaults (on Gecko, not IE). I did mention in the comment but it's probably not very obvious. If you have suggestions how to reword the comment please let me know. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2098#note_23288
On Tue Feb 7 15:00:46 2023 +0000, Gabriel Ivăncescu wrote:
From what I can see, Gecko only suppresses events that reach the document. So, all nodes (and thus elements) linked to the document. The ones unlinked won't generate any events, anyway, so that's fine. It also suppresses timeouts (those are on the window), but nothing else. However this is not important because we don't use Gecko's timeouts anyway. Other events that are *not* linked to the document and aren't delayed by Gecko: - Async XHR events (the biggest issue)→but these don't have defaults on Gecko AFAIK, so should be fine. - Message events (posted to window)→we don't use Gecko's anyway. - Storage events, but again, we don't use Gecko's anyway, so it's fine. - Possibly other such similar events? That said, such events would need to be sent/posted by Gecko before the event spin loop or during it to pose a problem. *And* they'd need to have defaults (on Gecko, not IE). I did mention in the comment but it's probably not very obvious. If you have suggestions how to reword the comment please let me know. Back to my question, it's "none that we care", right? In that case, I'm not sure if event dispatcher is the right place to handle it. We already have some custom delaying code both for Message and Storage events, we could probably replace it by something like `post_event()` that would take care of both blocked delays. For XHR we need a separated mechanism anyway.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2098#note_23390
On Wed Feb 8 01:17:17 2023 +0000, Jacek Caban wrote:
Back to my question, it's "none that we care", right? In that case, I'm not sure if event dispatcher is the right place to handle it. We already have some custom delaying code both for Message and Storage events, we could probably replace it by something like `post_event()` that would take care of both blocked delays. For XHR we need a separated mechanism anyway. I think so, but I'm worried about other events dispatched to the window (in handle_htmlevent). Should I special case them? (with post_event)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2098#note_23448
On Wed Feb 8 15:59:03 2023 +0000, Gabriel Ivăncescu wrote:
I think so, but I'm worried about other events dispatched to the window (in handle_htmlevent). Should I special case them? (with post_event) I found a bit more other issues (some were existing now already), for example with readystatechange tasks. It's deeper than just dispatch_event, they use call_property_onchanged for example, but also dispatch the event.
I'm thinking to rewrite this in more generic way using different task lists, one for current tasks, one for event tasks, and one for async XHRs. Then I'll conditionally dispatch or post the event in nsevents (for window), the rest can stay dispatch_event since they're either synchronous or sent to document (on Gecko side, since we care whether it delays them or not). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2098#note_23587
On Thu Feb 9 15:26:12 2023 +0000, Gabriel Ivăncescu wrote:
I found a bit more other issues (some were existing now already), for example with readystatechange tasks. It's deeper than just dispatch_event, they use call_property_onchanged for example, but also dispatch the event. I'm thinking to rewrite this in more generic way using different task lists, one for current tasks, one for event tasks, and one for async XHRs. Then I'll conditionally dispatch or post the event in nsevents (for window), the rest can stay dispatch_event since they're either synchronous or sent to document (on Gecko side, since we care whether it delays them or not). I'm not sure about the details, but yes, moving delaying code out of `dispatch_event()` (to its callers) seems right. That should make the blocking actually simpler: I think that we don't need depth-based testing. I'd expect that async events should be able to be simply blocked or not.
Using thread data is also a bit questionable. We could have two separated iframes or even totally separated HTML document instances (as in HTMLDocumentObj) and with your solution they'd block each other. I guess that's why Gecko uses per-document blocking. Maybe it would be a good idea to do something similar? Note that even XHRs can be associated with a document that was used to create them. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2098#note_23607
On Thu Feb 9 16:20:38 2023 +0000, Jacek Caban wrote:
I'm not sure about the details, but yes, moving delaying code out of `dispatch_event()` (to its callers) seems right. That should make the blocking actually simpler: I think that we don't need depth-based testing. I'd expect that async events should be able to be simply blocked or not. Using thread data is also a bit questionable. We could have two separated iframes or even totally separated HTML document instances (as in HTMLDocumentObj) and with your solution they'd block each other. I guess that's why Gecko uses per-document blocking. Maybe it would be a good idea to do something similar? Note that even XHRs can be associated with a document that was used to create them. Isn't javascript still "single threaded" (or rather, sequential) with two separate iframes? It would be pretty weird to do a "sync" send() and end up in an unrelated handler from such script. This should extend to more than just script though, should exhibit it from C code as well.
Good point about multiple HTMLDocumentObj, I'll see if I can add some tests for that and for iframes. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2098#note_23610
On Thu Feb 9 16:44:19 2023 +0000, Gabriel Ivăncescu wrote:
Isn't javascript still "single threaded" (or rather, sequential) with two separate iframes? It would be pretty weird to do a "sync" send() and end up in an unrelated handler from such script. This should extend to more than just script though, should exhibit it from C code as well. Good point about multiple HTMLDocumentObj, I'll see if I can add some tests for that and for iframes. A single JS is "single threaded" in that sense, yes, but that it's not clear what does it mean for scripts running in separated contexts. For example, I'm not sure, but I think that Gecko blocks events in the document and its child frames, but not parents.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2098#note_23613
participants (3)
-
Gabriel Ivăncescu -
Jacek Caban (＠jacek)
-
Marvin