Title: [277493] trunk
Revision
277493
Author
achristen...@apple.com
Date
2021-05-14 09:56:47 -0700 (Fri, 14 May 2021)

Log Message

Resource Timing: secureConnectionStart == 0 when a connection is re-used
https://bugs.webkit.org/show_bug.cgi?id=225733

Reviewed by Chris Dumez.

LayoutTests/imported/w3c:

* web-platform-tests/resource-timing/resource_connection_reuse_mixed_content-expected.txt:

Source/WebCore:

Covered by a newly-passing web platform test, which was already passed by Chrome and Firefox.

* page/PerformanceResourceTiming.cpp:
(WebCore::PerformanceResourceTiming::secureConnectionStart const):
* platform/network/NetworkLoadMetrics.h:
I use a sentinel value to distinguish no secure connection from a reused secure connection.
* platform/network/ResourceHandle.h:
* platform/network/cocoa/NetworkLoadMetrics.mm:
(WebCore::packageTimingData):
(WebCore::copyTimingData):
Introduce a way to get timing data from CFNetwork using a more modern API.
(WebCore::timingValue): Deleted.
* platform/network/cocoa/WebCoreNSURLSession.mm:
(-[WebCoreNSURLSessionDataTask _timingData]):
* platform/network/mac/ResourceHandleMac.mm:
(WebCore::ResourceHandle::getConnectionTimingData): Deleted.
* platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.mm:
(-[WebCoreResourceHandleAsOperationQueueDelegate connection:didReceiveResponse:]):

Source/WebKit:

* NetworkProcess/cocoa/NetworkSessionCocoa.mm:
(-[WKNetworkSessionDelegate URLSession:dataTask:didReceiveResponse:completionHandler:]):

LayoutTests:

* TestExpectations:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (277492 => 277493)


--- trunk/LayoutTests/ChangeLog	2021-05-14 16:10:38 UTC (rev 277492)
+++ trunk/LayoutTests/ChangeLog	2021-05-14 16:56:47 UTC (rev 277493)
@@ -1,3 +1,12 @@
+2021-05-14  Alex Christensen  <achristen...@webkit.org>
+
+        Resource Timing: secureConnectionStart == 0 when a connection is re-used
+        https://bugs.webkit.org/show_bug.cgi?id=225733
+
+        Reviewed by Chris Dumez.
+
+        * TestExpectations:
+
 2021-05-14  Amir Mark Jr  <amir_m...@apple.com>
 
         [MacOS] imported/w3c/web-platform-tests/webxr/xrBoundedReferenceSpace_updates.https.html is a flaky failure

Modified: trunk/LayoutTests/TestExpectations (277492 => 277493)


--- trunk/LayoutTests/TestExpectations	2021-05-14 16:10:38 UTC (rev 277492)
+++ trunk/LayoutTests/TestExpectations	2021-05-14 16:56:47 UTC (rev 277493)
@@ -940,7 +940,6 @@
 imported/w3c/web-platform-tests/resource-timing/cors-preflight.any.html [ Failure Pass ]
 imported/w3c/web-platform-tests/resource-timing/crossorigin-sandwich-TAO.sub.html [ Pass Failure ]
 imported/w3c/web-platform-tests/resource-timing/crossorigin-sandwich-partial-TAO.sub.html [ Pass Failure ]
-imported/w3c/web-platform-tests/resource-timing/resource_connection_reuse_mixed_content.html [ Pass Failure ]
 
 imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-default-feature-policy.https.html [ DumpJSConsoleLogInStdErr ]
 imported/w3c/web-platform-tests/mediacapture-record/passthrough/MediaRecorder-passthrough.https.html [ Failure Timeout Pass ]

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (277492 => 277493)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2021-05-14 16:10:38 UTC (rev 277492)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2021-05-14 16:56:47 UTC (rev 277493)
@@ -1,3 +1,12 @@
+2021-05-14  Alex Christensen  <achristen...@webkit.org>
+
+        Resource Timing: secureConnectionStart == 0 when a connection is re-used
+        https://bugs.webkit.org/show_bug.cgi?id=225733
+
+        Reviewed by Chris Dumez.
+
+        * web-platform-tests/resource-timing/resource_connection_reuse_mixed_content-expected.txt:
+
 2021-05-13  Dean Jackson  <d...@apple.com>
 
         [WebXR] Allow WebXR to be tested on PLATFORM(COCOA)

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/resource-timing/resource_connection_reuse_mixed_content-expected.txt (277492 => 277493)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/resource-timing/resource_connection_reuse_mixed_content-expected.txt	2021-05-14 16:10:38 UTC (rev 277492)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/resource-timing/resource_connection_reuse_mixed_content-expected.txt	2021-05-14 16:56:47 UTC (rev 277493)
@@ -8,7 +8,7 @@
 PASS There should be 2 PerformanceEntries
 PASS connectStart and fetchStart should be the same
 PASS connectEnd and fetchStart should be the same
