Diff
Modified: trunk/LayoutTests/ChangeLog (233559 => 233560)
--- trunk/LayoutTests/ChangeLog 2018-07-06 02:44:34 UTC (rev 233559)
+++ trunk/LayoutTests/ChangeLog 2018-07-06 03:20:20 UTC (rev 233560)
@@ -1,3 +1,16 @@
+2018-07-05 Ryosuke Niwa <[email protected]>
+
+ Youtube video pages crash after a couple of minutes
+ https://bugs.webkit.org/show_bug.cgi?id=187316
+
+ Reviewed by Antti Koivisto.
+
+ Added a regression test to remove an iframe with a video while there is a main content
+ which is eligible to be shown in the Touch Bar.
+
+ * media/remove-video-best-media-element-in-main-frame-crash-expected.txt: Added.
+ * media/remove-video-best-media-element-in-main-frame-crash.html: Added.
+
2018-07-05 Fujii Hironori <[email protected]>
REGRESSION(r233495) [cairo] drawGlyphsShadow should use the fast path for zero blur-radius
Added: trunk/LayoutTests/media/remove-video-best-media-element-in-main-frame-crash-expected.txt (0 => 233560)
--- trunk/LayoutTests/media/remove-video-best-media-element-in-main-frame-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/media/remove-video-best-media-element-in-main-frame-crash-expected.txt 2018-07-06 03:20:20 UTC (rev 233560)
@@ -0,0 +1,4 @@
+This tests removing a video element inside an iframe while another video in the main frame is playing does not hit a release assertion.
+
+PASS
+
Added: trunk/LayoutTests/media/remove-video-best-media-element-in-main-frame-crash.html (0 => 233560)
--- trunk/LayoutTests/media/remove-video-best-media-element-in-main-frame-crash.html (rev 0)
+++ trunk/LayoutTests/media/remove-video-best-media-element-in-main-frame-crash.html 2018-07-06 03:20:20 UTC (rev 233560)
@@ -0,0 +1,49 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p id="description">This tests removing a video element inside an iframe while another video in the main frame is playing does not hit a release assertion.</p>
+<div id="result"></div>
+<video id="video" autoplay width="500"></video>
+<script src=""
+<script>
+
+if (window.testRunner) {
+ testRunner.waitUntilDone();
+ testRunner.dumpAsText();
+}
+
+let count = 0;
+function continueTest() {
+ count++;
+ if (count < 2)
+ return;
+ setTimeout(() => {
+ iframe.remove();
+ result.textContent = 'PASS';
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, 0);
+}
+
+if (window.internals) {
+ testRunner.overridePreference("WebKitMainContentUserGestureOverrideEnabled", 1);
+ internals.setMediaElementRestrictions(video, "OverrideUserGestureRequirementForMainContent");
+} else
+ description.innerHTML += '<br>This test requries OverrideUserGestureRequirementForMainContent. Manually set this setting or use WebKitTestRunner.';
+
+const videoURL = findMediaFile("video", "content/test");
+const iframe = document.createElement('iframe');
+document.body.appendChild(iframe);
+video.src = ""
+video.addEventListener('loadeddata', continueTest);
+
+const doc = iframe.contentDocument;
+doc.open();
+doc.write(`<!DOCTYPE html><html><body>`);
+doc.write(`<video autoplay src=""
+doc.write(`<script>document.querySelector('video').addEventListener('loadeddata', () => top.continueTest());</` + `script>`);
+doc.close();
+
+</script>
+</body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (233559 => 233560)
--- trunk/Source/WebCore/ChangeLog 2018-07-06 02:44:34 UTC (rev 233559)
+++ trunk/Source/WebCore/ChangeLog 2018-07-06 03:20:20 UTC (rev 233560)
@@ -1,5 +1,48 @@
2018-07-05 Ryosuke Niwa <[email protected]>
+ Youtube video pages crash after a couple of minutes
+ https://bugs.webkit.org/show_bug.cgi?id=187316
+
+ Reviewed by Antti Koivisto.
+
+ The crash was caused by HTMLMediaElement::stopWithoutDestroyingMediaPlayer invoking updatePlaybackControlsManager,
+ which traverses all media players across different documents including the one in the main frame while its iframe
+ is getting removed (to update the Touch Bar's media control).
+
+ Fixed the bug by making this code async in both stopWithoutDestroyingMediaPlayer and ~HTMLMediaElement. To do this,
+ this patch moves the timer to update the playback controls manager from HTMLMediaElement to Page since scheduling
+ a timer owned by HTMLMediaElement in its destructor wouldn't work as the timer would get destructed immediately.
+
+ Also replaced the call to clientWillPausePlayback by a call to stopSession in stopWithoutDestroyingMediaPlayer
+ since the former also updates the layout synchronously via updateNowPlayingInfo; the latter function schedules
+ a timer via scheduleUpdateNowPlayingInfo instead.
+
+ Test: media/remove-video-best-media-element-in-main-frame-crash.html
+
+ * html/HTMLMediaElement.cpp:
+ (WebCore::HTMLMediaElement::~HTMLMediaElement): Call scheduleUpdatePlaybackControlsManager now that timer has been
+ moved to Page.
+ (WebCore::HTMLMediaElement::bestMediaElementForShowingPlaybackControlsManager): Made this return a RefPtr instead of
+ a raw pointer while we're at it.
+ (WebCore::HTMLMediaElement::clearMediaPlayer): Call scheduleUpdatePlaybackControlsManager.
+ (WebCore::HTMLMediaElement::stopWithoutDestroyingMediaPlayer): Ditto. Also invoke stopSession instead of
+ clientWillPausePlayback on MediaSession since clientWillPausePlayback will synchronously try to update the layout.
+ (WebCore::HTMLMediaElement::contextDestroyed):
+ (WebCore::HTMLMediaElement::stop):
+ (WebCore::HTMLMediaElement::schedulePlaybackControlsManagerUpdate): Renamed from scheduleUpdatePlaybackControlsManager.
+ (WebCore::HTMLMediaElement::updatePlaybackControlsManager): Moved to Page::playbackControlsManagerUpdateTimerFired.
+ * html/HTMLMediaElement.h:
+ * page/Page.cpp:
+ (WebCore::Page::Page):
+ (WebCore::Page::schedulePlaybackControlsManagerUpdate): Added.
+ (WebCore::Page::playbackControlsManagerUpdateTimerFired): Moved from HTMLMediaElement::updatePlaybackControlsManager.
+ * page/Page.h:
+ * testing/Internals.cpp:
+ (WebCore::Internals::bestMediaElementForShowingPlaybackControlsManager):
+ * testing/Internals.h:
+
+2018-07-05 Ryosuke Niwa <[email protected]>
+
REGRESSION(r233496): Crash in WebCore::VideoTrack::clearClient()
https://bugs.webkit.org/show_bug.cgi?id=187377
Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (233559 => 233560)
--- trunk/Source/WebCore/html/HTMLMediaElement.cpp 2018-07-06 02:44:34 UTC (rev 233559)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp 2018-07-06 03:20:20 UTC (rev 233560)
@@ -680,7 +680,6 @@
m_seekTaskQueue.close();
m_promiseTaskQueue.close();
m_pauseAfterDetachedTaskQueue.close();
- m_updatePlaybackControlsManagerQueue.close();
m_playbackControlsManagerBehaviorRestrictionsQueue.close();
m_resourceSelectionTaskQueue.close();
m_visibilityChangeTaskQueue.close();
@@ -696,7 +695,7 @@
}
m_mediaSession = nullptr;
- updatePlaybackControlsManager();
+ schedulePlaybackControlsManagerUpdate();
}
static bool needsAutoplayPlayPauseEventsQuirk(const Document& document)
@@ -709,7 +708,7 @@
return loader && loader->allowedAutoplayQuirks().contains(AutoplayQuirk::SynthesizedPauseEvents);
}
-HTMLMediaElement* HTMLMediaElement::bestMediaElementForShowingPlaybackControlsManager(MediaElementSession::PlaybackControlsPurpose purpose)
+RefPtr<HTMLMediaElement> HTMLMediaElement::bestMediaElementForShowingPlaybackControlsManager(MediaElementSession::PlaybackControlsPurpose purpose)
{
auto allSessions = PlatformMediaSessionManager::sharedManager().currentSessionsMatching([] (const PlatformMediaSession& session) {
return is<MediaElementSession>(session);
@@ -1140,7 +1139,7 @@
dispatchEvent(Event::create(eventNames().playingEvent, false, true));
resolvePendingPlayPromises(WTFMove(pendingPlayPromises));
- scheduleUpdatePlaybackControlsManager();
+ schedulePlaybackControlsManagerUpdate();
}
bool HTMLMediaElement::hasEverNotifiedAboutPlaying() const
@@ -3744,7 +3743,7 @@
m_mediaSession->canProduceAudioChanged();
}
- scheduleUpdatePlaybackControlsManager();
+ schedulePlaybackControlsManagerUpdate();
}
#if USE(AUDIO_SESSION) && PLATFORM(MAC)
@@ -4403,7 +4402,7 @@
if (!m_receivedLayoutSizeChanged) {
m_receivedLayoutSizeChanged = true;
- scheduleUpdatePlaybackControlsManager();
+ schedulePlaybackControlsManagerUpdate();
}
// If the video is a candidate for main content, we should register it for viewport visibility callbacks
@@ -5329,7 +5328,7 @@
}
if (shouldBePlaying) {
- scheduleUpdatePlaybackControlsManager();
+ schedulePlaybackControlsManagerUpdate();
setDisplayMode(Video);
invalidateCachedTime();
@@ -5358,7 +5357,7 @@
startPlaybackProgressTimer();
setPlaying(true);
} else {
- scheduleUpdatePlaybackControlsManager();
+ schedulePlaybackControlsManagerUpdate();
if (!playerPaused)
m_player->pause();
@@ -5522,7 +5521,7 @@
m_player->invalidate();
m_player = nullptr;
}
- updatePlaybackControlsManager();
+ schedulePlaybackControlsManagerUpdate();
stopPeriodicTimers();
m_pendingActionTimer.stop();
@@ -5562,13 +5561,13 @@
setPreparedToReturnVideoLayerToInline(true);
- updatePlaybackControlsManager();
+ schedulePlaybackControlsManagerUpdate();
setInActiveDocument(false);
// Stop the playback without generating events
setPlaying(false);
setPausedInternal(true);
- m_mediaSession->clientWillPausePlayback();
+ m_mediaSession->stopSession();
setPlaybackWithoutUserGesture(PlaybackWithoutUserGesture::None);
@@ -5587,7 +5586,6 @@
m_shadowDOMTaskQueue.close();
m_promiseTaskQueue.close();
m_pauseAfterDetachedTaskQueue.close();
- m_updatePlaybackControlsManagerQueue.close();
#if ENABLE(ENCRYPTED_MEDIA)
m_encryptedMediaQueue.close();
#endif
@@ -5606,7 +5604,6 @@
m_asyncEventQueue.close();
m_promiseTaskQueue.close();
- m_updatePlaybackControlsManagerQueue.close();
m_resourceSelectionTaskQueue.close();
// Once an active DOM object has been stopped it can not be restarted, so we can deallocate
@@ -6553,7 +6550,7 @@
#endif
m_player = MediaPlayer::create(*this);
m_player->setShouldBufferData(m_shouldBufferData);
- scheduleUpdatePlaybackControlsManager();
+ schedulePlaybackControlsManagerUpdate();
#if ENABLE(WEB_AUDIO)
if (m_audioSourceNode) {
@@ -7865,7 +7862,7 @@
m_visibilityChangeTaskQueue.enqueueTask([this] {
m_mediaSession->isVisibleInViewportChanged();
updateShouldAutoplay();
- scheduleUpdatePlaybackControlsManager();
+ schedulePlaybackControlsManagerUpdate();
});
}
@@ -7908,25 +7905,14 @@
return renderer && renderer->visibleInViewportState() == VisibleInViewportState::Yes;
}
-void HTMLMediaElement::updatePlaybackControlsManager()
+void HTMLMediaElement::schedulePlaybackControlsManagerUpdate()
{
Page* page = document().page();
if (!page)
return;
-
- // FIXME: Ensure that the renderer here should be up to date.
- if (auto bestMediaElement = bestMediaElementForShowingPlaybackControlsManager(MediaElementSession::PlaybackControlsPurpose::ControlsManager))
- page->chrome().client().setUpPlaybackControlsManager(*bestMediaElement);
- else
- page->chrome().client().clearPlaybackControlsManager();
+ page->schedulePlaybackControlsManagerUpdate();
}
-void HTMLMediaElement::scheduleUpdatePlaybackControlsManager()
-{
- if (!m_updatePlaybackControlsManagerQueue.hasPendingTasks())
- m_updatePlaybackControlsManagerQueue.enqueueTask(std::bind(&HTMLMediaElement::updatePlaybackControlsManager, this));
-}
-
void HTMLMediaElement::playbackControlsManagerBehaviorRestrictionsTimerFired()
{
if (m_playbackControlsManagerBehaviorRestrictionsQueue.hasPendingTasks())
@@ -7942,7 +7928,7 @@
return;
mediaElementSession->addBehaviorRestriction(MediaElementSession::RequirePlaybackToControlControlsManager);
- protectedThis->scheduleUpdatePlaybackControlsManager();
+ protectedThis->schedulePlaybackControlsManagerUpdate();
});
}
@@ -7963,7 +7949,7 @@
m_videoFullscreenMode = mode;
visibilityStateChanged();
- scheduleUpdatePlaybackControlsManager();
+ schedulePlaybackControlsManagerUpdate();
}
#if !RELEASE_LOG_DISABLED
Modified: trunk/Source/WebCore/html/HTMLMediaElement.h (233559 => 233560)
--- trunk/Source/WebCore/html/HTMLMediaElement.h 2018-07-06 02:44:34 UTC (rev 233559)
+++ trunk/Source/WebCore/html/HTMLMediaElement.h 2018-07-06 03:20:20 UTC (rev 233560)
@@ -155,7 +155,7 @@
static HashSet<HTMLMediaElement*>& allMediaElements();
- WEBCORE_EXPORT static HTMLMediaElement* bestMediaElementForShowingPlaybackControlsManager(MediaElementSession::PlaybackControlsPurpose);
+ WEBCORE_EXPORT static RefPtr<HTMLMediaElement> bestMediaElementForShowingPlaybackControlsManager(MediaElementSession::PlaybackControlsPurpose);
static bool isRunningDestructor();
@@ -898,7 +898,7 @@
void pauseAfterDetachedTask();
void updatePlaybackControlsManager();
- void scheduleUpdatePlaybackControlsManager();
+ void schedulePlaybackControlsManagerUpdate();
void playbackControlsManagerBehaviorRestrictionsTimerFired();
void updateRenderer();
@@ -933,7 +933,6 @@
GenericTaskQueue<Timer> m_shadowDOMTaskQueue;
GenericTaskQueue<Timer> m_promiseTaskQueue;
GenericTaskQueue<Timer> m_pauseAfterDetachedTaskQueue;
- GenericTaskQueue<Timer> m_updatePlaybackControlsManagerQueue;
GenericTaskQueue<Timer> m_playbackControlsManagerBehaviorRestrictionsQueue;
GenericTaskQueue<Timer> m_resourceSelectionTaskQueue;
GenericTaskQueue<Timer> m_visibilityChangeTaskQueue;
Modified: trunk/Source/WebCore/page/Page.cpp (233559 => 233560)
--- trunk/Source/WebCore/page/Page.cpp 2018-07-06 02:44:34 UTC (rev 233559)
+++ trunk/Source/WebCore/page/Page.cpp 2018-07-06 03:20:20 UTC (rev 233560)
@@ -239,6 +239,7 @@
, m_userContentProvider(*WTFMove(pageConfiguration.userContentProvider))
, m_visitedLinkStore(*WTFMove(pageConfiguration.visitedLinkStore))
, m_sessionID(PAL::SessionID::defaultSessionID())
+ , m_playbackControlsManagerUpdateTimer(*this, &Page::playbackControlsManagerUpdateTimerFired)
, m_isUtilityPage(isUtilityPageChromeClient(chrome().client()))
, m_performanceMonitor(isUtilityPage() ? nullptr : std::make_unique<PerformanceMonitor>(*this))
, m_lowPowerModeNotifier(std::make_unique<LowPowerModeNotifier>([this](bool isLowPowerModeEnabled) { handleLowModePowerChange(isLowPowerModeEnabled); }))
@@ -1487,6 +1488,20 @@
chrome().client().isPlayingMediaDidChange(state, sourceElementID);
}
+void Page::schedulePlaybackControlsManagerUpdate()
+{
+ if (!m_playbackControlsManagerUpdateTimer.isActive())
+ m_playbackControlsManagerUpdateTimer.startOneShot(0_s);
+}
+
+void Page::playbackControlsManagerUpdateTimerFired()
+{
+ if (auto bestMediaElement = HTMLMediaElement::bestMediaElementForShowingPlaybackControlsManager(MediaElementSession::PlaybackControlsPurpose::ControlsManager))
+ chrome().client().setUpPlaybackControlsManager(*bestMediaElement);
+ else
+ chrome().client().clearPlaybackControlsManager();
+}
+
void Page::setMuted(MediaProducer::MutedStateFlags muted)
{
if (m_mutedState == muted)
Modified: trunk/Source/WebCore/page/Page.h (233559 => 233560)
--- trunk/Source/WebCore/page/Page.h 2018-07-06 02:44:34 UTC (rev 233559)
+++ trunk/Source/WebCore/page/Page.h 2018-07-06 03:20:20 UTC (rev 233560)
@@ -569,6 +569,7 @@
MediaProducer::MutedStateFlags mutedState() const { return m_mutedState; }
bool isAudioMuted() const { return m_mutedState & MediaProducer::AudioIsMuted; }
bool isMediaCaptureMuted() const { return m_mutedState & MediaProducer::CaptureDevicesAreMuted; };
+ void schedulePlaybackControlsManagerUpdate();
WEBCORE_EXPORT void setMuted(MediaProducer::MutedStateFlags);
WEBCORE_EXPORT void stopMediaCapture();
@@ -674,6 +675,8 @@
std::optional<std::pair<MediaCanStartListener&, Document&>> takeAnyMediaCanStartListener();
+ void playbackControlsManagerUpdateTimerFired();
+
Vector<Ref<PluginViewBase>> pluginViews();
void handleLowModePowerChange(bool);
@@ -849,7 +852,9 @@
bool m_isRestoringCachedPage { false };
MediaProducer::MediaStateFlags m_mediaState { MediaProducer::IsNotPlaying };
-
+
+ Timer m_playbackControlsManagerUpdateTimer;
+
bool m_allowsMediaDocumentInlinePlayback { false };
bool m_allowsPlaybackControlsForAutoplayingAudio { false };
bool m_showAllPlugins { false };
Modified: trunk/Source/WebCore/testing/Internals.cpp (233559 => 233560)
--- trunk/Source/WebCore/testing/Internals.cpp 2018-07-06 02:44:34 UTC (rev 233559)
+++ trunk/Source/WebCore/testing/Internals.cpp 2018-07-06 03:20:20 UTC (rev 233560)
@@ -3750,7 +3750,7 @@
}
#if ENABLE(VIDEO)
-HTMLMediaElement* Internals::bestMediaElementForShowingPlaybackControlsManager(Internals::PlaybackControlsPurpose purpose)
+RefPtr<HTMLMediaElement> Internals::bestMediaElementForShowingPlaybackControlsManager(Internals::PlaybackControlsPurpose purpose)
{
return HTMLMediaElement::bestMediaElementForShowingPlaybackControlsManager(purpose);
}
Modified: trunk/Source/WebCore/testing/Internals.h (233559 => 233560)
--- trunk/Source/WebCore/testing/Internals.h 2018-07-06 02:44:34 UTC (rev 233559)
+++ trunk/Source/WebCore/testing/Internals.h 2018-07-06 03:20:20 UTC (rev 233560)
@@ -704,7 +704,7 @@
#if ENABLE(VIDEO)
using PlaybackControlsPurpose = MediaElementSession::PlaybackControlsPurpose;
- HTMLMediaElement* bestMediaElementForShowingPlaybackControlsManager(PlaybackControlsPurpose);
+ RefPtr<HTMLMediaElement> bestMediaElementForShowingPlaybackControlsManager(PlaybackControlsPurpose);
using MediaSessionState = PlatformMediaSession::State;
MediaSessionState mediaSessionState(HTMLMediaElement&);
