Address safer CPP failures in FetchBodyOwner

https://bugs.webkit.org/show_bug.cgi?id=290291

Reviewed by Geoffrey Garen.

* Source/WebCore/Modules/fetch/FetchBody.h:
(WebCore::FetchBody::protectedReadableStream const):
(WebCore::FetchBody::protectedReadableStream):
* Source/WebCore/Modules/fetch/FetchBodyOwner.cpp:
(WebCore::FetchBodyOwner::~FetchBodyOwner):
(WebCore::FetchBodyOwner::isDisturbed const):
(WebCore::FetchBodyOwner::isDisturbedOrLocked const):
(WebCore::FetchBodyOwner::formData):
(WebCore::FetchBodyOwner::loadBlob):
(WebCore::FetchBodyOwner::blobLoadingSucceeded):
(WebCore::FetchBodyOwner::blobLoadingFailed):
(WebCore::FetchBodyOwner::blobChunk):
(WebCore::FetchBodyOwner::BlobLoader::didFail):
(WebCore::FetchBodyOwner::BlobLoader::didSucceed):
(WebCore::FetchBodyOwner::createReadableStream):
(WebCore::FetchBodyOwner::consumeBodyAsStream):
* Source/WebCore/Modules/fetch/FetchBodyOwner.h:
* Source/WebCore/Modules/fetch/FetchResponse.cpp:
(WebCore::FetchResponse::create):
(WebCore::FetchResponse::clone):
* Source/WebCore/Modules/fetch/FetchResponse.h:
* Source/WebCore/SaferCPPExpectations/NoUncountedMemberCheckerExpectations:
* Source/WebCore/SaferCPPExpectations/UncountedCallArgsCheckerExpectations:

Canonical link: https://commits.webkit.org/292575@main

Author: cdumez

Source/WebCore/Modules/fetch/FetchBody.h