-FAIL secureConnectionStart and fetchStart should be the same assert_equals: secureConnectionStart and fetchStart should be the same expected 0 but got 18.000000000000004
+PASS secureConnectionStart and fetchStart should be the same
 PASS domainLookupStart and fetchStart should be the same
 PASS domainLookupEnd and fetchStart should be the same
 

Modified: trunk/Source/WebCore/ChangeLog (277492 => 277493)


--- trunk/Source/WebCore/ChangeLog	2021-05-14 16:10:38 UTC (rev 277492)
+++ trunk/Source/WebCore/ChangeLog	2021-05-14 16:56:47 UTC (rev 277493)
@@ -1,3 +1,29 @@
+2021-05-14  Alex Christensen  <achristen...@webkit.org>
+
+        Resource Timing: secureConnectionStart == 0 when a connection is re-used
+        https://bugs.webkit.org/show_bug.cgi?id=225733
+
+        Reviewed by Chris Dumez.
+
+        Covered by a newly-passing web platform test, which was already passed by Chrome and Firefox.
+
+        * page/PerformanceResourceTiming.cpp:
+        (WebCore::PerformanceResourceTiming::secureConnectionStart const):
+        * platform/network/NetworkLoadMetrics.h:
+        I use a sentinel value to distinguish no secure connection from a reused secure connection.
+        * platform/network/ResourceHandle.h:
+        * platform/network/cocoa/NetworkLoadMetrics.mm:
+        (WebCore::packageTimingData):
+        (WebCore::copyTimingData):
+        Introduce a way to get timing data from CFNetwork using a more modern API.
+        (WebCore::timingValue): Deleted.
+        * platform/network/cocoa/WebCoreNSURLSession.mm:
+        (-[WebCoreNSURLSessionDataTask _timingData]):
+        * platform/network/mac/ResourceHandleMac.mm:
+        (WebCore::ResourceHandle::getConnectionTimingData): Deleted.
+        * platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.mm:
+        (-[WebCoreResourceHandleAsOperationQueueDelegate connection:didReceiveResponse:]):
+
 2021-05-14  Chris Dumez  <cdu...@apple.com>
 
         Rename FileSystem::getFileSize() to FileSystem::fileSize()

Modified: trunk/Source/WebCore/page/PerformanceResourceTiming.cpp (277492 => 277493)


--- trunk/Source/WebCore/page/PerformanceResourceTiming.cpp	2021-05-14 16:10:38 UTC (rev 277492)
+++ trunk/Source/WebCore/page/PerformanceResourceTiming.cpp	2021-05-14 16:56:47 UTC (rev 277493)
@@ -176,6 +176,9 @@
     if (!m_shouldReportDetails)
         return 0.0;
 
+    if (m_networkLoadMetrics.secureConnectionStart == reusedTLSConnectionSentinel)
+        return fetchStart();
+
     if (m_networkLoadMetrics.secureConnectionStart <= 0_ms)
         return 0.0;
 

Modified: trunk/Source/WebCore/platform/network/NetworkLoadMetrics.h (277492 => 277493)


--- trunk/Source/WebCore/platform/network/NetworkLoadMetrics.h	2021-05-14 16:10:38 UTC (rev 277492)
+++ trunk/Source/WebCore/platform/network/NetworkLoadMetrics.h	2021-05-14 16:56:47 UTC (rev 277493)
@@ -46,7 +46,9 @@
 #endif
 
 #if PLATFORM(COCOA)
