Bug 153100

Summary: LayoutTest media/airplay-target-availability.html is flaky
Product: WebKit Reporter: Ryan Haddad <ryanhaddad>
Component: MediaAssignee: 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 Flags
Proposed patch. none

Description Ryan Haddad 2016-01-14 11:49:48 PST
LayoutTest media/airplay-target-availability.html is flaky

The test passes most of the time, but it seems to be intermittently failing with this diff:

--- /Volumes/Data/slave/elcapitan-debug-tests-wk2/build/layout-test-results/media/airplay-target-availability-expected.txt
+++ /Volumes/Data/slave/elcapitan-debug-tests-wk2/build/layout-test-results/media/airplay-target-availability-actual.txt
@@ -1,9 +1,6 @@
  
 
 Test that 'webkitplaybacktargetavailabilitychanged' event is sent when at least one video element has playable media.
-
-
-EVENT: 'webkitplaybacktargetavailabilitychanged', event.availability = 'not-available'
 
 
 EVENT: 'webkitplaybacktargetavailabilitychanged', event.availability = 'not-available'

Most recent failing run:
<https://build.webkit.org/builders/Apple%20El%20Capitan%20Debug%20WK2%20(Tests)/builds/2206>

Flakiness dashboard:
<http://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=media%2Fairplay-target-availability.html>
Comment 1 Alexey Proskuryakov 2016-01-18 15:12:57 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.
Comment 2 Radar WebKit Bug Importer 2016-01-26 08:16:26 PST
<rdar://problem/24346796>
Comment 3 Eric Carlson 2016-01-26 08:17:30 PST
Created attachment 269883 [details]
Proposed patch.
Comment 4 Daniel Bates 2016-01-26 08:54:28 PST
Comment on attachment 269883 [details]
Proposed patch.

This looks sane to me.
Comment 5 Daniel Bates 2016-01-26 08:57:44 PST
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.
Comment 6 Eric Carlson 2016-01-26 09:18:34 PST
Committed r195592: https://trac.webkit.org/r195592