Title: [205938] trunk
Revision
205938
Author
[email protected]
Date
2016-09-14 16:51:51 -0700 (Wed, 14 Sep 2016)

Log Message

Media controls behave strangely when changing media sources
https://bugs.webkit.org/show_bug.cgi?id=161914
<rdar://problem/28227805>

Reviewed by Tim Horton.

Source/WebCore:

Addresses media controls flickering while changing the source of a media element. To accomplish this, we make
the following changes to the media controls main content heuristic:

- Prevent elements that are not mostly within the mainframe rect (or elements with empty rects) from showing
  media controls. Many websites that rely on same document navigation will move videos offscreen when navigating
  to a section of their site that does not play media. Without this check, we would not know to hide a video
  element on certain popular websites that use this technique, since the video has been interacted with in the
  past.

- Rather than check whether a media element currently has video/audio sources, check whether it has ever had
  audio. Many websites will use the same media element across different videos and change only the source, and
  we should not prevent a media element from having media controls on grounds of having no audio or video in
  this case.

- Rather than add user gesture and playback behavior restrictions before dispatching an ended event, add only
  the gesture restriction immediately, and add the playback restriction after waiting for a grace period only if
  the user has not interacted with the video since ending, and the video is not currently playing or about to
  play. This gives the user a chance to interact with the controls when a video ends, but also allows the page
  to load or begin playing a new video with the same media element without thrashing media control state.

Adds 3 new API tests.

* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::HTMLMediaElement):
(WebCore::HTMLMediaElement::~HTMLMediaElement):
(WebCore::HTMLMediaElement::mediaPlayerActiveSourceBuffersChanged):
(WebCore::HTMLMediaElement::seekWithTolerance):
(WebCore::HTMLMediaElement::beginScrubbing):
(WebCore::HTMLMediaElement::addBehaviorRestrictionsOnEndIfNecessary):
(WebCore::HTMLMediaElement::mediaPlayerCharacteristicChanged):
(WebCore::HTMLMediaElement::playbackControlsManagerBehaviorRestrictionsTimerFired):
* html/HTMLMediaElement.h:
(WebCore::HTMLMediaElement::hasEverHadAudio):
(WebCore::HTMLMediaElement::hasEverHadVideo):
* html/MediaElementSession.cpp:
(WebCore::MediaElementSession::canShowControlsManager):
(WebCore::isElementRectMostlyInMainFrame):
* platform/graphics/MediaPlayer.h:
(WebCore::MediaPlayerClient::mediaPlayerActiveSourceBuffersChanged):
* platform/graphics/MediaPlayerPrivate.h:
(WebCore::MediaPlayerPrivateInterface::notifyActiveSourceBuffersChanged):
* platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h:
* platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::notifyActiveSourceBuffersChanged):
* platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm:
(WebCore::MediaSourcePrivateAVFObjC::removeSourceBuffer):
(WebCore::MediaSourcePrivateAVFObjC::sourceBufferPrivateDidChangeActiveState):

Source/WebKit2:

Allows a web page to have an active video for a media control manager even if no audio or video is currently
being produced. This is because the media element may be in a state where it is changing its source and does not
currently have a video or audio track.

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::hasActiveVideoForControlsManager):

Tools:

Adds three new unit tests verifying that media controls remain stable during common `src` change scenarios. Also
tweaks an existing test to account for new `ended` behavior.

* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/WebKit2Cocoa/VideoControlsManager.mm:
(-[VideoControlsManagerTestWebView waitForMediaControlsToShow]):
(-[VideoControlsManagerTestWebView waitForMediaControlsToHide]):
(TestWebKitAPI::TEST):
* TestWebKitAPI/Tests/WebKit2Cocoa/change-video-source-on-click.html: Added.
* TestWebKitAPI/Tests/WebKit2Cocoa/change-video-source-on-end.html: Added.
* TestWebKitAPI/Tests/WebKit2Cocoa/large-video-offscreen.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (205937 => 205938)


