[webkit-reviews] review granted: [Bug 120448] [Windows] Video inside a web page always falls back to non-hardware accelerated : [Attachment 210165] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Brent Fulgham <bfulgham at webkit.org>'s
request for review:
Bug 120448: [Windows] Video inside a web page always falls back to non-hardware
accelerated
https://bugs.webkit.org/show_bug.cgi?id=120448

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=210165&action=review


>
Source/WebCore/platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundation
CF.h:88
> +    virtual bool requiresImmediateCompositing() const { return true; }

I’d like you to use OVERRIDE.

I’d like to see a comment explaining why this player requires this; the comment
in the change log is good, but I think it should be in the code too. To have a
good place for the comment, I suggest not inlining this, since virtual calls
won’t be inlined anyway.

> Source/WebCore/rendering/RenderVideo.cpp:287
> +    MediaPlayer* p = mediaElement()->player();
> +    return p ? p->requiresImmediateCompositing() : false;

I would write it like this:

    MediaPlayer* player = mediaElement()->player();
    return player && player->requiresImmediateCompositing();