[PATCH 0/3] MR4024: uiautomationcore: COM focus change event handler event advisement support.
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/uiautomationcore/tests/uiautomation.c | 396 ++++++++++++++++++++- 1 file changed, 387 insertions(+), 9 deletions(-) diff --git a/dlls/uiautomationcore/tests/uiautomation.c b/dlls/uiautomationcore/tests/uiautomation.c index 6cdc19b739d..d5f8bb492a4 100644 --- a/dlls/uiautomationcore/tests/uiautomation.c +++ b/dlls/uiautomationcore/tests/uiautomation.c @@ -8785,15 +8785,149 @@ static SAFEARRAY WINAPI *test_uia_provider_callback(HWND hwnd, enum ProviderType return get_safearray_for_elprov(elprov); } +/* + * Windows 11 will infinitely loop if a clientside provider isn't returned for + * an HWND in certain circumstances. In order to prevent this, we use these + * temporary clientside providers. + */ +struct ClientSideProvider +{ + IRawElementProviderSimple IRawElementProviderSimple_iface; + LONG ref; + + enum ProviderType prov_type; + HWND hwnd; +}; + +static inline struct ClientSideProvider *impl_from_ClientSideProvider(IRawElementProviderSimple *iface) +{ + return CONTAINING_RECORD(iface, struct ClientSideProvider, IRawElementProviderSimple_iface); +} + +static HRESULT WINAPI ClientSideProvider_QueryInterface(IRawElementProviderSimple *iface, REFIID riid, void **ppv) +{ + *ppv = NULL; + if (IsEqualIID(riid, &IID_IRawElementProviderSimple) || IsEqualIID(riid, &IID_IUnknown)) + *ppv = iface; + else + return E_NOINTERFACE; + + IRawElementProviderSimple_AddRef(iface); + return S_OK; +} + +static ULONG WINAPI ClientSideProvider_AddRef(IRawElementProviderSimple *iface) +{ + struct ClientSideProvider *This = impl_from_ClientSideProvider(iface); + return InterlockedIncrement(&This->ref); +} + +static ULONG WINAPI ClientSideProvider_Release(IRawElementProviderSimple *iface) +{ + struct ClientSideProvider *This = impl_from_ClientSideProvider(iface); + ULONG ref = InterlockedDecrement(&This->ref); + + if (!ref) + HeapFree(GetProcessHeap(), 0, This); + + return ref; +} + +static HRESULT WINAPI ClientSideProvider_get_ProviderOptions(IRawElementProviderSimple *iface, + enum ProviderOptions *ret_val) +{ + struct ClientSideProvider *This = impl_from_ClientSideProvider(iface); + + *ret_val = ProviderOptions_ClientSideProvider; + if (This->prov_type == ProviderType_NonClientArea) + *ret_val |= ProviderOptions_NonClientAreaProvider; + + return S_OK; +} + +static HRESULT WINAPI ClientSideProvider_GetPatternProvider(IRawElementProviderSimple *iface, + PATTERNID pattern_id, IUnknown **ret_val) +{ + *ret_val = NULL; + return S_OK; +} + +static HRESULT WINAPI ClientSideProvider_GetPropertyValue(IRawElementProviderSimple *iface, PROPERTYID prop_id, + VARIANT *ret_val) +{ + struct ClientSideProvider *This = impl_from_ClientSideProvider(iface); + + VariantInit(ret_val); + switch (prop_id) + { + case UIA_NativeWindowHandlePropertyId: + if (This->prov_type == ProviderType_BaseHwnd) + { + V_VT(ret_val) = VT_I4; + V_I4(ret_val) = HandleToULong(This->hwnd); + } + break; + + case UIA_ProviderDescriptionPropertyId: + V_VT(ret_val) = VT_BSTR; + V_BSTR(ret_val) = SysAllocString(L"ClientSideProvider"); + break; + + default: + break; + } + + return S_OK; +} + +static HRESULT WINAPI ClientSideProvider_get_HostRawElementProvider(IRawElementProviderSimple *iface, + IRawElementProviderSimple **ret_val) +{ + struct ClientSideProvider *This = impl_from_ClientSideProvider(iface); + + return UiaHostProviderFromHwnd(This->hwnd, ret_val); +} + +static IRawElementProviderSimpleVtbl ClientSideProviderVtbl = { + ClientSideProvider_QueryInterface, + ClientSideProvider_AddRef, + ClientSideProvider_Release, + ClientSideProvider_get_ProviderOptions, + ClientSideProvider_GetPatternProvider, + ClientSideProvider_GetPropertyValue, + ClientSideProvider_get_HostRawElementProvider, +}; + +static IRawElementProviderSimple *create_temporary_clientside_provider(HWND hwnd, enum ProviderType prov_type) +{ + struct ClientSideProvider *prov = HeapAlloc(GetProcessHeap(), 0, sizeof(*prov)); + + if (!prov) + { + trace("Failed to allocate memory for temporary clientside provider\n"); + return NULL; + } + + prov->IRawElementProviderSimple_iface.lpVtbl = &ClientSideProviderVtbl; + prov->ref = 1; + prov->prov_type = prov_type; + prov->hwnd = hwnd; + + return &prov->IRawElementProviderSimple_iface; +} + /* * Same deal as test_uia_provider_callback, except we only return a provider * if we can match one by HWND. This is necessary due to certain versions of * Windows 10 unpredictably attempting to create elements in the background. */ +static DWORD last_clientside_callback_time; static SAFEARRAY WINAPI *uia_com_win_event_clientside_provider_callback(HWND hwnd, enum ProviderType prov_type) { IRawElementProviderSimple *elprov = NULL; + SAFEARRAY *sa = NULL; + last_clientside_callback_time = GetTickCount(); switch (prov_type) { case ProviderType_BaseHwnd: @@ -8829,7 +8963,56 @@ static SAFEARRAY WINAPI *uia_com_win_event_clientside_provider_callback(HWND hwn break; } - return get_safearray_for_elprov(elprov); + if (!elprov) + { + if (!(elprov = create_temporary_clientside_provider(hwnd, prov_type))) + return NULL; + } + else + IRawElementProviderSimple_AddRef(elprov); + + sa = get_safearray_for_elprov(elprov); + IRawElementProviderSimple_Release(elprov); + return sa; +} + +/* + * Some versions of Windows 10 query multiple unrelated HWNDs when + * winevents are fired, this function waits for these to stop before + * continuing to the next test. + */ +#define TIME_SINCE_LAST_CALLBACK_TIMEOUT 200 +static BOOL wait_for_clientside_callbacks(DWORD total_timeout) +{ + DWORD prev_last_callback_time, cur_time = 0; + DWORD start_time = GetTickCount(); + BOOL ret; + + if (last_clientside_callback_time < start_time) + prev_last_callback_time = start_time; + else + prev_last_callback_time = last_clientside_callback_time; + + cur_time = start_time; + while (1) + { + Sleep(50); + + cur_time = GetTickCount(); + if (prev_last_callback_time < last_clientside_callback_time) + prev_last_callback_time = last_clientside_callback_time; + + if ((cur_time - start_time) >= total_timeout) + ret = TRUE; + else if ((cur_time - prev_last_callback_time) >= TIME_SINCE_LAST_CALLBACK_TIMEOUT) + ret = FALSE; + else + continue; + + break; + } + + return ret; } static void test_UiaRegisterProviderCallback(void) @@ -9393,6 +9576,14 @@ static void test_cache_req_sa_(SAFEARRAY *sa, LONG exp_lbound[2], LONG exp_elems } } +#define test_provider_event_advise_added( prov, event_id, todo) \ + test_provider_event_advise_added_( (prov), (event_id), (todo), __FILE__, __LINE__) +static void test_provider_event_advise_added_(struct Provider *prov, int event_id, BOOL todo, const char *file, int line) +{ + todo_wine_if (todo) ok_(file, line)(prov->advise_events_added_event_id == event_id, "%s: Unexpected advise event added, event ID %d.\n", + prov->prov_name, prov->advise_events_added_event_id); +} + static const struct prov_method_sequence cache_req_seq1[] = { { &Provider, PROV_GET_PROPERTY_VALUE }, /* UIA_ProviderDescriptionPropertyId. */ { 0 } @@ -14864,8 +15055,43 @@ struct com_win_event_test_thread_data HWND test_child_hwnd; }; +static void set_method_event_handle_for_providers(struct Provider *main, struct Provider *hwnd, struct Provider *nc, + HANDLE *handles, int method) +{ + if (!handles || method < 0) + { + set_provider_method_event_data(main, NULL, -1); + set_provider_method_event_data(hwnd, NULL, -1); + set_provider_method_event_data(nc, NULL, -1); + } + else + { + set_provider_method_event_data(main, handles[0], method); + set_provider_method_event_data(hwnd, handles[1], method); + set_provider_method_event_data(nc, handles[2], method); + } +} + +static void reset_event_advise_values_for_hwnd_providers(struct Provider *main, struct Provider *hwnd, struct Provider *nc) +{ + initialize_provider_advise_events_ids(main); + initialize_provider_advise_events_ids(hwnd); + initialize_provider_advise_events_ids(nc); +} + +#define test_hwnd_providers_event_advise_added( main, hwnd, nc, event_id, todo) \ + test_hwnd_providers_event_advise_added_( (main), (hwnd), (nc), (event_id), (todo), __FILE__, __LINE__) +static void test_hwnd_providers_event_advise_added_(struct Provider *main, struct Provider *hwnd, struct Provider *nc, + int event_id, BOOL todo, const char *file, int line) +{ + test_provider_event_advise_added_(main, event_id, todo, file, line); + test_provider_event_advise_added_(hwnd, event_id, todo, file, line); + test_provider_event_advise_added_(nc, event_id, todo, file, line); +} + static void test_uia_com_event_handler_event_advisement(IUIAutomation *uia_iface, HWND test_hwnd, HWND test_child_hwnd) { + GUITHREADINFO info = { .cbSize = sizeof(info) }; IUIAutomationElement *elem; HANDLE method_event[4]; int event_handle_count; @@ -15079,6 +15305,166 @@ static void test_uia_com_event_handler_event_advisement(IUIAutomation *uia_iface if (is_win11) set_provider_method_event_data(&Provider2, NULL, -1); + /* + * IUIAutomationFocusChangedEventHandler is treated differently than all + * other event handlers - it only advises providers of events the first + * time it encounters an EVENT_OBJECT_FOCUS WinEvent for an HWND. + */ + reset_event_advise_values_for_hwnd_providers(&Provider_proxy, &Provider_hwnd, &Provider_nc); + reset_event_advise_values_for_hwnd_providers(&Provider, &Provider_hwnd2, &Provider_nc2); + reset_event_advise_values_for_hwnd_providers(&Provider2, &Provider_hwnd3, &Provider_nc3); + + set_uia_hwnd_expects(0, 6, 6, 5, 0); + SET_EXPECT(child_winproc_GETOBJECT_UiaRoot); /* Only sent on Win11. */ + FocusChangedHandler.event_handler_added = FALSE; + hr = IUIAutomation_AddFocusChangedEventHandler(uia_iface, NULL, + &FocusChangedHandler.IUIAutomationFocusChangedEventHandler_iface); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(FocusChangedHandler.ref > 1, "Unexpected refcnt %ld\n", FocusChangedHandler.ref); + check_uia_hwnd_expects_at_most(0, 6, 6, 5, 0); + CHECK_CALLED_AT_MOST(child_winproc_GETOBJECT_UiaRoot, 1); + + /* + * Windows 10 version 1507 and below only advise the currently focused + * HWNDs providers of events being added - newer versions instead advise + * the desktop HWND's providers of events being added. We're matching the + * newer behavior, so skip the tests on older versions. + */ + if (!Provider_nc.advise_events_added_event_id && !Provider_hwnd.advise_events_added_event_id && + !Provider_proxy.advise_events_added_event_id) + { + win_skip("Win10v1507 and below advise the currently focused HWND and not the desktop HWND, skipping tests.\n"); + + test_hwnd_providers_event_advise_added(&Provider, &Provider_hwnd2, &Provider_nc2, UIA_AutomationFocusChangedEventId, FALSE); + set_uia_hwnd_expects(0, 1, 1, 0, 0); + hr = IUIAutomation_RemoveFocusChangedEventHandler(uia_iface, + &FocusChangedHandler.IUIAutomationFocusChangedEventHandler_iface); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + check_uia_hwnd_expects(0, FALSE, 1, FALSE, 1, FALSE, 0, FALSE, 0, FALSE); + goto exit; + } + + test_hwnd_providers_event_advise_added(&Provider_proxy, &Provider_hwnd, &Provider_nc, UIA_AutomationFocusChangedEventId, FALSE); + + /* + * EVENT_OBJECT_SHOW doesn't advise events on anything other than the + * desktop HWND for focus changed event handlers. + */ + event_handle_count = 3; + set_uia_hwnd_expects(0, 1, 1, 2, 0); /* Win11 sends WM_GETOBJECT twice. */ + set_method_event_handle_for_providers(&Provider, &Provider_hwnd2, &Provider_nc2, method_event, ADVISE_EVENTS_EVENT_ADDED); + NotifyWinEvent(EVENT_OBJECT_SHOW, test_hwnd, OBJID_WINDOW, CHILDID_SELF); + ok(msg_wait_for_all_events(method_event, event_handle_count, 750) == WAIT_TIMEOUT, "Wait for method_event(s) didn't time out.\n"); + if (wait_for_clientside_callbacks(2000)) trace("Kept getting callbacks up until timeout\n"); + check_uia_hwnd_expects_at_most(0, 1, 1, 2, 0); + set_method_event_handle_for_providers(&Provider, &Provider_hwnd2, &Provider_nc2, NULL, -1); + + set_method_event_handle_for_providers(&Provider2, &Provider_hwnd3, &Provider_nc3, method_event, ADVISE_EVENTS_EVENT_ADDED); + SET_EXPECT_MULTI(child_winproc_GETOBJECT_UiaRoot, 2); /* Only done twice on Win11. */ + set_uia_hwnd_expects(0, 1, 1, 1, 0); + NotifyWinEvent(EVENT_OBJECT_SHOW, test_child_hwnd, OBJID_WINDOW, CHILDID_SELF); + ok(msg_wait_for_all_events(method_event, event_handle_count, 750) == WAIT_TIMEOUT, "Wait for method_event(s) didn't time out.\n"); + if (wait_for_clientside_callbacks(2000)) trace("Kept getting callbacks up until timeout\n"); + set_method_event_handle_for_providers(&Provider2, &Provider_hwnd3, &Provider_nc3, NULL, -1); + check_uia_hwnd_expects(0, FALSE, 1, FALSE, 1, FALSE, 1, TRUE, 0, FALSE); + CHECK_CALLED_AT_MOST(child_winproc_GETOBJECT_UiaRoot, 2); + + /* + * This will advise events. + */ + set_method_event_handle_for_providers(&Provider_proxy, &Provider_hwnd, &Provider_nc, method_event, ADVISE_EVENTS_EVENT_ADDED); + reset_event_advise_values_for_hwnd_providers(&Provider_proxy, &Provider_hwnd, &Provider_nc); + SET_EXPECT(child_winproc_GETOBJECT_UiaRoot); /* Only sent on Win11. */ + set_uia_hwnd_expects(0, 3, 3, 1, 0); /* Only Win11 sends WM_GETOBJECT. */ + + NotifyWinEvent(EVENT_OBJECT_SHOW, GetDesktopWindow(), OBJID_WINDOW, CHILDID_SELF); + ok(msg_wait_for_all_events(method_event, event_handle_count, 2000) != WAIT_TIMEOUT, "Wait for method_event(s) timed out.\n"); + if (wait_for_clientside_callbacks(2000)) trace("Kept getting callbacks up until timeout\n"); + + test_hwnd_providers_event_advise_added(&Provider_proxy, &Provider_hwnd, &Provider_nc, UIA_AutomationFocusChangedEventId, FALSE); + set_method_event_handle_for_providers(&Provider_proxy, &Provider_hwnd, &Provider_nc, NULL, -1); + check_uia_hwnd_expects_at_most(0, 3, 3, 1, 0); + CHECK_CALLED_AT_MOST(child_winproc_GETOBJECT_UiaRoot, 1); + + /* + * The HWND that was focused when adding the event handler is ignored, + * EVENT_OBJECT_FOCUS only advises of events the first time + * EVENT_OBJECT_FOCUS is raised on an HWND, and it tracks if it has + * encountered a particular HWND before. + */ + set_method_event_handle_for_providers(&Provider, &Provider_hwnd2, &Provider_nc2, method_event, ADVISE_EVENTS_EVENT_ADDED); + set_uia_hwnd_expects(0, 1, 1, 3, 0); /* Only Win11 sends WM_GETOBJECT 3 times. */ + + NotifyWinEvent(EVENT_OBJECT_FOCUS, test_hwnd, OBJID_CLIENT, CHILDID_SELF); + ok(msg_wait_for_all_events(method_event, event_handle_count, 750) == WAIT_TIMEOUT, "Wait for method_event(s) didn't time out.\n"); + if (wait_for_clientside_callbacks(2000)) trace("Kept getting callbacks up until timeout\n"); + + set_method_event_handle_for_providers(&Provider, &Provider_hwnd2, &Provider_nc2, NULL, -1); + check_uia_hwnd_expects_at_most(0, 1, 1, 3, 0); + + /* Only OBJID_CLIENT is listened for, all other OBJIDs are ignored. */ + set_method_event_handle_for_providers(&Provider2, &Provider_hwnd3, &Provider_nc3, method_event, ADVISE_EVENTS_EVENT_ADDED); + + NotifyWinEvent(EVENT_OBJECT_FOCUS, test_child_hwnd, OBJID_WINDOW, CHILDID_SELF); + ok(msg_wait_for_all_events(method_event, event_handle_count, 750) == WAIT_TIMEOUT, "Wait for method_event(s) didn't time out.\n"); + if (wait_for_clientside_callbacks(2000)) trace("Kept getting callbacks up until timeout\n"); + + /* + * First time EVENT_OBJECT_FOCUS is fired for this HWND, it will be + * advised of UIA_AutomationFocusChangedEventId being listened for. + * But only the serverside provider. + */ + reset_event_advise_values_for_hwnd_providers(&Provider2, &Provider_hwnd3, &Provider_nc3); + set_provider_method_event_data(&Provider2, method_event[0], ADVISE_EVENTS_EVENT_ADDED); + SET_EXPECT_MULTI(child_winproc_GETOBJECT_UiaRoot, 3); /* Only sent 3 times on Win11. */ + set_uia_hwnd_expects(0, 1, 1, 2, 0); /* Only Win11 sends WM_GETOBJECT 2 times. */ + + NotifyWinEvent(EVENT_OBJECT_FOCUS, test_child_hwnd, OBJID_CLIENT, CHILDID_SELF); + todo_wine ok(msg_wait_for_all_events(method_event, 1, 2000) != WAIT_TIMEOUT, "Wait for method_event(s) timed out.\n"); + if (wait_for_clientside_callbacks(2000)) trace("Kept getting callbacks up until timeout\n"); + + check_uia_hwnd_expects_at_most(0, 1, 1, 2, 0); + CHECK_CALLED_AT_MOST(child_winproc_GETOBJECT_UiaRoot, 3); + test_provider_event_advise_added(&Provider2, UIA_AutomationFocusChangedEventId, TRUE); + test_provider_event_advise_added(&Provider_hwnd3, 0, FALSE); + test_provider_event_advise_added(&Provider_nc3, 0, FALSE); + + /* Doing it again has no effect, it has already been advised. */ + reset_event_advise_values_for_hwnd_providers(&Provider2, &Provider_hwnd3, &Provider_nc3); + SET_EXPECT_MULTI(child_winproc_GETOBJECT_UiaRoot, 3); /* Only sent 3 times on Win11. */ + set_uia_hwnd_expects(0, 1, 1, 2, 0); /* Only Win11 sends WM_GETOBJECT 2 times. */ + + NotifyWinEvent(EVENT_OBJECT_FOCUS, test_child_hwnd, OBJID_CLIENT, CHILDID_SELF); + ok(msg_wait_for_all_events(method_event, 1, 750) == WAIT_TIMEOUT, "Wait for method_event(s) didn't time out.\n"); + if (wait_for_clientside_callbacks(2000)) trace("Kept getting callbacks up until timeout\n"); + + set_provider_method_event_data(&Provider2, NULL, -1); + check_uia_hwnd_expects_at_most(0, 1, 1, 2, 0); + CHECK_CALLED_AT_MOST(child_winproc_GETOBJECT_UiaRoot, 3); + test_hwnd_providers_event_advise_added(&Provider2, &Provider_hwnd3, &Provider_nc3, 0, FALSE); + + /* + * Same deal here - we've already encountered this HWND and advised it + * when the event handler was added initially, so it too is ignored. + */ + set_method_event_handle_for_providers(&Provider, &Provider_hwnd2, &Provider_nc2, method_event, ADVISE_EVENTS_EVENT_ADDED); + reset_event_advise_values_for_hwnd_providers(&Provider, &Provider_hwnd2, &Provider_nc2); + set_uia_hwnd_expects(0, 1, 1, 3, 0); /* Only Win11 sends WM_GETOBJECT 3 times. */ + + NotifyWinEvent(EVENT_OBJECT_FOCUS, test_hwnd, OBJID_CLIENT, CHILDID_SELF); + ok(msg_wait_for_all_events(method_event, event_handle_count, 750) == WAIT_TIMEOUT, "Wait for method_event(s) didn't time out.\n"); + if (wait_for_clientside_callbacks(2000)) trace("Kept getting callbacks up until timeout\n"); + + set_method_event_handle_for_providers(&Provider, &Provider_hwnd2, &Provider_nc2, NULL, -1); + check_uia_hwnd_expects_at_most(0, 1, 1, 3, 0); + test_hwnd_providers_event_advise_added(&Provider, &Provider_hwnd2, &Provider_nc2, 0, FALSE); + + set_uia_hwnd_expects(0, 1, 1, 0, 0); + hr = IUIAutomation_RemoveFocusChangedEventHandler(uia_iface, + &FocusChangedHandler.IUIAutomationFocusChangedEventHandler_iface); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + check_uia_hwnd_expects(0, FALSE, 1, FALSE, 1, FALSE, 0, FALSE, 0, FALSE); + exit: for (i = 0; i < ARRAY_SIZE(method_event); i++) CloseHandle(method_event[i]); @@ -17113,14 +17499,6 @@ static void test_uia_event_win_event_mapping_(DWORD win_event, HWND hwnd, LONG o todo_wine_if(todo) CHECK_CALLED(uia_event_callback2); } -#define test_provider_event_advise_added( prov, event_id, todo) \ - test_provider_event_advise_added_( (prov), (event_id), (todo), __FILE__, __LINE__) -static void test_provider_event_advise_added_(struct Provider *prov, int event_id, BOOL todo, const char *file, int line) -{ - todo_wine_if (todo) ok_(file, line)(prov->advise_events_added_event_id == event_id, "%s: Unexpected advise event added, event ID %d.\n", - prov->prov_name, prov->advise_events_added_event_id); -} - static DWORD WINAPI uia_proxy_provider_win_event_handler_test_thread(LPVOID param) { struct UiaCacheRequest cache_req = { (struct UiaCondition *)&UiaTrueCondition, TreeScope_Element, NULL, 0, NULL, 0, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4024
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/uiautomationcore/uia_client.c | 2 +- dlls/uiautomationcore/uia_com_client.c | 87 ++++++++++++++++++++++++++ dlls/uiautomationcore/uia_private.h | 1 + 3 files changed, 89 insertions(+), 1 deletion(-) diff --git a/dlls/uiautomationcore/uia_client.c b/dlls/uiautomationcore/uia_client.c index f7b602ab2ed..f57ffb4bc4c 100644 --- a/dlls/uiautomationcore/uia_client.c +++ b/dlls/uiautomationcore/uia_client.c @@ -2631,7 +2631,7 @@ static HRESULT uia_get_provider_from_hwnd(struct uia_node *node) return SendMessageW(client_thread.hwnd, WM_UIA_CLIENT_GET_NODE_PROV, (WPARAM)&args, (LPARAM)node); } -static HRESULT create_uia_node_from_hwnd(HWND hwnd, HUIANODE *out_node, int node_flags) +HRESULT create_uia_node_from_hwnd(HWND hwnd, HUIANODE *out_node, int node_flags) { struct uia_node *node; HRESULT hr; diff --git a/dlls/uiautomationcore/uia_com_client.c b/dlls/uiautomationcore/uia_com_client.c index 6d3d2216169..01272f89064 100644 --- a/dlls/uiautomationcore/uia_com_client.c +++ b/dlls/uiautomationcore/uia_com_client.c @@ -903,6 +903,7 @@ static HRESULT get_uia_cache_request_struct_from_iface(IUIAutomationCacheRequest */ static struct uia_com_event_handlers { + struct rb_tree handler_event_id_map; struct rb_tree handler_map; LONG handler_count; @@ -917,6 +918,22 @@ static CRITICAL_SECTION_DEBUG com_event_handlers_cs_debug = }; static CRITICAL_SECTION com_event_handlers_cs = { &com_event_handlers_cs_debug, -1, 0, 0, 0, 0 }; +struct uia_event_handler_event_id_map_entry +{ + struct rb_entry entry; + int event_id; + + struct list handlers_list; +}; + +static int uia_com_event_handler_event_id_compare(const void *key, const struct rb_entry *entry) +{ + struct uia_event_handler_event_id_map_entry *event_entry = RB_ENTRY_VALUE(entry, struct uia_event_handler_event_id_map_entry, entry); + int event_id = *((int *)key); + + return (event_entry->event_id > event_id) - (event_entry->event_id < event_id); +} + struct uia_event_handler_identifier { IUnknown *handler_iface; SAFEARRAY *runtime_id; @@ -932,6 +949,9 @@ struct uia_event_handler_map_entry int event_id; struct list handlers_list; + + struct uia_event_handler_event_id_map_entry *handler_event_id_map; + struct list handler_event_id_map_list_entry; }; static int uia_com_event_handler_id_compare(const void *key, const struct rb_entry *entry) @@ -1027,6 +1047,32 @@ HRESULT uia_com_win_event_callback(DWORD event_id, HWND hwnd, LONG obj_id, LONG break; } + case EVENT_OBJECT_FOCUS: + { + static const int uia_event_id = UIA_AutomationFocusChangedEventId; + struct rb_entry *rb_entry; + HRESULT hr; + + if (obj_id != OBJID_CLIENT) + break; + + EnterCriticalSection(&com_event_handlers_cs); + + if ((rb_entry = rb_get(&com_event_handlers.handler_event_id_map, &uia_event_id))) + { + HUIANODE node = NULL; + + hr = create_uia_node_from_hwnd(hwnd, &node, NODE_FLAG_IGNORE_CLIENTSIDE_HWND_PROVS); + if (SUCCEEDED(hr)) + FIXME("EVENT_OBJECT_FOCUS event advisement currently unimplemented\n"); + + UiaNodeRelease(node); + } + + LeaveCriticalSection(&com_event_handlers_cs); + break; + } + default: break; } @@ -1034,6 +1080,29 @@ HRESULT uia_com_win_event_callback(DWORD event_id, HWND hwnd, LONG obj_id, LONG return S_OK; } +static HRESULT uia_event_handlers_add_handler_to_event_id_map(struct uia_event_handler_map_entry *event_map) +{ + struct uia_event_handler_event_id_map_entry *event_id_map; + struct rb_entry *rb_entry; + + if ((rb_entry = rb_get(&com_event_handlers.handler_event_id_map, &event_map->event_id))) + event_id_map = RB_ENTRY_VALUE(rb_entry, struct uia_event_handler_event_id_map_entry, entry); + else + { + if (!(event_id_map = calloc(1, sizeof(*event_id_map)))) + return E_OUTOFMEMORY; + + event_id_map->event_id = event_map->event_id; + list_init(&event_id_map->handlers_list); + rb_put(&com_event_handlers.handler_event_id_map, &event_map->event_id, &event_id_map->entry); + } + + list_add_tail(&event_id_map->handlers_list, &event_map->handler_event_id_map_list_entry); + event_map->handler_event_id_map = event_id_map; + + return S_OK; +} + static HRESULT uia_event_handlers_add_handler(IUnknown *handler_iface, SAFEARRAY *runtime_id, int event_id, struct uia_com_event *event) { @@ -1045,7 +1114,10 @@ static HRESULT uia_event_handlers_add_handler(IUnknown *handler_iface, SAFEARRAY EnterCriticalSection(&com_event_handlers_cs); if (!com_event_handlers.handler_count) + { rb_init(&com_event_handlers.handler_map, uia_com_event_handler_id_compare); + rb_init(&com_event_handlers.handler_event_id_map, uia_com_event_handler_event_id_compare); + } if ((rb_entry = rb_get(&com_event_handlers.handler_map, &event_ident))) event_map = RB_ENTRY_VALUE(rb_entry, struct uia_event_handler_map_entry, entry); @@ -1065,6 +1137,14 @@ static HRESULT uia_event_handlers_add_handler(IUnknown *handler_iface, SAFEARRAY } event_map->event_id = event_id; + hr = uia_event_handlers_add_handler_to_event_id_map(event_map); + if (FAILED(hr)) + { + SafeArrayDestroy(event_map->runtime_id); + free(event_map); + goto exit; + } + event_map->handler_iface = handler_iface; IUnknown_AddRef(event_map->handler_iface); @@ -1102,6 +1182,13 @@ static void uia_event_handler_map_entry_destroy(struct uia_event_handler_map_ent com_event_handlers.handler_count--; } + list_remove(&entry->handler_event_id_map_list_entry); + if (list_empty(&entry->handler_event_id_map->handlers_list)) + { + rb_remove(&com_event_handlers.handler_event_id_map, &entry->handler_event_id_map->entry); + free(entry->handler_event_id_map); + } + rb_remove(&com_event_handlers.handler_map, &entry->entry); IUnknown_Release(entry->handler_iface); SafeArrayDestroy(entry->runtime_id); diff --git a/dlls/uiautomationcore/uia_private.h b/dlls/uiautomationcore/uia_private.h index 0a8f2d69dbd..0c4f1ee9dda 100644 --- a/dlls/uiautomationcore/uia_private.h +++ b/dlls/uiautomationcore/uia_private.h @@ -219,6 +219,7 @@ HRESULT navigate_uia_node(struct uia_node *node, int nav_dir, HUIANODE *out_node HRESULT create_uia_node_from_elprov(IRawElementProviderSimple *elprov, HUIANODE *out_node, BOOL get_hwnd_providers, int node_flags) DECLSPEC_HIDDEN; HRESULT uia_node_from_lresult(LRESULT lr, HUIANODE *huianode) DECLSPEC_HIDDEN; +HRESULT create_uia_node_from_hwnd(HWND hwnd, HUIANODE *out_node, int node_flags) DECLSPEC_HIDDEN; HRESULT uia_condition_check(HUIANODE node, struct UiaCondition *condition) DECLSPEC_HIDDEN; BOOL uia_condition_matched(HRESULT hr) DECLSPEC_HIDDEN; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4024
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/uiautomationcore/tests/uiautomation.c | 4 +-- dlls/uiautomationcore/uia_com_client.c | 40 +++++++++++++++++++++- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/dlls/uiautomationcore/tests/uiautomation.c b/dlls/uiautomationcore/tests/uiautomation.c index d5f8bb492a4..4523e09693f 100644 --- a/dlls/uiautomationcore/tests/uiautomation.c +++ b/dlls/uiautomationcore/tests/uiautomation.c @@ -15420,12 +15420,12 @@ static void test_uia_com_event_handler_event_advisement(IUIAutomation *uia_iface set_uia_hwnd_expects(0, 1, 1, 2, 0); /* Only Win11 sends WM_GETOBJECT 2 times. */ NotifyWinEvent(EVENT_OBJECT_FOCUS, test_child_hwnd, OBJID_CLIENT, CHILDID_SELF); - todo_wine ok(msg_wait_for_all_events(method_event, 1, 2000) != WAIT_TIMEOUT, "Wait for method_event(s) timed out.\n"); + ok(msg_wait_for_all_events(method_event, 1, 2000) != WAIT_TIMEOUT, "Wait for method_event(s) timed out.\n"); if (wait_for_clientside_callbacks(2000)) trace("Kept getting callbacks up until timeout\n"); check_uia_hwnd_expects_at_most(0, 1, 1, 2, 0); CHECK_CALLED_AT_MOST(child_winproc_GETOBJECT_UiaRoot, 3); - test_provider_event_advise_added(&Provider2, UIA_AutomationFocusChangedEventId, TRUE); + test_provider_event_advise_added(&Provider2, UIA_AutomationFocusChangedEventId, FALSE); test_provider_event_advise_added(&Provider_hwnd3, 0, FALSE); test_provider_event_advise_added(&Provider_nc3, 0, FALSE); diff --git a/dlls/uiautomationcore/uia_com_client.c b/dlls/uiautomationcore/uia_com_client.c index 01272f89064..b6e06d21935 100644 --- a/dlls/uiautomationcore/uia_com_client.c +++ b/dlls/uiautomationcore/uia_com_client.c @@ -974,10 +974,37 @@ struct uia_com_event { HUIAEVENT event; BOOL from_cui8; + struct rb_tree focus_hwnd_map; struct list event_handler_map_list_entry; struct uia_event_handler_map_entry *handler_map; }; +static void uia_com_focus_win_event_handler(HUIANODE node, HWND hwnd, struct uia_event_handler_event_id_map_entry *event_id_map) +{ + struct uia_event_handler_map_entry *entry; + HRESULT hr; + + LIST_FOR_EACH_ENTRY(entry, &event_id_map->handlers_list, struct uia_event_handler_map_entry, handler_event_id_map_list_entry) + { + struct uia_com_event *event; + + LIST_FOR_EACH_ENTRY(event, &entry->handlers_list, struct uia_com_event, event_handler_map_list_entry) + { + if (uia_hwnd_map_check_hwnd(&event->focus_hwnd_map, hwnd)) + continue; + + /* Advise provider that we're listening for focus events. */ + hr = uia_event_advise_node((struct uia_event *)event->event, node); + if (FAILED(hr)) + WARN("uia_event_advise_node failed with hr %#lx\n", hr); + + hr = uia_hwnd_map_add_hwnd(&event->focus_hwnd_map, hwnd); + if (FAILED(hr)) + WARN("Failed to add hwnd for focus winevent, hr %#lx\n", hr); + } + } +} + HRESULT uia_com_win_event_callback(DWORD event_id, HWND hwnd, LONG obj_id, LONG child_id, DWORD thread_id, DWORD event_time) { LONG handler_count; @@ -1060,11 +1087,13 @@ HRESULT uia_com_win_event_callback(DWORD event_id, HWND hwnd, LONG obj_id, LONG if ((rb_entry = rb_get(&com_event_handlers.handler_event_id_map, &uia_event_id))) { + struct uia_event_handler_event_id_map_entry *event_id_map; HUIANODE node = NULL; + event_id_map = RB_ENTRY_VALUE(rb_entry, struct uia_event_handler_event_id_map_entry, entry); hr = create_uia_node_from_hwnd(hwnd, &node, NODE_FLAG_IGNORE_CLIENTSIDE_HWND_PROVS); if (SUCCEEDED(hr)) - FIXME("EVENT_OBJECT_FOCUS event advisement currently unimplemented\n"); + uia_com_focus_win_event_handler(node, hwnd, event_id_map); UiaNodeRelease(node); } @@ -1155,6 +1184,13 @@ static HRESULT uia_event_handlers_add_handler(IUnknown *handler_iface, SAFEARRAY list_add_tail(&event_map->handlers_list, &event->event_handler_map_list_entry); event->handler_map = event_map; com_event_handlers.handler_count++; + if (event_id == UIA_AutomationFocusChangedEventId) + { + GUITHREADINFO info = { sizeof(info) }; + + if (GetGUIThreadInfo(0, &info) && info.hwndFocus) + uia_hwnd_map_add_hwnd(&event->focus_hwnd_map, info.hwndFocus); + } exit: LeaveCriticalSection(&com_event_handlers_cs); @@ -1165,6 +1201,7 @@ exit: static void uia_event_handler_destroy(struct uia_com_event *event) { list_remove(&event->event_handler_map_list_entry); + uia_hwnd_map_destroy(&event->focus_hwnd_map); if (event->event) UiaRemoveEvent(event->event); if (event->git_cookie) @@ -3459,6 +3496,7 @@ static HRESULT uia_add_com_event_handler(IUIAutomation6 *iface, EVENTID event_id com_event->from_cui8 = element->from_cui8; list_init(&com_event->event_handler_map_list_entry); + uia_hwnd_map_init(&com_event->focus_hwnd_map); hr = IUnknown_QueryInterface(handler_unk, handler_riid, (void **)&handler_iface); if (FAILED(hr)) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4024
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=138358 Your paranoid android. === debian11b (64 bit WoW report) === mshtml: htmldoc.c:3310: Test failed: Incorrect error code: -2146697208 htmldoc.c:3315: Test failed: Page address: L"http://test.winehq.org/tests/winehq_snapshot/" htmldoc.c:3197: Test failed: unexpected state L"loading", expected 3 htmldoc.c:3197: Test failed: VT_I4(out)=1, expected 3 htmldoc.c:3197: Test failed: VT_I4(out)=1, expected 3 htmldoc.c:3253: Test failed: unexpected state L"loading", expected 3 htmldoc.c:3253: Test failed: VT_I4(out)=1, expected 3 htmldoc.c:3253: Test failed: VT_I4(out)=1, expected 3 htmldoc.c:3019: Test failed: unexpected state L"loading", expected 3 htmldoc.c:3019: Test failed: VT_I4(out)=1, expected 3 htmldoc.c:3019: Test failed: VT_I4(out)=1, expected 3 htmldoc.c:5587: Test failed: doScroll failed: 8000000a htmldoc.c:6088: Test failed: expected Invoke_OnReadyStateChange_Interactive htmldoc.c:6110: Test failed: expected OnChanged_1005 htmldoc.c:6155: Test succeeded inside todo block: expected IsErrorUrl htmldoc.c:5587: Test failed: doScroll failed: 8000000a htmldoc.c:8247: Test failed: domain = L"test.winehq.org" htmldoc.c:8252: Test failed: put_domain failed: 00000000, expected E_INVALIDARG htmldoc.c:8257: Test failed: domain = L"test.winehq.org" htmldoc.c:5587: Test failed: doScroll failed: 8000000a htmldoc.c:5587: Test failed: doScroll failed: 8000000a htmldoc.c:5587: Test failed: doScroll failed: 8000000a htmldoc.c:5587: Test failed: doScroll failed: 8000000a htmldoc.c:5587: Test failed: doScroll failed: 8000000a htmldoc.c:3310: Test failed: Incorrect error code: -2146697208 htmldoc.c:3315: Test failed: Page address: L"http://test.winehq.org/tests/winehq_snapshot/" htmldoc.c:3197: Test failed: unexpected state L"loading", expected 3 htmldoc.c:3197: Test failed: VT_I4(out)=1, expected 3 htmldoc.c:3197: Test failed: VT_I4(out)=1, expected 3 htmldoc.c:3253: Test failed: unexpected state L"loading", expected 3 htmldoc.c:3253: Test failed: VT_I4(out)=1, expected 3 htmldoc.c:3253: Test failed: VT_I4(out)=1, expected 3 htmldoc.c:3019: Test failed: unexpected state L"loading", expected 3 htmldoc.c:3019: Test failed: VT_I4(out)=1, expected 3 htmldoc.c:3019: Test failed: VT_I4(out)=1, expected 3 htmldoc.c:5587: Test failed: doScroll failed: 8000000a htmldoc.c:6088: Test failed: expected Invoke_OnReadyStateChange_Interactive htmldoc.c:6110: Test failed: expected OnChanged_1005 htmldoc.c:6149: Test failed: expected FireNavigateComplete2 htmldoc.c:6155: Test succeeded inside todo block: expected IsErrorUrl htmldoc.c:5587: Test failed: doScroll failed: 8000000a htmldoc.c:8247: Test failed: domain = L"test.winehq.org" htmldoc.c:8252: Test failed: put_domain failed: 00000000, expected E_INVALIDARG htmldoc.c:8257: Test failed: domain = L"test.winehq.org" htmldoc.c:5587: Test failed: doScroll failed: 8000000a htmldoc.c:3725: Test failed: L"domInteractive" is 0 htmldoc.c:5587: Test failed: doScroll failed: 8000000a htmldoc.c:5587: Test failed: doScroll failed: 8000000a htmldoc.c:3310: Test failed: Incorrect error code: -2146697208 htmldoc.c:3315: Test failed: Page address: L"http://test.winehq.org/tests/winehq_snapshot/#test" htmldoc: Timeout script.c:1141: Test failed: L"/events.html: extern_http_script_rs = loading,loaded, expected loading,loaded,complete, or complete," xmlhttprequest: Timeout
Esme Povirk (@madewokherd) commented about dlls/uiautomationcore/tests/uiautomation.c:
+/* + * Some versions of Windows 10 query multiple unrelated HWNDs when + * winevents are fired, this function waits for these to stop before + * continuing to the next test. + */ +#define TIME_SINCE_LAST_CALLBACK_TIMEOUT 200 +static BOOL wait_for_clientside_callbacks(DWORD total_timeout) +{ + DWORD prev_last_callback_time, cur_time = 0; + DWORD start_time = GetTickCount(); + BOOL ret; + + if (last_clientside_callback_time < start_time) + prev_last_callback_time = start_time; + else + prev_last_callback_time = last_clientside_callback_time; We usually avoid comparisons like this for `GetTickCount()` values, because it can wrap around at 49.7 days uptime. Comparisons like `(cur_time - start_time) >= total_timeout` work correctly even in case of wraparound.
Is there any reason to have `prev_last_callback_time` at all? What if we just set last_clientside_callback_time to start_time here and used that? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4024#note_47886
Esme Povirk (@madewokherd) commented about dlls/uiautomationcore/tests/uiautomation.c:
} }
+#define test_provider_event_advise_added( prov, event_id, todo) \ + test_provider_event_advise_added_( (prov), (event_id), (todo), __FILE__, __LINE__) +static void test_provider_event_advise_added_(struct Provider *prov, int event_id, BOOL todo, const char *file, int line) +{ + todo_wine_if (todo) ok_(file, line)(prov->advise_events_added_event_id == event_id, "%s: Unexpected advise event added, event ID %d.\n", + prov->prov_name, prov->advise_events_added_event_id); +} +
I think it would be simpler to leave out the `todo` argument and call the function with `todo_wine` instead. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4024#note_47887
On Fri Oct 6 16:57:54 2023 +0000, Esme Povirk wrote:
We usually avoid comparisons like this for `GetTickCount()` values, because it can wrap around at 49.7 days uptime. Comparisons like `(cur_time - start_time) >= total_timeout` work correctly even in case of wraparound. Is there any reason to have `prev_last_callback_time` at all? What if we just set last_clientside_callback_time to start_time here and used that? I think the issue with getting rid of `prev_last_callback_time` here is that `last_clientside_callback_time` can (and likely will) be set on another thread. Which would be broken here anyways, seeing as we call:
cur_time = GetTickCount();
if (prev_last_callback_time < last_clientside_callback_time)
prev_last_callback_time = last_clientside_callback_time;
and set `prev_last_callback_time` _after_ we set `cur_time`, which could mean `last_clientside_callback_time` could potentially be written more recently than `cur_time`. This will need some thought to do properly. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4024#note_47892
On Fri Oct 6 18:00:23 2023 +0000, Connor McAdams wrote:
I think the issue with getting rid of `prev_last_callback_time` here is that `last_clientside_callback_time` can (and likely will) be set on another thread. Which would be broken here anyways, seeing as we call: ``` cur_time = GetTickCount(); if (prev_last_callback_time < last_clientside_callback_time) prev_last_callback_time = last_clientside_callback_time; ``` and set `prev_last_callback_time` _after_ we set `cur_time`, which could mean `last_clientside_callback_time` could potentially be written more recently than `cur_time`. This will need some thought to do properly. What about an event instead? So basically, in `uia_com_win_event_clientside_provider_callback` you would set an event. Then in this loop:
* WaitForSingleObject on the event with a timeout of `TIME_SINCE_LAST_CALLBACK_TIMEOUT`. * Return TRUE if the full timeout elapsed. * Return FALSE if WaitForSingleObject timed out. * Else, continue. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4024#note_47893
On Fri Oct 6 18:13:47 2023 +0000, Esme Povirk wrote:
What about an event instead? So basically, in `uia_com_win_event_clientside_provider_callback` you would set an event. Then in this loop: * WaitForSingleObject on the event with a timeout of `TIME_SINCE_LAST_CALLBACK_TIMEOUT`. * Return TRUE if the full timeout elapsed. * Return FALSE if WaitForSingleObject timed out. * Else, continue. I suppose this would require locking to ensure that other threads don't try to set a stale event handle.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4024#note_47894
On Fri Oct 6 18:25:11 2023 +0000, Esme Povirk wrote:
I think it would be simpler to leave out the `todo` argument and call the function with `todo_wine` instead. Oh, you just moved it up. Never mind then, not worth changing.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4024#note_47895
Esme Povirk (@madewokherd) commented about dlls/uiautomationcore/uia_com_client.c:
+ + LIST_FOR_EACH_ENTRY(event, &entry->handlers_list, struct uia_com_event, event_handler_map_list_entry) + { + if (uia_hwnd_map_check_hwnd(&event->focus_hwnd_map, hwnd)) + continue; + + /* Advise provider that we're listening for focus events. */ + hr = uia_event_advise_node((struct uia_event *)event->event, node); + if (FAILED(hr)) + WARN("uia_event_advise_node failed with hr %#lx\n", hr); + + hr = uia_hwnd_map_add_hwnd(&event->focus_hwnd_map, hwnd); + if (FAILED(hr)) + WARN("Failed to add hwnd for focus winevent, hr %#lx\n", hr); + } + } There should probably be a corresponding EVENT_OBJECT_DESTROY handler in case hwnds are reused.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4024#note_47897
Esme Povirk (@madewokherd) commented about dlls/uiautomationcore/uia_com_client.c:
list_add_tail(&event_map->handlers_list, &event->event_handler_map_list_entry); event->handler_map = event_map; com_event_handlers.handler_count++; + if (event_id == UIA_AutomationFocusChangedEventId) + { + GUITHREADINFO info = { sizeof(info) }; + + if (GetGUIThreadInfo(0, &info) && info.hwndFocus) + uia_hwnd_map_add_hwnd(&event->focus_hwnd_map, info.hwndFocus);
It seems wrong to add this to the map without advising. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4024#note_47899
participants (4)
-
Connor McAdams -
Connor McAdams (＠cmcadams)
-
Esme Povirk (＠madewokherd)
-
Marvin