--- trunk/Source/WebCore/ChangeLog	2016-09-14 23:17:59 UTC (rev 205937)
+++ trunk/Source/WebCore/ChangeLog	2016-09-14 23:51:51 UTC (rev 205938)
@@ -1,3 +1,59 @@
+2016-09-14  Wenson Hsieh  <[email protected]>
+
+        Media controls behave strangely when changing media sources
+        https://bugs.webkit.org/show_bug.cgi?id=161914
+        <rdar://problem/28227805>
+
+        Reviewed by Tim Horton.
+
+        Addresses media controls flickering while changing the source of a media element. To accomplish this, we make
+        the following changes to the media controls main content heuristic:
+
+        - Prevent elements that are not mostly within the mainframe rect (or elements with empty rects) from showing
+          media controls. Many websites that rely on same document navigation will move videos offscreen when navigating
+          to a section of their site that does not play media. Without this check, we would not know to hide a video
+          element on certain popular websites that use this technique, since the video has been interacted with in the
+          past.
+
+        - Rather than check whether a media element currently has video/audio sources, check whether it has ever had
+          audio. Many websites will use the same media element across different videos and change only the source, and
+          we should not prevent a media element from having media controls on grounds of having no audio or video in
+          this case.
+
+        - Rather than add user gesture and playback behavior restrictions before dispatching an ended event, add only
+          the gesture restriction immediately, and add the playback restriction after waiting for a grace period only if
+          the user has not interacted with the video since ending, and the video is not currently playing or about to
+          play. This gives the user a chance to interact with the controls when a video ends, but also allows the page
+          to load or begin playing a new video with the same media element without thrashing media control state.
+
+        Adds 3 new API tests.
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::HTMLMediaElement):
+        (WebCore::HTMLMediaElement::~HTMLMediaElement):
+        (WebCore::HTMLMediaElement::mediaPlayerActiveSourceBuffersChanged):
+        (WebCore::HTMLMediaElement::seekWithTolerance):
+        (WebCore::HTMLMediaElement::beginScrubbing):
+        (WebCore::HTMLMediaElement::addBehaviorRestrictionsOnEndIfNecessary):
+        (WebCore::HTMLMediaElement::mediaPlayerCharacteristicChanged):
+        (WebCore::HTMLMediaElement::playbackControlsManagerBehaviorRestrictionsTimerFired):
+        * html/HTMLMediaElement.h:
+        (WebCore::HTMLMediaElement::hasEverHadAudio):
+        (WebCore::HTMLMediaElement::hasEverHadVideo):
+        * html/MediaElementSession.cpp:
+        (WebCore::MediaElementSession::canShowControlsManager):
+        (WebCore::isElementRectMostlyInMainFrame):
+        * platform/graphics/MediaPlayer.h:
+        (WebCore::MediaPlayerClient::mediaPlayerActiveSourceBuffersChanged):
+        * platform/graphics/MediaPlayerPrivate.h:
+        (WebCore::MediaPlayerPrivateInterface::notifyActiveSourceBuffersChanged):
+        * platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h:
+        * platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:
+        (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::notifyActiveSourceBuffersChanged):
+        * platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm:
+        (WebCore::MediaSourcePrivateAVFObjC::removeSourceBuffer):
+        (WebCore::MediaSourcePrivateAVFObjC::sourceBufferPrivateDidChangeActiveState):
+
 2016-09-14  Eric Carlson  <[email protected]>
 
         [MediaStream] Minor cleanup

Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (205937 => 205938)


--- trunk/Source/WebCore/html/HTMLMediaElement.cpp	2016-09-14 23:17:59 UTC (rev 205937)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp	2016-09-14 23:51:51 UTC (rev 205938)
@@ -163,6 +163,8 @@
 static const double ScanRepeatDelay = 1.5;
 static const double ScanMaximumRate = 8;
 
+static const double HideMediaControlsAfterEndedDelay = 6;
+
 static void setFlags(unsigned& value, unsigned flags)
 {
     value |= flags;
@@ -426,6 +428,7 @@
     , m_progressEventTimer(*this, &HTMLMediaElement::progressEventTimerFired)
     , m_playbackProgressTimer(*this, &HTMLMediaElement::playbackProgressTimerFired)
     , m_scanTimer(*this, &HTMLMediaElement::scanTimerFired)
+    , m_playbackControlsManagerBehaviorRestrictionsTimer(*this, &HTMLMediaElement::playbackControlsManagerBehaviorRestrictionsTimerFired)
     , m_playedTimeRanges()
     , m_asyncEventQueue(*this)
     , m_requestedPlaybackRate(1)
@@ -482,6 +485,8 @@
     , m_creatingControls(false)
     , m_receivedLayoutSizeChanged(false)
     , m_hasEverNotifiedAboutPlaying(false)
+    , m_hasEverHadAudio(false)
+    , m_hasEverHadVideo(false)
 #if ENABLE(MEDIA_CONTROLS_SCRIPT)
     , m_mediaControlsDependOnPageScaleFactor(false)
     , m_haveSetUpCaptionContainer(false)
@@ -630,6 +635,7 @@
     m_promiseTaskQueue.close();
     m_pauseAfterDetachedTaskQueue.close();
     m_updatePlaybackControlsManagerQueue.close();
+    m_playbackControlsManagerBehaviorRestrictionsQueue.close();
 
     m_completelyLoaded = true;
 
@@ -972,6 +978,12 @@
     m_pendingActionTimer.startOneShot(0);
 }
 
