Bug 234851 – ASSERTION FAILED: pathnames.size() == sandboxExtensionsHandleArray.size() in WebKit::WebPlatformStrategies::getPathnamesForType()

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Ryan Haddad]

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

[Radar WebKit Bug Importer]

<rdar://problem/87100377>

[Wenson Hsieh]

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).

[Wenson Hsieh]

Created attachment 448335 [details]
Patch

[Darin Adler]

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?

[Wenson Hsieh]

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.

[Wenson Hsieh]

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.

[Darin Adler]

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.

[Wenson Hsieh]

Created attachment 448343 [details]
For landing

[Wenson Hsieh]

(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;
}
```

[Darin Adler]

(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.

[Wenson Hsieh]

(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.

[EWS]

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].