[Webkit-unassigned] [Bug 56983] [chromium] Implement preload=none, setPreload hooks to media player
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Mar 25 09:06:12 PDT 2011
https://bugs.webkit.org/show_bug.cgi?id=56983
Eric Carlson <eric.carlson at apple.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #86851|review?, commit-queue? |review-
Flag| |
--- Comment #4 from Eric Carlson <eric.carlson at apple.com> 2011-03-25 09:06:13 PST ---
(From update of attachment 86851)
View in context: https://bugs.webkit.org/attachment.cgi?id=86851&action=review
r- for now as I am not sure the details are quite right yet.
> Source/WebKit/chromium/public/WebMediaPlayer.h:103
> + virtual bool setPreload(Preload) { return false; };
Does this function really need to return a bool?
> Source/WebKit/chromium/src/WebMediaPlayerClientImpl.cpp:190
> + return static_cast<unsigned>(MediaPlayer::MetaData);
Don't you want to return m_preload here instead of hard coding MediaPlayer::MetaData?
> Source/WebKit/chromium/src/WebMediaPlayerClientImpl.cpp:216
> + Frame* frame = static_cast<HTMLMediaElement*>(
> + m_mediaPlayer->mediaPlayerClient())->document()->frame();
This indentation is odd, I thought at first that a stray tab had snuck in. I am not sure that the line break helps readability.
> Source/WebKit/chromium/src/WebMediaPlayerClientImpl.cpp:259
> + if (m_delayingLoad)
> + startDelayedLoad();
If you don't set m_delayingLoad to false before calling startDelayedLoad(), won't it ASSERT if this is called when m_preload == MediaPlayer::None?
> Source/WebKit/chromium/src/WebMediaPlayerClientImpl.cpp:466
> + if (m_delayingLoad && m_preload != MediaPlayer::None)
> + startDelayedLoad();
Ditto.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list