Bug 146095

Summary:

Update AVKit usage of pip

Product:

WebKit

Reporter:

Jon Lee <jonlee>

Component:

Media

Assignee:

Jon Lee <jonlee>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

andersca, dino, eric.carlson, jeremyj-wk, jer.noble, ossy, webkit-bug-importer

Priority:

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

Jon Lee

Reported 2015-06-17 19:40:17 PDT

Update AVKit usage of pip, with additional cleanup in preferences rdar://problem/21386853

Attachments
Refactor allowsOptimizedFullscreen, set bool with member instead of local var. (5.12 KB, patch) 2015-06-17 19:46 PDT, Jon Lee	no flags	Details Formatted Diff Diff
Update delegate calls (7.39 KB, patch) 2015-06-17 19:46 PDT, Jon Lee	no flags	Details Formatted Diff Diff
Replace start/stop PiP (3.04 KB, patch) 2015-06-17 19:46 PDT, Jon Lee	no flags	Details Formatted Diff Diff
isPiPPossible (1.90 KB, patch) 2015-06-17 19:47 PDT, Jon Lee	no flags	Details Formatted Diff Diff
prepareForPictureInPictureStopWithCompletionHandler (4.35 KB, patch) 2015-06-17 19:47 PDT, Jon Lee	no flags	Details Formatted Diff Diff
enterFullscreenOptimized (1.94 KB, patch) 2015-06-17 19:47 PDT, Jon Lee	no flags	Details Formatted Diff Diff
remove unused setIsOptimized (785 bytes, patch) 2015-06-17 19:47 PDT, Jon Lee	no flags	Details Formatted Diff Diff
Change media player enum to VideoFullscreenModePictureInPicture (8.99 KB, patch) 2015-06-17 19:47 PDT, Jon Lee	no flags	Details Formatted Diff Diff
Remove unused AVPlayerViewControllerOptimizedFullscreenStopReason (929 bytes, patch) 2015-06-17 19:47 PDT, Jon Lee	no flags	Details Formatted Diff Diff
Replace wkIsOptimizedFullscreenSupported (5.38 KB, patch) 2015-06-17 19:47 PDT, Jon Lee	no flags	Details Formatted Diff Diff
Replace AVPlayerLayer setEnterOptimizedFullscreenModeEnabled (2.63 KB, patch) 2015-06-17 19:47 PDT, Jon Lee	no flags	Details Formatted Diff Diff
Update local variable names (6.99 KB, patch) 2015-06-17 19:47 PDT, Jon Lee	no flags	Details Formatted Diff Diff
Update mayAutomaticallyShowVideoOptimized (4.16 KB, patch) 2015-06-17 19:47 PDT, Jon Lee	no flags	Details Formatted Diff Diff
Replace _isShowingVideoOptimized and _mayAutomaticallyShowVideoOptimized (2.48 KB, patch) 2015-06-17 19:47 PDT, Jon Lee	no flags	Details Formatted Diff Diff
Update setting to allowsPictureInPictureMediaPlayback (11.70 KB, patch) 2015-06-17 19:48 PDT, Jon Lee	no flags	Details Formatted Diff Diff
Update MediaElementSession::allowsAlternateFullscreen (4.31 KB, patch) 2015-06-17 19:48 PDT, Jon Lee	no flags	Details Formatted Diff Diff
Add WebPreferences allowsPictureInPictureMediaPlayback; refactor alternate fullscreen to use that. Later patch will remove the preference since it is being used by another framework (1.67 KB, patch) 2015-06-17 19:48 PDT, Jon Lee	no flags	Details Formatted Diff Diff
Update shouldAllowAlternateFullscreen (3.77 KB, patch) 2015-06-17 19:48 PDT, Jon Lee	no flags	Details Formatted Diff Diff
Patch for submission (59.12 KB, patch) 2015-06-17 23:25 PDT, Jon Lee	eric.carlson: review+	Details Formatted Diff Diff
Patch (55.27 KB, patch) 2015-06-18 15:38 PDT, Jon Lee	no flags	Details Formatted Diff Diff
Patch for submission (55.72 KB, patch) 2015-06-18 16:20 PDT, Jon Lee	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Jon Lee

