Media controls behave strangely when changing media sources
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Sep 2016 23:51:51 +0000 (23:51 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Sep 2016 23:51:51 +0000 (23:51 +0000)
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.

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

17 files changed:
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLMediaElement.cpp
Source/WebCore/html/HTMLMediaElement.h
Source/WebCore/html/MediaElementSession.cpp
Source/WebCore/platform/graphics/MediaPlayer.h
Source/WebCore/platform/graphics/MediaPlayerPrivate.h
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm
Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.mm
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/WebPageProxy.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
Tools/TestWebKitAPI/Tests/WebKit2Cocoa/VideoControlsManager.mm
Tools/TestWebKitAPI/Tests/WebKit2Cocoa/change-video-source-on-click.html [new file with mode: 0644]
Tools/TestWebKitAPI/Tests/WebKit2Cocoa/change-video-source-on-end.html [new file with mode: 0644]
Tools/TestWebKitAPI/Tests/WebKit2Cocoa/large-video-offscreen.html [new file with mode: 0644]

index f28f743..e5b62dd 100644 (file)
@@ -1,3 +1,59 @@
+2016-09-14  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        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  <eric.carlson@apple.com>
 
         [MediaStream] Minor cleanup
index 7804c3b..6b2d25b 100644 (file)
@@ -163,6 +163,8 @@ static const double SeekTime = 0.2;
 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 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tagName, Document& docum
     , 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 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tagName, Document& docum
     , 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 @@ HTMLMediaElement::~HTMLMediaElement()
     m_promiseTaskQueue.close();
     m_pauseAfterDetachedTaskQueue.close();
     m_updatePlaybackControlsManagerQueue.close();
+    m_playbackControlsManagerBehaviorRestrictionsQueue.close();
 
     m_completelyLoaded = true;
 
@@ -972,6 +978,12 @@ void HTMLMediaElement::scheduleNextSourceChild()
     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 @@ void HTMLMediaElement::seekWithTolerance(const MediaTime& inTime, const MediaTim
         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 @@ void HTMLMediaElement::beginScrubbing()
             setPausedInternal(true);
         }
     }
+
+    m_mediaSession->removeBehaviorRestriction(MediaElementSession::RequireUserGestureToControlControlsManager);
 }
 
 void HTMLMediaElement::endScrubbing()
@@ -4537,7 +4554,9 @@ void HTMLMediaElement::addBehaviorRestrictionsOnEndIfNecessary()
     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 @@ void HTMLMediaElement::mediaPlayerCharacteristicChanged(MediaPlayer*)
     document().updateIsPlayingMedia();
 #endif
 
+    m_hasEverHadAudio |= hasAudio();
+    m_hasEverHadVideo |= hasVideo();
+
     endProcessingMediaPlayerCallback();
 }
 
@@ -7301,6 +7323,25 @@ void HTMLMediaElement::scheduleUpdatePlaybackControlsManager()
         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)
index 96c070d..a2d63cc 100644 (file)
@@ -473,6 +473,9 @@ public:
     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 @@ private:
     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 @@ private:
     void pauseAfterDetachedTask();
     void updatePlaybackControlsManager();
     void scheduleUpdatePlaybackControlsManager();
+    void playbackControlsManagerBehaviorRestrictionsTimerFired();
 
     void updateRenderer();
 
@@ -804,12 +810,14 @@ private:
     Timer m_progressEventTimer;
     Timer m_playbackProgressTimer;
     Timer m_scanTimer;
+    Timer m_playbackControlsManagerBehaviorRestrictionsTimer;
     GenericTaskQueue<Timer> m_seekTaskQueue;
     GenericTaskQueue<Timer> m_resizeTaskQueue;
     GenericTaskQueue<Timer> m_shadowDOMTaskQueue;
     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 @@ private:
     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;
