Created attachment 448310 [details] crash log The following API test is consistently asserting on macOS debug bots since it was added with https://trac.webkit.org/changeset/287547/webkit TestWebKitAPI.WKAttachmentTestsMac.InsertNonExistentImageFileAsAttachment ASSERTION FAILED: pathnames.size() == sandboxExtensionsHandleArray.size() /Volumes/Data/worker/bigsur-debug/build/Source/WebKit/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp(158) : virtual void WebKit::WebPlatformStrategies::getPathnamesForType(Vector<WTF::String> &, const WTF::String &, const WTF::String &, const WebCore::PasteboardContext *) 1 0x7c256b599 WTFCrash 2 0x788004d2b WTFCrashWithInfo(int, char const*, char const*, int) 3 0x78a4c7f95 WebKit::WebPlatformStrategies::getPathnamesForType(WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&, WTF::String const&, WTF::String const&, WebCore::PasteboardContext const*) 4 0x79f15af31 WebCore::Pasteboard::read(WebCore::PasteboardWebContentReader&, WebCore::WebContentReadingPolicy, std::__1::optional<unsigned long>) 5 0x79db115c8 WebCore::Editor::webContentFromPasteboard(WebCore::Pasteboard&, WebCore::SimpleRange const&, bool, bool&) 6 0x79dbd58d6 WebCore::Editor::pasteWithPasteboard(WebCore::Pasteboard*, WTF::OptionSet<WebCore::Editor::PasteOption>) 7 0x7a070fdce WebCore::Editor::paste(WebCore::Pasteboard&, WebCore::Editor::FromMenuOrKeyBinding) 8 0x7a070fc73 WebCore::Editor::paste(WebCore::Editor::FromMenuOrKeyBinding) 9 0x7a073fe64 WebCore::executePaste(WebCore::Frame&, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&) 10 0x7a0711dbb WebCore::Editor::Command::execute(WTF::String const&, WebCore::Event*) const 11 0x78a74e7bf WebKit::WebPage::executeEditingCommand(WTF::String const&, WTF::String const&) 12 0x78a74e645 WebKit::WebPage::executeEditCommand(WTF::String const&, WTF::String const&) 13 0x78a74e609 WebKit::WebPage::executeEditCommandWithCallback(WTF::String const&, WTF::String const&, WTF::CompletionHandler<void ()>&&) 14 0x78a812d6e void IPC::callMemberFunctionImpl<WebKit::WebPage, void (WebKit::WebPage::*)(WTF::String const&, WTF::String const&, WTF::CompletionHandler<void ()>&&), void (), std::__1::tuple<WTF::String, WTF::String>, 0ul, 1ul>(WebKit::WebPage*, void (WebKit::WebPage::*)(WTF::String const&, WTF::String const&, WTF::CompletionHandler<void ()>&&), WTF::CompletionHandler<void ()>&&, std::__1::tuple<WTF::String, WTF::String>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul>) 15 0x78a812265 void IPC::callMemberFunction<WebKit::WebPage, void (WebKit::WebPage::*)(WTF::String const&, WTF::String const&, WTF::CompletionHandler<void ()>&&), void (), std::__1::tuple<WTF::String, WTF::String>, std::__1::integer_sequence<unsigned long, 0ul, 1ul> >(std::__1::tuple<WTF::String, WTF::String>&&, WTF::CompletionHandler<void ()>&&, WebKit::WebPage*, void (WebKit::WebPage::*)(WTF::String const&, WTF::String const&, WTF::CompletionHandler<void ()>&&)) 16 0x78a7ccb9f void IPC::handleMessageAsync<Messages::WebPage::ExecuteEditCommandWithCallback, WebKit::WebPage, void (WebKit::WebPage::*)(WTF::String const&, WTF::String const&, WTF::CompletionHandler<void ()>&&)>(IPC::Connection&, IPC::Decoder&, WebKit::WebPage*, void (WebKit::WebPage::*)(WTF::String const&, WTF::String const&, WTF::CompletionHandler<void ()>&&)) 17 0x78a7c4a5e WebKit::WebPage::didReceiveWebPageMessage(IPC::Connection&, IPC::Decoder&) 18 0x78a76273e WebKit::WebPage::didReceiveMessage(IPC::Connection&, IPC::Decoder&) 19 0x7895618b5 IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&) 20 0x789ff1277 WebKit::WebProcess::didReceiveMessage(IPC::Connection&, IPC::Decoder&) 21 0x78951e074 IPC::Connection::dispatchMessage(IPC::Decoder&) 22 0x78951e80f IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) 23 0x78951ee40 IPC::Connection::dispatchOneIncomingMessage() 24 0x78953caf8 IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14::operator()() 25 0x78953c9ee WTF::Detail::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14, void>::call() 26 0x7c2599a12 WTF::Function<void ()>::operator()() const 27 0x7c262f7b0 WTF::RunLoop::performWork() 28 0x7c26341e1 WTF::RunLoop::performWork(void*) 29 0x7fff206f697c __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ 30 0x7fff206f68e4 __CFRunLoopDoSource0 31 0x7fff206f6664 __CFRunLoopDoSources0 https://results.webkit.org/?suite=api-tests&test=TestWebKitAPI.WKAttachmentTestsMac.InsertNonExistentImageFileAsAttachment
<rdar://problem/87100377>
It seems like I added the first test that attempts to paste an attachment with a missing file :/ At a glance, we can probably adjust the assertion (either removing it entirely or accounting for missing files, somehow).
Created attachment 448335 [details] Patch
Comment on attachment 448335 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=448335&action=review > Source/WebKit/UIProcess/Cocoa/WebPasteboardProxyCocoa.mm:175 > // On iOS, files are copied into app's container upon paste. This is a confusing comment, because it’s in a Mac-specific code section, but says "On iOS". > Source/WebKit/UIProcess/Cocoa/WebPasteboardProxyCocoa.mm:180 > + return SandboxExtension::createHandle(filename, SandboxExtension::Type::ReadOnly).value_or(SandboxExtension::Handle { }); Can we omit the SandboxExtension::Handle in the value_or call?
Comment on attachment 448335 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=448335&action=review Thanks for the review! >> Source/WebKit/UIProcess/Cocoa/WebPasteboardProxyCocoa.mm:175 >> // On iOS, files are copied into app's container upon paste. > > This is a confusing comment, because it’s in a Mac-specific code section, but says "On iOS". Good catch — I moved this comment to just outside of the `PLATFORM(MAC)` guard. >> Source/WebKit/UIProcess/Cocoa/WebPasteboardProxyCocoa.mm:180 >> + return SandboxExtension::createHandle(filename, SandboxExtension::Type::ReadOnly).value_or(SandboxExtension::Handle { }); > > Can we omit the SandboxExtension::Handle in the value_or call? So I tried to do this at first, but ran into compilation errors trying to call value_or :( I was able to replicate the error in a simple C++ test case: ```(main.cpp): struct Foo { int value { 0 }; }; int main(int argc, const char * argv[]) { std::optional<Foo> foo; std::cout << foo.value_or({ }).value << std::endl; } ``` with output: ``` clang++ -std=c++17 main.cpp main.cpp:20:22: error: no matching member function for call to 'value_or' std::cout << foo.value_or({ }).value << std::endl; ~~~~^~~~~~~~ […] usr/include/c++/v1/optional:986:26: note: candidate template ignored: couldn't infer template argument '_Up' constexpr value_type value_or(_Up&& __v) const& ^ […] usr/include/c++/v1/optional:998:26: note: candidate template ignored: couldn't infer template argument '_Up' constexpr value_type value_or(_Up&& __v) && ^ 1 error generated. make: *** [main.o] Error 1 ``` I think I'll leave this as-is, for now.
So sad that value_or doesn’t provide enough context. We might want to create a helper that does what value_or does, but uses default initialization of the type stored in the optional.
Created attachment 448343 [details] For landing
(In reply to Darin Adler from comment #7) > So sad that value_or doesn’t provide enough context. We might want to create > a helper that does what value_or does, but uses default initialization of > the type stored in the optional. Yes, that would be nice! Perhaps, something along these lines? (As a helper function in `wtf/StdLibExtras.h`): ``` template<typename T> T valueOrDefault(const std::optional<T>& optional) { return optional.value_or(T { }); } ``` (Ex.) ``` int main(int argc, const char * argv[]) { std::optional<Foo> foo; std::cout << valueOrDefault(foo).value << std::endl; } ```
(In reply to Wenson Hsieh from comment #9) > Perhaps, something along these lines? Yes, exactly. Maybe with an even shorter name if we can figure it out.
(In reply to Darin Adler from comment #10) > (In reply to Wenson Hsieh from comment #9) > > Perhaps, something along these lines? > > Yes, exactly. Maybe with an even shorter name if we can figure it out. Sounds good! I'll create a followup for this. A quick search for \.value_or\(\w+ \{\s\} shows 27 hits across WebKit, so it definitely seems like a useful thing to have.
Committed r287600 (245728@main): <https://commits.webkit.org/245728@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 448343 [details].