[webkit-reviews] review granted: [Bug 50228] Theme not updated when MediaPlayer m_private engine changes : [Attachment 75385] proposed patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 50228: Theme not updated when MediaPlayer m_private engine changes
https://bugs.webkit.org/show_bug.cgi?id=50228

Attachment 75385: proposed patch
https://bugs.webkit.org/attachment.cgi?id=75385&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=75385&action=review

> WebCore/html/HTMLMediaElement.cpp:1964
> +void HTMLMediaElement::mediaPlayerBackendUpdated(MediaPlayer*)

I am not sure about this function name, the term "backend" isn't used anywhere
else in this file. "Media engine" is used in both comments and
"mediaEngineError", so maybe "mediaPlayerEngineUpdated" or
"mediaPlayerEngineChanged" instead?

> WebCore/html/HTMLMediaElement.cpp:1969
> +	   toRenderMedia(renderer())->updateFromElement();

Why is this cast necessary, updateFromElement() is called elsewhere in the file
without it?


r+, though I would prefer a different name for the callback.