Changeset 287532 in webkit


Ignore:
Timestamp:
Jan 3, 2022 12:17:59 AM (7 months ago)
Author:
youenn@apple.com
Message:

FetchRequest.clone does not need to be called with the current context
​https://bugs.webkit.org/show_bug.cgi?id=234515

Reviewed by Darin Adler.

Source/WebCore:

Make FetchRequest, FetchResponse and FetchBodyOwner take a ScriptExecutionContext* instead of a ScriptExecutionContext&.
This allows cloning FetchRequest and FetchResponse with the context of the original request or response.

Update call site, as well as for AbortSignal.

For FetchResponse, we did a change to throw in case of cloning FetchResponse on a stopped context.
It appeared that Firefox is not doing that, and Chrome is only doing that in some specific cases.
For that reason, it is better to go back to our previous behavior of not throwing.

To ease testing, we add an internals API to check whether a request/response is linked to a closed context.

Covered by updated tests.

  • Modules/cache/DOMCache.cpp:
  • Modules/fetch/FetchBodyOwner.cpp:
  • Modules/fetch/FetchBodyOwner.h:
  • Modules/fetch/FetchRequest.cpp:
  • Modules/fetch/FetchRequest.h:
  • Modules/fetch/FetchRequest.idl:
  • Modules/fetch/FetchResponse.cpp:
  • Modules/fetch/FetchResponse.h:
  • dom/AbortController.cpp:
  • dom/AbortSignal.cpp:
  • dom/AbortSignal.h:
  • testing/Internals.cpp:
  • testing/Internals.h:
  • testing/Internals.idl:
  • testing/ServiceWorkerInternals.cpp:

LayoutTests:

  • http/wpt/fetch/clone-realm-expected.txt:
  • http/wpt/fetch/clone-realm.html:
  • http/wpt/fetch/resources/clone-realm-iframe.html: Added.
