Title: [221397] trunk/Source/WebCore
Revision
221397
Author
commit-qu...@webkit.org
Date
2017-08-30 14:19:46 -0700 (Wed, 30 Aug 2017)

Log Message

Remove FetchRequest::InternalRequest
https://bugs.webkit.org/show_bug.cgi?id=176085

Patch by Youenn Fablet <you...@apple.com> on 2017-08-30
Reviewed by Alex Christensen.

No change of behavior.

Removing InternalRequest struct and passing/defining fields directly.

* Modules/cache/Cache.cpp:
(WebCore::Cache::updateRecords):
* Modules/fetch/FetchRequest.cpp:
(WebCore::computeReferrer):
(WebCore::buildOptions):
(WebCore::methodCanHaveBody):
(WebCore::FetchRequest::initializeOptions):
(WebCore::FetchRequest::initializeWith):
(WebCore::FetchRequest::setBody):
(WebCore::FetchRequest::create):
(WebCore::FetchRequest::referrer const):
(WebCore::FetchRequest::urlString const):
(WebCore::FetchRequest::resourceRequest const):
(WebCore::FetchRequest::clone):
(WebCore::setReferrer): Deleted.
* Modules/fetch/FetchRequest.h:
(WebCore::FetchRequest::FetchRequest):
(WebCore::FetchRequest::cache const):
(WebCore::FetchRequest::credentials const):
(WebCore::FetchRequest::destination const):
(WebCore::FetchRequest::mode const):
(WebCore::FetchRequest::redirect const):
(WebCore::FetchRequest::referrerPolicy const):
(WebCore::FetchRequest::type const):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (221396 => 221397)


--- trunk/Source/WebCore/ChangeLog	2017-08-30 20:45:43 UTC (rev 221396)
+++ trunk/Source/WebCore/ChangeLog	2017-08-30 21:19:46 UTC (rev 221397)
@@ -1,5 +1,41 @@
 2017-08-30  Youenn Fablet  <you...@apple.com>
 
+        Remove FetchRequest::InternalRequest
+        https://bugs.webkit.org/show_bug.cgi?id=176085
+
+        Reviewed by Alex Christensen.
+
+        No change of behavior.
+
+        Removing InternalRequest struct and passing/defining fields directly.
+
+        * Modules/cache/Cache.cpp:
+        (WebCore::Cache::updateRecords):
+        * Modules/fetch/FetchRequest.cpp:
+        (WebCore::computeReferrer):
+        (WebCore::buildOptions):
+        (WebCore::methodCanHaveBody):
+        (WebCore::FetchRequest::initializeOptions):
+        (WebCore::FetchRequest::initializeWith):
+        (WebCore::FetchRequest::setBody):
+        (WebCore::FetchRequest::create):
+        (WebCore::FetchRequest::referrer const):
+        (WebCore::FetchRequest::urlString const):
+        (WebCore::FetchRequest::resourceRequest const):
+        (WebCore::FetchRequest::clone):
+        (WebCore::setReferrer): Deleted.
+        * Modules/fetch/FetchRequest.h:
+        (WebCore::FetchRequest::FetchRequest):
+        (WebCore::FetchRequest::cache const):
+        (WebCore::FetchRequest::credentials const):
+        (WebCore::FetchRequest::destination const):
+        (WebCore::FetchRequest::mode const):
+        (WebCore::FetchRequest::redirect const):
+        (WebCore::FetchRequest::referrerPolicy const):
+        (WebCore::FetchRequest::type const):
+
+2017-08-30  Youenn Fablet  <you...@apple.com>
+
         Add support for FetchRequest.body
         https://bugs.webkit.org/show_bug.cgi?id=176066
         <rdar://problem/34148373>

Modified: trunk/Source/WebCore/Modules/cache/Cache.cpp (221396 => 221397)


