Bug 233931 - Use Span for IPC::ArrayReference and DataReference
Summary: Use Span for IPC::ArrayReference and DataReference
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-12-07 08:59 PST by Alex Christensen
Modified: 2022-01-31 05:25 PST (History)
6 users (show)

See Also:


Attachments
Patch (27.11 KB, patch)
2021-12-07 09:01 PST, Alex Christensen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (28.13 KB, patch)
2021-12-07 10:00 PST, Alex Christensen
wenson_hsieh: review+
achristensen: commit-queue+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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