+void HTMLMediaElement::mediaPlayerActiveSourceBuffersChanged(const MediaPlayer*)
+{
+    m_hasEverHadAudio |= hasAudio();
+    m_hasEverHadVideo |= hasVideo();
+}
+
 void HTMLMediaElement::scheduleEvent(const AtomicString& eventName)
 {
 #if LOG_MEDIA_EVENTS
@@ -2675,6 +2687,9 @@
         m_seekTaskQueue.enqueueTask(std::bind(&HTMLMediaElement::seekTask, this));
     } else
         seekTask();
+
+    if (ScriptController::processingUserGestureForMedia())
+        m_mediaSession->removeBehaviorRestriction(MediaElementSession::RequireUserGestureToControlControlsManager);
 }
 
 void HTMLMediaElement::seekTask()
@@ -3491,6 +3506,8 @@
             setPausedInternal(true);
         }
     }
+
+    m_mediaSession->removeBehaviorRestriction(MediaElementSession::RequireUserGestureToControlControlsManager);
 }
 
 void HTMLMediaElement::endScrubbing()
@@ -4537,7 +4554,9 @@
     if (isFullscreen())
         return;
 
-    m_mediaSession->addBehaviorRestriction(MediaElementSession::RequirePlaybackToControlControlsManager | MediaElementSession::RequireUserGestureToControlControlsManager);
+    m_mediaSession->addBehaviorRestriction(MediaElementSession::RequireUserGestureToControlControlsManager);
+    m_playbackControlsManagerBehaviorRestrictionsTimer.stop();
+    m_playbackControlsManagerBehaviorRestrictionsTimer.startOneShot(HideMediaControlsAfterEndedDelay);
 }
 
 void HTMLMediaElement::mediaPlayerVolumeChanged(MediaPlayer*)
@@ -4772,6 +4791,9 @@
     document().updateIsPlayingMedia();
 #endif
 
+    m_hasEverHadAudio |= hasAudio();
+    m_hasEverHadVideo |= hasVideo();
+
     endProcessingMediaPlayerCallback();
 }
 
@@ -7301,6 +7323,25 @@
         m_updatePlaybackControlsManagerQueue.enqueueTask(std::bind(&HTMLMediaElement::updatePlaybackControlsManager, this));
 }
 
