Bug 232520 - Web process shouldn't crash if ImageBuffer::ensureBackendCreated() fails
Summary: Web process shouldn't crash if ImageBuffer::ensureBackendCreated() fails
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: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on: 234680
Blocks: 225377
  Show dependency treegraph
 
Reported: 2021-10-29 17:52 PDT by Myles C. Maxfield
Modified: 2022-01-07 11:57 PST (History)
6 users (show)

See Also:


Attachments
Patch (4.59 KB, patch)
2021-10-30 00:49 PDT, Myles C. Maxfield
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2021-10-29 17:52:30 PDT
RemoteLayerBackingStore::encode() assumes it can never fail, but it can. There may be other places, too.
Comment 1 Radar WebKit Bug Importer 2021-10-29 17:53:17 PDT
<rdar://problem/84829995>
Comment 2 Myles C. Maxfield 2021-10-30 00:49:40 PDT
Created attachment 442906 [details]
Patch
Comment 3 Cameron McCormack (:heycam) 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.
Comment 4 Myles C. Maxfield 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.
Comment 5 Myles C. Maxfield 2021-10-30 19:37:31 PDT
Committed r285088 (243730@main): <https://commits.webkit.org/243730@main>
Comment 6 WebKit Commit Bot 2021-12-25 02:11:56 PST
Re-opened since this is blocked by bug 234680
Comment 7 Myles C. Maxfield 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.
Comment 8 Myles C. Maxfield 2022-01-07 11:57:20 PST
Committed r287775 (245835@trunk): <https://commits.webkit.org/245835@trunk>