Youtube.com is unable to enter the back/forward cache on macOS
https://bugs.webkit.org/show_bug.cgi?id=202754
<rdar://problem/56117666>
Reviewed by Eric Carlson.
Source/WebCore:
As of r250542, the MainThreadGenericEventQueue used by both MediaSource and
SourceBuffer to fire event is PageCache-aware. As a result, both these
ActiveDOMObjects can now safety suspend without risking running script while
in the page cache. I did have to update some logic in MediaSource::removeSourceBuffer()
to make sure we do not unnecessarily construct new ActiveDOMObjects while
suspending, as this is not allowed.
Test: media/media-source/media-source-page-cache.html
* Modules/mediasource/MediaSource.cpp:
(WebCore::MediaSource::removeSourceBuffer):
(WebCore::MediaSource::canSuspendForDocumentSuspension const):
* Modules/mediasource/SourceBuffer.cpp:
(WebCore::SourceBuffer::canSuspendForDocumentSuspension const):
* Modules/mediasource/SourceBuffer.h:
LayoutTests:
Add layout test coverage.
* media/media-source/media-source-page-cache-expected.txt: Added.
* media/media-source/media-source-page-cache.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@250935 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index d4b37e5..24e55e2 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2019-10-09 Chris Dumez <cdumez@apple.com>
+
+ Youtube.com is unable to enter the back/forward cache on macOS
+ https://bugs.webkit.org/show_bug.cgi?id=202754
+ <rdar://problem/56117666>
+
+ Reviewed by Eric Carlson.
+
+ Add layout test coverage.
+
+ * media/media-source/media-source-page-cache-expected.txt: Added.
+ * media/media-source/media-source-page-cache.html: Added.
+
2019-10-09 Truitt Savell <tsavell@apple.com>
Mark crypto/workers/subtle/aes-indexeddb.html as a timeout on Mojave Release wk2
diff --git a/LayoutTests/media/media-source/media-source-page-cache-expected.txt b/LayoutTests/media/media-source/media-source-page-cache-expected.txt
new file mode 100644
index 0000000..8bdcc97
--- /dev/null
+++ b/LayoutTests/media/media-source/media-source-page-cache-expected.txt
@@ -0,0 +1,11 @@
+
+RUN(video.src = URL.createObjectURL(source))
+pageshow - not from cache
+EVENT(sourceopen)
+RUN(sourceBuffer = source.addSourceBuffer("video/mock; codecs=mock"))
+EXPECTED (source.sourceBuffers[0] == '[object SourceBuffer]') OK
+pagehide - entering cache
+pageshow - from cache
+PASS: Page was restored from Page Cache
+END OF TEST
+
diff --git a/LayoutTests/media/media-source/media-source-page-cache.html b/LayoutTests/media/media-source/media-source-page-cache.html
new file mode 100644
index 0000000..85cc316
--- /dev/null
+++ b/LayoutTests/media/media-source/media-source-page-cache.html
@@ -0,0 +1,51 @@
+<!-- webkit-test-runner [ enablePageCache=true ] -->
+<!DOCTYPE html>
+<html>
+<head>
+ <title>mock-media-source</title>
+ <script src="mock-media-source.js"></script>
+ <script src="../video-test.js"></script>
+ <script>
+ var source;
+ var sourceBuffer;
+
+ if (window.internals)
+ internals.initializeMockMediaSource();
+
+ window.addEventListener("pageshow", function(event) {
+ consoleWrite("pageshow - " + (event.persisted ? "" : "not ") + "from cache");
+ if (event.persisted) {
+ consoleWrite("PASS: Page was restored from Page Cache");
+ endTest();
+ }
+ });
+
+ window.addEventListener("pagehide", function(event) {
+ consoleWrite("pagehide - " + (event.persisted ? "" : "not ") + "entering cache");
+ if (!event.persisted) {
+ consoleWrite("FAIL: Page failed to enter the Page Cache");
+ endTest();
+ }
+ });
+
+ function runTest() {
+ findMediaElement();
+
+ source = new MediaSource();
+ waitForEvent('sourceopen', sourceOpen, false, false, source);
+ run('video.src = URL.createObjectURL(source)');
+ }
+
+ function sourceOpen() {
+ run('sourceBuffer = source.addSourceBuffer("video/mock; codecs=mock")');
+ testExpected('source.sourceBuffers[0]', sourceBuffer);
+ testLink.click();
+ }
+
+ </script>
+</head>
+<body onload="runTest()">
+ <video></video>
+ <a id="testLink" href="../../fast/history/resources/page-cache-helper.html" style="display: none">Link</a>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 1a1ca3e..a5de855 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,27 @@
+2019-10-09 Chris Dumez <cdumez@apple.com>
+
+ Youtube.com is unable to enter the back/forward cache on macOS
+ https://bugs.webkit.org/show_bug.cgi?id=202754
+ <rdar://problem/56117666>
+
+ Reviewed by Eric Carlson.
+
+ As of r250542, the MainThreadGenericEventQueue used by both MediaSource and
+ SourceBuffer to fire event is PageCache-aware. As a result, both these
+ ActiveDOMObjects can now safety suspend without risking running script while
+ in the page cache. I did have to update some logic in MediaSource::removeSourceBuffer()
+ to make sure we do not unnecessarily construct new ActiveDOMObjects while
+ suspending, as this is not allowed.
+
+ Test: media/media-source/media-source-page-cache.html
+
+ * Modules/mediasource/MediaSource.cpp:
+ (WebCore::MediaSource::removeSourceBuffer):
+ (WebCore::MediaSource::canSuspendForDocumentSuspension const):
+ * Modules/mediasource/SourceBuffer.cpp:
+ (WebCore::SourceBuffer::canSuspendForDocumentSuspension const):
+ * Modules/mediasource/SourceBuffer.h:
+
2019-10-09 Daniel Bates <dabates@apple.com>
Add support for CompactPointerTuple<..., OptionSet<...>>
diff --git a/Source/WebCore/Modules/mediasource/MediaSource.cpp b/Source/WebCore/Modules/mediasource/MediaSource.cpp
index b948927..8edd77c 100644
--- a/Source/WebCore/Modules/mediasource/MediaSource.cpp
+++ b/Source/WebCore/Modules/mediasource/MediaSource.cpp
@@ -736,18 +736,18 @@
ASSERT(scriptExecutionContext());
if (!scriptExecutionContext()->activeDOMObjectsAreStopped()) {
// 4. Let SourceBuffer audioTracks list equal the AudioTrackList object returned by sourceBuffer.audioTracks.
- auto& audioTracks = buffer.audioTracks();
+ auto* audioTracks = buffer.audioTracksIfExists();
// 5. If the SourceBuffer audioTracks list is not empty, then run the following steps:
- if (audioTracks.length()) {
+ if (audioTracks && audioTracks->length()) {
// 5.1 Let HTMLMediaElement audioTracks list equal the AudioTrackList object returned by the audioTracks
// attribute on the HTMLMediaElement.
// 5.2 Let the removed enabled audio track flag equal false.
bool removedEnabledAudioTrack = false;
// 5.3 For each AudioTrack object in the SourceBuffer audioTracks list, run the following steps:
- while (audioTracks.length()) {
- auto& track = *audioTracks.lastItem();
+ while (audioTracks->length()) {
+ auto& track = *audioTracks->lastItem();
// 5.3.1 Set the sourceBuffer attribute on the AudioTrack object to null.
track.setSourceBuffer(nullptr);
@@ -766,7 +766,7 @@
// 5.3.5 Remove the AudioTrack object from the SourceBuffer audioTracks list.
// 5.3.6 Queue a task to fire a trusted event named removetrack, that does not bubble and is not
// cancelable, and that uses the TrackEvent interface, at the SourceBuffer audioTracks list.
- audioTracks.remove(track);
+ audioTracks->remove(track);
}
// 5.4 If the removed enabled audio track flag equals true, then queue a task to fire a simple event
@@ -776,18 +776,18 @@
}
// 6. Let SourceBuffer videoTracks list equal the VideoTrackList object returned by sourceBuffer.videoTracks.
- auto& videoTracks = buffer.videoTracks();
+ auto* videoTracks = buffer.videoTracksIfExists();
// 7. If the SourceBuffer videoTracks list is not empty, then run the following steps:
- if (videoTracks.length()) {
+ if (videoTracks && videoTracks->length()) {
// 7.1 Let HTMLMediaElement videoTracks list equal the VideoTrackList object returned by the videoTracks
// attribute on the HTMLMediaElement.
// 7.2 Let the removed selected video track flag equal false.
bool removedSelectedVideoTrack = false;
// 7.3 For each VideoTrack object in the SourceBuffer videoTracks list, run the following steps:
- while (videoTracks.length()) {
- auto& track = *videoTracks.lastItem();
+ while (videoTracks->length()) {
+ auto& track = *videoTracks->lastItem();
// 7.3.1 Set the sourceBuffer attribute on the VideoTrack object to null.
track.setSourceBuffer(nullptr);
@@ -806,7 +806,7 @@
// 7.3.5 Remove the VideoTrack object from the SourceBuffer videoTracks list.
// 7.3.6 Queue a task to fire a trusted event named removetrack, that does not bubble and is not
// cancelable, and that uses the TrackEvent interface, at the SourceBuffer videoTracks list.
- videoTracks.remove(track);
+ videoTracks->remove(track);
}
// 7.4 If the removed selected video track flag equals true, then queue a task to fire a simple event
@@ -816,18 +816,18 @@
}
// 8. Let SourceBuffer textTracks list equal the TextTrackList object returned by sourceBuffer.textTracks.
- auto& textTracks = buffer.textTracks();
+ auto* textTracks = buffer.textTracksIfExists();
// 9. If the SourceBuffer textTracks list is not empty, then run the following steps:
- if (textTracks.length()) {
+ if (textTracks && textTracks->length()) {
// 9.1 Let HTMLMediaElement textTracks list equal the TextTrackList object returned by the textTracks
// attribute on the HTMLMediaElement.
// 9.2 Let the removed enabled text track flag equal false.
bool removedEnabledTextTrack = false;
// 9.3 For each TextTrack object in the SourceBuffer textTracks list, run the following steps:
- while (textTracks.length()) {
- auto& track = *textTracks.lastItem();
+ while (textTracks->length()) {
+ auto& track = *textTracks->lastItem();
// 9.3.1 Set the sourceBuffer attribute on the TextTrack object to null.
track.setSourceBuffer(nullptr);
@@ -846,7 +846,7 @@
// 9.3.5 Remove the TextTrack object from the SourceBuffer textTracks list.
// 9.3.6 Queue a task to fire a trusted event named removetrack, that does not bubble and is not
// cancelable, and that uses the TrackEvent interface, at the SourceBuffer textTracks list.
- textTracks.remove(track);
+ textTracks->remove(track);
}
// 9.4 If the removed enabled text track flag equals true, then queue a task to fire a simple event
@@ -989,8 +989,7 @@
bool MediaSource::canSuspendForDocumentSuspension() const
{
- // FIXME: Do better.
- return isClosed();
+ return true;
}
const char* MediaSource::activeDOMObjectName() const
diff --git a/Source/WebCore/Modules/mediasource/SourceBuffer.cpp b/Source/WebCore/Modules/mediasource/SourceBuffer.cpp
index d67a662..03368ee 100644
--- a/Source/WebCore/Modules/mediasource/SourceBuffer.cpp
+++ b/Source/WebCore/Modules/mediasource/SourceBuffer.cpp
@@ -540,7 +540,7 @@
bool SourceBuffer::canSuspendForDocumentSuspension() const
{
- return !hasPendingActivity();
+ return true;
}
const char* SourceBuffer::activeDOMObjectName() const
diff --git a/Source/WebCore/Modules/mediasource/SourceBuffer.h b/Source/WebCore/Modules/mediasource/SourceBuffer.h
index dd5daf6..7d95122 100644
--- a/Source/WebCore/Modules/mediasource/SourceBuffer.h
+++ b/Source/WebCore/Modules/mediasource/SourceBuffer.h
@@ -79,8 +79,11 @@
#if ENABLE(VIDEO_TRACK)
VideoTrackList& videoTracks();
+ VideoTrackList* videoTracksIfExists() const { return m_videoTracks.get(); }
AudioTrackList& audioTracks();
+ AudioTrackList* audioTracksIfExists() const { return m_audioTracks.get(); }
TextTrackList& textTracks();
+ TextTrackList* textTracksIfExists() const { return m_textTracks.get(); }
#endif
double appendWindowStart() const;
