Bug 137533

Summary: MediaPlayer::characteristicChanged() is not called when new tracks are found in SourceBufferPrivateAVFObjC
Product: WebKit Reporter: Ada Chan <adachan>
Component: MediaAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: adachan, aestes, calvaris, commit-queue, eric.carlson, glenn, jer.noble, philipj, sergio
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing
none
Patch
none
Patch
none
Follow up patch none

Description Ada Chan 2014-10-08 12:31:07 PDT 
When new audio/video tracks are parsed in SourceBufferPrivateAVFObjC, I would expect MediaPlayer::characteristicsChanged() to be called since the media's characteristics (such as hasAudio, hasVideo) have changed.

1. Open a youtube video page showing HTML5 video.
2. Place a breakpoint in MediaPlayer::characteristicsChanged().

RESULTS: I don't see that called due to a change in hasAudio/hasVideo characteristics.
Comment 1 Jer Noble 2014-10-08 13:31:46 PDT 
Created attachment 239491 [details]
Patch
Comment 2 Darin Adler 2014-10-09 14:50:05 PDT 
Comment on attachment 239491 [details]
Patch

No regression test possible?
Comment 3 Jer Noble 2014-10-10 23:48:12 PDT 
(In reply to comment #2)
> (From update of attachment 239491 [details])
> No regression test possible?

A TestWebKitAPI unit test should be possible.  I'm working on one now.
Comment 4 Jer Noble 2014-10-11 00:55:20 PDT 
Created attachment 239668 [details]
Patch for landing
Comment 5 Jer Noble 2014-10-11 10:48:07 PDT 
Created attachment 239680 [details]
Patch

Also unregister for availability changes when the page is hidden (e.g., when in a background tab).
Comment 6 Jer Noble 2014-10-11 23:23:09 PDT 
Created attachment 239701 [details]
Patch
Comment 7 Eric Carlson 2014-10-12 08:31:43 PDT 
(In reply to comment #6)
> Created an attachment (id=239701) [details]
> Patch

I think you meant to attach this to bug 137633.
Comment 8 Jer Noble 2014-10-13 10:49:18 PDT 
Committed r174652: <http://trac.webkit.org/changeset/174652>
Comment 9 Andy Estes 2014-10-13 13:01:21 PDT 
Looks like the new API test is timing out on both the Mavericks and Mountain Lion bots. For example:

https://build.webkit.org/builders/Apple%20Mavericks%20Debug%20WK1%20%28Tests%29/builds/8217/steps/run-api-tests/logs/stdio
Comment 10 Jer Noble 2014-10-13 13:30:45 PDT 
Oof. MSE is only enabled on Yosemite.
Comment 11 Jer Noble 2014-10-13 14:20:04 PDT 
Reopening to attach new patch.
Comment 12 Jer Noble 2014-10-13 14:20:07 PDT 
Created attachment 239744 [details]
Follow up patch
Comment 13 Andy Estes 2014-10-13 14:22:43 PDT 
Comment on attachment 239744 [details]
Follow up patch

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

> Tools/TestWebKitAPI/Tests/WebKit2/WKPageIsPlayingAudio.cpp:58
> +    EXPECT_TRUE(JSValueIsBoolean(scriptContext, resultValue));

Won't this cause the test to fail on unsupported platforms?
Comment 14 Andy Estes 2014-10-13 14:24:19 PDT 
Comment on attachment 239744 [details]
Follow up patch

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

>> Tools/TestWebKitAPI/Tests/WebKit2/WKPageIsPlayingAudio.cpp:58
>> +    EXPECT_TRUE(JSValueIsBoolean(scriptContext, resultValue));
> 
> Won't this cause the test to fail on unsupported platforms?

Oh never mind. Sigh. IsBoolean != ToBoolean.
Comment 15 WebKit Commit Bot 2014-10-13 18:16:28 PDT 
Comment on attachment 239744 [details]
Follow up patch

Clearing flags on attachment: 239744

Committed r174667: <http://trac.webkit.org/changeset/174667>
Comment 16 WebKit Commit Bot 2014-10-13 18:16:32 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 17 Jer Noble 2014-12-16 13:28:11 PST 
<rdar://problem/19269053>