[webkit-reviews] review granted: [Bug 207048] Allow different back-ends for ImageBuffer : [Attachment 389449] Patch for review

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 207048: Allow different back-ends for ImageBuffer
https://bugs.webkit.org/show_bug.cgi?id=207048

Attachment 389449: Patch  for review

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




--- Comment #6 from Tim Horton <thorton at apple.com> ---
Comment on attachment 389449
  --> https://bugs.webkit.org/attachment.cgi?id=389449
Patch  for review

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

I think this is fine, except the name.

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:834
> -		2DDE1D081F574D0E00D1A365 /* JSVRStageParameters.h in Headers */
= {isa = PBXBuildFile; fileRef = 2DDE1CF61F574BFF00D1A365 /*
JSVRStageParameters.h */; };
> +		2DDE1CE31F574AE500D1A365 /* (null) in Headers */ = {isa =
PBXBuildFile; };

This all doesn't look good

> Source/WebCore/platform/graphics/ContextualImageBuffer.h:33
> +class ContextualImageBuffer : public ImageBuffer {

I'm not sure I follow the "Contextual" name (but maybe you can explain)

(also, I'm not sure why it exists? Why isn't this just all IN ImageBuffer?)

(reading more... is it so you can hide BackendType from ImageBuffer clients?
why not just `using` it away?)

> Source/WebCore/platform/graphics/ContextualImageBuffer.h:145
> +	   const_cast<ContextualImageBuffer&>(*this).flushContext();

Maybe we should just bite the bullet and de-const getImageData if it's a lie?