[webkit-reviews] review denied: [Bug 73858] [chromium] Set opaque() for VideoLayerChromium : [Attachment 117933] Patch

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Dana Jansens
<danakj at chromium.org>'s request for review:
Bug 73858: [chromium] Set opaque() for VideoLayerChromium
https://bugs.webkit.org/show_bug.cgi?id=73858

Attachment 117933: Patch
https://bugs.webkit.org/attachment.cgi?id=117933&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=117933&action=review


> Source/WebKit/chromium/ChangeLog:3
> +	   [chromium] Set opaque() for VideoLayerChromium

This ChangeLog summary seems inconsistent with the contents of the patch.

> Source/WebKit/chromium/public/WebVideoFrame.h:65
> +    virtual bool isFormatOpaque()

do you mean for this function to be overridden by the implementation class?
if not, then perhaps this should not be virtual.  you may also want to define
this non-inline since it generates a non-trivial amount of code.  or, perhaps
this should just be a static helper function defined in
WebMediaPlayerClientImpl.cpp?