[PATCH 0/13] MR3597: mshtml: Implement Cycle Collection for all mshtml objects (dispex), part 5.
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmllocation.c | 50 +++++++++++++++++++++++++----------- dlls/mshtml/htmlwindow.c | 32 +++++++++++++++-------- dlls/mshtml/mshtml_private.h | 8 ++++-- 3 files changed, 62 insertions(+), 28 deletions(-) diff --git a/dlls/mshtml/htmllocation.c b/dlls/mshtml/htmllocation.c index 02f503969db..35446c57bd3 100644 --- a/dlls/mshtml/htmllocation.c +++ b/dlls/mshtml/htmllocation.c @@ -36,19 +36,14 @@ WINE_DEFAULT_DEBUG_CHANNEL(mshtml); -static inline HTMLOuterWindow *get_window(HTMLLocation *This) -{ - return CONTAINING_RECORD(This, HTMLOuterWindow, location); -} - static IUri *get_uri(HTMLLocation *This) { - return get_window(This)->uri; + return This->window->uri; } static HRESULT get_url_components(HTMLLocation *This, URL_COMPONENTSW *url) { - const WCHAR *doc_url = get_window(This)->url ? get_window(This)->url : L"about:blank"; + const WCHAR *doc_url = This->window->url ? This->window->url : L"about:blank"; if(!InternetCrackUrlW(doc_url, 0, 0, url)) { FIXME("InternetCrackUrlW failed: 0x%08lx\n", GetLastError()); @@ -93,13 +88,27 @@ static HRESULT WINAPI HTMLLocation_QueryInterface(IHTMLLocation *iface, REFIID r static ULONG WINAPI HTMLLocation_AddRef(IHTMLLocation *iface) { HTMLLocation *This = impl_from_IHTMLLocation(iface); - return IHTMLWindow2_AddRef(&get_window(This)->base.IHTMLWindow2_iface); + LONG ref = InterlockedIncrement(&This->ref); + + TRACE("(%p) ref=%ld\n", This, ref); + + return ref; } static ULONG WINAPI HTMLLocation_Release(IHTMLLocation *iface) { HTMLLocation *This = impl_from_IHTMLLocation(iface); - return IHTMLWindow2_Release(&get_window(This)->base.IHTMLWindow2_iface); + LONG ref = InterlockedDecrement(&This->ref); + + TRACE("(%p) ref=%ld\n", This, ref); + + if(!ref) { + IHTMLWindow2_Release(&This->window->base.IHTMLWindow2_iface); + release_dispex(&This->dispex); + free(This); + } + + return ref; } static HRESULT WINAPI HTMLLocation_GetTypeInfoCount(IHTMLLocation *iface, UINT *pctinfo) @@ -139,7 +148,7 @@ static HRESULT WINAPI HTMLLocation_put_href(IHTMLLocation *iface, BSTR v) TRACE("(%p)->(%s)\n", This, debugstr_w(v)); - return navigate_url(get_window(This), v, get_uri(This), BINDING_NAVIGATED); + return navigate_url(This->window, v, get_uri(This), BINDING_NAVIGATED); } static HRESULT WINAPI HTMLLocation_get_href(IHTMLLocation *iface, BSTR *p) @@ -500,7 +509,7 @@ static HRESULT WINAPI HTMLLocation_put_hash(IHTMLLocation *iface, BSTR v) memcpy(hash + 1, v, size - sizeof(WCHAR)); } - hres = navigate_url(get_window(This), hash, get_uri(This), BINDING_NAVIGATED); + hres = navigate_url(This->window, hash, get_uri(This), BINDING_NAVIGATED); if(hash != v) free(hash); @@ -544,12 +553,12 @@ static HRESULT WINAPI HTMLLocation_reload(IHTMLLocation *iface, VARIANT_BOOL fla TRACE("(%p)->(%x)\n", This, flag); /* reload is supposed to fail if called from a script with different origin, but IE doesn't care */ - if(!is_main_content_window(get_window(This))) { + if(!is_main_content_window(This->window)) { FIXME("Unsupported on iframe\n"); return E_NOTIMPL; } - return reload_page(get_window(This)); + return reload_page(This->window); } static HRESULT WINAPI HTMLLocation_replace(IHTMLLocation *iface, BSTR bstr) @@ -558,7 +567,7 @@ static HRESULT WINAPI HTMLLocation_replace(IHTMLLocation *iface, BSTR bstr) TRACE("(%p)->(%s)\n", This, debugstr_w(bstr)); - return navigate_url(get_window(This), bstr, get_uri(This), BINDING_NAVIGATED | BINDING_REPLACE); + return navigate_url(This->window, bstr, get_uri(This), BINDING_NAVIGATED | BINDING_REPLACE); } static HRESULT WINAPI HTMLLocation_assign(IHTMLLocation *iface, BSTR bstr) @@ -618,10 +627,21 @@ static dispex_static_data_t HTMLLocation_dispex = { HTMLLocation_iface_tids }; -void HTMLLocation_Init(HTMLLocation *location) +HRESULT create_location(HTMLOuterWindow *window, HTMLLocation **ret) { + HTMLLocation *location; + + if(!(location = calloc(1, sizeof(*location)))) + return E_OUTOFMEMORY; + location->IHTMLLocation_iface.lpVtbl = &HTMLLocationVtbl; + location->ref = 1; + location->window = window; + IHTMLWindow2_AddRef(&window->base.IHTMLWindow2_iface); init_dispatch(&location->dispex, (IUnknown*)&location->IHTMLLocation_iface, &HTMLLocation_dispex, COMPAT_MODE_QUIRKS); + + *ret = location; + return S_OK; } diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index c1a258651cf..7c40ca83188 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -62,13 +62,17 @@ HTMLOuterWindow *mozwindow_to_window(const mozIDOMWindowProxy *mozwindow) return entry ? WINE_RB_ENTRY_VALUE(entry, HTMLOuterWindow, entry) : NULL; } -static void get_location(HTMLOuterWindow *This, HTMLLocation **ret) +static HRESULT get_location(HTMLOuterWindow *This, HTMLLocation **ret) { - if(!This->location.dispex.outer) - HTMLLocation_Init(&This->location); + if(!This->location) { + HRESULT hres = create_location(This, &This->location); + if(FAILED(hres)) + return hres; + } - IHTMLLocation_AddRef(&This->location.IHTMLLocation_iface); - *ret = &This->location; + IHTMLLocation_AddRef(&This->location->IHTMLLocation_iface); + *ret = This->location; + return S_OK; } void get_top_window(HTMLOuterWindow *window, HTMLOuterWindow **ret) @@ -122,8 +126,8 @@ static void detach_inner_window(HTMLInnerWindow *window) if(doc) detach_document_node(doc); - if(outer_window && outer_window->location.dispex.outer) - dispex_props_unlink(&outer_window->location.dispex); + if(outer_window && outer_window->location) + dispex_props_unlink(&outer_window->location->dispex); abort_window_bindings(window); remove_target_tasks(window->task_magic); @@ -283,8 +287,8 @@ static void release_outer_window(HTMLOuterWindow *This) if(This->base.inner_window) detach_inner_window(This->base.inner_window); - if(This->location.dispex.outer) - release_dispex(&This->location.dispex); + if(This->location) + IHTMLLocation_Release(&This->location->IHTMLLocation_iface); if(This->frame_element) This->frame_element->content_window = NULL; @@ -779,10 +783,14 @@ static HRESULT WINAPI HTMLWindow2_get_location(IHTMLWindow2 *iface, IHTMLLocatio { HTMLWindow *This = impl_from_IHTMLWindow2(iface); HTMLLocation *location; + HRESULT hres; TRACE("(%p)->(%p)\n", This, p); - get_location(This->outer_window, &location); + hres = get_location(This->outer_window, &location); + if(FAILED(hres)) + return hres; + *p = &location->IHTMLLocation_iface; return S_OK; } @@ -4080,7 +4088,9 @@ static HRESULT IHTMLWindow2_location_hook(DispatchEx *dispex, WORD flags, DISPPA TRACE("forwarding to location.href\n"); - get_location(This->base.outer_window, &location); + hres = get_location(This->base.outer_window, &location); + if(FAILED(hres)) + return hres; hres = IDispatchEx_InvokeEx(&location->dispex.IDispatchEx_iface, DISPID_VALUE, 0, flags, dp, res, ei, caller); IHTMLLocation_Release(&location->IHTMLLocation_iface); diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index 63d07c872e3..de91aa39418 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -527,6 +527,10 @@ typedef struct { struct HTMLLocation { DispatchEx dispex; IHTMLLocation IHTMLLocation_iface; + + LONG ref; + + HTMLOuterWindow *window; }; typedef struct { @@ -607,7 +611,7 @@ struct HTMLOuterWindow { BOOL readystate_pending; HTMLInnerWindow *pending_window; - HTMLLocation location; + HTMLLocation *location; IMoniker *mon; IUri *uri; IUri *uri_nofrag; @@ -1024,7 +1028,7 @@ void get_top_window(HTMLOuterWindow*,HTMLOuterWindow**); HRESULT HTMLOptionElementFactory_Create(HTMLInnerWindow*,HTMLOptionElementFactory**); HRESULT HTMLImageElementFactory_Create(HTMLInnerWindow*,HTMLImageElementFactory**); HRESULT HTMLXMLHttpRequestFactory_Create(HTMLInnerWindow*,HTMLXMLHttpRequestFactory**); -void HTMLLocation_Init(HTMLLocation*); +HRESULT create_location(HTMLOuterWindow*,HTMLLocation**); HRESULT create_navigator(compat_mode_t,IOmNavigator**); HRESULT create_html_screen(compat_mode_t,IHTMLScreen**); HRESULT create_performance(HTMLInnerWindow*,IHTMLPerformance**); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3597
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmllocation.c | 49 ++++++++++++++++++++++++++++-------- dlls/mshtml/mshtml_private.h | 2 -- 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/dlls/mshtml/htmllocation.c b/dlls/mshtml/htmllocation.c index 35446c57bd3..5941c383db4 100644 --- a/dlls/mshtml/htmllocation.c +++ b/dlls/mshtml/htmllocation.c @@ -73,7 +73,7 @@ static HRESULT WINAPI HTMLLocation_QueryInterface(IHTMLLocation *iface, REFIID r *ppv = NULL; FIXME("(%p)->(IID_IMarshal %p)\n", This, ppv); return E_NOINTERFACE; - }else if(dispex_query_interface_no_cc(&This->dispex, riid, ppv)) { + }else if(dispex_query_interface(&This->dispex, riid, ppv)) { return *ppv ? S_OK : E_NOINTERFACE; }else { *ppv = NULL; @@ -88,7 +88,7 @@ static HRESULT WINAPI HTMLLocation_QueryInterface(IHTMLLocation *iface, REFIID r static ULONG WINAPI HTMLLocation_AddRef(IHTMLLocation *iface) { HTMLLocation *This = impl_from_IHTMLLocation(iface); - LONG ref = InterlockedIncrement(&This->ref); + LONG ref = dispex_ref_incr(&This->dispex); TRACE("(%p) ref=%ld\n", This, ref); @@ -98,16 +98,10 @@ static ULONG WINAPI HTMLLocation_AddRef(IHTMLLocation *iface) static ULONG WINAPI HTMLLocation_Release(IHTMLLocation *iface) { HTMLLocation *This = impl_from_IHTMLLocation(iface); - LONG ref = InterlockedDecrement(&This->ref); + LONG ref = dispex_ref_decr(&This->dispex); TRACE("(%p) ref=%ld\n", This, ref); - if(!ref) { - IHTMLWindow2_Release(&This->window->base.IHTMLWindow2_iface); - release_dispex(&This->dispex); - free(This); - } - return ref; } @@ -616,13 +610,47 @@ static const IHTMLLocationVtbl HTMLLocationVtbl = { HTMLLocation_toString }; +static inline HTMLLocation *impl_from_DispatchEx(DispatchEx *iface) +{ + return CONTAINING_RECORD(iface, HTMLLocation, dispex); +} + +static void HTMLLocation_traverse(DispatchEx *dispex, nsCycleCollectionTraversalCallback *cb) +{ + HTMLLocation *This = impl_from_DispatchEx(dispex); + if(This->window) + note_cc_edge((nsISupports*)&This->window->base.IHTMLWindow2_iface, "window", cb); +} + +static void HTMLLocation_unlink(DispatchEx *dispex) +{ + HTMLLocation *This = impl_from_DispatchEx(dispex); + if(This->window) { + HTMLOuterWindow *window = This->window; + This->window = NULL; + IHTMLWindow2_Release(&window->base.IHTMLWindow2_iface); + } +} + +static void HTMLLocation_destructor(DispatchEx *dispex) +{ + HTMLLocation *This = impl_from_DispatchEx(dispex); + free(This); +} + +static const dispex_static_data_vtbl_t HTMLLocation_dispex_vtbl = { + .destructor = HTMLLocation_destructor, + .traverse = HTMLLocation_traverse, + .unlink = HTMLLocation_unlink +}; + static const tid_t HTMLLocation_iface_tids[] = { IHTMLLocation_tid, 0 }; static dispex_static_data_t HTMLLocation_dispex = { "Location", - NULL, + &HTMLLocation_dispex_vtbl, DispHTMLLocation_tid, HTMLLocation_iface_tids }; @@ -635,7 +663,6 @@ HRESULT create_location(HTMLOuterWindow *window, HTMLLocation **ret) return E_OUTOFMEMORY; location->IHTMLLocation_iface.lpVtbl = &HTMLLocationVtbl; - location->ref = 1; location->window = window; IHTMLWindow2_AddRef(&window->base.IHTMLWindow2_iface); diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index de91aa39418..cade50cf9c7 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -528,8 +528,6 @@ struct HTMLLocation { DispatchEx dispex; IHTMLLocation IHTMLLocation_iface; - LONG ref; - HTMLOuterWindow *window; }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3597
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlwindow.c | 131 ++++++++++++++++++++++++----------- dlls/mshtml/mshtml_private.h | 2 + dlls/mshtml/nsembed.c | 1 + dlls/mshtml/persist.c | 3 +- 4 files changed, 95 insertions(+), 42 deletions(-) diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index 7c40ca83188..32c80fc7cf3 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -45,6 +45,8 @@ WINE_DEFAULT_DEBUG_CHANNEL(mshtml); +static ExternalCycleCollectionParticipant outer_window_ccp; + static int window_map_compare(const void *key, const struct wine_rb_entry *entry) { HTMLOuterWindow *window = WINE_RB_ENTRY_VALUE(entry, HTMLOuterWindow, entry); @@ -255,63 +257,34 @@ static HRESULT WINAPI outer_window_QueryInterface(IHTMLWindow2 *iface, REFIID ri if(hres != S_FALSE) return hres; - return EventTarget_QI_no_cc(&This->base.inner_window->event_target, riid, ppv); + if(IsEqualGUID(&IID_nsXPCOMCycleCollectionParticipant, riid)) { + *ppv = &outer_window_ccp; + return S_OK; + }else if(IsEqualGUID(&IID_nsCycleCollectionISupports, riid)) { + *ppv = &This->base.IHTMLWindow2_iface; + return S_OK; + }else { + return EventTarget_QI_no_cc(&This->base.inner_window->event_target, riid, ppv); + } } static ULONG WINAPI outer_window_AddRef(IHTMLWindow2 *iface) { HTMLOuterWindow *This = HTMLOuterWindow_from_IHTMLWindow2(iface); - LONG ref = InterlockedIncrement(&This->base.ref); + LONG ref = ccref_incr(&This->ccref, (nsISupports*)&This->base.IHTMLWindow2_iface); TRACE("(%p) ref=%ld\n", This, ref); return ref; } -static void release_outer_window(HTMLOuterWindow *This) -{ - if(This->browser) { - list_remove(&This->browser_entry); - This->browser = NULL; - } - - if(This->pending_window) { - abort_window_bindings(This->pending_window); - This->pending_window->base.outer_window = NULL; - IHTMLWindow2_Release(&This->pending_window->base.IHTMLWindow2_iface); - } - - remove_target_tasks(This->task_magic); - set_current_mon(This, NULL, 0); - set_current_uri(This, NULL); - if(This->base.inner_window) - detach_inner_window(This->base.inner_window); - - if(This->location) - IHTMLLocation_Release(&This->location->IHTMLLocation_iface); - - if(This->frame_element) - This->frame_element->content_window = NULL; - - if(This->nswindow) - nsIDOMWindow_Release(This->nswindow); - if(This->window_proxy) - mozIDOMWindowProxy_Release(This->window_proxy); - - wine_rb_remove(&window_map, &This->entry); - free(This); -} - static ULONG WINAPI outer_window_Release(IHTMLWindow2 *iface) { HTMLOuterWindow *This = HTMLOuterWindow_from_IHTMLWindow2(iface); - LONG ref = InterlockedDecrement(&This->base.ref); + LONG ref = ccref_decr(&This->ccref, (nsISupports*)&This->base.IHTMLWindow2_iface, &outer_window_ccp); TRACE("(%p) ref=%ld\n", This, ref); - if(!ref) - release_outer_window(This); - return ref; } @@ -4198,6 +4171,81 @@ static dispex_static_data_t HTMLWindow_dispex = { HTMLWindow_init_dispex_info }; +static nsresult NSAPI outer_window_traverse(void *ccp, void *p, nsCycleCollectionTraversalCallback *cb) +{ + HTMLOuterWindow *window = HTMLOuterWindow_from_IHTMLWindow2(p); + + describe_cc_node(&window->ccref, "OuterWindow", cb); + + if(window->pending_window) + note_cc_edge((nsISupports*)&window->pending_window->base.IHTMLWindow2_iface, "pending_window", cb); + if(window->base.inner_window) + note_cc_edge((nsISupports*)&window->base.inner_window->base.IHTMLWindow2_iface, "inner_window", cb); + if(window->location) + note_cc_edge((nsISupports*)&window->location->IHTMLLocation_iface, "location", cb); + if(window->nswindow) + note_cc_edge((nsISupports*)window->nswindow, "nswindow", cb); + if(window->window_proxy) + note_cc_edge((nsISupports*)window->window_proxy, "window_proxy", cb); + return NS_OK; +} + +static nsresult NSAPI outer_window_unlink(void *p) +{ + HTMLOuterWindow *window = HTMLOuterWindow_from_IHTMLWindow2(p); + + remove_target_tasks(window->task_magic); + + if(window->browser) { + list_remove(&window->browser_entry); + window->browser = NULL; + } + 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; + 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->location) { + HTMLLocation *location = window->location; + window->location = NULL; + IHTMLLocation_Release(&location->IHTMLLocation_iface); + } + if(window->frame_element) { + window->frame_element->content_window = NULL; + window->frame_element = NULL; + } + unlink_ref(&window->nswindow); + if(window->window_proxy) { + unlink_ref(&window->window_proxy); + wine_rb_remove(&window_map, &window->entry); + } + return NS_OK; +} + +static void NSAPI outer_window_delete_cycle_collectable(void *p) +{ + HTMLOuterWindow *window = HTMLOuterWindow_from_IHTMLWindow2(p); + outer_window_unlink(p); + free(window); +} + +void init_window_cc(void) +{ + static const CCObjCallback ccp_callback = { + outer_window_traverse, + outer_window_unlink, + outer_window_delete_cycle_collectable + }; + ccp_init(&outer_window_ccp, &ccp_callback); +} + static void *alloc_window(size_t size) { HTMLWindow *window; @@ -4219,7 +4267,6 @@ static void *alloc_window(size_t size) window->IProvideMultipleClassInfo_iface.lpVtbl = &ProvideMultipleClassInfoVtbl; window->IWineHTMLWindowPrivate_iface.lpVtbl = &WineHTMLWindowPrivateVtbl; window->IWineHTMLWindowCompatPrivate_iface.lpVtbl = &WineHTMLWindowCompatPrivateVtbl; - window->ref = 1; return window; } @@ -4248,6 +4295,7 @@ static HRESULT create_inner_window(HTMLOuterWindow *outer_window, IMoniker *mon, window->base.outer_window = outer_window; window->base.inner_window = window; + window->base.ref = 1; EventTarget_Init(&window->event_target, (IUnknown*)&window->base.IHTMLWindow2_iface, &HTMLWindow_dispex, COMPAT_MODE_NONE); @@ -4278,6 +4326,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); + ccref_init(&window->ccref, 1); mozIDOMWindowProxy_AddRef(mozwindow); window->window_proxy = mozwindow; diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index cade50cf9c7..fd6192f4127 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -594,6 +594,7 @@ struct HTMLWindow { struct HTMLOuterWindow { HTMLWindow base; + nsCycleCollectingAutoRefCnt ccref; LONG task_magic; nsIDOMWindow *nswindow; @@ -1089,6 +1090,7 @@ void set_viewer_zoom(GeckoBrowser*,float); float get_viewer_zoom(GeckoBrowser*); void init_dispex_cc(void); +void init_window_cc(void); HRESULT nsuri_to_url(LPCWSTR,BOOL,BSTR*); diff --git a/dlls/mshtml/nsembed.c b/dlls/mshtml/nsembed.c index d2053235c03..1b31e4f7159 100644 --- a/dlls/mshtml/nsembed.c +++ b/dlls/mshtml/nsembed.c @@ -597,6 +597,7 @@ static BOOL init_xpcom(const PRUnichar *gre_path) } init_dispex_cc(); + init_window_cc(); return TRUE; } diff --git a/dlls/mshtml/persist.c b/dlls/mshtml/persist.c index bea2ab65750..dd79d85a1c5 100644 --- a/dlls/mshtml/persist.c +++ b/dlls/mshtml/persist.c @@ -466,7 +466,8 @@ static void notif_readystate(HTMLOuterWindow *window) static void notif_readystate_proc(event_task_t *task) { - notif_readystate(task->window->base.outer_window); + if(task->window->base.outer_window) + notif_readystate(task->window->base.outer_window); } static void notif_readystate_destr(event_task_t *task) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3597
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/omnavigator.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/dlls/mshtml/omnavigator.c b/dlls/mshtml/omnavigator.c index 646017aee9e..ec531bbf6b7 100644 --- a/dlls/mshtml/omnavigator.c +++ b/dlls/mshtml/omnavigator.c @@ -1631,7 +1631,8 @@ static ULONG WINAPI HTMLPerformanceTiming_Release(IHTMLPerformanceTiming *iface) if(!ref) { if(This->dispex.outer) release_dispex(&This->dispex); - free(This); + else + free(This); } return ref; @@ -1956,13 +1957,28 @@ static const IHTMLPerformanceTimingVtbl HTMLPerformanceTimingVtbl = { HTMLPerformanceTiming_toJSON }; +static inline HTMLPerformanceTiming *HTMLPerformanceTiming_from_DispatchEx(DispatchEx *iface) +{ + return CONTAINING_RECORD(iface, HTMLPerformanceTiming, dispex); +} + +static void HTMLPerformanceTiming_destructor(DispatchEx *dispex) +{ + HTMLPerformanceTiming *This = HTMLPerformanceTiming_from_DispatchEx(dispex); + free(This); +} + +static const dispex_static_data_vtbl_t HTMLPerformanceTiming_dispex_vtbl = { + .destructor = HTMLPerformanceTiming_destructor, +}; + static const tid_t HTMLPerformanceTiming_iface_tids[] = { IHTMLPerformanceTiming_tid, 0 }; static dispex_static_data_t HTMLPerformanceTiming_dispex = { "PerformanceTiming", - NULL, + &HTMLPerformanceTiming_dispex_vtbl, IHTMLPerformanceTiming_tid, HTMLPerformanceTiming_iface_tids }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3597
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/dispex.c | 23 +++++++++++------------ dlls/mshtml/htmlevent.c | 10 +++++----- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c index 237c61e89bf..e32840c47e3 100644 --- a/dlls/mshtml/dispex.c +++ b/dlls/mshtml/dispex.c @@ -640,7 +640,7 @@ static inline dispex_dynamic_data_t *get_dynamic_data(DispatchEx *This) if(!This->dynamic_data) return NULL; - if(This->info->desc->vtbl && This->info->desc->vtbl->populate_props) + if(This->info->desc->vtbl->populate_props) This->info->desc->vtbl->populate_props(This); return This->dynamic_data; @@ -737,7 +737,7 @@ static HRESULT dispex_value(DispatchEx *This, LCID lcid, WORD flags, DISPPARAMS { HRESULT hres; - if(This->info->desc->vtbl && This->info->desc->vtbl->value) + if(This->info->desc->vtbl->value) return This->info->desc->vtbl->value(This, lcid, flags, params, res, ei, caller); switch(flags) { @@ -1059,7 +1059,7 @@ static HRESULT get_builtin_id(DispatchEx *This, BSTR name, DWORD grfdex, DISPID min = n+1; } - if(This->info->desc->vtbl && This->info->desc->vtbl->get_dispid) { + if(This->info->desc->vtbl->get_dispid) { HRESULT hres; hres = This->info->desc->vtbl->get_dispid(This, name, grfdex, ret); @@ -1711,7 +1711,7 @@ static HRESULT WINAPI DispatchEx_InvokeEx(IDispatchEx *iface, DISPID id, LCID lc switch(get_dispid_type(id)) { case DISPEXPROP_CUSTOM: - if(!This->info->desc->vtbl || !This->info->desc->vtbl->invoke) + if(!This->info->desc->vtbl->invoke) return DISP_E_MEMBERNOTFOUND; return This->info->desc->vtbl->invoke(This, id, lcid, wFlags, pdp, pvarRes, pei, pspCaller); @@ -1764,7 +1764,7 @@ static HRESULT WINAPI DispatchEx_InvokeEx(IDispatchEx *iface, DISPID id, LCID lc case DISPEXPROP_BUILTIN: if(wFlags == DISPATCH_CONSTRUCT) { if(id == DISPID_VALUE) { - if(This->info->desc->vtbl && This->info->desc->vtbl->value) { + if(This->info->desc->vtbl->value) { return This->info->desc->vtbl->value(This, lcid, wFlags, pdp, pvarRes, pei, pspCaller); } FIXME("DISPATCH_CONSTRUCT flag but missing value function\n"); @@ -1806,7 +1806,7 @@ static HRESULT WINAPI DispatchEx_DeleteMemberByDispID(IDispatchEx *iface, DISPID TRACE("(%p)->(%lx)\n", This, id); - if(is_custom_dispid(id) && This->info->desc->vtbl && This->info->desc->vtbl->delete) + if(is_custom_dispid(id) && This->info->desc->vtbl->delete) return This->info->desc->vtbl->delete(This, id); if(dispex_compat_mode(This) < COMPAT_MODE_IE8) { @@ -1849,7 +1849,7 @@ static HRESULT WINAPI DispatchEx_GetMemberName(IDispatchEx *iface, DISPID id, BS return E_OUTOFMEMORY; if(is_custom_dispid(id)) { - if(This->info->desc->vtbl && This->info->desc->vtbl->get_name) + if(This->info->desc->vtbl->get_name) return This->info->desc->vtbl->get_name(This, id, pbstrName); return DISP_E_MEMBERNOTFOUND; } @@ -1933,7 +1933,7 @@ static HRESULT WINAPI DispatchEx_GetNextDispID(IDispatchEx *iface, DWORD grfdex, id = DISPID_STARTENUM; } - if(This->info->desc->vtbl && This->info->desc->vtbl->next_dispid) { + if(This->info->desc->vtbl->next_dispid) { hres = This->info->desc->vtbl->next_dispid(This, id, pid); if(hres != S_FALSE) return hres; @@ -2115,7 +2115,7 @@ void release_dispex(DispatchEx *This) { dynamic_prop_t *prop; - if(This->info->desc->vtbl && This->info->desc->vtbl->unlink) + if(This->info->desc->vtbl->unlink) This->info->desc->vtbl->unlink(This); if(!This->dynamic_data) @@ -2145,8 +2145,7 @@ void release_dispex(DispatchEx *This) free(This->dynamic_data); destructor: - if(This->info->desc->vtbl) - This->info->desc->vtbl->destructor(This); + This->info->desc->vtbl->destructor(This); } void init_dispatch(DispatchEx *dispex, IUnknown *outer, dispex_static_data_t *data, compat_mode_t compat_mode) @@ -2158,7 +2157,7 @@ void init_dispatch(DispatchEx *dispex, IUnknown *outer, dispex_static_data_t *da dispex->dynamic_data = NULL; ccref_init(&dispex->ccref, 1); - if(data->vtbl && data->vtbl->get_compat_mode) { + if(data->vtbl->get_compat_mode) { /* delayed init */ if(!data->delayed_init_info) { EnterCriticalSection(&cs_dispex_static_data); diff --git a/dlls/mshtml/htmlevent.c b/dlls/mshtml/htmlevent.c index 9f3dbae81d7..e4f299d7986 100644 --- a/dlls/mshtml/htmlevent.c +++ b/dlls/mshtml/htmlevent.c @@ -3669,7 +3669,7 @@ static void call_event_handlers(EventTarget *event_target, DOMEvent *event, disp free(listeners); if(event->phase != DEP_CAPTURING_PHASE && event_info[event->event_id].dispid - && (vtbl = dispex_get_vtbl(&event_target->dispex)) && vtbl->get_cp_container) + && (vtbl = dispex_get_vtbl(&event_target->dispex))->get_cp_container) cp_container = vtbl->get_cp_container(&event_target->dispex); if(cp_container) { if(cp_container->cps) { @@ -3762,7 +3762,7 @@ static HRESULT dispatch_event_object(EventTarget *event_target, DOMEvent *event, target_chain[chain_cnt++] = iter; - if(!(vtbl = dispex_get_vtbl(&iter->dispex)) || !vtbl->get_parent_event_target) + if(!(vtbl = dispex_get_vtbl(&iter->dispex))->get_parent_event_target) break; iter = vtbl->get_parent_event_target(&iter->dispex); } while(iter); @@ -3774,7 +3774,7 @@ static HRESULT dispatch_event_object(EventTarget *event_target, DOMEvent *event, } target_vtbl = dispex_get_vtbl(&event_target->dispex); - if(target_vtbl && target_vtbl->set_current_event) + if(target_vtbl->set_current_event) prev_event = target_vtbl->set_current_event(&event_target->dispex, event->event_obj); if(event->target) @@ -3801,7 +3801,7 @@ static HRESULT dispatch_event_object(EventTarget *event_target, DOMEvent *event, if(r) *r = variant_bool(!event->prevent_default); - if(target_vtbl && target_vtbl->set_current_event) { + if(target_vtbl->set_current_event) { IHTMLEventObj *prev = target_vtbl->set_current_event(&event_target->dispex, prev_event); if(prev) IHTMLEventObj_Release(prev); @@ -3814,7 +3814,7 @@ static HRESULT dispatch_event_object(EventTarget *event_target, DOMEvent *event, BOOL prevent_default = event->prevent_default; for(i = 0; !prevent_default && i < chain_cnt; i++) { vtbl = dispex_get_vtbl(&target_chain[i]->dispex); - if(!vtbl || !vtbl->handle_event_default) + if(!vtbl->handle_event_default) continue; hres = vtbl->handle_event_default(&event_target->dispex, event->event_id, event->nsevent, &prevent_default); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3597
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/omnavigator.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/dlls/mshtml/omnavigator.c b/dlls/mshtml/omnavigator.c index ec531bbf6b7..e76cfac0e5f 100644 --- a/dlls/mshtml/omnavigator.c +++ b/dlls/mshtml/omnavigator.c @@ -2183,8 +2183,6 @@ typedef struct { DispatchEx dispex; IHTMLPerformance IHTMLPerformance_iface; - LONG ref; - IHTMLPerformanceNavigation *navigation; HTMLPerformanceTiming *timing; } HTMLPerformance; @@ -2204,7 +2202,7 @@ static HRESULT WINAPI HTMLPerformance_QueryInterface(IHTMLPerformance *iface, RE *ppv = &This->IHTMLPerformance_iface; }else if(IsEqualGUID(&IID_IHTMLPerformance, riid)) { *ppv = &This->IHTMLPerformance_iface; - }else if(dispex_query_interface_no_cc(&This->dispex, riid, ppv)) { + }else if(dispex_query_interface(&This->dispex, riid, ppv)) { return *ppv ? S_OK : E_NOINTERFACE; }else { WARN("Unsupported interface %s\n", debugstr_mshtml_guid(riid)); @@ -2219,7 +2217,7 @@ static HRESULT WINAPI HTMLPerformance_QueryInterface(IHTMLPerformance *iface, RE static ULONG WINAPI HTMLPerformance_AddRef(IHTMLPerformance *iface) { HTMLPerformance *This = impl_from_IHTMLPerformance(iface); - LONG ref = InterlockedIncrement(&This->ref); + LONG ref = dispex_ref_incr(&This->dispex); TRACE("(%p) ref=%ld\n", This, ref); @@ -2229,13 +2227,10 @@ static ULONG WINAPI HTMLPerformance_AddRef(IHTMLPerformance *iface) static ULONG WINAPI HTMLPerformance_Release(IHTMLPerformance *iface) { HTMLPerformance *This = impl_from_IHTMLPerformance(iface); - LONG ref = InterlockedDecrement(&This->ref); + LONG ref = dispex_ref_decr(&This->dispex); TRACE("(%p) ref=%ld\n", This, ref); - if(!ref) - release_dispex(&This->dispex); - return ref; } @@ -2348,6 +2343,15 @@ static inline HTMLPerformance *HTMLPerformance_from_DispatchEx(DispatchEx *iface return CONTAINING_RECORD(iface, HTMLPerformance, dispex); } +static void HTMLPerformance_traverse(DispatchEx *dispex, nsCycleCollectionTraversalCallback *cb) +{ + HTMLPerformance *This = HTMLPerformance_from_DispatchEx(dispex); + if(This->navigation) + note_cc_edge((nsISupports*)This->navigation, "navigation", cb); + if(This->timing) + note_cc_edge((nsISupports*)&This->timing->IHTMLPerformanceTiming_iface, "timing", cb); +} + static void HTMLPerformance_unlink(DispatchEx *dispex) { HTMLPerformance *This = HTMLPerformance_from_DispatchEx(dispex); @@ -2367,6 +2371,7 @@ static void HTMLPerformance_destructor(DispatchEx *dispex) static const dispex_static_data_vtbl_t HTMLPerformance_dispex_vtbl = { .destructor = HTMLPerformance_destructor, + .traverse = HTMLPerformance_traverse, .unlink = HTMLPerformance_unlink }; @@ -2391,7 +2396,6 @@ HRESULT create_performance(HTMLInnerWindow *window, IHTMLPerformance **ret) return E_OUTOFMEMORY; performance->IHTMLPerformance_iface.lpVtbl = &HTMLPerformanceVtbl; - performance->ref = 1; init_dispatch(&performance->dispex, (IUnknown*)&performance->IHTMLPerformance_iface, &HTMLPerformance_dispex, compat_mode); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3597
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/omnavigator.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/dlls/mshtml/omnavigator.c b/dlls/mshtml/omnavigator.c index e76cfac0e5f..d8053328380 100644 --- a/dlls/mshtml/omnavigator.c +++ b/dlls/mshtml/omnavigator.c @@ -2003,7 +2003,6 @@ typedef struct { DispatchEx dispex; IHTMLPerformanceNavigation IHTMLPerformanceNavigation_iface; - LONG ref; HTMLPerformanceTiming *timing; } HTMLPerformanceNavigation; @@ -2022,7 +2021,7 @@ static HRESULT WINAPI HTMLPerformanceNavigation_QueryInterface(IHTMLPerformanceN *ppv = &This->IHTMLPerformanceNavigation_iface; }else if(IsEqualGUID(&IID_IHTMLPerformanceNavigation, riid)) { *ppv = &This->IHTMLPerformanceNavigation_iface; - }else if(dispex_query_interface_no_cc(&This->dispex, riid, ppv)) { + }else if(dispex_query_interface(&This->dispex, riid, ppv)) { return *ppv ? S_OK : E_NOINTERFACE; }else { WARN("Unsupported interface %s\n", debugstr_mshtml_guid(riid)); @@ -2037,7 +2036,7 @@ static HRESULT WINAPI HTMLPerformanceNavigation_QueryInterface(IHTMLPerformanceN static ULONG WINAPI HTMLPerformanceNavigation_AddRef(IHTMLPerformanceNavigation *iface) { HTMLPerformanceNavigation *This = impl_from_IHTMLPerformanceNavigation(iface); - LONG ref = InterlockedIncrement(&This->ref); + LONG ref = dispex_ref_incr(&This->dispex); TRACE("(%p) ref=%ld\n", This, ref); @@ -2047,13 +2046,10 @@ static ULONG WINAPI HTMLPerformanceNavigation_AddRef(IHTMLPerformanceNavigation static ULONG WINAPI HTMLPerformanceNavigation_Release(IHTMLPerformanceNavigation *iface) { HTMLPerformanceNavigation *This = impl_from_IHTMLPerformanceNavigation(iface); - LONG ref = InterlockedDecrement(&This->ref); + LONG ref = dispex_ref_decr(&This->dispex); TRACE("(%p) ref=%ld\n", This, ref); - if(!ref) - release_dispex(&This->dispex); - return ref; } @@ -2147,6 +2143,13 @@ static inline HTMLPerformanceNavigation *HTMLPerformanceNavigation_from_Dispatch return CONTAINING_RECORD(iface, HTMLPerformanceNavigation, dispex); } +static void HTMLPerformanceNavigation_traverse(DispatchEx *dispex, nsCycleCollectionTraversalCallback *cb) +{ + HTMLPerformanceNavigation *This = HTMLPerformanceNavigation_from_DispatchEx(dispex); + if(This->timing) + note_cc_edge((nsISupports*)&This->timing->IHTMLPerformanceTiming_iface, "timing", cb); +} + static void HTMLPerformanceNavigation_unlink(DispatchEx *dispex) { HTMLPerformanceNavigation *This = HTMLPerformanceNavigation_from_DispatchEx(dispex); @@ -2165,6 +2168,7 @@ static void HTMLPerformanceNavigation_destructor(DispatchEx *dispex) static const dispex_static_data_vtbl_t HTMLPerformanceNavigation_dispex_vtbl = { .destructor = HTMLPerformanceNavigation_destructor, + .traverse = HTMLPerformanceNavigation_traverse, .unlink = HTMLPerformanceNavigation_unlink }; @@ -2284,7 +2288,6 @@ static HRESULT WINAPI HTMLPerformance_get_navigation(IHTMLPerformance *iface, return E_OUTOFMEMORY; navigation->IHTMLPerformanceNavigation_iface.lpVtbl = &HTMLPerformanceNavigationVtbl; - navigation->ref = 1; navigation->timing = This->timing; IHTMLPerformanceTiming_AddRef(&This->timing->IHTMLPerformanceTiming_iface); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3597
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/dispex.c | 9 +++------ dlls/mshtml/mshtml_private.h | 6 ++++++ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c index e32840c47e3..b0c54706ff8 100644 --- a/dlls/mshtml/dispex.c +++ b/dlls/mshtml/dispex.c @@ -2031,10 +2031,8 @@ static nsresult NSAPI dispex_traverse(void *ccp, void *p, nsCycleCollectionTrave if(!This->dynamic_data) return NS_OK; - for(prop = This->dynamic_data->props; prop < This->dynamic_data->props + This->dynamic_data->prop_cnt; prop++) { - if(V_VT(&prop->var) == VT_DISPATCH) - note_cc_edge((nsISupports*)V_DISPATCH(&prop->var), "dispex_data", cb); - } + for(prop = This->dynamic_data->props; prop < This->dynamic_data->props + This->dynamic_data->prop_cnt; prop++) + traverse_variant(&prop->var, "dispex_data", cb); if(This->dynamic_data->func_disps) { func_obj_entry_t *iter = This->dynamic_data->func_disps, *end = iter + This->info->func_disp_cnt; @@ -2043,8 +2041,7 @@ static nsresult NSAPI dispex_traverse(void *ccp, void *p, nsCycleCollectionTrave if(!iter->func_obj) continue; note_cc_edge((nsISupports*)&iter->func_obj->dispex.IDispatchEx_iface, "func_obj", cb); - if(V_VT(&iter->val) == VT_DISPATCH) - note_cc_edge((nsISupports*)V_DISPATCH(&iter->val), "func_val", cb); + traverse_variant(&iter->val, "func_val", cb); } } diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index fd6192f4127..094c58a2919 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -1521,6 +1521,12 @@ static inline void unlink_variant(VARIANT *v) unlink_ref(&V_UNKNOWN(v)); } +static inline void traverse_variant(VARIANT *v, const char *name, nsCycleCollectionTraversalCallback *cb) +{ + if(V_VT(v) == VT_DISPATCH || V_VT(v) == VT_UNKNOWN) + note_cc_edge((nsISupports*)V_UNKNOWN(v), "dispex_data", cb); +} + #ifdef __i386__ extern void *call_thiscall_func; #endif -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3597
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlwindow.c | 45 +++++++++++++++++++++++++++++------- dlls/mshtml/mshtml_private.h | 2 -- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index 32c80fc7cf3..4040473bd6e 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -220,13 +220,13 @@ static HRESULT WINAPI HTMLWindow2_QueryInterface(IHTMLWindow2 *iface, REFIID rii if(hres != S_FALSE) return hres; - return EventTarget_QI_no_cc(&This->event_target, riid, ppv); + return EventTarget_QI(&This->event_target, riid, ppv); } static ULONG WINAPI HTMLWindow2_AddRef(IHTMLWindow2 *iface) { HTMLInnerWindow *This = HTMLInnerWindow_from_IHTMLWindow2(iface); - LONG ref = InterlockedIncrement(&This->base.ref); + LONG ref = dispex_ref_incr(&This->event_target.dispex); TRACE("(%p) ref=%ld\n", This, ref); @@ -236,13 +236,10 @@ static ULONG WINAPI HTMLWindow2_AddRef(IHTMLWindow2 *iface) static ULONG WINAPI HTMLWindow2_Release(IHTMLWindow2 *iface) { HTMLInnerWindow *This = HTMLInnerWindow_from_IHTMLWindow2(iface); - LONG ref = InterlockedDecrement(&This->base.ref); + LONG ref = dispex_ref_decr(&This->event_target.dispex); TRACE("(%p) ref=%ld\n", This, ref); - if(!ref) - release_dispex(&This->event_target.dispex); - return ref; } @@ -264,7 +261,7 @@ static HRESULT WINAPI outer_window_QueryInterface(IHTMLWindow2 *iface, REFIID ri *ppv = &This->base.IHTMLWindow2_iface; return S_OK; }else { - return EventTarget_QI_no_cc(&This->base.inner_window->event_target, riid, ppv); + return EventTarget_QI(&This->base.inner_window->event_target, riid, ppv); } } @@ -3823,6 +3820,38 @@ static inline HTMLInnerWindow *impl_from_DispatchEx(DispatchEx *iface) return CONTAINING_RECORD(iface, HTMLInnerWindow, event_target.dispex); } +static void HTMLWindow_traverse(DispatchEx *dispex, nsCycleCollectionTraversalCallback *cb) +{ + HTMLInnerWindow *This = impl_from_DispatchEx(dispex); + HTMLOuterWindow *child; + + LIST_FOR_EACH_ENTRY(child, &This->children, HTMLOuterWindow, sibling_entry) + note_cc_edge((nsISupports*)&child->base.IHTMLWindow2_iface, "child", cb); + if(This->doc) + note_cc_edge((nsISupports*)&This->doc->node.IHTMLDOMNode_iface, "doc", cb); + if(This->console) + note_cc_edge((nsISupports*)This->console, "console", cb); + if(This->image_factory) + note_cc_edge((nsISupports*)&This->image_factory->IHTMLImageElementFactory_iface, "image_factory", cb); + if(This->option_factory) + note_cc_edge((nsISupports*)&This->option_factory->IHTMLOptionElementFactory_iface, "option_factory", cb); + if(This->xhr_factory) + note_cc_edge((nsISupports*)&This->xhr_factory->IHTMLXMLHttpRequestFactory_iface, "xhr_factory", cb); + if(This->mutation_observer_ctor) + note_cc_edge((nsISupports*)This->mutation_observer_ctor, "mutation_observer_ctor", cb); + if(This->screen) + note_cc_edge((nsISupports*)This->screen, "screen", cb); + if(This->history) + note_cc_edge((nsISupports*)&This->history->IOmHistory_iface, "history", cb); + if(This->navigator) + note_cc_edge((nsISupports*)This->navigator, "navigator", cb); + if(This->session_storage) + note_cc_edge((nsISupports*)This->session_storage, "session_storage", cb); + if(This->local_storage) + note_cc_edge((nsISupports*)This->local_storage, "local_storage", cb); + traverse_variant(&This->performance, "performance", cb); +} + static void HTMLWindow_unlink(DispatchEx *dispex) { HTMLInnerWindow *This = impl_from_DispatchEx(dispex); @@ -4147,6 +4176,7 @@ static IHTMLEventObj *HTMLWindow_set_current_event(DispatchEx *dispex, IHTMLEven static const event_target_vtbl_t HTMLWindow_event_target_vtbl = { { .destructor = HTMLWindow_destructor, + .traverse = HTMLWindow_traverse, .unlink = HTMLWindow_unlink, .get_name = HTMLWindow_get_name, .invoke = HTMLWindow_invoke, @@ -4295,7 +4325,6 @@ static HRESULT create_inner_window(HTMLOuterWindow *outer_window, IMoniker *mon, window->base.outer_window = outer_window; window->base.inner_window = window; - window->base.ref = 1; EventTarget_Init(&window->event_target, (IUnknown*)&window->base.IHTMLWindow2_iface, &HTMLWindow_dispex, COMPAT_MODE_NONE); diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index 094c58a2919..0a0d33b441c 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -585,8 +585,6 @@ struct HTMLWindow { IWineHTMLWindowPrivate IWineHTMLWindowPrivate_iface; IWineHTMLWindowCompatPrivate IWineHTMLWindowCompatPrivate_iface; - LONG ref; - HTMLInnerWindow *inner_window; HTMLOuterWindow *outer_window; }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3597
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/mshtml_private.h | 6 +++--- dlls/mshtml/navigate.c | 6 +++--- dlls/mshtml/omnavigator.c | 30 +++++++++++++++++++----------- 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index 0a0d33b441c..703eeae17ab 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -546,9 +546,6 @@ typedef struct { LONG ref; - ULONG navigation_type; - ULONG redirect_count; - ULONGLONG navigation_start_time; ULONGLONG unload_event_start_time; ULONGLONG unload_event_end_time; @@ -658,6 +655,9 @@ struct HTMLInnerWindow { IDispatch *mutation_observer_ctor; nsChannelBSC *bscallback; struct list bindings; + + ULONG navigation_type; + ULONG redirect_count; }; typedef enum { diff --git a/dlls/mshtml/navigate.c b/dlls/mshtml/navigate.c index 3583fb91069..b13570f3ca3 100644 --- a/dlls/mshtml/navigate.c +++ b/dlls/mshtml/navigate.c @@ -1351,9 +1351,9 @@ static HRESULT nsChannelBSC_start_binding(BSCallback *bsc) DWORD flags = This->bsc.window->base.outer_window->load_flags; if(flags & BINDING_FROMHIST) - This->bsc.window->performance_timing->navigation_type = 2; /* TYPE_BACK_FORWARD */ + This->bsc.window->navigation_type = 2; /* TYPE_BACK_FORWARD */ if(flags & BINDING_REFRESH) - This->bsc.window->performance_timing->navigation_type = 1; /* TYPE_RELOAD */ + This->bsc.window->navigation_type = 1; /* TYPE_RELOAD */ This->bsc.window->base.outer_window->base.inner_window->doc->skip_mutation_notif = FALSE; This->bsc.window->performance_timing->navigation_start_time = get_time_stamp(); @@ -1726,7 +1726,7 @@ static HRESULT nsChannelBSC_on_progress(BSCallback *bsc, ULONG progress, ULONG t break; case BINDSTATUS_REDIRECTING: if(This->is_doc_channel) { - This->bsc.window->performance_timing->redirect_count++; + This->bsc.window->redirect_count++; if(!This->bsc.window->performance_timing->redirect_time) This->bsc.window->performance_timing->redirect_time = get_time_stamp(); } diff --git a/dlls/mshtml/omnavigator.c b/dlls/mshtml/omnavigator.c index d8053328380..a884a6a9334 100644 --- a/dlls/mshtml/omnavigator.c +++ b/dlls/mshtml/omnavigator.c @@ -2003,7 +2003,7 @@ typedef struct { DispatchEx dispex; IHTMLPerformanceNavigation IHTMLPerformanceNavigation_iface; - HTMLPerformanceTiming *timing; + HTMLInnerWindow *window; } HTMLPerformanceNavigation; static inline HTMLPerformanceNavigation *impl_from_IHTMLPerformanceNavigation(IHTMLPerformanceNavigation *iface) @@ -2094,7 +2094,7 @@ static HRESULT WINAPI HTMLPerformanceNavigation_get_type(IHTMLPerformanceNavigat TRACE("(%p)->(%p)\n", This, p); - *p = This->timing->navigation_type; + *p = This->window->navigation_type; return S_OK; } @@ -2104,7 +2104,7 @@ static HRESULT WINAPI HTMLPerformanceNavigation_get_redirectCount(IHTMLPerforman TRACE("(%p)->(%p)\n", This, p); - *p = This->timing->redirect_count; + *p = This->window->redirect_count; return S_OK; } @@ -2146,17 +2146,17 @@ static inline HTMLPerformanceNavigation *HTMLPerformanceNavigation_from_Dispatch static void HTMLPerformanceNavigation_traverse(DispatchEx *dispex, nsCycleCollectionTraversalCallback *cb) { HTMLPerformanceNavigation *This = HTMLPerformanceNavigation_from_DispatchEx(dispex); - if(This->timing) - note_cc_edge((nsISupports*)&This->timing->IHTMLPerformanceTiming_iface, "timing", cb); + if(This->window) + note_cc_edge((nsISupports*)&This->window->base.IHTMLWindow2_iface, "window", cb); } static void HTMLPerformanceNavigation_unlink(DispatchEx *dispex) { HTMLPerformanceNavigation *This = HTMLPerformanceNavigation_from_DispatchEx(dispex); - if(This->timing) { - HTMLPerformanceTiming *timing = This->timing; - This->timing = NULL; - IHTMLPerformanceTiming_Release(&timing->IHTMLPerformanceTiming_iface); + if(This->window) { + HTMLInnerWindow *window = This->window; + This->window = NULL; + IHTMLWindow2_Release(&window->base.IHTMLWindow2_iface); } } @@ -2187,6 +2187,7 @@ typedef struct { DispatchEx dispex; IHTMLPerformance IHTMLPerformance_iface; + HTMLInnerWindow *window; IHTMLPerformanceNavigation *navigation; HTMLPerformanceTiming *timing; } HTMLPerformance; @@ -2288,8 +2289,8 @@ static HRESULT WINAPI HTMLPerformance_get_navigation(IHTMLPerformance *iface, return E_OUTOFMEMORY; navigation->IHTMLPerformanceNavigation_iface.lpVtbl = &HTMLPerformanceNavigationVtbl; - navigation->timing = This->timing; - IHTMLPerformanceTiming_AddRef(&This->timing->IHTMLPerformanceTiming_iface); + navigation->window = This->window; + IHTMLWindow2_AddRef(&This->window->base.IHTMLWindow2_iface); init_dispatch(&navigation->dispex, (IUnknown*)&navigation->IHTMLPerformanceNavigation_iface, &HTMLPerformanceNavigation_dispex, dispex_compat_mode(&This->dispex)); @@ -2358,6 +2359,11 @@ static void HTMLPerformance_traverse(DispatchEx *dispex, nsCycleCollectionTraver static void HTMLPerformance_unlink(DispatchEx *dispex) { HTMLPerformance *This = HTMLPerformance_from_DispatchEx(dispex); + if(This->window) { + HTMLInnerWindow *window = This->window; + This->window = NULL; + IHTMLWindow2_Release(&window->base.IHTMLWindow2_iface); + } unlink_ref(&This->navigation); if(This->timing) { HTMLPerformanceTiming *timing = This->timing; @@ -2399,6 +2405,8 @@ HRESULT create_performance(HTMLInnerWindow *window, IHTMLPerformance **ret) return E_OUTOFMEMORY; performance->IHTMLPerformance_iface.lpVtbl = &HTMLPerformanceVtbl; + performance->window = window; + IHTMLWindow2_AddRef(&window->base.IHTMLWindow2_iface); init_dispatch(&performance->dispex, (IUnknown*)&performance->IHTMLPerformance_iface, &HTMLPerformance_dispex, compat_mode); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3597
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlwindow.c | 8 -- dlls/mshtml/mshtml_private.h | 43 ++++------ dlls/mshtml/mutation.c | 2 +- dlls/mshtml/navigate.c | 20 ++--- dlls/mshtml/nsevents.c | 25 +++--- dlls/mshtml/omnavigator.c | 155 ++++++++++++++++++----------------- dlls/mshtml/view.c | 4 +- 7 files changed, 123 insertions(+), 134 deletions(-) diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index 4040473bd6e..1efd48e34f1 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -3909,7 +3909,6 @@ static void HTMLWindow_unlink(DispatchEx *dispex) This->local_storage = NULL; IHTMLStorage_Release(local_storage); } - IHTMLPerformanceTiming_Release(&This->performance_timing->IHTMLPerformanceTiming_iface); unlink_variant(&This->performance); } @@ -4304,19 +4303,12 @@ static void *alloc_window(size_t size) static HRESULT create_inner_window(HTMLOuterWindow *outer_window, IMoniker *mon, HTMLInnerWindow **ret) { HTMLInnerWindow *window; - HRESULT hres; window = alloc_window(sizeof(HTMLInnerWindow)); if(!window) return E_OUTOFMEMORY; window->base.IHTMLWindow2_iface.lpVtbl = &HTMLWindow2Vtbl; - hres = create_performance_timing(&window->performance_timing); - if(FAILED(hres)) { - free(window); - return hres; - } - list_init(&window->children); list_init(&window->script_hosts); list_init(&window->bindings); diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index 703eeae17ab..37fa5bda9ba 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -540,30 +540,6 @@ typedef struct { HTMLInnerWindow *window; } OmHistory; -typedef struct { - DispatchEx dispex; - IHTMLPerformanceTiming IHTMLPerformanceTiming_iface; - - LONG ref; - - ULONGLONG navigation_start_time; - ULONGLONG unload_event_start_time; - ULONGLONG unload_event_end_time; - ULONGLONG redirect_time; - ULONGLONG dns_lookup_time; - ULONGLONG connect_time; - ULONGLONG request_time; - ULONGLONG response_start_time; - ULONGLONG response_end_time; - ULONGLONG dom_interactive_time; - ULONGLONG dom_complete_time; - ULONGLONG dom_content_loaded_event_start_time; - ULONGLONG dom_content_loaded_event_end_time; - ULONGLONG load_event_start_time; - ULONGLONG load_event_end_time; - ULONGLONG first_paint_time; -} HTMLPerformanceTiming; - typedef struct nsChannelBSC nsChannelBSC; struct HTMLWindow { @@ -639,7 +615,6 @@ struct HTMLInnerWindow { BOOL performance_initialized; VARIANT performance; - HTMLPerformanceTiming *performance_timing; unsigned blocking_depth; unsigned parser_callback_cnt; @@ -658,6 +633,23 @@ struct HTMLInnerWindow { ULONG navigation_type; ULONG redirect_count; + + ULONGLONG navigation_start_time; + ULONGLONG unload_event_start_time; + ULONGLONG unload_event_end_time; + ULONGLONG redirect_time; + ULONGLONG dns_lookup_time; + ULONGLONG connect_time; + ULONGLONG request_time; + ULONGLONG response_start_time; + ULONGLONG response_end_time; + ULONGLONG dom_interactive_time; + ULONGLONG dom_complete_time; + ULONGLONG dom_content_loaded_event_start_time; + ULONGLONG dom_content_loaded_event_end_time; + ULONGLONG load_event_start_time; + ULONGLONG load_event_end_time; + ULONGLONG first_paint_time; }; typedef enum { @@ -1029,7 +1021,6 @@ HRESULT create_location(HTMLOuterWindow*,HTMLLocation**); HRESULT create_navigator(compat_mode_t,IOmNavigator**); HRESULT create_html_screen(compat_mode_t,IHTMLScreen**); HRESULT create_performance(HTMLInnerWindow*,IHTMLPerformance**); -HRESULT create_performance_timing(HTMLPerformanceTiming**); HRESULT create_history(HTMLInnerWindow*,OmHistory**); HRESULT create_namespace_collection(compat_mode_t,IHTMLNamespaceCollection**); HRESULT create_dom_implementation(HTMLDocumentNode*,IHTMLDOMImplementation**); diff --git a/dlls/mshtml/mutation.c b/dlls/mshtml/mutation.c index 823c95b950a..864a224a7dd 100644 --- a/dlls/mshtml/mutation.c +++ b/dlls/mshtml/mutation.c @@ -313,7 +313,7 @@ static nsresult run_end_load(HTMLDocumentNode *This, nsISupports *arg1, nsISuppo bind_event_scripts(This); if(This->window == window) { - window->performance_timing->dom_interactive_time = get_time_stamp(); + window->dom_interactive_time = get_time_stamp(); set_ready_state(This->outer_window, READYSTATE_INTERACTIVE); } IHTMLWindow2_Release(&window->base.IHTMLWindow2_iface); diff --git a/dlls/mshtml/navigate.c b/dlls/mshtml/navigate.c index b13570f3ca3..8172abd8040 100644 --- a/dlls/mshtml/navigate.c +++ b/dlls/mshtml/navigate.c @@ -1095,7 +1095,7 @@ static HRESULT read_stream_data(nsChannelBSC *This, IStream *stream) IWinInetHttpInfo *wininet_info; if(This->is_doc_channel) - This->bsc.window->performance_timing->response_start_time = get_time_stamp(); + This->bsc.window->response_start_time = get_time_stamp(); This->response_processed = TRUE; if(This->bsc.binding) { @@ -1356,7 +1356,7 @@ static HRESULT nsChannelBSC_start_binding(BSCallback *bsc) This->bsc.window->navigation_type = 1; /* TYPE_RELOAD */ This->bsc.window->base.outer_window->base.inner_window->doc->skip_mutation_notif = FALSE; - This->bsc.window->performance_timing->navigation_start_time = get_time_stamp(); + This->bsc.window->navigation_start_time = get_time_stamp(); } return S_OK; @@ -1524,7 +1524,7 @@ static HRESULT nsChannelBSC_stop_binding(BSCallback *bsc, HRESULT result) nsChannelBSC *This = nsChannelBSC_from_BSCallback(bsc); if(This->is_doc_channel && This->bsc.window) { - This->bsc.window->performance_timing->response_end_time = get_time_stamp(); + This->bsc.window->response_end_time = get_time_stamp(); if(result != E_ABORT) { if(FAILED(result)) handle_navigation_error(This, result); @@ -1727,21 +1727,21 @@ static HRESULT nsChannelBSC_on_progress(BSCallback *bsc, ULONG progress, ULONG t case BINDSTATUS_REDIRECTING: if(This->is_doc_channel) { This->bsc.window->redirect_count++; - if(!This->bsc.window->performance_timing->redirect_time) - This->bsc.window->performance_timing->redirect_time = get_time_stamp(); + if(!This->bsc.window->redirect_time) + This->bsc.window->redirect_time = get_time_stamp(); } return handle_redirect(This, status_text); case BINDSTATUS_FINDINGRESOURCE: - if(This->is_doc_channel && !This->bsc.window->performance_timing->dns_lookup_time) - This->bsc.window->performance_timing->dns_lookup_time = get_time_stamp(); + if(This->is_doc_channel && !This->bsc.window->dns_lookup_time) + This->bsc.window->dns_lookup_time = get_time_stamp(); break; case BINDSTATUS_CONNECTING: if(This->is_doc_channel) - This->bsc.window->performance_timing->connect_time = get_time_stamp(); + This->bsc.window->connect_time = get_time_stamp(); break; case BINDSTATUS_SENDINGREQUEST: if(This->is_doc_channel) - This->bsc.window->performance_timing->request_time = get_time_stamp(); + This->bsc.window->request_time = get_time_stamp(); break; case BINDSTATUS_BEGINDOWNLOADDATA: { IWinInetHttpInfo *http_info; @@ -1798,7 +1798,7 @@ static HRESULT nsChannelBSC_on_response(BSCallback *bsc, DWORD response_code, HRESULT hres; if(This->is_doc_channel) - This->bsc.window->performance_timing->response_start_time = get_time_stamp(); + This->bsc.window->response_start_time = get_time_stamp(); This->response_processed = TRUE; This->nschannel->response_status = response_code; diff --git a/dlls/mshtml/nsevents.c b/dlls/mshtml/nsevents.c index 76a3f4ab4c6..78a54c83955 100644 --- a/dlls/mshtml/nsevents.c +++ b/dlls/mshtml/nsevents.c @@ -238,7 +238,7 @@ static nsresult handle_dom_content_loaded(HTMLDocumentNode *doc, nsIDOMEvent *ns HRESULT hres; if(doc->window) - doc->window->performance_timing->dom_content_loaded_event_start_time = get_time_stamp(); + doc->window->dom_content_loaded_event_start_time = get_time_stamp(); hres = create_event_from_nsevent(nsevent, dispex_compat_mode(&doc->node.event_target.dispex), &event); if(SUCCEEDED(hres)) { @@ -247,7 +247,7 @@ static nsresult handle_dom_content_loaded(HTMLDocumentNode *doc, nsIDOMEvent *ns } if(doc->window) - doc->window->performance_timing->dom_content_loaded_event_end_time = get_time_stamp(); + doc->window->dom_content_loaded_event_end_time = get_time_stamp(); return NS_OK; } @@ -345,7 +345,7 @@ static nsresult handle_load(HTMLDocumentNode *doc, nsIDOMEvent *event) if(doc_obj) handle_docobj_load(doc_obj); - doc->window->performance_timing->dom_complete_time = get_time_stamp(); + doc->window->dom_complete_time = get_time_stamp(); set_ready_state(doc->outer_window, READYSTATE_COMPLETE); if(doc_obj) { @@ -363,7 +363,7 @@ static nsresult handle_load(HTMLDocumentNode *doc, nsIDOMEvent *event) IUnknown_Release(doc_obj->outer_unk); } - doc->window->performance_timing->load_event_start_time = get_time_stamp(); + doc->window->load_event_start_time = get_time_stamp(); if(doc->dom_document) { hres = create_document_event(doc, EVENTID_LOAD, &load_event); @@ -381,7 +381,7 @@ static nsresult handle_load(HTMLDocumentNode *doc, nsIDOMEvent *event) IDOMEvent_Release(&load_event->IDOMEvent_iface); } - doc->window->performance_timing->load_event_end_time = get_time_stamp(); + doc->window->load_event_end_time = get_time_stamp(); return NS_OK; } @@ -406,8 +406,7 @@ static nsresult handle_beforeunload(HTMLDocumentNode *doc, nsIDOMEvent *nsevent) static nsresult handle_unload(HTMLDocumentNode *doc, nsIDOMEvent *nsevent) { - HTMLPerformanceTiming *timing = NULL; - HTMLInnerWindow *window; + HTMLInnerWindow *window, *pending_window; DOMEvent *event; HRESULT hres; @@ -415,11 +414,9 @@ static nsresult handle_unload(HTMLDocumentNode *doc, nsIDOMEvent *nsevent) return NS_OK; doc->unload_sent = TRUE; - if(window->base.outer_window->pending_window) - timing = window->base.outer_window->pending_window->performance_timing; - - if(timing) - timing->unload_event_start_time = get_time_stamp(); + pending_window = window->base.outer_window->pending_window; + if(pending_window) + pending_window->unload_event_start_time = get_time_stamp(); hres = create_event_from_nsevent(nsevent, dispex_compat_mode(&doc->node.event_target.dispex), &event); if(SUCCEEDED(hres)) { @@ -427,8 +424,8 @@ static nsresult handle_unload(HTMLDocumentNode *doc, nsIDOMEvent *nsevent) IDOMEvent_Release(&event->IDOMEvent_iface); } - if(timing) - timing->unload_event_end_time = get_time_stamp(); + if(pending_window) + pending_window->unload_event_end_time = get_time_stamp(); return NS_OK; } diff --git a/dlls/mshtml/omnavigator.c b/dlls/mshtml/omnavigator.c index a884a6a9334..8e937328835 100644 --- a/dlls/mshtml/omnavigator.c +++ b/dlls/mshtml/omnavigator.c @@ -1584,6 +1584,14 @@ HRESULT create_navigator(compat_mode_t compat_mode, IOmNavigator **navigator) return S_OK; } +typedef struct { + DispatchEx dispex; + IHTMLPerformanceTiming IHTMLPerformanceTiming_iface; + + LONG ref; + HTMLInnerWindow *window; +} HTMLPerformanceTiming; + static inline HTMLPerformanceTiming *impl_from_IHTMLPerformanceTiming(IHTMLPerformanceTiming *iface) { return CONTAINING_RECORD(iface, HTMLPerformanceTiming, IHTMLPerformanceTiming_iface); @@ -1628,12 +1636,8 @@ static ULONG WINAPI HTMLPerformanceTiming_Release(IHTMLPerformanceTiming *iface) TRACE("(%p) ref=%ld\n", This, ref); - if(!ref) { - if(This->dispex.outer) - release_dispex(&This->dispex); - else - free(This); - } + if(!ref) + release_dispex(&This->dispex); return ref; } @@ -1675,20 +1679,22 @@ static HRESULT WINAPI HTMLPerformanceTiming_Invoke(IHTMLPerformanceTiming *iface static ULONGLONG get_fetch_time(HTMLPerformanceTiming *This) { + HTMLInnerWindow *window = This->window; + /* If there's no prior doc unloaded and no redirects, fetch time == navigationStart time */ - if(!This->unload_event_end_time && !This->redirect_time) - return This->navigation_start_time; + if(!window->unload_event_end_time && !window->redirect_time) + return window->navigation_start_time; - if(This->dns_lookup_time) - return This->dns_lookup_time; - if(This->connect_time) - return This->connect_time; - if(This->request_time) - return This->request_time; - if(This->unload_event_end_time) - return This->unload_event_end_time; + if(window->dns_lookup_time) + return window->dns_lookup_time; + if(window->connect_time) + return window->connect_time; + if(window->request_time) + return window->request_time; + if(window->unload_event_end_time) + return window->unload_event_end_time; - return This->redirect_time; + return window->redirect_time; } static HRESULT WINAPI HTMLPerformanceTiming_get_navigationStart(IHTMLPerformanceTiming *iface, ULONGLONG *p) @@ -1697,7 +1703,7 @@ static HRESULT WINAPI HTMLPerformanceTiming_get_navigationStart(IHTMLPerformance TRACE("(%p)->(%p)\n", This, p); - *p = This->navigation_start_time; + *p = This->window->navigation_start_time; return S_OK; } @@ -1707,7 +1713,7 @@ static HRESULT WINAPI HTMLPerformanceTiming_get_unloadEventStart(IHTMLPerformanc TRACE("(%p)->(%p)\n", This, p); - *p = This->unload_event_start_time; + *p = This->window->unload_event_start_time; return S_OK; } @@ -1717,7 +1723,7 @@ static HRESULT WINAPI HTMLPerformanceTiming_get_unloadEventEnd(IHTMLPerformanceT TRACE("(%p)->(%p)\n", This, p); - *p = This->unload_event_end_time; + *p = This->window->unload_event_end_time; return S_OK; } @@ -1727,7 +1733,7 @@ static HRESULT WINAPI HTMLPerformanceTiming_get_redirectStart(IHTMLPerformanceTi TRACE("(%p)->(%p)\n", This, p); - *p = This->redirect_time; + *p = This->window->redirect_time; return S_OK; } @@ -1737,7 +1743,7 @@ static HRESULT WINAPI HTMLPerformanceTiming_get_redirectEnd(IHTMLPerformanceTimi TRACE("(%p)->(%p)\n", This, p); - *p = This->redirect_time ? get_fetch_time(This) : 0; + *p = This->window->redirect_time ? get_fetch_time(This) : 0; return S_OK; } @@ -1757,7 +1763,7 @@ static HRESULT WINAPI HTMLPerformanceTiming_get_domainLookupStart(IHTMLPerforman TRACE("(%p)->(%p)\n", This, p); - *p = This->dns_lookup_time ? This->dns_lookup_time : get_fetch_time(This); + *p = This->window->dns_lookup_time ? This->window->dns_lookup_time : get_fetch_time(This); return S_OK; } @@ -1767,8 +1773,8 @@ static HRESULT WINAPI HTMLPerformanceTiming_get_domainLookupEnd(IHTMLPerformance TRACE("(%p)->(%p)\n", This, p); - *p = This->connect_time ? This->connect_time : - This->dns_lookup_time ? This->dns_lookup_time : get_fetch_time(This); + *p = This->window->connect_time ? This->window->connect_time : + This->window->dns_lookup_time ? This->window->dns_lookup_time : get_fetch_time(This); return S_OK; } @@ -1778,8 +1784,8 @@ static HRESULT WINAPI HTMLPerformanceTiming_get_connectStart(IHTMLPerformanceTim TRACE("(%p)->(%p)\n", This, p); - *p = This->connect_time ? This->connect_time : - This->dns_lookup_time ? This->dns_lookup_time : get_fetch_time(This); + *p = This->window->connect_time ? This->window->connect_time : + This->window->dns_lookup_time ? This->window->dns_lookup_time : get_fetch_time(This); return S_OK; } @@ -1789,9 +1795,9 @@ static HRESULT WINAPI HTMLPerformanceTiming_get_connectEnd(IHTMLPerformanceTimin TRACE("(%p)->(%p)\n", This, p); - *p = This->request_time ? This->request_time : - This->connect_time ? This->connect_time : - This->dns_lookup_time ? This->dns_lookup_time : get_fetch_time(This); + *p = This->window->request_time ? This->window->request_time : + This->window->connect_time ? This->window->connect_time : + This->window->dns_lookup_time ? This->window->dns_lookup_time : get_fetch_time(This); return S_OK; } @@ -1801,9 +1807,9 @@ static HRESULT WINAPI HTMLPerformanceTiming_get_requestStart(IHTMLPerformanceTim TRACE("(%p)->(%p)\n", This, p); - *p = This->request_time ? This->request_time : - This->connect_time ? This->connect_time : - This->dns_lookup_time ? This->dns_lookup_time : get_fetch_time(This); + *p = This->window->request_time ? This->window->request_time : + This->window->connect_time ? This->window->connect_time : + This->window->dns_lookup_time ? This->window->dns_lookup_time : get_fetch_time(This); return S_OK; } @@ -1813,7 +1819,7 @@ static HRESULT WINAPI HTMLPerformanceTiming_get_responseStart(IHTMLPerformanceTi TRACE("(%p)->(%p)\n", This, p); - *p = This->response_start_time; + *p = This->window->response_start_time; return S_OK; } @@ -1823,7 +1829,7 @@ static HRESULT WINAPI HTMLPerformanceTiming_get_responseEnd(IHTMLPerformanceTimi TRACE("(%p)->(%p)\n", This, p); - *p = This->response_end_time; + *p = This->window->response_end_time; return S_OK; } @@ -1834,7 +1840,7 @@ static HRESULT WINAPI HTMLPerformanceTiming_get_domLoading(IHTMLPerformanceTimin TRACE("(%p)->(%p)\n", This, p); /* Make sure this is after responseEnd, when the Gecko parser starts */ - *p = This->response_end_time; + *p = This->window->response_end_time; return S_OK; } @@ -1844,7 +1850,7 @@ static HRESULT WINAPI HTMLPerformanceTiming_get_domInteractive(IHTMLPerformanceT TRACE("(%p)->(%p)\n", This, p); - *p = This->dom_interactive_time; + *p = This->window->dom_interactive_time; return S_OK; } @@ -1854,7 +1860,7 @@ static HRESULT WINAPI HTMLPerformanceTiming_get_domContentLoadedEventStart(IHTML TRACE("(%p)->(%p)\n", This, p); - *p = This->dom_content_loaded_event_start_time; + *p = This->window->dom_content_loaded_event_start_time; return S_OK; } @@ -1864,7 +1870,7 @@ static HRESULT WINAPI HTMLPerformanceTiming_get_domContentLoadedEventEnd(IHTMLPe TRACE("(%p)->(%p)\n", This, p); - *p = This->dom_content_loaded_event_end_time; + *p = This->window->dom_content_loaded_event_end_time; return S_OK; } @@ -1874,7 +1880,7 @@ static HRESULT WINAPI HTMLPerformanceTiming_get_domComplete(IHTMLPerformanceTimi TRACE("(%p)->(%p)\n", This, p); - *p = This->dom_complete_time; + *p = This->window->dom_complete_time; return S_OK; } @@ -1884,7 +1890,7 @@ static HRESULT WINAPI HTMLPerformanceTiming_get_loadEventStart(IHTMLPerformanceT TRACE("(%p)->(%p)\n", This, p); - *p = This->load_event_start_time; + *p = This->window->load_event_start_time; return S_OK; } @@ -1894,7 +1900,7 @@ static HRESULT WINAPI HTMLPerformanceTiming_get_loadEventEnd(IHTMLPerformanceTim TRACE("(%p)->(%p)\n", This, p); - *p = This->load_event_end_time; + *p = This->window->load_event_end_time; return S_OK; } @@ -1904,7 +1910,7 @@ static HRESULT WINAPI HTMLPerformanceTiming_get_msFirstPaint(IHTMLPerformanceTim TRACE("(%p)->(%p)\n", This, p); - *p = This->first_paint_time; + *p = This->window->first_paint_time; return S_OK; } @@ -1962,6 +1968,16 @@ static inline HTMLPerformanceTiming *HTMLPerformanceTiming_from_DispatchEx(Dispa return CONTAINING_RECORD(iface, HTMLPerformanceTiming, dispex); } +static void HTMLPerformanceTiming_unlink(DispatchEx *dispex) +{ + HTMLPerformanceTiming *This = HTMLPerformanceTiming_from_DispatchEx(dispex); + if(This->window) { + HTMLInnerWindow *window = This->window; + This->window = NULL; + IHTMLWindow2_Release(&window->base.IHTMLWindow2_iface); + } +} + static void HTMLPerformanceTiming_destructor(DispatchEx *dispex) { HTMLPerformanceTiming *This = HTMLPerformanceTiming_from_DispatchEx(dispex); @@ -1970,6 +1986,7 @@ static void HTMLPerformanceTiming_destructor(DispatchEx *dispex) static const dispex_static_data_vtbl_t HTMLPerformanceTiming_dispex_vtbl = { .destructor = HTMLPerformanceTiming_destructor, + .unlink = HTMLPerformanceTiming_unlink }; static const tid_t HTMLPerformanceTiming_iface_tids[] = { @@ -1983,22 +2000,6 @@ static dispex_static_data_t HTMLPerformanceTiming_dispex = { HTMLPerformanceTiming_iface_tids }; -HRESULT create_performance_timing(HTMLPerformanceTiming **ret) -{ - HTMLPerformanceTiming *timing; - - timing = calloc(1, sizeof(*timing)); - if(!timing) - return E_OUTOFMEMORY; - - timing->IHTMLPerformanceTiming_iface.lpVtbl = &HTMLPerformanceTimingVtbl; - timing->ref = 1; - - /* Defer initializing the dispex until it's actually needed (for compat mode) */ - *ret = timing; - return S_OK; -} - typedef struct { DispatchEx dispex; IHTMLPerformanceNavigation IHTMLPerformanceNavigation_iface; @@ -2189,7 +2190,7 @@ typedef struct { HTMLInnerWindow *window; IHTMLPerformanceNavigation *navigation; - HTMLPerformanceTiming *timing; + IHTMLPerformanceTiming *timing; } HTMLPerformance; static inline HTMLPerformance *impl_from_IHTMLPerformance(IHTMLPerformance *iface) @@ -2308,7 +2309,25 @@ static HRESULT WINAPI HTMLPerformance_get_timing(IHTMLPerformance *iface, IHTMLP TRACE("(%p)->(%p)\n", This, p); - IHTMLPerformanceTiming_AddRef(*p = &This->timing->IHTMLPerformanceTiming_iface); + if(!This->timing) { + HTMLPerformanceTiming *timing; + + timing = calloc(1, sizeof(*timing)); + if(!timing) + return E_OUTOFMEMORY; + + timing->IHTMLPerformanceTiming_iface.lpVtbl = &HTMLPerformanceTimingVtbl; + timing->ref = 1; + timing->window = This->window; + IHTMLWindow2_AddRef(&This->window->base.IHTMLWindow2_iface); + + init_dispatch(&timing->dispex, (IUnknown*)&timing->IHTMLPerformanceTiming_iface, + &HTMLPerformanceTiming_dispex, dispex_compat_mode(&This->dispex)); + + This->timing = &timing->IHTMLPerformanceTiming_iface; + } + + IHTMLPerformanceTiming_AddRef(*p = This->timing); return S_OK; } @@ -2353,7 +2372,7 @@ static void HTMLPerformance_traverse(DispatchEx *dispex, nsCycleCollectionTraver if(This->navigation) note_cc_edge((nsISupports*)This->navigation, "navigation", cb); if(This->timing) - note_cc_edge((nsISupports*)&This->timing->IHTMLPerformanceTiming_iface, "timing", cb); + note_cc_edge((nsISupports*)This->timing, "timing", cb); } static void HTMLPerformance_unlink(DispatchEx *dispex) @@ -2365,11 +2384,7 @@ static void HTMLPerformance_unlink(DispatchEx *dispex) IHTMLWindow2_Release(&window->base.IHTMLWindow2_iface); } unlink_ref(&This->navigation); - if(This->timing) { - HTMLPerformanceTiming *timing = This->timing; - This->timing = NULL; - IHTMLPerformanceTiming_Release(&timing->IHTMLPerformanceTiming_iface); - } + unlink_ref(&This->timing); } static void HTMLPerformance_destructor(DispatchEx *dispex) @@ -2411,12 +2426,6 @@ HRESULT create_performance(HTMLInnerWindow *window, IHTMLPerformance **ret) init_dispatch(&performance->dispex, (IUnknown*)&performance->IHTMLPerformance_iface, &HTMLPerformance_dispex, compat_mode); - performance->timing = window->performance_timing; - IHTMLPerformanceTiming_AddRef(&performance->timing->IHTMLPerformanceTiming_iface); - - init_dispatch(&performance->timing->dispex, (IUnknown*)&performance->timing->IHTMLPerformanceTiming_iface, - &HTMLPerformanceTiming_dispex, compat_mode); - *ret = &performance->IHTMLPerformance_iface; return S_OK; } diff --git a/dlls/mshtml/view.c b/dlls/mshtml/view.c index 43ebc5284f5..fe7448ec0cf 100644 --- a/dlls/mshtml/view.c +++ b/dlls/mshtml/view.c @@ -50,8 +50,8 @@ static void paint_document(HTMLDocumentObj *This) RECT rect; HDC hdc; - if(This->window && This->window->base.inner_window && !This->window->base.inner_window->performance_timing->first_paint_time) - This->window->base.inner_window->performance_timing->first_paint_time = get_time_stamp(); + if(This->window && This->window->base.inner_window && !This->window->base.inner_window->first_paint_time) + This->window->base.inner_window->first_paint_time = get_time_stamp(); GetClientRect(This->hwnd, &rect); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3597
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/omnavigator.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/dlls/mshtml/omnavigator.c b/dlls/mshtml/omnavigator.c index 8e937328835..32030b106c2 100644 --- a/dlls/mshtml/omnavigator.c +++ b/dlls/mshtml/omnavigator.c @@ -1588,7 +1588,6 @@ typedef struct { DispatchEx dispex; IHTMLPerformanceTiming IHTMLPerformanceTiming_iface; - LONG ref; HTMLInnerWindow *window; } HTMLPerformanceTiming; @@ -1607,7 +1606,7 @@ static HRESULT WINAPI HTMLPerformanceTiming_QueryInterface(IHTMLPerformanceTimin *ppv = &This->IHTMLPerformanceTiming_iface; }else if(IsEqualGUID(&IID_IHTMLPerformanceTiming, riid)) { *ppv = &This->IHTMLPerformanceTiming_iface; - }else if(dispex_query_interface_no_cc(&This->dispex, riid, ppv)) { + }else if(dispex_query_interface(&This->dispex, riid, ppv)) { return *ppv ? S_OK : E_NOINTERFACE; }else { WARN("Unsupported interface %s\n", debugstr_mshtml_guid(riid)); @@ -1622,7 +1621,7 @@ static HRESULT WINAPI HTMLPerformanceTiming_QueryInterface(IHTMLPerformanceTimin static ULONG WINAPI HTMLPerformanceTiming_AddRef(IHTMLPerformanceTiming *iface) { HTMLPerformanceTiming *This = impl_from_IHTMLPerformanceTiming(iface); - LONG ref = InterlockedIncrement(&This->ref); + LONG ref = dispex_ref_incr(&This->dispex); TRACE("(%p) ref=%ld\n", This, ref); @@ -1632,13 +1631,10 @@ static ULONG WINAPI HTMLPerformanceTiming_AddRef(IHTMLPerformanceTiming *iface) static ULONG WINAPI HTMLPerformanceTiming_Release(IHTMLPerformanceTiming *iface) { HTMLPerformanceTiming *This = impl_from_IHTMLPerformanceTiming(iface); - LONG ref = InterlockedDecrement(&This->ref); + LONG ref = dispex_ref_decr(&This->dispex); TRACE("(%p) ref=%ld\n", This, ref); - if(!ref) - release_dispex(&This->dispex); - return ref; } @@ -1968,6 +1964,13 @@ static inline HTMLPerformanceTiming *HTMLPerformanceTiming_from_DispatchEx(Dispa return CONTAINING_RECORD(iface, HTMLPerformanceTiming, dispex); } +static void HTMLPerformanceTiming_traverse(DispatchEx *dispex, nsCycleCollectionTraversalCallback *cb) +{ + HTMLPerformanceTiming *This = HTMLPerformanceTiming_from_DispatchEx(dispex); + if(This->window) + note_cc_edge((nsISupports*)&This->window->base.IHTMLWindow2_iface, "window", cb); +} + static void HTMLPerformanceTiming_unlink(DispatchEx *dispex) { HTMLPerformanceTiming *This = HTMLPerformanceTiming_from_DispatchEx(dispex); @@ -1986,6 +1989,7 @@ static void HTMLPerformanceTiming_destructor(DispatchEx *dispex) static const dispex_static_data_vtbl_t HTMLPerformanceTiming_dispex_vtbl = { .destructor = HTMLPerformanceTiming_destructor, + .traverse = HTMLPerformanceTiming_traverse, .unlink = HTMLPerformanceTiming_unlink }; @@ -2317,7 +2321,6 @@ static HRESULT WINAPI HTMLPerformance_get_timing(IHTMLPerformance *iface, IHTMLP return E_OUTOFMEMORY; timing->IHTMLPerformanceTiming_iface.lpVtbl = &HTMLPerformanceTimingVtbl; - timing->ref = 1; timing->window = This->window; IHTMLWindow2_AddRef(&This->window->base.IHTMLWindow2_iface); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3597
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlevent.c | 21 --------------------- dlls/mshtml/mshtml_private.h | 1 - dlls/mshtml/xmlhttprequest.c | 24 ++++++++++++++++++------ 3 files changed, 18 insertions(+), 28 deletions(-) diff --git a/dlls/mshtml/htmlevent.c b/dlls/mshtml/htmlevent.c index e4f299d7986..797e92492bc 100644 --- a/dlls/mshtml/htmlevent.c +++ b/dlls/mshtml/htmlevent.c @@ -4499,27 +4499,6 @@ HRESULT EventTarget_QI(EventTarget *event_target, REFIID riid, void **ppv) return E_NOINTERFACE; } -HRESULT EventTarget_QI_no_cc(EventTarget *event_target, REFIID riid, void **ppv) -{ - if(IsEqualGUID(riid, &IID_IEventTarget)) { - if(use_event_quirks(event_target)) { - WARN("IEventTarget queried, but not supported by in document mode\n"); - *ppv = NULL; - return E_NOINTERFACE; - } - IEventTarget_AddRef(&event_target->IEventTarget_iface); - *ppv = &event_target->IEventTarget_iface; - return S_OK; - } - - if(dispex_query_interface_no_cc(&event_target->dispex, riid, ppv)) - return *ppv ? S_OK : E_NOINTERFACE; - - WARN("(%p)->(%s %p)\n", event_target, debugstr_mshtml_guid(riid), ppv); - *ppv = NULL; - return E_NOINTERFACE; -} - void EventTarget_init_dispex_info(dispex_data_t *dispex_info, compat_mode_t compat_mode) { static const dispex_hook_t IEventTarget_hooks[] = { diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index 37fa5bda9ba..841376c461a 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -1227,7 +1227,6 @@ void HTMLElement_Init(HTMLElement*,HTMLDocumentNode*,nsIDOMElement*,dispex_stati void EventTarget_Init(EventTarget*,IUnknown*,dispex_static_data_t*,compat_mode_t); HRESULT EventTarget_QI(EventTarget*,REFIID,void**); -HRESULT EventTarget_QI_no_cc(EventTarget*,REFIID,void**); void EventTarget_init_dispex_info(dispex_data_t*,compat_mode_t); HRESULT HTMLDOMNode_QI(HTMLDOMNode*,REFIID,void**); diff --git a/dlls/mshtml/xmlhttprequest.c b/dlls/mshtml/xmlhttprequest.c index 33770faa1bf..650a85cf659 100644 --- a/dlls/mshtml/xmlhttprequest.c +++ b/dlls/mshtml/xmlhttprequest.c @@ -136,7 +136,6 @@ struct HTMLXMLHttpRequest { IHTMLXMLHttpRequest2 IHTMLXMLHttpRequest2_iface; IWineXMLHttpRequestPrivate IWineXMLHttpRequestPrivate_iface; IProvideClassInfo2 IProvideClassInfo2_iface; - LONG ref; LONG task_magic; LONG ready_state; response_type_t response_type; @@ -526,7 +525,7 @@ static HRESULT WINAPI HTMLXMLHttpRequest_QueryInterface(IHTMLXMLHttpRequest *ifa }else if(IsEqualGUID(&IID_IProvideClassInfo2, riid)) { *ppv = &This->IProvideClassInfo2_iface; }else { - return EventTarget_QI_no_cc(&This->event_target, riid, ppv); + return EventTarget_QI(&This->event_target, riid, ppv); } IUnknown_AddRef((IUnknown*)*ppv); @@ -536,7 +535,7 @@ static HRESULT WINAPI HTMLXMLHttpRequest_QueryInterface(IHTMLXMLHttpRequest *ifa static ULONG WINAPI HTMLXMLHttpRequest_AddRef(IHTMLXMLHttpRequest *iface) { HTMLXMLHttpRequest *This = impl_from_IHTMLXMLHttpRequest(iface); - LONG ref = InterlockedIncrement(&This->ref); + LONG ref = dispex_ref_incr(&This->event_target.dispex); TRACE("(%p) ref=%ld\n", This, ref); @@ -546,12 +545,14 @@ static ULONG WINAPI HTMLXMLHttpRequest_AddRef(IHTMLXMLHttpRequest *iface) static ULONG WINAPI HTMLXMLHttpRequest_Release(IHTMLXMLHttpRequest *iface) { HTMLXMLHttpRequest *This = impl_from_IHTMLXMLHttpRequest(iface); - LONG ref = InterlockedDecrement(&This->ref); + LONG ref = dispex_ref_decr(&This->event_target.dispex); TRACE("(%p) ref=%ld\n", This, ref); + /* The Cycle Collector might delay the unlink or destruction after ref reaches 0, + but we don't want a task to possibly grab ref to us and send events anymore. */ if(!ref) - release_dispex(&This->event_target.dispex); + This->magic++; return ref; } @@ -1521,6 +1522,17 @@ static inline HTMLXMLHttpRequest *impl_from_DispatchEx(DispatchEx *iface) return CONTAINING_RECORD(iface, HTMLXMLHttpRequest, event_target.dispex); } +static void HTMLXMLHttpRequest_traverse(DispatchEx *dispex, nsCycleCollectionTraversalCallback *cb) +{ + HTMLXMLHttpRequest *This = impl_from_DispatchEx(dispex); + if(This->window) + note_cc_edge((nsISupports*)&This->window->base.IHTMLWindow2_iface, "window", cb); + if(This->pending_progress_event) + note_cc_edge((nsISupports*)&This->pending_progress_event->IDOMEvent_iface, "pending_progress_event", cb); + if(This->nsxhr) + note_cc_edge((nsISupports*)This->nsxhr, "nsxhr", cb); +} + static void HTMLXMLHttpRequest_unlink(DispatchEx *dispex) { HTMLXMLHttpRequest *This = impl_from_DispatchEx(dispex); @@ -1595,6 +1607,7 @@ static void HTMLXMLHttpRequest_init_dispex_info(dispex_data_t *info, compat_mode static const event_target_vtbl_t HTMLXMLHttpRequest_event_target_vtbl = { { .destructor = HTMLXMLHttpRequest_destructor, + .traverse = HTMLXMLHttpRequest_traverse, .unlink = HTMLXMLHttpRequest_unlink }, .get_gecko_target = HTMLXMLHttpRequest_get_gecko_target, @@ -1739,7 +1752,6 @@ static HRESULT WINAPI HTMLXMLHttpRequestFactory_create(IHTMLXMLHttpRequestFactor ret->IProvideClassInfo2_iface.lpVtbl = &ProvideClassInfo2Vtbl; EventTarget_Init(&ret->event_target, (IUnknown*)&ret->IHTMLXMLHttpRequest_iface, &HTMLXMLHttpRequest_dispex, This->window->doc->document_mode); - ret->ref = 1; /* Always register the handlers because we need them to track state */ event_listener->nsIDOMEventListener_iface.lpVtbl = &XMLHttpReqEventListenerVtbl; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3597
Jacek Caban (@jacek) commented about dlls/mshtml/xmlhttprequest.c:
static ULONG WINAPI HTMLXMLHttpRequest_Release(IHTMLXMLHttpRequest *iface) { HTMLXMLHttpRequest *This = impl_from_IHTMLXMLHttpRequest(iface); - LONG ref = InterlockedDecrement(&This->ref); + LONG ref = dispex_ref_decr(&This->event_target.dispex);
TRACE("(%p) ref=%ld\n", This, ref);
+ /* The Cycle Collector might delay the unlink or destruction after ref reaches 0, + but we don't want a task to possibly grab ref to us and send events anymore. */ If we reach 0 ref count, it means that we're already unlinked, so where do those events come from?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3597#note_42641
It's possible that an event task is still pending to be processed. Tasks don't hold ref to the XHR, but they do possibly grab it once processing starts. This wasn't an issue before because we called remove_target_tasks immediately when ref reached 0. But gecko's CC doesn't necessarily call unlink/delete_cycle_collectable immediately when ref reaches 0, it usually delays it. So a task could get processed between ref reaching 0 and the CC actually cleaning it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3597#note_42713
On Fri Aug 18 12:57:59 2023 +0000, Gabriel Ivăncescu wrote:
It's possible that an event task is still pending to be processed. Tasks don't hold ref to the XHR, but they do possibly grab it once processing starts. This wasn't an issue before because we called remove_target_tasks immediately when ref reached 0. But gecko's CC doesn't necessarily call unlink/delete_cycle_collectable immediately when ref reaches 0, it usually delays it. So a task could get processed between ref reaching 0 and the CC actually cleaning it. It looks like we will need an equivalent of Gecko's `NS_IMPL_CYCLE_COLLECTING_RELEASE_WITH_LAST_RELEASE`. The problem with your patch is that if deletion happens, you're accessing a freed object. Gecko avoids that by explicitly requesting not to delete it with `shouldDelete` and does the deletion later if needed. Unfortunately, that would require Gecko interface extension.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3597#note_42732
On Fri Aug 18 12:57:59 2023 +0000, Jacek Caban wrote:
It looks like we will need an equivalent of Gecko's `NS_IMPL_CYCLE_COLLECTING_RELEASE_WITH_LAST_RELEASE`. The problem with your patch is that if deletion happens, you're accessing a freed object. Gecko avoids that by explicitly requesting not to delete it with `shouldDelete` and does the deletion later if needed. Unfortunately, that would require Gecko interface extension. That shouldn't be a problem since we call `remove_target_tasks` when unlinking/deleting it, which gets rid of the task, unless I'm missing something?
I mean I could call `remove_target_tasks` instead of incrementing `magic`, but it seems like pointless overhead because it will be called again anyway during unlink/delete. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3597#note_42733
On Fri Aug 18 12:59:57 2023 +0000, Gabriel Ivăncescu wrote:
That shouldn't be a problem since we call `remove_target_tasks` when unlinking/deleting it, which gets rid of the task, unless I'm missing something? I mean I could call `remove_target_tasks` instead of incrementing `magic`, but it seems like pointless overhead because it will be called again anyway during unlink/delete. What I mean is that if `This` is destroyed by `dispex_ref_decr`, then `This->magic++` operates on invalid memory. I guess that `remove_target_tasks` could work around it if you store the magic before calling `dispex_ref_decr`, but it's ugly...
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3597#note_42735
On Fri Aug 18 13:11:10 2023 +0000, Jacek Caban wrote:
What I mean is that if `This` is destroyed by `dispex_ref_decr`, then `This->magic++` operates on invalid memory. I guess that `remove_target_tasks` could work around it if you store the magic before calling `dispex_ref_decr`, but it's ugly... Ah right, I could check for ref being 0 when processing the task and skip it then, is that fine?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3597#note_42737
On Fri Aug 18 13:23:59 2023 +0000, Gabriel Ivăncescu wrote:
Ah right, I could check for ref being 0 when processing the task and skip it then, is that fine? Maybe, but we don't have ref value getter currently. I guess you we could hardcode `nsrefcnt` layout until the next Gecko release, I'm not sure. Let's take care of that in another MR. It would be good to move that commit closer to the end of conversion series so that we can see if there are more cases like that. Other patches look good to me.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3597#note_42738
participants (2)
-
Gabriel Ivăncescu -
Jacek Caban (＠jacek)