| Summary: | ASSERTION FAILED: pathnames.size() == sandboxExtensionsHandleArray.size() in WebKit::WebPlatformStrategies::getPathnamesForType() | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Ryan Haddad <ryanhaddad> | ||||||||
| Component: | New Bugs | Assignee: | Wenson Hsieh <wenson_hsieh> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | akeerthi, darin, hi, megan_gardner, thorton, webkit-bot-watchers-bugzilla, webkit-bug-importer, wenson_hsieh | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| See Also: |
https://bugs.webkit.org/show_bug.cgi?id=234803 https://bugs.webkit.org/show_bug.cgi?id=234865 |
||||||||||
| Attachments: |
|
||||||||||
|
Description
Ryan Haddad
2022-01-04 10:50:28 PST
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. 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]. |