Bug 218144 - Add null checks and smart pointers in PDF and Plugin code
Summary: Add null checks and smart pointers in PDF and Plugin code
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: InRadar
Depends on:
Blocks:
 
Reported: 2020-10-23 18:17 PDT by Alex Christensen
Modified: 2020-10-26 13:06 PDT (History)
4 users (show)

See Also:


Attachments
Patch (29.90 KB, patch)
2020-10-23 18:29 PDT, Alex Christensen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (29.91 KB, patch)
2020-10-23 18:40 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (30.90 KB, patch)
2020-10-26 10:48 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2020-10-23 18:17:23 PDT
Add null checks and smart pointers in PDF and Plugin code
Comment 1 Alex Christensen 2020-10-23 18:29:24 PDT
Created attachment 412220 [details]
Patch
Comment 2 Alex Christensen 2020-10-23 18:40:02 PDT
Created attachment 412221 [details]
Patch
Comment 3 youenn fablet 2020-10-26 10:40:38 PDT
Comment on attachment 412221 [details]
Patch

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

> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:620
> +        Document* document = frame.coreFrame()->document();

auto

> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:1855
> +    m_frame->page()->updatePDFHUDLocation(*this, frameForHUD());

Are we sure page is non null?

> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:1922
>      LocalDefaultSystemAppearance localAppearance(page->useDarkAppearance());

Check page?

> Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:2302
> +    if (!m_frame)

Could do if (!m_frame || !m_frame->coreFrame()).

> Source/WebKit/WebProcess/WebPage/WebFrame.h:160
> +    void setLoadListener(LoadListener* loadListener) { m_loadListener = makeWeakPtr(loadListener); }

Could we pass a LoadListener&?
Comment 4 Alex Christensen 2020-10-26 10:46:12 PDT
Comment on attachment 412221 [details]
Patch

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

>> Source/WebKit/WebProcess/WebPage/WebFrame.h:160
>> +    void setLoadListener(LoadListener* loadListener) { m_loadListener = makeWeakPtr(loadListener); }
> 
> Could we pass a LoadListener&?

nope, but I'll update the old call sites that call it with 0 to call it with nullptr.
Comment 5 Alex Christensen 2020-10-26 10:48:53 PDT
Created attachment 412335 [details]
Patch
Comment 6 EWS 2020-10-26 11:41:02 PDT
Committed r268989: <https://trac.webkit.org/changeset/268989>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 412335 [details].
Comment 7 Radar WebKit Bug Importer 2020-10-26 11:42:18 PDT
<rdar://problem/70688355>
Comment 8 David Kilzer (:ddkilzer) 2020-10-26 13:06:19 PDT
<rdar://problem/69931543>