@@ -89,6 +89,8 @@ class FetchBody {
     bool hasReadableStream() const { return !!m_readableStream; }
     const ReadableStream* readableStream() const { return m_readableStream.get(); }
     ReadableStream* readableStream() { return m_readableStream.get(); }
+    RefPtr<const ReadableStream> protectedReadableStream() const { return readableStream(); }
+    RefPtr<ReadableStream> protectedReadableStream() { return readableStream(); }
     void setReadableStream(Ref<ReadableStream>&& stream)
     {
         ASSERT(!m_readableStream);

Source/WebCore/Modules/fetch/FetchBodyOwner.cpp

@@ -56,8 +56,8 @@ FetchBodyOwner::FetchBodyOwner(ScriptExecutionContext* context, std::optional<Fe
 
 FetchBodyOwner::~FetchBodyOwner()
 {
-    if (m_readableStreamSource)
-        m_readableStreamSource->detach();
+    if (RefPtr readableStreamSource = m_readableStreamSource)
+        readableStreamSource->detach();
 }
 
 void FetchBodyOwner::stop()
@@ -83,8 +83,8 @@ bool FetchBodyOwner::isDisturbed() const
     if (m_isDisturbed)
         return true;
 
-    if (body().readableStream())
-        return body().readableStream()->isDisturbed();
+    if (RefPtr readableStream = body().readableStream())
+        return readableStream->isDisturbed();
 
     return false;
 }
@@ -97,8 +97,8 @@ bool FetchBodyOwner::isDisturbedOrLocked() const
     if (m_isDisturbed)
         return true;
 
-    if (body().readableStream())
-        return body().readableStream()->isDisturbed() || body().readableStream()->isLocked();
+    if (RefPtr readableStream = body().readableStream())
+        return readableStream->isDisturbed() || readableStream->isLocked();
 
     return false;
 }
@@ -211,7 +211,7 @@ void FetchBodyOwner::formData(Ref<DeferredPromise>&& promise)
     if (isBodyNullOrOpaque()) {
         if (isBodyNull()) {
             // If the content-type is 'application/x-www-form-urlencoded', a body is not required and we should package an empty byte sequence as per the specification.
-            if (auto formData = FetchBodyConsumer::packageFormData(promise->scriptExecutionContext(), contentType(), { })) {
+            if (auto formData = FetchBodyConsumer::packageFormData(promise->protectedScriptExecutionContext().get(), contentType(), { })) {
                 promise->resolve<IDLInterface<DOMFormData>>(*formData);
                 return;
             }
@@ -277,7 +277,7 @@ void FetchBodyOwner::loadBlob(const Blob& blob, FetchBodyConsumer* consumer)
     m_blobLoader.emplace(*this);
     m_blobLoader->loader = makeUnique<FetchLoader>(*m_blobLoader, consumer);
 
-    m_blobLoader->loader->start(*scriptExecutionContext(), blob);
+    m_blobLoader->loader->start(*protectedScriptExecutionContext(), blob);
     if (!m_blobLoader->loader->isStarted()) {
         m_body->loadingFailed(Exception { ExceptionCode::TypeError, "Blob loading failed"_s });
         m_blobLoader = std::nullopt;
@@ -295,10 +295,8 @@ void FetchBodyOwner::finishBlobLoading()
 void FetchBodyOwner::blobLoadingSucceeded()
 {
     ASSERT(!isBodyNull());
-    if (m_readableStreamSource) {
-        m_readableStreamSource->close();
-        m_readableStreamSource = nullptr;
-    }
+    if (RefPtr readableStreamSource = std::exchange(m_readableStreamSource, nullptr))
+        readableStreamSource->close();
 
     m_body->loadingSucceeded(contentType());
     if (!m_blobLoader)
@@ -310,19 +308,19 @@ void FetchBodyOwner::blobLoadingSucceeded()
 void FetchBodyOwner::blobLoadingFailed()
 {
     ASSERT(!isBodyNull());
-    if (m_readableStreamSource) {
-        if (!m_readableStreamSource->isCancelling())
-            m_readableStreamSource->error(Exception { ExceptionCode::TypeError, "Blob loading failed"_s });
-        m_readableStreamSource = nullptr;
+    if (RefPtr readableStreamSource = std::exchange(m_readableStreamSource, nullptr)) {
+        if (!readableStreamSource->isCancelling())
+            readableStreamSource->error(Exception { ExceptionCode::TypeError, "Blob loading failed"_s });
     } else
         m_body->loadingFailed(Exception { ExceptionCode::TypeError, "Blob loading failed"_s });
     finishBlobLoading();
 }
 
 void FetchBodyOwner::blobChunk(const SharedBuffer& buffer)
 {
-    ASSERT(m_readableStreamSource);
-    if (!m_readableStreamSource->enqueue(buffer.tryCreateArrayBuffer()))
+    RefPtr readableStreamSource = m_readableStreamSource;
+    ASSERT(readableStreamSource);
+    if (!readableStreamSource->enqueue(buffer.tryCreateArrayBuffer()))
         stop();
 }
 
@@ -341,13 +339,12 @@ void FetchBodyOwner::BlobLoader::didFail(const ResourceError&)
 {
     // didFail might be called within FetchLoader::start call.
     if (loader->isStarted())
-        owner.blobLoadingFailed();
+        protectedOwner()->blobLoadingFailed();
 }
 
 void FetchBodyOwner::BlobLoader::didSucceed(const NetworkLoadMetrics&)
 {
-    Ref protectedOwner = Ref { owner };
-    protectedOwner->blobLoadingSucceeded();
+    protectedOwner()->blobLoadingSucceeded();
 }
 
 ExceptionOr<RefPtr<ReadableStream>> FetchBodyOwner::readableStream(JSC::JSGlobalObject& state)
@@ -372,7 +369,7 @@ ExceptionOr<void> FetchBodyOwner::createReadableStream(JSC::JSGlobalObject& stat
         if (UNLIKELY(streamOrException.hasException()))
             return streamOrException.releaseException();
         m_body->setReadableStream(streamOrException.releaseReturnValue());
-        m_body->readableStream()->lock();
+        m_body->protectedReadableStream()->lock();
         return { };
     }
 
@@ -388,15 +385,16 @@ ExceptionOr<void> FetchBodyOwner::createReadableStream(JSC::JSGlobalObject& stat
 
 void FetchBodyOwner::consumeBodyAsStream()
 {
-    ASSERT(m_readableStreamSource);
+    RefPtr readableStreamSource = m_readableStreamSource;
+    ASSERT(readableStreamSource);
 
     if (auto exception = loadingException()) {
-        m_readableStreamSource->error(*exception);
+        readableStreamSource->error(*exception);
         return;
     }
 
-    body().consumeAsStream(*this, *m_readableStreamSource);
-    if (!m_readableStreamSource->isPulling())
+    body().consumeAsStream(*this, *readableStreamSource);
+    if (!readableStreamSource->isPulling())
         m_readableStreamSource = nullptr;
 }
 

Source/WebCore/Modules/fetch/FetchBodyOwner.h

@@ -126,19 +126,21 @@ class FetchBodyOwner : public RefCountedAndCanMakeWeakPtr<FetchBodyOwner>, publi
 
         // FetchLoaderClient API
         void didReceiveResponse(const ResourceResponse&) final;
-        void didReceiveData(const SharedBuffer& buffer) final { owner.blobChunk(buffer); }
+        void didReceiveData(const SharedBuffer& buffer) final { protectedOwner()->blobChunk(buffer); }
         void didFail(const ResourceError&) final;
         void didSucceed(const NetworkLoadMetrics&) final;
 
-        FetchBodyOwner& owner;
+        Ref<FetchBodyOwner> protectedOwner() const { return owner.get(); }
+
+        WeakRef<FetchBodyOwner> owner;
         std::unique_ptr<FetchLoader> loader;
     };
 
 protected:
     std::optional<FetchBody> m_body;
     bool m_isDisturbed { false };
     RefPtr<FetchBodySource> m_readableStreamSource;
-    Ref<FetchHeaders> m_headers;
+    const Ref<FetchHeaders> m_headers;
 
 private:
     std::optional<BlobLoader> m_blobLoader;

Source/WebCore/Modules/fetch/FetchResponse.cpp

@@ -56,12 +56,18 @@ static inline bool isNullBodyStatus(int status)
 FetchResponse::~FetchResponse() = default;
 
 Ref<FetchResponse> FetchResponse::create(ScriptExecutionContext* context, std::optional<FetchBody>&& body, FetchHeaders::Guard guard, ResourceResponse&& response)
+{
+    bool isOpaque = response.tainting() == ResourceResponse::Tainting::Opaque;
+    Ref headers = isOpaque ? FetchHeaders::create(guard) : FetchHeaders::create(guard, HTTPHeaderMap { response.httpHeaderFields() });
+    return FetchResponse::create(context, WTFMove(body), WTFMove(headers), WTFMove(response));
+}
+
+Ref<FetchResponse> FetchResponse::create(ScriptExecutionContext* context, std::optional<FetchBody>&& body, Ref<FetchHeaders>&& headers, ResourceResponse&& response)
 {
     bool isSynthetic = response.type() == ResourceResponse::Type::Default || response.type() == ResourceResponse::Type::Error;
     bool isOpaque = response.tainting() == ResourceResponse::Tainting::Opaque;
-    auto headers = isOpaque ? FetchHeaders::create(guard) : FetchHeaders::create(guard, HTTPHeaderMap { response.httpHeaderFields() });
 
-    auto fetchResponse = adoptRef(*new FetchResponse(context, WTFMove(body), WTFMove(headers), WTFMove(response)));
+    Ref fetchResponse = adoptRef(*new FetchResponse(context, WTFMove(body), WTFMove(headers), WTFMove(response)));
     fetchResponse->suspendIfNeeded();
     if (!isSynthetic)
         fetchResponse->m_filteredResponse = ResourceResponseBase::filter(fetchResponse->m_internalResponse, ResourceResponse::PerformExposeAllHeadersCheck::Yes);
@@ -211,9 +217,9 @@ ExceptionOr<Ref<FetchResponse>> FetchResponse::clone()
     if (m_internalResponse.type() == ResourceResponse::Type::Default)
         m_internalResponse.setHTTPHeaderFields(HTTPHeaderMap { headers().internalHeaders() });
 
-    auto clone = FetchResponse::create(scriptExecutionContext(), std::nullopt, headers().guard(), ResourceResponse { m_internalResponse });
+    Ref headers = FetchHeaders::create(this->headers());
+    auto clone = FetchResponse::create(scriptExecutionContext(), std::nullopt, WTFMove(headers), ResourceResponse { m_internalResponse });
     clone->cloneBody(*this);
-    clone->m_headers = FetchHeaders::create(headers());
     clone->m_opaqueLoadIdentifier = m_opaqueLoadIdentifier;
     clone->m_bodySizeWithPadding = m_bodySizeWithPadding;
     return clone;

Source/WebCore/Modules/fetch/FetchResponse.h

@@ -63,6 +63,7 @@ class FetchResponse final : public FetchBodyOwner {
     virtual ~FetchResponse();
 
     WEBCORE_EXPORT static Ref<FetchResponse> create(ScriptExecutionContext*, std::optional<FetchBody>&&, FetchHeaders::Guard, ResourceResponse&&);
+    static Ref<FetchResponse> create(ScriptExecutionContext*, std::optional<FetchBody>&&, Ref<FetchHeaders>&&, ResourceResponse&&);
 
     static ExceptionOr<Ref<FetchResponse>> create(ScriptExecutionContext&, std::optional<FetchBody::Init>&&, Init&&);
     static ExceptionOr<Ref<FetchResponse>> create(ScriptExecutionContext&, std::optional<FetchBodyWithType>&&, Init&&);

Source/WebCore/SaferCPPExpectations/NoUncountedMemberCheckerExpectations

@@ -1,6 +1,5 @@
 Modules/WebGPU/GPUPresentationContextDescriptor.h
 Modules/encryptedmedia/MediaKeyStatusMap.h
-Modules/fetch/FetchBodyOwner.h
 Modules/mediastream/RTCPeerConnection.h
 Modules/mediastream/libwebrtc/LibWebRTCObservers.h
 Modules/notifications/NotificationResourcesLoader.h

Source/WebCore/SaferCPPExpectations/UncountedCallArgsCheckerExpectations

@@ -58,8 +58,6 @@ Modules/entriesapi/FileSystemEntry.cpp
 Modules/entriesapi/FileSystemFileEntry.cpp
 Modules/fetch/FetchBody.cpp
 Modules/fetch/FetchBodyConsumer.cpp
-Modules/fetch/FetchBodyOwner.cpp
-Modules/fetch/FetchBodyOwner.h
 Modules/fetch/FetchBodySource.cpp
 Modules/fetch/FetchLoader.cpp
 Modules/fetch/FetchRequest.cpp