[PATCH v2 0/8] MR4178: mshtml: Keep refs from associated objects to the window.
-- v2: mshtml: Keep ref from the OmHistory to the inner window. mshtml: Keep ref from LocalStorage and SessionStorage to the inner window. mshtml: Keep ref from the XMLHttpRequestFactory to the inner window. mshtml: Keep ref from the OptionElementFactory to the inner window. mshtml: Keep ref from the ImageElementFactory to the inner window. mshtml/tests: Test parent window links after navigation. mshtml: Properly traverse document nodes. mshtml: Keep ref from the document node to the inner window. https://gitlab.winehq.org/wine/wine/-/merge_requests/4178
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmldoc.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/dlls/mshtml/htmldoc.c b/dlls/mshtml/htmldoc.c index 9750ad19765..f1da5eaad07 100644 --- a/dlls/mshtml/htmldoc.c +++ b/dlls/mshtml/htmldoc.c @@ -5700,6 +5700,12 @@ void detach_document_node(HTMLDocumentNode *doc) { unsigned i; + if(doc->window) { + HTMLInnerWindow *window = doc->window; + doc->window = NULL; + IHTMLWindow2_Release(&window->base.IHTMLWindow2_iface); + } + while(!list_empty(&doc->plugin_hosts)) detach_plugin_host(LIST_ENTRY(list_head(&doc->plugin_hosts), PluginHost, entry)); @@ -5855,6 +5861,15 @@ static void *HTMLDocumentNode_query_interface(DispatchEx *dispex, REFIID riid) return HTMLDOMNode_query_interface(&This->node.event_target.dispex, riid); } +static void HTMLDocumentNode_traverse(DispatchEx *dispex, nsCycleCollectionTraversalCallback *cb) +{ + HTMLDocumentNode *This = impl_from_DispatchEx(dispex); + HTMLDOMNode_traverse(dispex, cb); + + if(This->window) + note_cc_edge((nsISupports*)&This->window->base.IHTMLWindow2_iface, "window", cb); +} + static void HTMLDocumentNode_unlink(DispatchEx *dispex) { HTMLDocumentNode *This = impl_from_DispatchEx(dispex); @@ -5865,13 +5880,8 @@ static void HTMLDocumentNode_unlink(DispatchEx *dispex) detach_document_node(This); This->dom_document = NULL; This->html_document = NULL; - This->window = NULL; }else if(This->window) { detach_document_node(This); - - /* document fragments own reference to inner window */ - IHTMLWindow2_Release(&This->window->base.IHTMLWindow2_iface); - This->window = NULL; } } @@ -6057,7 +6067,7 @@ static const event_target_vtbl_t HTMLDocumentNode_event_target_vtbl = { { .query_interface = HTMLDocumentNode_query_interface, .destructor = HTMLDocumentNode_destructor, - .traverse = HTMLDOMNode_traverse, + .traverse = HTMLDocumentNode_traverse, .unlink = HTMLDocumentNode_unlink, .get_name = HTMLDocumentNode_get_name, .invoke = HTMLDocumentNode_invoke, @@ -6166,6 +6176,9 @@ static HTMLDocumentNode *alloc_doc_node(HTMLDocumentObj *doc_obj, HTMLInnerWindo doc->outer_window = window ? window->base.outer_window : NULL; doc->window = window; + if(window) + IHTMLWindow2_AddRef(&window->base.IHTMLWindow2_iface); + ConnectionPointContainer_Init(&doc->cp_container, (IUnknown*)&doc->IHTMLDocument2_iface, HTMLDocumentNode_cpc); HTMLDocumentNode_Persist_Init(doc); HTMLDocumentNode_Service_Init(doc); @@ -6241,8 +6254,6 @@ static HRESULT create_document_fragment(nsIDOMNode *nsnode, HTMLDocumentNode *do if(!doc_frag) return E_OUTOFMEMORY; - IHTMLWindow2_AddRef(&doc_frag->window->base.IHTMLWindow2_iface); - HTMLDOMNode_Init(doc_node, &doc_frag->node, nsnode, &HTMLDocumentNode_dispex); doc_frag->node.vtbl = &HTMLDocumentFragmentImplVtbl; doc_frag->document_mode = lock_document_mode(doc_node); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4178
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmldoc.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dlls/mshtml/htmldoc.c b/dlls/mshtml/htmldoc.c index f1da5eaad07..4daf5ccc58d 100644 --- a/dlls/mshtml/htmldoc.c +++ b/dlls/mshtml/htmldoc.c @@ -5868,6 +5868,10 @@ static void HTMLDocumentNode_traverse(DispatchEx *dispex, nsCycleCollectionTrave if(This->window) note_cc_edge((nsISupports*)&This->window->base.IHTMLWindow2_iface, "window", cb); + if(This->dom_implementation) + note_cc_edge((nsISupports*)This->dom_implementation, "dom_implementation", cb); + if(This->namespaces) + note_cc_edge((nsISupports*)This->namespaces, "namespaces", cb); } static void HTMLDocumentNode_unlink(DispatchEx *dispex) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4178
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/tests/events.c | 43 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/dlls/mshtml/tests/events.c b/dlls/mshtml/tests/events.c index 7d139ccafe1..a55ebd8d26e 100644 --- a/dlls/mshtml/tests/events.c +++ b/dlls/mshtml/tests/events.c @@ -3234,6 +3234,48 @@ static void test_iframe_connections(IHTMLDocument2 *doc) IHTMLDocument2_Release(iframes_doc); } +static void test_window_refs(IHTMLDocument2 *doc) +{ + IHTMLWindow2 *self, *parent, *child; + IHTMLFrameBase2 *iframe; + IHTMLDocument6 *doc6; + IHTMLElement2 *elem; + HRESULT hres; + BSTR bstr; + + hres = IHTMLDocument2_QueryInterface(doc, &IID_IHTMLDocument6, (void**)&doc6); + ok(hres == S_OK, "Could not get IHTMLDocument6 iface: %08lx\n", hres); + bstr = SysAllocString(L"ifr"); + hres = IHTMLDocument6_getElementById(doc6, bstr, &elem); + ok(hres == S_OK, "getElementById failed: %08lx\n", hres); + IHTMLDocument6_Release(doc6); + SysFreeString(bstr); + + hres = IHTMLElement2_QueryInterface(elem, &IID_IHTMLFrameBase2, (void**)&iframe); + ok(hres == S_OK, "Could not get IHTMLFrameBase2 iface: %08lx\n", hres); + IHTMLElement2_Release(elem); + hres = IHTMLFrameBase2_get_contentWindow(iframe, &child); + ok(hres == S_OK, "get_contentWindow failed: %08lx\n", hres); + IHTMLFrameBase2_Release(iframe); + + hres = IHTMLWindow2_get_self(window, &self); + ok(hres == S_OK, "get_self failed: %08lx\n", hres); + hres = IHTMLWindow2_get_parent(child, &parent); + ok(hres == S_OK, "get_parent failed: %08lx\n", hres); + ok(parent == self, "parent != self\n"); + IHTMLWindow2_Release(parent); + IHTMLWindow2_Release(self); + + navigate(doc, L"blank.html"); + + hres = IHTMLWindow2_get_parent(child, &parent); + ok(hres == S_OK, "get_parent failed: %08lx\n", hres); + ok(parent == child, "parent != child\n"); + + IHTMLWindow2_Release(parent); + IHTMLWindow2_Release(child); +} + static void test_doc_obj(IHTMLDocument2 *doc) { static DISPID propput_dispid = DISPID_PROPERTYPUT; @@ -6523,6 +6565,7 @@ START_TEST(events) run_test_from_res(L"doc_with_prop_ie9.html", test_visibilitychange); run_test_from_res(L"blank_ie10.html", test_visibilitychange); run_test_from_res(L"iframe.html", test_unload_event); + run_test_from_res(L"iframe.html", test_window_refs); run_test(empty_doc_ie9_str, test_create_event); run_test(img_doc_ie9_str, test_imgload); run_test(input_image_doc_ie9_str, test_inputload); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4178
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlimg.c | 31 +++++++++++++++++++++++-------- dlls/mshtml/htmlwindow.c | 1 - dlls/mshtml/tests/events.c | 15 ++++++++++++++- 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/dlls/mshtml/htmlimg.c b/dlls/mshtml/htmlimg.c index cf69b62e744..07ce5643ad1 100644 --- a/dlls/mshtml/htmlimg.c +++ b/dlls/mshtml/htmlimg.c @@ -846,7 +846,7 @@ static HRESULT WINAPI HTMLImageElementFactory_create(IHTMLImageElementFactory *i VARIANT width, VARIANT height, IHTMLImgElement **img_elem) { HTMLImageElementFactory *This = impl_from_IHTMLImageElementFactory(iface); - HTMLDocumentNode *doc; + HTMLDocumentNode *doc = This->window->doc; IHTMLImgElement *img; HTMLElement *elem; nsIDOMElement *nselem; @@ -856,13 +856,6 @@ static HRESULT WINAPI HTMLImageElementFactory_create(IHTMLImageElementFactory *i TRACE("(%p)->(%s %s %p)\n", This, debugstr_variant(&width), debugstr_variant(&height), img_elem); - if(!This->window || !This->window->doc) { - WARN("NULL doc\n"); - return E_UNEXPECTED; - } - - doc = This->window->doc; - *img_elem = NULL; hres = create_nselem(doc, L"IMG", &nselem); @@ -921,6 +914,25 @@ static void *HTMLImageElementFactory_query_interface(DispatchEx *dispex, REFIID return NULL; } +static void HTMLImageElementFactory_traverse(DispatchEx *dispex, nsCycleCollectionTraversalCallback *cb) +{ + HTMLImageElementFactory *This = impl_from_DispatchEx(dispex); + + if(This->window) + note_cc_edge((nsISupports*)&This->window->base.IHTMLWindow2_iface, "window", cb); +} + +static void HTMLImageElementFactory_unlink(DispatchEx *dispex) +{ + HTMLImageElementFactory *This = impl_from_DispatchEx(dispex); + + if(This->window) { + HTMLInnerWindow *window = This->window; + This->window = NULL; + IHTMLWindow2_Release(&window->base.IHTMLWindow2_iface); + } +} + static void HTMLImageElementFactory_destructor(DispatchEx *dispex) { HTMLImageElementFactory *This = impl_from_DispatchEx(dispex); @@ -963,6 +975,8 @@ static const tid_t HTMLImageElementFactory_iface_tids[] = { static const dispex_static_data_vtbl_t HTMLImageElementFactory_dispex_vtbl = { .query_interface = HTMLImageElementFactory_query_interface, .destructor = HTMLImageElementFactory_destructor, + .traverse = HTMLImageElementFactory_traverse, + .unlink = HTMLImageElementFactory_unlink, .value = HTMLImageElementFactory_value, }; @@ -983,6 +997,7 @@ HRESULT HTMLImageElementFactory_Create(HTMLInnerWindow *window, HTMLImageElement ret->IHTMLImageElementFactory_iface.lpVtbl = &HTMLImageElementFactoryVtbl; ret->window = window; + IHTMLWindow2_AddRef(&window->base.IHTMLWindow2_iface); init_dispatch(&ret->dispex, &HTMLImageElementFactory_dispex, dispex_compat_mode(&window->event_target.dispex)); diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index 47e65472c01..3e08a90d5fc 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -3977,7 +3977,6 @@ static void HTMLWindow_unlink(DispatchEx *dispex) if(This->image_factory) { HTMLImageElementFactory *image_factory = This->image_factory; - This->image_factory->window = NULL; This->image_factory = NULL; IHTMLImageElementFactory_Release(&image_factory->IHTMLImageElementFactory_iface); } diff --git a/dlls/mshtml/tests/events.c b/dlls/mshtml/tests/events.c index a55ebd8d26e..515a2cf443c 100644 --- a/dlls/mshtml/tests/events.c +++ b/dlls/mshtml/tests/events.c @@ -3236,13 +3236,18 @@ static void test_iframe_connections(IHTMLDocument2 *doc) static void test_window_refs(IHTMLDocument2 *doc) { + IHTMLImageElementFactory *image_factory; IHTMLWindow2 *self, *parent, *child; + IHTMLImgElement *img_elem; IHTMLFrameBase2 *iframe; IHTMLDocument6 *doc6; IHTMLElement2 *elem; + VARIANT vempty; HRESULT hres; BSTR bstr; + V_VT(&vempty) = VT_EMPTY; + hres = IHTMLDocument2_QueryInterface(doc, &IID_IHTMLDocument6, (void**)&doc6); ok(hres == S_OK, "Could not get IHTMLDocument6 iface: %08lx\n", hres); bstr = SysAllocString(L"ifr"); @@ -3258,6 +3263,9 @@ static void test_window_refs(IHTMLDocument2 *doc) ok(hres == S_OK, "get_contentWindow failed: %08lx\n", hres); IHTMLFrameBase2_Release(iframe); + hres = IHTMLWindow2_get_Image(window, &image_factory); + ok(hres == S_OK, "get_Image failed: %08lx\n", hres); + hres = IHTMLWindow2_get_self(window, &self); ok(hres == S_OK, "get_self failed: %08lx\n", hres); hres = IHTMLWindow2_get_parent(child, &parent); @@ -3271,9 +3279,14 @@ static void test_window_refs(IHTMLDocument2 *doc) hres = IHTMLWindow2_get_parent(child, &parent); ok(hres == S_OK, "get_parent failed: %08lx\n", hres); ok(parent == child, "parent != child\n"); - IHTMLWindow2_Release(parent); IHTMLWindow2_Release(child); + + hres = IHTMLImageElementFactory_create(image_factory, vempty, vempty, &img_elem); + ok(hres == S_OK, "create failed: %08lx\n", hres); + ok(img_elem != NULL, "img_elem == NULL\n"); + IHTMLImageElementFactory_Release(image_factory); + IHTMLImgElement_Release(img_elem); } static void test_doc_obj(IHTMLDocument2 *doc) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4178
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlselect.c | 27 ++++++++++++++++++++++----- dlls/mshtml/htmlwindow.c | 1 - dlls/mshtml/tests/events.c | 10 ++++++++++ 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/dlls/mshtml/htmlselect.c b/dlls/mshtml/htmlselect.c index 2daed7e8b2c..1b867195595 100644 --- a/dlls/mshtml/htmlselect.c +++ b/dlls/mshtml/htmlselect.c @@ -491,11 +491,6 @@ static HRESULT WINAPI HTMLOptionElementFactory_create(IHTMLOptionElementFactory TRACE("(%p)->(%s %s %s %s %p)\n", This, debugstr_variant(&text), debugstr_variant(&value), debugstr_variant(&defaultselected), debugstr_variant(&selected), optelem); - if(!This->window || !This->window->doc) { - WARN("NULL doc\n"); - return E_UNEXPECTED; - } - *optelem = NULL; hres = create_nselem(This->window->doc, L"OPTION", &nselem); @@ -555,6 +550,25 @@ static void *HTMLOptionElementFactory_query_interface(DispatchEx *dispex, REFIID return NULL; } +static void HTMLOptionElementFactory_traverse(DispatchEx *dispex, nsCycleCollectionTraversalCallback *cb) +{ + HTMLOptionElementFactory *This = HTMLOptionElementFactory_from_DispatchEx(dispex); + + if(This->window) + note_cc_edge((nsISupports*)&This->window->base.IHTMLWindow2_iface, "window", cb); +} + +static void HTMLOptionElementFactory_unlink(DispatchEx *dispex) +{ + HTMLOptionElementFactory *This = HTMLOptionElementFactory_from_DispatchEx(dispex); + + if(This->window) { + HTMLInnerWindow *window = This->window; + This->window = NULL; + IHTMLWindow2_Release(&window->base.IHTMLWindow2_iface); + } +} + static void HTMLOptionElementFactory_destructor(DispatchEx *dispex) { HTMLOptionElementFactory *This = HTMLOptionElementFactory_from_DispatchEx(dispex); @@ -601,6 +615,8 @@ static const tid_t HTMLOptionElementFactory_iface_tids[] = { static const dispex_static_data_vtbl_t HTMLOptionElementFactory_dispex_vtbl = { .query_interface = HTMLOptionElementFactory_query_interface, .destructor = HTMLOptionElementFactory_destructor, + .traverse = HTMLOptionElementFactory_traverse, + .unlink = HTMLOptionElementFactory_unlink, .value = HTMLOptionElementFactory_value, }; @@ -621,6 +637,7 @@ HRESULT HTMLOptionElementFactory_Create(HTMLInnerWindow *window, HTMLOptionEleme ret->IHTMLOptionElementFactory_iface.lpVtbl = &HTMLOptionElementFactoryVtbl; ret->window = window; + IHTMLWindow2_AddRef(&window->base.IHTMLWindow2_iface); init_dispatch(&ret->dispex, &HTMLOptionElementFactory_dispex, dispex_compat_mode(&window->event_target.dispex)); diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index 3e08a90d5fc..85794b7fe85 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -3982,7 +3982,6 @@ static void HTMLWindow_unlink(DispatchEx *dispex) } if(This->option_factory) { HTMLOptionElementFactory *option_factory = This->option_factory; - This->option_factory->window = NULL; This->option_factory = NULL; IHTMLOptionElementFactory_Release(&option_factory->IHTMLOptionElementFactory_iface); } diff --git a/dlls/mshtml/tests/events.c b/dlls/mshtml/tests/events.c index 515a2cf443c..0ccb892603a 100644 --- a/dlls/mshtml/tests/events.c +++ b/dlls/mshtml/tests/events.c @@ -3236,8 +3236,10 @@ static void test_iframe_connections(IHTMLDocument2 *doc) static void test_window_refs(IHTMLDocument2 *doc) { + IHTMLOptionElementFactory *option_factory; IHTMLImageElementFactory *image_factory; IHTMLWindow2 *self, *parent, *child; + IHTMLOptionElement *option_elem; IHTMLImgElement *img_elem; IHTMLFrameBase2 *iframe; IHTMLDocument6 *doc6; @@ -3265,6 +3267,8 @@ static void test_window_refs(IHTMLDocument2 *doc) hres = IHTMLWindow2_get_Image(window, &image_factory); ok(hres == S_OK, "get_Image failed: %08lx\n", hres); + hres = IHTMLWindow2_get_Option(window, &option_factory); + ok(hres == S_OK, "get_Option failed: %08lx\n", hres); hres = IHTMLWindow2_get_self(window, &self); ok(hres == S_OK, "get_self failed: %08lx\n", hres); @@ -3287,6 +3291,12 @@ static void test_window_refs(IHTMLDocument2 *doc) ok(img_elem != NULL, "img_elem == NULL\n"); IHTMLImageElementFactory_Release(image_factory); IHTMLImgElement_Release(img_elem); + + hres = IHTMLOptionElementFactory_create(option_factory, vempty, vempty, vempty, vempty, &option_elem); + ok(hres == S_OK, "create failed: %08lx\n", hres); + ok(option_elem != NULL, "option_elem == NULL\n"); + IHTMLOptionElementFactory_Release(option_factory); + IHTMLOptionElement_Release(option_elem); } static void test_doc_obj(IHTMLDocument2 *doc) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4178
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlwindow.c | 1 - dlls/mshtml/tests/events.c | 21 ++++++++++++++++++++- dlls/mshtml/xmlhttprequest.c | 25 +++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index 85794b7fe85..36aede7244e 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -3987,7 +3987,6 @@ static void HTMLWindow_unlink(DispatchEx *dispex) } if(This->xhr_factory) { HTMLXMLHttpRequestFactory *xhr_factory = This->xhr_factory; - This->xhr_factory->window = NULL; This->xhr_factory = NULL; IHTMLXMLHttpRequestFactory_Release(&xhr_factory->IHTMLXMLHttpRequestFactory_iface); } diff --git a/dlls/mshtml/tests/events.c b/dlls/mshtml/tests/events.c index 0ccb892603a..33654fe46b5 100644 --- a/dlls/mshtml/tests/events.c +++ b/dlls/mshtml/tests/events.c @@ -3237,14 +3237,17 @@ static void test_iframe_connections(IHTMLDocument2 *doc) static void test_window_refs(IHTMLDocument2 *doc) { IHTMLOptionElementFactory *option_factory; + IHTMLXMLHttpRequestFactory *xhr_factory; IHTMLImageElementFactory *image_factory; IHTMLWindow2 *self, *parent, *child; IHTMLOptionElement *option_elem; IHTMLImgElement *img_elem; + IHTMLXMLHttpRequest *xhr; IHTMLFrameBase2 *iframe; + IHTMLWindow5 *window5; IHTMLDocument6 *doc6; IHTMLElement2 *elem; - VARIANT vempty; + VARIANT vempty, var; HRESULT hres; BSTR bstr; @@ -3265,6 +3268,16 @@ static void test_window_refs(IHTMLDocument2 *doc) ok(hres == S_OK, "get_contentWindow failed: %08lx\n", hres); IHTMLFrameBase2_Release(iframe); + hres = IHTMLWindow2_QueryInterface(window, &IID_IHTMLWindow5, (void**)&window5); + ok(hres == S_OK, "Could not get IHTMLWindow5: %08lx\n", hres); + hres = IHTMLWindow5_get_XMLHttpRequest(window5, &var); + ok(hres == S_OK, "get_XMLHttpRequest failed: %08lx\n", hres); + ok(V_VT(&var) == VT_DISPATCH, "V_VT(XMLHttpRequest) = %d\n", V_VT(&var)); + hres = IDispatch_QueryInterface(V_DISPATCH(&var), &IID_IHTMLXMLHttpRequestFactory, (void**)&xhr_factory); + ok(hres == S_OK, "Could not get IHTMLXMLHttpRequestFactory: %08lx\n", hres); + IHTMLWindow5_Release(window5); + VariantClear(&var); + hres = IHTMLWindow2_get_Image(window, &image_factory); ok(hres == S_OK, "get_Image failed: %08lx\n", hres); hres = IHTMLWindow2_get_Option(window, &option_factory); @@ -3286,6 +3299,12 @@ static void test_window_refs(IHTMLDocument2 *doc) IHTMLWindow2_Release(parent); IHTMLWindow2_Release(child); + hres = IHTMLXMLHttpRequestFactory_create(xhr_factory, &xhr); + todo_wine + ok(hres == S_OK, "create failed: %08lx\n", hres); + IHTMLXMLHttpRequestFactory_Release(xhr_factory); + if(hres == S_OK) IHTMLXMLHttpRequest_Release(xhr); + hres = IHTMLImageElementFactory_create(image_factory, vempty, vempty, &img_elem); ok(hres == S_OK, "create failed: %08lx\n", hres); ok(img_elem != NULL, "img_elem == NULL\n"); diff --git a/dlls/mshtml/xmlhttprequest.c b/dlls/mshtml/xmlhttprequest.c index 44f83dd8809..20049a13ee7 100644 --- a/dlls/mshtml/xmlhttprequest.c +++ b/dlls/mshtml/xmlhttprequest.c @@ -1686,6 +1686,9 @@ static HRESULT WINAPI HTMLXMLHttpRequestFactory_create(IHTMLXMLHttpRequestFactor TRACE("(%p)->(%p)\n", This, p); + if(!This->window->base.outer_window) + return E_FAIL; + nsxhr = create_nsxhr(This->window->base.outer_window->nswindow); if(!nsxhr) return E_FAIL; @@ -1768,6 +1771,25 @@ static void *HTMLXMLHttpRequestFactory_query_interface(DispatchEx *dispex, REFII return NULL; } +static void HTMLXMLHttpRequestFactory_traverse(DispatchEx *dispex, nsCycleCollectionTraversalCallback *cb) +{ + HTMLXMLHttpRequestFactory *This = factory_from_DispatchEx(dispex); + + if(This->window) + note_cc_edge((nsISupports*)&This->window->base.IHTMLWindow2_iface, "window", cb); +} + +static void HTMLXMLHttpRequestFactory_unlink(DispatchEx *dispex) +{ + HTMLXMLHttpRequestFactory *This = factory_from_DispatchEx(dispex); + + if(This->window) { + HTMLInnerWindow *window = This->window; + This->window = NULL; + IHTMLWindow2_Release(&window->base.IHTMLWindow2_iface); + } +} + static void HTMLXMLHttpRequestFactory_destructor(DispatchEx *dispex) { HTMLXMLHttpRequestFactory *This = factory_from_DispatchEx(dispex); @@ -1800,6 +1822,8 @@ static HRESULT HTMLXMLHttpRequestFactory_value(DispatchEx *iface, LCID lcid, WOR static const dispex_static_data_vtbl_t HTMLXMLHttpRequestFactory_dispex_vtbl = { .query_interface = HTMLXMLHttpRequestFactory_query_interface, .destructor = HTMLXMLHttpRequestFactory_destructor, + .traverse = HTMLXMLHttpRequestFactory_traverse, + .unlink = HTMLXMLHttpRequestFactory_unlink, .value = HTMLXMLHttpRequestFactory_value }; @@ -1824,6 +1848,7 @@ HRESULT HTMLXMLHttpRequestFactory_Create(HTMLInnerWindow* window, HTMLXMLHttpReq ret->IHTMLXMLHttpRequestFactory_iface.lpVtbl = &HTMLXMLHttpRequestFactoryVtbl; ret->window = window; + IHTMLWindow2_AddRef(&window->base.IHTMLWindow2_iface); init_dispatch(&ret->dispex, &HTMLXMLHttpRequestFactory_dispex, dispex_compat_mode(&window->event_target.dispex)); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4178
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlstorage.c | 30 ++++++++++++++++++++++-------- dlls/mshtml/htmlwindow.c | 2 -- dlls/mshtml/mshtml_private.h | 2 -- 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/dlls/mshtml/htmlstorage.c b/dlls/mshtml/htmlstorage.c index d3bbb30580e..efd4b2d88ae 100644 --- a/dlls/mshtml/htmlstorage.c +++ b/dlls/mshtml/htmlstorage.c @@ -322,8 +322,6 @@ static HRESULT send_storage_event(HTMLStorage *storage, BSTR key, BSTR old_value HRESULT hres = S_OK; ctx.url = NULL; - if(!window) - goto done; if(window->base.outer_window->uri_nofrag) { hres = IUri_GetDisplayUri(window->base.outer_window->uri_nofrag, &ctx.url); if(hres != S_OK) @@ -1041,6 +1039,25 @@ static void *HTMLStorage_query_interface(DispatchEx *dispex, REFIID riid) return NULL; } +static void HTMLStorage_traverse(DispatchEx *dispex, nsCycleCollectionTraversalCallback *cb) +{ + HTMLStorage *This = impl_from_DispatchEx(dispex); + + if(This->window) + note_cc_edge((nsISupports*)&This->window->base.IHTMLWindow2_iface, "window", cb); +} + +static void HTMLStorage_unlink(DispatchEx *dispex) +{ + HTMLStorage *This = impl_from_DispatchEx(dispex); + + if(This->window) { + HTMLInnerWindow *window = This->window; + This->window = NULL; + IHTMLWindow2_Release(&window->base.IHTMLWindow2_iface); + } +} + static void HTMLStorage_destructor(DispatchEx *dispex) { HTMLStorage *This = impl_from_DispatchEx(dispex); @@ -1298,6 +1315,8 @@ static HRESULT HTMLStorage_next_dispid(DispatchEx *dispex, DISPID id, DISPID *pi static const dispex_static_data_vtbl_t HTMLStorage_dispex_vtbl = { .query_interface = HTMLStorage_query_interface, .destructor = HTMLStorage_destructor, + .traverse = HTMLStorage_traverse, + .unlink = HTMLStorage_unlink, .get_dispid = HTMLStorage_get_dispid, .get_name = HTMLStorage_get_name, .invoke = HTMLStorage_invoke, @@ -1464,15 +1483,10 @@ HRESULT create_html_storage(HTMLInnerWindow *window, BOOL local, IHTMLStorage ** storage->IHTMLStorage_iface.lpVtbl = &HTMLStorageVtbl; storage->window = window; + IHTMLWindow2_AddRef(&window->base.IHTMLWindow2_iface); init_dispatch(&storage->dispex, &HTMLStorage_dispex, dispex_compat_mode(&window->event_target.dispex)); *p = &storage->IHTMLStorage_iface; return S_OK; } - -void detach_html_storage(IHTMLStorage *iface) -{ - HTMLStorage *storage = impl_from_IHTMLStorage(iface); - storage->window = NULL; -} diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index 36aede7244e..dc1e42020fd 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -4001,13 +4001,11 @@ static void HTMLWindow_unlink(DispatchEx *dispex) unlink_ref(&This->navigator); if(This->session_storage) { IHTMLStorage *session_storage = This->session_storage; - detach_html_storage(session_storage); This->session_storage = NULL; IHTMLStorage_Release(session_storage); } if(This->local_storage) { IHTMLStorage *local_storage = This->local_storage; - detach_html_storage(local_storage); This->local_storage = NULL; IHTMLStorage_Release(local_storage); } diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index 7515fbfd2cd..504504dd1d4 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -998,9 +998,7 @@ HRESULT create_history(HTMLInnerWindow*,OmHistory**); HRESULT create_namespace_collection(compat_mode_t,IHTMLNamespaceCollection**); HRESULT create_dom_implementation(HTMLDocumentNode*,IHTMLDOMImplementation**); void detach_dom_implementation(IHTMLDOMImplementation*); - HRESULT create_html_storage(HTMLInnerWindow*,BOOL,IHTMLStorage**); -void detach_html_storage(IHTMLStorage*); void HTMLDocument_View_Init(HTMLDocumentObj*); void HTMLDocumentObj_Persist_Init(HTMLDocumentObj*); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4178
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlwindow.c | 1 - dlls/mshtml/omnavigator.c | 24 +++++++++++++++++++++++- dlls/mshtml/tests/events.c | 12 +++++++++++- 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index dc1e42020fd..38723919f15 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -3994,7 +3994,6 @@ static void HTMLWindow_unlink(DispatchEx *dispex) unlink_ref(&This->screen); if(This->history) { OmHistory *history = This->history; - This->history->window = NULL; This->history = NULL; IOmHistory_Release(&history->IOmHistory_iface); } diff --git a/dlls/mshtml/omnavigator.c b/dlls/mshtml/omnavigator.c index 58cd0c78fb2..db7da8276ef 100644 --- a/dlls/mshtml/omnavigator.c +++ b/dlls/mshtml/omnavigator.c @@ -644,7 +644,7 @@ static HRESULT WINAPI OmHistory_get_length(IOmHistory *iface, short *p) TRACE("(%p)->(%p)\n", This, p); - if(This->window && This->window->base.outer_window) + if(This->window->base.outer_window) browser = This->window->base.outer_window->browser; *p = browser && browser->doc->travel_log @@ -703,6 +703,25 @@ static void *OmHistory_query_interface(DispatchEx *dispex, REFIID riid) return NULL; } +static void OmHistory_traverse(DispatchEx *dispex, nsCycleCollectionTraversalCallback *cb) +{ + OmHistory *This = OmHistory_from_DispatchEx(dispex); + + if(This->window) + note_cc_edge((nsISupports*)&This->window->base.IHTMLWindow2_iface, "window", cb); +} + +static void OmHistory_unlink(DispatchEx *dispex) +{ + OmHistory *This = OmHistory_from_DispatchEx(dispex); + + if(This->window) { + HTMLInnerWindow *window = This->window; + This->window = NULL; + IHTMLWindow2_Release(&window->base.IHTMLWindow2_iface); + } +} + static void OmHistory_destructor(DispatchEx *dispex) { OmHistory *This = OmHistory_from_DispatchEx(dispex); @@ -712,6 +731,8 @@ static void OmHistory_destructor(DispatchEx *dispex) static const dispex_static_data_vtbl_t OmHistory_dispex_vtbl = { .query_interface = OmHistory_query_interface, .destructor = OmHistory_destructor, + .traverse = OmHistory_traverse, + .unlink = OmHistory_unlink, }; static const tid_t OmHistory_iface_tids[] = { @@ -738,6 +759,7 @@ HRESULT create_history(HTMLInnerWindow *window, OmHistory **ret) history->IOmHistory_iface.lpVtbl = &OmHistoryVtbl; history->window = window; + IHTMLWindow2_AddRef(&window->base.IHTMLWindow2_iface); *ret = history; return S_OK; diff --git a/dlls/mshtml/tests/events.c b/dlls/mshtml/tests/events.c index 33654fe46b5..64b07c750dc 100644 --- a/dlls/mshtml/tests/events.c +++ b/dlls/mshtml/tests/events.c @@ -3247,8 +3247,10 @@ static void test_window_refs(IHTMLDocument2 *doc) IHTMLWindow5 *window5; IHTMLDocument6 *doc6; IHTMLElement2 *elem; + IOmHistory *history; VARIANT vempty, var; HRESULT hres; + short length; BSTR bstr; V_VT(&vempty) = VT_EMPTY; @@ -3282,6 +3284,8 @@ static void test_window_refs(IHTMLDocument2 *doc) ok(hres == S_OK, "get_Image failed: %08lx\n", hres); hres = IHTMLWindow2_get_Option(window, &option_factory); ok(hres == S_OK, "get_Option failed: %08lx\n", hres); + hres = IHTMLWindow2_get_history(window, &history); + ok(hres == S_OK, "get_history failed: %08lx\n", hres); hres = IHTMLWindow2_get_self(window, &self); ok(hres == S_OK, "get_self failed: %08lx\n", hres); @@ -3316,6 +3320,12 @@ static void test_window_refs(IHTMLDocument2 *doc) ok(option_elem != NULL, "option_elem == NULL\n"); IHTMLOptionElementFactory_Release(option_factory); IHTMLOptionElement_Release(option_elem); + + hres = IOmHistory_get_length(history, &length); + ok(hres == S_OK, "get_length failed: %08lx\n", hres); + todo_wine + ok(length == 42, "length = %d\n", length); + IOmHistory_Release(history); } static void test_doc_obj(IHTMLDocument2 *doc) @@ -4396,7 +4406,7 @@ static HRESULT WINAPI TravelLog_Clone(ITravelLog *iface, ITravelLog **pptl) static DWORD WINAPI TravelLog_CountEntries(ITravelLog *iface, IUnknown *punk) { - return 0; + return 42; } static HRESULT WINAPI TravelLog_Revert(ITravelLog *iface) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4178
Looks like child windows indeed get detached, but rest don't. Note that both OmHistory and XHR tests are currently todo on Wine because they use outer window, and that also gets detached. I guess we will indeed need to keep refs from inner windows to outer windows after all, but I'll leave that aside for a follow-up MR, so for now they're todo_wine. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4178#note_50504
Jacek Caban (@jacek) commented about dlls/mshtml/tests/events.c:
static DWORD WINAPI TravelLog_CountEntries(ITravelLog *iface, IUnknown *punk) { - return 0; + return 42;
This change doesn't belong here. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4178#note_50507
On Wed Nov 1 15:18:06 2023 +0000, Jacek Caban wrote:
This change doesn't belong here. Nevermind, it's fine.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4178#note_50508
I guess we will indeed need to keep refs from inner windows to outer windows after all, but I'll leave that aside for a follow-up MR, so for now they're todo_wine.
There are other ways to support that, without keeping refs from inner windows. I find it hard to justify spending time on it if it's just for sake of this test, so let's leave it aside, please. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4178#note_50509
This merge request was approved by Jacek Caban. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4178
On Wed Nov 1 16:30:20 2023 +0000, Jacek Caban wrote:
I guess we will indeed need to keep refs from inner windows to outer windows after all, but I'll leave that aside for a follow-up MR, so for now they're todo_wine. There are other ways to support that, without keeping refs from inner windows. I find it hard to justify spending time on it if it's just for sake of this test, so let's leave it aside, please. It might be needed for other things as well. For example according to some tests, it's needed for document.parentWindow (as well as these tests ofc, well rather their behavior).
One possible solution would be to hold refs to the outer window instead (from these objects). But does that really make things better? It's still a cyclic ref to the outer window in such case. Note that currently the outer_window field in HTMLDocumentNode is broken. It doesn't hold a ref. So it can potentially crash and access invalid memory in very niche cases. It probably doesn't matter much in practice but I don't like it. We can't just detach it btw, as some existing tests will fail if we do that, as well as future get_parentWindow test. Anyway we will see later, I still have another MR to send up first with some cyclic refs and a leak fix. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4178#note_50532
participants (2)
-
Gabriel Ivăncescu -
Jacek Caban (＠jacek)