Bug 208599

Summary: Lay initial groundwork for new PDF loading model
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebKit Misc.Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, benjamin, cdumez, cmarcelo, commit-queue, ews-watchlist, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
achristensen: review+
PFL
none
PFL
none
PFL none

Brady Eidson
Reported 2020-03-04 13:47:07 PST
Lay initial groundwork for new PDF loading model
Attachments
Patch (17.36 KB, patch)
2020-03-04 14:08 PST, Brady Eidson
no flags
Patch (18.88 KB, patch)
2020-03-04 15:15 PST, Brady Eidson
achristensen: review+
PFL (18.88 KB, patch)
2020-03-04 15:58 PST, Brady Eidson
no flags
PFL (18.96 KB, patch)
2020-03-04 16:00 PST, Brady Eidson
no flags
PFL (18.96 KB, patch)
2020-03-04 16:42 PST, Brady Eidson
no flags
Brady Eidson
Comment 1 2020-03-04 14:08:44 PST
Alex Christensen
Comment 2 2020-03-04 14:14:59 PST
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>?
Brady Eidson
Comment 3 2020-03-04 15:15:33 PST
Alex Christensen
Comment 4 2020-03-04 15:22:25 PST
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.
Brady Eidson
Comment 5 2020-03-04 15:58:57 PST
Brady Eidson
Comment 6 2020-03-04 16:00:19 PST
Brady Eidson
Comment 7 2020-03-04 16:42:42 PST
WebKit Commit Bot
Comment 8 2020-03-04 21:26:19 PST
Comment on attachment 392508 [details] PFL Clearing flags on attachment: 392508 Committed r257900: <https://trac.webkit.org/changeset/257900>
WebKit Commit Bot
Comment 9 2020-03-04 21:26:21 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 10 2020-03-04 21:27:39 PST
Note You need to log in before you can comment on or make changes to this bug.