Attachment 373305 Details for Bug 199380 – Patch

[patch] Patch

bug-199380-20190702085150.patch (text/plain), 4.13 KB, created by Zan Dobersek on 2019-07-01 23:51:52 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Zan Dobersek

Created: 2019-07-01 23:51:52 PDT

Size: 4.13 KB

patch

obsolete

>Subversion Revision: 247036
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index da5999f8b3ebad6dba95f30bf323974ec4118843..f7148e099d6d490966623c12bfb152c4c8083732 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2019-07-01  Zan Dobersek  <zdobersek@igalia.com>
>+
>+        FetchResponse::BodyLoader should not be movable
>+        https://bugs.webkit.org/show_bug.cgi?id=199380
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The FetchResponse::BodyLoader class has a FetchLoader member that is
>+        initialized in the start() method with the reference of the owning
>+        FetchResponse::BodyLoader object. This reference doesn't change when
>+        the FetchResponse::BodyLoader object is moved into a different object
>+        and the FetchLoader unique_ptr along with it, leading to problems when
>+        that FetchLoader tries to invoke the FetchLoaderClient methods on the
>+        FetchResponse::BodyLoader object that's been moved from and is possibly
>+        already destroyed.
>+
>+        To avoid this, the FetchResponse::BodyLoader has the move constructor
>+        removed and is now managed through std::unique_ptr instead of Optional,
>+        ensuring the FetchResponse::BodyLoader object itself isn't moved around.
>+
>+        * Modules/fetch/FetchResponse.cpp:
>+        (WebCore::FetchResponse::fetch):
>+        (WebCore::FetchResponse::BodyLoader::didSucceed):
>+        (WebCore::FetchResponse::BodyLoader::didFail):
>+        * Modules/fetch/FetchResponse.h:
>+
> 2019-07-01  Zalan Bujtas  <zalan@apple.com>
> 
>         [Text autosizing] [iPadOS] AutosizeStatus::idempotentTextSize returns the computed font size in certain cases.
>diff --git a/Source/WebCore/Modules/fetch/FetchResponse.cpp b/Source/WebCore/Modules/fetch/FetchResponse.cpp
>index 4fe7616a49b6d12f6a841a33bfa31c8d9b0bb26b..d7696072965f45466c6d7106d738e3d705bec5d0 100644
>--- a/Source/WebCore/Modules/fetch/FetchResponse.cpp
>+++ b/Source/WebCore/Modules/fetch/FetchResponse.cpp
>@@ -240,9 +240,9 @@ void FetchResponse::fetch(ScriptExecutionContext& context, FetchRequest& request
> 
>     response->addAbortSteps(request.signal());
> 
>-    response->m_bodyLoader.emplace(response.get(), WTFMove(responseCallback));
>+    response->m_bodyLoader = std::make_unique<BodyLoader>(response.get(), WTFMove(responseCallback));
>     if (!response->m_bodyLoader->start(context, request))
>-        response->m_bodyLoader = WTF::nullopt;
>+        response->m_bodyLoader = nullptr;
> }
> 
> const String& FetchResponse::url() const
>@@ -280,7 +280,7 @@ void FetchResponse::BodyLoader::didSucceed()
> 
>     if (m_loader->isStarted()) {
>         Ref<FetchResponse> protector(m_response);
>-        m_response.m_bodyLoader = WTF::nullopt;
>+        m_response.m_bodyLoader = nullptr;
>     }
> }
> 
>@@ -307,7 +307,7 @@ void FetchResponse::BodyLoader::didFail(const ResourceError& error)
>     // Check whether didFail is called as part of FetchLoader::start.
>     if (m_loader && m_loader->isStarted()) {
>         Ref<FetchResponse> protector(m_response);
>-        m_response.m_bodyLoader = WTF::nullopt;
>+        m_response.m_bodyLoader = nullptr;
>     }
> }
> 
>diff --git a/Source/WebCore/Modules/fetch/FetchResponse.h b/Source/WebCore/Modules/fetch/FetchResponse.h
>index 74c1454f9aaf31241816c51e46e0f04e70ae1322..5c83a37a1b837ce3175f749c0e1753b594b52d26 100644
>--- a/Source/WebCore/Modules/fetch/FetchResponse.h
>+++ b/Source/WebCore/Modules/fetch/FetchResponse.h
>@@ -128,9 +128,9 @@ private:
>     void addAbortSteps(Ref<AbortSignal>&&);
> 
>     class BodyLoader final : public FetchLoaderClient {
>+        WTF_MAKE_FAST_ALLOCATED;
>     public:
>         BodyLoader(FetchResponse&, NotificationCallback&&);
>-        BodyLoader(BodyLoader&&) = default;
>         ~BodyLoader();
> 
>         bool start(ScriptExecutionContext&, const FetchRequest&);
>@@ -160,7 +160,7 @@ private:
> 
>     mutable Optional<ResourceResponse> m_filteredResponse;
>     ResourceResponse m_internalResponse;
>-    Optional<BodyLoader> m_bodyLoader;
>+    std::unique_ptr<BodyLoader> m_bodyLoader;
>     mutable String m_responseURL;
>     // Opaque responses will padd their body size when used with Cache API.
>     uint64_t m_bodySizeWithPadding { 0 };

Actions: View | Formatted Diff | Diff

Attachments on bug 199380: 373265 | 373305