[webkit-reviews] review granted: [Bug 127297] Allow MediaSessionManager to restrict 'preload' behavior : [Attachment 221672] Proposed patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jan 20 10:44:59 PST 2014


Jer Noble <jer.noble at apple.com> has granted Eric Carlson
<eric.carlson at apple.com>'s request for review:
Bug 127297: Allow MediaSessionManager to restrict 'preload' behavior
https://bugs.webkit.org/show_bug.cgi?id=127297

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

------- Additional Comments from Jer Noble <jer.noble at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=221672&action=review


r=me with one nit.

> Source/WebCore/html/HTMLMediaSession.cpp:146
> +MediaPlayer::Preload HTMLMediaSession::applyPreloadRestrictions(const
HTMLMediaElement&, MediaPlayer::Preload preload) const

I'm not a huge fan of this name.  It implies that the parameter is being
modified, rather than a new preload being returned.  How about
"effectivePreload()" or "effectivePreloadForElement()", like your variables
above?	If you added an accessor for m_preload, you wouldn't even have to pass
in the m_preload attribute; effectivePreloadForElement() could just as the
media element for it's m_preload value and return the effective
(post-restrictions) preload value.


More information about the webkit-reviews mailing list