[PATCH 0/4] MR4312: mshtml: Return outer windows to external callers and use refs to them.
This fixes some inconsistency issues when it comes to inner vs outer windows. On second patch I keep ref again to outer windows from the inner windows, and there are several reasons for this: * It fixes the existing tests to match native IE. * It simplifies the code (it's always valid now, no second-guessing or what-ifs needed) and gets rid of `outer_window` in HTMLDocumentNode. * It **fixes** the `outer_window` in HTMLDocumentNode → it was basically prone to crashes in niche cases since it didn't hold a ref before. We can't couple it to the inner `window` field either, because that one gets detached, and that will fail *existing* tests. For example: `events.c:3224: Test failed: put_URL failed: 80004005`. * Instead of having to keep outer window refs in specific objects, it's simpler to have it consistent everywhere. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4312
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> UIEvent.view is still todo_wine because it returns NULL for some reason, but it is clear from the existing tests that it must match the outer window. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlevent.c | 2 +- dlls/mshtml/htmlwindow.c | 15 ++++++++------- dlls/mshtml/script.c | 2 +- dlls/mshtml/tests/dom.c | 15 +++++++++++++-- dlls/mshtml/tests/events.c | 12 ++++++++++++ dlls/mshtml/tests/jstest.html | 4 ++-- dlls/mshtml/tests/script.c | 1 - 7 files changed, 37 insertions(+), 14 deletions(-) diff --git a/dlls/mshtml/htmlevent.c b/dlls/mshtml/htmlevent.c index d6ad65b1d0b..6ee133d848a 100644 --- a/dlls/mshtml/htmlevent.c +++ b/dlls/mshtml/htmlevent.c @@ -1338,7 +1338,7 @@ static HRESULT WINAPI DOMUIEvent_get_view(IDOMUIEvent *iface, IHTMLWindow2 **p) mozIDOMWindowProxy_Release(moz_window); } if(view) - IHTMLWindow2_AddRef((*p = &view->base.inner_window->base.IHTMLWindow2_iface)); + IHTMLWindow2_AddRef((*p = &view->base.IHTMLWindow2_iface)); else *p = NULL; return S_OK; diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index 38723919f15..fc17635542d 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -504,11 +504,12 @@ static HRESULT WINAPI HTMLWindow2_get_length(IHTMLWindow2 *iface, LONG *p) static HRESULT WINAPI HTMLWindow2_get_frames(IHTMLWindow2 *iface, IHTMLFramesCollection2 **p) { HTMLWindow *This = impl_from_IHTMLWindow2(iface); + FIXME("(%p)->(%p): semi-stub\n", This, p); /* FIXME: Should return a separate Window object */ - *p = (IHTMLFramesCollection2*)&This->IHTMLWindow2_iface; - IHTMLWindow2_AddRef(iface); + *p = (IHTMLFramesCollection2*)&This->outer_window->base.IHTMLWindow2_iface; + IHTMLWindow2_AddRef(&This->outer_window->base.IHTMLWindow2_iface); return S_OK; } @@ -993,8 +994,8 @@ static HRESULT WINAPI HTMLWindow2_get_self(IHTMLWindow2 *iface, IHTMLWindow2 **p TRACE("(%p)->(%p)\n", This, p); /* FIXME: We should return kind of proxy window here. */ - IHTMLWindow2_AddRef(&This->IHTMLWindow2_iface); - *p = &This->IHTMLWindow2_iface; + *p = &This->outer_window->base.IHTMLWindow2_iface; + IHTMLWindow2_AddRef(*p); return S_OK; } @@ -1019,8 +1020,8 @@ static HRESULT WINAPI HTMLWindow2_get_window(IHTMLWindow2 *iface, IHTMLWindow2 * TRACE("(%p)->(%p)\n", This, p); /* FIXME: We should return kind of proxy window here. */ - IHTMLWindow2_AddRef(&This->IHTMLWindow2_iface); - *p = &This->IHTMLWindow2_iface; + *p = &This->outer_window->base.IHTMLWindow2_iface; + IHTMLWindow2_AddRef(*p); return S_OK; } @@ -4132,7 +4133,7 @@ static HRESULT HTMLWindow_invoke(DispatchEx *dispex, DISPID id, LCID lcid, WORD return DISP_E_MEMBERNOTFOUND; V_VT(res) = VT_DISPATCH; - V_DISPATCH(res) = (IDispatch*)&frame->base.inner_window->base.IHTMLWindow2_iface; + V_DISPATCH(res) = (IDispatch*)&frame->base.IHTMLWindow2_iface; IDispatch_AddRef(V_DISPATCH(res)); return S_OK; } diff --git a/dlls/mshtml/script.c b/dlls/mshtml/script.c index 149abcf70e2..86fffbe5656 100644 --- a/dlls/mshtml/script.c +++ b/dlls/mshtml/script.c @@ -378,7 +378,7 @@ static HRESULT WINAPI ActiveScriptSite_GetItemInfo(IActiveScriptSite *iface, LPC return E_FAIL; /* FIXME: Return proxy object */ - *ppiunkItem = (IUnknown*)&This->window->base.IHTMLWindow2_iface; + *ppiunkItem = (IUnknown*)&This->window->base.outer_window->base.IHTMLWindow2_iface; IUnknown_AddRef(*ppiunkItem); return S_OK; diff --git a/dlls/mshtml/tests/dom.c b/dlls/mshtml/tests/dom.c index b7079447cf2..58a1cc6cf20 100644 --- a/dlls/mshtml/tests/dom.c +++ b/dlls/mshtml/tests/dom.c @@ -7319,9 +7319,10 @@ static void test_window(IHTMLDocument2 *doc) hres = IHTMLWindow2_get_self(window, &self); ok(hres == S_OK, "get_self failed: %08lx\n", hres); - ok(window2 != NULL, "self == NULL\n"); + ok(self != NULL, "self == NULL\n"); ok(self == window2, "self != window2\n"); + todo_wine ok(window != window2, "window == window2\n"); IHTMLWindow2_Release(window2); @@ -10931,7 +10932,7 @@ static void test_frames_collection(IHTMLFramesCollection2 *frames, const WCHAR * static void test_frameset(IHTMLDocument2 *doc) { - IHTMLWindow2 *window; + IHTMLWindow2 *window, *window2, *self; IHTMLFramesCollection2 *frames; IHTMLDocument6 *doc6; IHTMLElement *elem; @@ -10946,6 +10947,16 @@ static void test_frameset(IHTMLDocument2 *doc) if(FAILED(hres)) return; + hres = IHTMLFramesCollection2_QueryInterface(frames, &IID_IHTMLWindow2, (void**)&window2); + ok(hres == S_OK, "QueryInterface(IID_IHTMLWindow2) failed: 0x%08lx\n", hres); + todo_wine ok(window != window2, "window == window2\n"); + + hres = IHTMLWindow2_get_self(window, &self); + ok(hres == S_OK, "get_self failed: %08lx\n", hres); + ok(self == window2, "self != window2\n"); + IHTMLWindow2_Release(window2); + IHTMLWindow2_Release(self); + test_frames_collection(frames, L"fr1"); IHTMLFramesCollection2_Release(frames); diff --git a/dlls/mshtml/tests/events.c b/dlls/mshtml/tests/events.c index 64b07c750dc..e5b9984e546 100644 --- a/dlls/mshtml/tests/events.c +++ b/dlls/mshtml/tests/events.c @@ -3343,6 +3343,7 @@ static void test_doc_obj(IHTMLDocument2 *doc) IHTMLPerformance *perf, *perf2; IOmHistory *history, *history2; IHTMLScreen *screen, *screen2; + IHTMLWindow2 *self, *window2; IEventTarget *event_target; DISPPARAMS dp = { 0 }; IHTMLWindow7 *window7; @@ -3504,6 +3505,11 @@ static void test_doc_obj(IHTMLDocument2 *doc) IHTMLWindow7_Release(window7); VariantClear(&res); + /* Test "proxy" windows as well, they're actually outer window proxies, but not the same */ + hres = IHTMLWindow2_get_self(window, &self); + ok(hres == S_OK, "get_self failed: %08lx\n", hres); + ok(self != NULL, "self == NULL\n"); + /* Add props to location, since it gets lost on navigation, despite being same object */ bstr = SysAllocString(L"wineTestProp"); hres = IHTMLLocation_QueryInterface(location, &IID_IDispatchEx, (void**)&dispex); @@ -3658,6 +3664,12 @@ static void test_doc_obj(IHTMLDocument2 *doc) IHTMLPerformance_Release(perf); IHTMLWindow7_Release(window7); VariantClear(&res); + + hres = IHTMLWindow2_get_self(window, &window2); + ok(hres == S_OK, "get_self failed: %08lx\n", hres); + ok(self == window2, "self != window2\n"); + IHTMLWindow2_Release(window2); + IHTMLWindow2_Release(self); } static void test_create_event(IHTMLDocument2 *doc) diff --git a/dlls/mshtml/tests/jstest.html b/dlls/mshtml/tests/jstest.html index 3bf9f316e7a..81fbea0c493 100644 --- a/dlls/mshtml/tests/jstest.html +++ b/dlls/mshtml/tests/jstest.html @@ -76,8 +76,8 @@ function test_document_name_as_index() { ok("iframeid" in window, "iframeid is not in window"); e = document.getElementById("iframeid"); ok(!!e, "e is null"); - ok(iframeid != e, "iframeid == e"); - ok(iframeid.frameElement === e, "frameid != e.contentWindow"); + ok(iframeid === e.contentWindow.self, "frameid != e.contentWindow.self"); + ok(iframeid.frameElement === e, "frameid.frameElement != e"); } function test_remove_style_attribute() { diff --git a/dlls/mshtml/tests/script.c b/dlls/mshtml/tests/script.c index de1fd056169..68ad3002a7f 100644 --- a/dlls/mshtml/tests/script.c +++ b/dlls/mshtml/tests/script.c @@ -4498,7 +4498,6 @@ static void test_exec_script(IHTMLDocument2 *doc, const WCHAR *codew, const WCHA hres = IHTMLDocument2_get_parentWindow(doc, &window); ok(hres == S_OK, "get_parentWindow failed: %08lx\n", hres); - todo_wine ok(iface_cmp((IUnknown *)window, (IUnknown *)window_dispex), "window != dispex_window\n"); code = SysAllocString(codew); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4312
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmldoc.c | 14 +++++++------- dlls/mshtml/htmlwindow.c | 35 +++++++++++++++++++++-------------- dlls/mshtml/tests/events.c | 10 +++++++--- 3 files changed, 35 insertions(+), 24 deletions(-) diff --git a/dlls/mshtml/htmldoc.c b/dlls/mshtml/htmldoc.c index 540bbbd1626..9f9791c3b1e 100644 --- a/dlls/mshtml/htmldoc.c +++ b/dlls/mshtml/htmldoc.c @@ -5707,12 +5707,6 @@ 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)); @@ -5884,14 +5878,20 @@ static void HTMLDocumentNode_traverse(DispatchEx *dispex, nsCycleCollectionTrave static void HTMLDocumentNode_unlink(DispatchEx *dispex) { HTMLDocumentNode *This = impl_from_DispatchEx(dispex); + HTMLInnerWindow *window = This->window; HTMLDOMNode_unlink(dispex); + if(window) { + This->window = NULL; + IHTMLWindow2_Release(&window->base.IHTMLWindow2_iface); + } + if(This->dom_document) { release_document_mutation(This); detach_document_node(This); This->dom_document = NULL; This->html_document = NULL; - }else if(This->window) { + }else if(window) { detach_document_node(This); } } diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index fc17635542d..88724d41007 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -134,12 +134,6 @@ static void detach_inner_window(HTMLInnerWindow *window) abort_window_bindings(window); remove_target_tasks(window->task_magic); release_script_hosts(window); - window->base.outer_window = NULL; - - if(outer_window && outer_window->base.inner_window == window) { - outer_window->base.inner_window = NULL; - IHTMLWindow2_Release(&window->base.IHTMLWindow2_iface); - } } static inline HTMLWindow *impl_from_IHTMLWindow2(IHTMLWindow2 *iface) @@ -3931,6 +3925,8 @@ static void HTMLWindow_traverse(DispatchEx *dispex, nsCycleCollectionTraversalCa HTMLOuterWindow *child; traverse_event_target(&This->event_target, cb); + if(This->base.outer_window) + note_cc_edge((nsISupports*)&This->base.outer_window->base.IHTMLWindow2_iface, "outer_window", cb); LIST_FOR_EACH_ENTRY(child, &This->children, HTMLOuterWindow, sibling_entry) note_cc_edge((nsISupports*)&child->base.IHTMLWindow2_iface, "child", cb); if(This->doc) @@ -3967,9 +3963,13 @@ static void HTMLWindow_unlink(DispatchEx *dispex) unlink_ref(&This->console); detach_inner_window(This); + if(This->base.outer_window) { + HTMLOuterWindow *outer_window = This->base.outer_window; + This->base.outer_window = NULL; + IHTMLWindow2_Release(&outer_window->base.IHTMLWindow2_iface); + } if(This->doc) { HTMLDocumentNode *doc = This->doc; - This->doc->window = NULL; This->doc = NULL; IHTMLDOMNode_Release(&doc->node.IHTMLDOMNode_iface); } @@ -4344,16 +4344,19 @@ static nsresult NSAPI outer_window_unlink(void *p) } if(window->pending_window) { HTMLInnerWindow *pending_window = window->pending_window; - abort_window_bindings(pending_window); - pending_window->base.outer_window = NULL; window->pending_window = NULL; + detach_inner_window(pending_window); IHTMLWindow2_Release(&pending_window->base.IHTMLWindow2_iface); } set_current_mon(window, NULL, 0); set_current_uri(window, NULL); - if(window->base.inner_window) - detach_inner_window(window->base.inner_window); + if(window->base.inner_window) { + HTMLInnerWindow *inner_window = window->base.inner_window; + window->base.inner_window = NULL; + detach_inner_window(inner_window); + IHTMLWindow2_Release(&inner_window->base.IHTMLWindow2_iface); + } if(window->location) { HTMLLocation *location = window->location; window->location = NULL; @@ -4429,6 +4432,7 @@ static HRESULT create_inner_window(HTMLOuterWindow *outer_window, IMoniker *mon, window->base.outer_window = outer_window; window->base.inner_window = window; + IHTMLWindow2_AddRef(&outer_window->base.IHTMLWindow2_iface); EventTarget_Init(&window->event_target, &HTMLWindow_dispex, COMPAT_MODE_NONE); @@ -4512,7 +4516,6 @@ HRESULT create_pending_window(HTMLOuterWindow *outer_window, nsChannelBSC *chann if(outer_window->pending_window) { abort_window_bindings(outer_window->pending_window); - outer_window->pending_window->base.outer_window = NULL; IHTMLWindow2_Release(&outer_window->pending_window->base.IHTMLWindow2_iface); } @@ -4552,8 +4555,12 @@ HRESULT update_window_doc(HTMLInnerWindow *window) return S_OK; } - if(outer_window->base.inner_window) - detach_inner_window(outer_window->base.inner_window); + if(outer_window->base.inner_window) { + HTMLInnerWindow *inner_window = outer_window->base.inner_window; + outer_window->base.inner_window = NULL; + detach_inner_window(inner_window); + IHTMLWindow2_Release(&inner_window->base.IHTMLWindow2_iface); + } outer_window->base.inner_window = window; outer_window->pending_window = NULL; diff --git a/dlls/mshtml/tests/events.c b/dlls/mshtml/tests/events.c index e5b9984e546..386b17d2b81 100644 --- a/dlls/mshtml/tests/events.c +++ b/dlls/mshtml/tests/events.c @@ -3304,10 +3304,10 @@ static void test_window_refs(IHTMLDocument2 *doc) IHTMLWindow2_Release(child); hres = IHTMLXMLHttpRequestFactory_create(xhr_factory, &xhr); - todo_wine ok(hres == S_OK, "create failed: %08lx\n", hres); + ok(xhr != NULL, "xhr == NULL\n"); IHTMLXMLHttpRequestFactory_Release(xhr_factory); - if(hres == S_OK) IHTMLXMLHttpRequest_Release(xhr); + IHTMLXMLHttpRequest_Release(xhr); hres = IHTMLImageElementFactory_create(image_factory, vempty, vempty, &img_elem); ok(hres == S_OK, "create failed: %08lx\n", hres); @@ -3323,7 +3323,6 @@ static void test_window_refs(IHTMLDocument2 *doc) 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); } @@ -3577,7 +3576,12 @@ static void test_doc_obj(IHTMLDocument2 *doc) ok(hres == S_OK, "get_document failed: %08lx\n", hres); ok(doc_node != doc_node2, "doc_node == doc_node2\n"); IHTMLDocument2_Release(doc_node2); + + hres = IHTMLDocument2_get_parentWindow(doc_node, &window2); + ok(hres == S_OK, "get_parentWindow failed: %08lx\n", hres); + ok(window == window2, "window != window2\n"); IHTMLDocument2_Release(doc_node); + IHTMLWindow2_Release(window2); hres = IHTMLWindow2_get_location(window, &location2); ok(hres == S_OK, "get_location failed: %08lx\n", hres); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4312
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Since we keep valid ref to inner window now, we do not need it anymore. Furthermore, it was wrong because it didn't even hold a ref to the outer window, and it wasn't detaching it either... Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/editor.c | 8 +++---- dlls/mshtml/htmlanchor.c | 6 ++--- dlls/mshtml/htmldoc.c | 43 ++++++++++++++++++------------------ dlls/mshtml/htmlframe.c | 6 ++--- dlls/mshtml/mshtml_private.h | 1 - dlls/mshtml/mutation.c | 2 +- dlls/mshtml/nsevents.c | 10 ++++----- dlls/mshtml/olecmd.c | 6 ++--- dlls/mshtml/pluginhost.c | 4 ++-- dlls/mshtml/range.c | 2 +- dlls/mshtml/secmgr.c | 8 +++---- 11 files changed, 47 insertions(+), 49 deletions(-) diff --git a/dlls/mshtml/editor.c b/dlls/mshtml/editor.c index 478c30932c1..879e71878ed 100644 --- a/dlls/mshtml/editor.c +++ b/dlls/mshtml/editor.c @@ -146,7 +146,7 @@ static DWORD query_ns_edit_status(HTMLDocumentNode *doc, const char *nscmd) nsICommandParams *nsparam; cpp_bool b = FALSE; - if(doc->browser->usermode != EDITMODE || doc->outer_window->readystate < READYSTATE_INTERACTIVE) + if(doc->browser->usermode != EDITMODE || doc->window->base.outer_window->readystate < READYSTATE_INTERACTIVE) return OLECMDF_SUPPORTED; if(nscmd) { @@ -180,7 +180,7 @@ static DWORD query_align_status(HTMLDocumentNode *doc, const WCHAR *align) cpp_bool b; nsresult nsres; - if(doc->browser->usermode != EDITMODE || doc->outer_window->readystate < READYSTATE_INTERACTIVE) + if(doc->browser->usermode != EDITMODE || doc->window->base.outer_window->readystate < READYSTATE_INTERACTIVE) return OLECMDF_SUPPORTED; if(!doc->html_document) { @@ -203,7 +203,7 @@ static nsISelection *get_ns_selection(HTMLDocumentNode *doc) nsISelection *nsselection = NULL; nsresult nsres; - nsres = nsIDOMWindow_GetSelection(doc->outer_window->nswindow, &nsselection); + nsres = nsIDOMWindow_GetSelection(doc->window->base.outer_window->nswindow, &nsselection); if(NS_FAILED(nsres)) ERR("GetSelection failed %08lx\n", nsres); @@ -666,7 +666,7 @@ static HRESULT query_justify(HTMLDocumentNode *doc, OLECMD *cmd) case IDM_JUSTIFYLEFT: TRACE("(%p) IDM_JUSTIFYLEFT\n", doc); /* FIXME: We should set OLECMDF_LATCHED only if it's set explicitly. */ - if(doc->browser->usermode != EDITMODE || doc->outer_window->readystate < READYSTATE_INTERACTIVE) + if(doc->browser->usermode != EDITMODE || doc->window->base.outer_window->readystate < READYSTATE_INTERACTIVE) cmd->cmdf = OLECMDF_SUPPORTED; else cmd->cmdf = OLECMDF_SUPPORTED | OLECMDF_ENABLED; diff --git a/dlls/mshtml/htmlanchor.c b/dlls/mshtml/htmlanchor.c index 5398f87ce55..5f8a98faf17 100644 --- a/dlls/mshtml/htmlanchor.c +++ b/dlls/mshtml/htmlanchor.c @@ -49,11 +49,11 @@ static HRESULT navigate_href_new_window(HTMLElement *element, nsAString *href_st HRESULT hres; nsAString_GetData(href_str, &href); - hres = create_relative_uri(element->node.doc->outer_window, href, &uri); + hres = create_relative_uri(element->node.doc->window->base.outer_window, href, &uri); if(FAILED(hres)) return hres; - hres = navigate_new_window(element->node.doc->outer_window, uri, target, NULL, NULL); + hres = navigate_new_window(element->node.doc->window->base.outer_window, uri, target, NULL, NULL); IUri_Release(uri); return hres; } @@ -110,7 +110,7 @@ static HRESULT navigate_href(HTMLElement *element, nsAString *href_str, nsAStrin const PRUnichar *href; HRESULT hres; - window = get_target_window(element->node.doc->outer_window, target_str, &use_new_window); + window = get_target_window(element->node.doc->window->base.outer_window, target_str, &use_new_window); if(!window) { if(use_new_window) { const PRUnichar *target; diff --git a/dlls/mshtml/htmldoc.c b/dlls/mshtml/htmldoc.c index 9f9791c3b1e..afb3f819022 100644 --- a/dlls/mshtml/htmldoc.c +++ b/dlls/mshtml/htmldoc.c @@ -905,7 +905,7 @@ static HRESULT WINAPI HTMLDocument_get_readyState(IHTMLDocument2 *iface, BSTR *p if(!p) return E_POINTER; - return get_readystate_string(This->outer_window ? This->outer_window->readystate : 0, p); + return get_readystate_string(This->window ? This->window->base.outer_window->readystate : 0, p); } static HRESULT WINAPI HTMLDocument_get_frames(IHTMLDocument2 *iface, IHTMLFramesCollection2 **p) @@ -914,11 +914,11 @@ static HRESULT WINAPI HTMLDocument_get_frames(IHTMLDocument2 *iface, IHTMLFrames TRACE("(%p)->(%p)\n", This, p); - if(!This->outer_window) { + if(!This->window) { /* Not implemented by IE */ return E_NOTIMPL; } - return IHTMLWindow2_get_frames(&This->outer_window->base.IHTMLWindow2_iface, p); + return IHTMLWindow2_get_frames(&This->window->base.outer_window->base.IHTMLWindow2_iface, p); } static HRESULT WINAPI HTMLDocument_get_embeds(IHTMLDocument2 *iface, IHTMLElementCollection **p) @@ -1094,12 +1094,12 @@ static HRESULT WINAPI HTMLDocument_put_URL(IHTMLDocument2 *iface, BSTR v) TRACE("(%p)->(%s)\n", This, debugstr_w(v)); - if(!This->outer_window) { + if(!This->window) { FIXME("No window available\n"); return E_FAIL; } - return navigate_url(This->outer_window, v, This->outer_window->uri, BINDING_NAVIGATED); + return navigate_url(This->window->base.outer_window, v, This->window->base.outer_window->uri, BINDING_NAVIGATED); } static HRESULT WINAPI HTMLDocument_get_URL(IHTMLDocument2 *iface, BSTR *p) @@ -1108,7 +1108,7 @@ static HRESULT WINAPI HTMLDocument_get_URL(IHTMLDocument2 *iface, BSTR *p) TRACE("(%p)->(%p)\n", iface, p); - *p = SysAllocString(This->outer_window && This->outer_window->url ? This->outer_window->url : L"about:blank"); + *p = SysAllocString(This->window && This->window->base.outer_window->url ? This->window->base.outer_window->url : L"about:blank"); return *p ? S_OK : E_OUTOFMEMORY; } @@ -1149,12 +1149,12 @@ static HRESULT WINAPI HTMLDocument_get_domain(IHTMLDocument2 *iface, BSTR *p) return E_NOTIMPL; } - if(This->outer_window && !This->outer_window->uri) + if(This->window && !This->window->base.outer_window->uri) return E_FAIL; nsAString_Init(&nsstr, NULL); nsres = nsIDOMHTMLDocument_GetDomain(This->html_document, &nsstr); - if(NS_SUCCEEDED(nsres) && This->outer_window && This->outer_window->uri) { + if(NS_SUCCEEDED(nsres) && This->window && This->window->base.outer_window->uri) { const PRUnichar *str; HRESULT hres; @@ -1162,7 +1162,7 @@ static HRESULT WINAPI HTMLDocument_get_domain(IHTMLDocument2 *iface, BSTR *p) if(!*str) { TRACE("Gecko returned empty string, fallback to loaded URL.\n"); nsAString_Finish(&nsstr); - hres = IUri_GetHost(This->outer_window->uri, p); + hres = IUri_GetHost(This->window->base.outer_window->uri, p); return FAILED(hres) ? hres : S_OK; } } @@ -1177,10 +1177,10 @@ static HRESULT WINAPI HTMLDocument_put_cookie(IHTMLDocument2 *iface, BSTR v) TRACE("(%p)->(%s)\n", This, debugstr_w(v)); - if(!This->outer_window) + if(!This->window) return S_OK; - bret = InternetSetCookieExW(This->outer_window->url, NULL, v, 0, 0); + bret = InternetSetCookieExW(This->window->base.outer_window->url, NULL, v, 0, 0); if(!bret) { FIXME("InternetSetCookieExW failed: %lu\n", GetLastError()); return HRESULT_FROM_WIN32(GetLastError()); @@ -1197,13 +1197,13 @@ static HRESULT WINAPI HTMLDocument_get_cookie(IHTMLDocument2 *iface, BSTR *p) TRACE("(%p)->(%p)\n", This, p); - if(!This->outer_window) { + if(!This->window) { *p = NULL; return S_OK; } size = 0; - bret = InternetGetCookieExW(This->outer_window->url, NULL, NULL, &size, 0, NULL); + bret = InternetGetCookieExW(This->window->base.outer_window->url, NULL, NULL, &size, 0, NULL); if(!bret && GetLastError() != ERROR_INSUFFICIENT_BUFFER) { WARN("InternetGetCookieExW failed: %lu\n", GetLastError()); *p = NULL; @@ -1219,7 +1219,7 @@ static HRESULT WINAPI HTMLDocument_get_cookie(IHTMLDocument2 *iface, BSTR *p) if(!*p) return E_OUTOFMEMORY; - bret = InternetGetCookieExW(This->outer_window->url, NULL, *p, &size, 0, NULL); + bret = InternetGetCookieExW(This->window->base.outer_window->url, NULL, *p, &size, 0, NULL); if(!bret) { ERR("InternetGetCookieExW failed: %lu\n", GetLastError()); return E_FAIL; @@ -1448,7 +1448,7 @@ static HRESULT WINAPI HTMLDocument_open(IHTMLDocument2 *iface, BSTR url, VARIANT *pomWindowResult = NULL; - if(!This->outer_window) + if(!This->window) return E_FAIL; if(!This->dom_document) { @@ -1475,8 +1475,8 @@ static HRESULT WINAPI HTMLDocument_open(IHTMLDocument2 *iface, BSTR url, VARIANT if(tmp) nsISupports_Release(tmp); - *pomWindowResult = (IDispatch*)&This->outer_window->base.IHTMLWindow2_iface; - IHTMLWindow2_AddRef(&This->outer_window->base.IHTMLWindow2_iface); + *pomWindowResult = (IDispatch*)&This->window->base.outer_window->base.IHTMLWindow2_iface; + IHTMLWindow2_AddRef(&This->window->base.outer_window->base.IHTMLWindow2_iface); return S_OK; } @@ -2349,7 +2349,7 @@ static HRESULT WINAPI HTMLDocument3_get_documentElement(IHTMLDocument3 *iface, I TRACE("(%p)->(%p)\n", This, p); - if(This->outer_window && This->outer_window->readystate == READYSTATE_UNINITIALIZED) { + if(This->window && This->window->base.outer_window->readystate == READYSTATE_UNINITIALIZED) { *p = NULL; return S_OK; } @@ -4876,7 +4876,7 @@ static void HTMLDocumentNode_on_advise(IUnknown *iface, cp_static_data_t *cp) { HTMLDocumentNode *This = CONTAINING_RECORD((IHTMLDocument2*)iface, HTMLDocumentNode, IHTMLDocument2_iface); - if(This->outer_window) + if(This->window) update_doc_cp_events(This, cp); } @@ -6070,10 +6070,10 @@ static HRESULT HTMLDocumentNode_location_hook(DispatchEx *dispex, WORD flags, DI { HTMLDocumentNode *This = impl_from_DispatchEx(dispex); - if(!(flags & DISPATCH_PROPERTYPUT) || !This->outer_window) + if(!(flags & DISPATCH_PROPERTYPUT) || !This->window) return S_FALSE; - return IDispatchEx_InvokeEx(&This->outer_window->base.IDispatchEx_iface, DISPID_IHTMLWINDOW2_LOCATION, + return IDispatchEx_InvokeEx(&This->window->base.outer_window->base.IDispatchEx_iface, DISPID_IHTMLWINDOW2_LOCATION, 0, flags, dp, res, ei, caller); } @@ -6187,7 +6187,6 @@ static HTMLDocumentNode *alloc_doc_node(HTMLDocumentObj *doc_obj, HTMLInnerWindo doc->IDocumentRange_iface.lpVtbl = &DocumentRangeVtbl; doc->doc_obj = doc_obj; - doc->outer_window = window ? window->base.outer_window : NULL; doc->window = window; if(window) diff --git a/dlls/mshtml/htmlframe.c b/dlls/mshtml/htmlframe.c index 1a78dc3cc07..6334c2f0d35 100644 --- a/dlls/mshtml/htmlframe.c +++ b/dlls/mshtml/htmlframe.c @@ -50,7 +50,7 @@ static HRESULT set_frame_doc(HTMLFrameBase *frame, nsIDOMDocument *nsdoc) window = mozwindow_to_window(mozwindow); if(!window && frame->element.node.doc->browser) { hres = create_outer_window(frame->element.node.doc->browser, mozwindow, - frame->element.node.doc->outer_window, &window); + frame->element.node.doc->window->base.outer_window, &window); /* Don't hold ref to the created window; the parent keeps ref to it */ if(SUCCEEDED(hres)) @@ -132,7 +132,7 @@ static HRESULT WINAPI HTMLFrameBase_put_src(IHTMLFrameBase *iface, BSTR v) TRACE("(%p)->(%s)\n", This, debugstr_w(v)); - if(!This->content_window || !This->element.node.doc || !This->element.node.doc->outer_window) { + if(!This->content_window || !This->element.node.doc || !This->element.node.doc->window) { nsAString nsstr; nsresult nsres; @@ -150,7 +150,7 @@ static HRESULT WINAPI HTMLFrameBase_put_src(IHTMLFrameBase *iface, BSTR v) return S_OK; } - return navigate_url(This->content_window, v, This->element.node.doc->outer_window->uri, BINDING_NAVIGATED); + return navigate_url(This->content_window, v, This->element.node.doc->window->base.outer_window->uri, BINDING_NAVIGATED); } static HRESULT WINAPI HTMLFrameBase_get_src(IHTMLFrameBase *iface, BSTR *p) diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index 23e1decc68b..e912be76af0 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -939,7 +939,6 @@ struct HTMLDocumentNode { nsIDocumentObserver nsIDocumentObserver_iface; ConnectionPointContainer cp_container; - HTMLOuterWindow *outer_window; HTMLInnerWindow *window; HTMLDocumentObj *doc_obj; diff --git a/dlls/mshtml/mutation.c b/dlls/mshtml/mutation.c index a1438debd6f..845c189f98b 100644 --- a/dlls/mshtml/mutation.c +++ b/dlls/mshtml/mutation.c @@ -314,7 +314,7 @@ static nsresult run_end_load(HTMLDocumentNode *This, nsISupports *arg1, nsISuppo if(This->window == window) { window->dom_interactive_time = get_time_stamp(); - set_ready_state(This->outer_window, READYSTATE_INTERACTIVE); + set_ready_state(This->window->base.outer_window, READYSTATE_INTERACTIVE); } IHTMLWindow2_Release(&window->base.IHTMLWindow2_iface); return NS_OK; diff --git a/dlls/mshtml/nsevents.c b/dlls/mshtml/nsevents.c index 8c6631626d1..991e6decd1e 100644 --- a/dlls/mshtml/nsevents.c +++ b/dlls/mshtml/nsevents.c @@ -336,7 +336,7 @@ static nsresult handle_load(HTMLDocumentNode *doc, nsIDOMEvent *event) TRACE("(%p)\n", doc); - if(!doc->outer_window) + if(!doc->window) return NS_ERROR_FAILURE; if(doc->doc_obj && doc->doc_obj->doc_node == doc) { doc_obj = doc->doc_obj; @@ -348,7 +348,7 @@ static nsresult handle_load(HTMLDocumentNode *doc, nsIDOMEvent *event) handle_docobj_load(doc_obj); doc->window->dom_complete_time = get_time_stamp(); - set_ready_state(doc->outer_window, READYSTATE_COMPLETE); + set_ready_state(doc->window->base.outer_window, READYSTATE_COMPLETE); if(doc_obj) { if(doc_obj->view_sink) @@ -358,9 +358,9 @@ static nsresult handle_load(HTMLDocumentNode *doc, nsIDOMEvent *event) update_title(doc_obj); - if(doc_obj->doc_object_service && !(doc->outer_window->load_flags & BINDING_REFRESH)) + if(doc_obj->doc_object_service && !(doc->window->base.outer_window->load_flags & BINDING_REFRESH)) IDocObjectService_FireDocumentComplete(doc_obj->doc_object_service, - &doc->outer_window->base.IHTMLWindow2_iface, 0); + &doc->window->base.outer_window->base.IHTMLWindow2_iface, 0); IUnknown_Release(doc_obj->outer_unk); } @@ -516,7 +516,7 @@ static nsIDOMEventTarget *get_default_document_target(HTMLDocumentNode *doc) nsISupports *target_iface; nsresult nsres; - target_iface = doc->window ? (nsISupports*)doc->outer_window->nswindow : (nsISupports*)doc->dom_document; + target_iface = doc->window ? (nsISupports*)doc->window->base.outer_window->nswindow : (nsISupports*)doc->dom_document; nsres = nsISupports_QueryInterface(target_iface, &IID_nsIDOMEventTarget, (void**)&target); return NS_SUCCEEDED(nsres) ? target : NULL; } diff --git a/dlls/mshtml/olecmd.c b/dlls/mshtml/olecmd.c index 27690b44e79..f5f80cffd68 100644 --- a/dlls/mshtml/olecmd.c +++ b/dlls/mshtml/olecmd.c @@ -69,7 +69,7 @@ static nsIClipboardCommands *get_clipboard_commands(HTMLDocumentNode *doc) nsIDocShell *doc_shell; nsresult nsres; - nsres = get_nsinterface((nsISupports*)doc->outer_window->nswindow, &IID_nsIDocShell, (void**)&doc_shell); + nsres = get_nsinterface((nsISupports*)doc->window->base.outer_window->nswindow, &IID_nsIDocShell, (void**)&doc_shell); if(NS_FAILED(nsres)) { ERR("Could not get nsIDocShell interface\n"); return NULL; @@ -487,10 +487,10 @@ static HRESULT exec_refresh(HTMLDocumentNode *doc, DWORD nCmdexecopt, VARIANT *p } } - if(!doc->outer_window) + if(!doc->window) return E_UNEXPECTED; - return reload_page(doc->outer_window); + return reload_page(doc->window->base.outer_window); } static HRESULT exec_stop(HTMLDocumentNode *doc, DWORD nCmdexecopt, VARIANT *pvaIn, VARIANT *pvaOut) diff --git a/dlls/mshtml/pluginhost.c b/dlls/mshtml/pluginhost.c index f910600de18..ae52cb40859 100644 --- a/dlls/mshtml/pluginhost.c +++ b/dlls/mshtml/pluginhost.c @@ -2227,12 +2227,12 @@ static HRESULT WINAPI PHServiceProvider_QueryService(IServiceProvider *iface, RE TRACE("(%p)->(%s %s %p)\n", This, debugstr_guid(guidService), debugstr_guid(riid), ppv); - if(!This->doc || !This->doc->outer_window) { + if(!This->doc || !This->doc->window) { *ppv = NULL; return E_NOINTERFACE; } - return IServiceProvider_QueryService(&This->doc->outer_window->base.IServiceProvider_iface, + return IServiceProvider_QueryService(&This->doc->window->base.outer_window->base.IServiceProvider_iface, guidService, riid, ppv); } diff --git a/dlls/mshtml/range.c b/dlls/mshtml/range.c index 738818d93fb..cb5efc984dc 100644 --- a/dlls/mshtml/range.c +++ b/dlls/mshtml/range.c @@ -1287,7 +1287,7 @@ static HRESULT WINAPI HTMLTxtRange_select(IHTMLTxtRange *iface) TRACE("(%p)\n", This); - nsres = nsIDOMWindow_GetSelection(This->doc->outer_window->nswindow, &nsselection); + nsres = nsIDOMWindow_GetSelection(This->doc->window->base.outer_window->nswindow, &nsselection); if(NS_FAILED(nsres)) { ERR("GetSelection failed: %08lx\n", nsres); return E_FAIL; diff --git a/dlls/mshtml/secmgr.c b/dlls/mshtml/secmgr.c index 24c7ef2c817..7da65fe014f 100644 --- a/dlls/mshtml/secmgr.c +++ b/dlls/mshtml/secmgr.c @@ -76,10 +76,10 @@ static HRESULT WINAPI InternetHostSecurityManager_ProcessUrlAction(IInternetHost TRACE("(%p)->(%ld %p %ld %p %ld %lx %lx)\n", This, dwAction, pPolicy, cbPolicy, pContext, cbContext, dwFlags, dwReserved); - if(!This->outer_window) + if(!This->window) return E_UNEXPECTED; - url = This->outer_window->url ? This->outer_window->url : L"about:blank"; + url = This->window->base.outer_window->url ? This->window->base.outer_window->url : L"about:blank"; return IInternetSecurityManager_ProcessUrlAction(get_security_manager(), url, dwAction, pPolicy, cbPolicy, pContext, cbContext, dwFlags, dwReserved); @@ -181,10 +181,10 @@ static HRESULT WINAPI InternetHostSecurityManager_QueryCustomPolicy(IInternetHos TRACE("(%p)->(%s %p %p %p %ld %lx)\n", This, debugstr_guid(guidKey), ppPolicy, pcbPolicy, pContext, cbContext, dwReserved); - if(!This->outer_window) + if(!This->window) return E_UNEXPECTED; - url = This->outer_window->url ? This->outer_window->url : L"about:blank"; + url = This->window->base.outer_window->url ? This->window->base.outer_window->url : L"about:blank"; hres = IInternetSecurityManager_QueryCustomPolicy(get_security_manager(), url, guidKey, ppPolicy, pcbPolicy, pContext, cbContext, dwReserved); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4312
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmldoc.c | 2 +- dlls/mshtml/htmlform.c | 2 +- dlls/mshtml/htmlwindow.c | 48 ++++++++++++++---------------------- dlls/mshtml/mutation.c | 4 +-- dlls/mshtml/omnavigator.c | 3 +-- dlls/mshtml/script.c | 2 +- dlls/mshtml/xmlhttprequest.c | 3 --- 7 files changed, 24 insertions(+), 40 deletions(-) diff --git a/dlls/mshtml/htmldoc.c b/dlls/mshtml/htmldoc.c index afb3f819022..47698ca01e1 100644 --- a/dlls/mshtml/htmldoc.c +++ b/dlls/mshtml/htmldoc.c @@ -3627,7 +3627,7 @@ static HRESULT WINAPI HTMLDocument7_get_defaultView(IHTMLDocument7 *iface, IHTML TRACE("(%p)->(%p)\n", This, p); - if(This->window && This->window->base.outer_window) { + if(This->window) { *p = &This->window->base.outer_window->base.IHTMLWindow2_iface; IHTMLWindow2_AddRef(*p); }else { diff --git a/dlls/mshtml/htmlform.c b/dlls/mshtml/htmlform.c index fc30742d540..f83e36abb60 100644 --- a/dlls/mshtml/htmlform.c +++ b/dlls/mshtml/htmlform.c @@ -570,7 +570,7 @@ static HRESULT WINAPI HTMLFormElement_submit(IHTMLFormElement *iface) if(This->element.node.doc) { HTMLDocumentNode *doc = This->element.node.doc; - if(doc->window && doc->window->base.outer_window) + if(doc->window) this_window = doc->window->base.outer_window; } if(!this_window) { diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index 88724d41007..a2aeb62dfd1 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -578,7 +578,7 @@ static HRESULT WINAPI HTMLWindow2_alert(IHTMLWindow2 *iface, BSTR message) TRACE("(%p)->(%s)\n", This, debugstr_w(message)); - if(!This->outer_window || !This->outer_window->browser) + if(!This->outer_window->browser) return E_UNEXPECTED; if(!LoadStringW(get_shdoclc(), IDS_MESSAGE_BOX_TITLE, title, ARRAY_SIZE(title))) { @@ -611,7 +611,7 @@ static HRESULT WINAPI HTMLWindow2_confirm(IHTMLWindow2 *iface, BSTR message, if(!confirmed) return E_INVALIDARG; - if(!This->outer_window || !This->outer_window->browser) + if(!This->outer_window->browser) return E_UNEXPECTED; if(!LoadStringW(get_shdoclc(), IDS_MESSAGE_BOX_TITLE, wszTitle, ARRAY_SIZE(wszTitle))) { @@ -705,7 +705,7 @@ static HRESULT WINAPI HTMLWindow2_prompt(IHTMLWindow2 *iface, BSTR message, TRACE("(%p)->(%s %s %p)\n", This, debugstr_w(message), debugstr_w(dststr), textdata); - if(!This->outer_window || !This->outer_window->browser) + if(!This->outer_window->browser) return E_UNEXPECTED; if(textdata) V_VT(textdata) = VT_NULL; @@ -840,7 +840,7 @@ static HRESULT WINAPI HTMLWindow2_close(IHTMLWindow2 *iface) TRACE("(%p)\n", This); - if(!window || !window->browser) { + if(!window->browser) { FIXME("No document object\n"); return E_FAIL; } @@ -947,7 +947,7 @@ static HRESULT WINAPI HTMLWindow2_open(IHTMLWindow2 *iface, BSTR url, BSTR name, if(replace) FIXME("unsupported relace argument\n"); - if(!window || !window->browser || !window->uri_nofrag) + if(!window->browser || !window->uri_nofrag) return E_UNEXPECTED; if(name && *name == '_') { @@ -1304,7 +1304,7 @@ static HRESULT WINAPI HTMLWindow2_focus(IHTMLWindow2 *iface) TRACE("(%p)->()\n", This); - if(!This->outer_window || !This->outer_window->browser) + if(!This->outer_window->browser) return E_UNEXPECTED; SetFocus(This->outer_window->browser->doc->hwnd); @@ -1471,7 +1471,7 @@ static HRESULT WINAPI HTMLWindow2_get_external(IHTMLWindow2 *iface, IDispatch ** TRACE("(%p)->(%p)\n", This, p); - if(!This->outer_window || !This->outer_window->browser) + if(!This->outer_window->browser) return E_UNEXPECTED; *p = NULL; @@ -2390,9 +2390,6 @@ static HRESULT WINAPI HTMLWindow7_getComputedStyle(IHTMLWindow7 *iface, IHTMLDOM TRACE("(%p)->(%p %s %p)\n", This, node, debugstr_w(pseudo_elt), p); - if(!This->outer_window || !This->inner_window) - return E_UNEXPECTED; - hres = IHTMLDOMNode_QueryInterface(node, &IID_IHTMLElement, (void**)&elem); if(FAILED(hres)) return hres; @@ -2826,7 +2823,7 @@ static HRESULT WINAPI HTMLPrivateWindow_SuperNavigate(IHTMLPrivateWindow *iface, if(flags & ~2) FIXME("unimplemented flags %lx\n", flags & ~2); - if(!window || !window->browser) + if(!window->browser) return E_FAIL; if(window->browser->doc->hostui) { @@ -3635,6 +3632,7 @@ static HRESULT WINAPI WindowDispEx_GetDispID(IDispatchEx *iface, BSTR bstrName, { HTMLWindow *This = impl_from_IDispatchEx(iface); HTMLInnerWindow *window = This->inner_window; + HTMLOuterWindow *frame; HRESULT hres; TRACE("(%p)->(%s %lx %p)\n", This, debugstr_w(bstrName), grfdex, pid); @@ -3647,20 +3645,16 @@ static HRESULT WINAPI WindowDispEx_GetDispID(IDispatchEx *iface, BSTR bstrName, if(hres != DISP_E_UNKNOWNNAME) return hres; - if(This->outer_window) { - HTMLOuterWindow *frame; - - hres = get_frame_by_name(This->outer_window, bstrName, FALSE, &frame); - if(SUCCEEDED(hres) && frame) { - global_prop_t *prop; + hres = get_frame_by_name(This->outer_window, bstrName, FALSE, &frame); + if(SUCCEEDED(hres) && frame) { + global_prop_t *prop; - prop = alloc_global_prop(window, GLOBAL_FRAMEVAR, bstrName); - if(!prop) - return E_OUTOFMEMORY; + prop = alloc_global_prop(window, GLOBAL_FRAMEVAR, bstrName); + if(!prop) + return E_OUTOFMEMORY; - *pid = prop_to_dispid(window, prop); - return S_OK; - } + *pid = prop_to_dispid(window, prop); + return S_OK; } if(window->doc) { @@ -3893,7 +3887,7 @@ static HRESULT WINAPI HTMLWindowSP_QueryService(IServiceProvider *iface, REFGUID TRACE("(%p)->(%s %s %p)\n", This, debugstr_mshtml_guid(guidService), debugstr_mshtml_guid(riid), ppv); - if(!This->outer_window || !This->outer_window->browser) + if(!This->outer_window->browser) return E_NOINTERFACE; return IServiceProvider_QueryService(&This->outer_window->browser->doc->IServiceProvider_iface, @@ -4118,9 +4112,6 @@ static HRESULT HTMLWindow_invoke(DispatchEx *dispex, DISPID id, LCID lcid, WORD return E_NOTIMPL; } case GLOBAL_FRAMEVAR: - if(!This->base.outer_window) - return E_UNEXPECTED; - switch(flags) { case DISPATCH_PROPERTYGET: { HTMLOuterWindow *frame; @@ -4533,9 +4524,6 @@ HRESULT update_window_doc(HTMLInnerWindow *window) assert(!window->doc); - if(!outer_window) - return E_UNEXPECTED; - nsres = nsIDOMWindow_GetDocument(outer_window->nswindow, &nsdoc); if(NS_FAILED(nsres) || !nsdoc) { ERR("GetDocument failed: %08lx\n", nsres); diff --git a/dlls/mshtml/mutation.c b/dlls/mshtml/mutation.c index 845c189f98b..3454bc2161b 100644 --- a/dlls/mshtml/mutation.c +++ b/dlls/mshtml/mutation.c @@ -437,7 +437,7 @@ static void set_document_mode(HTMLDocumentNode *doc, compat_mode_t document_mode TRACE("%p: %d\n", doc, document_mode); - max_compat_mode = doc->window && doc->window->base.outer_window + max_compat_mode = doc->window ? get_max_compat_mode(doc->window->base.outer_window->uri) : COMPAT_MODE_IE11; if(max_compat_mode < document_mode) { @@ -896,7 +896,7 @@ static void NSAPI nsDocumentObserver_BindToDocument(nsIDocumentObserver *iface, but it is not set by default on native, and the behavior is still different. This was tested by removing all iexplore.exe values from any FeatureControl subkeys, and renaming the test executable to iexplore.exe, which changed its default compat mode in such cases. */ - if(This->window && This->window->base.outer_window && is_iexplore()) { + if(This->window && is_iexplore()) { HTMLOuterWindow *window = This->window->base.outer_window; DWORD zone; HRESULT hres; diff --git a/dlls/mshtml/omnavigator.c b/dlls/mshtml/omnavigator.c index db7da8276ef..465c7fb08ee 100644 --- a/dlls/mshtml/omnavigator.c +++ b/dlls/mshtml/omnavigator.c @@ -644,8 +644,7 @@ static HRESULT WINAPI OmHistory_get_length(IOmHistory *iface, short *p) TRACE("(%p)->(%p)\n", This, p); - if(This->window->base.outer_window) - browser = This->window->base.outer_window->browser; + browser = This->window->base.outer_window->browser; *p = browser && browser->doc->travel_log ? ITravelLog_CountEntries(browser->doc->travel_log, browser->doc->browser_service) diff --git a/dlls/mshtml/script.c b/dlls/mshtml/script.c index 86fffbe5656..9efdef2b4c0 100644 --- a/dlls/mshtml/script.c +++ b/dlls/mshtml/script.c @@ -518,7 +518,7 @@ static HRESULT WINAPI ActiveScriptSiteWindow_GetWindow(IActiveScriptSiteWindow * TRACE("(%p)->(%p)\n", This, phwnd); - if(!This->window || !This->window->base.outer_window) + if(!This->window) return E_UNEXPECTED; *phwnd = This->window->base.outer_window->browser->doc->hwnd; diff --git a/dlls/mshtml/xmlhttprequest.c b/dlls/mshtml/xmlhttprequest.c index 20049a13ee7..8fd0d9e1325 100644 --- a/dlls/mshtml/xmlhttprequest.c +++ b/dlls/mshtml/xmlhttprequest.c @@ -1686,9 +1686,6 @@ 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; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4312
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=139603 Your paranoid android. === debian11b (64 bit WoW report) === dinput: device8.c:2238: Test failed: 0x700: got key_state[0] 0
First of all, you're misinterpreting events.c results, we're incompatible with native in a different way and doc node reference to outer window just hides it, see: https://testbot.winehq.org/JobDetails.pl?Key=139652 CC is not a magic fix for everything. DOM part of reference graph is already very large: entire DOM tree needs to be kept alive as long as at least a single DOM node reference is alive and that includes all sub-frames. If anything, it would be interesting to see if we can break this graph, not extend it carelessly. For example, Gecko has non-owning references to the document from nodes, while we use CC reference. Instead, this MR merges reference graphs from all previous documents, meaning that any reference to a node that's no longer a part of DOM tree will keep alive all DOM trees, including their sub-frames and all DOM trees orphaned by navigation. It feels like a major step backwards. Navigation is a great opportunity to break references and allow subsequent collection, I think we really should use this opportunity. And note that what I describe is not just about making leaks "leak less". There are perfectly valid situations where references like you propose would actually cause leaks. Imagine the main documents runs a code like `document.myprop = external.createMyProp(iframe.contentWindow)` and `createMyProp` returns an object that holds a reference to the argument (`iframe.contentWindow`). If child window would store a strong reference to parent window, that would be a cycle and it wouldn't be collected even if entire document is orphaned by navigation. That said, there is definitely a room for improvements for inner/outer windows. I started refactoring this area a few years ago, but never had a chance to finish it. The huge part that prevents some of changes is binding code, which is rather ugly ATM. I imagine that it would be interesting to refactor it and, as a part of that, move the moniker, URL, etc. from outer window to the inner one. We could probably also avoid accessing outer window in a number of other places by storing Gecko inner window inside our inner window. That could fix a few tests that you mention. And yes, I think we'd want to try to get rid of outer window pointer from document nodes but it's tricky. The test that you quoted is not a proof that it's not possible, but if we still need them after all, using weak references would be much better. Anyway, I think that a strong parent reference just hides problems and proper fixes can easily become a time sink. Does it block your "proxy" patches in any way? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4312#note_51382
Actually, the example wasn't accurate, because the MR is about outer windows, not parent, but the point is the same. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4312#note_51383
On Wed Nov 8 14:51:03 2023 +0000, Jacek Caban wrote:
First of all, you're misinterpreting events.c results, we're incompatible with native in a different way and doc node reference to outer window just hides it, see: https://testbot.winehq.org/JobDetails.pl?Key=139652 CC is not a magic fix for everything. DOM part of reference graph is already very large: entire DOM tree needs to be kept alive as long as at least a single DOM node reference is alive and that includes all sub-frames. If anything, it would be interesting to see if we can break this graph, not extend it carelessly. For example, Gecko has non-owning references to the document from nodes, while we use CC reference. Instead, this MR merges reference graphs from all previous documents, meaning that any reference to a node that's no longer a part of DOM tree will keep alive all DOM trees, including their sub-frames and all DOM trees orphaned by navigation. It feels like a major step backwards. Navigation is a great opportunity to break references and allow subsequent collection, I think we really should use this opportunity. And note that what I describe is not just about making leaks "leak less". There are perfectly valid situations where references like you propose would actually cause leaks. Imagine the main documents runs a code like `document.myprop = external.createMyProp(iframe.contentWindow)` and `createMyProp` returns an object that holds a reference to the argument (`iframe.contentWindow`). If child window would store a strong reference to parent window, that would be a cycle and it wouldn't be collected even if entire document is orphaned by navigation. That said, there is definitely a room for improvements for inner/outer windows. I started refactoring this area a few years ago, but never had a chance to finish it. The huge part that prevents some of changes is binding code, which is rather ugly ATM. I imagine that it would be interesting to refactor it and, as a part of that, move the moniker, URL, etc. from outer window to the inner one. We could probably also avoid accessing outer window in a number of other places by storing Gecko inner window inside our inner window. That could fix a few tests that you mention. And yes, I think we'd want to try to get rid of outer window pointer from document nodes but it's tricky. The test that you quoted is not a proof that it's not possible, but if we still need them after all, using weak references would be much better. Anyway, I think that a strong parent reference just hides problems and proper fixes can easily become a time sink. Does it block your "proxy" patches in any way? I see, the test is kind of misleading, I can probably change it (even if we don't fix it in Wine) and add a todo_wine of course (since now it will block waiting for it expecting to load).
I don't think this necessarily blocks anything, it does simplify some things though, because we can uniformly access it from everywhere inner window is used, but if we move more of the outer window fields to the inner window, then it won't really matter. And I wanted to get rid of most of these revamps first before code freeze at least, it will make it easier to rebase then to stuff like Proton, especially if they need total overhauls based on reviews (as you can guess from this now). On second thought, what should I do about the document's get_parentWindow? It's not the same doc (unlike with iframe test), but it still returns the outer window. Should I keep a strong ref from the document to the outer window in doc->outer_window and use it there? Although it won't make much difference than holding it from inner windows, though. Alternatively, I could keep some sort of weak ref to the outer_window from the doc, but it would probably require it to be added to a list of detached docs on the outer window (so it knows to detach them when it gets killed). Is that better? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4312#note_51515
On second thought, what should I do about the document's get_parentWindow? It's not the same doc (unlike with iframe test), but it still returns the outer window.
It already returns outer window AFAICS, so you may just leave it alone. Depending on future of window object, we may forward it to `IHTMLWindow2::get_window` or something along those lines, but I don't see why you want to touch it in this context.
Should I keep a strong ref from the document to the outer window in doc->outer_window and use it there? Although it won't make much difference than holding it from inner windows, though.
No, that would just hide the problem. Ultimately, I think we will want to get rid of it and have only inner window reference in document node (which may be used to get outer window when really needed).
Alternatively, I could keep some sort of weak ref to the outer_window from the doc, but it would probably require it to be added to a list of detached docs on the outer window (so it knows to detach them when it gets killed). Is that better?
If we really need such reference, I think that a weak reference from inner to outer window would be better than document to outer (and yes, it would need a list of orphaned inner windows). But to decide that we really need it, a much deeper look at those problems is needed. So far it seems like it would be just hiding other problems, but I can't be sure. Even if we decide that it's needed, NULL checks would be still needed as well, so it defeats your motivation for the change. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4312#note_51617
On Wed Nov 8 23:24:59 2023 +0000, Jacek Caban wrote:
On second thought, what should I do about the document's get_parentWindow? It's not the same doc (unlike with iframe test), but it still returns the outer window. It already returns outer window AFAICS, so you may just leave it alone. Depending on future of window object, we may forward it to `IHTMLWindow2::get_window` or something along those lines, but I don't see why you want to touch it in this context. Should I keep a strong ref from the document to the outer window in doc->outer_window and use it there? Although it won't make much difference than holding it from inner windows, though. No, that would just hide the problem. Ultimately, I think we will want to get rid of it and have only inner window reference in document node (which may be used to get outer window when really needed). Alternatively, I could keep some sort of weak ref to the outer_window from the doc, but it would probably require it to be added to a list of detached docs on the outer window (so it knows to detach them when it gets killed). Is that better? If we really need such reference, I think that a weak reference from inner to outer window would be better than document to outer (and yes, it would need a list of orphaned inner windows). But to decide that we really need it, a much deeper look at those problems is needed. So far it seems like it would be just hiding other problems, but I can't be sure. Even if we decide that it's needed, NULL checks would be still needed as well, so it defeats your motivation for the change. So essentially I add a test that shows get_parentWindow returns the same outer window even after the document (and inner window) is detached, which means we'll need at least a weak ref either from the inner window or the document, or a strong ref but that's a bad idea as you said.
Storing the weak ref in the inner window sounds like a much better idea, thanks. Then I can get rid of `outer_window` from the doc again. I've been doing it with the document and need to clean it up a bit but seems to be mostly fine. Perhaps I can re-use the base window's outer_window for this? It's already a "weak" ref of sorts (since it can point to itself if it's the outer window), I can just delay NULLing it until it's actually killed, rather than detached. I'll see. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4312#note_51734
participants (3)
-
Gabriel Ivăncescu -
Jacek Caban (＠jacek)
-
Marvin