RESOLVED FIXED 33889
autobuffer value not forwarded media element to MediaPlayer 
https://bugs.webkit.org/show_bug.cgi?id=33889
Summary autobuffer value not forwarded media element to MediaPlayer
Philippe Normand
Reported 2010-01-20 02:44:29 PST
In HTMLMediaElement parseMappedAttribute() forwards autobuffer only if m_player has been created. m_player.setAutobuffer() should also be called just after the m_player has been created too (in loadResource()).
Attachments
Forward autobuffer value to MediaPlayer instance. (1.43 KB, patch)
2010-01-20 07:42 PST, Philippe Normand 		darin: review+
DetailsFormatted DiffDiff
updated patch (2.20 KB, patch)
2010-01-25 05:11 PST, Philippe Normand 		eric: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Philippe Normand
Comment 1 2010-01-20 07:42:40 PST
Created attachment 47032 [details] Forward autobuffer value to MediaPlayer instance.
Darin Adler
Comment 2 2010-01-20 08:22:12 PST
Comment on attachment 47032 [details] Forward autobuffer value to MediaPlayer instance. Code change looks great. We normally require test cases or an explanation of why a test case is lacking. Since nobody is doing anything with the autobuffer flag right now, at least as far as I can tell with grep, I guess that's not possible here.
Philippe Normand
Comment 3 2010-01-20 08:31:21 PST
Well there is a test for autobuffer but it checks only the DOM attributes. After this patch I thought it would also be good for the MediaPlayer to call setAutobuffer() on its media engine after loading an url. Otherwise each media engine implementing setAutoBuffer() would need to manually call it. What do think? I am fine with sending an updated patch. BTW I need this for Bug 30004 ;)
Philippe Normand
Comment 4 2010-01-25 05:11:33 PST
Created attachment 47341 [details] updated patch
Eric Seidel (no email)
Comment 5 2010-01-26 15:04:18 PST
Comment on attachment 47341 [details] updated patch How do we test this? Looks sane, but all changes should have tests or explanation as to why testing is impossible.
Philippe Normand
Comment 6 2010-01-27 00:40:20 PST
(In reply to comment #5) > (From update of attachment 47341 [details]) > How do we test this? Looks sane, but all changes should have tests or > explanation as to why testing is impossible. Well I am still new to this but I didn't see unittests for the MediaPlayer, maybe it would make sense to have some? The MediaPlayerPrivate implementations are not unittested either, afaics. Anyway I landed this patch as r53910. I will think about unittesting the MediaPlayer at least.
Note You need to log in before you can comment on or make changes to this bug.