--- trunk/Source/WebCore/Modules/cache/Cache.cpp	2017-08-30 20:45:43 UTC (rev 221396)
+++ trunk/Source/WebCore/Modules/cache/Cache.cpp	2017-08-30 21:19:46 UTC (rev 221397)
@@ -496,8 +496,7 @@
             newRecords.append(WTFMove(current));
         } else {
             auto requestHeaders = FetchHeaders::create(record.requestHeadersGuard, HTTPHeaderMap { record.request.httpHeaderFields() });
-            FetchRequest::InternalRequest internalRequest = { WTFMove(record.request), WTFMove(record.options), WTFMove(record.referrer) };
-            auto request = FetchRequest::create(*scriptExecutionContext(), std::nullopt, WTFMove(requestHeaders), WTFMove(internalRequest));
+            auto request = FetchRequest::create(*scriptExecutionContext(), std::nullopt, WTFMove(requestHeaders),  WTFMove(record.request), WTFMove(record.options), WTFMove(record.referrer));
 
             auto responseHeaders = FetchHeaders::create(record.responseHeadersGuard, HTTPHeaderMap { record.response.httpHeaderFields() });
             auto response = FetchResponse::create(*scriptExecutionContext(), std::nullopt, WTFMove(responseHeaders), WTFMove(record.response));

Modified: trunk/Source/WebCore/Modules/fetch/FetchRequest.cpp (221396 => 221397)


--- trunk/Source/WebCore/Modules/fetch/FetchRequest.cpp	2017-08-30 20:45:43 UTC (rev 221396)
+++ trunk/Source/WebCore/Modules/fetch/FetchRequest.cpp	2017-08-30 21:19:46 UTC (rev 221397)
@@ -49,66 +49,64 @@
     return std::nullopt;
 }
 
-static std::optional<Exception> setReferrer(FetchRequest::InternalRequest& request, ScriptExecutionContext& context, const String& referrer)
+static ExceptionOr<String> computeReferrer(ScriptExecutionContext& context, const String& referrer)
 {
-    if (referrer.isEmpty()) {
-        request.referrer = ASCIILiteral("no-referrer");
-        return std::nullopt;
-    }
+    if (referrer.isEmpty())
+        return String { "no-referrer" };
+
     // FIXME: Tighten the URL parsing algorithm according https://url.spec.whatwg.org/#concept-url-parser.
     URL referrerURL = context.completeURL(referrer);
     if (!referrerURL.isValid())
         return Exception { TypeError, ASCIILiteral("Referrer is not a valid URL.") };
 
-    if (referrerURL.protocolIs("about") && referrerURL.path() == "client") {
-        request.referrer = ASCIILiteral("client");
-        return std::nullopt;
-    }
+    if (referrerURL.protocolIs("about") && referrerURL.path() == "client")
+        return String { "client" };
 
     if (!(context.securityOrigin() && context.securityOrigin()->canRequest(referrerURL)))
         return Exception { TypeError, ASCIILiteral("Referrer is not same-origin.") };
 
-    request.referrer = referrerURL.string();
-    return std::nullopt;
+    return String { referrerURL.string() };
 }
 
-static std::optional<Exception> buildOptions(FetchRequest::InternalRequest& request, ScriptExecutionContext& context, const FetchRequest::Init& init)
+static std::optional<Exception> buildOptions(FetchOptions& options, ResourceRequest& request, String& referrer, ScriptExecutionContext& context, const FetchRequest::Init& init)
 {
     if (!init.window.isUndefinedOrNull() && !init.window.isEmpty())
         return Exception { TypeError, ASCIILiteral("Window can only be null.") };
 
     if (!init.referrer.isNull()) {
-        if (auto exception = setReferrer(request, context, init.referrer))
-            return exception;
+        auto result = computeReferrer(context, init.referrer);
+        if (result.hasException())
+            return result.releaseException();
+        referrer = result.releaseReturnValue();
     }
 
     if (init.referrerPolicy)
-        request.options.referrerPolicy = init.referrerPolicy.value();
+        options.referrerPolicy = init.referrerPolicy.value();
 
     if (init.mode)
-        request.options.mode = init.mode.value();
-    if (request.options.mode == FetchOptions::Mode::Navigate)
+        options.mode = init.mode.value();
+    if (options.mode == FetchOptions::Mode::Navigate)
         return Exception { TypeError, ASCIILiteral("Request constructor does not accept navigate fetch mode.") };
 
     if (init.credentials)
-        request.options.credentials = init.credentials.value();
+        options.credentials = init.credentials.value();
 
     if (init.cache)
-        request.options.cache = init.cache.value();
-    if (request.options.cache == FetchOptions::Cache::OnlyIfCached && request.options.mode != FetchOptions::Mode::SameOrigin)
+        options.cache = init.cache.value();
+    if (options.cache == FetchOptions::Cache::OnlyIfCached && options.mode != FetchOptions::Mode::SameOrigin)
         return Exception { TypeError, ASCIILiteral("only-if-cached cache option requires fetch mode to be same-origin.")  };
 
     if (init.redirect)
-        request.options.redirect = init.redirect.value();
+        options.redirect = init.redirect.value();
 
     if (!init.integrity.isNull())
-        request.options.integrity = init.integrity;
+        options.integrity = init.integrity;
 
     if (init.keepalive && init.keepalive.value())
-        request.options.keepAlive = true;
+        options.keepAlive = true;
 
     if (!init.method.isNull()) {
-        if (auto exception = setMethod(request.request, init.method))
+        if (auto exception = setMethod(request, init.method))
             return exception;
     }
 
@@ -115,9 +113,9 @@
     return std::nullopt;
 }
 
