Summary: | LayoutTest media/airplay-target-availability.html is flaky | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryan Haddad <ryanhaddad> | ||||
Component: | Media | Assignee: | Eric Carlson <eric.carlson> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | ap, commit-queue, eric.carlson, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Ryan Haddad
2016-01-14 11:49:48 PST
This is a new test, may be worth taking a look soon. Also, seems pretty clearly like a bug that we are dispatching an availability changed event when availability is the same. Created attachment 269883 [details]
Proposed patch.
Comment on attachment 269883 [details]
Proposed patch.
This looks sane to me.
Comment on attachment 269883 [details] Proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=269883&action=review > Source/WebCore/ChangeLog:4 > + https://bugs.webkit.org/show_bug.cgi?id=153100 Please add the radar bug under this line to make it convenient for a person to find the associated radar without having to open the WebKit bug. > Source/WebCore/Modules/mediasession/WebMediaSessionManager.cpp:361 > + LOG(Media, "WebMediaSessionManager::configurePlaybackTargetMonitoring - monitoringRequired = %i", (int)(monitoringRequired || (hasAvailabilityListener && haveClientWithMedia))); Nit: This is OK as-is. We prefer using static_cast<int> in C++ code. > LayoutTests/media/airplay-target-availability.html:22 > + video1.removeEventListener('webkitplaybacktargetavailabilitychanged', noSource, true); Nit: This is OK as-is. We seem to alternative between single and double quotes throughout this patch. We should pick one quoting style and stick without. Committed r195592: https://trac.webkit.org/r195592 |