[PATCH v4 0/8] 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. -- v4: mshtml: Send all readystatechange events for synchronous XHRs in IE9 mshtml: Implement synchronous XMLHttpRequest. mshtml: Add separate task list for tasks dispatching events. mshtml: Track readyState in XHRs and report it manually. mshtml: Register all event handlers when creating the XMLHttpRequest. mshtml: Pass optional args to XMLHttpRequest.open() correctly. mshtml: Free the task after the destructor. mshtml: Use proper types for readystate_locked and readystate_pending. 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/mshtml_private.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index bec982a75fc..1873799dbf9 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -568,8 +568,8 @@ struct HTMLOuterWindow { struct list browser_entry; READYSTATE readystate; - BOOL readystate_locked; - unsigned readystate_pending; + unsigned readystate_locked; + BOOL readystate_pending; HTMLInnerWindow *pending_window; HTMLLocation location; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2098
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> And get rid of default_task_destr since it's only used in one place anyway. It was already confusing and leaking in some cases. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/navigate.c | 4 ---- dlls/mshtml/nsio.c | 1 - dlls/mshtml/olecmd.c | 1 - dlls/mshtml/persist.c | 7 +++++-- dlls/mshtml/task.c | 15 +++++---------- 5 files changed, 10 insertions(+), 18 deletions(-) diff --git a/dlls/mshtml/navigate.c b/dlls/mshtml/navigate.c index fd92483cbf5..b8d7937f071 100644 --- a/dlls/mshtml/navigate.c +++ b/dlls/mshtml/navigate.c @@ -1415,7 +1415,6 @@ static void stop_request_task_destr(task_t *_task) stop_request_task_t *task = (stop_request_task_t*)_task; IBindStatusCallback_Release(&task->bsc->bsc.IBindStatusCallback_iface); - free(task); } static HRESULT async_stop_request(nsChannelBSC *This) @@ -1950,7 +1949,6 @@ static void start_doc_binding_task_destr(task_t *_task) start_doc_binding_task_t *task = (start_doc_binding_task_t*)_task; IHTMLWindow2_Release(&task->pending_window->base.IHTMLWindow2_iface); - free(task); } HRESULT async_start_doc_binding(HTMLOuterWindow *window, HTMLInnerWindow *pending_window, DWORD flags) @@ -2107,7 +2105,6 @@ static void navigate_javascript_task_destr(task_t *_task) navigate_javascript_task_t *task = (navigate_javascript_task_t*)_task; IUri_Release(task->uri); - free(task); } typedef struct { @@ -2139,7 +2136,6 @@ static void navigate_task_destr(task_t *_task) IBindStatusCallback_Release(&task->bscallback->bsc.IBindStatusCallback_iface); IMoniker_Release(task->mon); IUri_Release(task->uri); - free(task); } static HRESULT navigate_fragment(HTMLOuterWindow *window, IUri *uri) diff --git a/dlls/mshtml/nsio.c b/dlls/mshtml/nsio.c index 612d91ed386..509a83efbd3 100644 --- a/dlls/mshtml/nsio.c +++ b/dlls/mshtml/nsio.c @@ -986,7 +986,6 @@ static void start_binding_task_destr(task_t *_task) start_binding_task_t *task = (start_binding_task_t*)_task; IBindStatusCallback_Release(&task->bscallback->bsc.IBindStatusCallback_iface); - free(task); } static nsresult async_open(nsChannel *This, HTMLOuterWindow *window, BOOL is_doc_channel, UINT32 load_type, diff --git a/dlls/mshtml/olecmd.c b/dlls/mshtml/olecmd.c index 5767751ac14..9e59778ccb9 100644 --- a/dlls/mshtml/olecmd.c +++ b/dlls/mshtml/olecmd.c @@ -446,7 +446,6 @@ static void refresh_destr(task_t *_task) refresh_task_t *task = (refresh_task_t*)_task; IHTMLWindow2_Release(&task->window->base.IHTMLWindow2_iface); - free(task); } HRESULT reload_page(HTMLOuterWindow *window) diff --git a/dlls/mshtml/persist.c b/dlls/mshtml/persist.c index 581a25c4e2e..e527b3dc3c5 100644 --- a/dlls/mshtml/persist.c +++ b/dlls/mshtml/persist.c @@ -240,6 +240,10 @@ static void set_progress_proc(task_t *_task) } } +static void set_progress_destr(task_t *_task) +{ +} + static void set_downloading_proc(task_t *_task) { download_proc_task_t *task = (download_proc_task_t*)_task; @@ -275,7 +279,6 @@ static void set_downloading_task_destr(task_t *_task) download_proc_task_t *task = (download_proc_task_t*)_task; CoTaskMemFree(task->url); - free(task); } void prepare_for_binding(HTMLDocumentObj *This, IMoniker *mon, DWORD flags) @@ -406,7 +409,7 @@ HRESULT set_moniker(HTMLOuterWindow *window, IMoniker *mon, IUri *nav_uri, IBind task = malloc(sizeof(docobj_task_t)); task->doc = doc_obj; - hres = push_task(&task->header, set_progress_proc, NULL, doc_obj->task_magic); + hres = push_task(&task->header, set_progress_proc, set_progress_destr, doc_obj->task_magic); if(FAILED(hres)) { CoTaskMemFree(url); return hres; diff --git a/dlls/mshtml/task.c b/dlls/mshtml/task.c index 2987b4bf19d..34b6c711ddd 100644 --- a/dlls/mshtml/task.c +++ b/dlls/mshtml/task.c @@ -46,27 +46,20 @@ typedef struct { struct list entry; } task_timer_t; -static void default_task_destr(task_t *task) -{ - free(task); -} - HRESULT push_task(task_t *task, task_proc_t proc, task_proc_t destr, LONG magic) { thread_data_t *thread_data; thread_data = get_thread_data(TRUE); if(!thread_data) { - if(destr) - destr(task); - else - free(task); + destr(task); + free(task); return E_OUTOFMEMORY; } task->target_magic = magic; task->proc = proc; - task->destr = destr ? destr : default_task_destr; + task->destr = destr; list_add_tail(&thread_data->task_list, &task->entry); @@ -128,6 +121,7 @@ void remove_target_tasks(LONG target) if(task->target_magic == target) { list_remove(&task->entry); task->destr(task); + free(task); } } } @@ -341,6 +335,7 @@ static LRESULT WINAPI hidden_proc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lPa task->proc(task); task->destr(task); + free(task); } return 0; -- 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 | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/dlls/mshtml/xmlhttprequest.c b/dlls/mshtml/xmlhttprequest.c index b4269bbe8d1..dff54abb8e2 100644 --- a/dlls/mshtml/xmlhttprequest.c +++ b/dlls/mshtml/xmlhttprequest.c @@ -546,8 +546,9 @@ static HRESULT HTMLXMLHttpRequest_open_hook(DispatchEx *dispex, WORD flags, static HRESULT WINAPI HTMLXMLHttpRequest_open(IHTMLXMLHttpRequest *iface, BSTR bstrMethod, BSTR bstrUrl, VARIANT varAsync, VARIANT varUser, VARIANT varPassword) { HTMLXMLHttpRequest *This = impl_from_IHTMLXMLHttpRequest(iface); - nsACString method, url; nsAString user, password; + nsACString method, url; + unsigned opt_argc = 1; nsresult nsres; HRESULT hres; @@ -594,8 +595,11 @@ static HRESULT WINAPI HTMLXMLHttpRequest_open(IHTMLXMLHttpRequest *iface, BSTR b return hres; } - nsres = nsIXMLHttpRequest_Open(This->nsxhr, &method, &url, TRUE, - &user, &password, 0); + if(V_VT(&varPassword) != VT_EMPTY && V_VT(&varPassword) != VT_ERROR) + opt_argc += 2; + else if(V_VT(&varUser) != VT_EMPTY && V_VT(&varUser) != VT_ERROR) + opt_argc += 1; + nsres = nsIXMLHttpRequest_Open(This->nsxhr, &method, &url, !!V_BOOL(&varAsync), &user, &password, opt_argc); 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 we can properly track state for synchronous XHRs. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/xmlhttprequest.c | 90 +++++++++++++++++------------------- 1 file changed, 42 insertions(+), 48 deletions(-) diff --git a/dlls/mshtml/xmlhttprequest.c b/dlls/mshtml/xmlhttprequest.c index dff54abb8e2..8a2dc281b77 100644 --- a/dlls/mshtml/xmlhttprequest.c +++ b/dlls/mshtml/xmlhttprequest.c @@ -128,7 +128,6 @@ typedef struct { nsIDOMEventListener nsIDOMEventListener_iface; LONG ref; HTMLXMLHttpRequest *xhr; - DWORD events_mask; } XMLHttpReqEventListener; struct HTMLXMLHttpRequest { @@ -146,16 +145,14 @@ struct HTMLXMLHttpRequest { static void detach_xhr_event_listener(XMLHttpReqEventListener *event_listener) { nsIDOMEventTarget *event_target; - DWORD events_mask, i; - nsAString str; nsresult nsres; + nsAString str; + unsigned i; nsres = nsIXMLHttpRequest_QueryInterface(event_listener->xhr->nsxhr, &IID_nsIDOMEventTarget, (void**)&event_target); assert(nsres == NS_OK); - for(events_mask = event_listener->events_mask, i = 0; events_mask; events_mask >>= 1, i++) { - if(!(events_mask & 1)) - continue; + for(i = 0; i < ARRAY_SIZE(events) ; i++) { nsAString_InitDepend(&str, get_event_name(events[i])); nsres = nsIDOMEventTarget_RemoveEventListener(event_target, &str, &event_listener->nsIDOMEventListener_iface, FALSE); nsAString_Finish(&str); @@ -164,7 +161,6 @@ static void detach_xhr_event_listener(XMLHttpReqEventListener *event_listener) nsIDOMEventTarget_Release(event_target); - event_listener->xhr->event_listener = NULL; event_listener->xhr = NULL; nsIDOMEventListener_Release(&event_listener->nsIDOMEventListener_iface); } @@ -298,8 +294,7 @@ static ULONG WINAPI HTMLXMLHttpRequest_Release(IHTMLXMLHttpRequest *iface) TRACE("(%p) ref=%ld\n", This, ref); if(!ref) { - if(This->event_listener) - detach_xhr_event_listener(This->event_listener); + detach_xhr_event_listener(This->event_listener); release_event_target(&This->event_target); release_dispex(&This->event_target.dispex); nsIXMLHttpRequest_Release(This->nsxhr); @@ -1287,45 +1282,8 @@ static nsISupports *HTMLXMLHttpRequest_get_gecko_target(DispatchEx *dispex) static void HTMLXMLHttpRequest_bind_event(DispatchEx *dispex, eventid_t eid) { - HTMLXMLHttpRequest *This = impl_from_DispatchEx(dispex); - nsIDOMEventTarget *nstarget; - nsAString type_str; - const WCHAR *name; - nsresult nsres; - unsigned i; - - TRACE("(%p)\n", This); - - for(i = 0; i < ARRAY_SIZE(events); i++) - if(eid == events[i]) - break; - if(i >= ARRAY_SIZE(events)) - return; - - if(!This->event_listener) { - This->event_listener = malloc(sizeof(*This->event_listener)); - if(!This->event_listener) - return; - - This->event_listener->nsIDOMEventListener_iface.lpVtbl = &XMLHttpReqEventListenerVtbl; - This->event_listener->ref = 1; - This->event_listener->xhr = This; - This->event_listener->events_mask = 0; - } - - nsres = nsIXMLHttpRequest_QueryInterface(This->nsxhr, &IID_nsIDOMEventTarget, (void**)&nstarget); - assert(nsres == NS_OK); - - name = get_event_name(events[i]); - nsAString_InitDepend(&type_str, name); - nsres = nsIDOMEventTarget_AddEventListener(nstarget, &type_str, &This->event_listener->nsIDOMEventListener_iface, FALSE, TRUE, 2); - nsAString_Finish(&type_str); - if(NS_FAILED(nsres)) - ERR("AddEventListener(%s) failed: %08lx\n", debugstr_w(name), nsres); - - nsIDOMEventTarget_Release(nstarget); - - This->event_listener->events_mask |= 1 << i; + /* Do nothing. To be able to track state and queue events manually, when blocked + * by sync XHRs in their send() event loop, we always register the handlers. */ } static void HTMLXMLHttpRequest_init_dispex_info(dispex_data_t *info, compat_mode_t compat_mode) @@ -1471,6 +1429,10 @@ static HRESULT WINAPI HTMLXMLHttpRequestFactory_create(IHTMLXMLHttpRequestFactor HTMLXMLHttpRequestFactory *This = impl_from_IHTMLXMLHttpRequestFactory(iface); HTMLXMLHttpRequest *ret; nsIXMLHttpRequest *nsxhr; + nsIDOMEventTarget *nstarget; + XMLHttpReqEventListener *event_listener; + nsresult nsres; + unsigned i; TRACE("(%p)->(%p)\n", This, p); @@ -1483,6 +1445,14 @@ static HRESULT WINAPI HTMLXMLHttpRequestFactory_create(IHTMLXMLHttpRequestFactor nsIXMLHttpRequest_Release(nsxhr); return E_OUTOFMEMORY; } + + event_listener = malloc(sizeof(*event_listener)); + if(!event_listener) { + free(ret); + nsIXMLHttpRequest_Release(nsxhr); + return E_OUTOFMEMORY; + } + ret->nsxhr = nsxhr; ret->IHTMLXMLHttpRequest_iface.lpVtbl = &HTMLXMLHttpRequestVtbl; @@ -1493,6 +1463,30 @@ static HRESULT WINAPI HTMLXMLHttpRequestFactory_create(IHTMLXMLHttpRequestFactor &HTMLXMLHttpRequest_dispex, This->window->doc->document_mode); ret->ref = 1; + /* Always register the handlers because we need them to track state */ + event_listener->nsIDOMEventListener_iface.lpVtbl = &XMLHttpReqEventListenerVtbl; + event_listener->ref = 1; + event_listener->xhr = ret; + ret->event_listener = event_listener; + + nsres = nsIXMLHttpRequest_QueryInterface(nsxhr, &IID_nsIDOMEventTarget, (void**)&nstarget); + assert(nsres == NS_OK); + + for(i = 0; i < ARRAY_SIZE(events); i++) { + const WCHAR *name = get_event_name(events[i]); + nsAString type_str; + + nsAString_InitDepend(&type_str, name); + nsres = nsIDOMEventTarget_AddEventListener(nstarget, &type_str, &event_listener->nsIDOMEventListener_iface, FALSE, TRUE, 2); + nsAString_Finish(&type_str); + if(NS_FAILED(nsres)) { + WARN("AddEventListener(%s) failed: %08lx\n", debugstr_w(name), nsres); + IHTMLXMLHttpRequest_Release(&ret->IHTMLXMLHttpRequest_iface); + return map_nsresult(nsres); + } + } + nsIDOMEventTarget_Release(nstarget); + *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> 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 | 71 ++++++++++++++++-------------------- 1 file changed, 31 insertions(+), 40 deletions(-) diff --git a/dlls/mshtml/xmlhttprequest.c b/dlls/mshtml/xmlhttprequest.c index 8a2dc281b77..db006e12bae 100644 --- a/dlls/mshtml/xmlhttprequest.c +++ b/dlls/mshtml/xmlhttprequest.c @@ -137,6 +137,7 @@ struct HTMLXMLHttpRequest { IWineXMLHttpRequestPrivate IWineXMLHttpRequestPrivate_iface; IProvideClassInfo2 IProvideClassInfo2_iface; LONG ref; + LONG readyState; response_type_t response_type; nsIXMLHttpRequest *nsxhr; XMLHttpReqEventListener *event_listener; @@ -220,6 +221,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; @@ -228,6 +230,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); @@ -339,19 +344,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; } @@ -373,6 +371,11 @@ static HRESULT WINAPI HTMLXMLHttpRequest_get_responseText(IHTMLXMLHttpRequest *i if(!p) return E_POINTER; + if(This->readyState < READYSTATE_INTERACTIVE) { + *p = NULL; + return S_OK; + } + nsAString_Init(&nsstr, NULL); nsres = nsIXMLHttpRequest_GetResponseText(This->nsxhr, &nsstr); return return_nsstr(nsres, &nsstr, p); @@ -389,6 +392,11 @@ static HRESULT WINAPI HTMLXMLHttpRequest_get_responseXML(IHTMLXMLHttpRequest *if TRACE("(%p)->(%p)\n", This, p); + if(This->readyState < READYSTATE_COMPLETE) { + *p = NULL; + return S_OK; + } + if(dispex_compat_mode(&This->event_target.dispex) >= COMPAT_MODE_IE10) { nsIDOMDocument *nsdoc; nsresult nsres; @@ -443,6 +451,11 @@ static HRESULT WINAPI HTMLXMLHttpRequest_get_status(IHTMLXMLHttpRequest *iface, if(!p) return E_POINTER; + if(This->readyState < READYSTATE_LOADED) { + *p = 0; + return E_FAIL; + } + nsres = nsIXMLHttpRequest_GetStatus(This->nsxhr, &val); if(NS_FAILED(nsres)) { ERR("nsIXMLHttpRequest_GetStatus failed: %08lx\n", nsres); @@ -460,19 +473,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 < READYSTATE_LOADED) { *p = NULL; return E_FAIL; } @@ -503,6 +510,7 @@ static HRESULT WINAPI HTMLXMLHttpRequest_get_onreadystatechange(IHTMLXMLHttpRequ static HRESULT WINAPI HTMLXMLHttpRequest_abort(IHTMLXMLHttpRequest *iface) { HTMLXMLHttpRequest *This = impl_from_IHTMLXMLHttpRequest(iface); + UINT16 readyState; nsresult nsres; TRACE("(%p)->()\n", This); @@ -513,6 +521,10 @@ static HRESULT WINAPI HTMLXMLHttpRequest_abort(IHTMLXMLHttpRequest *iface) return E_FAIL; } + /* Gecko changed to READYSTATE_UNINITIALIZED if it did abort */ + nsres = nsIXMLHttpRequest_GetReadyState(This->nsxhr, &readyState); + if(NS_SUCCEEDED(nsres)) + This->readyState = readyState; return S_OK; } @@ -656,19 +668,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 < READYSTATE_LOADED) { *p = NULL; return E_FAIL; } @@ -684,8 +690,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) @@ -693,11 +697,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 < READYSTATE_LOADED) { *p = NULL; return E_FAIL; } @@ -938,8 +938,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); @@ -957,8 +955,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 < READYSTATE_COMPLETE) { V_VT(p) = VT_EMPTY; break; } @@ -985,17 +982,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 < READYSTATE_LOADING || This->readyState > READYSTATE_INTERACTIVE) { /* FIXME: Return InvalidStateError */ return E_FAIL; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2098
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> They need to be handled separately because they are blocked by sync XHRs for the given window. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlstorage.c | 14 +++---- dlls/mshtml/htmlwindow.c | 16 +++----- dlls/mshtml/mshtml_private.h | 13 ++++++ dlls/mshtml/persist.c | 21 +++------- dlls/mshtml/script.c | 10 ++--- dlls/mshtml/task.c | 76 +++++++++++++++++++++++++++--------- 6 files changed, 92 insertions(+), 58 deletions(-) diff --git a/dlls/mshtml/htmlstorage.c b/dlls/mshtml/htmlstorage.c index 21e3df5c3fb..24eec4ba078 100644 --- a/dlls/mshtml/htmlstorage.c +++ b/dlls/mshtml/htmlstorage.c @@ -200,15 +200,14 @@ static inline HTMLStorage *impl_from_IHTMLStorage(IHTMLStorage *iface) static HRESULT build_session_origin(IUri*,BSTR,BSTR*); struct storage_event_task { - task_t header; - HTMLInnerWindow *window; + event_task_t header; DOMEvent *event; }; -static void storage_event_proc(task_t *_task) +static void storage_event_proc(event_task_t *_task) { struct storage_event_task *task = (struct storage_event_task*)_task; - HTMLInnerWindow *window = task->window; + HTMLInnerWindow *window = task->header.window; DOMEvent *event = task->event; VARIANT_BOOL cancelled; @@ -221,11 +220,10 @@ static void storage_event_proc(task_t *_task) } } -static void storage_event_destr(task_t *_task) +static void storage_event_destr(event_task_t *_task) { struct storage_event_task *task = (struct storage_event_task*)_task; IDOMEvent_Release(&task->event->IDOMEvent_iface); - IHTMLWindow2_Release(&task->window->base.IHTMLWindow2_iface); } struct send_storage_event_ctx { @@ -254,9 +252,7 @@ static HRESULT push_storage_event_task(struct send_storage_event_ctx *ctx, HTMLI } task->event = event; - task->window = window; - IHTMLWindow2_AddRef(&task->window->base.IHTMLWindow2_iface); - return push_task(&task->header, storage_event_proc, storage_event_destr, window->task_magic); + return push_event_task(&task->header, window, storage_event_proc, storage_event_destr, window->task_magic); } static HRESULT send_storage_event_impl(struct send_storage_event_ctx *ctx, HTMLInnerWindow *window) diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index ed497eacf4e..2d5db55c14d 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -2201,22 +2201,20 @@ static HRESULT WINAPI HTMLWindow6_get_maxConnectionsPerServer(IHTMLWindow6 *ifac } struct post_message_task { - task_t header; - HTMLInnerWindow *window; + event_task_t header; DOMEvent *event; -} ; +}; -static void post_message_proc(task_t *_task) +static void post_message_proc(event_task_t *_task) { struct post_message_task *task = (struct post_message_task *)_task; - dispatch_event(&task->window->event_target, task->event); + dispatch_event(&task->header.window->event_target, task->event); } -static void post_message_destr(task_t *_task) +static void post_message_destr(event_task_t *_task) { struct post_message_task *task = (struct post_message_task *)_task; IDOMEvent_Release(&task->event->IDOMEvent_iface); - IHTMLWindow2_Release(&task->window->base.IHTMLWindow2_iface); } static HRESULT WINAPI HTMLWindow6_postMessage(IHTMLWindow6 *iface, BSTR msg, VARIANT targetOrigin) @@ -3297,9 +3295,7 @@ static HRESULT WINAPI window_private_postMessage(IWineHTMLWindowPrivate *iface, } task->event = event; - task->window = window; - IHTMLWindow2_AddRef(&task->window->base.IHTMLWindow2_iface); - return push_task(&task->header, post_message_proc, post_message_destr, window->task_magic); + return push_event_task(&task->header, window, post_message_proc, post_message_destr, window->task_magic); } dispatch_event(&window->event_target, event); diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index 1873799dbf9..6f0a4043115 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -1283,6 +1283,17 @@ struct task_t { struct list entry; }; +typedef struct event_task_t event_task_t; +typedef void (*event_task_proc_t)(event_task_t*); + +struct event_task_t { + LONG target_magic; + event_task_proc_t proc; + event_task_proc_t destr; + struct list entry; + HTMLInnerWindow *window; +}; + typedef struct { task_t header; HTMLDocumentObj *doc; @@ -1291,6 +1302,7 @@ typedef struct { typedef struct { HWND thread_hwnd; struct list task_list; + struct list event_task_list; struct list timer_list; struct wine_rb_tree session_storage_map; } thread_data_t; @@ -1302,6 +1314,7 @@ void destroy_session_storage(thread_data_t*) DECLSPEC_HIDDEN; LONG get_task_target_magic(void) DECLSPEC_HIDDEN; HRESULT push_task(task_t*,task_proc_t,task_proc_t,LONG) DECLSPEC_HIDDEN; +HRESULT push_event_task(event_task_t*,HTMLInnerWindow*,event_task_proc_t,event_task_proc_t,LONG) DECLSPEC_HIDDEN; void remove_target_tasks(LONG) DECLSPEC_HIDDEN; ULONGLONG get_time_stamp(void) DECLSPEC_HIDDEN; diff --git a/dlls/mshtml/persist.c b/dlls/mshtml/persist.c index e527b3dc3c5..486643d8b21 100644 --- a/dlls/mshtml/persist.c +++ b/dlls/mshtml/persist.c @@ -455,21 +455,13 @@ static void notif_readystate(HTMLOuterWindow *window) } } -typedef struct { - task_t header; - HTMLOuterWindow *window; -} readystate_task_t; - -static void notif_readystate_proc(task_t *_task) +static void notif_readystate_proc(event_task_t *task) { - readystate_task_t *task = (readystate_task_t*)_task; - notif_readystate(task->window); + notif_readystate(task->window->base.outer_window); } -static void notif_readystate_destr(task_t *_task) +static void notif_readystate_destr(event_task_t *task) { - readystate_task_t *task = (readystate_task_t*)_task; - IHTMLWindow2_Release(&task->window->base.IHTMLWindow2_iface); } void set_ready_state(HTMLOuterWindow *window, READYSTATE readystate) @@ -479,7 +471,7 @@ void set_ready_state(HTMLOuterWindow *window, READYSTATE readystate) window->readystate = readystate; if(window->readystate_locked) { - readystate_task_t *task; + event_task_t *task; HRESULT hres; if(window->readystate_pending || prev_state == readystate) @@ -489,10 +481,7 @@ void set_ready_state(HTMLOuterWindow *window, READYSTATE readystate) if(!task) return; - IHTMLWindow2_AddRef(&window->base.IHTMLWindow2_iface); - task->window = window; - - hres = push_task(&task->header, notif_readystate_proc, notif_readystate_destr, window->task_magic); + hres = push_event_task(task, window->base.inner_window, notif_readystate_proc, notif_readystate_destr, window->task_magic); if(SUCCEEDED(hres)) window->readystate_pending = TRUE; return; diff --git a/dlls/mshtml/script.c b/dlls/mshtml/script.c index 8b1c1f11938..0e2241663dc 100644 --- a/dlls/mshtml/script.c +++ b/dlls/mshtml/script.c @@ -773,11 +773,11 @@ static void dispatch_script_readystatechange_event(HTMLScriptElement *script) } typedef struct { - task_t header; + event_task_t header; HTMLScriptElement *elem; } fire_readystatechange_task_t; -static void fire_readystatechange_proc(task_t *_task) +static void fire_readystatechange_proc(event_task_t *_task) { fire_readystatechange_task_t *task = (fire_readystatechange_task_t*)_task; @@ -788,7 +788,7 @@ static void fire_readystatechange_proc(task_t *_task) dispatch_script_readystatechange_event(task->elem); } -static void fire_readystatechange_task_destr(task_t *_task) +static void fire_readystatechange_task_destr(event_task_t *_task) { fire_readystatechange_task_t *task = (fire_readystatechange_task_t*)_task; @@ -818,8 +818,8 @@ static void set_script_elem_readystate(HTMLScriptElement *script_elem, READYSTAT IHTMLScriptElement_AddRef(&script_elem->IHTMLScriptElement_iface); task->elem = script_elem; - hres = push_task(&task->header, fire_readystatechange_proc, fire_readystatechange_task_destr, - script_elem->element.node.doc->window->task_magic); + hres = push_event_task(&task->header, script_elem->element.node.doc->window, fire_readystatechange_proc, + fire_readystatechange_task_destr, script_elem->element.node.doc->window->task_magic); if(SUCCEEDED(hres)) script_elem->pending_readystatechange_event = TRUE; }else { diff --git a/dlls/mshtml/task.c b/dlls/mshtml/task.c index 34b6c711ddd..37667dcc9cc 100644 --- a/dlls/mshtml/task.c +++ b/dlls/mshtml/task.c @@ -67,21 +67,33 @@ HRESULT push_task(task_t *task, task_proc_t proc, task_proc_t destr, LONG magic) return S_OK; } -static task_t *pop_task(void) +static void release_event_task(event_task_t *task) +{ + task->destr(task); + IHTMLWindow2_Release(&task->window->base.IHTMLWindow2_iface); + free(task); +} + +HRESULT push_event_task(event_task_t *task, HTMLInnerWindow *window, event_task_proc_t proc, event_task_proc_t destr, LONG magic) { thread_data_t *thread_data; - task_t *task; - thread_data = get_thread_data(FALSE); - if(!thread_data) - return NULL; + task->target_magic = magic; + task->proc = proc; + task->destr = destr; + task->window = window; + IHTMLWindow2_AddRef(&window->base.IHTMLWindow2_iface); - if(list_empty(&thread_data->task_list)) - return NULL; + thread_data = get_thread_data(TRUE); + if(!thread_data) { + release_event_task(task); + return E_OUTOFMEMORY; + } + + list_add_tail(&thread_data->event_task_list, &task->entry); - task = LIST_ENTRY(thread_data->task_list.next, task_t, entry); - list_remove(&task->entry); - return task; + PostMessageW(thread_data->thread_hwnd, WM_PROCESSTASK, 0, 0); + return S_OK; } static void release_task_timer(HWND thread_hwnd, task_timer_t *timer) @@ -124,6 +136,14 @@ void remove_target_tasks(LONG target) free(task); } } + + LIST_FOR_EACH_SAFE(liter, ltmp, &thread_data->event_task_list) { + event_task_t *task = LIST_ENTRY(liter, event_task_t, entry); + if(task->target_magic == target) { + list_remove(&task->entry); + release_event_task(task); + } + } } LONG get_task_target_magic(void) @@ -326,18 +346,37 @@ static LRESULT process_timer(void) static LRESULT WINAPI hidden_proc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) { + thread_data_t *thread_data; + switch(msg) { case WM_PROCESSTASK: - while(1) { - task_t *task = pop_task(); - if(!task) - break; + thread_data = get_thread_data(FALSE); + if(!thread_data) + return 0; - task->proc(task); - task->destr(task); - free(task); + while(1) { + struct list *head = list_head(&thread_data->task_list); + + if(head) { + task_t *task = LIST_ENTRY(head, task_t, entry); + list_remove(&task->entry); + task->proc(task); + task->destr(task); + free(task); + continue; + } + + head = list_head(&thread_data->event_task_list); + if(head) { + event_task_t *task = LIST_ENTRY(head, event_task_t, entry); + list_remove(&task->entry); + task->proc(task); + release_event_task(task); + continue; + } + + break; } - return 0; case WM_TIMER: return process_timer(); @@ -410,6 +449,7 @@ thread_data_t *get_thread_data(BOOL create) TlsSetValue(mshtml_tls, thread_data); list_init(&thread_data->task_list); + list_init(&thread_data->event_task_list); list_init(&thread_data->timer_list); wine_rb_init(&thread_data->session_storage_map, session_storage_map_cmp); } -- 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/htmlwindow.c | 2 + dlls/mshtml/mshtml_private.h | 5 + dlls/mshtml/task.c | 46 ++++- dlls/mshtml/tests/events.c | 231 +++++++++++++++++++-- dlls/mshtml/tests/rsrc.rc | 3 + dlls/mshtml/tests/script.c | 26 ++- dlls/mshtml/tests/xhr.js | 134 ++++++++++++ dlls/mshtml/tests/xhr_iframe.html | 23 +++ dlls/mshtml/tests/xmlhttprequest.c | 18 +- dlls/mshtml/xmlhttprequest.c | 315 +++++++++++++++++++++++++++-- 10 files changed, 741 insertions(+), 62 deletions(-) create mode 100644 dlls/mshtml/tests/xhr_iframe.html diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index 2d5db55c14d..c19896e419e 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -3294,6 +3294,8 @@ static HRESULT WINAPI window_private_postMessage(IWineHTMLWindowPrivate *iface, return E_OUTOFMEMORY; } + /* Because message events can be sent to different windows, they get blocked by any context */ + task->header.thread_blocked = TRUE; task->event = event; return push_event_task(&task->header, window, post_message_proc, post_message_destr, window->task_magic); } diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index 6f0a4043115..ed01ab20a39 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -607,6 +607,7 @@ struct HTMLInnerWindow { VARIANT performance; HTMLPerformanceTiming *performance_timing; + unsigned blocking_depth; unsigned parser_callback_cnt; struct list script_queue; @@ -1288,6 +1289,7 @@ typedef void (*event_task_proc_t)(event_task_t*); struct event_task_t { LONG target_magic; + BOOL thread_blocked; event_task_proc_t proc; event_task_proc_t destr; struct list entry; @@ -1304,11 +1306,14 @@ typedef struct { struct list task_list; struct list event_task_list; struct list timer_list; + struct list *pending_xhr_events_tail; struct wine_rb_tree session_storage_map; + unsigned int blocking_depth; } thread_data_t; thread_data_t *get_thread_data(BOOL) DECLSPEC_HIDDEN; HWND get_thread_hwnd(void) DECLSPEC_HIDDEN; +void unblock_tasks_and_timers(thread_data_t*) 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; diff --git a/dlls/mshtml/task.c b/dlls/mshtml/task.c index 37667dcc9cc..f9a569d96ae 100644 --- a/dlls/mshtml/task.c +++ b/dlls/mshtml/task.c @@ -96,6 +96,13 @@ HRESULT push_event_task(event_task_t *task, HTMLInnerWindow *window, event_task_ return S_OK; } +static void unlink_event_task(event_task_t *task, thread_data_t *thread_data) +{ + if(thread_data->pending_xhr_events_tail == &task->entry) + thread_data->pending_xhr_events_tail = task->entry.prev; + list_remove(&task->entry); +} + static void release_task_timer(HWND thread_hwnd, task_timer_t *timer) { list_remove(&timer->entry); @@ -140,7 +147,7 @@ void remove_target_tasks(LONG target) LIST_FOR_EACH_SAFE(liter, ltmp, &thread_data->event_task_list) { event_task_t *task = LIST_ENTRY(liter, event_task_t, entry); if(task->target_magic == target) { - list_remove(&task->entry); + unlink_event_task(task, thread_data); release_event_task(task); } } @@ -303,7 +310,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_depth) { KillTimer(thread_data->thread_hwnd, TIMER_ID); return 0; } @@ -338,7 +345,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_depth); KillTimer(thread_data->thread_hwnd, TIMER_ID); return 0; @@ -366,16 +373,19 @@ static LRESULT WINAPI hidden_proc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lPa continue; } - head = list_head(&thread_data->event_task_list); - if(head) { + head = &thread_data->event_task_list; + while((head = list_next(&thread_data->event_task_list, head))) { event_task_t *task = LIST_ENTRY(head, event_task_t, entry); - list_remove(&task->entry); - task->proc(task); - release_event_task(task); - continue; - } - break; + if((!task->thread_blocked || !thread_data->blocking_depth) && !task->window->blocking_depth) { + unlink_event_task(task, thread_data); + task->proc(task); + release_event_task(task); + break; + } + } + if(!head) + break; } return 0; case WM_TIMER: @@ -451,6 +461,7 @@ thread_data_t *get_thread_data(BOOL create) list_init(&thread_data->task_list); list_init(&thread_data->event_task_list); list_init(&thread_data->timer_list); + thread_data->pending_xhr_events_tail = &thread_data->event_task_list; wine_rb_init(&thread_data->session_storage_map, session_storage_map_cmp); } @@ -467,3 +478,16 @@ ULONGLONG get_time_stamp(void) GetSystemTimeAsFileTime(&time); return (((ULONGLONG)time.dwHighDateTime << 32) + time.dwLowDateTime) / 10000 - time_epoch; } + +void unblock_tasks_and_timers(thread_data_t *thread_data) +{ + if(!list_empty(&thread_data->event_task_list)) + PostMessageW(thread_data->thread_hwnd, WM_PROCESSTASK, 0, 0); + + if(!thread_data->blocking_depth && !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); + } +} diff --git a/dlls/mshtml/tests/events.c b/dlls/mshtml/tests/events.c index a410b464756..f278ae7fea2 100644 --- a/dlls/mshtml/tests/events.c +++ b/dlls/mshtml/tests/events.c @@ -111,6 +111,8 @@ DEFINE_EXPECT(window1_onstorage); DEFINE_EXPECT(doc2_onstorage); DEFINE_EXPECT(doc2_onstoragecommit); DEFINE_EXPECT(window2_onstorage); +DEFINE_EXPECT(async_xhr_done); +DEFINE_EXPECT(sync_xhr_done); static HWND container_hwnd = NULL; static IHTMLWindow2 *window; @@ -3730,6 +3732,60 @@ static HRESULT WINAPI window2_onstorage(IDispatchEx *iface, DISPID id, LCID lcid EVENT_HANDLER_FUNC_OBJ(window2_onstorage); +static HRESULT WINAPI async_xhr(IDispatchEx *iface, DISPID id, LCID lcid, WORD wFlags, DISPPARAMS *pdp, + VARIANT *pvarRes, EXCEPINFO *pei, IServiceProvider *pspCaller) +{ + IHTMLXMLHttpRequest *xhr; + LONG ready_state; + HRESULT hres; + + ok(pdp != NULL, "pdp == NULL\n"); + ok(pdp->cArgs == (document_mode < 9 ? 1 : 2), "pdp->cArgs = %d\n", pdp->cArgs); + ok(pdp->cNamedArgs == 1, "pdp->cNamedArgs = %d\n", pdp->cNamedArgs); + ok(pdp->rgdispidNamedArgs[0] == DISPID_THIS, "pdp->rgdispidNamedArgs[0] = %ld\n", pdp->rgdispidNamedArgs[0]); + ok(V_VT(pdp->rgvarg) == VT_DISPATCH, "V_VT(this) = %d\n", V_VT(pdp->rgvarg)); + ok(V_DISPATCH(pdp->rgvarg) != NULL, "V_DISPATCH(this) == NULL\n"); + + hres = IDispatch_QueryInterface(V_DISPATCH(pdp->rgvarg), &IID_IHTMLXMLHttpRequest, (void**)&xhr); + ok(hres == S_OK, "Could not get IHTMLXMLHttpRequest: %08lx\n", hres); + + hres = IHTMLXMLHttpRequest_get_readyState(xhr, &ready_state); + if(SUCCEEDED(hres) && ready_state == 4) + CHECK_EXPECT(async_xhr_done); + + IHTMLXMLHttpRequest_Release(xhr); + return S_OK; +} + +EVENT_HANDLER_FUNC_OBJ(async_xhr); + +static HRESULT WINAPI sync_xhr(IDispatchEx *iface, DISPID id, LCID lcid, WORD wFlags, DISPPARAMS *pdp, + VARIANT *pvarRes, EXCEPINFO *pei, IServiceProvider *pspCaller) +{ + IHTMLXMLHttpRequest *xhr; + LONG ready_state; + HRESULT hres; + + ok(pdp != NULL, "pdp == NULL\n"); + ok(pdp->cArgs == (document_mode < 9 ? 1 : 2), "pdp->cArgs = %d\n", pdp->cArgs); + ok(pdp->cNamedArgs == 1, "pdp->cNamedArgs = %d\n", pdp->cNamedArgs); + ok(pdp->rgdispidNamedArgs[0] == DISPID_THIS, "pdp->rgdispidNamedArgs[0] = %ld\n", pdp->rgdispidNamedArgs[0]); + ok(V_VT(pdp->rgvarg) == VT_DISPATCH, "V_VT(this) = %d\n", V_VT(pdp->rgvarg)); + ok(V_DISPATCH(pdp->rgvarg) != NULL, "V_DISPATCH(this) == NULL\n"); + + hres = IDispatch_QueryInterface(V_DISPATCH(pdp->rgvarg), &IID_IHTMLXMLHttpRequest, (void**)&xhr); + ok(hres == S_OK, "Could not get IHTMLXMLHttpRequest: %08lx\n", hres); + + hres = IHTMLXMLHttpRequest_get_readyState(xhr, &ready_state); + if(SUCCEEDED(hres) && ready_state == 4) + CHECK_EXPECT(sync_xhr_done); + + IHTMLXMLHttpRequest_Release(xhr); + return S_OK; +} + +EVENT_HANDLER_FUNC_OBJ(sync_xhr); + static HRESULT QueryInterface(REFIID,void**); static HRESULT browserservice_qi(REFIID,void**); static HRESULT wb_qi(REFIID,void**); @@ -5133,8 +5189,27 @@ typedef struct { ULONG size; const char *data; const char *ptr; + + HANDLE delay_event; + LONG delay; } ProtocolHandler; +static ProtocolHandler *delay_with_signal_handler; + +static DWORD WINAPI delay_proc(void *arg) +{ + PROTOCOLDATA protocol_data = {PI_FORCE_ASYNC}; + ProtocolHandler *protocol_handler = arg; + + 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; +} + static DWORD WINAPI async_switch_proc(void *arg) { PROTOCOLDATA protocol_data = { PI_FORCE_ASYNC }; @@ -5177,6 +5252,8 @@ static ULONG WINAPI Protocol_Release(IInternetProtocolEx *iface) LONG ref = InterlockedDecrement(&This->ref); if(!ref) { + if(This->delay_event) + CloseHandle(This->delay_event); if(This->sink) IInternetProtocolSink_Release(This->sink); if(This->uri) @@ -5203,25 +5280,34 @@ static HRESULT WINAPI Protocol_Continue(IInternetProtocolEx *iface, PROTOCOLDATA HRESULT hres; BSTR bstr; - hres = IInternetProtocolSink_QueryInterface(This->sink, &IID_IServiceProvider, (void**)&service_provider); - ok(hres == S_OK, "Could not get IServiceProvider iface: %08lx\n", hres); + if(This->delay >= 0) { + hres = IInternetProtocolSink_QueryInterface(This->sink, &IID_IServiceProvider, (void**)&service_provider); + ok(hres == S_OK, "Could not get IServiceProvider iface: %08lx\n", hres); - hres = IServiceProvider_QueryService(service_provider, &IID_IHttpNegotiate, &IID_IHttpNegotiate, (void**)&http_negotiate); - IServiceProvider_Release(service_provider); - ok(hres == S_OK, "Could not get IHttpNegotiate interface: %08lx\n", hres); + hres = IServiceProvider_QueryService(service_provider, &IID_IHttpNegotiate, &IID_IHttpNegotiate, (void**)&http_negotiate); + IServiceProvider_Release(service_provider); + ok(hres == S_OK, "Could not get IHttpNegotiate interface: %08lx\n", hres); - hres = IUri_GetDisplayUri(This->uri, &bstr); - ok(hres == S_OK, "Failed to get display uri: %08lx\n", hres); - hres = IHttpNegotiate_BeginningTransaction(http_negotiate, bstr, L"", 0, &addl_headers); - ok(hres == S_OK, "BeginningTransaction failed: %08lx\n", hres); - CoTaskMemFree(addl_headers); - SysFreeString(bstr); + hres = IUri_GetDisplayUri(This->uri, &bstr); + ok(hres == S_OK, "Failed to get display uri: %08lx\n", hres); + hres = IHttpNegotiate_BeginningTransaction(http_negotiate, bstr, L"", 0, &addl_headers); + ok(hres == S_OK, "BeginningTransaction failed: %08lx\n", hres); + CoTaskMemFree(addl_headers); + SysFreeString(bstr); - bstr = SysAllocString(L"HTTP/1.1 200 OK\r\nContent-Type: text/html\r\n\r\n"); - hres = IHttpNegotiate_OnResponse(http_negotiate, 200, bstr, NULL, NULL); - ok(hres == S_OK, "OnResponse failed: %08lx\n", hres); - IHttpNegotiate_Release(http_negotiate); - SysFreeString(bstr); + bstr = SysAllocString(L"HTTP/1.1 200 OK\r\nContent-Type: text/html\r\n\r\n"); + hres = IHttpNegotiate_OnResponse(http_negotiate, 200, bstr, NULL, NULL); + ok(hres == S_OK, "OnResponse failed: %08lx\n", hres); + IHttpNegotiate_Release(http_negotiate); + SysFreeString(bstr); + + if(This->delay || This->delay_event) { + IInternetProtocolEx_AddRef(&This->IInternetProtocolEx_iface); + QueueUserWorkItem(delay_proc, This, 0); + return S_OK; + } + } + This->delay = 0; hres = IInternetProtocolSink_ReportData(This->sink, BSCF_FIRSTDATANOTIFICATION | BSCF_LASTDATANOTIFICATION, This->size, This->size); @@ -5298,6 +5384,8 @@ static HRESULT WINAPI ProtocolEx_StartEx(IInternetProtocolEx *iface, IUri *uri, IInternetBindInfo *pOIBindInfo, DWORD grfPI, HANDLE *dwReserved) { ProtocolHandler *This = impl_from_IInternetProtocolEx(iface); + HRESULT hres; + BSTR query; This->data = protocol_doc_str; This->size = strlen(This->data); @@ -5306,6 +5394,23 @@ static HRESULT WINAPI ProtocolEx_StartEx(IInternetProtocolEx *iface, IUri *uri, IUri_AddRef(This->uri = uri); This->ptr = This->data; + hres = IUri_GetQuery(uri, &query); + if(hres == S_OK) { + 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()); + } + } + SysFreeString(query); + } + IInternetProtocolEx_AddRef(&This->IInternetProtocolEx_iface); QueueUserWorkItem(async_switch_proc, This, 0); return E_PENDING; @@ -6008,6 +6113,95 @@ done: IHTMLDocument2_Release(doc[1]); } +static void test_sync_xhr_events(const char *doc_str) +{ + IHTMLXMLHttpRequest *xhr[2]; + IHTMLDocument2 *doc[2]; + IHTMLDocument6 *doc6; + VARIANT var, vempty; + HRESULT hres; + unsigned i; + + for(i = 0; i < ARRAY_SIZE(doc); i++) + doc[i] = create_document_with_origin(doc_str); + + document_mode = 0; + V_VT(&vempty) = VT_EMPTY; + + hres = IHTMLDocument2_QueryInterface(doc[0], &IID_IHTMLDocument6, (void**)&doc6); + if(SUCCEEDED(hres)) { + hres = IHTMLDocument6_get_documentMode(doc6, &var); + ok(hres == S_OK, "get_documentMode failed: %08lx\n", hres); + ok(V_VT(&var) == VT_R4, "V_VT(documentMode) = %u\n", V_VT(&var)); + document_mode = V_R4(&var); + IHTMLDocument6_Release(doc6); + } + + for(i = 0; i < ARRAY_SIZE(doc); i++) { + IHTMLXMLHttpRequestFactory *ctor; + IHTMLWindow5 *window5; + IHTMLWindow2 *window; + BSTR bstr, method; + + hres = IHTMLDocument2_get_parentWindow(doc[i], &window); + ok(hres == S_OK, "[%u] get_parentWindow failed: %08lx\n", i, hres); + ok(window != NULL, "[%u] window == NULL\n", i); + + hres = IHTMLWindow2_QueryInterface(window, &IID_IHTMLWindow5, (void**)&window5); + ok(hres == S_OK, "[%u] Could not get IHTMLWindow5: %08lx\n", i, hres); + IHTMLWindow2_Release(window); + + hres = IHTMLWindow5_get_XMLHttpRequest(window5, &var); + ok(hres == S_OK, "[%u] get_XMLHttpRequest failed: %08lx\n", i, hres); + ok(V_VT(&var) == VT_DISPATCH, "[%u] V_VT(XMLHttpRequest) == %d\n", i, V_VT(&var)); + ok(V_DISPATCH(&var) != NULL, "[%u] V_DISPATCH(XMLHttpRequest) == NULL\n", i); + IHTMLWindow5_Release(window5); + + hres = IDispatch_QueryInterface(V_DISPATCH(&var), &IID_IHTMLXMLHttpRequestFactory, (void**)&ctor); + ok(hres == S_OK, "[%u] Could not get IHTMLXMLHttpRequestFactory: %08lx\n", i, hres); + IDispatch_Release(V_DISPATCH(&var)); + hres = IHTMLXMLHttpRequestFactory_create(ctor, &xhr[i]); + ok(hres == S_OK, "[%u] create failed: %08lx\n", i, hres); + IHTMLXMLHttpRequestFactory_Release(ctor); + + V_VT(&var) = VT_BOOL; + V_BOOL(&var) = i ? VARIANT_FALSE : VARIANT_TRUE; + method = SysAllocString(L"GET"); + bstr = SysAllocString(L"blank.html?delay_with_signal"); + hres = IHTMLXMLHttpRequest_open(xhr[i], method, bstr, var, vempty, vempty); + ok(hres == S_OK, "[%u] open failed: %08lx\n", i, hres); + SysFreeString(method); + SysFreeString(bstr); + + V_VT(&var) = VT_DISPATCH; + V_DISPATCH(&var) = (IDispatch*)(i ? &sync_xhr_obj : &async_xhr_obj); + hres = IHTMLXMLHttpRequest_put_onreadystatechange(xhr[i], var); + ok(hres == S_OK, "[%u] put_onreadystatechange failed: %08lx\n", i, hres); + } + + /* async xhr */ + hres = IHTMLXMLHttpRequest_send(xhr[0], vempty); + ok(hres == S_OK, "async xhr send failed: %08lx\n", hres); + + /* sync xhr */ + SET_EXPECT(sync_xhr_done); + hres = IHTMLXMLHttpRequest_send(xhr[1], vempty); + ok(hres == S_OK, "sync xhr send failed: %08lx\n", hres); + CHECK_CALLED(sync_xhr_done); + + SET_EXPECT(async_xhr_done); + pump_msgs(&called_async_xhr_done); + CHECK_CALLED(async_xhr_done); + + for(i = 0; i < ARRAY_SIZE(xhr); i++) + IHTMLXMLHttpRequest_Release(xhr[i]); + + set_client_site(doc[0], FALSE); + set_client_site(doc[1], FALSE); + IHTMLDocument2_Release(doc[0]); + IHTMLDocument2_Release(doc[1]); +} + static BOOL check_ie(void) { IHTMLDocument2 *doc; @@ -6069,8 +6263,11 @@ START_TEST(events) test_empty_document(); test_storage_events(empty_doc_str); - if(is_ie9plus) + test_sync_xhr_events(empty_doc_str); + if(is_ie9plus) { test_storage_events(empty_doc_ie9_str); + test_sync_xhr_events(empty_doc_ie9_str); + } DestroyWindow(container_hwnd); }else { diff --git a/dlls/mshtml/tests/rsrc.rc b/dlls/mshtml/tests/rsrc.rc index 10b92ab78cb..61b53520781 100644 --- a/dlls/mshtml/tests/rsrc.rc +++ b/dlls/mshtml/tests/rsrc.rc @@ -88,6 +88,9 @@ doc_with_prop_ie9.html HTML "doc_with_prop_ie9.html" /* @makedep: iframe.html */ iframe.html HTML "iframe.html" +/* @makedep: xhr_iframe.html */ +xhr_iframe.html HTML "xhr_iframe.html" + /* For res: protocol test: */ /* @makedep: jstest.html */ diff --git a/dlls/mshtml/tests/script.c b/dlls/mshtml/tests/script.c index 2bcd588f51f..cd27a05e397 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; @@ -3880,6 +3886,8 @@ static ULONG WINAPI Protocol_Release(IInternetProtocolEx *iface) LONG ref = InterlockedDecrement(&This->ref); if(!ref) { + if(This->delay_event) + CloseHandle(This->delay_event); if(This->sink) IInternetProtocolSink_Release(This->sink); if(This->stream) @@ -4061,8 +4069,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..ebe1a44168a 100644 --- a/dlls/mshtml/tests/xhr.js +++ b/dlls/mshtml/tests/xhr.js @@ -76,6 +76,139 @@ 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 ]; + function onmsg(e) { a.push("msg" + e.data); } + document.ondblclick = function() { a.push("dblclick"); }; + window.addEventListener("message", onmsg); + 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.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); + window.removeEventListener("message", onmsg); + document.ondblclick = null; + a = [ 0 ]; + + // Events dispatched to other iframes are not blocked by a send() in another context, + // except for async XHR events (which are a special case again), messages, and timeouts. + var iframe = document.createElement("iframe"), iframe2 = document.createElement("iframe"); + iframe.onload = function() { + iframe2.onload = function() { + function onmsg(e) { + a.push(e.data); + if(e.data === "echo") + iframe2.contentWindow.postMessage("sync_xhr", "*"); + }; + + window.setTimeout(function() { + var r = a.join(","); + ok(r === "0,1,async_xhr,echo,sync_xhr(pre-send),sync_xhr(DONE),sync_xhr,async_xhr(DONE)", + "[iframes 1] unexpected order: " + r); + a = [ 0 ]; + + window.setTimeout(function() { + var r = a.join(","); + ok(r === "0,1,echo,blank(DONE),sync_xhr(pre-send),sync_xhr(DONE),sync_xhr", + "[iframes 2] unexpected order: " + r); + window.removeEventListener("message", onmsg); + next_test(); + }, 0); + + iframe.onload = function() { a.push("blank(DONE)"); }; + iframe.src = "blank.html?delay_with_signal"; + iframe2.contentWindow.postMessage("echo", "*"); + a.push(1); + }, 0); + + window.addEventListener("message", onmsg); + iframe.contentWindow.postMessage("async_xhr", "*"); + iframe2.contentWindow.postMessage("echo", "*"); + a.push(1); + }; + iframe2.src = "xhr_iframe.html"; + document.body.appendChild(iframe2); + }; + iframe.src = "xhr_iframe.html"; + document.body.appendChild(iframe); + }, 0); +} + function test_content_types() { var xhr = new XMLHttpRequest(), types, i = 0, override = false; var v = document.documentMode; @@ -291,6 +424,7 @@ function test_response() { var tests = [ test_xhr, + test_sync_xhr, test_content_types, test_abort, test_timeout, diff --git a/dlls/mshtml/tests/xhr_iframe.html b/dlls/mshtml/tests/xhr_iframe.html new file mode 100644 index 00000000000..1a683f700e1 --- /dev/null +++ b/dlls/mshtml/tests/xhr_iframe.html @@ -0,0 +1,23 @@ +<html><head><script type="text/javascript">window.onmessage = function(e) +{ + if(e.data === "echo") + parent.postMessage("echo", "*"); + else if(e.data === "async_xhr") { + var async_xhr = new XMLHttpRequest(); + async_xhr.open("POST", "echo.php?delay_with_signal", true); + async_xhr.onreadystatechange = function() { if(async_xhr.readyState == 4) parent.postMessage("async_xhr(DONE)", "*"); }; + async_xhr.setRequestHeader("X-Test", "True"); + async_xhr.send("foo"); + parent.postMessage("async_xhr", "*"); + } + else if(e.data === "sync_xhr") { + var sync_xhr = new XMLHttpRequest(); + sync_xhr.open("POST", "echo.php?delay_with_signal", false); + sync_xhr.onreadystatechange = function() { if(sync_xhr.readyState == 4) parent.postMessage("sync_xhr(DONE)", "*"); }; + sync_xhr.setRequestHeader("X-Test", "True"); + parent.postMessage("sync_xhr(pre-send)", "*"); + sync_xhr.send("bar"); + parent.postMessage("sync_xhr", "*"); + } +} +</script><body></body></html> 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 db006e12bae..aceef111998 100644 --- a/dlls/mshtml/xmlhttprequest.c +++ b/dlls/mshtml/xmlhttprequest.c @@ -137,10 +137,17 @@ struct HTMLXMLHttpRequest { IWineXMLHttpRequestPrivate IWineXMLHttpRequestPrivate_iface; IProvideClassInfo2 IProvideClassInfo2_iface; LONG ref; + LONG task_magic; LONG readyState; response_type_t response_type; + BOOLEAN synchronous; + DWORD magic; + DWORD pending_events_magic; + HTMLInnerWindow *window; nsIXMLHttpRequest *nsxhr; XMLHttpReqEventListener *event_listener; + DOMEvent *pending_progress_event; + size_t pending_progress_event_text_len; }; static void detach_xhr_event_listener(XMLHttpReqEventListener *event_listener) @@ -166,6 +173,155 @@ static void detach_xhr_event_listener(XMLHttpReqEventListener *event_listener) nsIDOMEventListener_Release(&event_listener->nsIDOMEventListener_iface); } +static nsresult sync_xhr_send(HTMLXMLHttpRequest *xhr, nsIVariant *nsbody) +{ + thread_data_t *thread_data = get_thread_data(TRUE); + HTMLInnerWindow *window = xhr->window; + 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). + * We'll keep snapshots and synthesize them when unblocked for async XHR events. + * + * Note that 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 those we do not handle manually. 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... + * + * For details (and bunch of problems to consider) see: https://bugzil.la/697151 + */ + window->base.outer_window->readystate_locked++; + window->blocking_depth++; + thread_data->blocking_depth++; + nsres = nsIXMLHttpRequest_Send(xhr->nsxhr, nsbody); + thread_data->blocking_depth--; + window->base.outer_window->readystate_locked--; + + if(!--window->blocking_depth) + unblock_tasks_and_timers(thread_data); + + return nsres; +} + +struct pending_xhr_events_task { + event_task_t header; + HTMLXMLHttpRequest *xhr; +}; + +static void pending_xhr_events_proc(event_task_t *_task) +{ + struct pending_xhr_events_task *task = (struct pending_xhr_events_task*)_task; + HTMLXMLHttpRequest *xhr = task->xhr; + DWORD magic = xhr->pending_events_magic; + UINT16 readyState = xhr->readyState; + BOOLEAN send_load, send_loadend; + DOMEvent *event; + HRESULT hres; + + /* Don't bother if it was aborted or reopened */ + if(xhr->magic != magic) + return; + + /* Synthesize the necessary events that led us to this current state */ + nsIXMLHttpRequest_GetReadyState(xhr->nsxhr, &readyState); + if(readyState == READYSTATE_UNINITIALIZED) + return; + IHTMLXMLHttpRequest_AddRef(&xhr->IHTMLXMLHttpRequest_iface); + + send_loadend = send_load = (xhr->readyState != readyState && readyState == READYSTATE_COMPLETE); + for(;;) { + if(xhr->pending_progress_event && + xhr->readyState == (xhr->pending_progress_event->event_id == EVENTID_PROGRESS ? READYSTATE_INTERACTIVE : READYSTATE_COMPLETE)) + { + DOMEvent *pending_progress_event = xhr->pending_progress_event; + xhr->pending_progress_event = NULL; + + if(pending_progress_event->event_id == EVENTID_PROGRESS) { + const PRUnichar *text; + nsAString nsstr; + + /* If we are not done yet, further progress events will arrive, which can potentially be + blocked again and queued, so when that happens we need to return the current progress */ + if(readyState != READYSTATE_COMPLETE) { + nsAString_Init(&nsstr, NULL); + if(NS_SUCCEEDED(nsIXMLHttpRequest_GetResponseText(xhr->nsxhr, &nsstr))) { + /* Avoid recalculating from the beginning, since it can't be shorter */ + nsAString_GetData(&nsstr, &text); + if(text && text[0]) + xhr->pending_progress_event_text_len += wcslen(text + xhr->pending_progress_event_text_len); + nsAString_Finish(&nsstr); + } + } + }else { + send_load = FALSE; + send_loadend = TRUE; + } + + dispatch_event(&xhr->event_target, pending_progress_event); + IDOMEvent_Release(&pending_progress_event->IDOMEvent_iface); + if(xhr->magic != magic) + goto ret; + } + + if(xhr->readyState >= readyState) + break; + + xhr->readyState++; + hres = create_document_event(xhr->window->doc, EVENTID_READYSTATECHANGE, &event); + if(SUCCEEDED(hres)) { + dispatch_event(&xhr->event_target, event); + IDOMEvent_Release(&event->IDOMEvent_iface); + if(xhr->magic != magic) + goto ret; + } + } + + if(send_load) { + hres = create_document_event(xhr->window->doc, EVENTID_LOAD, &event); + if(SUCCEEDED(hres)) { + dispatch_event(&xhr->event_target, event); + IDOMEvent_Release(&event->IDOMEvent_iface); + if(xhr->magic != magic) + goto ret; + } + } + + if(send_loadend) { + hres = create_document_event(xhr->window->doc, EVENTID_LOADEND, &event); + if(SUCCEEDED(hres)) { + dispatch_event(&xhr->event_target, event); + IDOMEvent_Release(&event->IDOMEvent_iface); + if(xhr->magic != magic) + goto ret; + } + } + + /* We got here because the magic cookie didn't change, make sure further events are synthesized with a new task */ + xhr->pending_events_magic = magic - 1; + +ret: + IHTMLXMLHttpRequest_Release(&xhr->IHTMLXMLHttpRequest_iface); +} + +static void pending_xhr_events_destr(event_task_t *_task) +{ +} + static inline XMLHttpReqEventListener *impl_from_nsIDOMEventListener(nsIDOMEventListener *iface) { @@ -221,23 +377,102 @@ static nsrefcnt NSAPI XMLHttpReqEventListener_Release(nsIDOMEventListener *iface static nsresult NSAPI XMLHttpReqEventListener_HandleEvent(nsIDOMEventListener *iface, nsIDOMEvent *nsevent) { XMLHttpReqEventListener *This = impl_from_nsIDOMEventListener(iface); - UINT16 readyState; + unsigned blocking_depth = 0; + thread_data_t *thread_data; 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; + if(NS_SUCCEEDED(nsIXMLHttpRequest_GetReadyState(This->xhr->nsxhr, &val))) + readyState = val; + + if((thread_data = get_thread_data(FALSE))) + blocking_depth = thread_data->blocking_depth; 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); - IDOMEvent_Release(&event->IDOMEvent_iface); + if(FAILED(hres)) { + if(!blocking_depth || This->xhr->synchronous) + This->xhr->readyState = readyState; + return NS_ERROR_OUT_OF_MEMORY; + } + + if(blocking_depth) { + if(!This->xhr->synchronous) { + if(This->xhr->magic != This->xhr->pending_events_magic) { + struct pending_xhr_events_task *task; + + remove_target_tasks(This->xhr->task_magic); + + if(!(task = malloc(sizeof(*task)))) { + IDOMEvent_Release(&event->IDOMEvent_iface); + return NS_ERROR_OUT_OF_MEMORY; + } + + This->xhr->pending_events_magic = This->xhr->magic; + task->header.target_magic = This->xhr->task_magic; + task->header.thread_blocked = TRUE; + task->header.proc = pending_xhr_events_proc; + task->header.destr = pending_xhr_events_destr; + task->header.window = This->xhr->window; + task->xhr = This->xhr; + IHTMLWindow2_AddRef(&This->xhr->window->base.IHTMLWindow2_iface); + + list_add_after(thread_data->pending_xhr_events_tail, &task->header.entry); + thread_data->pending_xhr_events_tail = &task->header.entry; + } + + switch(event->event_id) { + case EVENTID_PROGRESS: + case EVENTID_ABORT: + case EVENTID_ERROR: + case EVENTID_TIMEOUT: + if(This->xhr->pending_progress_event) + IDOMEvent_Release(&This->xhr->pending_progress_event->IDOMEvent_iface); + This->xhr->pending_progress_event = event; + break; + default: + IDOMEvent_Release(&event->IDOMEvent_iface); + break; + } + + 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 < READYSTATE_COMPLETE && event->event_id == EVENTID_READYSTATECHANGE) { + IDOMEvent_Release(&event->IDOMEvent_iface); + This->xhr->readyState = readyState; + return NS_OK; + } } + + if(event->event_id == EVENTID_PROGRESS && !This->xhr->synchronous) { + const PRUnichar *text; + nsAString nsstr; + + /* Update the text length for an eventual future blocking on a pending progress event, + since we're about to dispatch a progress event, so code may expect it to be updated */ + 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->pending_progress_event_text_len += wcslen(text + This->xhr->pending_progress_event_text_len); + nsAString_Finish(&nsstr); + } + } + + This->xhr->readyState = readyState; + dispatch_event(&This->xhr->event_target, event); + IDOMEvent_Release(&event->IDOMEvent_iface); return NS_OK; } @@ -299,7 +534,11 @@ static ULONG WINAPI HTMLXMLHttpRequest_Release(IHTMLXMLHttpRequest *iface) TRACE("(%p) ref=%ld\n", This, ref); if(!ref) { + remove_target_tasks(This->task_magic); detach_xhr_event_listener(This->event_listener); + if(This->pending_progress_event) + IDOMEvent_Release(&This->pending_progress_event->IDOMEvent_iface); + IHTMLWindow2_Release(&This->window->base.IHTMLWindow2_iface); release_event_target(&This->event_target); release_dispex(&This->event_target.dispex); nsIXMLHttpRequest_Release(This->nsxhr); @@ -363,6 +602,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; @@ -378,7 +619,27 @@ 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(This->pending_progress_event) { + if(!This->pending_progress_event_text_len) + *p = NULL; + else if(!(*p = SysAllocStringLen(text, This->pending_progress_event_text_len))) + hres = E_OUTOFMEMORY; + }else { + /* Update this, as responseText could be retrieved *between* progress events when not blocked, + then next progress event gets blocked, and responseText is called again in another handler */ + This->pending_progress_event_text_len = wcslen(text); + if(!(*p = SysAllocStringLen(text, This->pending_progress_event_text_len))) + hres = E_OUTOFMEMORY; + } + nsAString_Finish(&nsstr); + } + return hres; } static HRESULT WINAPI HTMLXMLHttpRequest_get_responseXML(IHTMLXMLHttpRequest *iface, IDispatch **p) @@ -510,14 +771,17 @@ static HRESULT WINAPI HTMLXMLHttpRequest_get_onreadystatechange(IHTMLXMLHttpRequ static HRESULT WINAPI HTMLXMLHttpRequest_abort(IHTMLXMLHttpRequest *iface) { HTMLXMLHttpRequest *This = impl_from_IHTMLXMLHttpRequest(iface); + DWORD prev_magic = This->magic; UINT16 readyState; nsresult nsres; TRACE("(%p)->()\n", This); + This->magic++; nsres = nsIXMLHttpRequest_SlowAbort(This->nsxhr); if(NS_FAILED(nsres)) { ERR("nsIXMLHttpRequest_SlowAbort failed: %08lx\n", nsres); + This->magic = prev_magic; return E_FAIL; } @@ -553,9 +817,12 @@ static HRESULT HTMLXMLHttpRequest_open_hook(DispatchEx *dispex, WORD flags, static HRESULT WINAPI HTMLXMLHttpRequest_open(IHTMLXMLHttpRequest *iface, BSTR bstrMethod, BSTR bstrUrl, VARIANT varAsync, VARIANT varUser, VARIANT varPassword) { HTMLXMLHttpRequest *This = impl_from_IHTMLXMLHttpRequest(iface); + BOOLEAN prev_synchronous; nsAString user, password; nsACString method, url; unsigned opt_argc = 1; + size_t prev_text_len; + DWORD prev_magic; nsresult nsres; HRESULT hres; @@ -570,15 +837,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; @@ -602,6 +860,14 @@ 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) */ + prev_magic = This->magic; + prev_synchronous = This->synchronous; + prev_text_len = This->pending_progress_event_text_len; + This->pending_progress_event_text_len = 0; + This->synchronous = !V_BOOL(&varAsync); + This->magic++; + if(V_VT(&varPassword) != VT_EMPTY && V_VT(&varPassword) != VT_ERROR) opt_argc += 2; else if(V_VT(&varUser) != VT_EMPTY && V_VT(&varUser) != VT_ERROR) @@ -615,6 +881,9 @@ static HRESULT WINAPI HTMLXMLHttpRequest_open(IHTMLXMLHttpRequest *iface, BSTR b if(NS_FAILED(nsres)) { ERR("nsIXMLHttpRequest_Open failed: %08lx\n", nsres); + This->magic = prev_magic; + This->synchronous = prev_synchronous; + This->pending_progress_event_text_len = prev_text_len; return E_FAIL; } @@ -651,13 +920,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, (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; @@ -1445,6 +1719,9 @@ static HRESULT WINAPI HTMLXMLHttpRequestFactory_create(IHTMLXMLHttpRequestFactor } ret->nsxhr = nsxhr; + ret->window = This->window; + ret->task_magic = get_task_target_magic(); + 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
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 | 28 +++++++++++++++++++++++++++- 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/dlls/mshtml/tests/xhr.js b/dlls/mshtml/tests/xhr.js index ebe1a44168a..a9750fcfdd9 100644 --- a/dlls/mshtml/tests/xhr.js +++ b/dlls/mshtml/tests/xhr.js @@ -154,7 +154,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 aceef111998..0b715b9ddff 100644 --- a/dlls/mshtml/xmlhttprequest.c +++ b/dlls/mshtml/xmlhttprequest.c @@ -379,6 +379,7 @@ static nsresult NSAPI XMLHttpReqEventListener_HandleEvent(nsIDOMEventListener *i XMLHttpReqEventListener *This = impl_from_nsIDOMEventListener(iface); unsigned blocking_depth = 0; thread_data_t *thread_data; + compat_mode_t compat_mode; DOMEvent *event; LONG readyState; HRESULT hres; @@ -396,7 +397,8 @@ static nsresult NSAPI XMLHttpReqEventListener_HandleEvent(nsIDOMEventListener *i if((thread_data = get_thread_data(FALSE))) blocking_depth = thread_data->blocking_depth; - 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(FAILED(hres)) { if(!blocking_depth || This->xhr->synchronous) This->xhr->readyState = readyState; @@ -452,6 +454,30 @@ static nsresult NSAPI XMLHttpReqEventListener_HandleEvent(nsIDOMEventListener *i This->xhr->readyState = readyState; return NS_OK; } + + /* 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 < READYSTATE_COMPLETE && ( + event->event_id == EVENTID_READYSTATECHANGE || event->event_id == EVENTID_PROGRESS || event->event_id == EVENTID_LOADSTART)) { + DOMEvent *readystatechange_event; + DWORD magic = This->xhr->magic; + unsigned i; + + for(i = READYSTATE_LOADING; i < READYSTATE_COMPLETE; 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); + + if(This->xhr->magic != magic) { + IDOMEvent_Release(&event->IDOMEvent_iface); + return NS_OK; + } + } + } } if(event->event_id == EVENTID_PROGRESS && !This->xhr->synchronous) { -- 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=129733 Your paranoid android. === w11pro64_amd (64 bit report) === Report validation errors: mshtml:dom crashed (c0000374)
Here's the new version. Unfortunately it's more complicated than I would have liked, but it's necessary to handle all the cases (at least from what I could think of). First issue handled now is that aborted XHRs can be also restarted (according to Gecko code, which I followed closely). To handle this properly, I store a magic cookie (actually two) in the XHR object. Since both abort() and open() are sync operations, the cookie is incremented on them (and reverted on failure), and then checked for delayed events. We store two magic cookies, because one of them is the one matching the XHR object's magic cookie when the pending events task was added. If they mismatch, the task gets removed, and a new one with a new pending events cookie is created. So all the older task events are discarded, as they should be. Note that we have to keep the pending events cookie on the XHR object, rather than the task, because we need to know if we should create a new task or if it's queued already and we just handle events there. The other main problem is that `responseText` still had to be handled, although for different reasons (it was still broken before). We still need to track it, albeit only on progress events dispatched this time, or when `responseText` itself is called. Consider the following scenario that encompasses all 3 possible corner cases, assuming the async_xhr's full response is "1234567890": * async_xhr.send() is called, but has a slow response from server. * sync_xhr.send() is called, so it starts blocking. * async_xhr receives progress from server, with progress event, now it has "1234" partial response. * sync_xhr calls one of its handlers, which then does `async_xhr.responseText`. At this point, this must be NULL/empty, because we've delayed its progress event in the first place. * sync_xhr.send() finishes blocking. * jscript code calls `async_xhr.responseText`, which must return NULL again, as we haven't entered the msg loop. * code returns and we process msg loop. We dispatch the progress event, its handler checks `async_xhr.responseText`, which must now be "1234". Then it blocks using another sync_xhr.send(). * async_xhr receives progress from server, with progress event, now it has "123456" partial response. * sync_xhr calls one of its handlers, `async_xhr.responseText` must return "1234", not "123456", i.e. we have to keep track of its state at least before dispatching any progress events. * sync_xhr.send() finishes blocking, and we go into msg loop, we dispatch progress event, update async_xhr's state to "123456". * later **during some other event handler**, it checks `async_xhr.responseText`, which now returns "1234567" (we're unblocked, so this is fine), with **no progress event sent yet**. * sync_xhr.send() is called, so it starts blocking again. * async_xhr gets more progress now, finishes loading, but we must delay it because we're blocked. * in a sync_xhr handler, it checks `async_xhr.responseText`, which must be "1234567" because we've had that returned before, even with no progress event dispatched yet. So, we must keep track of its state when calling responseText without being blocked, as well. Both "123456" (old state at progress event) and "1234567890" (current, fully completed state) would be wrong here! Also another thing to keep in mind is that normal progress events are sent before XHR switches to DONE/COMPLETE readystate, while the others (abort, error, timeout) are sent *after* it switches to DONE. This had to be handled when synthesizing them. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2098#note_24823
On Tue Feb 21 15:08:17 2023 +0000, Gabriel Ivăncescu wrote:
Here's the new version. Unfortunately it's more complicated than I would have liked, but it's necessary to handle all the cases (at least from what I could think of). First issue handled now is that aborted XHRs can be also restarted (according to Gecko code, which I followed closely). To handle this properly, I store a magic cookie (actually two) in the XHR object. Since both abort() and open() are sync operations, the cookie is incremented on them (and reverted on failure), and then checked for delayed events. We store two magic cookies, because one of them is the one matching the XHR object's magic cookie when the pending events task was added. If they mismatch, the task gets removed, and a new one with a new pending events cookie is created. So all the older task events are discarded, as they should be. Note that we have to keep the pending events cookie on the XHR object, rather than the task, because we need to know if we should create a new task or if it's queued already and we just handle events there. The other main problem is that `responseText` still had to be handled, although for different reasons (it was still broken before). We still need to track it, albeit only on progress events dispatched this time, or when `responseText` itself is called. Consider the following scenario that encompasses all 3 possible corner cases, assuming the async_xhr's full response is "1234567890": * async_xhr.send() is called, but has a slow response from server. * sync_xhr.send() is called, so it starts blocking. * async_xhr receives progress from server, with progress event, now it has "1234" partial response. * sync_xhr calls one of its handlers, which then does `async_xhr.responseText`. At this point, this must be NULL/empty, because we've delayed its progress event in the first place. * sync_xhr.send() finishes blocking. * jscript code calls `async_xhr.responseText`, which must return NULL again, as we haven't entered the msg loop. * code returns and we process msg loop. We dispatch the progress event, its handler checks `async_xhr.responseText`, which must now be "1234". Then it blocks using another sync_xhr.send(). * async_xhr receives progress from server, with progress event, now it has "123456" partial response. * sync_xhr calls one of its handlers, `async_xhr.responseText` must return "1234", not "123456", i.e. we have to keep track of its state at least before dispatching any progress events. * sync_xhr.send() finishes blocking, and we go into msg loop, we dispatch progress event, update async_xhr's state to "123456". * later **during some other event handler**, it checks `async_xhr.responseText`, which now returns "1234567" (we're unblocked, so this is fine), with **no progress event sent yet**. * sync_xhr.send() is called, so it starts blocking again. * async_xhr gets more progress now, finishes loading, but we must delay it because we're blocked. * in a sync_xhr handler, it checks `async_xhr.responseText`, which must be "1234567" because we've had that returned before, even with no progress event dispatched yet. So, we must keep track of its state when calling responseText without being blocked, as well. Both "123456" (old state at progress event) and "1234567890" (current, fully completed state) would be wrong here! Also another thing to keep in mind is that normal progress events are sent before XHR switches to DONE/COMPLETE readystate, while the others (abort, error, timeout) are sent *after* it switches to DONE. This had to be handled when synthesizing them. It's not clear to me that we need to worry about `responseText`'s length in such cases or at least I don't think that spec gives such guarantees (https://xhr.spec.whatwg.org/#the-send()-method, 'If not roughly 50ms have passed since these steps were last invoked, then return.' for example). Sync XHRs aside, the assumption that `responseText`'s length matches progress event is only valid inside the handler, not between events. An impact of sync XHR inside event handler is less clear, but it still seems fine to me to continue aggregating async XHR data in that case.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2098#note_25025
Jacek Caban (@jacek) commented about dlls/mshtml/xmlhttprequest.c:
IWineXMLHttpRequestPrivate IWineXMLHttpRequestPrivate_iface; IProvideClassInfo2 IProvideClassInfo2_iface; LONG ref; + LONG readyState;
Please use something like `ready_state`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2098#note_25026
Jacek Caban (@jacek) commented about dlls/mshtml/tests/events.c:
HRESULT hres; BSTR bstr;
- hres = IInternetProtocolSink_QueryInterface(This->sink, &IID_IServiceProvider, (void**)&service_provider); - ok(hres == S_OK, "Could not get IServiceProvider iface: %08lx\n", hres); + if(This->delay >= 0) {
Checking delay here is a bit weird, maybe a dedicated variable to store an info if any response on this handler was already reported would be cleaner. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2098#note_25027
On Tue Feb 21 15:08:16 2023 +0000, Jacek Caban wrote:
It's not clear to me that we need to worry about `responseText`'s length in such cases or at least I don't think that spec gives such guarantees (https://xhr.spec.whatwg.org/#the-send()-method, 'If not roughly 50ms have passed since these steps were last invoked, then return.' for example). Sync XHRs aside, the assumption that `responseText`'s length matches progress event is only valid inside the handler, not between events. An impact of sync XHR inside event handler is less clear, but it still seems fine to me to continue aggregating async XHR data in that case. The problem isn't matching it necessarily exactly, but rather that it must be *at least* as long as the last one reported, never *less*.
In the example I gave, progress event gets called, some progress is reported in `responseText`, e.g. "123456". Subsequent calls must always return *at least* "123456", never less (not "12345" or anything less). Now imagine that the script stores the value of responseText into a global variable. Nothing out of the ordinary here. Now it's "123456". It expects that it's always increasing, and never less, so it can't become shorter like "12345". This is perfectly fine. Next, we're out of the blocking call, maybe out of the handler entirely. It's not blocked now, and `responseText` gets called again. We didn't receive or dispatch any progress events yet, but this time, Gecko reports "1234567" (which is perfectly valid). The script now stores "1234567" in the global variable, or something to that effect. So far so good. But what happens if we now block inside a sync_xhr send() and something calls async_xhr.responseText? What will we return? We can't return Gecko's value, because we're blocked, and it could be even further, perhaps completed even. We can't return the former "state saved at progress event" either, because that was "123456", which is shorter than "1234567" and breaks the assumptions that it can never be less! So we have to keep track of it even when we call responseText when non-blocking. In this case, update it to "1234567", so now we return "1234567", which doesn't break the "it can't be shorter" constraint. Anyway this isn't a big deal? It's just the update in get_responseText. SysAllocString() would calculate the length anyway, so it's not a performance issue either for long responses. I don't think it should be an issue? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2098#note_25030
We can't return Gecko's value, because we're blocked, and it could be even further, perhaps completed even.
Why not? That holds the "at least" assumption. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2098#note_25034
On Tue Feb 21 16:05:21 2023 +0000, Jacek Caban wrote:
We can't return Gecko's value, because we're blocked, and it could be even further, perhaps completed even. Why not? That holds the "at least" assumption. Because we still have pending (blocked) progress events in that situation, due to sync xhr. It doesn't make sense to return more progress than the event itself has *before* the event is dispatched, so it has to wait.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2098#note_25051
It doesn't make sense to return more progress than the event itself has *before* the event is dispatched, so it has to wait.
Why it doesn't make sense? What's wrong with it? I'm trying to understand why do you insist on complicating the implementation and I don't see where this assumption comes from. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2098#note_25132
On Wed Feb 22 11:32:26 2023 +0000, Jacek Caban wrote:
It doesn't make sense to return more progress than the event itself has *before* the event is dispatched, so it has to wait. Why it doesn't make sense? What's wrong with it? I'm trying to understand why do you insist on complicating the implementation and I don't see where this assumption comes from. From what I can see, when a progress event is dispatched, the progress is always updated compared to what was before. It's not just Gecko's implementation, polyfills also do this, e.g: https://github.com/wisniewskit/FirefoxSynchronousXHRPolyfix/blob/master/Fire...
But in such scenario above, if I remove the responseText handling, Gecko could potentially return the entire text string, without readyState event being sent yet, nor progress event. Later we dispatch those events, and we have the "same" progress, which feels wrong. I don't know, just feels wrong to me; I could potentially split it into a separate patch. Not sure if that's any better? Or do you still want me to remove it completely? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2098#note_25147
From what I can see, when a progress event is dispatched, the progress is always updated compared to what was before. It's not just Gecko's implementation
Gecko also doesn't "fully" support synchronous XHR, so I wouldn't read too much into that. As I quoted above, spec doesn't give such guarantees.
But in such scenario above, if I remove the responseText handling, Gecko could potentially return the entire text string, without readyState event being sent yet, nor progress event.
I think that readystate event is guaranteed with ready state check in get_responseText. At the moment of dispatching, progress event would still be precise. It's just between events (including extra sync XHRs) when it could grow.
I don't know, just feels wrong to me; I could potentially split it into a separate patch. Not sure if that's any better? Or do you still want me to remove it completely?
Keeping this MR (especially the large commit) as simple as possible is a good idea. I think that keeping this part (including its design implications) out of this MR would be good. If you still think it's justified, we can revisit it separately. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2098#note_25194
participants (3)
-
Gabriel Ivăncescu -
Jacek Caban (＠jacek)
-
Marvin