microsoft / cppwinrt

C++/WinRT
MIT License
1.64k stars 236 forks source link

Feature Request: Allow partial custom activation handlers #1345

Closed CarlosNihelton closed 11 months ago

CarlosNihelton commented 1 year ago

Version

C++/WinRT v2.0.220110.5

Summary

The conversation https://github.com/microsoft/cppwinrt/pull/477#discussion_r1300499404 suggests that it's desirable to have a way for an activation handler to return back to the default procedure in case it's not interested in a certain class. That's useful when tests deal with a couple of WinRT types, but only part of them are to be mocked.

The inclusion of the global winrt_activation_handler in #477 requires the custom activation handler to deal with all possible activation scenarios. In certain kinds of tests that might be an overkill.

A small change as suggested below would allow handlers by convention to return a magic value meaning they cannot handle that activation and the original procedure should take over.

        if (winrt_activation_handler)
        {
            if (auto hr = winrt_activation_handler(*(void**)(&name), guid_of<Interface>(), result);
                hr != impl::error_not_implemented) {
                    return hr;
                }
        }

Augmenting test\custom_activation.cpp to try instantiating some other class the custom_factory is not interested in would result in a uncorrectable failure (even if we'd remove the REQUIRE clauses from the handler and the custom_factory implementation code).

Reproducible example

// in the end of `test\custom_activation.cpp`
// An activation handler that doesn't handle WwwFormUrlDecoder activation.
bool invoked_unimplemented = false;
int32_t __stdcall another_handler(void* classId, winrt::guid const& iid, void** factory) noexcept
{
    if (iid == guid_of<IWwwFormUrlDecoderRuntimeClassFactory>()) {
        invoked_unimplemented = true;
        return winrt::impl::error_not_implemented;
    }
    *factory = detach_abi(make<custom_factory>());
    return 0;
}

TEST_CASE("partial_custom_activation")
{
    clear_factory_cache();

    // Set up global handler
    REQUIRE(!winrt_activation_handler);
    winrt_activation_handler = another_handler;
    REQUIRE(!invoked_unimplemented);

    // Activates something the handler cannot deal with. It shouldn't crash and the decoder should behave normally.
    WwwFormUrlDecoder decoder{ L"uname=c&passwd=d" };
    REQUIRE(invoked_unimplemented);
    REQUIRE(decoder.GetFirstValueByName(L"uname") == L"c");
    REQUIRE(decoder.GetFirstValueByName(L"passwd") == L"d");

    // Remove global handler
    winrt_activation_handler = nullptr;
    clear_factory_cache();
}

Expected behavior

No failure, no crash, the custom handler would defer activating the unhandled classes to the default procedure.

Actual behavior

The custom handler must handle all possible activations or crash (SIGSEGV)

Additional comments

No response

kennykerr commented 1 year ago

I've tagged a few project maintainers. If one of them is available to offer guidance and mentorship for this issue, they will reach out according to the contributing guide to discuss and agree on an approach.

@microsoft/cppwinrt-maintainers

github-actions[bot] commented 12 months ago

This issue is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days.