WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 208599
Lay initial groundwork for new PDF loading model
https://bugs.webkit.org/show_bug.cgi?id=208599
Summary
Lay initial groundwork for new PDF loading model
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
Details
Formatted Diff
Diff
Patch
(18.88 KB, patch)
2020-03-04 15:15 PST
,
Brady Eidson
achristensen
: review+
Details
Formatted Diff
Diff
PFL
(18.88 KB, patch)
2020-03-04 15:58 PST
,
Brady Eidson
no flags
Details
Formatted Diff
Diff
PFL
(18.96 KB, patch)
2020-03-04 16:00 PST
,
Brady Eidson
no flags
Details
Formatted Diff
Diff
PFL
(18.96 KB, patch)
2020-03-04 16:42 PST
,
Brady Eidson
no flags
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Brady Eidson
Comment 1
2020-03-04 14:08:44 PST
Created
attachment 392478
[details]
Patch
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
Created
attachment 392493
[details]
Patch
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
Created
attachment 392500
[details]
PFL
Brady Eidson
Comment 6
2020-03-04 16:00:19 PST
Created
attachment 392501
[details]
PFL
Brady Eidson
Comment 7
2020-03-04 16:42:42 PST
Created
attachment 392508
[details]
PFL
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
<
rdar://problem/60071098
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug