NetworkLoadMetrics should be shared by multiple ResourceResponse instances
authorysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Feb 2020 19:53:14 +0000 (19:53 +0000)
committerysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Feb 2020 19:53:14 +0000 (19:53 +0000)
https://bugs.webkit.org/show_bug.cgi?id=207747

Reviewed by Keith Miller.

Source/WebCore:

ResourceResponse is value data, and it is copied multiple times in various places, (1) to create a new ResourceResponse
which has slightly different fields, or (1) to hold ResourceResponse even after loading finishes. For example, DocumentLoader
has Vector<ResourceResponse> to replay response dispatching in the case of loading from BackForwardCache. The problem is
that ResourceResponse is very large: 440 bytes.

While we sometimes copy ResourceResponse to modify some part of it, NetworkLoadMetrics is immutable. It is set when response is created,
and is never changed. And NetworkLoadMetrics is large: sizeof(NetworkLoadMetrics) is 184 bytes. Given that we have multiple
copies of ResourceResponse in WebCore, we should share NetworkLoadMetrics by them.

This patch puts Box<NetworkLoadMetrics> in ResourceResponse to share it with all copied ResourceResponses. We do not make NetworkLoadMetrics
RefCounted<> for now since some legit data structures embed NetworkLoadMetrics. This patch adds ArgumentCoder for Box so that we
can encode / decode Box<NetworkLoadMetrics> in ResourceResponse in IPC. To ensure NetworkLoadMetrics in ResourceResponse immutable,
we add ResourceResponse::setDeprecatedNetworkLoadMetrics instead of modifying NetworkLoadMetrics already created in ResourceResponse.

We also attempt to compact ResourceResponse more by using bit-fields. And removing m_isValid field in ParsedContentRange since
this can be represented by the different field. These changes make sizeof(ResourceResponse) from 440 to 248.

No behavior change.

* inspector/agents/InspectorNetworkAgent.cpp:
(WebCore::InspectorNetworkAgent::buildObjectForTiming):
(WebCore::InspectorNetworkAgent::buildObjectForResourceResponse):
* inspector/agents/InspectorNetworkAgent.h:
* loader/DocumentThreadableLoader.cpp:
(WebCore::DocumentThreadableLoader::loadRequest):
* loader/SubresourceLoader.cpp:
(WebCore::SubresourceLoader::didFinishLoading):
* page/PerformanceTiming.cpp:
(WebCore::PerformanceTiming::domainLookupStart const):
(WebCore::PerformanceTiming::domainLookupEnd const):
(WebCore::PerformanceTiming::connectStart const):
(WebCore::PerformanceTiming::connectEnd const):
(WebCore::PerformanceTiming::secureConnectionStart const):
(WebCore::PerformanceTiming::requestStart const):
(WebCore::PerformanceTiming::responseStart const):
* platform/network/NetworkLoadMetrics.h:
* platform/network/ParsedContentRange.cpp:
(WebCore::areContentRangeValuesValid):
(WebCore::parseContentRange):
(WebCore::ParsedContentRange::ParsedContentRange):
(WebCore::ParsedContentRange::headerValue const):
* platform/network/ParsedContentRange.h:
(WebCore::ParsedContentRange::isValid const):
(WebCore::ParsedContentRange::invalidValue):
(WebCore::ParsedContentRange::MarkableTraits::isEmptyValue):
(WebCore::ParsedContentRange::MarkableTraits::emptyValue):
(WebCore::ParsedContentRange::ParsedContentRange): Deleted.
* platform/network/ResourceHandle.h:
* platform/network/ResourceResponseBase.cpp:
(WebCore::ResourceResponseBase::ResourceResponseBase):
(WebCore::ResourceResponseBase::crossThreadData const):
(WebCore::ResourceResponseBase::fromCrossThreadData):
(WebCore::ResourceResponseBase::compare):
* platform/network/ResourceResponseBase.h:
(WebCore::ResourceResponseBase::deprecatedNetworkLoadMetricsOrNull const):
(WebCore::ResourceResponseBase::setDeprecatedNetworkLoadMetrics):
(WebCore::ResourceResponseBase::encode const):
(WebCore::ResourceResponseBase::decode):
(WebCore::ResourceResponseBase::deprecatedNetworkLoadMetrics const): Deleted.
* platform/network/cf/ResourceResponse.h:
(WebCore::ResourceResponse::ResourceResponse):
* platform/network/cocoa/NetworkLoadMetrics.mm:
(WebCore::copyTimingData):
* platform/network/curl/CurlResourceHandleDelegate.cpp:
(WebCore::CurlResourceHandleDelegate::curlDidReceiveResponse):
* platform/network/curl/ResourceResponse.h:
* platform/network/curl/ResourceResponseCurl.cpp:
(WebCore::ResourceResponse::setDeprecatedNetworkLoadMetrics): Deleted.
* platform/network/mac/ResourceHandleMac.mm:
(WebCore::ResourceHandle::getConnectionTimingData):
* platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.mm:
(-[WebCoreResourceHandleAsOperationQueueDelegate connection:didReceiveResponse:]):

Source/WebKit:

Add ArgumentCoder support for Box<T>.

* NetworkProcess/cocoa/NetworkSessionCocoa.mm:
(-[WKNetworkSessionDelegate URLSession:dataTask:didReceiveResponse:completionHandler:]):
* NetworkProcess/soup/NetworkDataTaskSoup.cpp:
(WebKit::NetworkDataTaskSoup::dispatchDidReceiveResponse):
* Platform/IPC/ArgumentCoders.h:
(IPC::ArgumentCoder<Box<T>>::encode):
(IPC::ArgumentCoder<Box<T>>::decode):

Tools:

* TestWebKitAPI/Tests/WebCore/ParsedContentRange.cpp:
(TestWebKitAPI::TEST):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@256632 268f45cc-cd09-0410-ab3c-d52691b4dbfc

26 files changed:
Source/WebCore/ChangeLog
Source/WebCore/inspector/agents/InspectorNetworkAgent.cpp
Source/WebCore/inspector/agents/InspectorNetworkAgent.h
Source/WebCore/loader/DocumentThreadableLoader.cpp
Source/WebCore/loader/SubresourceLoader.cpp
Source/WebCore/page/PerformanceTiming.cpp
Source/WebCore/platform/network/NetworkLoadMetrics.h
Source/WebCore/platform/network/ParsedContentRange.cpp
Source/WebCore/platform/network/ParsedContentRange.h
Source/WebCore/platform/network/ResourceHandle.h
Source/WebCore/platform/network/ResourceResponseBase.cpp
Source/WebCore/platform/network/ResourceResponseBase.h
Source/WebCore/platform/network/cf/ResourceResponse.h
Source/WebCore/platform/network/cocoa/NetworkLoadMetrics.mm
Source/WebCore/platform/network/curl/CurlResourceHandleDelegate.cpp
Source/WebCore/platform/network/curl/ResourceResponse.h
Source/WebCore/platform/network/curl/ResourceResponseCurl.cpp
Source/WebCore/platform/network/mac/ResourceHandleMac.mm
Source/WebCore/platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.mm
Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm
Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp
Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp
Source/WebKit/Platform/IPC/ArgumentCoders.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebCore/ParsedContentRange.cpp

index a039168..c20f4af 100644 (file)
@@ -1,3 +1,83 @@
+2020-02-14  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        NetworkLoadMetrics should be shared by multiple ResourceResponse instances
+        https://bugs.webkit.org/show_bug.cgi?id=207747
+
+        Reviewed by Keith Miller.
+
+        ResourceResponse is value data, and it is copied multiple times in various places, (1) to create a new ResourceResponse
+        which has slightly different fields, or (1) to hold ResourceResponse even after loading finishes. For example, DocumentLoader
+        has Vector<ResourceResponse> to replay response dispatching in the case of loading from BackForwardCache. The problem is
+        that ResourceResponse is very large: 440 bytes.
+
+        While we sometimes copy ResourceResponse to modify some part of it, NetworkLoadMetrics is immutable. It is set when response is created,
+        and is never changed. And NetworkLoadMetrics is large: sizeof(NetworkLoadMetrics) is 184 bytes. Given that we have multiple
+        copies of ResourceResponse in WebCore, we should share NetworkLoadMetrics by them.
+
+        This patch puts Box<NetworkLoadMetrics> in ResourceResponse to share it with all copied ResourceResponses. We do not make NetworkLoadMetrics
+        RefCounted<> for now since some legit data structures embed NetworkLoadMetrics. This patch adds ArgumentCoder for Box so that we
+        can encode / decode Box<NetworkLoadMetrics> in ResourceResponse in IPC. To ensure NetworkLoadMetrics in ResourceResponse immutable,
+        we add ResourceResponse::setDeprecatedNetworkLoadMetrics instead of modifying NetworkLoadMetrics already created in ResourceResponse.
+
+        We also attempt to compact ResourceResponse more by using bit-fields. And removing m_isValid field in ParsedContentRange since
+        this can be represented by the different field. These changes make sizeof(ResourceResponse) from 440 to 248.
+
+        No behavior change.
+
+        * inspector/agents/InspectorNetworkAgent.cpp:
+        (WebCore::InspectorNetworkAgent::buildObjectForTiming):
+        (WebCore::InspectorNetworkAgent::buildObjectForResourceResponse):
+        * inspector/agents/InspectorNetworkAgent.h:
+        * loader/DocumentThreadableLoader.cpp:
+        (WebCore::DocumentThreadableLoader::loadRequest):
+        * loader/SubresourceLoader.cpp:
+        (WebCore::SubresourceLoader::didFinishLoading):
+        * page/PerformanceTiming.cpp:
+        (WebCore::PerformanceTiming::domainLookupStart const):
+        (WebCore::PerformanceTiming::domainLookupEnd const):
+        (WebCore::PerformanceTiming::connectStart const):
+        (WebCore::PerformanceTiming::connectEnd const):
+        (WebCore::PerformanceTiming::secureConnectionStart const):
+        (WebCore::PerformanceTiming::requestStart const):
+        (WebCore::PerformanceTiming::responseStart const):
+        * platform/network/NetworkLoadMetrics.h:
+        * platform/network/ParsedContentRange.cpp:
+        (WebCore::areContentRangeValuesValid):
+        (WebCore::parseContentRange):
+        (WebCore::ParsedContentRange::ParsedContentRange):
+        (WebCore::ParsedContentRange::headerValue const):
+        * platform/network/ParsedContentRange.h:
+        (WebCore::ParsedContentRange::isValid const):
+        (WebCore::ParsedContentRange::invalidValue):
+        (WebCore::ParsedContentRange::MarkableTraits::isEmptyValue):
+        (WebCore::ParsedContentRange::MarkableTraits::emptyValue):
+        (WebCore::ParsedContentRange::ParsedContentRange): Deleted.
+        * platform/network/ResourceHandle.h:
+        * platform/network/ResourceResponseBase.cpp:
+        (WebCore::ResourceResponseBase::ResourceResponseBase):
+        (WebCore::ResourceResponseBase::crossThreadData const):
+        (WebCore::ResourceResponseBase::fromCrossThreadData):
+        (WebCore::ResourceResponseBase::compare):
+        * platform/network/ResourceResponseBase.h:
+        (WebCore::ResourceResponseBase::deprecatedNetworkLoadMetricsOrNull const):
+        (WebCore::ResourceResponseBase::setDeprecatedNetworkLoadMetrics):
+        (WebCore::ResourceResponseBase::encode const):
+        (WebCore::ResourceResponseBase::decode):
+        (WebCore::ResourceResponseBase::deprecatedNetworkLoadMetrics const): Deleted.
+        * platform/network/cf/ResourceResponse.h:
+        (WebCore::ResourceResponse::ResourceResponse):
+        * platform/network/cocoa/NetworkLoadMetrics.mm:
+        (WebCore::copyTimingData):
+        * platform/network/curl/CurlResourceHandleDelegate.cpp:
+        (WebCore::CurlResourceHandleDelegate::curlDidReceiveResponse):
+        * platform/network/curl/ResourceResponse.h:
+        * platform/network/curl/ResourceResponseCurl.cpp:
+        (WebCore::ResourceResponse::setDeprecatedNetworkLoadMetrics): Deleted.
+        * platform/network/mac/ResourceHandleMac.mm:
+        (WebCore::ResourceHandle::getConnectionTimingData):
+        * platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.mm:
+        (-[WebCoreResourceHandleAsOperationQueueDelegate connection:didReceiveResponse:]):
+
 2020-02-14  Andres Gonzalez  <andresg_22@apple.com>
 
         Implementation of AXIsolatedObject::isDescendantOfObject.
index 3e2faaa..ee9edb3 100644 (file)
@@ -213,27 +213,32 @@ static Ref<JSON::Object> buildObjectForHeaders(const HTTPHeaderMap& headers)
     return headersObject;
 }
 
-Ref<Inspector::Protocol::Network::ResourceTiming> InspectorNetworkAgent::buildObjectForTiming(const NetworkLoadMetrics& timing, ResourceLoader& resourceLoader)
+Ref<Inspector::Protocol::Network::ResourceTiming> InspectorNetworkAgent::buildObjectForTiming(const NetworkLoadMetrics* timing, ResourceLoader& resourceLoader)
 {
     auto& loadTiming = resourceLoader.loadTiming();
 
     auto elapsedTimeSince = [&] (const MonotonicTime& time) {
         return m_environment.executionStopwatch()->elapsedTimeSince(time).seconds();
     };
+    Optional<NetworkLoadMetrics> empty;
+    if (!timing) {
+        empty.emplace();
+        timing = &empty.value();
+    }
 
     return Inspector::Protocol::Network::ResourceTiming::create()
         .setStartTime(elapsedTimeSince(loadTiming.startTime()))
         .setRedirectStart(elapsedTimeSince(loadTiming.redirectStart()))
         .setRedirectEnd(elapsedTimeSince(loadTiming.redirectEnd()))
         .setFetchStart(elapsedTimeSince(loadTiming.fetchStart()))
-        .setDomainLookupStart(timing.domainLookupStart.milliseconds())
-        .setDomainLookupEnd(timing.domainLookupEnd.milliseconds())
-        .setConnectStart(timing.connectStart.milliseconds())
-        .setConnectEnd(timing.connectEnd.milliseconds())
-        .setSecureConnectionStart(timing.secureConnectionStart.milliseconds())
-        .setRequestStart(timing.requestStart.milliseconds())
-        .setResponseStart(timing.responseStart.milliseconds())
-        .setResponseEnd(timing.responseEnd.milliseconds())
+        .setDomainLookupStart(timing->domainLookupStart.milliseconds())
+        .setDomainLookupEnd(timing->domainLookupEnd.milliseconds())
+        .setConnectStart(timing->connectStart.milliseconds())
+        .setConnectEnd(timing->connectEnd.milliseconds())
+        .setSecureConnectionStart(timing->secureConnectionStart.milliseconds())
+        .setRequestStart(timing->requestStart.milliseconds())
+        .setResponseStart(timing->responseStart.milliseconds())
+        .setResponseEnd(timing->responseEnd.milliseconds())
         .release();
 }
 
@@ -351,7 +356,7 @@ RefPtr<Inspector::Protocol::Network::Response> InspectorNetworkAgent::buildObjec
         .release();
 
     if (resourceLoader)
-        responseObject->setTiming(buildObjectForTiming(response.deprecatedNetworkLoadMetrics(), *resourceLoader));
+        responseObject->setTiming(buildObjectForTiming(response.deprecatedNetworkLoadMetricsOrNull(), *resourceLoader));
 
     if (auto& certificateInfo = response.certificateInfo()) {
         auto securityPayload = Inspector::Protocol::Security::Security::create()
index a68f845..b038a18 100644 (file)
@@ -146,7 +146,7 @@ private:
     WebSocket* webSocketForRequestId(const String& requestId);
 
     RefPtr<Inspector::Protocol::Network::Initiator> buildInitiatorObject(Document*, Optional<const ResourceRequest&> = WTF::nullopt);
-    Ref<Inspector::Protocol::Network::ResourceTiming> buildObjectForTiming(const NetworkLoadMetrics&, ResourceLoader&);
+    Ref<Inspector::Protocol::Network::ResourceTiming> buildObjectForTiming(const NetworkLoadMetrics*, ResourceLoader&);
     Ref<Inspector::Protocol::Network::Metrics> buildObjectForMetrics(const NetworkLoadMetrics&);
     RefPtr<Inspector::Protocol::Network::Response> buildObjectForResourceResponse(const ResourceResponse&, ResourceLoader*);
     Ref<Inspector::Protocol::Network::CachedResource> buildObjectForCachedResource(CachedResource*);
index 64ee5dd..3742128 100644 (file)
@@ -630,7 +630,13 @@ void DocumentThreadableLoader::loadRequest(ResourceRequest&& request, SecurityCh
         didReceiveData(identifier, data->data(), data->size());
 
     if (RuntimeEnabledFeatures::sharedFeatures().resourceTimingEnabled()) {
-        auto resourceTiming = ResourceTiming::fromSynchronousLoad(requestURL, m_options.initiator, loadTiming, response.deprecatedNetworkLoadMetrics(), response, securityOrigin());
+        const auto* timing = response.deprecatedNetworkLoadMetricsOrNull();
+        Optional<NetworkLoadMetrics> empty;
+        if (!timing) {
+            empty.emplace();
+            timing = &empty.value();
+        }
+        auto resourceTiming = ResourceTiming::fromSynchronousLoad(requestURL, m_options.initiator, loadTiming, *timing, response, securityOrigin());
         if (options().initiatorContext == InitiatorContext::Worker)
             finishedTimingForWorkerLoad(resourceTiming);
         else {
index 2b810c6..4a0ca3e 100644 (file)
@@ -691,7 +691,13 @@ void SubresourceLoader::didFinishLoading(const NetworkLoadMetrics& networkLoadMe
         // This is the legacy path for platforms (and ResourceHandle paths) that do not provide
         // complete load metrics in didFinishLoad. In those cases, fall back to the possibility
         // that they populated partial load timing information on the ResourceResponse.
-        reportResourceTiming(m_resource->response().deprecatedNetworkLoadMetrics());
+        const auto* timing = m_resource->response().deprecatedNetworkLoadMetricsOrNull();
+        Optional<NetworkLoadMetrics> empty;
+        if (!timing) {
+            empty.emplace();
+            timing = &empty.value();
+        }
+        reportResourceTiming(*timing);
     }
 
     if (m_resource->type() != CachedResource::Type::MainResource)
index 4efd54e..56a756d 100644 (file)
@@ -120,14 +120,14 @@ unsigned long long PerformanceTiming::domainLookupStart() const
     if (!loader)
         return fetchStart();
     
-    const NetworkLoadMetrics& timing = loader->response().deprecatedNetworkLoadMetrics();
+    const NetworkLoadMetrics* timing = loader->response().deprecatedNetworkLoadMetricsOrNull();
     
     // This will be -1 when a DNS request is not performed.
     // Rather than exposing a special value that indicates no DNS, we "backfill" with fetchStart.
-    if (timing.domainLookupStart < 0_ms)
+    if (!timing || timing->domainLookupStart < 0_ms)
         return fetchStart();
 
-    return resourceLoadTimeRelativeToFetchStart(timing.domainLookupStart);
+    return resourceLoadTimeRelativeToFetchStart(timing->domainLookupStart);
 }
 
 unsigned long long PerformanceTiming::domainLookupEnd() const
@@ -136,14 +136,14 @@ unsigned long long PerformanceTiming::domainLookupEnd() const
     if (!loader)
         return domainLookupStart();
     
-    const NetworkLoadMetrics& timing = loader->response().deprecatedNetworkLoadMetrics();
+    const NetworkLoadMetrics* timing = loader->response().deprecatedNetworkLoadMetricsOrNull();
     
     // This will be -1 when a DNS request is not performed.
     // Rather than exposing a special value that indicates no DNS, we "backfill" with domainLookupStart.
-    if (timing.domainLookupEnd < 0_ms)
+    if (!timing || timing->domainLookupEnd < 0_ms)
         return domainLookupStart();
 
-    return resourceLoadTimeRelativeToFetchStart(timing.domainLookupEnd);
+    return resourceLoadTimeRelativeToFetchStart(timing->domainLookupEnd);
 }
 
 unsigned long long PerformanceTiming::connectStart() const
@@ -152,18 +152,20 @@ unsigned long long PerformanceTiming::connectStart() const
     if (!loader)
         return domainLookupEnd();
 
-    const NetworkLoadMetrics& timing = loader->response().deprecatedNetworkLoadMetrics();
+    const NetworkLoadMetrics* timing = loader->response().deprecatedNetworkLoadMetricsOrNull();
     
     // connectStart will be -1 when a network request is not made.
     // Rather than exposing a special value that indicates no new connection, we "backfill" with domainLookupEnd.
-    Seconds connectStart = timing.connectStart;
+    if (!timing)
+        return domainLookupEnd();
+    Seconds connectStart = timing->connectStart;
     if (connectStart < 0_ms)
         return domainLookupEnd();
 
     // NetworkLoadMetrics's connect phase includes DNS, however Navigation Timing's
     // connect phase should not. So if there is DNS time, trim it from the start.
-    if (timing.domainLookupEnd >= 0_ms && timing.domainLookupEnd > connectStart)
-        connectStart = timing.domainLookupEnd;
+    if (timing->domainLookupEnd >= 0_ms && timing->domainLookupEnd > connectStart)
+        connectStart = timing->domainLookupEnd;
 
     return resourceLoadTimeRelativeToFetchStart(connectStart);
 }
@@ -174,14 +176,14 @@ unsigned long long PerformanceTiming::connectEnd() const
     if (!loader)
         return connectStart();
 
-    const NetworkLoadMetrics& timing = loader->response().deprecatedNetworkLoadMetrics();
+    const NetworkLoadMetrics* timing = loader->response().deprecatedNetworkLoadMetricsOrNull();
     
     // connectEnd will be -1 when a network request is not made.
     // Rather than exposing a special value that indicates no new connection, we "backfill" with connectStart.
-    if (timing.connectEnd < 0_ms)
+    if (!timing || timing->connectEnd < 0_ms)
         return connectStart();
 
-    return resourceLoadTimeRelativeToFetchStart(timing.connectEnd);
+    return resourceLoadTimeRelativeToFetchStart(timing->connectEnd);
 }
 
 unsigned long long PerformanceTiming::secureConnectionStart() const
@@ -190,12 +192,12 @@ unsigned long long PerformanceTiming::secureConnectionStart() const
     if (!loader)
         return 0;
 
-    const NetworkLoadMetrics& timing = loader->response().deprecatedNetworkLoadMetrics();
+    const NetworkLoadMetrics* timing = loader->response().deprecatedNetworkLoadMetricsOrNull();
     
-    if (timing.secureConnectionStart < 0_ms)
+    if (!timing || timing->secureConnectionStart < 0_ms)
         return 0;
 
-    return resourceLoadTimeRelativeToFetchStart(timing.secureConnectionStart);
+    return resourceLoadTimeRelativeToFetchStart(timing->secureConnectionStart);
 }
 
 unsigned long long PerformanceTiming::requestStart() const
@@ -204,10 +206,12 @@ unsigned long long PerformanceTiming::requestStart() const
     if (!loader)
         return connectEnd();
     
-    const NetworkLoadMetrics& timing = loader->response().deprecatedNetworkLoadMetrics();
+    Seconds requestStart = 0_ms;
+    if (const NetworkLoadMetrics* timing = loader->response().deprecatedNetworkLoadMetricsOrNull())
+        requestStart = timing->requestStart;
     
-    ASSERT(timing.requestStart >= 0_ms);
-    return resourceLoadTimeRelativeToFetchStart(timing.requestStart);
+    ASSERT(requestStart >= 0_ms);
+    return resourceLoadTimeRelativeToFetchStart(requestStart);
 }
 
 unsigned long long PerformanceTiming::responseStart() const
@@ -216,10 +220,12 @@ unsigned long long PerformanceTiming::responseStart() const
     if (!loader)
         return requestStart();
 
-    const NetworkLoadMetrics& timing = loader->response().deprecatedNetworkLoadMetrics();
+    Seconds responseStart = 0_ms;
+    if (const NetworkLoadMetrics* timing = loader->response().deprecatedNetworkLoadMetricsOrNull())
+        responseStart = timing->responseStart;
     
-    ASSERT(timing.responseStart >= 0_ms);
-    return resourceLoadTimeRelativeToFetchStart(timing.responseStart);
+    ASSERT(responseStart >= 0_ms);
+    return resourceLoadTimeRelativeToFetchStart(responseStart);
 }
 
 unsigned long long PerformanceTiming::responseEnd() const
index a36f349..6f63537 100644 (file)
@@ -27,6 +27,7 @@
 #pragma once
 
 #include "HTTPHeaderMap.h"
+#include <wtf/Box.h>
 #include <wtf/Optional.h>
 #include <wtf/Seconds.h>
 #include <wtf/persistence/PersistentDecoder.h>
@@ -47,6 +48,7 @@ enum class NetworkLoadPriority : uint8_t {
 };
 
 class NetworkLoadMetrics {
+    WTF_MAKE_FAST_ALLOCATED(NetworkLoadMetrics);
 public:
     NetworkLoadMetrics()
     {
@@ -184,7 +186,7 @@ public:
 };
 
 #if PLATFORM(COCOA)
-WEBCORE_EXPORT void copyTimingData(NSDictionary *timingData, NetworkLoadMetrics&);
+WEBCORE_EXPORT Box<NetworkLoadMetrics> copyTimingData(NSDictionary *timingData);
 #endif
 
 template<class Encoder>
index a3d4f04..18d4675 100644 (file)
@@ -39,11 +39,13 @@ static bool areContentRangeValuesValid(int64_t firstBytePosition, int64_t lastBy
     // or whose instance-length value is less than or equal to its last-byte-pos value, is invalid.
     if (firstBytePosition < 0)
         return false;
+    ASSERT(firstBytePosition >= 0);
 
     if (lastBytePosition < firstBytePosition)
         return false;
+    ASSERT(lastBytePosition >= 0);
 
-    if (instanceLength == ParsedContentRange::UnknownLength)
+    if (instanceLength == ParsedContentRange::unknownLength)
         return true;
 
     return lastBytePosition < instanceLength;
@@ -96,7 +98,7 @@ static bool parseContentRange(const String& headerValue, int64_t& firstBytePosit
 
     String instanceString = headerValue.substring(instanceLengthSeparatorToken + 1);
     if (instanceString == "*")
-        instanceLength = ParsedContentRange::UnknownLength;
+        instanceLength = ParsedContentRange::unknownLength;
     else {
         if (!instanceString.isAllSpecialCharacters<isASCIIDigit>())
             return false;
@@ -111,7 +113,8 @@ static bool parseContentRange(const String& headerValue, int64_t& firstBytePosit
 
 ParsedContentRange::ParsedContentRange(const String& headerValue)
 {
-    m_isValid = parseContentRange(headerValue, m_firstBytePosition, m_lastBytePosition, m_instanceLength);
+    if (!parseContentRange(headerValue, m_firstBytePosition, m_lastBytePosition, m_instanceLength))
+        m_instanceLength = invalidLength;
 }
 
 ParsedContentRange::ParsedContentRange(int64_t firstBytePosition, int64_t lastBytePosition, int64_t instanceLength)
@@ -119,14 +122,15 @@ ParsedContentRange::ParsedContentRange(int64_t firstBytePosition, int64_t lastBy
     , m_lastBytePosition(lastBytePosition)
     , m_instanceLength(instanceLength)
 {
-    m_isValid = areContentRangeValuesValid(m_firstBytePosition, m_lastBytePosition, m_instanceLength);
+    if (!areContentRangeValuesValid(m_firstBytePosition, m_lastBytePosition, m_instanceLength))
+        m_instanceLength = invalidLength;
 }
 
 String ParsedContentRange::headerValue() const
 {
-    if (!m_isValid)
+    if (!isValid())
         return String();
-    if (m_instanceLength == UnknownLength)
+    if (m_instanceLength == unknownLength)
         return makeString("bytes ", m_firstBytePosition, '-', m_lastBytePosition, "/*");
     return makeString("bytes ", m_firstBytePosition, '-', m_lastBytePosition, '/', m_instanceLength);
 }
index 5a0704b..452581f 100644 (file)
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
  */
 
-#ifndef ParsedContentRange_h
-#define ParsedContentRange_h
+#pragma once
 
 #include <wtf/Forward.h>
+#include <wtf/Markable.h>
 
 namespace WebCore {
 
 class ParsedContentRange {
 public:
+    static constexpr int64_t invalidLength = std::numeric_limits<int64_t>::min();
+    static constexpr int64_t unknownLength = std::numeric_limits<int64_t>::max();
+
     WEBCORE_EXPORT explicit ParsedContentRange(const String&);
-    ParsedContentRange() { }
     WEBCORE_EXPORT ParsedContentRange(int64_t firstBytePosition, int64_t lastBytePosition, int64_t instanceLength);
+    ParsedContentRange() = default;
 
-    bool isValid() const { return m_isValid; }
+    bool isValid() const { return m_instanceLength != invalidLength; }
     int64_t firstBytePosition() const { return m_firstBytePosition; }
     int64_t lastBytePosition() const { return m_lastBytePosition; }
     int64_t instanceLength() const { return m_instanceLength; }
 
+    static ParsedContentRange invalidValue()
+    {
+        return ParsedContentRange();
+    }
+
     WEBCORE_EXPORT String headerValue() const;
 
-    enum { UnknownLength = std::numeric_limits<int64_t>::max() };
+    struct MarkableTraits {
+        static bool isEmptyValue(const ParsedContentRange& range)
+        {
+            return !range.isValid();
+        }
 
-private:
-    template<typename T> static bool isPositive(T);
+        static ParsedContentRange emptyValue()
+        {
+            return ParsedContentRange::invalidValue();
+        }
+    };
 
+private:
     int64_t m_firstBytePosition { 0 };
     int64_t m_lastBytePosition { 0 };
-    int64_t m_instanceLength { UnknownLength };
-    bool m_isValid { false };
+    int64_t m_instanceLength { invalidLength };
 };
 
 }
-
-#endif
index 6807404..19e1a6b 100644 (file)
@@ -27,6 +27,7 @@
 
 #include "AuthenticationClient.h"
 #include "StoredCredentialsPolicy.h"
+#include <wtf/Box.h>
 #include <wtf/MonotonicTime.h>
 #include <wtf/RefCounted.h>
 #include <wtf/RefPtr.h>
@@ -131,9 +132,9 @@ public:
 
 #if PLATFORM(COCOA)
 #if USE(CFURLCONNECTION)
-    static void getConnectionTimingData(CFURLConnectionRef, NetworkLoadMetrics&);
+    static Box<NetworkLoadMetrics> getConnectionTimingData(CFURLConnectionRef);
 #else
-    static void getConnectionTimingData(NSURLConnection *, NetworkLoadMetrics&);
+    static Box<NetworkLoadMetrics> getConnectionTimingData(NSURLConnection *);
 #endif
 #endif
 
index e9acceb..ba02484 100644 (file)
@@ -55,7 +55,12 @@ ResourceResponseBase::ResourceResponseBase()
     , m_haveParsedLastModifiedHeader(false)
     , m_haveParsedContentRangeHeader(false)
     , m_isRedirected(false)
+    , m_isRangeRequested(false)
     , m_isNull(true)
+    , m_usedLegacyTLS(UsedLegacyTLS::No)
+    , m_tainting(Tainting::Basic)
+    , m_source(Source::Unknown)
+    , m_type(Type::Default)
 {
 }
 
@@ -72,7 +77,12 @@ ResourceResponseBase::ResourceResponseBase(const URL& url, const String& mimeTyp
     , m_haveParsedLastModifiedHeader(false)
     , m_haveParsedContentRangeHeader(false)
     , m_isRedirected(false)
+    , m_isRangeRequested(false)
     , m_isNull(false)
+    , m_usedLegacyTLS(UsedLegacyTLS::No)
+    , m_tainting(Tainting::Basic)
+    , m_source(Source::Unknown)
+    , m_type(Type::Default)
 {
 }
 
@@ -90,7 +100,8 @@ ResourceResponseBase::CrossThreadData ResourceResponseBase::crossThreadData() co
     data.httpVersion = httpVersion().isolatedCopy();
 
     data.httpHeaderFields = httpHeaderFields().isolatedCopy();
-    data.networkLoadMetrics = m_networkLoadMetrics.isolatedCopy();
+    if (m_networkLoadMetrics)
+        data.networkLoadMetrics = m_networkLoadMetrics->isolatedCopy();
     data.type = m_type;
     data.tainting = m_tainting;
     data.isRedirected = m_isRedirected;
@@ -113,7 +124,10 @@ ResourceResponse ResourceResponseBase::fromCrossThreadData(CrossThreadData&& dat
     response.setHTTPVersion(data.httpVersion);
 
     response.m_httpHeaderFields = WTFMove(data.httpHeaderFields);
-    response.m_networkLoadMetrics = data.networkLoadMetrics;
+    if (data.networkLoadMetrics)
+        response.m_networkLoadMetrics = Box<NetworkLoadMetrics>::create(WTFMove(data.networkLoadMetrics.value()));
+    else
+        response.m_networkLoadMetrics = nullptr;
     response.m_type = data.type;
     response.m_tainting = data.tainting;
     response.m_isRedirected = data.isRedirected;
@@ -808,8 +822,16 @@ bool ResourceResponseBase::compare(const ResourceResponse& a, const ResourceResp
         return false;
     if (a.httpHeaderFields() != b.httpHeaderFields())
         return false;
-    if (a.deprecatedNetworkLoadMetrics() != b.deprecatedNetworkLoadMetrics())
-        return false;
+    if (a.m_networkLoadMetrics.get() != b.m_networkLoadMetrics.get()) {
+        if (!a.m_networkLoadMetrics) {
+            if (NetworkLoadMetrics() != *b.m_networkLoadMetrics.get())
+                return false;
+        } else if (!b.m_networkLoadMetrics) {
+            if (NetworkLoadMetrics() != *a.m_networkLoadMetrics.get())
+                return false;
+        } else if (*a.m_networkLoadMetrics.get() != *b.m_networkLoadMetrics.get())
+            return false;
+    }
     return ResourceResponse::platformCompare(a, b);
 }
 
index 8894416..09c75b3 100644 (file)
@@ -31,6 +31,7 @@
 #include "HTTPHeaderMap.h"
 #include "NetworkLoadMetrics.h"
 #include "ParsedContentRange.h"
+#include <wtf/Box.h>
 #include <wtf/Markable.h>
 #include <wtf/URL.h>
 #include <wtf/WallTime.h>
@@ -42,6 +43,8 @@ class ResourceResponse;
 bool isScriptAllowedByNosniff(const ResourceResponse&);
 
 enum class UsedLegacyTLS : bool { No, Yes };
+static constexpr unsigned bitWidthOfUsedLegacyTLS = 1;
+static_assert(static_cast<unsigned>(UsedLegacyTLS::Yes) <= ((1U << bitWidthOfUsedLegacyTLS) - 1));
 
 // Do not use this class directly, use the class ResourceResponse instead
 class ResourceResponseBase {
@@ -68,7 +71,7 @@ public:
         String httpStatusText;
         String httpVersion;
         HTTPHeaderMap httpHeaderFields;
-        NetworkLoadMetrics networkLoadMetrics;
+        Optional<NetworkLoadMetrics> networkLoadMetrics;
         Type type;
         Tainting tainting;
         bool isRedirected;
@@ -150,6 +153,9 @@ public:
     const ParsedContentRange& contentRange() const;
 
     enum class Source : uint8_t { Unknown, Network, DiskCache, DiskCacheAfterValidation, MemoryCache, MemoryCacheAfterValidation, ServiceWorker, ApplicationCache, DOMCache, InspectorOverride };
+    static constexpr unsigned bitWidthOfSource = 4;
+    static_assert(static_cast<unsigned>(Source::InspectorOverride) <= ((1U << bitWidthOfSource) - 1));
+
     WEBCORE_EXPORT Source source() const;
     void setSource(Source source)
     {
@@ -160,7 +166,16 @@ public:
     // FIXME: This should be eliminated from ResourceResponse.
     // Network loading metrics should be delivered via didFinishLoad
     // and should not be part of the ResourceResponse.
-    NetworkLoadMetrics& deprecatedNetworkLoadMetrics() const { return m_networkLoadMetrics; }
+    const NetworkLoadMetrics* deprecatedNetworkLoadMetricsOrNull() const
+    {
+        if (m_networkLoadMetrics)
+            return m_networkLoadMetrics.get();
+        return nullptr;
+    }
+    void setDeprecatedNetworkLoadMetrics(Box<NetworkLoadMetrics>&& metrics)
+    {
+        m_networkLoadMetrics = WTFMove(metrics);
+    }
 
     // The ResourceResponse subclass may "shadow" this method to provide platform-specific memory usage information
     unsigned memoryUsage() const
@@ -222,7 +237,7 @@ protected:
     AtomString m_httpStatusText;
     AtomString m_httpVersion;
     HTTPHeaderMap m_httpHeaderFields;
-    mutable NetworkLoadMetrics m_networkLoadMetrics;
+    Box<NetworkLoadMetrics> m_networkLoadMetrics;
 
     mutable Optional<CertificateInfo> m_certificateInfo;
 
@@ -241,18 +256,17 @@ private:
     mutable bool m_haveParsedLastModifiedHeader : 1;
     mutable bool m_haveParsedContentRangeHeader : 1;
     bool m_isRedirected : 1;
+    bool m_isRangeRequested : 1;
 protected:
     bool m_isNull : 1;
-
+    unsigned m_initLevel : 3; // Controlled by ResourceResponse.
+    mutable UsedLegacyTLS m_usedLegacyTLS : bitWidthOfUsedLegacyTLS;
 private:
-    Source m_source { Source::Unknown };
-    Type m_type { Type::Default };
-    Tainting m_tainting { Tainting::Basic };
-    bool m_isRangeRequested { false };
-
+    Tainting m_tainting : bitWidthOfTainting;
+    Source m_source : bitWidthOfSource;
+    Type m_type : bitWidthOfType;
 protected:
     short m_httpStatusCode { 0 };
-    mutable UsedLegacyTLS m_usedLegacyTLS { UsedLegacyTLS::No };
 };
 
 inline bool operator==(const ResourceResponse& a, const ResourceResponse& b) { return ResourceResponseBase::compare(a, b); }
@@ -276,7 +290,7 @@ void ResourceResponseBase::encode(Encoder& encoder) const
 
     // We don't want to put the networkLoadMetrics info
     // into the disk cache, because we will never use the old info.
-    if (Encoder::isIPCEncoder)
+    if constexpr (Encoder::isIPCEncoder)
         encoder << m_networkLoadMetrics;
 
     encoder << m_httpStatusCode;
@@ -285,7 +299,8 @@ void ResourceResponseBase::encode(Encoder& encoder) const
     encoder.encodeEnum(m_type);
     encoder.encodeEnum(m_tainting);
     encoder << m_isRedirected;
-    encoder << m_usedLegacyTLS;
+    UsedLegacyTLS usedLegacyTLS = m_usedLegacyTLS;
+    encoder << usedLegacyTLS;
     encoder << m_isRangeRequested;
 }
 
@@ -318,24 +333,34 @@ bool ResourceResponseBase::decode(Decoder& decoder, ResourceResponseBase& respon
     if (!decoder.decode(response.m_httpHeaderFields))
         return false;
     // The networkLoadMetrics info is only send over IPC and not stored in disk cache.
-    if (Decoder::isIPCDecoder && !decoder.decode(response.m_networkLoadMetrics))
-        return false;
+    if constexpr (Decoder::isIPCDecoder) {
+        if (!decoder.decode(response.m_networkLoadMetrics))
+            return false;
+    }
     if (!decoder.decode(response.m_httpStatusCode))
         return false;
     if (!decoder.decode(response.m_certificateInfo))
         return false;
-    if (!decoder.decodeEnum(response.m_source))
+    Source source = Source::Unknown;
+    if (!decoder.decodeEnum(source))
         return false;
-    if (!decoder.decodeEnum(response.m_type))
+    response.m_source = source;
+    Type type = Type::Default;
+    if (!decoder.decodeEnum(type))
         return false;
-    if (!decoder.decodeEnum(response.m_tainting))
+    response.m_type = type;
+    Tainting tainting = Tainting::Basic;
+    if (!decoder.decodeEnum(tainting))
         return false;
+    response.m_tainting = tainting;
     bool isRedirected = false;
     if (!decoder.decode(isRedirected))
         return false;
     response.m_isRedirected = isRedirected;
-    if (!decoder.decode(response.m_usedLegacyTLS))
+    UsedLegacyTLS usedLegacyTLS = UsedLegacyTLS::No;
+    if (!decoder.decode(usedLegacyTLS))
         return false;
+    response.m_usedLegacyTLS = usedLegacyTLS;
     bool isRangeRequested = false;
     if (!decoder.decode(isRangeRequested))
         return false;
index a9f39f3..2ac7edb 100644 (file)
@@ -39,30 +39,30 @@ namespace WebCore {
 class ResourceResponse : public ResourceResponseBase {
 public:
     ResourceResponse()
-        : m_initLevel(AllFields)
     {
+        m_initLevel = AllFields;
     }
 
 #if USE(CFURLCONNECTION)
     ResourceResponse(CFURLResponseRef cfResponse)
-        : m_initLevel(Uninitialized)
-        , m_cfResponse(cfResponse)
+        : m_cfResponse(cfResponse)
     {
+        m_initLevel = Uninitialized;
         m_isNull = !cfResponse;
     }
 #else
     ResourceResponse(NSURLResponse *nsResponse)
-        : m_initLevel(Uninitialized)
-        , m_nsResponse(nsResponse)
+        : m_nsResponse(nsResponse)
     {
+        m_initLevel = Uninitialized;
         m_isNull = !nsResponse;
     }
 #endif
 
     ResourceResponse(const URL& url, const String& mimeType, long long expectedLength, const String& textEncodingName)
         : ResourceResponseBase(url, mimeType, expectedLength, textEncodingName)
-        , m_initLevel(AllFields)
     {
+        m_initLevel = AllFields;
     }
 
 #if PLATFORM(COCOA)
@@ -106,8 +106,6 @@ private:
 
     static bool platformCompare(const ResourceResponse& a, const ResourceResponse& b);
 
-    unsigned m_initLevel : 3;
-
 #if USE(QUICK_LOOK)
     bool m_isQuickLook { false };
 #endif
index fbb8d68..d1e63f9 100644 (file)
@@ -37,10 +37,12 @@ static double timingValue(NSDictionary *timingData, NSString *key)
     return 0.0;
 }
     
-void copyTimingData(NSDictionary *timingData, NetworkLoadMetrics& timing)
+Box<NetworkLoadMetrics> copyTimingData(NSDictionary *timingData)
 {
     if (!timingData)
-        return;
+        return nullptr;
+
+    Box<NetworkLoadMetrics> timing = Box<NetworkLoadMetrics>::create();
 
     // This is not the navigationStart time in monotonic time, but the other times are relative to this time
     // and only the differences between times are stored.
@@ -54,15 +56,16 @@ void copyTimingData(NSDictionary *timingData, NetworkLoadMetrics& timing)
     double requestStart = timingValue(timingData, @"_kCFNTimingDataRequestStart");
     double responseStart = timingValue(timingData, @"_kCFNTimingDataResponseStart");
 
-    timing.domainLookupStart = Seconds(domainLookupStart <= 0 ? -1 : domainLookupStart - referenceStart);
-    timing.domainLookupEnd = Seconds(domainLookupEnd <= 0 ? -1 : domainLookupEnd - referenceStart);
-    timing.connectStart = Seconds(connectStart <= 0 ? -1 : connectStart - referenceStart);
-    timing.secureConnectionStart = Seconds(secureConnectionStart <= 0 ? -1 : secureConnectionStart - referenceStart);
-    timing.connectEnd = Seconds(connectEnd <= 0 ? -1 : connectEnd - referenceStart);
-    timing.requestStart = Seconds(requestStart <= 0 ? 0 : requestStart - referenceStart);
-    timing.responseStart = Seconds(responseStart <= 0 ? 0 : responseStart - referenceStart);
+    timing->domainLookupStart = Seconds(domainLookupStart <= 0 ? -1 : domainLookupStart - referenceStart);
+    timing->domainLookupEnd = Seconds(domainLookupEnd <= 0 ? -1 : domainLookupEnd - referenceStart);
+    timing->connectStart = Seconds(connectStart <= 0 ? -1 : connectStart - referenceStart);
+    timing->secureConnectionStart = Seconds(secureConnectionStart <= 0 ? -1 : secureConnectionStart - referenceStart);
+    timing->connectEnd = Seconds(connectEnd <= 0 ? -1 : connectEnd - referenceStart);
+    timing->requestStart = Seconds(requestStart <= 0 ? 0 : requestStart - referenceStart);
+    timing->responseStart = Seconds(responseStart <= 0 ? 0 : responseStart - referenceStart);
 
     // NOTE: responseEnd is not populated in this code path.
+    return timing;
 }
     
 }
index 6179c8e..21dc446 100644 (file)
@@ -108,7 +108,7 @@ void CurlResourceHandleDelegate::curlDidReceiveResponse(CurlRequest& request, Cu
 
     m_response = ResourceResponse(receivedResponse);
     m_response.setCertificateInfo(WTFMove(receivedResponse.certificateInfo));
-    m_response.setDeprecatedNetworkLoadMetrics(WTFMove(receivedResponse.networkLoadMetrics));
+    m_response.setDeprecatedNetworkLoadMetrics(Box<NetworkLoadMetrics>::create(WTFMove(receivedResponse.networkLoadMetrics)));
 
     handleCookieHeaders(d(), request.resourceRequest(), receivedResponse);
 
index 7f5788f..5018e38 100644 (file)
@@ -51,7 +51,6 @@ public:
     void appendHTTPHeaderField(const String&);
 
     void setCertificateInfo(CertificateInfo&&);
-    void setDeprecatedNetworkLoadMetrics(NetworkLoadMetrics&&);
 
     bool shouldRedirect();
     bool isMovedPermanently() const;
index 190a0f2..a0f394f 100644 (file)
@@ -151,11 +151,6 @@ void ResourceResponse::setCertificateInfo(CertificateInfo&& certificateInfo)
     m_certificateInfo = WTFMove(certificateInfo);
 }
 
-void ResourceResponse::setDeprecatedNetworkLoadMetrics(NetworkLoadMetrics&& networkLoadMetrics)
-{
-    m_networkLoadMetrics = WTFMove(networkLoadMetrics);
-}
-
 String ResourceResponse::platformSuggestedFilename() const
 {
     return filenameFromHTTPContentDisposition(httpHeaderField(HTTPHeaderName::ContentDisposition));
index 03fa268..767c579 100644 (file)
@@ -655,9 +655,9 @@ void ResourceHandle::receivedChallengeRejection(const AuthenticationChallenge& c
     clearAuthentication();
 }
 
-void ResourceHandle::getConnectionTimingData(NSURLConnection *connection, NetworkLoadMetrics& timing)
+Box<NetworkLoadMetrics> ResourceHandle::getConnectionTimingData(NSURLConnection *connection)
 {
-    copyTimingData([connection _timingData], timing);
+    return copyTimingData([connection _timingData]);
 }
 
 } // namespace WebCore
index 830ed06..19e6205 100644 (file)
@@ -252,7 +252,7 @@ ALLOW_DEPRECATED_IMPLEMENTATIONS_END
 
         ResourceResponse resourceResponse(r.get());
         resourceResponse.setSource(ResourceResponse::Source::Network);
-        ResourceHandle::getConnectionTimingData(connection.get(), resourceResponse.deprecatedNetworkLoadMetrics());
+        resourceResponse.setDeprecatedNetworkLoadMetrics(ResourceHandle::getConnectionTimingData(connection.get()));
 
         m_handle->didReceiveResponse(WTFMove(resourceResponse), [self, protectedSelf = WTFMove(protectedSelf)] {
             m_semaphore.signal();
index 2f52fa3..808e7f2 100644 (file)
@@ -1,3 +1,20 @@
+2020-02-14  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        NetworkLoadMetrics should be shared by multiple ResourceResponse instances
+        https://bugs.webkit.org/show_bug.cgi?id=207747
+
+        Reviewed by Keith Miller.
+
+        Add ArgumentCoder support for Box<T>.
+
+        * NetworkProcess/cocoa/NetworkSessionCocoa.mm:
+        (-[WKNetworkSessionDelegate URLSession:dataTask:didReceiveResponse:completionHandler:]):
+        * NetworkProcess/soup/NetworkDataTaskSoup.cpp:
+        (WebKit::NetworkDataTaskSoup::dispatchDidReceiveResponse):
+        * Platform/IPC/ArgumentCoders.h:
+        (IPC::ArgumentCoder<Box<T>>::encode):
+        (IPC::ArgumentCoder<Box<T>>::decode):
+
 2020-02-14  Alberto Garcia  <berto@igalia.com>
 
         [WPE] WPEQtView.h includes the uninstalled WPEQtViewBackend.h
index 2a35924..b9f0154 100644 (file)
@@ -849,7 +849,7 @@ static inline void processServerTrustEvaluation(NetworkSessionCocoa& session, Se
 
         // FIXME: This cannot be eliminated until other code no longer relies on ResourceResponse's
         // NetworkLoadMetrics. For example, PerformanceTiming.
-        copyTimingData([dataTask _timingData], resourceResponse.deprecatedNetworkLoadMetrics());
+        resourceResponse.setDeprecatedNetworkLoadMetrics(WebCore::copyTimingData([dataTask _timingData]));
 
         auto completionHandlerCopy = Block_copy(completionHandler);
         networkDataTask->didReceiveResponse(WTFMove(resourceResponse), negotiatedLegacyTLS, [completionHandlerCopy, taskIdentifier](WebCore::PolicyAction policyAction) {
index ce05417..4736358 100644 (file)
@@ -149,7 +149,7 @@ void NetworkDataTaskCurl::curlDidReceiveResponse(CurlRequest& request, CurlRespo
 
     m_response = ResourceResponse(receivedResponse);
     m_response.setCertificateInfo(WTFMove(receivedResponse.certificateInfo));
-    m_response.setDeprecatedNetworkLoadMetrics(WTFMove(receivedResponse.networkLoadMetrics));
+    m_response.setDeprecatedNetworkLoadMetrics(Box<NetworkLoadMetrics>::create(WTFMove(receivedResponse.networkLoadMetrics)));
 
     handleCookieHeaders(request.resourceRequest(), receivedResponse);
 
index 9e41916..b2dd9e5 100644 (file)
@@ -367,16 +367,17 @@ void NetworkDataTaskSoup::dispatchDidReceiveResponse()
 {
     ASSERT(!m_response.isNull());
 
+    Box<NetworkLoadMetrics> timing = Box<NetworkLoadMetrics>::create();
+    timing->responseStart = m_networkLoadMetrics.responseStart;
+    timing->domainLookupStart = m_networkLoadMetrics.domainLookupStart;
+    timing->domainLookupEnd = m_networkLoadMetrics.domainLookupEnd;
+    timing->connectStart = m_networkLoadMetrics.connectStart;
+    timing->secureConnectionStart = m_networkLoadMetrics.secureConnectionStart;
+    timing->connectEnd = m_networkLoadMetrics.connectEnd;
+    timing->requestStart = m_networkLoadMetrics.requestStart;
+    timing->responseStart = m_networkLoadMetrics.responseStart;
     // FIXME: Remove this once nobody depends on deprecatedNetworkLoadMetrics.
-    NetworkLoadMetrics& deprecatedResponseMetrics = m_response.deprecatedNetworkLoadMetrics();
-    deprecatedResponseMetrics.responseStart = m_networkLoadMetrics.responseStart;
-    deprecatedResponseMetrics.domainLookupStart = m_networkLoadMetrics.domainLookupStart;
-    deprecatedResponseMetrics.domainLookupEnd = m_networkLoadMetrics.domainLookupEnd;
-    deprecatedResponseMetrics.connectStart = m_networkLoadMetrics.connectStart;
-    deprecatedResponseMetrics.secureConnectionStart = m_networkLoadMetrics.secureConnectionStart;
-    deprecatedResponseMetrics.connectEnd = m_networkLoadMetrics.connectEnd;
-    deprecatedResponseMetrics.requestStart = m_networkLoadMetrics.requestStart;
-    deprecatedResponseMetrics.responseStart = m_networkLoadMetrics.responseStart;
+    m_response.setDeprecatedNetworkLoadMetrics(WTFMove(timing));
 
     didReceiveResponse(ResourceResponse(m_response), NegotiatedLegacyTLS::No, [this, protectedThis = makeRef(*this)](PolicyAction policyAction) {
         if (m_state == State::Canceling || m_state == State::Completed) {
index 9acd7fd..58c4564 100644 (file)
@@ -28,6 +28,7 @@
 #include "Decoder.h"
 #include "Encoder.h"
 #include <utility>
+#include <wtf/Box.h>
 #include <wtf/Forward.h>
 #include <wtf/MonotonicTime.h>
 #include <wtf/SHA1.h>
@@ -123,6 +124,54 @@ template<typename T> struct ArgumentCoder<Optional<T>> {
     }
 };
 
+template<typename T> struct ArgumentCoder<Box<T>> {
+    static void encode(Encoder& encoder, const Box<T>& box)
+    {
+        if (!box) {
+            encoder << false;
+            return;
+        }
+
+        encoder << true;
+        encoder << *box.get();
+    }
+
+    static bool decode(Decoder& decoder, Box<T>& box)
+    {
+        bool isEngaged;
+        if (!decoder.decode(isEngaged))
+            return false;
+
+        if (!isEngaged) {
+            box = nullptr;
+            return true;
+        }
+
+        Box<T> value = Box<T>::create();
+        if (!decoder.decode(*value))
+            return false;
+
+        box = WTFMove(value);
+        return true;
+    }
+
+    static Optional<Box<T>> decode(Decoder& decoder)
+    {
+        Optional<bool> isEngaged;
+        decoder >> isEngaged;
+        if (!isEngaged)
+            return WTF::nullopt;
+        if (*isEngaged) {
+            Optional<T> value;
+            decoder >> value;
+            if (!value)
+                return WTF::nullopt;
+            return Optional<Box<T>>(Box<T>::create(WTFMove(*value)));
+        }
+        return Optional<Box<T>>(Box<T>(nullptr));
+    }
+};
+
 template<typename T, typename U> struct ArgumentCoder<std::pair<T, U>> {
     static void encode(Encoder& encoder, const std::pair<T, U>& pair)
     {
index 61dc1e9..3f17345 100644 (file)
@@ -1,3 +1,13 @@
+2020-02-14  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        NetworkLoadMetrics should be shared by multiple ResourceResponse instances
+        https://bugs.webkit.org/show_bug.cgi?id=207747
+
+        Reviewed by Keith Miller.
+
+        * TestWebKitAPI/Tests/WebCore/ParsedContentRange.cpp:
+        (TestWebKitAPI::TEST):
+
 2020-02-14  Alex Christensen  <achristensen@webkit.org>
 
         Allow UIDNAInfo.errors from uidna_nameToUnicode that would not cause URL parsing failures
index 9863883..20a957e 100644 (file)
@@ -40,7 +40,7 @@ TEST(WebCore, ParsedContentRangeFromString)
     ASSERT_EQ(0, ParsedContentRange("bytes 0-1/2").firstBytePosition());
     ASSERT_EQ(1, ParsedContentRange("bytes 0-1/2").lastBytePosition());
     ASSERT_EQ(2, ParsedContentRange("bytes 0-1/2").instanceLength());
-    ASSERT_EQ(ParsedContentRange::UnknownLength, ParsedContentRange("bytes 0-1/*").instanceLength());
+    ASSERT_EQ(ParsedContentRange::unknownLength, ParsedContentRange("bytes 0-1/*").instanceLength());
 
     // Whitespace errors
     ASSERT_FALSE(ParsedContentRange("bytes  0-1/*").isValid());
@@ -77,7 +77,7 @@ TEST(WebCore, ParsedContentRangeFromString)
 TEST(WebCore, ParsedContentRangeFromValues)
 {
     ASSERT_TRUE(ParsedContentRange(0, 1, 2).isValid());
-    ASSERT_TRUE(ParsedContentRange(0, 1, ParsedContentRange::UnknownLength).isValid());
+    ASSERT_TRUE(ParsedContentRange(0, 1, ParsedContentRange::unknownLength).isValid());
     ASSERT_FALSE(ParsedContentRange().isValid());
     ASSERT_FALSE(ParsedContentRange(1, 0, 2).isValid());
     ASSERT_FALSE(ParsedContentRange(0, 2, 1).isValid());
@@ -91,7 +91,7 @@ TEST(WebCore, ParsedContentRangeFromValues)
 TEST(WebCore, ParsedContentRangeToString)
 {
     ASSERT_STREQ("bytes 0-1/2", ParsedContentRange(0, 1, 2).headerValue().utf8().data());
-    ASSERT_STREQ("bytes 0-1/*", ParsedContentRange(0, 1, ParsedContentRange::UnknownLength).headerValue().utf8().data());
+    ASSERT_STREQ("bytes 0-1/*", ParsedContentRange(0, 1, ParsedContentRange::unknownLength).headerValue().utf8().data());
     ASSERT_STREQ("", ParsedContentRange().headerValue().utf8().data());
 }