Bug 218887 - [GPU Process] Make getImageData directly read the shared backend instead of sending a sync IPC
Summary: [GPU Process] Make getImageData directly read the shared backend instead of s...
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-11-12 23:12 PST by Ryosuke Niwa
Modified: 2021-01-14 22:54 PST (History)
5 users (show)

See Also:


Attachments
Patch (9.58 KB, patch)
2020-11-12 23:18 PST, Ryosuke Niwa
thorton: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2020-11-12 23:12:46 PST
We can make getImageData more efficient by flushing the context and reading off of the shared backend instead of sending a sync IPC.
Comment 1 Ryosuke Niwa 2020-11-12 23:18:32 PST
Created attachment 414004 [details]
Patch
Comment 2 Tim Horton 2020-11-13 00:56:31 PST
Does this mean we're now reading from the IOSurface mapped in the Web Content process? Because that would be a mistake going forward...
Comment 3 Tim Horton 2020-11-13 01:07:29 PST
Yes, it does. We can't do this.
Comment 4 Ryosuke Niwa 2020-11-13 01:32:34 PST
(In reply to Tim Horton from comment #3)
> Yes, it does. We can't do this.

Oh, right. That was another thing wrong with this approach. I guess I'd go back to adding new shared memory region.
Comment 5 Said Abou-Hallawa 2020-11-13 08:53:35 PST
(In reply to Tim Horton from comment #3)
> Yes, it does. We can't do this.

Can you please explain why "we can't do this"?

If IOSurface/ShareableBitmap mapped in the Web Content process is read-only, it will act exactly as any shared memory we are going to create.

Is there a threat of accessing the IOSurface from the Web Content process even if this access is read-only? Can this cause any security vulnerabilities?
Comment 6 Tim Horton 2020-11-13 11:39:32 PST
(In reply to Said Abou-Hallawa from comment #5)
> (In reply to Tim Horton from comment #3)
> > Yes, it does. We can't do this.
> 
> Can you please explain why "we can't do this"?
> 
> If IOSurface/ShareableBitmap mapped in the Web Content process is read-only,
> it will act exactly as any shared memory we are going to create.

ShareableBitmap, true. IOSurface, not true. I think you got the details elsewhere, but we won't be able to use IOSurface from the Web Content process, because it is *not* just a trivial shared memory buffer.
Comment 7 Radar WebKit Bug Importer 2020-11-19 23:13:55 PST
<rdar://problem/71617360>