Cherry-pick 265870.524@safari-7616-branch (5a87cf9). https://bugs.web…

…kit.org/show_bug.cgi?id=255629

    Regression(264919@main) Use-after-free of MediaRecorderPrivate in GPUProcessConnection::didClose()
    https://bugs.webkit.org/show_bug.cgi?id=261224
    rdar://114807341

    Reviewed by Alex Christensen.

    264919@main made WebKit::MediaRecorderPrivate subclass ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr.
    However, MediaRecorderPrivate is stored in std::unique_ptr<> throughout our code base, thus not obeying
    the refcount when managing its lifetime. This was the source of use-after-frees in
    GPUProcessConnection::didClose(), which held a strong Ref<> to the MediaRecorderPrivate but it wouldn't
    prevent the object from dying.

    To address the issue, we now use Ref<> / RefPtr<> everywhere for MediaRecorderPrivate. I also moved
    ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr to the base class (WebCore::MediaRecorderPrivate) since
    WebCore needs to know it can hold a Ref<> / RefPtr<> to such objects.

    * Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:
    (WebCore::MediaRecorder::createMediaRecorderPrivate):
    (WebCore::MediaRecorder::fetchData):
    * Source/WebCore/Modules/mediarecorder/MediaRecorder.h:
    * Source/WebCore/Modules/mediarecorder/MediaRecorderProvider.cpp:
    (WebCore::MediaRecorderProvider::createMediaRecorderPrivate):
    * Source/WebCore/Modules/mediarecorder/MediaRecorderProvider.h:
    * Source/WebCore/loader/EmptyClients.cpp:
    * Source/WebCore/platform/mediarecorder/MediaRecorderPrivate.h:
    * Source/WebCore/platform/mediarecorder/MediaRecorderPrivateAVFImpl.cpp:
    (WebCore::MediaRecorderPrivateAVFImpl::create):
    * Source/WebCore/platform/mediarecorder/MediaRecorderPrivateAVFImpl.h:
    * Source/WebCore/platform/mediarecorder/MediaRecorderPrivateGStreamer.cpp:
    (WebCore::MediaRecorderPrivateGStreamer::create):
    * Source/WebCore/platform/mediarecorder/MediaRecorderPrivateGStreamer.h:
    * Source/WebCore/platform/mediarecorder/MediaRecorderPrivateMock.cpp:
    (WebCore::MediaRecorderPrivateMock::create):
    * Source/WebCore/platform/mediarecorder/MediaRecorderPrivateMock.h:
    * Source/WebCore/testing/Internals.cpp:
    (WebCore::createRecorderMockSource):
    * Source/WebKit/WebProcess/GPU/webrtc/MediaRecorderPrivate.cpp:
    (WebKit::MediaRecorderPrivate::create):
    * Source/WebKit/WebProcess/GPU/webrtc/MediaRecorderPrivate.h:
    * Source/WebKit/WebProcess/GPU/webrtc/MediaRecorderProvider.cpp:
    (WebKit::MediaRecorderProvider::createMediaRecorderPrivate):
    * Source/WebKit/WebProcess/GPU/webrtc/MediaRecorderProvider.h:

    Canonical link: https://commits.webkit.org/265870.524@safari-7616-branch

Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp

@@ -78,7 +78,7 @@ void MediaRecorder::setCustomPrivateRecorderCreator(CreatorFunction creator)
     m_customCreator = creator;
 }
 