index 9d8ca2d..50fb7e1 100644 (file)
@@ -59,6 +59,7 @@ namespace WebCore {
 
 static const double elementMainContentCheckInterval = .250;
 
+static bool isElementRectMostlyInMainFrame(const HTMLMediaElement&);
 static bool isElementLargeEnoughForMainContent(const HTMLMediaElement&, MediaSessionMainContentPurpose);
 
 #if !LOG_DISABLED
@@ -222,7 +223,12 @@ bool MediaElementSession::canShowControlsManager() const
         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 @@ bool MediaElementSession::canShowControlsManager() const
             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 @@ static bool isMainContentForPurposesOfAutoplay(const HTMLMediaElement& element)
     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;
index 76e4deb..e289cfe 100644 (file)
@@ -196,6 +196,8 @@ public:
     // 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
index 137cefb..ad387a7 100644 (file)
@@ -275,6 +275,8 @@ public:
 #if ENABLE(AVF_CAPTIONS)
     virtual void notifyTrackModeChanged() { }
 #endif
+
+    virtual void notifyActiveSourceBuffersChanged() { }
 };
 
 }
index f71fe48..13715b5 100644 (file)
@@ -165,6 +165,7 @@ private:
     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;
 
index e3b3cbe..9f14e2d 100644 (file)
@@ -556,6 +556,11 @@ void MediaPlayerPrivateMediaSourceAVFObjC::acceleratedRenderingStateChanged()
         destroyLayer();
 }
 
+void MediaPlayerPrivateMediaSourceAVFObjC::notifyActiveSourceBuffersChanged()
+{
+    m_player->client().mediaPlayerActiveSourceBuffersChanged(m_player);
+}
+
 MediaPlayer::MovieLoadType MediaPlayerPrivateMediaSourceAVFObjC::movieLoadType() const
 {
     return MediaPlayer::StoredStream;
index ce25dcb..525bb90 100644 (file)
@@ -83,8 +83,10 @@ void MediaSourcePrivateAVFObjC::removeSourceBuffer(SourceBufferPrivate* buffer)
     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::seekCompleted()
 
 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();
+        }
     }
 }
 
index c5fcb49..eb138d5 100644 (file)
@@ -1,3 +1,18 @@
+2016-09-14  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        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  <bdakin@apple.com>
 
         Add needsPlainTextQuirk and send it to the UIProcess
index 5463fbf..02c2c22 100644 (file)
@@ -6329,7 +6329,7 @@ void WebPageProxy::videoControlsManagerDidChange()
 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
index e1e1013..ff8b753 100644 (file)
@@ -1,3 +1,23 @@
+2016-09-14  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        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  <jbedard@apple.com>
 
         Fix mastercfg_unittest
index dd46478..8a49c3b 100644 (file)
@@ -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 */; };
                        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 */,
                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>"; };
                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 */,
index d07d2e4..bd31862 100644 (file)
     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 @@ TEST(VideoControlsManager, VideoControlsManagerLargeAutoplayingVideoSeeksAfterEn
 {
     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 @@ TEST(VideoControlsManager, VideoControlsManagerTearsDownMediaControlsOnDealloc)
     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));
diff --git a/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/change-video-source-on-click.html b/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/change-video-source-on-click.html
new file mode 100644 (file)
index 0000000..c8c1d28
--- /dev/null
@@ -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 = "large-video-with-audio.mp4";
+            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="large-video-with-audio.mp4"></video>
+</body>
diff --git a/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/change-video-source-on-end.html b/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/change-video-source-on-end.html
new file mode 100644 (file)
index 0000000..8fdcbda
--- /dev/null
@@ -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 = "large-video-with-audio.mp4";
+            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="large-video-with-audio.mp4" onended=changeSource()></video>
+</body>
diff --git a/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/large-video-offscreen.html b/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/large-video-offscreen.html
new file mode 100644 (file)
index 0000000..50a685f
--- /dev/null
@@ -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="large-video-with-audio.mp4"></video>
+</body>
+<html>