[webkit-reviews] review granted: [Bug 225304] OffscreenCanvas should preserve context transform after transferToImageBitmap : [Attachment 427643] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Chris Lord <clord at igalia.com>'s
request for review:
Bug 225304: OffscreenCanvas should preserve context transform after
transferToImageBitmap
https://bugs.webkit.org/show_bug.cgi?id=225304

Attachment 427643: Patch

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




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 427643
  --> https://bugs.webkit.org/attachment.cgi?id=427643
Patch

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

> Source/WebCore/html/OffscreenCanvas.cpp:271
> +	   // As the canvas context state is stored in GraphicsContext, which
is tightly bound
> +	   // to buffer(), to avoid resetting the context state, we have to
make a copy and
> +	   // clear the original buffer rather than returning the original
buffer.

The comment "tightly bound to buffer()" seems to say that we can’t possibly get
a GraphicsContext to release its current buffer and allocate a new one.

Is that true, maybe due to limitations to CGGraphicsContext on Apple’s
platforms, or is this simply new work we’d need to do in GraphicsContext?

> Source/WebCore/html/OffscreenCanvas.cpp:272
> +	   ASSERT(buffer() && drawingContext());

Maybe assert these separately?

But also, we don’t normally have to assert pointers are non-null; dereferencing
them usually hits a null dereference quickly without us writing any additional
code.

And also, if we are using m_context below, and want to assert it’s not null,
then we should assert that rather than drawingContext().