+void HTMLMediaElement::playbackControlsManagerBehaviorRestrictionsTimerFired()
+{
+    if (m_playbackControlsManagerBehaviorRestrictionsQueue.hasPendingTasks())
+        return;
+
+    if (!m_mediaSession->hasBehaviorRestriction(MediaElementSession::RequireUserGestureToControlControlsManager))
+        return;
+
+    RefPtr<HTMLMediaElement> protectedThis(this);
+    m_playbackControlsManagerBehaviorRestrictionsQueue.enqueueTask([protectedThis] () {
+        MediaElementSession* mediaElementSession = protectedThis->m_mediaSession.get();
+        if (protectedThis->isPlaying() || mediaElementSession->state() == PlatformMediaSession::Autoplaying || mediaElementSession->state() == PlatformMediaSession::Playing)
+            return;
+
+        mediaElementSession->addBehaviorRestriction(MediaElementSession::RequirePlaybackToControlControlsManager);
+        protectedThis->scheduleUpdatePlaybackControlsManager();
+    });
+}
+
 bool HTMLMediaElement::shouldOverrideBackgroundLoadingRestriction() const
 {
 #if ENABLE(WIRELESS_PLAYBACK_TARGET)

Modified: trunk/Source/WebCore/html/HTMLMediaElement.h (205937 => 205938)


--- trunk/Source/WebCore/html/HTMLMediaElement.h	2016-09-14 23:17:59 UTC (rev 205937)
+++ trunk/Source/WebCore/html/HTMLMediaElement.h	2016-09-14 23:51:51 UTC (rev 205938)
@@ -473,6 +473,9 @@
     bool hasEverNotifiedAboutPlaying() const;
     void setShouldDelayLoadEvent(bool);
 
+    bool hasEverHadAudio() const { return m_hasEverHadAudio; }
+    bool hasEverHadVideo() const { return m_hasEverHadVideo; }
+
 protected:
     HTMLMediaElement(const QualifiedName&, Document&, bool createdByParser);
     virtual ~HTMLMediaElement();
@@ -620,6 +623,8 @@
     GraphicsDeviceAdapter* mediaPlayerGraphicsDeviceAdapter(const MediaPlayer*) const override;
 #endif
 
+    void mediaPlayerActiveSourceBuffersChanged(const MediaPlayer*) override;
+
     bool mediaPlayerShouldWaitForResponseToAuthenticationChallenge(const AuthenticationChallenge&) override;
     void mediaPlayerHandlePlaybackCommand(PlatformMediaSession::RemoteControlCommandType command) override { didReceiveRemoteControlCommand(command, nullptr); }
     String mediaPlayerSourceApplicationIdentifier() const override;
@@ -791,6 +796,7 @@
     void pauseAfterDetachedTask();
     void updatePlaybackControlsManager();
     void scheduleUpdatePlaybackControlsManager();
+    void playbackControlsManagerBehaviorRestrictionsTimerFired();
 
     void updateRenderer();
 
@@ -804,6 +810,7 @@
     Timer m_progressEventTimer;
     Timer m_playbackProgressTimer;
     Timer m_scanTimer;
+    Timer m_playbackControlsManagerBehaviorRestrictionsTimer;
     GenericTaskQueue<Timer> m_seekTaskQueue;
     GenericTaskQueue<Timer> m_resizeTaskQueue;
     GenericTaskQueue<Timer> m_shadowDOMTaskQueue;
@@ -810,6 +817,7 @@
     GenericTaskQueue<Timer> m_promiseTaskQueue;
     GenericTaskQueue<Timer> m_pauseAfterDetachedTaskQueue;
     GenericTaskQueue<Timer> m_updatePlaybackControlsManagerQueue;
+    GenericTaskQueue<Timer> m_playbackControlsManagerBehaviorRestrictionsQueue;
     RefPtr<TimeRanges> m_playedTimeRanges;
     GenericEventQueue m_asyncEventQueue;
 
@@ -946,6 +954,9 @@
     bool m_receivedLayoutSizeChanged : 1;
     bool m_hasEverNotifiedAboutPlaying : 1;
 
+    bool m_hasEverHadAudio : 1;
+    bool m_hasEverHadVideo : 1;
+
 #if ENABLE(MEDIA_CONTROLS_SCRIPT)
     bool m_mediaControlsDependOnPageScaleFactor : 1;
     bool m_haveSetUpCaptionContainer : 1;

Modified: trunk/Source/WebCore/html/MediaElementSession.cpp (205937 => 205938)


--- trunk/Source/WebCore/html/MediaElementSession.cpp	2016-09-14 23:17:59 UTC (rev 205937)
+++ trunk/Source/WebCore/html/MediaElementSession.cpp	2016-09-14 23:51:51 UTC (rev 205938)
@@ -59,6 +59,7 @@
 
 static const double elementMainContentCheckInterval = .250;
 
+static bool isElementRectMostlyInMainFrame(const HTMLMediaElement&);
 static bool isElementLargeEnoughForMainContent(const HTMLMediaElement&, MediaSessionMainContentPurpose);
 
 #if !LOG_DISABLED
@@ -222,7 +223,12 @@
         return true;
     }
 
-    if (!m_element.hasAudio()) {
+    if (!isElementRectMostlyInMainFrame(m_element)) {
+        LOG(Media, "MediaElementSession::canShowControlsManager - returning FALSE: Not in main frame");
+        return false;
+    }
+
+    if (!m_element.hasAudio() && !m_element.hasEverHadAudio()) {
         LOG(Media, "MediaElementSession::canShowControlsManager - returning FALSE: No audio");
         return false;
     }
@@ -268,7 +274,7 @@
             return false;
         }
 
-        if (!m_element.hasVideo()) {
+        if (!m_element.hasVideo() && !m_element.hasEverHadVideo()) {
             LOG(Media, "MediaElementSession::canShowControlsManager - returning FALSE: No video");
             return false;
         }
@@ -630,6 +636,27 @@
     return true;
 }
 