-ExceptionOr<std::unique_ptr<MediaRecorderPrivate>> MediaRecorder::createMediaRecorderPrivate(Document& document, MediaStreamPrivate& stream, const Options& options)
+ExceptionOr<Ref<MediaRecorderPrivate>> MediaRecorder::createMediaRecorderPrivate(Document& document, MediaStreamPrivate& stream, const Options& options)
 {
     auto* page = document.page();
     if (!page)
@@ -87,14 +87,13 @@ ExceptionOr<std::unique_ptr<MediaRecorderPrivate>> MediaRecorder::createMediaRec
     if (m_customCreator)
         return m_customCreator(stream, options);
 
+    RefPtr<MediaRecorderPrivate> result;
 #if PLATFORM(COCOA) || USE(GSTREAMER_TRANSCODER)
-    auto result = page->mediaRecorderProvider().createMediaRecorderPrivate(stream, options);
-#else
-    std::unique_ptr<MediaRecorderPrivate> result;
+    result = page->mediaRecorderProvider().createMediaRecorderPrivate(stream, options);
 #endif
     if (!result)
         return Exception { NotSupportedError, "The MediaRecorder is unsupported on this platform"_s };
-    return result;
+    return result.releaseNonNull();
 }
 
 MediaRecorder::MediaRecorder(Document& document, Ref<MediaStream>&& stream, Options&& options)