-OBJC_CLASS NSDictionary;
+OBJC_CLASS NSURLConnection;
+OBJC_CLASS NSURLResponse;
+OBJC_CLASS NSURLSessionTaskTransactionMetrics;
 #endif
 
 namespace WebCore {
@@ -60,6 +62,8 @@
 
 NETWORK_LOAD_METRICS_ADDITIONS_1;
 
+constexpr Seconds reusedTLSConnectionSentinel { -2 };
+
 class NetworkLoadMetricsWithoutNonTimingData {
     WTF_MAKE_FAST_ALLOCATED(NetworkLoadMetricsWithoutNonTimingData);
 public:
@@ -196,7 +200,8 @@
 };
 
 #if PLATFORM(COCOA)
-WEBCORE_EXPORT Box<NetworkLoadMetrics> copyTimingData(NSDictionary *timingData);
+Box<NetworkLoadMetrics> copyTimingData(NSURLConnection *, NSURLResponse *);
+WEBCORE_EXPORT Box<NetworkLoadMetrics> copyTimingData(NSURLSessionTaskTransactionMetrics *incompleteMetrics);
 #endif
 
 template<class Encoder>

Modified: trunk/Source/WebCore/platform/network/ResourceHandle.h (277492 => 277493)


--- trunk/Source/WebCore/platform/network/ResourceHandle.h	2021-05-14 16:10:38 UTC (rev 277492)
+++ trunk/Source/WebCore/platform/network/ResourceHandle.h	2021-05-14 16:56:47 UTC (rev 277493)
@@ -128,14 +128,6 @@
 #endif
 
 #if PLATFORM(COCOA)
-#if USE(CFURLCONNECTION)
-    static Box<NetworkLoadMetrics> getConnectionTimingData(CFURLConnectionRef);
-#else
-    static Box<NetworkLoadMetrics> getConnectionTimingData(NSURLConnection *);
-#endif
-#endif
-
-#if PLATFORM(COCOA)
     void schedule(WTF::SchedulePair&);
     void unschedule(WTF::SchedulePair&);
 #endif

Modified: trunk/Source/WebCore/platform/network/cocoa/NetworkLoadMetrics.mm (277492 => 277493)


--- trunk/Source/WebCore/platform/network/cocoa/NetworkLoadMetrics.mm	2021-05-14 16:10:38 UTC (rev 277492)
+++ trunk/Source/WebCore/platform/network/cocoa/NetworkLoadMetrics.mm	2021-05-14 16:56:47 UTC (rev 277493)
@@ -30,43 +30,67 @@
 
 namespace WebCore {
 
-static double timingValue(NSDictionary *timingData, NSString *key)
+static Box<NetworkLoadMetrics> packageTimingData(double fetchStart, double domainLookupStart, double domainLookupEnd, double connectStart, double secureConnectionStart, double connectEnd, double requestStart, double responseStart, bool reusedTLSConnection, NSString *protocol)
 {
-    if (id object = [timingData objectForKey:key])
-        return [object doubleValue];
-    return 0.0;
+    auto timing = Box<NetworkLoadMetrics>::create();
+
+    timing->fetchStart = Seconds(fetchStart);
+    timing->domainLookupStart = Seconds(domainLookupStart <= 0 ? -1 : domainLookupStart - fetchStart);
+    timing->domainLookupEnd = Seconds(domainLookupEnd <= 0 ? -1 : domainLookupEnd - fetchStart);
+    timing->connectStart = Seconds(connectStart <= 0 ? -1 : connectStart - fetchStart);
+    if (reusedTLSConnection && [protocol isEqualToString:@"https"])
+        timing->secureConnectionStart = reusedTLSConnectionSentinel;
+    else
+        timing->secureConnectionStart = Seconds(secureConnectionStart <= 0 ? -1 : secureConnectionStart - fetchStart);
+    timing->connectEnd = Seconds(connectEnd <= 0 ? -1 : connectEnd - fetchStart);
+    timing->requestStart = Seconds(requestStart <= 0 ? 0 : requestStart - fetchStart);
+    timing->responseStart = Seconds(responseStart <= 0 ? 0 : responseStart - fetchStart);
+
+    // NOTE: responseEnd is not populated in this code path.
+
+    return timing;
 }
-    
-Box<NetworkLoadMetrics> copyTimingData(NSDictionary *timingData)
+
+Box<NetworkLoadMetrics> copyTimingData(NSURLSessionTaskTransactionMetrics *incompleteMetrics)
 {
+    return packageTimingData(
+        incompleteMetrics.fetchStartDate.timeIntervalSince1970,
+        incompleteMetrics.domainLookupStartDate.timeIntervalSince1970,
+        incompleteMetrics.domainLookupEndDate.timeIntervalSince1970,
+        incompleteMetrics.connectStartDate.timeIntervalSince1970,
+        incompleteMetrics.secureConnectionStartDate.timeIntervalSince1970,
+        incompleteMetrics.connectEndDate.timeIntervalSince1970,
+        incompleteMetrics.requestStartDate.timeIntervalSince1970,
+        incompleteMetrics.responseStartDate.timeIntervalSince1970,
+        incompleteMetrics.reusedConnection,
+        incompleteMetrics.response.URL.scheme
+    );
+}
+
+Box<NetworkLoadMetrics> copyTimingData(NSURLConnection *connection, NSURLResponse *response)
+{
+    NSDictionary *timingData = [connection _timingData];
     if (!timingData)
         return nullptr;
 
-    Box<NetworkLoadMetrics> timing = Box<NetworkLoadMetrics>::create();
+    auto timingValue = [](NSDictionary *timingData, NSString *key) {
+        if (id object = [timingData objectForKey:key])
+            return [object doubleValue];
+        return 0.0;
+    };
 
-    // 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.
-    double referenceStart = timingValue(timingData, @"_kCFNTimingDataFetchStart");
-
-    double domainLookupStart = timingValue(timingData, @"_kCFNTimingDataDomainLookupStart");
-    double domainLookupEnd = timingValue(timingData, @"_kCFNTimingDataDomainLookupEnd");
-    double connectStart = timingValue(timingData, @"_kCFNTimingDataConnectStart");
-    double secureConnectionStart = timingValue(timingData, @"_kCFNTimingDataSecureConnectionStart");
-    double connectEnd = timingValue(timingData, @"_kCFNTimingDataConnectEnd");
-    double requestStart = timingValue(timingData, @"_kCFNTimingDataRequestStart");
-    double responseStart = timingValue(timingData, @"_kCFNTimingDataResponseStart");
-
-    timing->fetchStart = Seconds(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;
+    return packageTimingData(
+        timingValue(timingData, @"_kCFNTimingDataFetchStart"),
+        timingValue(timingData, @"_kCFNTimingDataDomainLookupStart"),
+        timingValue(timingData, @"_kCFNTimingDataDomainLookupEnd"),
+        timingValue(timingData, @"_kCFNTimingDataConnectStart"),
+        timingValue(timingData, @"_kCFNTimingDataSecureConnectionStart"),
+        timingValue(timingData, @"_kCFNTimingDataConnectEnd"),
+        timingValue(timingData, @"_kCFNTimingDataRequestStart"),
+        timingValue(timingData, @"_kCFNTimingDataResponseStart"),
+        timingValue(timingData, @"_kCFNTimingDataConnectionReused"),
+        response.URL.scheme
+    );
 }
     
 }

Modified: trunk/Source/WebCore/platform/network/cocoa/WebCoreNSURLSession.mm (277492 => 277493)


--- trunk/Source/WebCore/platform/network/cocoa/WebCoreNSURLSession.mm	2021-05-14 16:10:38 UTC (rev 277492)
+++ trunk/Source/WebCore/platform/network/cocoa/WebCoreNSURLSession.mm	2021-05-14 16:56:47 UTC (rev 277493)
@@ -802,7 +802,7 @@
 
 - (NSDictionary *)_timingData
 {
-    // FIXME: return a dictionary sourced from ResourceHandle::getConnectionTimingData().
+    // FIXME: Make sure nobody is using this and remove this. It is replaced by WebCoreNSURLSessionTaskTransactionMetrics.
     return @{ };
 }
 

Modified: trunk/Source/WebCore/platform/network/mac/ResourceHandleMac.mm (277492 => 277493)


--- trunk/Source/WebCore/platform/network/mac/ResourceHandleMac.mm	2021-05-14 16:10:38 UTC (rev 277492)
+++ trunk/Source/WebCore/platform/network/mac/ResourceHandleMac.mm	2021-05-14 16:56:47 UTC (rev 277493)
@@ -653,9 +653,4 @@
     clearAuthentication();
 }
 
-Box<NetworkLoadMetrics> ResourceHandle::getConnectionTimingData(NSURLConnection *connection)
-{
-    return copyTimingData([connection _timingData]);
-}
-
 } // namespace WebCore

