Summary: | Use Span for IPC::ArrayReference and DataReference | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alex Christensen <achristensen> | ||||||
Component: | New Bugs | Assignee: | Alex Christensen <achristensen> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | benjamin, cdumez, cmarcelo, eric.carlson, ews-watchlist, wenson_hsieh | ||||||
Priority: | P2 | ||||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=235889 | ||||||||
Attachments: |
|
Description
Alex Christensen
2021-12-07 08:59:59 PST
Created attachment 446181 [details]
Patch
Created attachment 446195 [details]
Patch
Comment on attachment 446195 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=446195&action=review > Source/WTF/wtf/Vector.h:657 > + Vector(Span<const T> span) Would it be better to take a `const Span<const T>&` instead? > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:1643 > + if (pdfData.empty()) { It does seem a bit odd that while Span uses `bool empty()`, all other WTF classes use `bool isEmpty()`. Maybe we should rename this for consistency at some point.. (In reply to Wenson Hsieh from comment #3) > Comment on attachment 446195 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=446195&action=review > > > Source/WTF/wtf/Vector.h:657 > > + Vector(Span<const T> span) > > Would it be better to take a `const Span<const T>&` instead? No, a Span is two registers so passed by value, like a StringView, and this is inline anyways. > > > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:1643 > > + if (pdfData.empty()) { > > It does seem a bit odd that while Span uses `bool empty()`, all other WTF > classes use `bool isEmpty()`. Maybe we should rename this for consistency at > some point.. WTF::Span is an implementation of std::span, which we'll switch to after turning on C++20. std::span::empty exists but std::span::isEmpty does not. |