@@ -295,11 +294,11 @@ ExceptionOr<void> MediaRecorder::resumeRecording()
 
 void MediaRecorder::fetchData(FetchDataCallback&& callback, TakePrivateRecorder takeRecorder)
 {
-    auto& privateRecorder = *m_private;
+    Ref privateRecorder = *m_private;
 
-    std::unique_ptr<MediaRecorderPrivate> takenPrivateRecorder;
+    RefPtr<MediaRecorderPrivate> takenPrivateRecorder;
     if (takeRecorder == TakePrivateRecorder::Yes)
-        takenPrivateRecorder = WTFMove(m_private);
+        takenPrivateRecorder = std::exchange(m_private, nullptr);
 
     auto fetchDataCallback = [this, privateRecorder = WTFMove(takenPrivateRecorder), callback = WTFMove(callback)](auto&& buffer, auto& mimeType, auto timeCode) mutable {
         queueTaskKeepingObjectAlive(*this, TaskSource::Networking, [buffer = WTFMove(buffer), mimeType, timeCode, callback = WTFMove(callback)]() mutable {
@@ -313,7 +312,7 @@ void MediaRecorder::fetchData(FetchDataCallback&& callback, TakePrivateRecorder
     }
 
     m_isFetchingData = true;
-    privateRecorder.fetchData([this, pendingActivity = makePendingActivity(*this), callback = WTFMove(fetchDataCallback)](auto&& buffer, auto& mimeType, auto timeCode) mutable {
+    privateRecorder->fetchData([this, pendingActivity = makePendingActivity(*this), callback = WTFMove(fetchDataCallback)](auto&& buffer, auto& mimeType, auto timeCode) mutable {
         m_isFetchingData = false;
         callback(WTFMove(buffer), mimeType, timeCode);
         for (auto& task : std::exchange(m_pendingFetchDataTasks, { }))

Source/WebCore/Modules/mediarecorder/MediaRecorder.h

@@ -59,7 +59,7 @@ class MediaRecorder final
     using Options = MediaRecorderPrivateOptions;
     static ExceptionOr<Ref<MediaRecorder>> create(Document&, Ref<MediaStream>&&, Options&& = { });
 
-    using CreatorFunction = ExceptionOr<std::unique_ptr<MediaRecorderPrivate>> (*)(MediaStreamPrivate&, const Options&);
+    using CreatorFunction = ExceptionOr<Ref<MediaRecorderPrivate>> (*)(MediaStreamPrivate&, const Options&);
 
     WEBCORE_EXPORT static void setCustomPrivateRecorderCreator(CreatorFunction);
 
@@ -87,7 +87,7 @@ class MediaRecorder final
 private:
     MediaRecorder(Document&, Ref<MediaStream>&&, Options&&);
 
-    static ExceptionOr<std::unique_ptr<MediaRecorderPrivate>> createMediaRecorderPrivate(Document&, MediaStreamPrivate&, const Options&);
+    static ExceptionOr<Ref<MediaRecorderPrivate>> createMediaRecorderPrivate(Document&, MediaStreamPrivate&, const Options&);
 
     Document* document() const;
 
@@ -130,7 +130,7 @@ class MediaRecorder final
 
     Options m_options;
     Ref<MediaStream> m_stream;
-    std::unique_ptr<MediaRecorderPrivate> m_private;
+    RefPtr<MediaRecorderPrivate> m_private;
     RecordingState m_state { RecordingState::Inactive };
     Vector<Ref<MediaStreamTrackPrivate>> m_tracks;
     std::optional<unsigned> m_timeSlice;

Source/WebCore/Modules/mediarecorder/MediaRecorderProvider.cpp

@@ -41,7 +41,7 @@
 
 namespace WebCore {
 
-std::unique_ptr<MediaRecorderPrivate> MediaRecorderProvider::createMediaRecorderPrivate(MediaStreamPrivate& stream, const MediaRecorderPrivateOptions& options)
+RefPtr<MediaRecorderPrivate> MediaRecorderProvider::createMediaRecorderPrivate(MediaStreamPrivate& stream, const MediaRecorderPrivateOptions& options)
 {
 #if PLATFORM(COCOA) && USE(AVFOUNDATION)
     return MediaRecorderPrivateAVFImpl::create(stream, options);

Source/WebCore/Modules/mediarecorder/MediaRecorderProvider.h

@@ -41,7 +41,7 @@ class WEBCORE_EXPORT MediaRecorderProvider {
     virtual ~MediaRecorderProvider() = default;
 
 #if ENABLE(MEDIA_RECORDER)
-    virtual std::unique_ptr<MediaRecorderPrivate> createMediaRecorderPrivate(MediaStreamPrivate&, const MediaRecorderPrivateOptions&);
+    virtual RefPtr<MediaRecorderPrivate> createMediaRecorderPrivate(MediaStreamPrivate&, const MediaRecorderPrivateOptions&);
     virtual bool isSupported(const String&);
 #endif
 };

Source/WebCore/loader/EmptyClients.cpp

@@ -1159,7 +1159,7 @@ class EmptyMediaRecorderProvider final : public MediaRecorderProvider {
     EmptyMediaRecorderProvider() = default;
 private:
 #if ENABLE(MEDIA_RECORDER)
-    std::unique_ptr<MediaRecorderPrivate> createMediaRecorderPrivate(MediaStreamPrivate&, const MediaRecorderPrivateOptions&) final { return nullptr; }
+    RefPtr<MediaRecorderPrivate> createMediaRecorderPrivate(MediaStreamPrivate&, const MediaRecorderPrivateOptions&) final { return nullptr; }
 #endif
 };
 

Source/WebCore/platform/mediarecorder/MediaRecorderPrivate.h

@@ -29,6 +29,7 @@
 #include "RealtimeMediaSource.h"
 #include <wtf/CompletionHandler.h>
 #include <wtf/Forward.h>
+#include <wtf/ThreadSafeWeakPtr.h>
 
 #if ENABLE(MEDIA_RECORDER)
 
@@ -48,10 +49,11 @@ class FragmentedSharedBuffer;
 struct MediaRecorderPrivateOptions;
 
 class MediaRecorderPrivate
-    : public RealtimeMediaSource::AudioSampleObserver
+    : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<MediaRecorderPrivate>
+    , public RealtimeMediaSource::AudioSampleObserver
     , public RealtimeMediaSource::VideoFrameObserver {
 public:
-    ~MediaRecorderPrivate();
+    virtual ~MediaRecorderPrivate();
 
     struct AudioVideoSelectedTracks {
         MediaStreamTrackPrivate* audioTrack { nullptr };
@@ -80,6 +82,7 @@ class MediaRecorderPrivate
     static BitRates computeBitRates(const MediaRecorderPrivateOptions&, const MediaStreamPrivate* = nullptr);
 
 protected:
+    MediaRecorderPrivate() = default;
     void setAudioSource(RefPtr<RealtimeMediaSource>&&);
     void setVideoSource(RefPtr<RealtimeMediaSource>&&);
 
@@ -93,7 +96,6 @@ class MediaRecorderPrivate
     virtual void pauseRecording(CompletionHandler<void()>&&) = 0;
     virtual void resumeRecording(CompletionHandler<void()>&&) = 0;
 
-private:
     bool m_shouldMuteAudio { false };
     bool m_shouldMuteVideo { false };
     RefPtr<RealtimeMediaSource> m_audioSource;

Source/WebCore/platform/mediarecorder/MediaRecorderPrivateAVFImpl.cpp

@@ -44,7 +44,7 @@
 
 namespace WebCore {
 
-std::unique_ptr<MediaRecorderPrivateAVFImpl> MediaRecorderPrivateAVFImpl::create(MediaStreamPrivate& stream, const MediaRecorderPrivateOptions& options)
+RefPtr<MediaRecorderPrivateAVFImpl> MediaRecorderPrivateAVFImpl::create(MediaStreamPrivate& stream, const MediaRecorderPrivateOptions& options)
 {
     // FIXME: we will need to implement support for multiple audio/video tracks
     // Currently we only choose the first track as the recorded track.
@@ -56,7 +56,7 @@ std::unique_ptr<MediaRecorderPrivateAVFImpl> MediaRecorderPrivateAVFImpl::create
     if (!writer)
         return nullptr;
 
-    auto recorder = std::unique_ptr<MediaRecorderPrivateAVFImpl>(new MediaRecorderPrivateAVFImpl(writer.releaseNonNull()));
+    auto recorder = adoptRef(*new MediaRecorderPrivateAVFImpl(writer.releaseNonNull()));
     if (selectedTracks.audioTrack) {
         recorder->setAudioSource(&selectedTracks.audioTrack->source());
         recorder->checkTrackState(*selectedTracks.audioTrack);

Source/WebCore/platform/mediarecorder/MediaRecorderPrivateAVFImpl.h

@@ -42,7 +42,7 @@ class MediaRecorderPrivateAVFImpl final
     : public MediaRecorderPrivate {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    static std::unique_ptr<MediaRecorderPrivateAVFImpl> create(MediaStreamPrivate&, const MediaRecorderPrivateOptions&);
+    static RefPtr<MediaRecorderPrivateAVFImpl> create(MediaStreamPrivate&, const MediaRecorderPrivateOptions&);
     ~MediaRecorderPrivateAVFImpl();
 
 private:

Source/WebCore/platform/mediarecorder/MediaRecorderPrivateGStreamer.cpp

@@ -38,7 +38,7 @@ namespace WebCore {
 GST_DEBUG_CATEGORY(webkit_media_recorder_debug);
 #define GST_CAT_DEFAULT webkit_media_recorder_debug
 
-std::unique_ptr<MediaRecorderPrivateGStreamer> MediaRecorderPrivateGStreamer::create(MediaStreamPrivate& stream, const MediaRecorderPrivateOptions& options)
+RefPtr<MediaRecorderPrivateGStreamer> MediaRecorderPrivateGStreamer::create(MediaStreamPrivate& stream, const MediaRecorderPrivateOptions& options)
 {
     ensureGStreamerInitialized();
     registerWebKitGStreamerElements();
@@ -51,7 +51,7 @@ std::unique_ptr<MediaRecorderPrivateGStreamer> MediaRecorderPrivateGStreamer::cr
     if (!recorder->preparePipeline())
         return nullptr;
 
-    return makeUnique<MediaRecorderPrivateGStreamer>(recorder.releaseNonNull());
+    return adoptRef(*new MediaRecorderPrivateGStreamer(recorder.releaseNonNull()));
 }
 
 MediaRecorderPrivateGStreamer::MediaRecorderPrivateGStreamer(Ref<MediaRecorderPrivateBackend>&& recorder)

Source/WebCore/platform/mediarecorder/MediaRecorderPrivateGStreamer.h

@@ -44,7 +44,6 @@ class MediaRecorderPrivateBackend : public ThreadSafeRefCountedAndCanMakeThreadS
     {
         return adoptRef(*new MediaRecorderPrivateBackend(stream, options));
     }
-
     ~MediaRecorderPrivateBackend();
 
     bool preparePipeline();
@@ -55,7 +54,6 @@ class MediaRecorderPrivateBackend : public ThreadSafeRefCountedAndCanMakeThreadS
     void pauseRecording(CompletionHandler<void()>&&);
     void resumeRecording(CompletionHandler<void()>&&);
     const String& mimeType() const;
-
     void setSelectTracksCallback(SelectTracksCallback&& callback) { m_selectTracksCallback = WTFMove(callback); }
 
 private:
@@ -94,13 +92,13 @@ class MediaRecorderPrivateBackend : public ThreadSafeRefCountedAndCanMakeThreadS
 class MediaRecorderPrivateGStreamer final : public MediaRecorderPrivate {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    static std::unique_ptr<MediaRecorderPrivateGStreamer> create(MediaStreamPrivate&, const MediaRecorderPrivateOptions&);
-    explicit MediaRecorderPrivateGStreamer(Ref<MediaRecorderPrivateBackend>&&);
+    static RefPtr<MediaRecorderPrivateGStreamer> create(MediaStreamPrivate&, const MediaRecorderPrivateOptions&);
     ~MediaRecorderPrivateGStreamer() = default;
 
     static bool isTypeSupported(const ContentType&);
 
 private:
+    explicit MediaRecorderPrivateGStreamer(Ref<MediaRecorderPrivateBackend>&&);
     void videoFrameAvailable(VideoFrame&, VideoFrameTimeMetadata) final { };
     void audioSamplesAvailable(const WTF::MediaTime&, const PlatformAudioData&, const AudioStreamDescription&, size_t) final { };
 

Source/WebCore/platform/mediarecorder/MediaRecorderPrivateMock.cpp

@@ -36,6 +36,11 @@
 
 namespace WebCore {
 
+Ref<MediaRecorderPrivateMock> MediaRecorderPrivateMock::create(MediaStreamPrivate& stream)
+{
+    return adoptRef(*new MediaRecorderPrivateMock(stream));
+}
+
 MediaRecorderPrivateMock::MediaRecorderPrivateMock(MediaStreamPrivate& stream)
 {
     auto selectedTracks = MediaRecorderPrivate::selectTracks(stream);

Source/WebCore/platform/mediarecorder/MediaRecorderPrivateMock.h

@@ -38,10 +38,11 @@ class MediaStreamTrackPrivate;
 class WEBCORE_EXPORT MediaRecorderPrivateMock final
     : public MediaRecorderPrivate {
 public:
-    explicit MediaRecorderPrivateMock(MediaStreamPrivate&);
+    static Ref<MediaRecorderPrivateMock> create(MediaStreamPrivate&);
     ~MediaRecorderPrivateMock();
 
 private:
+    explicit MediaRecorderPrivateMock(MediaStreamPrivate&);
     // MediaRecorderPrivate
     void videoFrameAvailable(VideoFrame&, VideoFrameTimeMetadata) final;
     void fetchData(FetchDataCallback&&) final;

Source/WebCore/testing/Internals.cpp

@@ -1776,9 +1776,9 @@ void Internals::setShouldInterruptAudioOnPageVisibilityChange(bool shouldInterru
 #endif // ENABLE(MEDIA_STREAM)
 
 #if ENABLE(MEDIA_RECORDER)
-static ExceptionOr<std::unique_ptr<MediaRecorderPrivate>> createRecorderMockSource(MediaStreamPrivate& stream, const MediaRecorderPrivateOptions&)
+static ExceptionOr<Ref<MediaRecorderPrivate>> createRecorderMockSource(MediaStreamPrivate& stream, const MediaRecorderPrivateOptions&)
 {
-    return std::unique_ptr<MediaRecorderPrivate>(new MediaRecorderPrivateMock(stream));
+    return { MediaRecorderPrivateMock::create(stream) };
 }
 
 void Internals::setCustomPrivateRecorderCreator()

Source/WebKit/WebProcess/GPU/webrtc/MediaRecorderPrivate.cpp

@@ -48,6 +48,11 @@ namespace WebKit {
 using namespace PAL;
 using namespace WebCore;
 
+Ref<MediaRecorderPrivate> MediaRecorderPrivate::create(WebCore::MediaStreamPrivate& stream, const WebCore::MediaRecorderPrivateOptions& options)
+{
+    return adoptRef(*new MediaRecorderPrivate(stream, options));
+}
+
 MediaRecorderPrivate::MediaRecorderPrivate(MediaStreamPrivate& stream, const MediaRecorderPrivateOptions& options)
     : m_identifier(MediaRecorderIdentifier::generate())
     , m_stream(stream)

Source/WebKit/WebProcess/GPU/webrtc/MediaRecorderPrivate.h

@@ -49,18 +49,19 @@ namespace WebKit {
 
 class MediaRecorderPrivate final
     : public WebCore::MediaRecorderPrivate
-    , public GPUProcessConnection::Client
-    , public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<MediaRecorderPrivate> {
+    , public GPUProcessConnection::Client {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    MediaRecorderPrivate(WebCore::MediaStreamPrivate&, const WebCore::MediaRecorderPrivateOptions&);
+    static Ref<MediaRecorderPrivate> create(WebCore::MediaStreamPrivate&, const WebCore::MediaRecorderPrivateOptions&);
     ~MediaRecorderPrivate();
 
-    void ref() const final { return ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<MediaRecorderPrivate>::ref(); }
-    void deref() const final { return ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<MediaRecorderPrivate>::deref(); }
-    ThreadSafeWeakPtrControlBlock& controlBlock() const final { return ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<MediaRecorderPrivate>::controlBlock(); }
+    void ref() const final { return WebCore::MediaRecorderPrivate::ref(); }
+    void deref() const final { return WebCore::MediaRecorderPrivate::deref(); }
+    ThreadSafeWeakPtrControlBlock& controlBlock() const final { return WebCore::MediaRecorderPrivate::controlBlock(); }
 
 private:
+    MediaRecorderPrivate(WebCore::MediaStreamPrivate&, const WebCore::MediaRecorderPrivateOptions&);
+
     // WebCore::MediaRecorderPrivate
     void videoFrameAvailable(WebCore::VideoFrame&, WebCore::VideoFrameTimeMetadata) final;
     void fetchData(CompletionHandler<void(RefPtr<WebCore::FragmentedSharedBuffer>&&, const String& mimeType, double)>&&) final;

Source/WebKit/WebProcess/GPU/webrtc/MediaRecorderProvider.cpp

@@ -37,12 +37,12 @@
 namespace WebKit {
 using namespace WebCore;
 
-std::unique_ptr<WebCore::MediaRecorderPrivate> MediaRecorderProvider::createMediaRecorderPrivate(MediaStreamPrivate& stream, const MediaRecorderPrivateOptions& options)
+RefPtr<WebCore::MediaRecorderPrivate> MediaRecorderProvider::createMediaRecorderPrivate(MediaStreamPrivate& stream, const MediaRecorderPrivateOptions& options)
 {
 #if ENABLE(GPU_PROCESS)
     auto* page = m_webPage.corePage();
     if (page && page->settings().webRTCPlatformCodecsInGPUProcessEnabled())
-        return makeUnique<MediaRecorderPrivate>(stream, options);
+        return MediaRecorderPrivate::create(stream, options);
 #endif
     return WebCore::MediaRecorderProvider::createMediaRecorderPrivate(stream, options);
 }

Source/WebKit/WebProcess/GPU/webrtc/MediaRecorderProvider.h

@@ -37,7 +37,7 @@ class MediaRecorderProvider final : public WebCore::MediaRecorderProvider {
 
 private:
 #if ENABLE(MEDIA_STREAM) && PLATFORM(COCOA)
-    std::unique_ptr<WebCore::MediaRecorderPrivate> createMediaRecorderPrivate(WebCore::MediaStreamPrivate&, const WebCore::MediaRecorderPrivateOptions&) final;
+    RefPtr<WebCore::MediaRecorderPrivate> createMediaRecorderPrivate(WebCore::MediaStreamPrivate&, const WebCore::MediaRecorderPrivateOptions&) final;
 #endif
 
 #if ENABLE(MEDIA_STREAM) && PLATFORM(COCOA) && ENABLE(GPU_PROCESS)