Comment 1 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.

Jon Lee

Comment 2 2015-06-17 19:46:49 PDT

Created attachment 255062 [details] Refactor allowsOptimizedFullscreen, set bool with member instead of local var.

Jon Lee

Comment 3 2015-06-17 19:46:53 PDT

Created attachment 255063 [details] Update delegate calls

Jon Lee

Comment 4 2015-06-17 19:46:56 PDT

Created attachment 255064 [details] Replace start/stop PiP

Jon Lee

Comment 5 2015-06-17 19:47:00 PDT

Created attachment 255065 [details] isPiPPossible

Jon Lee

Comment 6 2015-06-17 19:47:05 PDT

Created attachment 255066 [details] prepareForPictureInPictureStopWithCompletionHandler

Jon Lee

Comment 7 2015-06-17 19:47:09 PDT

Created attachment 255067 [details] enterFullscreenOptimized

Jon Lee

Comment 8 2015-06-17 19:47:13 PDT

Created attachment 255068 [details] remove unused setIsOptimized

Jon Lee

Comment 9 2015-06-17 19:47:17 PDT

Created attachment 255069 [details] Change media player enum to VideoFullscreenModePictureInPicture

Jon Lee

Comment 10 2015-06-17 19:47:20 PDT

Created attachment 255070 [details] Remove unused AVPlayerViewControllerOptimizedFullscreenStopReason

Jon Lee

Comment 11 2015-06-17 19:47:24 PDT

Created attachment 255071 [details] Replace wkIsOptimizedFullscreenSupported

Jon Lee

Comment 12 2015-06-17 19:47:41 PDT

Created attachment 255072 [details] Replace AVPlayerLayer setEnterOptimizedFullscreenModeEnabled

Jon Lee

Comment 13 2015-06-17 19:47:45 PDT

Created attachment 255073 [details] Update local variable names

Jon Lee

Comment 14 2015-06-17 19:47:51 PDT

Created attachment 255074 [details] Update mayAutomaticallyShowVideoOptimized

Jon Lee

Comment 15 2015-06-17 19:47:56 PDT

Created attachment 255075 [details] Replace _isShowingVideoOptimized and _mayAutomaticallyShowVideoOptimized

Jon Lee

Comment 16 2015-06-17 19:48:00 PDT

Created attachment 255076 [details] Update setting to allowsPictureInPictureMediaPlayback

Jon Lee

Comment 17 2015-06-17 19:48:05 PDT

Created attachment 255077 [details] Update MediaElementSession::allowsAlternateFullscreen

Jon Lee

Comment 18 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

Jon Lee

Comment 19 2015-06-17 19:48:14 PDT

Created attachment 255079 [details] Update shouldAllowAlternateFullscreen

Jon Lee

Comment 20 2015-06-17 23:25:48 PDT

Created attachment 255090 [details] Patch for submission

Jon Lee

Comment 21 2015-06-17 23:30:06 PDT

*** Bug 146094 has been marked as a duplicate of this bug. ***

Csaba Osztrogonác

Comment 22 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.

Csaba Osztrogonác

Comment 23 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.

Eric Carlson

Comment 24 2015-06-18 09:38:51 PDT

Comment on attachment 255090 [details] Patch for submission Did you miss an instance in Settings.in?

Jon Lee

Comment 25 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.

Jon Lee

Comment 26 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.

Jon Lee

Comment 27 2015-06-18 15:38:20 PDT

Created attachment 255142 [details] Patch

Jon Lee

Comment 28 2015-06-18 16:20:18 PDT

Created attachment 255149 [details] Patch for submission

Jon Lee

Comment 29 2015-06-18 16:43:05 PDT

Committed r185727: <http://trac.webkit.org/changeset/185727>

Note You need to log in before you can comment on or make changes to this bug.