+static bool isElementRectMostlyInMainFrame(const HTMLMediaElement& element)
+{
+    if (!element.renderer())
+        return false;
+
+    auto* documentFrame = element.document().frame();
+    if (!documentFrame)
+        return false;
+
+    auto mainFrameView = documentFrame->mainFrame().view();
+    if (!mainFrameView)
+        return false;
+
+    IntRect mainFrameRectAdjustedForScrollPosition = IntRect(-mainFrameView->documentScrollPositionRelativeToViewOrigin(), mainFrameView->contentsSize());
+    IntRect elementRectInMainFrame = element.clientRect();
+    unsigned int totalElementArea = elementRectInMainFrame.area();
+    elementRectInMainFrame.intersect(mainFrameRectAdjustedForScrollPosition);
+
+    return elementRectInMainFrame.area() > totalElementArea / 2;
+}
+
 static bool isElementLargeRelativeToMainFrame(const HTMLMediaElement& element)
 {
     static const double minimumPercentageOfMainFrameAreaForMainContent = 0.9;

Modified: trunk/Source/WebCore/platform/graphics/MediaPlayer.h (205937 => 205938)


--- trunk/Source/WebCore/platform/graphics/MediaPlayer.h	2016-09-14 23:17:59 UTC (rev 205937)
+++ trunk/Source/WebCore/platform/graphics/MediaPlayer.h	2016-09-14 23:51:51 UTC (rev 205938)
@@ -196,6 +196,8 @@
     // availability of the platformLayer().
     virtual void mediaPlayerRenderingModeChanged(MediaPlayer*) { }
 
+    virtual void mediaPlayerActiveSourceBuffersChanged(const MediaPlayer*) { }
+
 #if PLATFORM(WIN) && USE(AVFOUNDATION)
     virtual GraphicsDeviceAdapter* mediaPlayerGraphicsDeviceAdapter(const MediaPlayer*) const { return 0; }
 #endif

Modified: trunk/Source/WebCore/platform/graphics/MediaPlayerPrivate.h (205937 => 205938)


--- trunk/Source/WebCore/platform/graphics/MediaPlayerPrivate.h	2016-09-14 23:17:59 UTC (rev 205937)
+++ trunk/Source/WebCore/platform/graphics/MediaPlayerPrivate.h	2016-09-14 23:51:51 UTC (rev 205938)
@@ -275,6 +275,8 @@
 #if ENABLE(AVF_CAPTIONS)
     virtual void notifyTrackModeChanged() { }
 #endif
+
+    virtual void notifyActiveSourceBuffersChanged() { }
 };
 
 }

Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h (205937 => 205938)


--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h	2016-09-14 23:17:59 UTC (rev 205937)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h	2016-09-14 23:51:51 UTC (rev 205938)
@@ -165,6 +165,7 @@
     bool supportsAcceleratedRendering() const override;
     // called when the rendering system flips the into or out of accelerated rendering mode.
     void acceleratedRenderingStateChanged() override;
+    void notifyActiveSourceBuffersChanged() override;
 
     MediaPlayer::MovieLoadType movieLoadType() const override;
 

Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm (205937 => 205938)


--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm	2016-09-14 23:17:59 UTC (rev 205937)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm	2016-09-14 23:51:51 UTC (rev 205938)
@@ -556,6 +556,11 @@
         destroyLayer();
 }
 
+void MediaPlayerPrivateMediaSourceAVFObjC::notifyActiveSourceBuffersChanged()
+{
+    m_player->client().mediaPlayerActiveSourceBuffersChanged(m_player);
+}
+
 MediaPlayer::MovieLoadType MediaPlayerPrivateMediaSourceAVFObjC::movieLoadType() const
 {
     return MediaPlayer::StoredStream;

Modified: trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm (205937 => 205938)


--- trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm	2016-09-14 23:17:59 UTC (rev 205937)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm	2016-09-14 23:51:51 UTC (rev 205938)
@@ -83,8 +83,10 @@
     ASSERT(m_sourceBuffers.contains(buffer));
 
     size_t pos = m_activeSourceBuffers.find(buffer);
-    if (pos != notFound)
+    if (pos != notFound) {
         m_activeSourceBuffers.remove(pos);
+        m_player->notifyActiveSourceBuffersChanged();
+    }
 
     pos = m_sourceBuffers.find(buffer);
     m_sourceBuffers[pos]->clearMediaSource();
@@ -141,13 +143,17 @@
 
 void MediaSourcePrivateAVFObjC::sourceBufferPrivateDidChangeActiveState(SourceBufferPrivateAVFObjC* buffer, bool active)
 {
-    if (active && !m_activeSourceBuffers.contains(buffer))
+    if (active && !m_activeSourceBuffers.contains(buffer)) {
         m_activeSourceBuffers.append(buffer);
+        m_player->notifyActiveSourceBuffersChanged();
+    }
 
     if (!active) {
         size_t position = m_activeSourceBuffers.find(buffer);
-        if (position != notFound)
+        if (position != notFound) {
             m_activeSourceBuffers.remove(position);
+            m_player->notifyActiveSourceBuffersChanged();
+        }
     }
 }
 

Modified: trunk/Source/WebKit2/ChangeLog (205937 => 205938)


--- trunk/Source/WebKit2/ChangeLog	2016-09-14 23:17:59 UTC (rev 205937)
+++ trunk/Source/WebKit2/ChangeLog	2016-09-14 23:51:51 UTC (rev 205938)
@@ -1,3 +1,18 @@
+2016-09-14  Wenson Hsieh  <[email protected]>
+
+        Media controls behave strangely when changing media sources
+        https://bugs.webkit.org/show_bug.cgi?id=161914
+        <rdar://problem/28227805>
+
+        Reviewed by Tim Horton.
+
+        Allows a web page to have an active video for a media control manager even if no audio or video is currently
+        being produced. This is because the media element may be in a state where it is changing its source and does not
+        currently have a video or audio track.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::hasActiveVideoForControlsManager):
+
 2016-09-14  Beth Dakin  <[email protected]>
 
         Add needsPlainTextQuirk and send it to the UIProcess

Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp (205937 => 205938)


--- trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp	2016-09-14 23:17:59 UTC (rev 205937)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp	2016-09-14 23:51:51 UTC (rev 205938)
@@ -6329,7 +6329,7 @@
 bool WebPageProxy::hasActiveVideoForControlsManager() const
 {
 #if ENABLE(VIDEO_PRESENTATION_MODE)
-    return m_playbackSessionManager && m_playbackSessionManager->controlsManagerInterface() && m_mediaState & MediaProducer::HasAudioOrVideo;
+    return m_playbackSessionManager && m_playbackSessionManager->controlsManagerInterface();
 #else
     return false;
 #endif

Modified: trunk/Tools/ChangeLog (205937 => 205938)


--- trunk/Tools/ChangeLog	2016-09-14 23:17:59 UTC (rev 205937)
+++ trunk/Tools/ChangeLog	2016-09-14 23:51:51 UTC (rev 205938)
@@ -1,3 +1,23 @@
+2016-09-14  Wenson Hsieh  <[email protected]>
+
+        Media controls behave strangely when changing media sources
+        https://bugs.webkit.org/show_bug.cgi?id=161914
+        <rdar://problem/28227805>
+
+        Reviewed by Tim Horton.
+
+        Adds three new unit tests verifying that media controls remain stable during common `src` change scenarios. Also
+        tweaks an existing test to account for new `ended` behavior.
+
+        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+        * TestWebKitAPI/Tests/WebKit2Cocoa/VideoControlsManager.mm:
+        (-[VideoControlsManagerTestWebView waitForMediaControlsToShow]):
+        (-[VideoControlsManagerTestWebView waitForMediaControlsToHide]):
+        (TestWebKitAPI::TEST):
+        * TestWebKitAPI/Tests/WebKit2Cocoa/change-video-source-on-click.html: Added.
+        * TestWebKitAPI/Tests/WebKit2Cocoa/change-video-source-on-end.html: Added.
+        * TestWebKitAPI/Tests/WebKit2Cocoa/large-video-offscreen.html: Added.
+
 2016-09-14  Jonathan Bedard  <[email protected]>
 
         Fix mastercfg_unittest

Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (205937 => 205938)


--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj	2016-09-14 23:17:59 UTC (rev 205937)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj	2016-09-14 23:51:51 UTC (rev 205938)
@@ -75,6 +75,9 @@
 		2E691AF31D79E75E00129407 /* large-video-playing-scroll-away.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 2E691AF21D79E75400129407 /* large-video-playing-scroll-away.html */; };
 		2E7765CD16C4D80A00BA2BB1 /* mainIOS.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2E7765CC16C4D80A00BA2BB1 /* mainIOS.mm */; };
 		2E7765CF16C4D81100BA2BB1 /* mainMac.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2E7765CE16C4D81100BA2BB1 /* mainMac.mm */; };
+		2EFF06C31D88621E0004BB30 /* large-video-offscreen.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 2EFF06C21D8862120004BB30 /* large-video-offscreen.html */; };
+		2EFF06C51D8867760004BB30 /* change-video-source-on-click.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 2EFF06C41D8867700004BB30 /* change-video-source-on-click.html */; };
+		2EFF06C71D886A580004BB30 /* change-video-source-on-end.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 2EFF06C61D886A560004BB30 /* change-video-source-on-end.html */; };
 		33BE5AF9137B5AAE00705813 /* MouseMoveAfterCrash_Bundle.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 33BE5AF8137B5AAE00705813 /* MouseMoveAfterCrash_Bundle.cpp */; };
 		33DC8912141955FE00747EF7 /* simple-iframe.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 33DC890E1419539300747EF7 /* simple-iframe.html */; };
 		33DC89141419579F00747EF7 /* LoadCanceledNoServerRedirectCallback_Bundle.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 33DC89131419579F00747EF7 /* LoadCanceledNoServerRedirectCallback_Bundle.cpp */; };
