Bug 241435 - RenderImageResource::setCachedImage may produce a null renderer
Summary: RenderImageResource::setCachedImage may produce a null renderer
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords: InRadar
: 241512 (view as bug list)
Depends on:
Blocks:
 
Reported: 2022-06-08 13:33 PDT by zalan
Modified: 2022-06-14 09:41 PDT (History)
10 users (show)

See Also:

Attachments
[fast-cq]Patch (1.26 KB, patch)
2022-06-08 13:37 PDT, zalan 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description zalan 2022-06-08 13:33:29 PDT 
<rdar://93714107>
Comment 1 zalan 2022-06-08 13:37:42 PDT 
Created attachment 460101 [details]
[fast-cq]Patch
Comment 2 Brent Fulgham 2022-06-08 13:42:09 PDT 
Comment on attachment 460101 [details]
[fast-cq]Patch

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

> Source/WebCore/rendering/RenderImageResource.cpp:67
> +        // removeClient may have destroyed the renderer.

Don't we still want to set the m_cachedimage value below (even though the existing renderer was possible destroyed)?
Comment 3 Brent Fulgham 2022-06-08 13:51:09 PDT 
Comment on attachment 460101 [details]
[fast-cq]Patch

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

>> Source/WebCore/rendering/RenderImageResource.cpp:67
>> +        // removeClient may have destroyed the renderer.
> 
> Don't we still want to set the m_cachedimage value below (even though the existing renderer was possible destroyed)?

I spoke to Alan offline. If 'm_renderer' is nullptr, we expect 'this' to be nullptr as well, so we should early return.
Comment 4 EWS 2022-06-08 14:02:24 PDT 
Committed r295393 (251399@main): <https://commits.webkit.org/251399@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 460101 [details].
Comment 5 Antoine Quint 2022-06-14 08:15:07 PDT 
*** Bug 241512 has been marked as a duplicate of this bug. ***
Comment 6 Simon Fraser (smfr) 2022-06-14 08:49:18 PDT 
Bug 241512 has a user-visible symptom, suggesting that we could create a layout test for this patch.
Comment 7 zalan 2022-06-14 09:33:25 PDT 
(In reply to Simon Fraser (smfr) from comment #6)
> Bug 241512 has a user-visible symptom, suggesting that we could create a
> layout test for this patch.
That would be awesome given the speculative nature of this fix.
Comment 8 zalan 2022-06-14 09:41:29 PDT 
Yeah I can repro it (and it's <rdar://94689000>)