[Mac] media/pip-video-going-into-fullscreen.html is a flaky failure
authoradachan@apple.com <adachan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 3 Aug 2016 18:11:50 +0000 (18:11 +0000)
committeradachan@apple.com <adachan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 3 Aug 2016 18:11:50 +0000 (18:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=160469

Reviewed by Eric Carlson.

Source/WebCore:

When going from picture-in-picture directly to fullscreen, fix the issue where the
presentation mode unexpectedly changes back to inline after changing to fullscreen.

On Mac, standard fullscreen is not handled by WebVideoFullscreenManager.
When going from picture-in-picture directly to fullscreen, we call
WebVideoFullscreenInterfaceMac::exitFullscreenWithoutAnimationToMode().
We should update m_mode to VideoFullscreenModeStandard there to keep it in sync
with the fullscreen mode in HTMLMediaElement. Otherwise, we'll inadvertently
update the mode to inline when we clear the VideoFullscreenModePictureInPicture mode
in -[WebVideoFullscreenInterfaceMacObjC pipDidClose:].

Since standard fullscreen on Mac doesn't make use of the video fullscreen layer,
we need to make sure we return the video layer back to inline when the presentation
mode changes to "fullscreen". We only do this on Mac because iOS does use
the video fullscreen layer for standard fullscreen.

* Modules/mediacontrols/MediaControlsHost.cpp:
(WebCore::MediaControlsHost::setPreparedToReturnVideoLayerToInline):
Renamed from MediaControlsHost::setPreparedForInline to make it clear this is about
whether the video layer should be inline.
(WebCore::MediaControlsHost::setPreparedForInline): Deleted.
* Modules/mediacontrols/MediaControlsHost.h:
* Modules/mediacontrols/MediaControlsHost.idl:

* Modules/mediacontrols/mediaControlsApple.js:
(Controller.prototype.shouldReturnVideoLayerToInline):
On Mac, the video layer is inline when the presentation mode is "inline" or "fullscreen".
(Controller.prototype.handlePresentationModeChange):
Call shouldReturnVideoLayerToInline() to determine whether the video layer should be inline.

* Modules/mediacontrols/mediaControlsiOS.js:
(ControllerIOS.prototype.shouldReturnVideoLayerToInline):
Override this method since on iOS, the video layer is only inline when the presentation
mode is "inline".

* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::stopWithoutDestroyingMediaPlayer):
(WebCore::HTMLMediaElement::setPreparedToReturnVideoLayerToInline):
(WebCore::HTMLMediaElement::setPreparedForInline): Deleted.
* html/HTMLMediaElement.h:

* platform/mac/WebVideoFullscreenInterfaceMac.mm:
(WebCore::WebVideoFullscreenInterfaceMac::enterFullscreen):
Remove the assertion that the mode must be "picture-in-picture". I've run into this
assertion in layout tests. Since the EnterFullscreen message is sent in a dispatch_async
block in WebVideoFullscreenManager::didSetupFullscreen(), there's a chance that the
fullscreen mode tracked in WebVideoFullscreenInterfaceMac has already changed to
something else when WebVideoFullscreenInterfaceMac::enterFullscreen() is called.
(WebCore::WebVideoFullscreenInterfaceMac::exitFullscreenWithoutAnimationToMode):
If exiting to standard fullscreen, update m_mode to VideoFullscreenModeStandard.

LayoutTests:

Re-enable media/pip-video-going-into-fullscreen.html on Sierra.

* platform/mac-wk2/TestExpectations:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@204088 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/platform/mac-wk2/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediacontrols/MediaControlsHost.cpp
Source/WebCore/Modules/mediacontrols/MediaControlsHost.h
Source/WebCore/Modules/mediacontrols/MediaControlsHost.idl
Source/WebCore/Modules/mediacontrols/mediaControlsApple.js
Source/WebCore/Modules/mediacontrols/mediaControlsiOS.js
Source/WebCore/html/HTMLMediaElement.cpp
Source/WebCore/html/HTMLMediaElement.h
Source/WebCore/platform/mac/WebVideoFullscreenInterfaceMac.mm

