| Summary: | Add support for sending Expected<void, E> in IPC messages | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | BJ Burg <bburg> | ||||||
| Component: | WebKit Misc. | Assignee: | Devin Rousso <hi> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | bburg, benjamin, cdumez, cmarcelo, ews-watchlist, hi, joepeck, pangle, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=230580 | ||||||||
| Attachments: |
|
||||||||
Created attachment 439513 [details]
Patch
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. oh whoops I put this in the wrong spot good catch @Chris Dumez! ill just wait for the related patch to land and then move this to the right spot :) 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>> { }` Created attachment 439551 [details]
Patch
Comment on attachment 439551 [details]
Patch
Web Inspector-related changes look good to me. r+ if Chris is OK with the ArgumentCoder changes.
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]. |
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