-static bool methodCanHaveBody(const FetchRequest::InternalRequest& internalRequest)
+static bool methodCanHaveBody(const ResourceRequest& request)
 {
-    return internalRequest.request.httpMethod() != "GET" && internalRequest.request.httpMethod() != "HEAD";
+    return request.httpMethod() != "GET" && request.httpMethod() != "HEAD";
 }
 
 ExceptionOr<void> FetchRequest::initializeOptions(const Init& init)
@@ -124,15 +122,15 @@
 {
     ASSERT(scriptExecutionContext());
 
-    auto exception = buildOptions(m_internalRequest, *scriptExecutionContext(), init);
+    auto exception = buildOptions(m_options, m_request, m_referrer, *scriptExecutionContext(), init);
     if (exception)
         return WTFMove(exception.value());
 
-    if (m_internalRequest.options.mode == FetchOptions::Mode::NoCors) {
-        const String& method = m_internalRequest.request.httpMethod();
+    if (m_options.mode == FetchOptions::Mode::NoCors) {
+        const String& method = m_request.httpMethod();
         if (method != "GET" && method != "POST" && method != "HEAD")
             return Exception { TypeError, ASCIILiteral("Method must be GET, POST or HEAD in no-cors mode.") };
-        if (!m_internalRequest.options.integrity.isEmpty())
+        if (!m_options.integrity.isEmpty())
             return Exception { TypeError, ASCIILiteral("There cannot be an integrity in no-cors mode.") };
         m_headers->setGuard(FetchHeaders::Guard::RequestNoCors);
     }
@@ -148,12 +146,12 @@
     if (!requestURL.isValid() || !requestURL.user().isEmpty() || !requestURL.pass().isEmpty())
         return Exception { TypeError, ASCIILiteral("URL is not valid or contains user credentials.") };
 
-    m_internalRequest.options.mode = Mode::Cors;
-    m_internalRequest.options.credentials = Credentials::Omit;
-    m_internalRequest.referrer = ASCIILiteral("client");
-    m_internalRequest.request.setURL(requestURL);
-    m_internalRequest.request.setRequester(ResourceRequest::Requester::Fetch);
-    m_internalRequest.request.setInitiatorIdentifier(scriptExecutionContext()->resourceRequestIdentifier());
+    m_options.mode = Mode::Cors;
+    m_options.credentials = Credentials::Omit;
+    m_referrer = ASCIILiteral("client");
+    m_request.setURL(requestURL);
+    m_request.setRequester(ResourceRequest::Requester::Fetch);
+    m_request.setInitiatorIdentifier(scriptExecutionContext()->resourceRequestIdentifier());
 
     auto optionsResult = initializeOptions(init);
     if (optionsResult.hasException())
@@ -180,7 +178,9 @@
     if (input.isDisturbedOrLocked())
         return Exception {TypeError, ASCIILiteral("Request input is disturbed or locked.") };
 
-    m_internalRequest = input.m_internalRequest;
+    m_request = input.m_request;
+    m_options = input.m_options;
+    m_referrer = input.m_referrer;
 
     auto optionsResult = initializeOptions(init);
     if (optionsResult.hasException())
@@ -212,13 +212,13 @@
 
 ExceptionOr<void> FetchRequest::setBody(FetchBody::Init&& body)
 {
-    if (!methodCanHaveBody(m_internalRequest))
+    if (!methodCanHaveBody(m_request))
         return Exception { TypeError };
 
     ASSERT(scriptExecutionContext());
     extractBody(*scriptExecutionContext(), WTFMove(body));
 
-    if (m_internalRequest.options.keepAlive && hasReadableStreamBody())
+    if (m_options.keepAlive && hasReadableStreamBody())
         return Exception { TypeError, ASCIILiteral("Request cannot have a ReadableStream body and keepalive set to true") };
     return { };
 }
@@ -226,7 +226,7 @@
 ExceptionOr<void> FetchRequest::setBody(FetchRequest& request)
 {
     if (!request.isBodyNull()) {
-        if (!methodCanHaveBody(m_internalRequest))
+        if (!methodCanHaveBody(m_request))
             return Exception { TypeError };
         // FIXME: If body has a readable stream, we should pipe it to this new body stream.
         m_body = WTFMove(request.m_body);
@@ -233,7 +233,7 @@
         request.setDisturbed();
     }
 
-    if (m_internalRequest.options.keepAlive && hasReadableStreamBody())
+    if (m_options.keepAlive && hasReadableStreamBody())
         return Exception { TypeError, ASCIILiteral("Request cannot have a ReadableStream body and keepalive set to true") };
     return { };
 }
@@ -240,7 +240,7 @@
 
 ExceptionOr<Ref<FetchRequest>> FetchRequest::create(ScriptExecutionContext& context, Info&& input, Init&& init)
 {
-    auto request = adoptRef(*new FetchRequest(context, std::nullopt, FetchHeaders::create(FetchHeaders::Guard::Request), { }));
+    auto request = adoptRef(*new FetchRequest(context, std::nullopt, FetchHeaders::create(FetchHeaders::Guard::Request), { }, { }, { }));
 
     if (WTF::holds_alternative<String>(input)) {
         auto result = request->initializeWith(WTF::get<String>(input), WTFMove(init));
@@ -257,17 +257,17 @@
 
 String FetchRequest::referrer() const
 {
-    if (m_internalRequest.referrer == "no-referrer")
+    if (m_referrer == "no-referrer")
         return String();
-    if (m_internalRequest.referrer == "client")
+    if (m_referrer == "client")
         return ASCIILiteral("about:client");
-    return m_internalRequest.referrer;
+    return m_referrer;
 }
 
 const String& FetchRequest::urlString() const
 {
     if (m_requestURL.isNull())
-        m_requestURL = m_internalRequest.request.url().serialize();
+        m_requestURL = m_request.url().serialize();
     return m_requestURL;
 }
 
@@ -275,7 +275,7 @@
 {
     ASSERT(scriptExecutionContext());
 
-    ResourceRequest request = m_internalRequest.request;
+    ResourceRequest request = m_request;
     request.setHTTPHeaderFields(m_headers->internalHeaders());
 
     if (!isBodyNull())
@@ -289,7 +289,7 @@
     if (isDisturbedOrLocked())
         return Exception { TypeError };
 
-    auto clone = adoptRef(*new FetchRequest(context, std::nullopt, FetchHeaders::create(m_headers.get()), FetchRequest::InternalRequest(m_internalRequest)));
+    auto clone = adoptRef(*new FetchRequest(context, std::nullopt, FetchHeaders::create(m_headers.get()), ResourceRequest { m_request }, FetchOptions { m_options}, String { m_referrer }));
     clone->cloneBody(*this);
     return WTFMove(clone);
 }

Modified: trunk/Source/WebCore/Modules/fetch/FetchRequest.h (221396 => 221397)


--- trunk/Source/WebCore/Modules/fetch/FetchRequest.h	2017-08-30 20:45:43 UTC (rev 221396)
+++ trunk/Source/WebCore/Modules/fetch/FetchRequest.h	2017-08-30 21:19:46 UTC (rev 221397)
@@ -53,43 +53,38 @@
     using Redirect = FetchOptions::Redirect;
     using Type = FetchOptions::Type;
 
-    struct InternalRequest {
-        ResourceRequest request;
-        FetchOptions options;
-        String referrer;
-    };
 
     static ExceptionOr<Ref<FetchRequest>> create(ScriptExecutionContext&, Info&&, Init&&);
-    static Ref<FetchRequest> create(ScriptExecutionContext& context, std::optional<FetchBody>&& body, Ref<FetchHeaders>&& headers, InternalRequest&& request) { return adoptRef(*new FetchRequest(context, WTFMove(body), WTFMove(headers), WTFMove(request))); }
+    static Ref<FetchRequest> create(ScriptExecutionContext& context, std::optional<FetchBody>&& body, Ref<FetchHeaders>&& headers, ResourceRequest&& request, FetchOptions&& options, String&& referrer) { return adoptRef(*new FetchRequest(context, WTFMove(body), WTFMove(headers), WTFMove(request), WTFMove(options), WTFMove(referrer))); }
 
-    const String& method() const { return m_internalRequest.request.httpMethod(); }
+    const String& method() const { return m_request.httpMethod(); }
     const String& urlString() const;
     FetchHeaders& headers() { return m_headers.get(); }
     const FetchHeaders& headers() const { return m_headers.get(); }
 
-    Type type() const;
-    Destination destination() const;
+    Type type() const { return m_options.type; }
+    Destination destination() const { return m_options.destination; }
     String referrer() const;
-    ReferrerPolicy referrerPolicy() const;
-    Mode mode() const;
-    Credentials credentials() const;
-    Cache cache() const;
-    Redirect redirect() const;
-    bool keepalive() const { return m_internalRequest.options.keepAlive; };
+    ReferrerPolicy referrerPolicy() const { return m_options.referrerPolicy; }
+    Mode mode() const { return m_options.mode; }
+    Credentials credentials() const { return m_options.credentials; }
+    Cache cache() const { return m_options.cache; }
+    Redirect redirect() const { return m_options.redirect; }
+    bool keepalive() const { return m_options.keepAlive; };
 
-    const String& integrity() const { return m_internalRequest.options.integrity; }
+    const String& integrity() const { return m_options.integrity; }
 
     ExceptionOr<Ref<FetchRequest>> clone(ScriptExecutionContext&);
 
-    const FetchOptions& fetchOptions() const { return m_internalRequest.options; }
-    const ResourceRequest& internalRequest() const { return m_internalRequest.request; }
-    const String& internalRequestReferrer() const { return m_internalRequest.referrer; }
-    const URL& url() const { return m_internalRequest.request.url(); }
+    const FetchOptions& fetchOptions() const { return m_options; }
+    const ResourceRequest& internalRequest() const { return m_request; }
+    const String& internalRequestReferrer() const { return m_referrer; }
+    const URL& url() const { return m_request.url(); }
 
     ResourceRequest resourceRequest() const;
 
 private:
-    FetchRequest(ScriptExecutionContext&, std::optional<FetchBody>&&, Ref<FetchHeaders>&&, InternalRequest&&);
+    FetchRequest(ScriptExecutionContext&, std::optional<FetchBody>&&, Ref<FetchHeaders>&&, ResourceRequest&&, FetchOptions&&, String&& referrer);
 
     ExceptionOr<void> initializeOptions(const Init&);
     ExceptionOr<void> initializeWith(FetchRequest&, Init&&);
@@ -100,49 +95,19 @@
     const char* activeDOMObjectName() const final;
     bool canSuspendForDocumentSuspension() const final;
 
-    InternalRequest m_internalRequest;
+
+    ResourceRequest m_request;
+    FetchOptions m_options;
+    String m_referrer;
     mutable String m_requestURL;
 };
 
-inline FetchRequest::FetchRequest(ScriptExecutionContext& context, std::optional<FetchBody>&& body, Ref<FetchHeaders>&& headers, InternalRequest&& internalRequest)
+inline FetchRequest::FetchRequest(ScriptExecutionContext& context, std::optional<FetchBody>&& body, Ref<FetchHeaders>&& headers, ResourceRequest&& request, FetchOptions&& options, String&& referrer)
     : FetchBodyOwner(context, WTFMove(body), WTFMove(headers))
-    , m_internalRequest(WTFMove(internalRequest))
+    , m_request(WTFMove(request))
+    , m_options(WTFMove(options))
+    , m_referrer(WTFMove(referrer))
 {
 }
 
-inline auto FetchRequest::cache() const -> Cache
-{
-    return m_internalRequest.options.cache;
-}
-
-inline auto FetchRequest::credentials() const -> Credentials
-{
-    return m_internalRequest.options.credentials;
-}
-
-inline auto FetchRequest::destination() const -> Destination
-{
-    return m_internalRequest.options.destination;
-}
-
-inline auto FetchRequest::mode() const -> Mode
-{
-    return m_internalRequest.options.mode;
-}
-
-inline auto FetchRequest::redirect() const -> Redirect
-{
-    return m_internalRequest.options.redirect;
-}
-
-inline auto FetchRequest::referrerPolicy() const -> ReferrerPolicy
-{
-    return m_internalRequest.options.referrerPolicy;
-}
-
-inline auto FetchRequest::type() const -> Type
-{
-    return m_internalRequest.options.type;
-}
-
 } // namespace WebCore
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to