index fb39172..c7bc3a9 100644 (file)
@@ -1,3 +1,14 @@
+2016-08-02  Ada Chan  <adachan@apple.com>
+
+        [Mac] media/pip-video-going-into-fullscreen.html is a flaky failure
+        https://bugs.webkit.org/show_bug.cgi?id=160469
+
+        Reviewed by Eric Carlson.
+
+        Re-enable media/pip-video-going-into-fullscreen.html on Sierra.
+
+        * platform/mac-wk2/TestExpectations:
+
 2016-08-03  Youenn Fablet  <youenn@apple.com>
 
         http/tests/fetch/fetch-in-worker-crash.html is sometimes crashing
index a562879..9a15c38 100644 (file)
@@ -473,15 +473,13 @@ webkit.org/b/158639 [ Release Yosemite ] imported/blink/storage/indexeddb/blob-d
 [ Sierra+ ] media/fullscreen-api-enabled-media-with-presentation-mode.html [ Pass ]
 [ Sierra+ ] media/fullscreen-video-going-into-pip.html [ Pass ]
 [ Sierra+ ] media/navigate-with-pip-should-not-crash.html [ Pass ]
+[ Sierra+ ] media/pip-video-going-into-fullscreen.html [ Pass ]
 [ Sierra+ ] media/video-contained-in-fullscreen-element-going-into-pip.html [ Pass ]
 
 # rdar://problem/26885345
 [ Release Sierra+ ] media/click-placeholder-not-pausing.html [ Pass ]
 [ Debug Sierra+ ] media/click-placeholder-not-pausing.html [ Skip ]
 
-# rdar://problem/26901714
-[ Sierra+ ] media/pip-video-going-into-fullscreen.html [ Pass Failure ]
-
 # RTL Scrollbars are enabled on Sierra WebKit2.
 [ Sierra+ ] fast/scrolling/rtl-scrollbars.html [ Pass ]
 [ Sierra+ ] fast/scrolling/rtl-scrollbars-simple.html [ Pass ]
index ceaa629..7df04d8 100644 (file)
@@ -1,3 +1,61 @@
+2016-08-02  Ada Chan  <adachan@apple.com>
+
+        [Mac] media/pip-video-going-into-fullscreen.html is a flaky failure
+        https://bugs.webkit.org/show_bug.cgi?id=160469
+
+        Reviewed by Eric Carlson.
+
+        When going from picture-in-picture directly to fullscreen, fix the issue where the
+        presentation mode unexpectedly changes back to inline after changing to fullscreen.
+
+        On Mac, standard fullscreen is not handled by WebVideoFullscreenManager.
+        When going from picture-in-picture directly to fullscreen, we call
+        WebVideoFullscreenInterfaceMac::exitFullscreenWithoutAnimationToMode().
+        We should update m_mode to VideoFullscreenModeStandard there to keep it in sync
+        with the fullscreen mode in HTMLMediaElement. Otherwise, we'll inadvertently
+        update the mode to inline when we clear the VideoFullscreenModePictureInPicture mode
+        in -[WebVideoFullscreenInterfaceMacObjC pipDidClose:].
+
+        Since standard fullscreen on Mac doesn't make use of the video fullscreen layer,
+        we need to make sure we return the video layer back to inline when the presentation
+        mode changes to "fullscreen". We only do this on Mac because iOS does use
+        the video fullscreen layer for standard fullscreen.
+
+        * Modules/mediacontrols/MediaControlsHost.cpp:
+        (WebCore::MediaControlsHost::setPreparedToReturnVideoLayerToInline):
+        Renamed from MediaControlsHost::setPreparedForInline to make it clear this is about
+        whether the video layer should be inline.
+        (WebCore::MediaControlsHost::setPreparedForInline): Deleted.
+        * Modules/mediacontrols/MediaControlsHost.h:
+        * Modules/mediacontrols/MediaControlsHost.idl:
+
+        * Modules/mediacontrols/mediaControlsApple.js:
+        (Controller.prototype.shouldReturnVideoLayerToInline):
+        On Mac, the video layer is inline when the presentation mode is "inline" or "fullscreen".
+        (Controller.prototype.handlePresentationModeChange):
+        Call shouldReturnVideoLayerToInline() to determine whether the video layer should be inline.
+
+        * Modules/mediacontrols/mediaControlsiOS.js:
+        (ControllerIOS.prototype.shouldReturnVideoLayerToInline):
+        Override this method since on iOS, the video layer is only inline when the presentation
+        mode is "inline".
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::stopWithoutDestroyingMediaPlayer):
+        (WebCore::HTMLMediaElement::setPreparedToReturnVideoLayerToInline):
+        (WebCore::HTMLMediaElement::setPreparedForInline): Deleted.
+        * html/HTMLMediaElement.h:
+
+        * platform/mac/WebVideoFullscreenInterfaceMac.mm:
+        (WebCore::WebVideoFullscreenInterfaceMac::enterFullscreen):
+        Remove the assertion that the mode must be "picture-in-picture". I've run into this
+        assertion in layout tests. Since the EnterFullscreen message is sent in a dispatch_async
+        block in WebVideoFullscreenManager::didSetupFullscreen(), there's a chance that the
+        fullscreen mode tracked in WebVideoFullscreenInterfaceMac has already changed to
+        something else when WebVideoFullscreenInterfaceMac::enterFullscreen() is called.
+        (WebCore::WebVideoFullscreenInterfaceMac::exitFullscreenWithoutAnimationToMode):
+        If exiting to standard fullscreen, update m_mode to VideoFullscreenModeStandard.
+
 2016-08-03  Youenn Fablet  <youenn@apple.com>
 
         http/tests/fetch/fetch-in-worker-crash.html is sometimes crashing