@@ -538,6 +541,9 @@
 			dstPath = TestWebKitAPI.resources;
 			dstSubfolderSpec = 7;
 			files = (
+				2EFF06C71D886A580004BB30 /* change-video-source-on-end.html in Copy Resources */,
+				2EFF06C51D8867760004BB30 /* change-video-source-on-click.html in Copy Resources */,
+				2EFF06C31D88621E0004BB30 /* large-video-offscreen.html in Copy Resources */,
 				2E131C181D83A98A001BA36C /* wide-autoplaying-video-with-audio.html in Copy Resources */,
 				2E54F40D1D7BC84200921ADF /* large-video-mutes-onplaying.html in Copy Resources */,
 				2E691AF31D79E75E00129407 /* large-video-playing-scroll-away.html in Copy Resources */,
@@ -780,6 +786,9 @@
 		2E691AF21D79E75400129407 /* large-video-playing-scroll-away.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "large-video-playing-scroll-away.html"; sourceTree = "<group>"; };
 		2E7765CC16C4D80A00BA2BB1 /* mainIOS.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = mainIOS.mm; sourceTree = "<group>"; };
 		2E7765CE16C4D81100BA2BB1 /* mainMac.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = mainMac.mm; sourceTree = "<group>"; };
+		2EFF06C21D8862120004BB30 /* large-video-offscreen.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "large-video-offscreen.html"; sourceTree = "<group>"; };
+		2EFF06C41D8867700004BB30 /* change-video-source-on-click.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "change-video-source-on-click.html"; sourceTree = "<group>"; };
+		2EFF06C61D886A560004BB30 /* change-video-source-on-end.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "change-video-source-on-end.html"; sourceTree = "<group>"; };
 		333B9CE11277F23100FEFCE3 /* PreventEmptyUserAgent.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = PreventEmptyUserAgent.cpp; sourceTree = "<group>"; };
 		33BE5AF4137B5A6C00705813 /* MouseMoveAfterCrash.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = MouseMoveAfterCrash.cpp; sourceTree = "<group>"; };
 		33BE5AF8137B5AAE00705813 /* MouseMoveAfterCrash_Bundle.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = MouseMoveAfterCrash_Bundle.cpp; sourceTree = "<group>"; };
@@ -1455,6 +1464,9 @@
 		A16F66B81C40E9E100BD4D24 /* Resources */ = {
 			isa = PBXGroup;
 			children = (
+				2EFF06C61D886A560004BB30 /* change-video-source-on-end.html */,
+				2EFF06C41D8867700004BB30 /* change-video-source-on-click.html */,
+				2EFF06C21D8862120004BB30 /* large-video-offscreen.html */,
 				2E131C171D83A97E001BA36C /* wide-autoplaying-video-with-audio.html */,
 				2E54F40C1D7BC83900921ADF /* large-video-mutes-onplaying.html */,
 				2E691AF21D79E75400129407 /* large-video-playing-scroll-away.html */,

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/VideoControlsManager.mm (205937 => 205938)


--- trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/VideoControlsManager.mm	2016-09-14 23:17:59 UTC (rev 205937)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/VideoControlsManager.mm	2016-09-14 23:51:51 UTC (rev 205938)
@@ -119,6 +119,18 @@
     TestWebKitAPI::Util::run(&doneWaiting);
 }
 
+- (void)waitForMediaControlsToShow
+{
+    while (![self _hasActiveVideoForControlsManager])
+        [[NSRunLoop currentRunLoop] runMode:NSDefaultRunLoopMode beforeDate:[NSDate distantPast]];
+}
+
+- (void)waitForMediaControlsToHide
+{
+    while ([self _hasActiveVideoForControlsManager])
+        [[NSRunLoop currentRunLoop] runMode:NSDefaultRunLoopMode beforeDate:[NSDate distantPast]];
+}
+
 - (void)performAfterReceivingMessage:(NSString *)message action:(dispatch_block_t)action
 {
     RetainPtr<MessageHandler> handler = adoptNS([[MessageHandler alloc] initWithMessage:message handler:action]);
@@ -319,9 +331,13 @@
 {
     RetainPtr<VideoControlsManagerTestWebView*> webView = setUpWebViewForTestingVideoControlsManager(NSMakeRect(0, 0, 100, 100));
 
-    // Since the video has ended, the expectation is NO even if the page programmatically seeks to the beginning.
     [webView loadTestPageNamed:@"large-video-seek-after-ending"];
-    [webView expectControlsManager:NO afterReceivingMessage:@"ended"];
+
+    // Immediately after ending, the controls should still be present.
+    [webView expectControlsManager:YES afterReceivingMessage:@"ended"];
+
+    // At some point in the future, they should automatically hide.
+    [webView waitForMediaControlsToHide];
 }
 
 TEST(VideoControlsManager, VideoControlsManagerLargeAutoplayingVideoSeeksAndPlaysAfterEnding)
@@ -385,6 +401,33 @@
     TestWebKitAPI::Util::run(&finishedTest);
 }
 
+TEST(VideoControlsManager, VideoControlsManagerDoesNotShowMediaControlsForOffscreenVideo)
+{
+    RetainPtr<VideoControlsManagerTestWebView*> webView = setUpWebViewForTestingVideoControlsManager(NSMakeRect(0, 0, 1024, 768));
+
+    [webView loadTestPageNamed:@"large-video-offscreen"];
+    [webView expectControlsManager:NO afterReceivingMessage:@"moved"];
+}
+
+TEST(VideoControlsManager, VideoControlsManagerKeepsControlsStableDuringSrcChangeOnClick)
+{
+    RetainPtr<VideoControlsManagerTestWebView*> webView = setUpWebViewForTestingVideoControlsManager(NSMakeRect(0, 0, 800, 600));
+
+    [webView loadTestPageNamed:@"change-video-source-on-click"];
+    [webView waitForPageToLoadWithAutoplayingVideos:1];
+    [webView mouseDownAtPoint:NSMakePoint(400, 300)];
+
+    [webView expectControlsManager:YES afterReceivingMessage:@"changed"];
+}
+
+TEST(VideoControlsManager, VideoControlsManagerKeepsControlsStableDuringSrcChangeOnEnd)
+{
+    RetainPtr<VideoControlsManagerTestWebView*> webView = setUpWebViewForTestingVideoControlsManager(NSMakeRect(0, 0, 800, 600));
+
+    [webView loadTestPageNamed:@"change-video-source-on-end"];
+    [webView expectControlsManager:YES afterReceivingMessage:@"changed"];
+}
+
 TEST(VideoControlsManager, VideoControlsManagerSmallVideoInMediaDocument)
 {
     RetainPtr<VideoControlsManagerTestWebView*> webView = setUpWebViewForTestingVideoControlsManager(NSMakeRect(0, 0, 100, 100));

Added: trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/change-video-source-on-click.html (0 => 205938)


--- trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/change-video-source-on-click.html	                        (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/change-video-source-on-click.html	2016-09-14 23:51:51 UTC (rev 205938)
@@ -0,0 +1,33 @@
+<head>
+    <style>
+        video {
+            width: 800px;
+            height: 600px;
+        }
+    </style>
+    <script type="text/_javascript_">
+        function changeSource() {
+            var video = document.querySelector("video");
+            video.src = ""
+            video.play();
+            setTimeout(function() {
+                try {
+                    window.webkit.messageHandlers.playingHandler.postMessage("changed");
+                } catch(e) {
+                }
+            });
+        }
+
+        function handlePlaying() {
+            setTimeout(function() {
+                try {
+                    window.webkit.messageHandlers.playingHandler.postMessage("autoplayed");
+                } catch(e) {
+                }
+            }, 0);
+        }
+    </script>
+</head>
+<body>
+    <video autoplay _onplaying_=handlePlaying() _onmousedown_=changeSource() src=""
+</body>

Added: trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/change-video-source-on-end.html (0 => 205938)


--- trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/change-video-source-on-end.html	                        (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/change-video-source-on-end.html	2016-09-14 23:51:51 UTC (rev 205938)
@@ -0,0 +1,35 @@
+<head>
+    <style>
+        video {
+            width: 800px;
+            height: 600px;
+        }
+    </style>
+    <script type="text/_javascript_">
+        var hasBegunPlayingBefore = false;
+
+        function changeSource() {
+            var video = document.querySelector("video");
+            video.src = ""
+            video.play();
+            setTimeout(function() {
+                try {
+                    window.webkit.messageHandlers.playingHandler.postMessage("changed");
+                } catch(e) {
+                }
+            });
+        }
+
+        function handlePlaying() {
+            if (hasBegunPlayingBefore)
+                return;
+
+            hasBegunPlayingBefore = true;
+            var video = document.querySelector("video");
+            video.currentTime = video.duration - 0.5;
+        }
+    </script>
+</head>
+<body>
+    <video autoplay _onplaying_=handlePlaying() src="" _onended_=changeSource()></video>
+</body>

Added: trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/large-video-offscreen.html (0 => 205938)


--- trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/large-video-offscreen.html	                        (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/large-video-offscreen.html	2016-09-14 23:51:51 UTC (rev 205938)
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        video {
+            width: 480px;
+            height: 320px;
+            position: absolute;
+        }
+        .offscreen {
+            top: -320px;
+        }
+    </style>
+    <script>
+        function moveVideoOffscreen() {
+            document.querySelector("video").classList.add("offscreen");
+            setTimeout(function() {
+                try {
+                    window.webkit.messageHandlers.playingHandler.postMessage("moved");
+                } catch(e) {
+                }
+            }, 0);
+        }
+    </script>
+</head>
+<body>
+    <video autoplay _onplaying_=moveVideoOffscreen()><source src=""
+</body>
+<html>
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to