Bug 146095

Summary: Update AVKit usage of pip
Product: WebKit Reporter: Jon Lee <jonlee>
Component: MediaAssignee: Jon Lee <jonlee>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, dino, eric.carlson, jeremyj-wk, jer.noble, ossy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 145825    
Attachments:
Description Flags
Refactor allowsOptimizedFullscreen, set bool with member instead of local var.
none
Update delegate calls
none
Replace start/stop PiP
none
isPiPPossible
none
prepareForPictureInPictureStopWithCompletionHandler
none
enterFullscreenOptimized
none
remove unused setIsOptimized
none
Change media player enum to VideoFullscreenModePictureInPicture
none
Remove unused AVPlayerViewControllerOptimizedFullscreenStopReason
none
Replace wkIsOptimizedFullscreenSupported
none
Replace AVPlayerLayer setEnterOptimizedFullscreenModeEnabled
none
Update local variable names
none
Update mayAutomaticallyShowVideoOptimized
none
Replace _isShowingVideoOptimized and _mayAutomaticallyShowVideoOptimized
none
Update setting to allowsPictureInPictureMediaPlayback
none
Update MediaElementSession::allowsAlternateFullscreen
none
Add WebPreferences allowsPictureInPictureMediaPlayback; refactor alternate fullscreen to use that. Later patch will remove the preference since it is being used by another framework
none
Update shouldAllowAlternateFullscreen
none
Patch for submission
eric.carlson: review+
Patch
none
Patch for submission none

Description Jon Lee 2015-06-17 19:40:17 PDT
Update AVKit usage of pip, with additional cleanup in preferences

rdar://problem/21386853
Comment 1 Jon Lee 2015-06-17 19:41:12 PDT
Uploading several sub-patches for review, and one large one that is the combination of all of them, for EWS to chew on.
Comment 2 Jon Lee 2015-06-17 19:46:49 PDT
Created attachment 255062 [details]
Refactor allowsOptimizedFullscreen, set bool with member instead of local var.
Comment 3 Jon Lee 2015-06-17 19:46:53 PDT
Created attachment 255063 [details]
Update delegate calls
Comment 4 Jon Lee 2015-06-17 19:46:56 PDT
Created attachment 255064 [details]
Replace start/stop PiP
Comment 5 Jon Lee 2015-06-17 19:47:00 PDT
Created attachment 255065 [details]
isPiPPossible
Comment 6 Jon Lee 2015-06-17 19:47:05 PDT
Created attachment 255066 [details]
prepareForPictureInPictureStopWithCompletionHandler
Comment 7 Jon Lee 2015-06-17 19:47:09 PDT
Created attachment 255067 [details]
enterFullscreenOptimized
Comment 8 Jon Lee 2015-06-17 19:47:13 PDT
Created attachment 255068 [details]
remove unused setIsOptimized
Comment 9 Jon Lee 2015-06-17 19:47:17 PDT
Created attachment 255069 [details]
Change media player enum to VideoFullscreenModePictureInPicture
Comment 10 Jon Lee 2015-06-17 19:47:20 PDT
Created attachment 255070 [details]
Remove unused AVPlayerViewControllerOptimizedFullscreenStopReason
Comment 11 Jon Lee 2015-06-17 19:47:24 PDT
Created attachment 255071 [details]
Replace wkIsOptimizedFullscreenSupported
Comment 12 Jon Lee 2015-06-17 19:47:41 PDT
Created attachment 255072 [details]
Replace AVPlayerLayer setEnterOptimizedFullscreenModeEnabled
Comment 13 Jon Lee 2015-06-17 19:47:45 PDT
Created attachment 255073 [details]
Update local variable names
Comment 14 Jon Lee 2015-06-17 19:47:51 PDT
Created attachment 255074 [details]
Update mayAutomaticallyShowVideoOptimized
Comment 15 Jon Lee 2015-06-17 19:47:56 PDT
Created attachment 255075 [details]
Replace _isShowingVideoOptimized and _mayAutomaticallyShowVideoOptimized
Comment 16 Jon Lee 2015-06-17 19:48:00 PDT
Created attachment 255076 [details]
Update setting to allowsPictureInPictureMediaPlayback
Comment 17 Jon Lee 2015-06-17 19:48:05 PDT
Created attachment 255077 [details]
Update MediaElementSession::allowsAlternateFullscreen
Comment 18 Jon Lee 2015-06-17 19:48:09 PDT
Created attachment 255078 [details]
Add WebPreferences allowsPictureInPictureMediaPlayback; refactor alternate fullscreen to use that. Later patch will remove the preference since it is being used by another framework
Comment 19 Jon Lee 2015-06-17 19:48:14 PDT
Created attachment 255079 [details]
Update shouldAllowAlternateFullscreen
Comment 20 Jon Lee 2015-06-17 23:25:48 PDT
Created attachment 255090 [details]
Patch for submission
Comment 21 Jon Lee 2015-06-17 23:30:06 PDT
*** Bug 146094 has been marked as a duplicate of this bug. ***
Comment 22 Csaba Osztrogonác 2015-06-18 02:14:02 PDT
19 r? in one bug report? :o I'm not sure if it is a good idea.

The latest patch aggregates the previous smaller patch, am I right?
Please remove the r? flags and set the obsolete flags on the other patches.
Comment 23 Csaba Osztrogonác 2015-06-18 02:17:10 PDT
(In reply to comment #1)
> Uploading several sub-patches for review, and one large one that is the
> combination of all of them, for EWS to chew on.

It's not good at all, the common way is to upload only one patch per 
bug report. Additionally a patch without changelog entry is insufficient.
Comment 24 Eric Carlson 2015-06-18 09:38:51 PDT
Comment on attachment 255090 [details]
Patch for submission

Did you miss an instance in Settings.in?
Comment 25 Jon Lee 2015-06-18 10:13:31 PDT
(In reply to comment #24)
> Comment on attachment 255090 [details]
> Patch for submission
> 
> Did you miss an instance in Settings.in?

No, this is another case where the builders don't update the generated settings files appropriately. I ran into this before when renaming the other media settings.

It builds fine on my machine; I'll keep an eye on the bots.
Comment 26 Jon Lee 2015-06-18 11:32:12 PDT
I am going to check in a version of the patch that does not include the update to WKSI. That call should be removed. I'll file another bug to track that.
Comment 27 Jon Lee 2015-06-18 15:38:20 PDT
Created attachment 255142 [details]
Patch
Comment 28 Jon Lee 2015-06-18 16:20:18 PDT
Created attachment 255149 [details]
Patch for submission
Comment 29 Jon Lee 2015-06-18 16:43:05 PDT
Committed r185727: <http://trac.webkit.org/changeset/185727>