[webkit-reviews] review granted: [Bug 233756] WC variant RemoteGraphicsContextGL::platformLayer() should be removed : [Attachment 447329] Patch

[bugzilla-daemon at webkit.org]

Kimmo Kinnunen <kkinnunen at apple.com> has granted Fujii Hironori
<Hironori.Fujii at sony.com>'s request for review:
Bug 233756: WC variant RemoteGraphicsContextGL::platformLayer() should be
removed
https://bugs.webkit.org/show_bug.cgi?id=233756

Attachment 447329: Patch

https://bugs.webkit.org/attachment.cgi?id=447329&action=review




--- Comment #15 from Kimmo Kinnunen <kkinnunen at apple.com> ---
Comment on attachment 447329
  --> https://bugs.webkit.org/attachment.cgi?id=447329
Patch

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

Great, I think this paves the way for having the correct drawing buffer,
display buffer management for WC port!

> Source/WebKit/GPUProcess/graphics/wc/WCContentBuffer.h:74
> +    void platformLayerWillBeDestroyed() override

FWIW: So this signifies an discrepancy or a slight error:
It does not make sense that the "image" referenced by the wccontentbuffer can
be "destroyed". WCContentBuffer *is* the image.
In future work, if you spend time on this, you can change it so that instead of
storing the platform layer into the wccontentbuffer, you store ANGLE texture
id.
This way your compositor works correctly in the case about the display buffer
and drawing buffer.

> Source/WebKit/Shared/wc/WCContentBufferIdentifier.h:38
> +class WCContentBufferIdentifier {

I still think this is a bit bad to have structure like:
- Everything in WebKit uses ObjectIdentifier
- Except WCContentBufferIdentifier
There's very much a mental overhead of looking at the code and undestanding
"this should be an identifier, why is it not a normal identifier".
Also, you implement custom code for the "process qualified identifier" that we
also have.

Also, the security idea is that it is good to avoid exposing addresses of
controllable allocations.


So if there's a real concern about overflowing 64-bit integers: 
If my calculations are correct, you can animate 60fps webgl for 584942417355
years until an overflow.
That is 584942 years showing million canvas elements simultaneously 60fps.