Attachment 346060 Details for Bug 187284 – Patch

[patch] Patch

bug-187284-20180730084853.patch (text/plain), 39.24 KB, created by Chris Dumez on 2018-07-30 08:48:54 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-07-30 08:48:54 PDT

Size: 39.24 KB

patch

obsolete

>Subversion Revision: 234365
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index b60e0512ab197ebe2d030e98f76863b9981f514b..35f71ec8fa68f3e5a8f651d6a8fa92cba417977f 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,68 @@
>+2018-07-30  Chris Dumez  <cdumez@apple.com>
>+
>+        Nullptr crash accessing Document in GenericEventQueue::dispatchOneEvent()
>+        https://bugs.webkit.org/show_bug.cgi?id=187284
>+
>+        Unreviewed, roll r233496 and r233571 now that all known issues have been fixed via
>+        dependency bugs.
>+
>+        Test: media/track/video-track-addition-and-frame-removal.html
>+
>+        * Modules/encryptedmedia/legacy/WebKitMediaKeySession.cpp:
>+        (WebCore::WebKitMediaKeySession::suspend):
>+        (WebCore::WebKitMediaKeySession::resume):
>+        (WebCore::WebKitMediaKeySession::stop):
>+        * Modules/encryptedmedia/legacy/WebKitMediaKeySession.h:
>+        * Modules/mediasource/MediaSource.cpp:
>+        (WebCore::MediaSource::removeSourceBuffer):
>+        (WebCore::MediaSource::suspend):
>+        (WebCore::MediaSource::resume):
>+        * Modules/mediasource/MediaSource.h:
>+        * Modules/mediasource/SourceBuffer.cpp:
>+        (WebCore::SourceBuffer::suspend):
>+        (WebCore::SourceBuffer::resume):
>+        (WebCore::SourceBuffer::stop):
>+        * Modules/mediasource/SourceBuffer.h:
>+        * Modules/mediasource/SourceBufferList.cpp:
>+        (WebCore::SourceBufferList::SourceBufferList):
>+        (WebCore::SourceBufferList::canSuspendForDocumentSuspension const):
>+        (WebCore::SourceBufferList::suspend):
>+        (WebCore::SourceBufferList::resume):
>+        (WebCore::SourceBufferList::stop):
>+        (WebCore::SourceBufferList::activeDOMObjectName const):
>+        * Modules/mediasource/SourceBufferList.h:
>+        * Modules/mediasource/SourceBufferList.idl:
>+        * dom/Document.h:
>+        (WebCore::Document::hasBrowsingContext const):
>+        * dom/GenericEventQueue.cpp:
>+        (WebCore::GenericEventQueue::dispatchOneEvent):
>+        * html/HTMLMediaElement.cpp:
>+        (WebCore::HTMLMediaElement::prepareForLoad):
>+        (WebCore::HTMLMediaElement::selectMediaResource):
>+        (WebCore::HTMLMediaElement::prepareToPlay):
>+        (WebCore::HTMLMediaElement::playInternal):
>+        (WebCore::HTMLMediaElement::pauseInternal):
>+        (WebCore::HTMLMediaElement::sourceWasAdded):
>+        * html/track/AudioTrackList.cpp:
>+        (AudioTrackList::activeDOMObjectName const):
>+        * html/track/AudioTrackList.h:
>+        * html/track/AudioTrackList.idl:
>+        * html/track/TextTrackList.cpp:
>+        (TextTrackList::activeDOMObjectName const):
>+        * html/track/TextTrackList.h:
>+        * html/track/TextTrackList.idl:
>+        * html/track/TrackListBase.cpp:
>+        (WebCore::TrackListBase::TrackListBase):
>+        (WebCore::TrackListBase::canSuspendForDocumentSuspension const):
>+        (WebCore::TrackListBase::suspend):
>+        (WebCore::TrackListBase::resume):
>+        (WebCore::TrackListBase::stop):
>+        * html/track/TrackListBase.h:
>+        * html/track/VideoTrackList.cpp:
>+        (VideoTrackList::activeDOMObjectName const):
>+        * html/track/VideoTrackList.h:
>+        * html/track/VideoTrackList.idl:
>+
> 2018-07-30  Thibault Saunier  <tsaunier@igalia.com>
> 
>         [GStreamer] Make codecparsers optionnal
>diff --git a/Source/WebCore/Modules/encryptedmedia/legacy/WebKitMediaKeySession.cpp b/Source/WebCore/Modules/encryptedmedia/legacy/WebKitMediaKeySession.cpp
>index 73f2d262dd8fd11d5be6ab8c2fc966fd9d1c0567..f0bf1e0ca2df16f99c75a2f58933ea503e3af49b 100644
>--- a/Source/WebCore/Modules/encryptedmedia/legacy/WebKitMediaKeySession.cpp
>+++ b/Source/WebCore/Modules/encryptedmedia/legacy/WebKitMediaKeySession.cpp
>@@ -239,8 +239,19 @@ bool WebKitMediaKeySession::hasPendingActivity() const
>     return (m_keys && m_session) || m_asyncEventQueue.hasPendingEvents();
> }
> 
>+void WebKitMediaKeySession::suspend(ReasonForSuspension)
>+{
>+    ASSERT_NOT_REACHED();
>+}
>+
>+void WebKitMediaKeySession::resume()
>+{
>+    ASSERT_NOT_REACHED();
>+}
>+
> void WebKitMediaKeySession::stop()
> {
>+    m_asyncEventQueue.close();
>     close();
> }
> 
>diff --git a/Source/WebCore/Modules/encryptedmedia/legacy/WebKitMediaKeySession.h b/Source/WebCore/Modules/encryptedmedia/legacy/WebKitMediaKeySession.h
>index 1a22d32f5a675e2535d5806d10f653055fdcada6..1d0dc7c4aab0a8b88c957928ec6de47a653bd889 100644
>--- a/Source/WebCore/Modules/encryptedmedia/legacy/WebKitMediaKeySession.h
>+++ b/Source/WebCore/Modules/encryptedmedia/legacy/WebKitMediaKeySession.h
>@@ -76,6 +76,8 @@ private:
>     void refEventTarget() final { ref(); }
>     void derefEventTarget() final { deref(); }
> 
>+    void suspend(ReasonForSuspension) final;
>+    void resume() final;
>     void stop() final;
>     bool canSuspendForDocumentSuspension() const final;
>     const char* activeDOMObjectName() const final;
>diff --git a/Source/WebCore/Modules/mediasource/MediaSource.cpp b/Source/WebCore/Modules/mediasource/MediaSource.cpp
>index e85c91b25a691b06f6d9ee8911e950c9eb779177..9aee75d9812c948dfdad51a0d037d4dd8d09dcbb 100644
>--- a/Source/WebCore/Modules/mediasource/MediaSource.cpp
>+++ b/Source/WebCore/Modules/mediasource/MediaSource.cpp
>@@ -692,124 +692,127 @@ ExceptionOr<void> MediaSource::removeSourceBuffer(SourceBuffer& buffer)
>     // 3. If the sourceBuffer.updating attribute equals true, then run the following steps: ...
>     buffer.abortIfUpdating();
> 
>-    // 4. Let SourceBuffer audioTracks list equal the AudioTrackList object returned by sourceBuffer.audioTracks.
>-    auto& audioTracks = buffer.audioTracks();
>-
>-    // 5. If the SourceBuffer audioTracks list is not empty, then run the following steps:
>-    if (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();
>-
>-            // 5.3.1 Set the sourceBuffer attribute on the AudioTrack object to null.
>-            track.setSourceBuffer(nullptr);
>-
>-            // 5.3.2 If the enabled attribute on the AudioTrack object is true, then set the removed enabled
>-            // audio track flag to true.
>-            if (track.enabled())
>-                removedEnabledAudioTrack = true;
>-
>-            // 5.3.3 Remove the AudioTrack object from the HTMLMediaElement audioTracks list.
>-            // 5.3.4 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 HTMLMediaElement audioTracks list.
>-            if (mediaElement())
>-                mediaElement()->removeAudioTrack(track);
>-
>-            // 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);
>+    ASSERT(scriptExecutionContext());
>+    if (!scriptExecutionContext()->activeDOMObjectsAreStopped()) {
>+        // 4. Let SourceBuffer audioTracks list equal the AudioTrackList object returned by sourceBuffer.audioTracks.
>+        auto& audioTracks = buffer.audioTracks();
>+
>+        // 5. If the SourceBuffer audioTracks list is not empty, then run the following steps:
>+        if (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();
>+
>+                // 5.3.1 Set the sourceBuffer attribute on the AudioTrack object to null.
>+                track.setSourceBuffer(nullptr);
>+
>+                // 5.3.2 If the enabled attribute on the AudioTrack object is true, then set the removed enabled
>+                // audio track flag to true.
>+                if (track.enabled())
>+                    removedEnabledAudioTrack = true;
>+
>+                // 5.3.3 Remove the AudioTrack object from the HTMLMediaElement audioTracks list.
>+                // 5.3.4 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 HTMLMediaElement audioTracks list.
>+                if (mediaElement())
>+                    mediaElement()->removeAudioTrack(track);
>+
>+                // 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);
>+            }
>+
>+            // 5.4 If the removed enabled audio track flag equals true, then queue a task to fire a simple event
>+            // named change at the HTMLMediaElement audioTracks list.
>+            if (removedEnabledAudioTrack)
>+                mediaElement()->ensureAudioTracks().scheduleChangeEvent();
>         }
> 
>-        // 5.4 If the removed enabled audio track flag equals true, then queue a task to fire a simple event
>-        // named change at the HTMLMediaElement audioTracks list.
>-        if (removedEnabledAudioTrack)
>-            mediaElement()->ensureAudioTracks().scheduleChangeEvent();
>-    }
>-
>-    // 6. Let SourceBuffer videoTracks list equal the VideoTrackList object returned by sourceBuffer.videoTracks.
>-    auto& videoTracks = buffer.videoTracks();
>-
>-    // 7. If the SourceBuffer videoTracks list is not empty, then run the following steps:
>-    if (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();
>-
>-            // 7.3.1 Set the sourceBuffer attribute on the VideoTrack object to null.
>-            track.setSourceBuffer(nullptr);
>-
>-            // 7.3.2 If the selected attribute on the VideoTrack object is true, then set the removed selected
>-            // video track flag to true.
>-            if (track.selected())
>-                removedSelectedVideoTrack = true;
>-
>-            // 7.3.3 Remove the VideoTrack object from the HTMLMediaElement videoTracks list.
>-            // 7.3.4 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 HTMLMediaElement videoTracks list.
>-            if (mediaElement())
>-                mediaElement()->removeVideoTrack(track);
>-
>-            // 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);
>+        // 6. Let SourceBuffer videoTracks list equal the VideoTrackList object returned by sourceBuffer.videoTracks.
>+        auto& videoTracks = buffer.videoTracks();
>+
>+        // 7. If the SourceBuffer videoTracks list is not empty, then run the following steps:
>+        if (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();
>+
>+                // 7.3.1 Set the sourceBuffer attribute on the VideoTrack object to null.
>+                track.setSourceBuffer(nullptr);
>+
>+                // 7.3.2 If the selected attribute on the VideoTrack object is true, then set the removed selected
>+                // video track flag to true.
>+                if (track.selected())
>+                    removedSelectedVideoTrack = true;
>+
>+                // 7.3.3 Remove the VideoTrack object from the HTMLMediaElement videoTracks list.
>+                // 7.3.4 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 HTMLMediaElement videoTracks list.
>+                if (mediaElement())
>+                    mediaElement()->removeVideoTrack(track);
>+
>+                // 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);
>+            }
>+
>+            // 7.4 If the removed selected video track flag equals true, then queue a task to fire a simple event
>+            // named change at the HTMLMediaElement videoTracks list.
>+            if (removedSelectedVideoTrack)
>+                mediaElement()->ensureVideoTracks().scheduleChangeEvent();
>         }
> 
>-        // 7.4 If the removed selected video track flag equals true, then queue a task to fire a simple event
>-        // named change at the HTMLMediaElement videoTracks list.
>-        if (removedSelectedVideoTrack)
>-            mediaElement()->ensureVideoTracks().scheduleChangeEvent();
>-    }
>-
>-    // 8. Let SourceBuffer textTracks list equal the TextTrackList object returned by sourceBuffer.textTracks.
>-    auto& textTracks = buffer.textTracks();
>-
>-    // 9. If the SourceBuffer textTracks list is not empty, then run the following steps:
>-    if (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();
>-
>-            // 9.3.1 Set the sourceBuffer attribute on the TextTrack object to null.
>-            track.setSourceBuffer(nullptr);
>-
>-            // 9.3.2 If the mode attribute on the TextTrack object is set to "showing" or "hidden", then
>-            // set the removed enabled text track flag to true.
>-            if (track.mode() == TextTrack::Mode::Showing || track.mode() == TextTrack::Mode::Hidden)
>-                removedEnabledTextTrack = true;
>-
>-            // 9.3.3 Remove the TextTrack object from the HTMLMediaElement textTracks list.
>-            // 9.3.4 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 HTMLMediaElement textTracks list.
>-            if (mediaElement())
>-                mediaElement()->removeTextTrack(track);
>-
>-            // 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);
>+        // 8. Let SourceBuffer textTracks list equal the TextTrackList object returned by sourceBuffer.textTracks.
>+        auto& textTracks = buffer.textTracks();
>+
>+        // 9. If the SourceBuffer textTracks list is not empty, then run the following steps:
>+        if (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();
>+
>+                // 9.3.1 Set the sourceBuffer attribute on the TextTrack object to null.
>+                track.setSourceBuffer(nullptr);
>+
>+                // 9.3.2 If the mode attribute on the TextTrack object is set to "showing" or "hidden", then
>+                // set the removed enabled text track flag to true.
>+                if (track.mode() == TextTrack::Mode::Showing || track.mode() == TextTrack::Mode::Hidden)
>+                    removedEnabledTextTrack = true;
>+
>+                // 9.3.3 Remove the TextTrack object from the HTMLMediaElement textTracks list.
>+                // 9.3.4 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 HTMLMediaElement textTracks list.
>+                if (mediaElement())
>+                    mediaElement()->removeTextTrack(track);
>+
>+                // 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);
>+            }
>+
>+            // 9.4 If the removed enabled text track flag equals true, then queue a task to fire a simple event
>+            // named change at the HTMLMediaElement textTracks list.
>+            if (removedEnabledTextTrack)
>+                mediaElement()->ensureTextTracks().scheduleChangeEvent();
>         }
>-
>-        // 9.4 If the removed enabled text track flag equals true, then queue a task to fire a simple event
>-        // named change at the HTMLMediaElement textTracks list.
>-        if (removedEnabledTextTrack)
>-            mediaElement()->ensureTextTracks().scheduleChangeEvent();
>     }
> 
>     // 10. If sourceBuffer is in activeSourceBuffers, then remove sourceBuffer from activeSourceBuffers ...
>@@ -932,6 +935,25 @@ bool MediaSource::hasPendingActivity() const
>         || ActiveDOMObject::hasPendingActivity();
> }
> 
>+void MediaSource::suspend(ReasonForSuspension reason)
>+{
>+    switch (reason) {
>+    case ReasonForSuspension::PageCache:
>+    case ReasonForSuspension::PageWillBeSuspended:
>+        m_asyncEventQueue.suspend();
>+        break;
>+    case ReasonForSuspension::JavaScriptDebuggerPaused:
>+    case ReasonForSuspension::WillDeferLoading:
>+        // Do nothing, we don't pause media playback in these cases.
>+        break;
>+    }
>+}
>+
>+void MediaSource::resume()
>+{
>+    m_asyncEventQueue.resume();
>+}
>+
> void MediaSource::stop()
> {
>     m_asyncEventQueue.close();
>diff --git a/Source/WebCore/Modules/mediasource/MediaSource.h b/Source/WebCore/Modules/mediasource/MediaSource.h
>index 29346572e309e3a0fce67f5cd5cd066d62c0826c..bd5cd7cf449e07a601a28ee5cf72c0374ffa5225 100644
>--- a/Source/WebCore/Modules/mediasource/MediaSource.h
>+++ b/Source/WebCore/Modules/mediasource/MediaSource.h
>@@ -107,6 +107,8 @@ public:
> private:
>     explicit MediaSource(ScriptExecutionContext&);
> 
>+    void suspend(ReasonForSuspension) final;
>+    void resume() final;
>     void stop() final;
>     bool canSuspendForDocumentSuspension() const final;
>     const char* activeDOMObjectName() const final;
>diff --git a/Source/WebCore/Modules/mediasource/SourceBuffer.cpp b/Source/WebCore/Modules/mediasource/SourceBuffer.cpp
>index 1fa3974797ae24175a2730f161fa110ea0f38606..a5266ee25c1de634d84565fb1b219d98576cc8be 100644
>--- a/Source/WebCore/Modules/mediasource/SourceBuffer.cpp
>+++ b/Source/WebCore/Modules/mediasource/SourceBuffer.cpp
>@@ -458,8 +458,28 @@ bool SourceBuffer::hasPendingActivity() const
>     return m_source || m_asyncEventQueue.hasPendingEvents();
> }
> 
>+void SourceBuffer::suspend(ReasonForSuspension reason)
>+{
>+    switch (reason) {
>+    case ReasonForSuspension::PageCache:
>+    case ReasonForSuspension::PageWillBeSuspended:
>+        m_asyncEventQueue.suspend();
>+        break;
>+    case ReasonForSuspension::JavaScriptDebuggerPaused:
>+    case ReasonForSuspension::WillDeferLoading:
>+        // Do nothing, we don't pause media playback in these cases.
>+        break;
>+    }
>+}
>+
>+void SourceBuffer::resume()
>+{
>+    m_asyncEventQueue.resume();
>+}
>+
> void SourceBuffer::stop()
> {
>+    m_asyncEventQueue.close();
>     m_appendBufferTimer.stop();
>     m_removeTimer.stop();
> }
>diff --git a/Source/WebCore/Modules/mediasource/SourceBuffer.h b/Source/WebCore/Modules/mediasource/SourceBuffer.h
>index c1a3dc5b08b3ca9bcfaa3f0384953990b79f5a87..9a30799e9ee5613cedcb447311c9f050dba40fb8 100644
>--- a/Source/WebCore/Modules/mediasource/SourceBuffer.h
>+++ b/Source/WebCore/Modules/mediasource/SourceBuffer.h
>@@ -123,6 +123,8 @@ private:
>     void refEventTarget() final { ref(); }
>     void derefEventTarget() final { deref(); }
> 
>+    void suspend(ReasonForSuspension) final;
>+    void resume() final;
>     void stop() final;
>     const char* activeDOMObjectName() const final;
>     bool canSuspendForDocumentSuspension() const final;
>diff --git a/Source/WebCore/Modules/mediasource/SourceBufferList.cpp b/Source/WebCore/Modules/mediasource/SourceBufferList.cpp
>index 4e8f520d1402d1a472cd1aa7c9a00b28c936507b..ec178ee2b0a9d3aeecd7166b7334bb01be3e2ddd 100644
>--- a/Source/WebCore/Modules/mediasource/SourceBufferList.cpp
>+++ b/Source/WebCore/Modules/mediasource/SourceBufferList.cpp
>@@ -40,9 +40,10 @@
> namespace WebCore {
> 
> SourceBufferList::SourceBufferList(ScriptExecutionContext* context)
>-    : ContextDestructionObserver(context)
>+    : ActiveDOMObject(context)
>     , m_asyncEventQueue(*this)
> {
>+    suspendIfNeeded();
> }
> 
> SourceBufferList::~SourceBufferList()
>@@ -97,6 +98,39 @@ void SourceBufferList::scheduleEvent(const AtomicString& eventName)
>     m_asyncEventQueue.enqueueEvent(WTFMove(event));
> }
> 
>+bool SourceBufferList::canSuspendForDocumentSuspension() const
>+{
>+    return !m_asyncEventQueue.hasPendingEvents();
>+}
>+
>+void SourceBufferList::suspend(ReasonForSuspension reason)
>+{
>+    switch (reason) {
>+    case ReasonForSuspension::PageCache:
>+    case ReasonForSuspension::PageWillBeSuspended:
>+        m_asyncEventQueue.suspend();
>+        break;
>+    case ReasonForSuspension::JavaScriptDebuggerPaused:
>+    case ReasonForSuspension::WillDeferLoading:
>+        // Do nothing, we don't pause media playback in these cases.
>+        break;
>+    }
>+}
>+
>+void SourceBufferList::resume()
>+{
>+    m_asyncEventQueue.resume();
>+}
>+
>+void SourceBufferList::stop()
>+{
>+    m_asyncEventQueue.close();
>+}
>+
>+const char* SourceBufferList::activeDOMObjectName() const
>+{
>+    return "SourceBufferList";
>+}
> 
> } // namespace WebCore
> 
>diff --git a/Source/WebCore/Modules/mediasource/SourceBufferList.h b/Source/WebCore/Modules/mediasource/SourceBufferList.h
>index 4186b4fecb323effba630cdd9a23c6483fb5b3f7..073198dc732990575719078583154b8d1eaaa5d9 100644
>--- a/Source/WebCore/Modules/mediasource/SourceBufferList.h
>+++ b/Source/WebCore/Modules/mediasource/SourceBufferList.h
>@@ -32,7 +32,7 @@
> 
> #if ENABLE(MEDIA_SOURCE)
> 
>-#include "ContextDestructionObserver.h"
>+#include "ActiveDOMObject.h"
> #include "EventTarget.h"
> #include "GenericEventQueue.h"
> #include <wtf/RefCounted.h>
>@@ -42,7 +42,7 @@ namespace WebCore {
> 
> class SourceBuffer;
> 
>-class SourceBufferList final : public RefCounted<SourceBufferList>, public EventTargetWithInlineData, public ContextDestructionObserver {
>+class SourceBufferList final : public RefCounted<SourceBufferList>, public EventTargetWithInlineData, public ActiveDOMObject {
> public:
>     static Ref<SourceBufferList> create(ScriptExecutionContext* context)
>     {
>@@ -77,6 +77,12 @@ private:
>     void refEventTarget() override { ref(); }
>     void derefEventTarget() override { deref(); }
> 
>+    bool canSuspendForDocumentSuspension() const final;
>+    void suspend(ReasonForSuspension) final;
>+    void resume() final;
>+    void stop() final;
>+    const char* activeDOMObjectName() const final;
>+
>     GenericEventQueue m_asyncEventQueue;
> 
>     Vector<RefPtr<SourceBuffer>> m_list;
>diff --git a/Source/WebCore/Modules/mediasource/SourceBufferList.idl b/Source/WebCore/Modules/mediasource/SourceBufferList.idl
>index 6c2d564ff956cd9f2f082d84874657e486e5e622..99c185ae29846aa1b729e35e82fe0fbef48ffa78 100644
>--- a/Source/WebCore/Modules/mediasource/SourceBufferList.idl
>+++ b/Source/WebCore/Modules/mediasource/SourceBufferList.idl
>@@ -29,6 +29,7 @@
>  */
>  
> [
>+    ActiveDOMObject,
>     Conditional=MEDIA_SOURCE,
>     GenerateIsReachable=Impl,
> ] interface SourceBufferList : EventTarget {
>diff --git a/Source/WebCore/dom/Document.h b/Source/WebCore/dom/Document.h
>index a8d37226bf2c7f5a0169fa414106ae66a0a93c5d..83b0889c09984984fa268354a8f47145be7c282b 100644
>--- a/Source/WebCore/dom/Document.h
>+++ b/Source/WebCore/dom/Document.h
>@@ -813,6 +813,8 @@ public:
>     // In DOM Level 2, the Document's DOMWindow is called the defaultView.
>     WEBCORE_EXPORT WindowProxy* windowProxy() const;
> 
>+    bool hasBrowsingContext() const { return !!frame(); }
>+
>     Document& contextDocument() const;
>     void setContextDocument(Document& document) { m_contextDocument = makeWeakPtr(document); }
> 
>diff --git a/Source/WebCore/dom/GenericEventQueue.cpp b/Source/WebCore/dom/GenericEventQueue.cpp
>index 5a32483128b87fc98415ee413fcced6e7222a698..8cfa8f04cb12f0ce1ff46542be622a474130aae0 100644
>--- a/Source/WebCore/dom/GenericEventQueue.cpp
>+++ b/Source/WebCore/dom/GenericEventQueue.cpp
>@@ -26,8 +26,10 @@
> #include "config.h"
> #include "GenericEventQueue.h"
> 
>+#include "Document.h"
> #include "Event.h"
> #include "EventTarget.h"
>+#include "Node.h"
> #include "ScriptExecutionContext.h"
> #include "Timer.h"
> #include <wtf/MainThread.h>
>@@ -65,6 +67,9 @@ void GenericEventQueue::dispatchOneEvent()
>     Ref<EventTarget> protect(m_owner);
>     RefPtr<Event> event = m_pendingEvents.takeFirst();
>     EventTarget& target = event->target() ? *event->target() : m_owner;
>+    ASSERT_WITH_MESSAGE(!target.scriptExecutionContext()->activeDOMObjectsAreStopped(),
>+        "An attempt to dispatch an event on a stopped target by EventTargetInterface=%d (nodeName=%s target=%p owner=%p)",
>+        m_owner.eventTargetInterface(), m_owner.isNode() ? static_cast<Node&>(m_owner).nodeName().ascii().data() : "", &target, &m_owner);
>     target.dispatchEvent(*event);
> }
> 
>diff --git a/Source/WebCore/html/HTMLMediaElement.cpp b/Source/WebCore/html/HTMLMediaElement.cpp
>index e2f9a3701801a437bcb127260af96ff740a641bd..8fbcb82eb22a473adb2dabddd7ce5a7ee9e8d6ef 100644
>--- a/Source/WebCore/html/HTMLMediaElement.cpp
>+++ b/Source/WebCore/html/HTMLMediaElement.cpp
>@@ -1299,6 +1299,9 @@ void HTMLMediaElement::prepareForLoad()
>     m_loadState = WaitingForSource;
>     m_currentSourceNode = nullptr;
> 
>+    if (!document().hasBrowsingContext())
>+        return;
>+
>     createMediaPlayer();
> 
>     // 2 - Let pending tasks be a list of all tasks from the media element's media element event task source in one of the task queues.
>@@ -1416,7 +1419,6 @@ void HTMLMediaElement::selectMediaResource()
>     // put into the background.
>     m_mediaSession->removeBehaviorRestriction(MediaElementSession::RequirePageConsentToLoadMedia);
> 
>-
>     m_resourceSelectionTaskQueue.enqueueTask([this]  {
>         // 5. If the media elementâs blocked-on-parser flag is false, then populate the list of pending text tracks.
> #if ENABLE(VIDEO_TRACK)
>@@ -2893,7 +2895,7 @@ bool HTMLMediaElement::supportsScanning() const
> void HTMLMediaElement::prepareToPlay()
> {
>     INFO_LOG(LOGIDENTIFIER);
>-    if (m_havePreparedToPlay)
>+    if (m_havePreparedToPlay || !document().hasBrowsingContext())
>         return;
>     m_havePreparedToPlay = true;
>     if (m_player)
>@@ -3488,6 +3490,11 @@ void HTMLMediaElement::playInternal()
>         return;
>     }
> 
>+    if (!document().hasBrowsingContext()) {
>+        INFO_LOG(LOGIDENTIFIER, "  returning because there is no browsing context");
>+        return;
>+    }
>+
>     if (!m_mediaSession->clientWillBeginPlayback()) {
>         ALWAYS_LOG(LOGIDENTIFIER, "  returning because of interruption");
>         return;
>@@ -3579,6 +3586,11 @@ void HTMLMediaElement::pauseInternal()
>         return;
>     }
> 
>+    if (!document().hasBrowsingContext()) {
>+        INFO_LOG(LOGIDENTIFIER, "  returning because there is no browsing context");
>+        return;
>+    }
>+
>     if (!m_mediaSession->clientWillPausePlayback()) {
>         ALWAYS_LOG(LOGIDENTIFIER, "  returning because of interruption");
>         return;
>@@ -4641,6 +4653,11 @@ void HTMLMediaElement::sourceWasAdded(HTMLSourceElement& source)
>         INFO_LOG(LOGIDENTIFIER, "'src' is ", url);
>     }
> 
>+    if (!document().hasBrowsingContext()) {
>+        INFO_LOG(LOGIDENTIFIER, "<source> inserted inside a document without a browsing context is not loaded");
>+        return;
>+    }
>+
>     // We should only consider a <source> element when there is not src attribute at all.
>     if (hasAttributeWithoutSynchronization(srcAttr))
>         return;
>diff --git a/Source/WebCore/html/track/AudioTrackList.cpp b/Source/WebCore/html/track/AudioTrackList.cpp
>index 3b07f051ef00dc8b2d4531001e50dc18074b71b6..f8b5d0accf0182f523223216292ed5454ec46b4d 100644
>--- a/Source/WebCore/html/track/AudioTrackList.cpp
>+++ b/Source/WebCore/html/track/AudioTrackList.cpp
>@@ -81,4 +81,9 @@ EventTargetInterface AudioTrackList::eventTargetInterface() const
>     return AudioTrackListEventTargetInterfaceType;
> }
> 
>+const char* AudioTrackList::activeDOMObjectName() const
>+{
>+    return "AudioTrackList";
>+}
>+
> #endif
>diff --git a/Source/WebCore/html/track/AudioTrackList.h b/Source/WebCore/html/track/AudioTrackList.h
>index 0c637ddc3dd415b4830ffaac3e2e9bc1c1bd0e2e..c490020c1a4e148a5697ca87c05eac26b60f5b1f 100644
>--- a/Source/WebCore/html/track/AudioTrackList.h
>+++ b/Source/WebCore/html/track/AudioTrackList.h
>@@ -52,6 +52,7 @@ public:
> 
> private:
>     AudioTrackList(HTMLMediaElement*, ScriptExecutionContext*);
>+    const char* activeDOMObjectName() const final;
> };
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/html/track/AudioTrackList.idl b/Source/WebCore/html/track/AudioTrackList.idl
>index cde73f40a91692aa2d3dc241722b77eda1977c5c..7781c094918820ea6e76b3bfd247574741746930 100644
>--- a/Source/WebCore/html/track/AudioTrackList.idl
>+++ b/Source/WebCore/html/track/AudioTrackList.idl
>@@ -24,6 +24,7 @@
>  */
> 
> [
>+    ActiveDOMObject,
>     Conditional=VIDEO_TRACK,
>     GenerateIsReachable=ImplElementRoot,
>     JSCustomMarkFunction,
>diff --git a/Source/WebCore/html/track/TextTrackList.cpp b/Source/WebCore/html/track/TextTrackList.cpp
>index aa964f230ff4333947c0e481b2feb1fbd03acc14..9b691f5b10550a280f1803003c7cf5c7cd621839 100644
>--- a/Source/WebCore/html/track/TextTrackList.cpp
>+++ b/Source/WebCore/html/track/TextTrackList.cpp
>@@ -265,4 +265,9 @@ EventTargetInterface TextTrackList::eventTargetInterface() const
>     return TextTrackListEventTargetInterfaceType;
> }
> 
>+const char* TextTrackList::activeDOMObjectName() const
>+{
>+    return "TextTrackList";
>+}
>+
> #endif
>diff --git a/Source/WebCore/html/track/TextTrackList.h b/Source/WebCore/html/track/TextTrackList.h
>index c42f1e194929bef723aa386cf4c04bbec590d503..520429182dcdfa9f6bcfff28dbe18c1dbb142ea2 100644
>--- a/Source/WebCore/html/track/TextTrackList.h
>+++ b/Source/WebCore/html/track/TextTrackList.h
>@@ -62,6 +62,7 @@ private:
>     TextTrackList(HTMLMediaElement*, ScriptExecutionContext*);
> 
>     void invalidateTrackIndexesAfterTrack(TextTrack&);
>+    const char* activeDOMObjectName() const final;
> 
>     Vector<RefPtr<TrackBase>> m_addTrackTracks;
>     Vector<RefPtr<TrackBase>> m_elementTracks;
>diff --git a/Source/WebCore/html/track/TextTrackList.idl b/Source/WebCore/html/track/TextTrackList.idl
>index fedd73b55c26870b5f3386511b05a7a1a43d601f..35820a5dab8d5d2791d5b143a10dd4c2c857657d 100644
>--- a/Source/WebCore/html/track/TextTrackList.idl
>+++ b/Source/WebCore/html/track/TextTrackList.idl
>@@ -24,6 +24,7 @@
>  */
> 
> [
>+    ActiveDOMObject,
>     Conditional=VIDEO_TRACK,
>     GenerateIsReachable=ImplElementRoot,
>     JSCustomMarkFunction,
>diff --git a/Source/WebCore/html/track/TrackListBase.cpp b/Source/WebCore/html/track/TrackListBase.cpp
>index 2e96942152ac8d8de1fd50b5db6a2fd93a7a602a..147700f680f375377ce5a63eaf81f9aa2b0a9c3e 100644
>--- a/Source/WebCore/html/track/TrackListBase.cpp
>+++ b/Source/WebCore/html/track/TrackListBase.cpp
>@@ -34,14 +34,15 @@
> #include "ScriptExecutionContext.h"
> #include "TrackEvent.h"
> 
>-using namespace WebCore;
>+namespace WebCore {
> 
> TrackListBase::TrackListBase(HTMLMediaElement* element, ScriptExecutionContext* context)
>-    : ContextDestructionObserver(context)
>+    : ActiveDOMObject(context)
>     , m_element(element)
>     , m_asyncEventQueue(*this)
> {
>-    ASSERT(is<Document>(context));
>+    ASSERT(!context || is<Document>(context));
>+    suspendIfNeeded();
> }
> 
> TrackListBase::~TrackListBase()
>@@ -174,4 +175,35 @@ bool TrackListBase::isAnyTrackEnabled() const
>     return false;
> }
> 
>+bool TrackListBase::canSuspendForDocumentSuspension() const
>+{
>+    return !m_asyncEventQueue.hasPendingEvents();
>+}
>+
>+void TrackListBase::suspend(ReasonForSuspension reason)
>+{
>+    switch (reason) {
>+    case ReasonForSuspension::PageCache:
>+    case ReasonForSuspension::PageWillBeSuspended:
>+        m_asyncEventQueue.suspend();
>+        break;
>+    case ReasonForSuspension::JavaScriptDebuggerPaused:
>+    case ReasonForSuspension::WillDeferLoading:
>+        // Do nothing, we don't pause media playback in these cases.
>+        break;
>+    }
>+}
>+
>+void TrackListBase::resume()
>+{
>+    m_asyncEventQueue.resume();
>+}
>+
>+void TrackListBase::stop()
>+{
>+    m_asyncEventQueue.close();
>+}
>+
>+} // namespace WebCore
>+
> #endif
>diff --git a/Source/WebCore/html/track/TrackListBase.h b/Source/WebCore/html/track/TrackListBase.h
>index f3d644dbc2e2cf983e04ac8f933d58f522548b5f..fdae3b0de185cc8cb64bf954dbbe12704f3b32ae 100644
>--- a/Source/WebCore/html/track/TrackListBase.h
>+++ b/Source/WebCore/html/track/TrackListBase.h
>@@ -27,7 +27,7 @@
> 
> #if ENABLE(VIDEO_TRACK)
> 
>-#include "ContextDestructionObserver.h"
>+#include "ActiveDOMObject.h"
> #include "EventTarget.h"
> #include "GenericEventQueue.h"
> #include <wtf/RefCounted.h>
>@@ -39,7 +39,7 @@ class HTMLMediaElement;
> class Element;
> class TrackBase;
> 
>-class TrackListBase : public RefCounted<TrackListBase>, public EventTargetWithInlineData, public ContextDestructionObserver {
>+class TrackListBase : public RefCounted<TrackListBase>, public EventTargetWithInlineData, public ActiveDOMObject {
> public:
>     virtual ~TrackListBase();
> 
>@@ -74,6 +74,11 @@ protected:
> private:
>     void scheduleTrackEvent(const AtomicString& eventName, Ref<TrackBase>&&);
> 
>+    bool canSuspendForDocumentSuspension() const final;
>+    void suspend(ReasonForSuspension) final;
>+    void resume() final;
>+    void stop() final;
>+
>     // EventTarget
>     void refEventTarget() final { ref(); }
>     void derefEventTarget() final { deref(); }
>diff --git a/Source/WebCore/html/track/VideoTrackList.cpp b/Source/WebCore/html/track/VideoTrackList.cpp
>index 3c5b5f1c6d7bada881a9a664ebff8cf7d8c2e86d..a1549abcc85f617416633537de5c955036594710 100644
>--- a/Source/WebCore/html/track/VideoTrackList.cpp
>+++ b/Source/WebCore/html/track/VideoTrackList.cpp
>@@ -94,4 +94,9 @@ EventTargetInterface VideoTrackList::eventTargetInterface() const
>     return VideoTrackListEventTargetInterfaceType;
> }
> 
>+const char* VideoTrackList::activeDOMObjectName() const
>+{
>+    return "VideoTrackList";
>+}
>+
> #endif
>diff --git a/Source/WebCore/html/track/VideoTrackList.h b/Source/WebCore/html/track/VideoTrackList.h
>index 6b5be25b46a7b06e601a4a48f4d4b5510ea7587e..cc250fcb317f7279c343090be61173a7f4d8fbde 100644
>--- a/Source/WebCore/html/track/VideoTrackList.h
>+++ b/Source/WebCore/html/track/VideoTrackList.h
>@@ -53,6 +53,7 @@ public:
> 
> private:
>     VideoTrackList(HTMLMediaElement*, ScriptExecutionContext*);
>+    const char* activeDOMObjectName() const final;
> };
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/html/track/VideoTrackList.idl b/Source/WebCore/html/track/VideoTrackList.idl
>index 9c43fcd181df56c0752d176e18006945b9ae0347..18f40ed22660f2f4843b362dffce9e97c1e41a67 100644
>--- a/Source/WebCore/html/track/VideoTrackList.idl
>+++ b/Source/WebCore/html/track/VideoTrackList.idl
>@@ -24,6 +24,7 @@
>  */
> 
> [
>+    ActiveDOMObject,
>     Conditional=VIDEO_TRACK,
>     GenerateIsReachable=ImplElementRoot,
>     JSCustomMarkFunction,
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 0d0df34ecac58f112f12bdb0d1d3c85371012754..8a322444ad7df012e0eb3d671a2aa2f79091731b 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-07-30  Chris Dumez  <cdumez@apple.com>
>+
>+        Nullptr crash accessing Document in GenericEventQueue::dispatchOneEvent()
>+        https://bugs.webkit.org/show_bug.cgi?id=187284
>+
>+        Unreviewed, roll r233496 and r233571 now that all known issues have been fixed via
>+        dependency bugs.
>+
>+        * media/track/video-track-addition-and-frame-removal-expected.txt: Added.
>+        * media/track/video-track-addition-and-frame-removal.html: Added.
>+
> 2018-07-30  Per Arne Vollan  <pvollan@apple.com>
> 
>         [Win] Layout test fast/repaint/canvas-object-fit.html is failing.
>diff --git a/LayoutTests/media/track/video-track-addition-and-frame-removal-expected.txt b/LayoutTests/media/track/video-track-addition-and-frame-removal-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..f66254549c60d492c2d892b881912b4b4ef54b39
>--- /dev/null
>+++ b/LayoutTests/media/track/video-track-addition-and-frame-removal-expected.txt
>@@ -0,0 +1,4 @@
>+This tests removing a frame immediately after inserting a new track element.
>+WebKit should not hit any assertions.
>+
>+
>diff --git a/LayoutTests/media/track/video-track-addition-and-frame-removal.html b/LayoutTests/media/track/video-track-addition-and-frame-removal.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..ef35557d0cbbe8f8fc7bb014c8e9ab4053c5df9e
>--- /dev/null
>+++ b/LayoutTests/media/track/video-track-addition-and-frame-removal.html
>@@ -0,0 +1,29 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<p>This tests removing a frame immediately after inserting a new track element.<br>
>+WebKit should not hit any assertions.</p>
>+<script src=../../resources/gc.js></script>
>+<script>
>+
>+if (window.testRunner)
>+    testRunner.waitUntilDone();
>+
>+function startTest()
>+{
>+    const doc = frame.contentDocument;
>+    const trackElement = doc.createElement('track');
>+    doc.querySelector('video').appendChild(trackElement);
>+
>+    frame.remove();
>+    gc();
>+    setTimeout(() => {
>+        if (window.testRunner)
>+            testRunner.notifyDone();        
>+    }, 100);
>+}
>+
>+</script>
>+<iframe id="frame" src="./video-track-add-remove.html"></iframe>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 187284: 344236 | 344243 | 346060