[PATCH v2 0/6] 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.
-- v2: mshtml: Get rid of outer_window member in HTMLDocumentNode. mshtml/tests: Improve the iframe navigation test. mshtml: Remove the inner window ref from the doc only when it is actually mshtml: Keep the inner window's outer_window pointer alive until it is mshtml: Don't NULL out the doc's window when unlinking the window. mshtml: Return outer window to external callers.
From: Gabriel Ivăncescu gabrielopcode@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@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);
From: Gabriel Ivăncescu gabrielopcode@gmail.com
This would cause a leak since we have a cyclic ref now.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/mshtml/htmlwindow.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index fc17635542d..11c7c6d892d 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -3969,7 +3969,6 @@ static void HTMLWindow_unlink(DispatchEx *dispex)
if(This->doc) { HTMLDocumentNode *doc = This->doc; - This->doc->window = NULL; This->doc = NULL; IHTMLDOMNode_Release(&doc->node.IHTMLDOMNode_iface); }
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/mshtml/htmlwindow.c | 25 ++++++++++++++++++++----- dlls/mshtml/mshtml_private.h | 3 +++ dlls/mshtml/tests/events.c | 5 ++--- 3 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index 11c7c6d892d..4ff5086cfb0 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -110,6 +110,10 @@ static void detach_inner_window(HTMLInnerWindow *window) HTMLOuterWindow *outer_window = window->base.outer_window; HTMLDocumentNode *doc = window->doc;
+ /* Check if already detached */ + if(!list_empty(&window->outer_window_entry)) + return; + while(!list_empty(&window->children)) { HTMLOuterWindow *child = LIST_ENTRY(list_tail(&window->children), HTMLOuterWindow, sibling_entry);
@@ -134,11 +138,13 @@ 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); + if(outer_window) { + list_add_tail(&outer_window->detached_inner_windows, &window->outer_window_entry); + if(outer_window->base.inner_window == window) { + outer_window->base.inner_window = NULL; + IHTMLWindow2_Release(&window->base.IHTMLWindow2_iface); + } } }
@@ -4016,6 +4022,7 @@ static void HTMLWindow_destructor(DispatchEx *dispex) HTMLInnerWindow *This = impl_from_DispatchEx(dispex); unsigned i;
+ list_remove(&This->outer_window_entry); VariantClear(&This->performance);
for(i = 0; i < This->global_prop_cnt; i++) @@ -4337,6 +4344,12 @@ static nsresult NSAPI outer_window_unlink(void *p) { HTMLOuterWindow *window = HTMLOuterWindow_from_IHTMLWindow2(p);
+ while(!list_empty(&window->detached_inner_windows)) { + HTMLInnerWindow *inner_window = LIST_ENTRY(list_head(&window->detached_inner_windows), HTMLInnerWindow, outer_window_entry); + list_remove(&inner_window->outer_window_entry); + list_init(&inner_window->outer_window_entry); + inner_window->base.outer_window = NULL; + } if(window->browser) { list_remove(&window->browser_entry); window->browser = NULL; @@ -4425,6 +4438,7 @@ static HRESULT create_inner_window(HTMLOuterWindow *outer_window, IMoniker *mon, list_init(&window->script_hosts); list_init(&window->bindings); list_init(&window->script_queue); + list_init(&window->outer_window_entry);
window->base.outer_window = outer_window; window->base.inner_window = window; @@ -4459,6 +4473,7 @@ HRESULT create_outer_window(GeckoBrowser *browser, mozIDOMWindowProxy *mozwindow window->base.inner_window = NULL; window->browser = browser; list_add_head(&browser->outer_windows, &window->browser_entry); + list_init(&window->detached_inner_windows); ccref_init(&window->ccref, 1);
mozIDOMWindowProxy_AddRef(mozwindow); @@ -4511,7 +4526,7 @@ 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; + list_add_tail(&outer_window->detached_inner_windows, &outer_window->pending_window->outer_window_entry); IHTMLWindow2_Release(&outer_window->pending_window->base.IHTMLWindow2_iface); }
diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index 23e1decc68b..d7f1d21a161 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -573,6 +573,7 @@ struct HTMLOuterWindow {
struct list sibling_entry; struct wine_rb_entry entry; + struct list detached_inner_windows; };
struct HTMLInnerWindow { @@ -633,6 +634,8 @@ struct HTMLInnerWindow { ULONGLONG load_event_start_time; ULONGLONG load_event_end_time; ULONGLONG first_paint_time; + + struct list outer_window_entry; };
typedef enum { diff --git a/dlls/mshtml/tests/events.c b/dlls/mshtml/tests/events.c index e5b9984e546..c11d53d611f 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); }
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Not on every detach.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/mshtml/htmldoc.c | 14 +++++++------- dlls/mshtml/tests/events.c | 5 +++++ 2 files changed, 12 insertions(+), 7 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/tests/events.c b/dlls/mshtml/tests/events.c index c11d53d611f..386b17d2b81 100644 --- a/dlls/mshtml/tests/events.c +++ b/dlls/mshtml/tests/events.c @@ -3576,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);
From: Gabriel Ivăncescu gabrielopcode@gmail.com
It was confusing before since it made it seem like it might use the outer window, while in fact the document is unchanged on native. Now the "new" doc is used for navigating, since it's already checked to be the same as the iframes_doc (but that test fails in wine and is todo_wine).
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/mshtml/tests/events.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/dlls/mshtml/tests/events.c b/dlls/mshtml/tests/events.c index 386b17d2b81..96217fbc045 100644 --- a/dlls/mshtml/tests/events.c +++ b/dlls/mshtml/tests/events.c @@ -173,16 +173,16 @@ static const char iframe_doc_str[] =
static void navigate(IHTMLDocument2*,const WCHAR*);
-static BOOL iface_cmp(IUnknown *iface1, IUnknown *iface2) +static BOOL iface_cmp(void *iface1, void *iface2) { IUnknown *unk1, *unk2;
if(iface1 == iface2) return TRUE;
- IUnknown_QueryInterface(iface1, &IID_IUnknown, (void**)&unk1); + IUnknown_QueryInterface((IUnknown *)iface1, &IID_IUnknown, (void**)&unk1); IUnknown_Release(unk1); - IUnknown_QueryInterface(iface2, &IID_IUnknown, (void**)&unk2); + IUnknown_QueryInterface((IUnknown *)iface2, &IID_IUnknown, (void**)&unk2); IUnknown_Release(unk2);
return unk1 == unk2; @@ -3170,8 +3170,8 @@ static IHTMLDocument2 *get_iframe_doc(IHTMLIFrameElement *iframe)
static void test_iframe_connections(IHTMLDocument2 *doc) { + IHTMLDocument2 *iframes_doc, *new_doc; IHTMLIFrameElement *iframe; - IHTMLDocument2 *iframes_doc; DWORD cookie; IConnectionPoint *cp; IHTMLElement *element; @@ -3185,7 +3185,6 @@ static void test_iframe_connections(IHTMLDocument2 *doc) IHTMLElement_Release(element);
iframes_doc = get_iframe_doc(iframe); - IHTMLIFrameElement_Release(iframe);
cookie = register_cp((IUnknown*)iframes_doc, &IID_IDispatch, (IUnknown*)&div_onclick_disp);
@@ -3215,23 +3214,38 @@ static void test_iframe_connections(IHTMLDocument2 *doc) ok(hres == S_OK, "put_URL failed: %08lx\n", hres); SysFreeString(str);
+ new_doc = get_iframe_doc(iframe); + ok(iface_cmp(new_doc, iframes_doc), "new_doc != iframes_doc\n"); + IHTMLDocument2_Release(new_doc); + SET_EXPECT(iframe_onload); pump_msgs(&called_iframe_onload); CHECK_CALLED(iframe_onload);
+ new_doc = get_iframe_doc(iframe); + todo_wine + ok(iface_cmp(new_doc, iframes_doc), "new_doc != iframes_doc\n"); + str = SysAllocString(L"about:test"); - hres = IHTMLDocument2_put_URL(iframes_doc, str); + hres = IHTMLDocument2_put_URL(new_doc, str); ok(hres == S_OK, "put_URL failed: %08lx\n", hres); + IHTMLDocument2_Release(new_doc); SysFreeString(str);
SET_EXPECT(iframe_onload); pump_msgs(&called_iframe_onload); CHECK_CALLED(iframe_onload); + + new_doc = get_iframe_doc(iframe); + todo_wine + ok(iface_cmp(new_doc, iframes_doc), "new_doc != iframes_doc\n"); + IHTMLDocument2_Release(new_doc); }else { win_skip("Skipping iframe onload tests on IE older than 9.\n"); }
IHTMLDocument2_Release(iframes_doc); + IHTMLIFrameElement_Release(iframe); }
static void test_window_refs(IHTMLDocument2 *doc)
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Since we keep the base's outer_window valid until it's unlinked now, we do not need it anymore. Furthermore, it was wrong because it didn't even hold any sort of ref to the outer window...
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/mshtml/editor.c | 8 +++---- dlls/mshtml/htmlanchor.c | 6 ++--- dlls/mshtml/htmldoc.c | 44 ++++++++++++++++++------------------ 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, 48 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..ecfd8b9daef 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 ? 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 || !This->window->base.outer_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 || !This->window->base.outer_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,8 @@ 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 && This->window->base.outer_window->url ? + This->window->base.outer_window->url : L"about:blank"); return *p ? S_OK : E_OUTOFMEMORY; }
@@ -1149,12 +1150,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 && !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 && This->window->base.outer_window->uri) { const PRUnichar *str; HRESULT hres;
@@ -1162,7 +1163,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 +1178,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 || !This->window->base.outer_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 +1198,13 @@ static HRESULT WINAPI HTMLDocument_get_cookie(IHTMLDocument2 *iface, BSTR *p)
TRACE("(%p)->(%p)\n", This, p);
- if(!This->outer_window) { + if(!This->window || !This->window->base.outer_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 +1220,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 +1449,7 @@ static HRESULT WINAPI HTMLDocument_open(IHTMLDocument2 *iface, BSTR url, VARIANT
*pomWindowResult = NULL;
- if(!This->outer_window) + if(!This->window || !This->window->base.outer_window) return E_FAIL;
if(!This->dom_document) { @@ -1475,8 +1476,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 +2350,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 && This->window->base.outer_window->readystate == READYSTATE_UNINITIALIZED) { *p = NULL; return S_OK; } @@ -4876,7 +4877,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 && This->window->base.outer_window) update_doc_cp_events(This, cp); }
@@ -6070,10 +6071,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 || !This->window->base.outer_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 +6188,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..28bf42e2569 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 || !This->element.node.doc->window->base.outer_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 d7f1d21a161..9bbd55bad78 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -942,7 +942,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..84c06f6fe0a 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 || !doc->window->base.outer_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 && doc->window->base.outer_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..43113e4f35b 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 || !doc->window->base.outer_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..5f9e3588239 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 || !This->doc->window->base.outer_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..ea53166f354 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 || !This->window->base.outer_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 || !This->window->base.outer_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);
Jacek Caban (@jacek) commented about dlls/mshtml/script.c:
return E_FAIL; /* FIXME: Return proxy object */
- *ppiunkItem = (IUnknown*)&This->window->base.IHTMLWindow2_iface;
- *ppiunkItem = (IUnknown*)&This->window->base.outer_window->base.IHTMLWindow2_iface;
This is missing `outer_window` NULL check.
The change keeping `outer_window` pointer alive for longer affects tens of NULL checks that we have in the code. Did you actually review the impact? It seems to me that a number of those checks are less correct after the change. I really think it needs closer look and probably some refactoring first so that the change has less impact. I'd suggest dropping all but first two patches for now.
On Fri Nov 10 15:33:38 2023 +0000, Jacek Caban wrote:
The change keeping `outer_window` pointer alive for longer affects tens of NULL checks that we have in the code. Did you actually review the impact? It seems to me that a number of those checks are less correct after the change. I really think it needs closer look and probably some refactoring first so that the change has less impact. I'd suggest dropping all but first two patches for now.
What kind of impact are you expecting? Of course, places where we didn't check for it, are still probably going to crash (there probably are some of them), but that can't be *worse* than before, since it will actually happen less than before.
Is there some code that actually needs to know when it has been detached? I'll try to review all of them but if you know one off the top of your head, it would be interesting to see why.
On Fri Nov 10 15:33:38 2023 +0000, Gabriel Ivăncescu wrote:
What kind of impact are you expecting? Of course, places where we didn't check for it, are still probably going to crash (there probably are some of them), but that can't be *worse* than before, since it will actually happen less than before. Is there some code that actually needs to know when it has been detached? I'll try to review all of them but if you know one off the top of your head, it would be interesting to see why.
For example `get_cookie` on a detached document would use URL from another document.
-
Gabriel Ivăncescu -
Gabriel Ivăncescu (＠insn)
-
Jacek Caban (＠jacek)