[PATCH 0/3] MR8166: taskschd: Implements IRegisteredTaskCollection get_Count and get_Item

[Jacob Czekalla (＠JacobCzekalla)]

Implements IRegisteredTaskCollection get_Count and get_Item along with a test for both. When making the tests individually they were very similar so I merged them into one test function as per feedback.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8166

[PATCH 1/3] taskschd/tests: Adds a test for IRegisteredTaskCollection get_Item and get_Count.

[Jacob Czekalla]

From: Jacob Czekalla <jczekalla(a)codeweavers.com>

---
 dlls/taskschd/tests/scheduler.c | 95 +++++++++++++++++++++++++++++++++
 1 file changed, 95 insertions(+)

diff --git a/dlls/taskschd/tests/scheduler.c b/dlls/taskschd/tests/scheduler.c
index f9c209f1d84..d4256134766 100644
--- a/dlls/taskschd/tests/scheduler.c
+++ b/dlls/taskschd/tests/scheduler.c
@@ -1879,6 +1879,100 @@ static void test_TaskDefinition(void)
     ITaskService_Release(service);
 }
 
+static void test_get_Count_and_Item(void)
+{
+    HRESULT hr;
+    LONG num_tasks;
+    VARIANT v_null, index;
+    ITaskService *service;
+    ITaskFolder *folder, *root;
+    ITaskDefinition *task_def;
+    IRegistrationInfo *reg_info;
+    IActionCollection *actions;
+    IAction *action;
+    IExecAction *exec_action;
+    IRegisteredTask *task1, *ret_task1;
+    IRegisteredTaskCollection *tasks;
+    BSTR ret_task1_name = NULL;
+
+    V_VT(&v_null) = VT_NULL;
+
+    CoCreateInstance(&CLSID_TaskScheduler, NULL, CLSCTX_INPROC_SERVER, &IID_ITaskService, (void**)&service);
+    ITaskService_Connect(service, v_null, v_null, v_null, v_null);
+
+    ITaskService_GetFolder(service, (BSTR)L"\\", &root);
+    ITaskFolder_CreateFolder(root,(BSTR)L"\\Wine", v_null, &folder);
+
+    ITaskService_NewTask(service, 0, &task_def);
+    ITaskDefinition_get_RegistrationInfo(task_def, ®_info);
+    IRegistrationInfo_put_Author(reg_info, (BSTR)L"Wine Test");
+
+    ITaskDefinition_get_Actions(task_def, &actions);
+    IActionCollection_Create(actions, TASK_ACTION_EXEC, &action);
+    IAction_QueryInterface(action, &IID_IExecAction, (void**)&exec_action);
+    IExecAction_put_Path(exec_action, (BSTR)L"task1.exe");
+
+    ITaskFolder_RegisterTaskDefinition(folder, (BSTR)L"Task1", task_def, TASK_CREATE, v_null, v_null, TASK_LOGON_NONE, v_null, &task1);
+    ITaskFolder_GetTasks(folder, TASK_ENUM_HIDDEN, &tasks);
+
+    /* Test get_Count */
+    hr = IRegisteredTaskCollection_get_Count(tasks, NULL);
+    todo_wine
+    ok(hr == E_POINTER, "expected E_POINTER, got %#lx\n", hr);
+
+    hr = IRegisteredTaskCollection_get_Count(tasks, &num_tasks);
+    todo_wine
+    ok(hr == S_OK, "expected S_OK, got %#lx\n", hr);
+    todo_wine
+    ok(num_tasks == 1, "expected 1 task, got %ld\n", num_tasks);
+
+    /* Test get_Item */
+    VariantInit(&index);
+    index.vt = VT_UI4;
+    index.uiVal = 1;
+
+    hr = IRegisteredTaskCollection_get_Item(tasks, index, NULL);
+    todo_wine
+    ok(hr == E_POINTER, "expected E_POINTER, got %#lx\n", hr);
+
+    index.uiVal = 0;
+    hr = IRegisteredTaskCollection_get_Item(tasks, index, &ret_task1);
+    todo_wine
+    ok(hr == E_INVALIDARG, "expected E_INVALIDARG, got %#lx\n", hr);
+
+    index.uiVal = 2;
+    hr = IRegisteredTaskCollection_get_Item(tasks, index, &ret_task1);
+    todo_wine
+    ok(hr == E_INVALIDARG, "expected E_INVALIDARG, got %#lx\n", hr);
+
+    index.uiVal = 1;
+    hr = IRegisteredTaskCollection_get_Item(tasks, index, &ret_task1);
+    todo_wine
+    ok(hr == S_OK, "expected S_OK, got %#lx\n", hr);
+    if (hr == S_OK)
+    {
+        IRegisteredTask_get_Name(ret_task1, &ret_task1_name);
+        if(ret_task1_name)
+        {
+            todo_wine
+            ok(!lstrcmpW(L"Task1", ret_task1_name), "expected name \"Task1\", got %ls\n", ret_task1_name);
+        }
+        else
+        {
+            todo_wine
+            ok(0, "expected name \"Task1\", got NULL\n");
+        }
+    }
+
+    ITaskFolder_DeleteTask(folder, (BSTR)L"Task1", 0);
+    ITaskFolder_DeleteFolder(root, (BSTR)L"\\Wine", 0);
+    IRegisteredTask_Release(task1);
+    IExecAction_Release(exec_action);
+    IRegistrationInfo_Release(reg_info);
+    ITaskDefinition_Release(task_def);
+    ITaskService_Release(service);
+}
+
 START_TEST(scheduler)
 {
     OleInitialize(NULL);
@@ -1888,6 +1982,7 @@ START_TEST(scheduler)
     test_FolderCollection();
     test_GetTask();
     test_TaskDefinition();
+    test_get_Count_and_Item();
 
     OleUninitialize();
 }
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/8166

