Bug 230907 – Add support for sending Expected<void, E> in IPC messages

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Blaze Burg]

While developing WebInspectorUIExtensionController it became apparent that Expected<void, Inspector::ExtensionError> does not work as expected. The first of many compilation errors looks something like:

In file included from /Users/bburg/repos/webkit/OpenSource/WebKitBuild/Release/DerivedSources/WebKit2/WebInspectorUIExtensionControllerMessageReceiver.cpp:29:
/Users/bburg/repos/webkit/OpenSource/Source/WebKit/Platform/IPC/ArgumentCoders.h:658:38: note: in instantiation of template class 'std::optional<void>' requested here
            std::optional<ValueType> value;
In file included from /Users/bburg/repos/webkit/OpenSource/WebKitBuild/Release/DerivedSources/WebKit2/WebInspectorUIExtensionControllerMessageReceiver.cpp:27:
In file included from /Users/bburg/repos/webkit/OpenSource/Source/WebKit/WebProcess/Inspector/WebInspectorUIExtensionController.h:30:
In file included from /Users/bburg/repos/webkit/OpenSource/Source/WebKit/Platform/IPC/Connection.h:31:
/Users/bburg/repos/webkit/OpenSource/Source/WebKit/Platform/IPC/Decoder.h:114:23: note: in instantiation of member function 'IPC::ArgumentCoder<std::experimental::expected<void, Inspector::ExtensionError>>::decode' requested here
            t = Impl::decode(*this);
/Users/bburg/repos/webkit/OpenSource/WebKitBuild/Release/DerivedSources/WebKit2/WebInspectorUIExtensionControllerMessageReceiver.cpp:180:13: note: in instantiation of function template specialization 'IPC::Decoder::operator>><std::experimental::expected<void, Inspector::ExtensionError>>' requested here
    decoder >> result;


I believe this can be fixed with a special-cased implementation in wtf/persistence/PersistentCoders.h

[Devin Rousso]

Created attachment 439513 [details]
Patch

[Chris Dumez]

Comment on attachment 439513 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=439513&action=review

> Source/WTF/ChangeLog:3
> +        Add support for sending Expected<void, E> in IPC messages

This says IPC

> Source/WTF/ChangeLog:8
> +        * wtf/persistence/PersistentCoders.h:

But it updates persistent coders that are used for serializing to disk.

Which one is right? Hard to say given that it is unused in this patch.

[Devin Rousso]

oh whoops I put this in the wrong spot

good catch @Chris Dumez!

[Devin Rousso]

ill just wait for the related patch to land and then move this to the right spot :)

[Chris Dumez]

Comment on attachment 439513 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=439513&action=review

> Source/WTF/wtf/persistence/PersistentCoders.h:121
> +            return std::optional<Expected<T, E>> { Unexpected<E> { WTFMove(*error) } };

Same comment as below.

> Source/WTF/wtf/persistence/PersistentCoders.h:128
> +        return std::optional<Expected<T, E>> { Expected<T, E> { WTFMove(*value) } };

Same comment as below.

> Source/WTF/wtf/persistence/PersistentCoders.h:156
> +            return std::optional<Expected<void, E>> { Unexpected<E> { WTFMove(*error) } };

I believe the following should work:
return Expected<void, E> { Unexpected<E> { WTFMove(*error) } };

> Source/WTF/wtf/persistence/PersistentCoders.h:159
> +        return std::optional<Expected<void, E>> { Expected<void, E> { } };

We probably don't see `std::optional<Expected<void, E>> { }`

[Devin Rousso]

Created attachment 439551 [details]
Patch

[Blaze Burg]

Comment on attachment 439551 [details]
Patch

Web Inspector-related changes look good to me. r+ if Chris is OK with the ArgumentCoder changes.

[EWS]

Committed r283220 (242262@main): <https://commits.webkit.org/242262@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 439551 [details].

[Radar WebKit Bug Importer]

<rdar://problem/83672326>