Bug 232520

Summary:

Web process shouldn't crash if ImageBuffer::ensureBackendCreated() fails

Product:

WebKit

Reporter:

Myles C. Maxfield <mmaxfield>

Component:

Layout and Rendering

Assignee:

Myles C. Maxfield <mmaxfield>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, commit-queue, heycam, simon.fraser, webkit-bug-importer, zalan

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

234680

Bug Blocks:

225377

Attachments:

Description	Flags
Patch	simon.fraser: review+

Myles C. Maxfield

Reported 2021-10-29 17:52:30 PDT

RemoteLayerBackingStore::encode() assumes it can never fail, but it can. There may be other places, too.

Attachments
Patch (4.59 KB, patch) 2021-10-30 00:49 PDT, Myles C. Maxfield	simon.fraser: review+	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Note You need to log in before you can comment on or make changes to this bug.

Radar WebKit Bug Importer

Comment 1 2021-10-29 17:53:17 PDT

<rdar://problem/84829995>

Myles C. Maxfield

Comment 2 2021-10-30 00:49:40 PDT

Created attachment 442906 [details] Patch

Cameron McCormack (:heycam)

Comment 3 2021-10-30 14:48:04 PDT

Comment on attachment 442906 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=442906&action=review > Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm:121 > + if (auto* backend = m_frontBuffer.imageBuffer->ensureBackendCreated()) Since we do this check in all situations -- mapped IOSurface, non-mapped IOSurface, bitmap -- maybe do it once before the switch.

Myles C. Maxfield

Comment 4 2021-10-30 19:34:53 PDT

Comment on attachment 442906 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=442906&action=review >> Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm:121 >> + if (auto* backend = m_frontBuffer.imageBuffer->ensureBackendCreated()) > > Since we do this check in all situations -- mapped IOSurface, non-mapped IOSurface, bitmap -- maybe do it once before the switch. The third case below is slightly different, but yes I can hoist it somewhat.

Myles C. Maxfield

Comment 5 2021-10-30 19:37:31 PDT

Committed r285088 (243730@main): <https://commits.webkit.org/243730@main>

WebKit Commit Bot

Comment 6 2021-12-25 02:11:56 PST

Re-opened since this is blocked by bug 234680

Myles C. Maxfield

Comment 7 2022-01-07 11:53:54 PST

The A/B test was using bogus data - before crashes were fixed, the memory data was reported from processes which didn't have the test page loaded. So, rolling this out was a mistake. Rolling back in now.

Myles C. Maxfield

Comment 8 2022-01-07 11:57:20 PST

Committed r287775 (245835@trunk): <https://commits.webkit.org/245835@trunk>