[webkit-reviews] review granted: [Bug 228531] Missing playing events when the ready state becomes HAVE_FUTURE_DATA/HAVE_ENOUGH_DATA from HAVE_METADATA state : [Attachment 434491] patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Tomoki Imai
<tomoki.imai at sony.com>'s request for review:
Bug 228531: Missing playing events when the ready state becomes
HAVE_FUTURE_DATA/HAVE_ENOUGH_DATA from HAVE_METADATA state
https://bugs.webkit.org/show_bug.cgi?id=228531

Attachment 434491: patch

https://bugs.webkit.org/attachment.cgi?id=434491&action=review




--- Comment #5 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 434491
  --> https://bugs.webkit.org/attachment.cgi?id=434491
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=434491&action=review

r=me with one minor suggested change

This has an r+, so you don't need another review. When you post your new patch,
replace "NOBODY (OOPS!)" with "Eric Carlson" and just ask for a commit and any
reviewer can land it: webkit-patch upload --no-review --request-commit ...

> Source/WebCore/html/HTMLMediaElement.cpp:2448
> +	   if (m_readyState == HAVE_FUTURE_DATA && oldState <=
HAVE_CURRENT_DATA) {
> +	       if (!tracksAreReady)
> +		   break;

I meant you could move the tracks test above this as we don't want to do either
test unless track data is available:

    if (!tracksAreReady)
	break;

    if (m_readyState == HAVE_FUTURE_DATA && oldState <= HAVE_CURRENT_DATA) {
	scheduleEvent(eventNames().canplayEvent);
	...