index 0e69df9..d7af35a 100644 (file)
@@ -215,9 +215,9 @@ bool MediaControlsHost::isVideoLayerInline()
     return m_mediaElement->isVideoLayerInline();
 }
 
-void MediaControlsHost::setPreparedForInline(bool value)
+void MediaControlsHost::setPreparedToReturnVideoLayerToInline(bool value)
 {
-    m_mediaElement->setPreparedForInline(value);
+    m_mediaElement->setPreparedToReturnVideoLayerToInline(value);
 }
 
 bool MediaControlsHost::userGestureRequired() const
index cf0975c..e992bfa 100644 (file)
@@ -66,7 +66,7 @@ public:
     bool supportsFullscreen();
     bool isVideoLayerInline();
     bool userGestureRequired() const;
-    void setPreparedForInline(bool);
+    void setPreparedToReturnVideoLayerToInline(bool);
 
     void updateCaptionDisplaySizes();
     void enteredFullscreen();
index 0013435..a2fbf4f 100644 (file)
@@ -42,7 +42,7 @@ enum DeviceType {
     readonly attribute TextTrack captionMenuAutomaticItem;
     readonly attribute DOMString captionDisplayMode;
     void setSelectedTextTrack(TextTrack? track);
-    void setPreparedForInline(boolean prepared);
+    void setPreparedToReturnVideoLayerToInline(boolean prepared);
     readonly attribute HTMLElement textTrackContainer;
     readonly attribute boolean allowsInlineMediaPlayback;
     readonly attribute boolean supportsFullscreen;
index 1167cdd..153fd83 100644 (file)
@@ -909,6 +909,12 @@ Controller.prototype = {
             setTimeout(this.showInlinePlaybackPlaceholderWhenSafe.bind(this), this.PlaceholderPollingDelay);
     },
 
+    shouldReturnVideoLayerToInline: function()
+    {
+        var presentationMode = this.presentationMode();
+        return presentationMode === 'inline' || presentationMode === 'fullscreen';
+    },
+
     handlePresentationModeChange: function(event)
     {
         var presentationMode = this.presentationMode();
@@ -951,7 +957,7 @@ Controller.prototype = {
         this.resetHideControlsTimer();
         if (presentationMode != 'fullscreen' && this.video.paused && this.controlsAreHidden())
             this.showControls();
-        this.host.setPreparedForInline(presentationMode === 'inline')
+        this.host.setPreparedToReturnVideoLayerToInline(this.shouldReturnVideoLayerToInline());
     },
 
     handleFullscreenChange: function(event)
index fadaacf..828bac4 100644 (file)
@@ -536,6 +536,11 @@ ControllerIOS.prototype = {
         Controller.prototype.setShouldListenForPlaybackTargetAvailabilityEvent.call(this, shouldListen);
     },
 
+    shouldReturnVideoLayerToInline: function()
+    {
+        return this.presentationMode() === 'inline';
+    },
+
     handlePresentationModeChange: function(event)
     {
         var presentationMode = this.presentationMode();
index ae78778..d6cf337 100644 (file)
@@ -5079,7 +5079,7 @@ void HTMLMediaElement::stopWithoutDestroyingMediaPlayer()
     if (m_videoFullscreenMode != VideoFullscreenModeNone)
         exitFullscreen();
 
-    setPreparedForInline(true);
+    setPreparedToReturnVideoLayerToInline(true);
 
     updatePlaybackControlsManager();
     m_inActiveDocument = false;
@@ -5529,7 +5529,7 @@ PlatformLayer* HTMLMediaElement::platformLayer() const
     return m_player ? m_player->platformLayer() : nullptr;
 }
 
-void HTMLMediaElement::setPreparedForInline(bool value)
+void HTMLMediaElement::setPreparedToReturnVideoLayerToInline(bool value)
 {
     m_preparedForInline = value;
     if (m_preparedForInline && m_preparedForInlineCompletionHandler) {
index b0e9967..9e36689 100644 (file)
@@ -130,7 +130,7 @@ public:
     WEBCORE_EXPORT PlatformMedia platformMedia() const;
     PlatformLayer* platformLayer() const;
     bool isVideoLayerInline();
-    void setPreparedForInline(bool);
+    void setPreparedToReturnVideoLayerToInline(bool);
     void waitForPreparedForInlineThen(std::function<void()> completionHandler = [] { });
 #if PLATFORM(IOS) || (PLATFORM(MAC) && ENABLE(VIDEO_PRESENTATION_MODE))
     void setVideoFullscreenLayer(PlatformLayer*, std::function<void()> completionHandler = [] { });
index 438e39c..8624ce9 100644 (file)
@@ -467,8 +467,7 @@ void WebVideoFullscreenInterfaceMac::enterFullscreen()
 
         if (m_fullscreenChangeObserver)
             m_fullscreenChangeObserver->didEnterFullscreen();
-    } else
-        ASSERT_NOT_REACHED();
+    }
 }
 
 void WebVideoFullscreenInterfaceMac::exitFullscreen(const IntRect& finalRect, NSWindow *parentWindow)
@@ -491,7 +490,14 @@ void WebVideoFullscreenInterfaceMac::exitFullscreenWithoutAnimationToMode(HTMLMe
     if ([m_webVideoFullscreenInterfaceObjC didRequestExitingPIP])
         return;
 
-    [m_webVideoFullscreenInterfaceObjC setExitingToStandardFullscreen:mode == HTMLMediaElementEnums::VideoFullscreenModeStandard];
+    bool isExitingToStandardFullscreen = mode == HTMLMediaElementEnums::VideoFullscreenModeStandard;
+    // On Mac, standard fullscreen is handled by the Fullscreen API and not by WebVideoFullscreenManager.
+    // Just update m_mode directly to HTMLMediaElementEnums::VideoFullscreenModeStandard in that case to keep
+    // m_mode in sync with the fullscreen mode in HTMLMediaElement.
+    if (isExitingToStandardFullscreen)
+        m_mode = HTMLMediaElementEnums::VideoFullscreenModeStandard;
+
+    [m_webVideoFullscreenInterfaceObjC setExitingToStandardFullscreen:isExitingToStandardFullscreen];
     [m_webVideoFullscreenInterfaceObjC exitPIP];
 }