Picture-in-Picture layout test cases interfere with each other
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Nov 2019 15:57:34 +0000 (15:57 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Nov 2019 15:57:34 +0000 (15:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=203614

Patch by Peng Liu <peng.liu6@apple.com> on 2019-11-01
Reviewed by Eric Carlson.

Source/WebCore:

Fix test running issues, no new test is needed.

Add a member variable m_pictureInPictureAPITestEnabled to the HTMLVideoElement class.
When this variable is true, HTMLVideoElement::setFullscreenMode() will mock the
behavior of the Picture-in-Picture mode changing without the AVKit/CoreMedia involvement.
So we can run multiple layout test cases for Picture-in-Picture API simultaneously.
Note: this solution only tests the code for the Picture-in-Picture API implementation!
We still need to use the API tests to test the Picture-in-Picture implementation end-to-end.

* html/HTMLVideoElement.cpp:
(WebCore::HTMLVideoElement::setFullscreenMode):
(WebCore::HTMLVideoElement::setPictureInPictureAPITestEnabled):
* html/HTMLVideoElement.h:
* html/HTMLVideoElement.idl:
* testing/Internals.cpp:
(WebCore::Internals::setPictureInPictureAPITestEnabled):
* testing/Internals.h:
* testing/Internals.idl:

LayoutTests:

Use "internals.setPictureInPictureAPITestEnabled(video, true)" to test
the picture-in-picture API implementation without AVKit/CoreMedia stuffs.

* media/picture-in-picture-api-enter-pip-1-expected.txt:
* media/picture-in-picture-api-enter-pip-1.html:
* media/picture-in-picture-api-enter-pip-2-expected.txt:
* media/picture-in-picture-api-enter-pip-2.html:
* media/picture-in-picture-api-enter-pip-3-expected.txt:
* media/picture-in-picture-api-enter-pip-3.html:
* media/picture-in-picture-api-enter-pip-4-expected.txt:
* media/picture-in-picture-api-enter-pip-4.html:
* media/picture-in-picture-api-exit-pip-1-expected.txt:
* media/picture-in-picture-api-exit-pip-1.html:
* media/picture-in-picture-api-exit-pip-2-expected.txt:
* media/picture-in-picture-api-exit-pip-2.html:
* media/picture-in-picture-api-pip-events-expected.txt:
* media/picture-in-picture-api-pip-events.html:
* media/picture-in-picture-api-pip-window-expected.txt:
* media/picture-in-picture-api-pip-window.html:

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

24 files changed:
LayoutTests/ChangeLog
LayoutTests/media/picture-in-picture-api-enter-pip-1-expected.txt
LayoutTests/media/picture-in-picture-api-enter-pip-1.html
LayoutTests/media/picture-in-picture-api-enter-pip-2-expected.txt
LayoutTests/media/picture-in-picture-api-enter-pip-2.html
LayoutTests/media/picture-in-picture-api-enter-pip-3-expected.txt
LayoutTests/media/picture-in-picture-api-enter-pip-3.html
LayoutTests/media/picture-in-picture-api-enter-pip-4-expected.txt
LayoutTests/media/picture-in-picture-api-enter-pip-4.html
LayoutTests/media/picture-in-picture-api-exit-pip-1-expected.txt
LayoutTests/media/picture-in-picture-api-exit-pip-1.html
LayoutTests/media/picture-in-picture-api-exit-pip-2-expected.txt
LayoutTests/media/picture-in-picture-api-exit-pip-2.html
LayoutTests/media/picture-in-picture-api-pip-events-expected.txt
LayoutTests/media/picture-in-picture-api-pip-events.html
LayoutTests/media/picture-in-picture-api-pip-window-expected.txt
LayoutTests/media/picture-in-picture-api-pip-window.html
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLVideoElement.cpp
Source/WebCore/html/HTMLVideoElement.h
Source/WebCore/html/HTMLVideoElement.idl
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl

index ce0373e..9f2b537 100644 (file)
@@ -1,3 +1,30 @@
+2019-11-01  Peng Liu  <peng.liu6@apple.com>
+
+        Picture-in-Picture layout test cases interfere with each other
+        https://bugs.webkit.org/show_bug.cgi?id=203614
+
+        Reviewed by Eric Carlson.
+
+        Use "internals.setPictureInPictureAPITestEnabled(video, true)" to test
+        the picture-in-picture API implementation without AVKit/CoreMedia stuffs.
+
+        * media/picture-in-picture-api-enter-pip-1-expected.txt:
+        * media/picture-in-picture-api-enter-pip-1.html:
+        * media/picture-in-picture-api-enter-pip-2-expected.txt:
+        * media/picture-in-picture-api-enter-pip-2.html:
+        * media/picture-in-picture-api-enter-pip-3-expected.txt:
+        * media/picture-in-picture-api-enter-pip-3.html:
+        * media/picture-in-picture-api-enter-pip-4-expected.txt:
+        * media/picture-in-picture-api-enter-pip-4.html:
+        * media/picture-in-picture-api-exit-pip-1-expected.txt:
+        * media/picture-in-picture-api-exit-pip-1.html:
+        * media/picture-in-picture-api-exit-pip-2-expected.txt:
+        * media/picture-in-picture-api-exit-pip-2.html:
+        * media/picture-in-picture-api-pip-events-expected.txt:
+        * media/picture-in-picture-api-pip-events.html:
+        * media/picture-in-picture-api-pip-window-expected.txt:
+        * media/picture-in-picture-api-pip-window.html:
+
 2019-11-01  Tim Horton  <timothy_horton@apple.com>
 
         Turn on IOSurface support in the iOS Simulator
index ac1c637..a1c782b 100644 (file)
@@ -1,6 +1,7 @@
 This tests that request Picture-in-Picture requires a user gesture.
 
 RUN(internals.settings.setAllowsPictureInPictureMediaPlayback(true))
+RUN(internals.setPictureInPictureAPITestEnabled(video, true))
 RUN(video.src = findMediaFile("video", "content/test"))
 EVENT(canplaythrough)
 EXPECTED (error.name == 'NotAllowedError') OK
index 83cf9e0..ef84ab1 100644 (file)
@@ -8,6 +8,7 @@
             findMediaElement();
 
             run('internals.settings.setAllowsPictureInPictureMediaPlayback(true)');
+            run('internals.setPictureInPictureAPITestEnabled(video, true)');
             run('video.src = findMediaFile("video", "content/test")');
             await waitFor(video, 'canplaythrough');
 
index 067a047..4898405 100644 (file)
@@ -1,6 +1,7 @@
 This tests that request Picture-in-Picture requires loaded metadata for the video element.
 
 RUN(internals.settings.setAllowsPictureInPictureMediaPlayback(true))
+RUN(internals.setPictureInPictureAPITestEnabled(video, true))
 EXPECTED (error.name == 'InvalidStateError') OK
 END OF TEST
 
index b1a85b2..68a5052 100644 (file)
@@ -8,6 +8,7 @@
             findMediaElement();
 
             run('internals.settings.setAllowsPictureInPictureMediaPlayback(true)');
+            run('internals.setPictureInPictureAPITestEnabled(video, true)');
             runWithKeyDown(function() {
                 video.requestPictureInPicture()
                 .then(() => {
index 5318137..09ad5f3 100644 (file)
@@ -1,6 +1,7 @@
 This tests that request Picture-in-Picture requires video track for the video element.
 
 RUN(internals.settings.setAllowsPictureInPictureMediaPlayback(true))
+RUN(internals.setPictureInPictureAPITestEnabled(video, true))
 RUN(video.src = findMediaFile("audio", "content/test"))
 EVENT(canplaythrough)
 EXPECTED (error.name == 'InvalidStateError') OK
index 4cb79a8..b8adc6f 100644 (file)
@@ -8,6 +8,7 @@
             findMediaElement();
 
             run('internals.settings.setAllowsPictureInPictureMediaPlayback(true)');
+            run('internals.setPictureInPictureAPITestEnabled(video, true)');
             run('video.src = findMediaFile("audio", "content/test")');
             await waitFor(video, 'canplaythrough');
             runWithKeyDown(function() {
index 1e76314..41ab9cd 100644 (file)
@@ -1,6 +1,7 @@
 This tests that request Picture-in-Picture resolves on user click.
 
 RUN(internals.settings.setAllowsPictureInPictureMediaPlayback(true))
+RUN(internals.setPictureInPictureAPITestEnabled(video, true))
 RUN(video.src = findMediaFile("video", "content/test"))
 EVENT(canplaythrough)
 END OF TEST
index ea4e9bc..6fa7209 100644 (file)
@@ -8,12 +8,16 @@
             findMediaElement();
 
             run('internals.settings.setAllowsPictureInPictureMediaPlayback(true)');
+            run('internals.setPictureInPictureAPITestEnabled(video, true)');
+
             run('video.src = findMediaFile("video", "content/test")');
             await waitFor(video, 'canplaythrough');
             runWithKeyDown(function() {
                 video.requestPictureInPicture()
                 .then(() => {
-                    endTest();
+                    document.exitPictureInPicture().then(endTest).catch(() => {
+                        failTest('Failed to exit the Picture-in-Picture mode.');
+                    });
                 })
                 .catch(() => {
                     failTest('request Picture-in-Picture does not resolve on user click.');
index ecc3777..05b9919 100644 (file)
@@ -1,6 +1,7 @@
 This tests that exit Picture-in-Picture resolves when there is a Picture-in-Picture video.
 
 RUN(internals.settings.setAllowsPictureInPictureMediaPlayback(true))
+RUN(internals.setPictureInPictureAPITestEnabled(video, true))
 RUN(video.src = findMediaFile("video", "content/test"))
 EVENT(canplaythrough)
 EVENT(enterpictureinpicture)
index 26839d1..4169554 100644 (file)
@@ -8,18 +8,15 @@
             findMediaElement();
 
             run('internals.settings.setAllowsPictureInPictureMediaPlayback(true)');
+            run('internals.setPictureInPictureAPITestEnabled(video, true)');
+
             run('video.src = findMediaFile("video", "content/test")');
             await waitFor(video, 'canplaythrough');
+
             runWithKeyDown(function() { video.requestPictureInPicture() });
             await waitFor(video, 'enterpictureinpicture');
-            runWithKeyDown(function() {
-                document.exitPictureInPicture()
-                .then(() => {
-                    endTest();
-                })
-                .catch(() => {
-                    failTest('Exit Picture-in-Picture resolves when there is a Picture-in-Picture video.')
-                });
+            document.exitPictureInPicture().then(endTest).catch(() => {
+                failTest('Failed to exit the Picture-in-Picture mode.');
             });
         });
     </script>
index 9115447..42120dc 100644 (file)
@@ -1,5 +1,7 @@
 This tests that exit Picture-in-Picture rejects when there is no Picture-in-Picture video.
 
+RUN(internals.settings.setAllowsPictureInPictureMediaPlayback(true))
+RUN(internals.setPictureInPictureAPITestEnabled(video, true))
 EXPECTED (error.name == 'InvalidStateError') OK
 END OF TEST
 
index f10795b..36c3b4e 100644 (file)
@@ -6,16 +6,18 @@
     <script>
         window.addEventListener('load', async event => {
             findMediaElement();
-            runWithKeyDown(function() {
-                document.exitPictureInPicture()
-                .then(() => {
-                    failTest('Exit Picture-in-Picture rejects when there is no Picture-in-Picture video.');
-                })
-                .catch(error => {
-                    window.error = error;
-                    testExpected('error.name', 'InvalidStateError');
-                    endTest();
-                });
+
+            run('internals.settings.setAllowsPictureInPictureMediaPlayback(true)');
+            run('internals.setPictureInPictureAPITestEnabled(video, true)');
+
+            document.exitPictureInPicture()
+            .then(() => {
+                failTest('Exit Picture-in-Picture rejects when there is no Picture-in-Picture video.');
+            })
+            .catch(error => {
+                window.error = error;
+                testExpected('error.name', 'InvalidStateError');
+                endTest();
             });
         });
     </script>
index ee284b2..ee42d53 100644 (file)
@@ -1,6 +1,7 @@
 This tests that events are fired correctly when a video element enters and exits the Picture-in-Picture mode.
 
 RUN(internals.settings.setAllowsPictureInPictureMediaPlayback(true))
+RUN(internals.setPictureInPictureAPITestEnabled(video, true))
 RUN(video.src = findMediaFile("video", "content/test"))
 EVENT(canplaythrough)
 EVENT(enterpictureinpicture)
index 33b14a9..fcee347 100644 (file)
@@ -8,14 +8,14 @@
             findMediaElement();
 
             run('internals.settings.setAllowsPictureInPictureMediaPlayback(true)');
+            run('internals.setPictureInPictureAPITestEnabled(video, true)');
 
             run('video.src = findMediaFile("video", "content/test")');
             await waitFor(video, 'canplaythrough');
 
             runWithKeyDown(function() { video.requestPictureInPicture(); });
             await waitFor(video, 'enterpictureinpicture');
-
-            runWithKeyDown(function() { document.exitPictureInPicture(); });
+            document.exitPictureInPicture();
             await waitFor(video, 'leavepictureinpicture');
 
             endTest();
index 45d6924..19a5dee 100644 (file)
@@ -1,6 +1,7 @@
 This tests that a pip window is returned correctly when a video element enters the Picture-in-Picture mode.
 
 RUN(internals.settings.setAllowsPictureInPictureMediaPlayback(true))
+RUN(internals.setPictureInPictureAPITestEnabled(video, true))
 RUN(video.src = findMediaFile("video", "content/test"))
 EVENT(canplaythrough)
 EXPECTED (pipWindow.width > '0') OK
index c7201d1..c0ff1d3 100644 (file)
@@ -8,6 +8,7 @@
             findMediaElement();
 
             run('internals.settings.setAllowsPictureInPictureMediaPlayback(true)');
+            run('internals.setPictureInPictureAPITestEnabled(video, true)');
 
             run('video.src = findMediaFile("video", "content/test")');
             await waitFor(video, 'canplaythrough');
                     window.pipWindow = pipWindow;
                     testExpected('pipWindow.width', 0, '>');
                     testExpected('pipWindow.height', 0, '>');
-                    endTest();
+
+                    document.exitPictureInPicture().then(endTest).catch(() => {
+                        failTest('Failed to exit the Picture-in-Picture mode.');
+                    });
                 })
                 .catch(error => {
                     failTest("Failed to enter the Picture-in-Picture mode.");
                 });
             });
+
         });
     </script>
 </head>
index c624799..6ec8013 100644 (file)
@@ -1,3 +1,29 @@
+2019-11-01  Peng Liu  <peng.liu6@apple.com>
+
+        Picture-in-Picture layout test cases interfere with each other
+        https://bugs.webkit.org/show_bug.cgi?id=203614
+
+        Reviewed by Eric Carlson.
+
+        Fix test running issues, no new test is needed.
+
+        Add a member variable m_pictureInPictureAPITestEnabled to the HTMLVideoElement class.
+        When this variable is true, HTMLVideoElement::setFullscreenMode() will mock the
+        behavior of the Picture-in-Picture mode changing without the AVKit/CoreMedia involvement.
+        So we can run multiple layout test cases for Picture-in-Picture API simultaneously.
+        Note: this solution only tests the code for the Picture-in-Picture API implementation!
+        We still need to use the API tests to test the Picture-in-Picture implementation end-to-end.
+
+        * html/HTMLVideoElement.cpp:
+        (WebCore::HTMLVideoElement::setFullscreenMode):
+        (WebCore::HTMLVideoElement::setPictureInPictureAPITestEnabled):
+        * html/HTMLVideoElement.h:
+        * html/HTMLVideoElement.idl:
+        * testing/Internals.cpp:
+        (WebCore::Internals::setPictureInPictureAPITestEnabled):
+        * testing/Internals.h:
+        * testing/Internals.idl:
+
 2019-11-01  Zalan Bujtas  <zalan@apple.com>
 
         [LFC][IFC] Add support for text-align: justify
index 60304ed..edad9fb 100644 (file)
@@ -449,6 +449,22 @@ void HTMLVideoElement::webkitSetPresentationMode(VideoPresentationMode mode)
 
 void HTMLVideoElement::setFullscreenMode(HTMLMediaElementEnums::VideoFullscreenMode mode)
 {
+#if ENABLE(PICTURE_IN_PICTURE_API)
+    if (m_pictureInPictureAPITestEnabled) {
+        if (mode == VideoFullscreenModePictureInPicture) {
+            fullscreenModeChanged(mode);
+            didBecomeFullscreenElement();
+            setVideoFullscreenFrame({0, 0, 100, 100});
+            return;
+        }
+
+        if (mode == VideoFullscreenModeNone) {
+            fullscreenModeChanged(mode);
+            return;
+        }
+    }
+#endif
+
     if (mode == VideoFullscreenModeNone && isFullscreen()) {
         exitFullscreen();
         return;
@@ -517,6 +533,11 @@ void HTMLVideoElement::setPictureInPictureObserver(PictureInPictureObserver* obs
 {
     m_pictureInPictureObserver = observer;
 }
+
+void HTMLVideoElement::setPictureInPictureAPITestEnabled(bool enabled)
+{
+    m_pictureInPictureAPITestEnabled = enabled;
+}
 #endif
 
 #endif
index 162b94e..7a029cb 100644 (file)
@@ -92,6 +92,7 @@ public:
 #if ENABLE(PICTURE_IN_PICTURE_API)
     WEBCORE_EXPORT void didBecomeFullscreenElement() final;
     void setPictureInPictureObserver(PictureInPictureObserver*);
+    WEBCORE_EXPORT void setPictureInPictureAPITestEnabled(bool);
 #endif
 #endif
 
@@ -139,6 +140,8 @@ private:
     bool m_waitingForPictureInPictureWindowFrame { false };
     bool m_isFullscreen { false };
     PictureInPictureObserver* m_pictureInPictureObserver { nullptr };
+
+    bool m_pictureInPictureAPITestEnabled { false };
 #endif
 };
 
index 1f93168..5194317 100644 (file)
@@ -25,6 +25,7 @@
 
 [
     Conditional=VIDEO,
+    ExportMacro=WEBCORE_EXPORT,
     JSGenerateToNativeObject,
 ] interface HTMLVideoElement : HTMLMediaElement {
     [CEReactions=NotNeeded, Reflect] attribute unsigned long width;
index 07ed39f..1bafc66 100644 (file)
@@ -5273,6 +5273,13 @@ void Internals::setMockWebAuthenticationConfiguration(const MockWebAuthenticatio
 }
 #endif
 
+#if ENABLE(PICTURE_IN_PICTURE_API)
+void Internals::setPictureInPictureAPITestEnabled(HTMLVideoElement& videoElement, bool enabled)
+{
+    videoElement.setPictureInPictureAPITestEnabled(enabled);
+}
+#endif
+
 void Internals::setMaxCanvasPixelMemory(unsigned size)
 {
     HTMLCanvasElement::setMaxPixelMemoryForTesting(size);
index c1992fd..0ba066b 100644 (file)
@@ -72,6 +72,7 @@ class HTMLLinkElement;
 class HTMLMediaElement;
 class HTMLPictureElement;
 class HTMLSelectElement;
+class HTMLVideoElement;
 class ImageData;
 class InspectorStubFrontend;
 class InternalSettings;
@@ -898,6 +899,10 @@ public:
     void setMockWebAuthenticationConfiguration(const MockWebAuthenticationConfiguration&);
 #endif
 
+#if ENABLE(PICTURE_IN_PICTURE_API)
+    void setPictureInPictureAPITestEnabled(HTMLVideoElement&, bool);
+#endif
+
     int processIdentifier() const;
 
 private:
index baf627e..d2f9b0b 100644 (file)
@@ -813,4 +813,6 @@ enum CompositingPolicy {
     void addPrefetchLoadEventListener(HTMLLinkElement link, EventListener? callback);
 
     [Conditional=WEB_AUTHN] void setMockWebAuthenticationConfiguration(MockWebAuthenticationConfiguration configuration);
+
+    [Conditional=PICTURE_IN_PICTURE_API] void setPictureInPictureAPITestEnabled(HTMLVideoElement videoElement, boolean enabled);
 };