Changeset 250935 in webkit

Timestamp:

Oct 9, 2019 3:03:29 PM (5 years ago)

Author:

Chris Dumez

Message:

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/media/media-source/media-source-page-cache-expected.txt (added)
• LayoutTests/media/media-source/media-source-page-cache.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/Modules/mediasource/MediaSource.cpp (modified) (10 diffs)
• Source/WebCore/Modules/mediasource/SourceBuffer.cpp (modified) (1 diff)
• Source/WebCore/Modules/mediasource/SourceBuffer.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/Modules/mediasource/MediaSource.cpp

@@ -737 +737 @@
     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.
@@ -747 +747 @@
 
             // 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.
@@ -767 +767 @@
                 // 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);
             }
 
@@ -777 +777 @@
 
         // 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.
@@ -787 +787 @@
 
             // 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.
@@ -807 +807 @@
                 // 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);
             }
 
@@ -817 +817 @@
 
         // 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.
@@ -827 +827 @@
 
             // 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.
@@ -847 +847 @@
                 // 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);
             }
 
@@ -990 +990 @@
 bool MediaSource::canSuspendForDocumentSuspension() const
 {
-    // FIXME: Do better.
-    return isClosed();
+    return true;
 }
 

trunk/Source/WebCore/Modules/mediasource/SourceBuffer.cpp

@@ -541 +541 @@
 bool SourceBuffer::canSuspendForDocumentSuspension() const
 {
-    return !hasPendingActivity();
+    return true;
 }
 

trunk/Source/WebCore/Modules/mediasource/SourceBuffer.h

@@ -80 +80 @@
 #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