Created attachment 392478 [details]
Patch

Comment on attachment 392478 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=392478&action=review

> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.h:41
> +// For now, disable new PDF APIs by default even on platforms where otherwise enabled.

FIXME: Enable this?

> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.h:126
> +    void getResourceBytesAtPosition(size_t count, off_t position, WTF::Function<void(const uint8_t*, size_t count)>&& completionHandler);

Will it be only called once?  CompletionHandler?

> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.h:328
> +        WTF::Function<void(const uint8_t*, size_t count)> completionHandler;

WTF:: is unnecessary.
CompletionHandler?

> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:585
> +    , m_pdfThread(Thread::create("PDF document thread", [protectedThis = makeRef(*this), this] { threadEntry(); }))

This looks like a ref cycle to me.

> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:714
> +    unsigned long long dataLength = m_data ? (unsigned long long)CFDataGetLength(m_data.get()) : 0;

static_cast

> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:1184
> +        if ((unsigned long long)CFDataGetLength(m_data.get()) >= request.position + request.count) {

If this case is needed, could it be static_cast<uint64_t>?

Created attachment 392493 [details]
Patch

Comment on attachment 392493 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=392493&action=review

> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.h:328
> +        size_t count;

It could prevent use of uninitialized memory to give these default initializers.

Created attachment 392500 [details]
PFL

Created attachment 392501 [details]
PFL

Created attachment 392508 [details]
PFL

Comment on attachment 392508 [details]
PFL

Clearing flags on attachment: 392508

Committed r257900: <https://trac.webkit.org/changeset/257900>

All reviewed patches have been landed.  Closing bug.

<rdar://problem/60071098>