Bug 226411

Summary:

Web process terminates when attempting to log in on bankid.no

Product:

WebKit

Reporter:

Wenson Hsieh <wenson_hsieh>

Component:

Layout and Rendering

Assignee:

Wenson Hsieh <wenson_hsieh>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, cdumez, sabouhallawa, simon.fraser, thorton, webkit-bug-importer, zalan

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
For EWS	none
Patch	none

Description Wenson Hsieh 2021-05-28 18:33:05 PDT

rdar://77419432

Comment 1 Wenson Hsieh 2021-05-28 19:19:20 PDT

Created attachment 430082 [details]
For EWS

Comment 2 Chris Dumez 2021-05-28 20:18:06 PDT

Comment on attachment 430082 [details]
For EWS

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

> Source/WebCore/platform/graphics/displaylists/DisplayListReplayer.h:60
> +    WEBCORE_EXPORT Replayer(GraphicsContext&, const DisplayList&, const ImageBufferHashMap* = nullptr, const NativeImageHashMap* = nullptr, const FontRenderingResourceMap* = nullptr, WebCore::ImageBuffer* maskImageBuffer = { }, Delegate* = nullptr);

nit: = nullptr would look clearer IMO.

> Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp:501
> +    m_currentMaskImageBuffer = makeRefPtr(imageBuffer);

m_currentMaskImageBuffer = &imageBuffer;

should work

> Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.h:87
> +    void didCreateMaskImageBuffer(WebCore::ImageBuffer&);

Shouldn't this be marked as final / override?

> Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.h:88
> +    void didResetMaskImageBuffer();

ditto.

Comment 3 Wenson Hsieh 2021-05-28 20:20:41 PDT

Comment on attachment 430082 [details]
For EWS

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

>> Source/WebCore/platform/graphics/displaylists/DisplayListReplayer.h:60
>> +    WEBCORE_EXPORT Replayer(GraphicsContext&, const DisplayList&, const ImageBufferHashMap* = nullptr, const NativeImageHashMap* = nullptr, const FontRenderingResourceMap* = nullptr, WebCore::ImageBuffer* maskImageBuffer = { }, Delegate* = nullptr);
> 
> nit: = nullptr would look clearer IMO.

Good point. Changed to `nullptr`.

>> Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp:501
>> +    m_currentMaskImageBuffer = makeRefPtr(imageBuffer);
> 
> m_currentMaskImageBuffer = &imageBuffer;
> 
> should work

Changed to `m_currentMaskImageBuffer = &imageBuffer;`!

>> Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.h:87
>> +    void didCreateMaskImageBuffer(WebCore::ImageBuffer&);
> 
> Shouldn't this be marked as final / override?

Ah, so these are just methods on RemoteRenderingBackend; the overridden methdos are in `RemoteImageBuffer`, where they are marked as `final`.

(The RemoteImageBuffer methods are just plumbing directly into `RemoteRenderingBackend`)

Comment 4 Wenson Hsieh 2021-05-28 20:26:13 PDT

Created attachment 430086 [details]
Patch

Comment 5 Wenson Hsieh 2021-05-28 22:07:48 PDT

Comment on attachment 430086 [details]
Patch

Thank you for the review!

Comment 6 EWS 2021-05-28 22:27:52 PDT

Committed r278235 (238272@main): <https://commits.webkit.org/238272@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 430086 [details].