[PATCH 0/14] MR3482: mshtml: Implement Cycle Collection for all mshtml objects (dispex), part 2.
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlstorage.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/dlls/mshtml/htmlstorage.c b/dlls/mshtml/htmlstorage.c index da97809177b..cad2b2dbd32 100644 --- a/dlls/mshtml/htmlstorage.c +++ b/dlls/mshtml/htmlstorage.c @@ -397,14 +397,8 @@ static ULONG WINAPI HTMLStorage_Release(IHTMLStorage *iface) TRACE("(%p) ref=%ld\n", This, ref); - if(!ref) { - release_session_map_entry(This->session_storage); + if(!ref) release_dispex(&This->dispex); - free(This->filename); - CloseHandle(This->mutex); - release_props(This); - free(This); - } return ref; } @@ -1063,6 +1057,16 @@ static inline HTMLStorage *impl_from_DispatchEx(DispatchEx *iface) return CONTAINING_RECORD(iface, HTMLStorage, dispex); } +static void HTMLStorage_destructor(DispatchEx *dispex) +{ + HTMLStorage *This = impl_from_DispatchEx(dispex); + release_session_map_entry(This->session_storage); + free(This->filename); + CloseHandle(This->mutex); + release_props(This); + free(This); +} + static HRESULT check_item(HTMLStorage *This, const WCHAR *key) { struct session_entry *session_entry; @@ -1308,7 +1312,7 @@ static HRESULT HTMLStorage_next_dispid(DispatchEx *dispex, DISPID id, DISPID *pi } static const dispex_static_data_vtbl_t HTMLStorage_dispex_vtbl = { - NULL, + HTMLStorage_destructor, NULL, NULL, HTMLStorage_get_dispid, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3482
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/selection.c | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/dlls/mshtml/selection.c b/dlls/mshtml/selection.c index 03a330d3f30..cc9c0acece6 100644 --- a/dlls/mshtml/selection.c +++ b/dlls/mshtml/selection.c @@ -93,14 +93,8 @@ static ULONG WINAPI HTMLSelectionObject_Release(IHTMLSelectionObject *iface) TRACE("(%p) ref=%ld\n", This, ref); - if(!ref) { - if(This->nsselection) - nsISelection_Release(This->nsselection); - if(This->doc) - list_remove(&This->entry); + if(!ref) release_dispex(&This->dispex); - free(This); - } return ref; } @@ -329,6 +323,32 @@ static const IHTMLSelectionObject2Vtbl HTMLSelectionObject2Vtbl = { HTMLSelectionObject2_get_typeDetail }; +static inline HTMLSelectionObject *impl_from_DispatchEx(DispatchEx *iface) +{ + return CONTAINING_RECORD(iface, HTMLSelectionObject, dispex); +} + +static void HTMLSelectionObject_unlink(DispatchEx *dispex) +{ + HTMLSelectionObject *This = impl_from_DispatchEx(dispex); + unlink_ref(&This->nsselection); + if(This->doc) { + This->doc = NULL; + list_remove(&This->entry); + } +} + +static void HTMLSelectionObject_destructor(DispatchEx *dispex) +{ + HTMLSelectionObject *This = impl_from_DispatchEx(dispex); + free(This); +} + +static const dispex_static_data_vtbl_t HTMLSelectionObject_dispex_vtbl = { + HTMLSelectionObject_destructor, + HTMLSelectionObject_unlink +}; + static const tid_t HTMLSelectionObject_iface_tids[] = { IHTMLSelectionObject_tid, IHTMLSelectionObject2_tid, @@ -336,7 +356,7 @@ static const tid_t HTMLSelectionObject_iface_tids[] = { }; static dispex_static_data_t HTMLSelectionObject_dispex = { L"MSSelection", - NULL, + &HTMLSelectionObject_dispex_vtbl, IHTMLSelectionObject_tid, /* FIXME: We have a test for that, but it doesn't expose IHTMLSelectionObject2 iface. */ HTMLSelectionObject_iface_tids }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3482
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/range.c | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/dlls/mshtml/range.c b/dlls/mshtml/range.c index 1f062f5022a..d55851ac87e 100644 --- a/dlls/mshtml/range.c +++ b/dlls/mshtml/range.c @@ -1799,12 +1799,8 @@ static ULONG WINAPI HTMLDOMRange_Release(IHTMLDOMRange *iface) TRACE("(%p) ref=%ld\n", This, ref); - if(!ref) { - if(This->nsrange) - nsIDOMRange_Release(This->nsrange); + if(!ref) release_dispex(&This->dispex); - free(This); - } return ref; } @@ -2063,6 +2059,28 @@ static const IHTMLDOMRangeVtbl HTMLDOMRangeVtbl = { HTMLDOMRange_getBoundingClientRect, }; +static inline HTMLDOMRange *HTMLDOMRange_from_DispatchEx(DispatchEx *iface) +{ + return CONTAINING_RECORD(iface, HTMLDOMRange, dispex); +} + +static void HTMLDOMRange_unlink(DispatchEx *dispex) +{ + HTMLDOMRange *This = HTMLDOMRange_from_DispatchEx(dispex); + unlink_ref(&This->nsrange); +} + +static void HTMLDOMRange_destructor(DispatchEx *dispex) +{ + HTMLDOMRange *This = HTMLDOMRange_from_DispatchEx(dispex); + free(This); +} + +static const dispex_static_data_vtbl_t HTMLDOMRange_dispex_vtbl = { + HTMLDOMRange_destructor, + HTMLDOMRange_unlink +}; + static const tid_t HTMLDOMRange_iface_tids[] = { IHTMLDOMRange_tid, 0 @@ -2070,7 +2088,7 @@ static const tid_t HTMLDOMRange_iface_tids[] = { static dispex_static_data_t HTMLDOMRange_dispex = { L"Range", - NULL, + &HTMLDOMRange_dispex_vtbl, DispHTMLDOMRange_tid, HTMLDOMRange_iface_tids }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3482
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/range.c | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/dlls/mshtml/range.c b/dlls/mshtml/range.c index d55851ac87e..562dbf35b49 100644 --- a/dlls/mshtml/range.c +++ b/dlls/mshtml/range.c @@ -855,14 +855,8 @@ static ULONG WINAPI HTMLTxtRange_Release(IHTMLTxtRange *iface) TRACE("(%p) ref=%ld\n", This, ref); - if(!ref) { - if(This->nsrange) - nsIDOMRange_Release(This->nsrange); - if(This->doc) - list_remove(&This->entry); + if(!ref) release_dispex(&This->dispex); - free(This); - } return ref; } @@ -1718,13 +1712,39 @@ static const IOleCommandTargetVtbl OleCommandTargetVtbl = { RangeCommandTarget_Exec }; +static inline HTMLTxtRange *HTMLTxtRange_from_DispatchEx(DispatchEx *iface) +{ + return CONTAINING_RECORD(iface, HTMLTxtRange, dispex); +} + +static void HTMLTxtRange_unlink(DispatchEx *dispex) +{ + HTMLTxtRange *This = HTMLTxtRange_from_DispatchEx(dispex); + unlink_ref(&This->nsrange); + if(This->doc) { + This->doc = NULL; + list_remove(&This->entry); + } +} + +static void HTMLTxtRange_destructor(DispatchEx *dispex) +{ + HTMLTxtRange *This = HTMLTxtRange_from_DispatchEx(dispex); + free(This); +} + +static const dispex_static_data_vtbl_t HTMLTxtRange_dispex_vtbl = { + HTMLTxtRange_destructor, + HTMLTxtRange_unlink +}; + static const tid_t HTMLTxtRange_iface_tids[] = { IHTMLTxtRange_tid, 0 }; static dispex_static_data_t HTMLTxtRange_dispex = { L"TextRange", - NULL, + &HTMLTxtRange_dispex_vtbl, IHTMLTxtRange_tid, HTMLTxtRange_iface_tids }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3482
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmlattr.c | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/dlls/mshtml/htmlattr.c b/dlls/mshtml/htmlattr.c index b7e40a23d55..0323e1d8de1 100644 --- a/dlls/mshtml/htmlattr.c +++ b/dlls/mshtml/htmlattr.c @@ -79,13 +79,8 @@ static ULONG WINAPI HTMLDOMAttribute_Release(IHTMLDOMAttribute *iface) TRACE("(%p) ref=%ld\n", This, ref); - if(!ref) { - assert(!This->elem); + if(!ref) release_dispex(&This->dispex); - VariantClear(&This->value); - free(This->name); - free(This); - } return ref; } @@ -480,6 +475,30 @@ static const IHTMLDOMAttribute2Vtbl HTMLDOMAttribute2Vtbl = { HTMLDOMAttribute2_cloneNode }; +static inline HTMLDOMAttribute *impl_from_DispatchEx(DispatchEx *iface) +{ + return CONTAINING_RECORD(iface, HTMLDOMAttribute, dispex); +} + +static void HTMLDOMAttribute_unlink(DispatchEx *dispex) +{ + HTMLDOMAttribute *This = impl_from_DispatchEx(dispex); + VariantClear(&This->value); +} + +static void HTMLDOMAttribute_destructor(DispatchEx *dispex) +{ + HTMLDOMAttribute *This = impl_from_DispatchEx(dispex); + assert(!This->elem); + free(This->name); + free(This); +} + +static const dispex_static_data_vtbl_t HTMLDOMAttribute_dispex_vtbl = { + HTMLDOMAttribute_destructor, + HTMLDOMAttribute_unlink +}; + static const tid_t HTMLDOMAttribute_iface_tids[] = { IHTMLDOMAttribute_tid, IHTMLDOMAttribute2_tid, @@ -487,7 +506,7 @@ static const tid_t HTMLDOMAttribute_iface_tids[] = { }; static dispex_static_data_t HTMLDOMAttribute_dispex = { L"Attr", - NULL, + &HTMLDOMAttribute_dispex_vtbl, DispHTMLDOMAttribute_tid, HTMLDOMAttribute_iface_tids }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3482
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/omnavigator.c | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/dlls/mshtml/omnavigator.c b/dlls/mshtml/omnavigator.c index 298d99f86eb..8b6a9d76db6 100644 --- a/dlls/mshtml/omnavigator.c +++ b/dlls/mshtml/omnavigator.c @@ -99,13 +99,8 @@ static ULONG WINAPI HTMLDOMImplementation_Release(IHTMLDOMImplementation *iface) TRACE("(%p) ref=%ld\n", This, ref); - if(!ref) { - assert(!This->browser); - if(This->implementation) - nsIDOMDOMImplementation_Release(This->implementation); + if(!ref) release_dispex(&This->dispex); - free(This); - } return ref; } @@ -294,6 +289,29 @@ static const IHTMLDOMImplementation2Vtbl HTMLDOMImplementation2Vtbl = { HTMLDOMImplementation2_hasFeature }; +static inline HTMLDOMImplementation *HTMLDOMImplementation_from_DispatchEx(DispatchEx *iface) +{ + return CONTAINING_RECORD(iface, HTMLDOMImplementation, dispex); +} + +static void HTMLDOMImplementation_unlink(DispatchEx *dispex) +{ + HTMLDOMImplementation *This = HTMLDOMImplementation_from_DispatchEx(dispex); + unlink_ref(&This->implementation); +} + +static void HTMLDOMImplementation_destructor(DispatchEx *dispex) +{ + HTMLDOMImplementation *This = HTMLDOMImplementation_from_DispatchEx(dispex); + assert(!This->browser); + free(This); +} + +static const dispex_static_data_vtbl_t HTMLDOMImplementation_dispex_vtbl = { + HTMLDOMImplementation_destructor, + HTMLDOMImplementation_unlink +}; + static void HTMLDOMImplementation_init_dispex_info(dispex_data_t *info, compat_mode_t compat_mode) { if(compat_mode >= COMPAT_MODE_IE9) @@ -306,7 +324,7 @@ static const tid_t HTMLDOMImplementation_iface_tids[] = { }; static dispex_static_data_t HTMLDOMImplementation_dispex = { L"DOMImplementation", - NULL, + &HTMLDOMImplementation_dispex_vtbl, DispHTMLDOMImplementation_tid, HTMLDOMImplementation_iface_tids, HTMLDOMImplementation_init_dispex_info -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3482
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/omnavigator.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/dlls/mshtml/omnavigator.c b/dlls/mshtml/omnavigator.c index 8b6a9d76db6..b3dbcb8020b 100644 --- a/dlls/mshtml/omnavigator.c +++ b/dlls/mshtml/omnavigator.c @@ -418,10 +418,8 @@ static ULONG WINAPI HTMLScreen_Release(IHTMLScreen *iface) TRACE("(%p) ref=%ld\n", This, ref); - if(!ref) { + if(!ref) release_dispex(&This->dispex); - free(This); - } return ref; } @@ -571,13 +569,28 @@ static const IHTMLScreenVtbl HTMLSreenVtbl = { HTMLScreen_get_fontSmoothingEnabled }; +static inline HTMLScreen *HTMLScreen_from_DispatchEx(DispatchEx *iface) +{ + return CONTAINING_RECORD(iface, HTMLScreen, dispex); +} + +static void HTMLScreen_destructor(DispatchEx *dispex) +{ + HTMLScreen *This = HTMLScreen_from_DispatchEx(dispex); + free(This); +} + +static const dispex_static_data_vtbl_t HTMLScreen_dispex_vtbl = { + HTMLScreen_destructor, +}; + static const tid_t HTMLScreen_iface_tids[] = { IHTMLScreen_tid, 0 }; static dispex_static_data_t HTMLScreen_dispex = { L"Screen", - NULL, + &HTMLScreen_dispex_vtbl, DispHTMLScreen_tid, HTMLScreen_iface_tids }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3482
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/omnavigator.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/dlls/mshtml/omnavigator.c b/dlls/mshtml/omnavigator.c index b3dbcb8020b..4de420c6961 100644 --- a/dlls/mshtml/omnavigator.c +++ b/dlls/mshtml/omnavigator.c @@ -656,10 +656,8 @@ static ULONG WINAPI OmHistory_Release(IOmHistory *iface) TRACE("(%p) ref=%ld\n", This, ref); - if(!ref) { + if(!ref) release_dispex(&This->dispex); - free(This); - } return ref; } @@ -748,13 +746,28 @@ static const IOmHistoryVtbl OmHistoryVtbl = { OmHistory_go }; +static inline OmHistory *OmHistory_from_DispatchEx(DispatchEx *iface) +{ + return CONTAINING_RECORD(iface, OmHistory, dispex); +} + +static void OmHistory_destructor(DispatchEx *dispex) +{ + OmHistory *This = OmHistory_from_DispatchEx(dispex); + free(This); +} + +static const dispex_static_data_vtbl_t OmHistory_dispex_vtbl = { + OmHistory_destructor, +}; + static const tid_t OmHistory_iface_tids[] = { IOmHistory_tid, 0 }; static dispex_static_data_t OmHistory_dispex = { L"History", - NULL, + &OmHistory_dispex_vtbl, DispHTMLHistory_tid, OmHistory_iface_tids }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3482
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/omnavigator.c | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/dlls/mshtml/omnavigator.c b/dlls/mshtml/omnavigator.c index 4de420c6961..076589a20b4 100644 --- a/dlls/mshtml/omnavigator.c +++ b/dlls/mshtml/omnavigator.c @@ -845,12 +845,8 @@ static ULONG WINAPI HTMLPluginsCollection_Release(IHTMLPluginsCollection *iface) TRACE("(%p) ref=%ld\n", This, ref); - if(!ref) { - if(This->navigator) - This->navigator->plugins = NULL; + if(!ref) release_dispex(&This->dispex); - free(This); - } return ref; } @@ -918,13 +914,38 @@ static const IHTMLPluginsCollectionVtbl HTMLPluginsCollectionVtbl = { HTMLPluginsCollection_refresh }; +static inline HTMLPluginsCollection *HTMLPluginsCollection_from_DispatchEx(DispatchEx *iface) +{ + return CONTAINING_RECORD(iface, HTMLPluginsCollection, dispex); +} + +static void HTMLPluginsCollection_unlink(DispatchEx *dispex) +{ + HTMLPluginsCollection *This = HTMLPluginsCollection_from_DispatchEx(dispex); + if(This->navigator) { + This->navigator->plugins = NULL; + This->navigator = NULL; + } +} + +static void HTMLPluginsCollection_destructor(DispatchEx *dispex) +{ + HTMLPluginsCollection *This = HTMLPluginsCollection_from_DispatchEx(dispex); + free(This); +} + +static const dispex_static_data_vtbl_t HTMLPluginsCollection_dispex_vtbl = { + HTMLPluginsCollection_destructor, + HTMLPluginsCollection_unlink +}; + static const tid_t HTMLPluginsCollection_iface_tids[] = { IHTMLPluginsCollection_tid, 0 }; static dispex_static_data_t HTMLPluginsCollection_dispex = { L"PluginArray", - NULL, + &HTMLPluginsCollection_dispex_vtbl, DispCPlugins_tid, HTMLPluginsCollection_iface_tids }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3482
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/omnavigator.c | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/dlls/mshtml/omnavigator.c b/dlls/mshtml/omnavigator.c index 076589a20b4..6f2b2ab8524 100644 --- a/dlls/mshtml/omnavigator.c +++ b/dlls/mshtml/omnavigator.c @@ -1022,12 +1022,8 @@ static ULONG WINAPI HTMLMimeTypesCollection_Release(IHTMLMimeTypesCollection *if TRACE("(%p) ref=%ld\n", This, ref); - if(!ref) { - if(This->navigator) - This->navigator->mime_types = NULL; + if(!ref) release_dispex(&This->dispex); - free(This); - } return ref; } @@ -1084,13 +1080,38 @@ static const IHTMLMimeTypesCollectionVtbl HTMLMimeTypesCollectionVtbl = { HTMLMimeTypesCollection_get_length }; +static inline HTMLMimeTypesCollection *HTMLMimeTypesCollection_from_DispatchEx(DispatchEx *iface) +{ + return CONTAINING_RECORD(iface, HTMLMimeTypesCollection, dispex); +} + +static void HTMLMimeTypesCollection_unlink(DispatchEx *dispex) +{ + HTMLMimeTypesCollection *This = HTMLMimeTypesCollection_from_DispatchEx(dispex); + if(This->navigator) { + This->navigator->mime_types = NULL; + This->navigator = NULL; + } +} + +static void HTMLMimeTypesCollection_destructor(DispatchEx *dispex) +{ + HTMLMimeTypesCollection *This = HTMLMimeTypesCollection_from_DispatchEx(dispex); + free(This); +} + +static const dispex_static_data_vtbl_t HTMLMimeTypesCollection_dispex_vtbl = { + HTMLMimeTypesCollection_destructor, + HTMLMimeTypesCollection_unlink +}; + static const tid_t HTMLMimeTypesCollection_iface_tids[] = { IHTMLMimeTypesCollection_tid, 0 }; static dispex_static_data_t HTMLMimeTypesCollection_dispex = { L"MimeTypeArray", - NULL, + &HTMLMimeTypesCollection_dispex_vtbl, IHTMLMimeTypesCollection_tid, HTMLMimeTypesCollection_iface_tids }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3482
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/omnavigator.c | 39 +++++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/dlls/mshtml/omnavigator.c b/dlls/mshtml/omnavigator.c index 6f2b2ab8524..b4fd0f0540b 100644 --- a/dlls/mshtml/omnavigator.c +++ b/dlls/mshtml/omnavigator.c @@ -1179,14 +1179,8 @@ static ULONG WINAPI OmNavigator_Release(IOmNavigator *iface) TRACE("(%p) ref=%ld\n", This, ref); - if(!ref) { - if(This->plugins) - This->plugins->navigator = NULL; - if(This->mime_types) - This->mime_types->navigator = NULL; + if(!ref) release_dispex(&This->dispex); - free(This); - } return ref; } @@ -1533,13 +1527,42 @@ static const IOmNavigatorVtbl OmNavigatorVtbl = { OmNavigator_get_userProfile }; +static inline OmNavigator *OmNavigator_from_DispatchEx(DispatchEx *iface) +{ + return CONTAINING_RECORD(iface, OmNavigator, dispex); +} + +static void OmNavigator_unlink(DispatchEx *dispex) +{ + OmNavigator *This = OmNavigator_from_DispatchEx(dispex); + if(This->plugins) { + This->plugins->navigator = NULL; + This->plugins = NULL; + } + if(This->mime_types) { + This->mime_types->navigator = NULL; + This->mime_types = NULL; + } +} + +static void OmNavigator_destructor(DispatchEx *dispex) +{ + OmNavigator *This = OmNavigator_from_DispatchEx(dispex); + free(This); +} + +static const dispex_static_data_vtbl_t OmNavigator_dispex_vtbl = { + OmNavigator_destructor, + OmNavigator_unlink +}; + static const tid_t OmNavigator_iface_tids[] = { IOmNavigator_tid, 0 }; static dispex_static_data_t OmNavigator_dispex = { L"Navigator", - NULL, + &OmNavigator_dispex_vtbl, DispHTMLNavigator_tid, OmNavigator_iface_tids }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3482
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/omnavigator.c | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/dlls/mshtml/omnavigator.c b/dlls/mshtml/omnavigator.c index b4fd0f0540b..fe82a1942dc 100644 --- a/dlls/mshtml/omnavigator.c +++ b/dlls/mshtml/omnavigator.c @@ -2035,11 +2035,8 @@ static ULONG WINAPI HTMLPerformanceNavigation_Release(IHTMLPerformanceNavigation TRACE("(%p) ref=%ld\n", This, ref); - if(!ref) { - IHTMLPerformanceTiming_Release(&This->timing->IHTMLPerformanceTiming_iface); + if(!ref) release_dispex(&This->dispex); - free(This); - } return ref; } @@ -2129,13 +2126,39 @@ static const IHTMLPerformanceNavigationVtbl HTMLPerformanceNavigationVtbl = { HTMLPerformanceNavigation_toJSON }; +static inline HTMLPerformanceNavigation *HTMLPerformanceNavigation_from_DispatchEx(DispatchEx *iface) +{ + return CONTAINING_RECORD(iface, HTMLPerformanceNavigation, dispex); +} + +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); + } +} + +static void HTMLPerformanceNavigation_destructor(DispatchEx *dispex) +{ + HTMLPerformanceNavigation *This = HTMLPerformanceNavigation_from_DispatchEx(dispex); + free(This); +} + +static const dispex_static_data_vtbl_t HTMLPerformanceNavigation_dispex_vtbl = { + HTMLPerformanceNavigation_destructor, + HTMLPerformanceNavigation_unlink +}; + static const tid_t HTMLPerformanceNavigation_iface_tids[] = { IHTMLPerformanceNavigation_tid, 0 }; static dispex_static_data_t HTMLPerformanceNavigation_dispex = { L"PerformanceNavigation", - NULL, + &HTMLPerformanceNavigation_dispex_vtbl, IHTMLPerformanceNavigation_tid, HTMLPerformanceNavigation_iface_tids }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3482
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/omnavigator.c | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/dlls/mshtml/omnavigator.c b/dlls/mshtml/omnavigator.c index fe82a1942dc..d0af5e0de02 100644 --- a/dlls/mshtml/omnavigator.c +++ b/dlls/mshtml/omnavigator.c @@ -2217,13 +2217,8 @@ static ULONG WINAPI HTMLPerformance_Release(IHTMLPerformance *iface) TRACE("(%p) ref=%ld\n", This, ref); - if(!ref) { - IHTMLPerformanceTiming_Release(&This->timing->IHTMLPerformanceTiming_iface); - if(This->navigation) - IHTMLPerformanceNavigation_Release(This->navigation); + if(!ref) release_dispex(&This->dispex); - free(This); - } return ref; } @@ -2332,13 +2327,40 @@ static const IHTMLPerformanceVtbl HTMLPerformanceVtbl = { HTMLPerformance_toJSON }; +static inline HTMLPerformance *HTMLPerformance_from_DispatchEx(DispatchEx *iface) +{ + return CONTAINING_RECORD(iface, HTMLPerformance, dispex); +} + +static void HTMLPerformance_unlink(DispatchEx *dispex) +{ + HTMLPerformance *This = HTMLPerformance_from_DispatchEx(dispex); + unlink_ref(&This->navigation); + if(This->timing) { + HTMLPerformanceTiming *timing = This->timing; + This->timing = NULL; + IHTMLPerformanceTiming_Release(&timing->IHTMLPerformanceTiming_iface); + } +} + +static void HTMLPerformance_destructor(DispatchEx *dispex) +{ + HTMLPerformance *This = HTMLPerformance_from_DispatchEx(dispex); + free(This); +} + +static const dispex_static_data_vtbl_t HTMLPerformance_dispex_vtbl = { + HTMLPerformance_destructor, + HTMLPerformance_unlink +}; + static const tid_t HTMLPerformance_iface_tids[] = { IHTMLPerformance_tid, 0 }; static dispex_static_data_t HTMLPerformance_dispex = { L"Performance", - NULL, + &HTMLPerformance_dispex_vtbl, IHTMLPerformance_tid, HTMLPerformance_iface_tids }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3482
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/omnavigator.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/dlls/mshtml/omnavigator.c b/dlls/mshtml/omnavigator.c index d0af5e0de02..f6cf0755a74 100644 --- a/dlls/mshtml/omnavigator.c +++ b/dlls/mshtml/omnavigator.c @@ -2441,10 +2441,8 @@ static ULONG WINAPI HTMLNamespaceCollection_Release(IHTMLNamespaceCollection *if TRACE("(%p) ref=%ld\n", This, ref); - if(!ref) { + if(!ref) release_dispex(&This->dispex); - free(This); - } return ref; } @@ -2520,13 +2518,28 @@ static const IHTMLNamespaceCollectionVtbl HTMLNamespaceCollectionVtbl = { HTMLNamespaceCollection_add }; +static inline HTMLNamespaceCollection *HTMLNamespaceCollection_from_DispatchEx(DispatchEx *iface) +{ + return CONTAINING_RECORD(iface, HTMLNamespaceCollection, dispex); +} + +static void HTMLNamespaceCollection_destructor(DispatchEx *dispex) +{ + HTMLNamespaceCollection *This = HTMLNamespaceCollection_from_DispatchEx(dispex); + free(This); +} + +static const dispex_static_data_vtbl_t HTMLNamespaceCollection_dispex_vtbl = { + HTMLNamespaceCollection_destructor, +}; + static const tid_t HTMLNamespaceCollection_iface_tids[] = { IHTMLNamespaceCollection_tid, 0 }; static dispex_static_data_t HTMLNamespaceCollection_dispex = { L"MSNamespaceInfoCollection", - NULL, + &HTMLNamespaceCollection_dispex_vtbl, DispHTMLNamespaceCollection_tid, HTMLNamespaceCollection_iface_tids }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3482
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=135549 Your paranoid android. === debian11 (build log) === error: patch failed: dlls/mshtml/htmlstorage.c:1308 Task: Patch failed to apply === debian11b (build log) === error: patch failed: dlls/mshtml/htmlstorage.c:1308 Task: Patch failed to apply
Jacek Caban (@jacek) commented about dlls/mshtml/htmlattr.c:
HTMLDOMAttribute2_cloneNode };
+static inline HTMLDOMAttribute *impl_from_DispatchEx(DispatchEx *iface) +{ + return CONTAINING_RECORD(iface, HTMLDOMAttribute, dispex); +} + +static void HTMLDOMAttribute_unlink(DispatchEx *dispex) +{ + HTMLDOMAttribute *This = impl_from_DispatchEx(dispex); + VariantClear(&This->value);
When unlink will be initiated by CC, you can't assume that there are no more refs, only that CC expects that after unlinking there will be no more refs. Since we don't control value's destructor, the object should be in a consistent state while calling it. For all we know it may call `get_nodeValue`. In practice we don't care what exactly we do in such an obscure case, but returning a dispatch that we don't own ref to is never right. We could instead have a helper like `unlink_variant` which would just do something like `if(V_VT(v) == VT_DISPATCH && V_DISPATCH(v)) unlink_ref(&V_DISPATCH(v));` and leave `VariantClear` for the destructor. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3482#note_41074
On Wed Aug 2 15:35:54 2023 +0000, Jacek Caban wrote:
When unlink will be initiated by CC, you can't assume that there are no more refs, only that CC expects that after unlinking there will be no more refs. Since we don't control value's destructor, the object should be in a consistent state while calling it. For all we know it may call `get_nodeValue`. In practice we don't care what exactly we do in such an obscure case, but returning a dispatch that we don't own ref to is never right. We could instead have a helper like `unlink_variant` which would just do something like `if(V_VT(v) == VT_DISPATCH && V_DISPATCH(v)) unlink_ref(&V_DISPATCH(v));` and leave `VariantClear` for the destructor. In that case, `unlink_ref` isn't going to work either because it NULLs it out, and that would return a NULL VT_DISPATCH in get_nodeValue, which is not exactly ideal, unless I'm missing something?
Probably it would need to be something like: ```c if(V_VT(v) != VT_DISPATCH) { VariantClear(v); return; } V_VT(v) = VT_EMPTY; if(V_DISPATCH(v)) IDispatch_Release(V_DISPATCH(v)); ``` Basically like VariantClear, except it puts VT_EMPTY first before releasing, what do you think? Actually in this case I should do same with VT_UNKNOWN. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3482#note_41098
On Wed Aug 2 17:46:59 2023 +0000, Gabriel Ivăncescu wrote:
In that case, `unlink_ref` isn't going to work either because it NULLs it out, and that would return a NULL VT_DISPATCH in get_nodeValue, which is not exactly ideal, unless I'm missing something? Probably it would need to be something like: ```c if(V_VT(v) != VT_DISPATCH) { VariantClear(v); return; } V_VT(v) = VT_EMPTY; if(V_DISPATCH(v)) IDispatch_Release(V_DISPATCH(v)); ``` Basically like VariantClear, except it puts VT_EMPTY first before releasing, what do you think? Actually in this case I should do same with VT_UNKNOWN. Or I could simplify it to:
VARIANT tmp = *v;
V_VT(v) = VT_EMPTY;
VariantClear(&tmp);
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3482#note_41100
On Wed Aug 2 17:48:59 2023 +0000, Gabriel Ivăncescu wrote:
Or I could simplify it to: ```c VARIANT tmp = *v; V_VT(v) = VT_EMPTY; VariantClear(&tmp); ``` I think that it's generally a good strategy to free things that are not participating in CC graph in destructor instead of unlink. It's just safer and we don't have similar considerations there. I'm not sure how much we need to worry about VT_UNKNOWN, but I imagine that we may have something like `traverse_variant` and in that case we'd just have to make sure that those two are consistent. For all corner cases like safe arrays of dispatches (that we're unlikely to care about in traversal), destructor is safe, so a simple `VariantClear` there is enough.
NULL VT_DISPATCH is not really wrong, such variants are used and we need to worry about them already. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3482#note_41106
participants (3)
-
Gabriel Ivăncescu -
Jacek Caban (＠jacek)
-
Marvin