Location:
trunk
Files:
1 added
19 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r287530 r287532  
     12022-01-03  Youenn Fablet  <youenn@apple.com>
     2
     3        FetchRequest.clone does not need to be called with the current context
     4        https://bugs.webkit.org/show_bug.cgi?id=234515
     5
     6        Reviewed by Darin Adler.
     7
     8        * http/wpt/fetch/clone-realm-expected.txt:
     9        * http/wpt/fetch/clone-realm.html:
     10        * http/wpt/fetch/resources/clone-realm-iframe.html: Added.
     11
    1122022-01-02  Diego Pino Garcia  <dpino@igalia.com>
    213

  • trunk/LayoutTests/http/wpt/fetch/clone-realm-expected.txt

    r286970 r287532  
    11
    2 PASS Cloning a response fails when its frame is detached
     2PASS Cloning a response succeeds even if its body is stream based
     3PASS Cloning request/response when its frame is detached
    34

  • trunk/LayoutTests/http/wpt/fetch/clone-realm.html

    r286970 r287532  
    1616    });
    1717}
     18
    1819promise_test(async (t) => {
    1920    const frame = await with_iframe('/');
    … …  
    2122    await response.clone().text();
    2223    frame.remove();
    23     try {
    24         response.clone();
    25         assert_not_reached();
    26     } catch (e) {
    27         assert_equals(e.name, 'InvalidStateError');
     24    response.clone();
     25}, "Cloning a response succeeds even if its body is stream based");
     26
     27promise_test(async (t) => {
     28    let frame = await with_iframe('/WebKit/fetch/resources/clone-realm-iframe.html');
     29
     30    if (window.internals) {
     31        assert_false(internals.isFetchObjectContextStopped(testResponse), "response before");
     32        assert_false(internals.isFetchObjectContextStopped(testRequest), "request before");
    2833    }
    29 }, "Cloning a response fails when its frame is detached");
     34
     35    frame.remove();
     36
     37    const clonedResponse = testResponse.clone();
     38    const clonedRequest = testRequest.clone();
     39
     40    if (window.internals) {
     41        assert_true(internals.isFetchObjectContextStopped(testResponse), "response after");
     42        assert_true(internals.isFetchObjectContextStopped(clonedResponse), "clonedResponse");
     43        assert_true(internals.isFetchObjectContextStopped(testRequest), "request after");
     44        assert_true(internals.isFetchObjectContextStopped(clonedRequest), "clonedRequest");
     45    }
     46}, "Cloning request/response when its frame is detached");
     47
    3048</script>
    3149    </body>

  • trunk/Source/WebCore/ChangeLog

    r287529 r287532  
     12022-01-03  Youenn Fablet  <youenn@apple.com>
     2
     3        FetchRequest.clone does not need to be called with the current context
     4        https://bugs.webkit.org/show_bug.cgi?id=234515
     5
     6        Reviewed by Darin Adler.
     7
     8        Make FetchRequest, FetchResponse and FetchBodyOwner take a ScriptExecutionContext* instead of a ScriptExecutionContext&.
     9        This allows cloning FetchRequest and FetchResponse with the context of the original request or response.
     10
     11        Update call site, as well as for AbortSignal.
     12
     13        For FetchResponse, we did a change to throw in case of cloning FetchResponse on a stopped context.
     14        It appeared that Firefox is not doing that, and Chrome is only doing that in some specific cases.
     15        For that reason, it is better to go back to our previous behavior of not throwing.
     16
     17        To ease testing, we add an internals API to check whether a request/response is linked to a closed context.
     18
     19        Covered by updated tests.
     20
     21        * Modules/cache/DOMCache.cpp:
     22        * Modules/fetch/FetchBodyOwner.cpp:
     23        * Modules/fetch/FetchBodyOwner.h:
     24        * Modules/fetch/FetchRequest.cpp:
     25        * Modules/fetch/FetchRequest.h:
     26        * Modules/fetch/FetchRequest.idl:
     27        * Modules/fetch/FetchResponse.cpp:
     28        * Modules/fetch/FetchResponse.h:
     29        * dom/AbortController.cpp:
     30        * dom/AbortSignal.cpp:
     31        * dom/AbortSignal.h:
     32        * testing/Internals.cpp:
     33        * testing/Internals.h:
     34        * testing/Internals.idl:
     35        * testing/ServiceWorkerInternals.cpp:
     36
    1372022-01-02  Manuel Rego Casasnovas  <rego@igalia.com>
    238

  • trunk/Source/WebCore/Modules/cache/DOMCache.cpp

    r287066 r287532  
    8484    auto resourceResponse = record.response;
    8585    resourceResponse.setSource(ResourceResponse::Source::DOMCache);
    86     auto response = FetchResponse::create(context, std::nullopt, record.responseHeadersGuard, WTFMove(resourceResponse));
     86    auto response = FetchResponse::create(&context, std::nullopt, record.responseHeadersGuard, WTFMove(resourceResponse));
    8787    response->setBodyData(copyResponseBody(record.responseBody), record.responseBodySize);
    8888    return response;

  • trunk/Source/WebCore/Modules/fetch/FetchBodyOwner.cpp

    r287177 r287532  
    4242namespace WebCore {
    4343
    44 FetchBodyOwner::FetchBodyOwner(ScriptExecutionContext& context, std::optional<FetchBody>&& body, Ref<FetchHeaders>&& headers)
    45     : ActiveDOMObject(&context)
     44FetchBodyOwner::FetchBodyOwner(ScriptExecutionContext* context, std::optional<FetchBody>&& body, Ref<FetchHeaders>&& headers)
     45    : ActiveDOMObject(context)
    4646    , m_body(WTFMove(body))
    4747    , m_headers(WTFMove(headers))

  • trunk/Source/WebCore/Modules/fetch/FetchBodyOwner.h

    r287177 r287532  
    7373
    7474protected:
    75     FetchBodyOwner(ScriptExecutionContext&, std::optional<FetchBody>&&, Ref<FetchHeaders>&&);
     75    FetchBodyOwner(ScriptExecutionContext*, std::optional<FetchBody>&&, Ref<FetchHeaders>&&);
    7676
    7777    const FetchBody& body() const { return *m_body; }

  • trunk/Source/WebCore/Modules/fetch/FetchRequest.cpp

    r287177 r287532  
    275275ExceptionOr<Ref<FetchRequest>> FetchRequest::create(ScriptExecutionContext& context, Info&& input, Init&& init)
    276276{
    277     auto request = adoptRef(*new FetchRequest(context, std::nullopt, FetchHeaders::create(FetchHeaders::Guard::Request), { }, { }, { }));
     277    auto request = adoptRef(*new FetchRequest(&context, std::nullopt, FetchHeaders::create(FetchHeaders::Guard::Request), { }, { }, { }));
    278278    request->suspendIfNeeded();
    279279
    … …  
    293293Ref<FetchRequest> FetchRequest::create(ScriptExecutionContext& context, std::optional<FetchBody>&& body, Ref<FetchHeaders>&& headers, ResourceRequest&& request, FetchOptions&& options, String&& referrer)
    294294{
    295     auto result = adoptRef(*new FetchRequest(context, WTFMove(body), WTFMove(headers), WTFMove(request), WTFMove(options), WTFMove(referrer)));
     295    auto result = adoptRef(*new FetchRequest(&context, WTFMove(body), WTFMove(headers), WTFMove(request), WTFMove(options), WTFMove(referrer)));
    296296    result->suspendIfNeeded();
    297297    return result;
    … …  
    327327}
    328328
    329 ExceptionOr<Ref<FetchRequest>> FetchRequest::clone(ScriptExecutionContext& context)
     329ExceptionOr<Ref<FetchRequest>> FetchRequest::clone()
    330330{
    331331    if (isDisturbedOrLocked())
    332332        return Exception { TypeError, "Body is disturbed or locked"_s };
    333333
    334     auto clone = adoptRef(*new FetchRequest(context, std::nullopt, FetchHeaders::create(m_headers.get()), ResourceRequest { m_request }, FetchOptions { m_options }, String { m_referrer }));
     334    auto clone = adoptRef(*new FetchRequest(scriptExecutionContext(), std::nullopt, FetchHeaders::create(m_headers.get()), ResourceRequest { m_request }, FetchOptions { m_options }, String { m_referrer }));
    335335    clone->suspendIfNeeded();
    336336    clone->cloneBody(*this);

  • trunk/Source/WebCore/Modules/fetch/FetchRequest.h

    r287177 r287532  
    7474    const String& integrity() const { return m_options.integrity; }
    7575
    76     ExceptionOr<Ref<FetchRequest>> clone(ScriptExecutionContext&);
     76    ExceptionOr<Ref<FetchRequest>> clone();
    7777
    7878    const FetchOptions& fetchOptions() const { return m_options; }
    … …  
    8686
    8787private:
    88     FetchRequest(ScriptExecutionContext&, std::optional<FetchBody>&&, Ref<FetchHeaders>&&, ResourceRequest&&, FetchOptions&&, String&& referrer);
     88    FetchRequest(ScriptExecutionContext*, std::optional<FetchBody>&&, Ref<FetchHeaders>&&, ResourceRequest&&, FetchOptions&&, String&& referrer);
    8989
    9090    ExceptionOr<void> initializeOptions(const Init&);
    … …  
    104104};
    105105
    106 inline FetchRequest::FetchRequest(ScriptExecutionContext& context, std::optional<FetchBody>&& body, Ref<FetchHeaders>&& headers, ResourceRequest&& request, FetchOptions&& options, String&& referrer)
     106inline FetchRequest::FetchRequest(ScriptExecutionContext* context, std::optional<FetchBody>&& body, Ref<FetchHeaders>&& headers, ResourceRequest&& request, FetchOptions&& options, String&& referrer)
    107107    : FetchBodyOwner(context, WTFMove(body), WTFMove(headers))
    108108    , m_request(WTFMove(request))

  • trunk/Source/WebCore/Modules/fetch/FetchRequest.idl

    r283463 r287532  
    3333[
    3434    ActiveDOMObject,
     35    ExportMacro=WEBCORE_EXPORT,
    3536    Exposed=(Window,Worker),
    3637    GenerateIsReachable=Impl,
    … …  
    5859    readonly attribute AbortSignal signal;
    5960
    60     [CallWith=ScriptExecutionContext, NewObject] FetchRequest clone();
     61    [NewObject] FetchRequest clone();
    6162};
    6263

  • trunk/Source/WebCore/Modules/fetch/FetchResponse.cpp

    r287177 r287532  
    4949}
    5050
    51 Ref<FetchResponse> FetchResponse::create(ScriptExecutionContext& context, std::optional<FetchBody>&& body, FetchHeaders::Guard guard, ResourceResponse&& response)
     51Ref<FetchResponse> FetchResponse::create(ScriptExecutionContext* context, std::optional<FetchBody>&& body, FetchHeaders::Guard guard, ResourceResponse&& response)
    5252{
    5353    bool isSynthetic = response.type() == ResourceResponse::Type::Default || response.type() == ResourceResponse::Type::Error;
    … …  
    128128   
    129129    // 12. Return r.
    130     auto r = adoptRef(*new FetchResponse(context, WTFMove(extractedBody), WTFMove(headers), { }));
     130    auto r = adoptRef(*new FetchResponse(&context, WTFMove(extractedBody), WTFMove(headers), { }));
    131131    r->suspendIfNeeded();
    132132
    … …  
    144144Ref<FetchResponse> FetchResponse::error(ScriptExecutionContext& context)
    145145{
    146     auto response = adoptRef(*new FetchResponse(context, { }, FetchHeaders::create(FetchHeaders::Guard::Immutable), { }));
     146    auto response = adoptRef(*new FetchResponse(&context, { }, FetchHeaders::create(FetchHeaders::Guard::Immutable), { }));
    147147    response->suspendIfNeeded();
    148148    response->m_internalResponse.setType(Type::Error);
    … …  
    160160    if (!ResourceResponse::isRedirectionStatusCode(status))
    161161        return Exception { RangeError, makeString("Status code ", status, "is not a redirection status code") };
    162     auto redirectResponse = adoptRef(*new FetchResponse(context, { }, FetchHeaders::create(FetchHeaders::Guard::Immutable), { }));
     162    auto redirectResponse = adoptRef(*new FetchResponse(&context, { }, FetchHeaders::create(FetchHeaders::Guard::Immutable), { }));
    163163    redirectResponse->suspendIfNeeded();
    164164    redirectResponse->m_internalResponse.setHTTPStatusCode(status);
    … …  
    168168}
    169169
    170 FetchResponse::FetchResponse(ScriptExecutionContext& context, std::optional<FetchBody>&& body, Ref<FetchHeaders>&& headers, ResourceResponse&& response)
     170FetchResponse::FetchResponse(ScriptExecutionContext* context, std::optional<FetchBody>&& body, Ref<FetchHeaders>&& headers, ResourceResponse&& response)
    171171    : FetchBodyOwner(context, WTFMove(body), WTFMove(headers))
    172172    , m_internalResponse(WTFMove(response))
    … …  
    176176ExceptionOr<Ref<FetchResponse>> FetchResponse::clone()
    177177{
    178     if (isContextStopped())
    179         return Exception { InvalidStateError, "Context is stopped"_s };
    180 
    181178    if (isDisturbedOrLocked())
    182179        return Exception { TypeError, "Body is disturbed or locked"_s };
    183180
    184     ASSERT(scriptExecutionContext());
    185     auto& context = *scriptExecutionContext();
    186 
    187181    // If loading, let's create a stream so that data is teed on both clones.
    188182    if (isLoading() && !m_readableStreamSource) {
    189         auto* globalObject = context.globalObject();
     183        auto* context = scriptExecutionContext();
     184
     185        auto* globalObject = context ? context->globalObject() : nullptr;
    190186        if (!globalObject)
    191187            return Exception { InvalidStateError, "Context is stopped"_s };
    … …  
    200196        m_internalResponse.setHTTPHeaderFields(HTTPHeaderMap { headers().internalHeaders() });
    201197
    202     auto clone = FetchResponse::create(context, std::nullopt, headers().guard(), ResourceResponse { m_internalResponse });
     198    auto clone = FetchResponse::create(scriptExecutionContext(), std::nullopt, headers().guard(), ResourceResponse { m_internalResponse });
    203199    clone->cloneBody(*this);
    204200    clone->m_opaqueLoadIdentifier = m_opaqueLoadIdentifier;
    … …  
    255251    InspectorInstrumentation::willFetch(context, request.url().string());
    256252
    257     auto response = adoptRef(*new FetchResponse(context, FetchBody { }, FetchHeaders::create(FetchHeaders::Guard::Immutable), { }));
     253    auto response = adoptRef(*new FetchResponse(&context, FetchBody { }, FetchHeaders::create(FetchHeaders::Guard::Immutable), { }));
    258254    response->suspendIfNeeded();
    259255

  • trunk/Source/WebCore/Modules/fetch/FetchResponse.h

    r287021 r287532  
    5858    };
    5959
    60     WEBCORE_EXPORT static Ref<FetchResponse> create(ScriptExecutionContext&, std::optional<FetchBody>&&, FetchHeaders::Guard, ResourceResponse&&);
     60    WEBCORE_EXPORT static Ref<FetchResponse> create(ScriptExecutionContext*, std::optional<FetchBody>&&, FetchHeaders::Guard, ResourceResponse&&);
    6161
    6262    static ExceptionOr<Ref<FetchResponse>> create(ScriptExecutionContext&, std::optional<FetchBody::Init>&&, Init&&);
    … …  
    113113
    114114private:
    115     FetchResponse(ScriptExecutionContext&, std::optional<FetchBody>&&, Ref<FetchHeaders>&&, ResourceResponse&&);
     115    FetchResponse(ScriptExecutionContext*, std::optional<FetchBody>&&, Ref<FetchHeaders>&&, ResourceResponse&&);
    116116
    117117    void stop() final;

  • trunk/Source/WebCore/dom/AbortController.cpp

    r285428 r287532  
    4242
    4343AbortController::AbortController(ScriptExecutionContext& context)
    44     : m_signal(AbortSignal::create(context))
     44    : m_signal(AbortSignal::create(&context))
    4545{
    4646}

  • trunk/Source/WebCore/dom/AbortSignal.cpp

    r286904 r287532  
    4040WTF_MAKE_ISO_ALLOCATED_IMPL(AbortSignal);
    4141
    42 Ref<AbortSignal> AbortSignal::create(ScriptExecutionContext& context)
     42Ref<AbortSignal> AbortSignal::create(ScriptExecutionContext* context)
    4343{
    4444    return adoptRef(*new AbortSignal(context));
    … …  
    5151    if (reason.isUndefined())
    5252        reason = toJS(&globalObject, &globalObject, DOMException::create(AbortError));
    53     return adoptRef(*new AbortSignal(context, Aborted::Yes, reason));
     53    return adoptRef(*new AbortSignal(&context, Aborted::Yes, reason));
    5454}
    5555
    56 AbortSignal::AbortSignal(ScriptExecutionContext& context, Aborted aborted, JSC::JSValue reason)
    57     : ContextDestructionObserver(&context)
     56AbortSignal::AbortSignal(ScriptExecutionContext* context, Aborted aborted, JSC::JSValue reason)
     57    : ContextDestructionObserver(context)
    5858    , m_aborted(aborted == Aborted::Yes)
    5959    , m_reason(reason)

  • trunk/Source/WebCore/dom/AbortSignal.h

    r286904 r287532  
    4343    WTF_MAKE_ISO_ALLOCATED_EXPORT(AbortSignal, WEBCORE_EXPORT);
    4444public:
    45     static Ref<AbortSignal> create(ScriptExecutionContext&);
     45    static Ref<AbortSignal> create(ScriptExecutionContext*);
    4646
    4747    static Ref<AbortSignal> abort(JSDOMGlobalObject&, ScriptExecutionContext&, JSC::JSValue reason);
    … …  
    6767private:
    6868    enum class Aborted : bool { No, Yes };
    69     explicit AbortSignal(ScriptExecutionContext&, Aborted = Aborted::No, JSC::JSValue reason = JSC::jsUndefined());
     69    explicit AbortSignal(ScriptExecutionContext*, Aborted = Aborted::No, JSC::JSValue reason = JSC::jsUndefined());
    7070
    7171    // EventTarget.

  • trunk/Source/WebCore/testing/Internals.cpp

    r287498 r287532  
    7979#include "ExtendableEvent.h"
    8080#include "ExtensionStyleSheets.h"
     81#include "FetchRequest.h"
    8182#include "FetchResponse.h"
    8283#include "File.h"
    … …  
    919920}
    920921
     922bool Internals::isFetchObjectContextStopped(const FetchObject& object)
     923{
     924    return switchOn(object, [](const RefPtr<FetchRequest>& request) {
     925        return request->isContextStopped();
     926    }, [](auto& response) {
     927        return response->isContextStopped();
     928    });
     929}
     930
    921931void Internals::clearMemoryCache()
    922932{

  • trunk/Source/WebCore/testing/Internals.h

    r287498 r287532  
    6969class EventListener;
    7070class ExtendableEvent;
     71class FetchRequest;
    7172class FetchResponse;
    7273class File;
    … …  
    191192    void setStrictRawResourceValidationPolicyDisabled(bool);
    192193
     194    using FetchObject = std::variant<RefPtr<FetchRequest>, RefPtr<FetchResponse>>;
     195    bool isFetchObjectContextStopped(const FetchObject&);
     196
    193197    void clearMemoryCache();
    194198    void pruneMemoryCacheToSize(unsigned size);

  • trunk/Source/WebCore/testing/Internals.idl

    r287404 r287532  
    306306};
    307307
     308typedef (FetchRequest or FetchResponse) FetchObject;
     309
    308310[
    309311    ExportMacro=WEBCORE_TESTSUPPORT_EXPORT,
    … …  
    337339    undefined setOverrideResourceLoadPriority(ResourceLoadPriority priority);
    338340    undefined setStrictRawResourceValidationPolicyDisabled(boolean disabled);
     341
     342    boolean isFetchObjectContextStopped(FetchObject object);
    339343
    340344    undefined clearBackForwardCache();

  • trunk/Source/WebCore/testing/ServiceWorkerInternals.cpp

    r287353 r287532  
    130130    response.setType(ResourceResponse::Type::Cors);
    131131    response.setTainting(ResourceResponse::Tainting::Opaque);
    132     auto fetchResponse = FetchResponse::create(context, FetchBody::fromFormData(context, formData), FetchHeaders::Guard::Response, WTFMove(response));
     132    auto fetchResponse = FetchResponse::create(&context, FetchBody::fromFormData(context, formData), FetchHeaders::Guard::Response, WTFMove(response));
    133133    fetchResponse->initializeOpaqueLoadIdentifierForTesting();
    134134    return fetchResponse;
Note: See TracChangeset for help on using the changeset viewer.