Modified: trunk/Source/WebCore/platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.mm (277492 => 277493)


--- trunk/Source/WebCore/platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.mm	2021-05-14 16:10:38 UTC (rev 277492)
+++ trunk/Source/WebCore/platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.mm	2021-05-14 16:56:47 UTC (rev 277493)
@@ -252,7 +252,7 @@
 
         ResourceResponse resourceResponse(r.get());
         resourceResponse.setSource(ResourceResponse::Source::Network);
-        resourceResponse.setDeprecatedNetworkLoadMetrics(ResourceHandle::getConnectionTimingData(connection.get()));
+        resourceResponse.setDeprecatedNetworkLoadMetrics(copyTimingData(connection.get(), r.get()));
 
         m_handle->didReceiveResponse(WTFMove(resourceResponse), [self, protectedSelf = WTFMove(protectedSelf)] {
             m_semaphore.signal();

Modified: trunk/Source/WebKit/ChangeLog (277492 => 277493)


--- trunk/Source/WebKit/ChangeLog	2021-05-14 16:10:38 UTC (rev 277492)
+++ trunk/Source/WebKit/ChangeLog	2021-05-14 16:56:47 UTC (rev 277493)
@@ -1,3 +1,13 @@
+2021-05-14  Alex Christensen  <achristen...@webkit.org>
+
+        Resource Timing: secureConnectionStart == 0 when a connection is re-used
+        https://bugs.webkit.org/show_bug.cgi?id=225733
+
+        Reviewed by Chris Dumez.
+
+        * NetworkProcess/cocoa/NetworkSessionCocoa.mm:
+        (-[WKNetworkSessionDelegate URLSession:dataTask:didReceiveResponse:completionHandler:]):
+
 2021-05-14  Chris Dumez  <cdu...@apple.com>
 
         Rename FileSystem::getFileSize() to FileSystem::fileSize()

Modified: trunk/Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm (277492 => 277493)


--- trunk/Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm	2021-05-14 16:10:38 UTC (rev 277492)
+++ trunk/Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm	2021-05-14 16:56:47 UTC (rev 277493)
@@ -878,8 +878,8 @@
         ASSERT(RunLoop::isMain());
 
         NegotiatedLegacyTLS negotiatedLegacyTLS = NegotiatedLegacyTLS::No;
+        NSURLSessionTaskTransactionMetrics *metrics = dataTask._incompleteTaskMetrics.transactionMetrics.lastObject;
 #if HAVE(TLS_PROTOCOL_VERSION_T)
-        NSURLSessionTaskTransactionMetrics *metrics = dataTask._incompleteTaskMetrics.transactionMetrics.lastObject;
         auto tlsVersion = (tls_protocol_version_t)metrics.negotiatedTLSProtocolVersion.unsignedShortValue;
         if (tlsVersion == tls_protocol_version_TLSv10 || tlsVersion == tls_protocol_version_TLSv11)
             negotiatedLegacyTLS = NegotiatedLegacyTLS::Yes;
@@ -906,9 +906,7 @@
         // all the fields when sending the response to the WebContent process over IPC.
         resourceResponse.disableLazyInitialization();
 
-        // FIXME: This cannot be eliminated until other code no longer relies on ResourceResponse's
-        // NetworkLoadMetrics. For example, PerformanceTiming.
-        resourceResponse.setDeprecatedNetworkLoadMetrics(WebCore::copyTimingData([dataTask _timingData]));
+        resourceResponse.setDeprecatedNetworkLoadMetrics(WebCore::copyTimingData(metrics));
 
         networkDataTask->didReceiveResponse(WTFMove(resourceResponse), negotiatedLegacyTLS, [completionHandler = makeBlockPtr(completionHandler), taskIdentifier](WebCore::PolicyAction policyAction) {
 #if !LOG_DISABLED
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to