Bug 233931

Summary: Use Span for IPC::ArrayReference and DataReference
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: New BugsAssignee: 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 Flags
Patch
ews-feeder: commit-queue-
Patch wenson_hsieh: review+, achristensen: commit-queue+

Description Alex Christensen 2021-12-07 08:59:59 PST
Use Span for IPC::ArrayReference and DataReference
Comment 1 Alex Christensen 2021-12-07 09:01:29 PST
Created attachment 446181 [details]
Patch
Comment 2 Alex Christensen 2021-12-07 10:00:36 PST
Created attachment 446195 [details]
Patch
Comment 3 Wenson Hsieh 2021-12-08 14:49:41 PST
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..
Comment 4 Alex Christensen 2021-12-08 16:09:04 PST
(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.
Comment 5 Alex Christensen 2021-12-09 16:24:04 PST
r286815