[PATCH 2/3] taskschd/regtask: Implements IRegisteredTaskCollection get_Item().

[Jacob Czekalla]

From: Jacob Czekalla <jczekalla(a)codeweavers.com>

---
 dlls/taskschd/regtask.c         | 56 +++++++++++++++++++++++++++++++--
 dlls/taskschd/tests/scheduler.c | 10 ------
 2 files changed, 54 insertions(+), 12 deletions(-)

diff --git a/dlls/taskschd/regtask.c b/dlls/taskschd/regtask.c
index bae0d4cb0b2..744f584b5a1 100644
--- a/dlls/taskschd/regtask.c
+++ b/dlls/taskschd/regtask.c
@@ -471,8 +471,60 @@ static HRESULT WINAPI regtasks_get_Count(IRegisteredTaskCollection *iface, LONG
 
 static HRESULT WINAPI regtasks_get_Item(IRegisteredTaskCollection *iface, VARIANT index, IRegisteredTask **regtask)
 {
-    FIXME("%p,%s,%p: stub\n", iface, debugstr_variant(&index), regtask);
-    return E_NOTIMPL;
+    HRESULT hr;
+    WCHAR *xml = NULL;
+    VARIANT converted_index;
+    TASK_NAMES task_names = NULL;
+    ITaskDefinition *definition = NULL;
+    RegisteredTaskCollection *collection = NULL;
+    DWORD start_index = 0, num_tasks = 0;
+
+    collection = impl_from_IRegisteredTaskCollection(iface);
+    if (!regtask)
+        return E_POINTER;
+    *regtask = NULL;
+
+    VariantInit(&converted_index);
+    hr = VariantChangeType(&converted_index, &index, 0, VT_UI4);
+    if (FAILED(hr))
+        return hr;
+
+    start_index = V_UI4(&index);
+    if (start_index == 0)
+    {
+        VariantClear(&converted_index);
+        return E_INVALIDARG;
+    }
+    start_index -= 1;
+
+    hr = SchRpcEnumTasks(collection->path, 0, &start_index, 1, &num_tasks, &task_names);
+    if (FAILED(hr))
+    {
+        VariantClear(&converted_index);
+        return hr;
+    }
+    if (!task_names)
+    {
+        VariantClear(&converted_index);
+        return E_INVALIDARG;
+    }
+
+    hr = TaskDefinition_create(&definition);
+    if (FAILED(hr))
+        goto cleanup;
+    hr = RegisteredTask_create(collection->path, task_names[0], definition, TASK_VALIDATE_ONLY, TASK_LOGON_INTERACTIVE_TOKEN, regtask, FALSE);
+    if (FAILED(hr))
+        goto cleanup;
+
+    hr = S_OK;
+cleanup:
+    MIDL_user_free(task_names[0]);
+    MIDL_user_free(task_names);
+    if (xml)
+        SysFreeString(xml);
+    VariantClear(&converted_index);
+
+    return hr;
 }
 
 static HRESULT WINAPI regtasks_get__NewEnum(IRegisteredTaskCollection *iface, IUnknown **penum)
diff --git a/dlls/taskschd/tests/scheduler.c b/dlls/taskschd/tests/scheduler.c
index d4256134766..b09fa8216ef 100644
--- a/dlls/taskschd/tests/scheduler.c
+++ b/dlls/taskschd/tests/scheduler.c
@@ -1932,36 +1932,26 @@ static void test_get_Count_and_Item(void)
     index.uiVal = 1;
 
     hr = IRegisteredTaskCollection_get_Item(tasks, index, NULL);
-    todo_wine
     ok(hr == E_POINTER, "expected E_POINTER, got %#lx\n", hr);
 
     index.uiVal = 0;
     hr = IRegisteredTaskCollection_get_Item(tasks, index, &ret_task1);
-    todo_wine
     ok(hr == E_INVALIDARG, "expected E_INVALIDARG, got %#lx\n", hr);
 
     index.uiVal = 2;
     hr = IRegisteredTaskCollection_get_Item(tasks, index, &ret_task1);
-    todo_wine
     ok(hr == E_INVALIDARG, "expected E_INVALIDARG, got %#lx\n", hr);
 
     index.uiVal = 1;
     hr = IRegisteredTaskCollection_get_Item(tasks, index, &ret_task1);
-    todo_wine
     ok(hr == S_OK, "expected S_OK, got %#lx\n", hr);
     if (hr == S_OK)
     {
         IRegisteredTask_get_Name(ret_task1, &ret_task1_name);
         if(ret_task1_name)
-        {
-            todo_wine
             ok(!lstrcmpW(L"Task1", ret_task1_name), "expected name \"Task1\", got %ls\n", ret_task1_name);
-        }
         else
-        {
-            todo_wine
             ok(0, "expected name \"Task1\", got NULL\n");
-        }
     }
 
     ITaskFolder_DeleteTask(folder, (BSTR)L"Task1", 0);
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/8166

[PATCH 3/3] taskschd/regtask: Implements IRegisteredTaskCollection get_Count().

[Jacob Czekalla]

From: Jacob Czekalla <jczekalla(a)codeweavers.com>

---
 dlls/taskschd/regtask.c         | 25 ++++++++++++++++++++++---
 dlls/taskschd/tests/scheduler.c |  3 ---
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/dlls/taskschd/regtask.c b/dlls/taskschd/regtask.c
index 744f584b5a1..2239809b1ca 100644
--- a/dlls/taskschd/regtask.c
+++ b/dlls/taskschd/regtask.c
@@ -464,9 +464,28 @@ static HRESULT WINAPI regtasks_Invoke(IRegisteredTaskCollection *iface, DISPID d
 
 static HRESULT WINAPI regtasks_get_Count(IRegisteredTaskCollection *iface, LONG *count)
 {
-    FIXME("%p,%p: stub\n", iface, count);
-    if (count) *count = 0;
-    return E_NOTIMPL;
+    HRESULT hr;
+    RegisteredTaskCollection *reg_tasks = impl_from_IRegisteredTaskCollection(iface);
+    TASK_NAMES task_names = NULL;
+    DWORD start_index = 0, num_tasks = 0, i = 0;
+
+    if (!count)
+        return E_POINTER;
+
+    hr = SchRpcEnumTasks(reg_tasks->path, 0, &start_index, 0, &num_tasks, &task_names);
+    if (FAILED(hr))
+    {
+        *count = 0;
+        return hr;
+    }
+
+    *count = num_tasks;
+
+    for (;i < num_tasks; i++)
+        MIDL_user_free(task_names[i]);
+    MIDL_user_free(task_names);
+
+    return S_OK;
 }
 
 static HRESULT WINAPI regtasks_get_Item(IRegisteredTaskCollection *iface, VARIANT index, IRegisteredTask **regtask)
diff --git a/dlls/taskschd/tests/scheduler.c b/dlls/taskschd/tests/scheduler.c
index b09fa8216ef..63f8fa786e7 100644
--- a/dlls/taskschd/tests/scheduler.c
+++ b/dlls/taskschd/tests/scheduler.c
@@ -1917,13 +1917,10 @@ static void test_get_Count_and_Item(void)
 
     /* Test get_Count */
     hr = IRegisteredTaskCollection_get_Count(tasks, NULL);
-    todo_wine
     ok(hr == E_POINTER, "expected E_POINTER, got %#lx\n", hr);
 
     hr = IRegisteredTaskCollection_get_Count(tasks, &num_tasks);
-    todo_wine
     ok(hr == S_OK, "expected S_OK, got %#lx\n", hr);
-    todo_wine
     ok(num_tasks == 1, "expected 1 task, got %ld\n", num_tasks);
 
     /* Test get_Item */
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/8166

[Alfred Agrell (＠Alcaro)]

Alfred Agrell (@Alcaro) commented about dlls/taskschd/regtask.c:
> +
> +    *count = num_tasks;
> +
> +    for (;i < num_tasks; i++)
> +        MIDL_user_free(task_names[i]);
> +    MIDL_user_free(task_names);
> +
> +    return S_OK;
>  }
>
> static HRESULT WINAPI regtasks_get_Item(IRegisteredTaskCollection *iface, VARIANT index, IRegisteredTask **regtask)
>  {
> -    FIXME("%p,%s,%p: stub\n", iface, debugstr_variant(&index), regtask);
> -    return E_NOTIMPL;
> +    HRESULT hr;
> +    WCHAR *xml = NULL;
> Unused variable, mind deleting it?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8166#note_104802

[Alfred Agrell (＠Alcaro)]

Alfred Agrell (@Alcaro) commented about dlls/taskschd/tests/scheduler.c:
> ITaskService_Release(service);
>  }
>
> +static void test_get_Count_and_Item(void)
> +{
> +    HRESULT hr;
> +    LONG num_tasks;
> +    VARIANT v_null, index;
> +    ITaskService *service;
> +    ITaskFolder *folder, *root;

Are these two objects leaked?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8166#note_104806

[Alfred Agrell (＠Alcaro)]

Alfred Agrell (@Alcaro) commented about dlls/taskschd/tests/scheduler.c:
> +
> +    hr = IRegisteredTaskCollection_get_Item(tasks, index, NULL);
> +    ok(hr == E_POINTER, "expected E_POINTER, got %#lx\n", hr);
> +
> +    index.uiVal = 0;
> +    hr = IRegisteredTaskCollection_get_Item(tasks, index, &ret_task1);
> +    ok(hr == E_INVALIDARG, "expected E_INVALIDARG, got %#lx\n", hr);
> +
> +    index.uiVal = 2;
> +    hr = IRegisteredTaskCollection_get_Item(tasks, index, &ret_task1);
> +    ok(hr == E_INVALIDARG, "expected E_INVALIDARG, got %#lx\n", hr);
> +
> +    index.uiVal = 1;
> +    hr = IRegisteredTaskCollection_get_Item(tasks, index, &ret_task1);
> +    ok(hr == S_OK, "expected S_OK, got %#lx\n", hr);
> +    if (hr == S_OK)
> If the ok() passes on everything, then the if is unnecessary. (And if it doesn't, you should add a broken() or win_skip() somewhere.)

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8166#note_104805

[Alfred Agrell (＠Alcaro)]

Alfred Agrell (@Alcaro) commented about dlls/taskschd/regtask.c:
> +    if (FAILED(hr))
> +    {
> +        VariantClear(&converted_index);
> +        return hr;
> +    }
> +    if (!task_names)
> +    {
> +        VariantClear(&converted_index);
> +        return E_INVALIDARG;
> +    }
> +
> +    hr = TaskDefinition_create(&definition);
> +    if (FAILED(hr))
> +        goto cleanup;
> +    hr = RegisteredTask_create(collection->path, task_names[0], definition, TASK_VALIDATE_ONLY, TASK_LOGON_INTERACTIVE_TOKEN, regtask, FALSE);
> +    if (FAILED(hr))
> Other RegisteredTask_create callers Release the definition object if hr != S_OK.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8166#note_104804

[Alfred Agrell (＠Alcaro)]

Alfred Agrell (@Alcaro) commented about dlls/taskschd/regtask.c:
> +    return S_OK;
>  }
>
> static HRESULT WINAPI regtasks_get_Item(IRegisteredTaskCollection *iface, VARIANT index, IRegisteredTask **regtask)
>  {
> -    FIXME("%p,%s,%p: stub\n", iface, debugstr_variant(&index), regtask);
> -    return E_NOTIMPL;
> +    HRESULT hr;
> +    WCHAR *xml = NULL;
> +    VARIANT converted_index;
> +    TASK_NAMES task_names = NULL;
> +    ITaskDefinition *definition = NULL;
> +    RegisteredTaskCollection *collection = NULL;
> +    DWORD start_index = 0, num_tasks = 0;
> +
> +    collection = impl_from_IRegisteredTaskCollection(iface);
> The other RegisteredTaskCollection* is initialized in the declaration section. I have no particular opinion in either direction, but better pick one.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8166#note_104803

[Alfred Agrell (＠Alcaro)]

Good start. Functionality looks good to me (I didn't test it, but the added tests look pretty comprehensive), just a few mem leaks and other oddities.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8166#note_104807

[Jacob Czekalla (＠JacobCzekalla)]

On Wed May 28 13:59:15 2025 +0000, Alfred Agrell wrote:
> If the ok() passes on everything, then the if is unnecessary. (And if it
> doesn't, you should add a broken() or win_skip() somewhere.)
> Good point. I am also not testing other functions so I should assume they work as intended,  therefore I will get rid of my other if statement below that as well.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8166#note_104810