Fix ILegacyIAccessibleProvider.GetSelection return type

[hughbe]

Proposed Changes

Running the following test

[WinFormsFact]
public void AccesibleObject_ILegacyIAccessibleProviderGetSelection_Invoke_ReturnsExpected()
{
    var o = new AccessibleObject();
    Test_ILegacyIAccessibleProviderGetSelection(o);
}

[DllImport(NativeTests, ExactSpelling = true)]
private static extern void Test_ILegacyIAccessibleProviderGetSelection(
    [MarshalAs(UnmanagedType.IUnknown)] object pUnk);

#define TEST extern "C" __declspec(dllexport)

TEST void WINAPI Test_ILegacyIAccessibleProviderGetSelection(IUnknown* pUnknown)
{
    HRESULT hr;
    ILegacyIAccessibleProvider* pLegacyIAccessibleProvider;

    hr = pUnknown->QueryInterface(IID_ILegacyIAccessibleProvider, (void**)&pLegacyIAccessibleProvider);
    assert(hr == S_OK);

#if false
    SAFEARRAY *result = (SAFEARRAY*)(long)0xdeadbeef;
    hr = pLegacyIAccessibleProvider->GetSelection(&result);
    assert(hr == S_OK);
    assert(result->cbElements == 1);
    assert(static_cast<IRawElementProviderSimple*>(result->pvData) == NULL);
    SafeArrayDestroy(result);
#endif

    // Negative tests
    hr = pLegacyIAccessibleProvider->GetSelection(NULL);
    assert(hr == S_OK);

    return S_OK;
});
}

I get an assertion failure in

hr = pLegacyIAccessibleProvider->GetSelection(&result);
assert(hr == S_OK);

Instead of S_OK, I get COR_E_SAFEARRAYTYPEMISMATCH (0x80131533) returned as the HRESULT.

Taking a look at the native definition, we seem to be returning object[] instead of an interface array. So I think we end up marshalling an array of variants (?) definitely not the interface!

From https://github.com/tpn/winsdk-10/blob/master/Include/10.0.10240.0/um/UIAutomationCore.idl#L845

HRESULT GetSelection (
    [out, retval] SAFEARRAY(IRawElementProviderSimple *) * pvarSelectedChildren );

The fix is to correct the return value.

Discussion

Following on from dotnet/runtime#35855, should we create our own SAFEARRAY type and somehow pass this in as a pointer?

Microsoft Reviewers: Open in CodeFlow

[codecov]

Codecov Report

Merging #3231 into master will decrease coverage by 31.41342%.
The diff coverage is 68.18182%.

@@                 Coverage Diff                  @@
##              master       #3231          +/-   ##
====================================================
- Coverage   66.98140%   35.56798%   -31.41342%     
====================================================
  Files           1342         896         -446     
  Lines         505621      253239      -252382     
  Branches       40889       36734        -4155     
====================================================
- Hits          338672       90072      -248600     
+ Misses        161382      158314        -3068     
+ Partials        5567        4853         -714     

Flag	Coverage Δ
#Debug	35.56798% <68.18182%> (-31.41342%)	⬇️
#production	35.56798% <68.18182%> (-0.01188%)	⬇️
#test	?

[RussKie]

👍

[RussKie]

@vladimir-krestov please review (keeping in mind #3524 issue).

[vladimir-krestov]

Sorry, I'm not good at Marshaling. So I can't say if it was made correctly.
But as I see, we have the same attribute implementation in ISelectionProvider.GetSelection() and it works well, also we have the same method implementation in AccessibleObjects (eg. ListBoxAccessibleObject.GetSelection()), they return an empty array instead null (it seems expected).
So, if it works it is great.
@hughbe, please test it using Inspect and Narrator if it is possible. Also, please try to add a unit test for this if it is possible.

@RussKie, I tested Hugh's branch and checked my case (#3524). This is doesn't fix my issue and I need to remove [ComImport] attribute anyway.

[vladimir-krestov]

Testers made testing of this fix and #3524 as ane dll. They haven't found any regressions. Their automation tests are passed. ✔️
@RussKie, FYI

[vladimir-krestov]

But these changes don't fix #3452 anyway. PR #3524 works well for it.