- Revision
- 229466
- Author
- [email protected]
- Date
- 2018-03-09 10:21:11 -0800 (Fri, 09 Mar 2018)
Log Message
webkitfullscreenchange event not fired at the same time as :-webkit-full-screen pseudo selector changes; causes glitchiness
https://bugs.webkit.org/show_bug.cgi?id=183383
Source/WebCore:
Reviewed by Eric Carlson.
Fire the webkitfullscreenchange event at the same time as the pseudo class selector changes, during the handling
of webkitDidEnterFullScreenForElement. For WebKit2 clients, this is guaranteed to be asynchronous, since the
calling method originates in the UIProcess. For WebKit1 clients (and WKTR and DRT), there's the possibility that
webkitWillEnterFullScreenForElement will be called synchronously from within
Document::requestFullScreenForElement(), so break that synchronousness by starting the
ChromeClient::enterFullScreenForElement(...) process in a async task.
Previously, the firing of the fullscreenchange event was done through a zero-length timer. Use a
GenericTaskQueue instead.
A number of layout tests depend on the behavior that the element will be in fullscreen when the 'playing' event
fires. This was true for DRT (but not WKTR), since its fullscreen implementations were deliberately synchronous, but
won't necessarily be true for all ports. Fix this in a subsequent patch.
* dom/Document.cpp:
(WebCore::Document::requestFullScreenForElement):
(WebCore::Document::webkitExitFullscreen):
(WebCore::Document::webkitWillEnterFullScreenForElement):
(WebCore::Document::webkitDidEnterFullScreenForElement):
(WebCore::Document::webkitDidExitFullScreenForElement):
(WebCore::Document::dispatchFullScreenChangeEvents):
* dom/Document.h:
* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::setReadyState):
(WebCore::HTMLMediaElement::playInternal):
(WebCore::HTMLMediaElement::mediaPlayerTimeChanged):
(WebCore::HTMLMediaElement::updatePlayState):
(WebCore::HTMLMediaElement::setPlaying):
LayoutTests:
Fix a couple tests that depended on non-standard behavior, and skip other tests to be fixed later.
Reviewed by Eric Carlson.
* media/fullscreen-video-going-into-pip.html:
* media/video-fullscreeen-only-playback.html:
* platform/mac/TestExpectations:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (229465 => 229466)
--- trunk/LayoutTests/ChangeLog 2018-03-09 18:02:49 UTC (rev 229465)
+++ trunk/LayoutTests/ChangeLog 2018-03-09 18:21:11 UTC (rev 229466)
@@ -1,3 +1,16 @@
+2018-03-09 Jer Noble <[email protected]>
+
+ webkitfullscreenchange event not fired at the same time as :-webkit-full-screen pseudo selector changes; causes glitchiness
+ https://bugs.webkit.org/show_bug.cgi?id=183383
+
+ Fix a couple tests that depended on non-standard behavior, and skip other tests to be fixed later.
+
+ Reviewed by Eric Carlson.
+
+ * media/fullscreen-video-going-into-pip.html:
+ * media/video-fullscreeen-only-playback.html:
+ * platform/mac/TestExpectations:
+
2018-03-09 Frederic Wang <[email protected]>
Unreviewed GTK+ gardening.
Modified: trunk/LayoutTests/media/fullscreen-video-going-into-pip.html (229465 => 229466)
--- trunk/LayoutTests/media/fullscreen-video-going-into-pip.html 2018-03-09 18:02:49 UTC (rev 229465)
+++ trunk/LayoutTests/media/fullscreen-video-going-into-pip.html 2018-03-09 18:21:11 UTC (rev 229466)
@@ -22,7 +22,7 @@
}
consoleWrite("Going into Full Screen");
- video.addEventListener('webkitfullscreenchange', onfullscreenchange);
+ video.addEventListener('webkitpresentationmodechanged', onfullscreenchange);
runWithKeyDown(function(){ video.webkitRequestFullscreen(); });
}
@@ -29,7 +29,7 @@
function onfullscreenchange()
{
testExpected("document.webkitCurrentFullScreenElement", video);
- video.removeEventListener('webkitfullscreenchange', onfullscreenchange);
+ video.removeEventListener('webkitpresentationmodechanged', onfullscreenchange);
consoleWrite("Going into Picture-in-Picture from Full Screen");
video.addEventListener('webkitpresentationmodechanged', onpresentationmodechanged);
Modified: trunk/LayoutTests/media/video-fullscreeen-only-playback.html (229465 => 229466)
--- trunk/LayoutTests/media/video-fullscreeen-only-playback.html 2018-03-09 18:02:49 UTC (rev 229465)
+++ trunk/LayoutTests/media/video-fullscreeen-only-playback.html 2018-03-09 18:21:11 UTC (rev 229466)
@@ -8,13 +8,11 @@
function fullscreenchange()
{
- if (document.webkitIsFullScreen)
- beginfullscreen();
- else
+ if (!document.webkitIsFullScreen)
endfullscreen();
}
- function beginfullscreen()
+ function playing()
{
consoleWrite("<br>** Entered fullscreen");
testExpected("video.webkitDisplayingFullscreen", true);
@@ -52,9 +50,8 @@
video = document.getElementsByTagName('video')[0];
waitForEvent("canplaythrough", canplaythrough);
- waitForEvent('playing');
+ waitForEvent('playing', playing);
- video.addEventListener('webkitbeginfullscreen', beginfullscreen, true);
video.addEventListener('webkitendfullscreen', endfullscreen, true);
video.addEventListener('webkitfullscreenchange', fullscreenchange, true);
Modified: trunk/LayoutTests/platform/mac/TestExpectations (229465 => 229466)
--- trunk/LayoutTests/platform/mac/TestExpectations 2018-03-09 18:02:49 UTC (rev 229465)
+++ trunk/LayoutTests/platform/mac/TestExpectations 2018-03-09 18:21:11 UTC (rev 229466)
@@ -1727,3 +1727,12 @@
webkit.org/b/172241 http/tests/appcache/404-resource-with-slow-main-resource.php [ Pass Failure ]
webkit.org/b/173946 [ Debug ] media/modern-media-controls/fullscreen-support/fullscreen-support-press.html [ Pass Failure ]
+
+webkit.org/b/183490 media/modern-media-controls/controls-visibility-support/controls-visibility-support-fullscreen-on-video.html [ Failure ]
+webkit.org/b/183490 media/modern-media-controls/media-controller/media-controller-fade-controls-when-entering-fullscreen.html [ Failure ]
+webkit.org/b/183490 media/modern-media-controls/media-controller/media-controller-fullscreen-change.html [ Failure ]
+webkit.org/b/183490 media/modern-media-controls/media-controller/media-controller-inline-to-fullscreen-to-inline.html [ Failure ]
+webkit.org/b/183490 media/modern-media-controls/start-support/start-support-fullscreen.html [ Failure ]
+webkit.org/b/183490 media/video-playsinline.html [ Failure ]
+webkit.org/b/183490 media/video-webkit-playsinline.html [ Failure ]
+webkit.org/b/183490 media/modern-media-controls/media-controller/media-controller-inline-to-fullscreen-to-pip-to-inline.html [ Failure ]
Modified: trunk/Source/WebCore/ChangeLog (229465 => 229466)
--- trunk/Source/WebCore/ChangeLog 2018-03-09 18:02:49 UTC (rev 229465)
+++ trunk/Source/WebCore/ChangeLog 2018-03-09 18:21:11 UTC (rev 229466)
@@ -1,3 +1,39 @@
+2018-03-09 Jer Noble <[email protected]>
+
+ webkitfullscreenchange event not fired at the same time as :-webkit-full-screen pseudo selector changes; causes glitchiness
+ https://bugs.webkit.org/show_bug.cgi?id=183383
+
+ Reviewed by Eric Carlson.
+
+ Fire the webkitfullscreenchange event at the same time as the pseudo class selector changes, during the handling
+ of webkitDidEnterFullScreenForElement. For WebKit2 clients, this is guaranteed to be asynchronous, since the
+ calling method originates in the UIProcess. For WebKit1 clients (and WKTR and DRT), there's the possibility that
+ webkitWillEnterFullScreenForElement will be called synchronously from within
+ Document::requestFullScreenForElement(), so break that synchronousness by starting the
+ ChromeClient::enterFullScreenForElement(...) process in a async task.
+
+ Previously, the firing of the fullscreenchange event was done through a zero-length timer. Use a
+ GenericTaskQueue instead.
+
+ A number of layout tests depend on the behavior that the element will be in fullscreen when the 'playing' event
+ fires. This was true for DRT (but not WKTR), since its fullscreen implementations were deliberately synchronous, but
+ won't necessarily be true for all ports. Fix this in a subsequent patch.
+
+ * dom/Document.cpp:
+ (WebCore::Document::requestFullScreenForElement):
+ (WebCore::Document::webkitExitFullscreen):
+ (WebCore::Document::webkitWillEnterFullScreenForElement):
+ (WebCore::Document::webkitDidEnterFullScreenForElement):
+ (WebCore::Document::webkitDidExitFullScreenForElement):
+ (WebCore::Document::dispatchFullScreenChangeEvents):
+ * dom/Document.h:
+ * html/HTMLMediaElement.cpp:
+ (WebCore::HTMLMediaElement::setReadyState):
+ (WebCore::HTMLMediaElement::playInternal):
+ (WebCore::HTMLMediaElement::mediaPlayerTimeChanged):
+ (WebCore::HTMLMediaElement::updatePlayState):
+ (WebCore::HTMLMediaElement::setPlaying):
+
2018-03-09 Zan Dobersek <[email protected]>
[Nicosia] Add threaded PaintingEngine implementation
Modified: trunk/Source/WebCore/dom/Document.cpp (229465 => 229466)
--- trunk/Source/WebCore/dom/Document.cpp 2018-03-09 18:02:49 UTC (rev 229465)
+++ trunk/Source/WebCore/dom/Document.cpp 2018-03-09 18:21:11 UTC (rev 229466)
@@ -495,9 +495,6 @@
, m_constantPropertyMap(std::make_unique<ConstantPropertyMap>(*this))
, m_documentClasses(documentClasses)
, m_eventQueue(*this)
-#if ENABLE(FULLSCREEN_API)
- , m_fullScreenChangeDelayTimer(*this, &Document::fullScreenChangeDelayTimerFired)
-#endif
, m_loadEventDelayTimer(*this, &Document::loadEventDelayTimerFired)
#if PLATFORM(IOS)
#if ENABLE(DEVICE_ORIENTATION)
@@ -6089,7 +6086,10 @@
// 5. Return, and run the remaining steps asynchronously.
// 6. Optionally, perform some animation.
m_areKeysEnabledInFullScreen = hasKeyboardAccess;
- page()->chrome().client().enterFullScreenForElement(*element);
+ m_fullScreenTaskQueue.enqueueTask([this, element = makeRefPtr(element)] {
+ if (auto page = this->page())
+ page->chrome().client().enterFullScreenForElement(*element);
+ });
// 7. Optionally, display a message indicating how the user can exit displaying the context object fullscreen.
return;
@@ -6096,7 +6096,9 @@
} while (0);
m_fullScreenErrorEventTargetQueue.append(element ? element : documentElement());
- m_fullScreenChangeDelayTimer.startOneShot(0_s);
+ m_fullScreenTaskQueue.enqueueTask([this] {
+ dispatchFullScreenChangeEvents();
+ });
}
void Document::webkitCancelFullScreen()
@@ -6174,19 +6176,21 @@
// 6. Return, and run the remaining steps asynchronously.
// 7. Optionally, perform some animation.
+ m_fullScreenTaskQueue.enqueueTask([this, newTop = makeRefPtr(newTop), fullScreenElement = m_fullScreenElement] {
+ auto* page = this->page();
+ if (!page)
+ return;
- if (!page())
- return;
+ // Only exit out of full screen window mode if there are no remaining elements in the
+ // full screen stack.
+ if (!newTop) {
+ page->chrome().client().exitFullScreenForElement(fullScreenElement.get());
+ return;
+ }
- // Only exit out of full screen window mode if there are no remaining elements in the
- // full screen stack.
- if (!newTop) {
- page()->chrome().client().exitFullScreenForElement(m_fullScreenElement.get());
- return;
- }
-
- // Otherwise, notify the chrome of the new full screen element.
- page()->chrome().client().enterFullScreenForElement(*newTop);
+ // Otherwise, notify the chrome of the new full screen element.
+ page->chrome().client().enterFullScreenForElement(*newTop);
+ });
}
bool Document::webkitFullscreenEnabled() const
@@ -6251,9 +6255,7 @@
m_fullScreenElement->setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(true);
resolveStyle(ResolveStyleType::Rebuild);
-#if PLATFORM(IOS) && ENABLE(FULLSCREEN_API)
- m_fullScreenChangeDelayTimer.startOneShot(0_s);
-#endif
+ dispatchFullScreenChangeEvents();
}
void Document::webkitDidEnterFullScreenForElement(Element*)
@@ -6265,10 +6267,6 @@
return;
m_fullScreenElement->didBecomeFullscreenElement();
-
-#if !PLATFORM(IOS) || !ENABLE(FULLSCREEN_API)
- m_fullScreenChangeDelayTimer.startOneShot(0_s);
-#endif
}
void Document::webkitWillExitFullScreenForElement(Element*)
@@ -6305,7 +6303,7 @@
bool eventTargetQueuesEmpty = m_fullScreenChangeEventTargetQueue.isEmpty() && m_fullScreenErrorEventTargetQueue.isEmpty();
Document& exitingDocument = eventTargetQueuesEmpty ? topDocument() : *this;
- exitingDocument.m_fullScreenChangeDelayTimer.startOneShot(0_s);
+ exitingDocument.dispatchFullScreenChangeEvents();
}
void Document::setFullScreenRenderer(RenderTreeBuilder& builder, RenderFullScreen& renderer)
@@ -6327,7 +6325,7 @@
m_fullScreenRenderer = makeWeakPtr(renderer);
}
-void Document::fullScreenChangeDelayTimerFired()
+void Document::dispatchFullScreenChangeEvents()
{
// Since we dispatch events in this function, it's possible that the
// document will be detached and GC'd. We protect it here to make sure we
Modified: trunk/Source/WebCore/dom/Document.h (229465 => 229466)
--- trunk/Source/WebCore/dom/Document.h 2018-03-09 18:02:49 UTC (rev 229465)
+++ trunk/Source/WebCore/dom/Document.h 2018-03-09 18:21:11 UTC (rev 229466)
@@ -35,6 +35,7 @@
#include "FocusDirection.h"
#include "FontSelectorClient.h"
#include "FrameDestructionObserver.h"
+#include "GenericTaskQueue.h"
#include "MediaProducer.h"
#include "MutationObserver.h"
#include "OrientationNotifier.h"
@@ -1136,7 +1137,7 @@
void setFullScreenRenderer(RenderTreeBuilder&, RenderFullScreen&);
RenderFullScreen* fullScreenRenderer() const { return m_fullScreenRenderer.get(); }
- void fullScreenChangeDelayTimerFired();
+ void dispatchFullScreenChangeEvents();
bool fullScreenIsAllowedForElement(Element*) const;
void fullScreenElementRemoved();
void removeFullScreenElementOfSubtree(Node&, bool amongChildrenOnly = false);
@@ -1683,7 +1684,7 @@
RefPtr<Element> m_fullScreenElement;
Vector<RefPtr<Element>> m_fullScreenElementStack;
WeakPtr<RenderFullScreen> m_fullScreenRenderer { nullptr };
- Timer m_fullScreenChangeDelayTimer;
+ GenericTaskQueue<Timer> m_fullScreenTaskQueue;
Deque<RefPtr<Node>> m_fullScreenChangeEventTargetQueue;
Deque<RefPtr<Node>> m_fullScreenErrorEventTargetQueue;
LayoutRect m_savedPlaceholderFrameRect;
Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (229465 => 229466)
--- trunk/Source/WebCore/html/HTMLMediaElement.cpp 2018-03-09 18:02:49 UTC (rev 229465)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp 2018-03-09 18:21:11 UTC (rev 229466)
@@ -2515,11 +2515,8 @@
setShouldDelayLoadEvent(false);
}
- bool isPotentiallyPlaying = potentiallyPlaying();
if (m_readyState == HAVE_FUTURE_DATA && oldState <= HAVE_CURRENT_DATA && tracksAreReady) {
scheduleEvent(eventNames().canplayEvent);
- if (isPotentiallyPlaying)
- scheduleNotifyAboutPlaying();
shouldUpdateDisplayState = true;
}
@@ -2529,9 +2526,6 @@
scheduleEvent(eventNames().canplaythroughEvent);
- if (isPotentiallyPlaying && oldState <= HAVE_CURRENT_DATA)
- scheduleNotifyAboutPlaying();
-
auto success = canTransitionFromAutoplayToPlay();
if (success) {
m_paused = false;
@@ -2539,7 +2533,6 @@
setPlaybackWithoutUserGesture(PlaybackWithoutUserGesture::Started);
m_playbackStartedTime = currentMediaTime().toDouble();
scheduleEvent(eventNames().playEvent);
- scheduleNotifyAboutPlaying();
} else if (success.value() == MediaPlaybackDenialReason::UserGestureRequired) {
ALWAYS_LOG(LOGIDENTIFIER, "Autoplay blocked, user gesture required");
setPlaybackWithoutUserGesture(PlaybackWithoutUserGesture::Prevented);
@@ -3497,8 +3490,6 @@
#endif
if (m_readyState <= HAVE_CURRENT_DATA)
scheduleEvent(eventNames().waitingEvent);
- else if (m_readyState >= HAVE_FUTURE_DATA)
- scheduleNotifyAboutPlaying();
} else if (m_readyState >= HAVE_FUTURE_DATA)
scheduleResolvePendingPlayPromises();
@@ -4713,9 +4704,9 @@
scheduleEvent(eventNames().endedEvent);
if (!wasSeeking)
addBehaviorRestrictionsOnEndIfNecessary();
-
setPlaybackWithoutUserGesture(PlaybackWithoutUserGesture::None);
}
+ setPlaying(false);
// If the media element has a current media controller, then report the controller state
// for the media element's current media controller.
updateMediaController();
@@ -5329,6 +5320,9 @@
m_playing = playing;
+ if (m_playing)
+ scheduleNotifyAboutPlaying();
+
#if ENABLE(MEDIA_SESSION)
document().updateIsPlayingMedia(m_elementID);
#else
