Attachment 349903 Details for Bug 189520 – PATCH

[patch] PATCH

189520.diff (text/plain), 10.02 KB, created by Basuke Suzuki on 2018-09-17 10:50:41 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Basuke Suzuki

Created: 2018-09-17 10:50:41 PDT

Size: 10.02 KB

patch

obsolete

>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 0f047efc3e2..645f7b9f5be 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,24 @@
>+2018-09-17  Basuke Suzuki  <Basuke.Suzuki@sony.com>
>+
>+        [Curl] Limit capturing extra metrics for Web Inspector when not required.
>+        https://bugs.webkit.org/show_bug.cgi?id=189520
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Respect the value of NetworkDataTask::shouldCaptureExtraNetworkLoadMetrics() to reduce the process
>+        time when they are not needed.
>+
>+        No new tests because there's no behavior change.
>+
>+        * platform/network/curl/CurlContext.cpp:
>+        (WebCore::CurlHandle::getNetworkLoadMetrics):
>+        (WebCore::CurlHandle::addExtraNetworkLoadMetrics):
>+        * platform/network/curl/CurlContext.h:
>+        * platform/network/curl/CurlRequest.cpp:
>+        (WebCore::CurlRequest::updateNetworkLoadMetrics):
>+        * platform/network/curl/CurlRequest.h:
>+        (WebCore::CurlRequest::setCaptureExtraNetworkLoadMetricsEnabled):
>+
> 2018-09-17  Youenn Fablet  <youenn@apple.com>
> 
>         Enable VCP for iOS and reenable it for MacOS
>diff --git a/Source/WebCore/platform/network/curl/CurlContext.cpp b/Source/WebCore/platform/network/curl/CurlContext.cpp
>index d36478ebfce..1206bc39e81 100644
>--- a/Source/WebCore/platform/network/curl/CurlContext.cpp
>+++ b/Source/WebCore/platform/network/curl/CurlContext.cpp
>@@ -737,13 +737,6 @@ std::optional<NetworkLoadMetrics> CurlHandle::getNetworkLoadMetrics(const WTF::S
>     double connect = 0.0;
>     double appConnect = 0.0;
>     double startTransfer = 0.0;
>-    long requestHeaderSize = 0;
>-    curl_off_t requestBodySize = 0;
>-    long responseHeaderSize = 0;
>-    curl_off_t responseBodySize = 0;
>-    long version = 0;
>-    char* ip = nullptr;
>-    long port = 0;
> 
>     if (!m_handle)
>         return std::nullopt;
>@@ -764,49 +757,62 @@ std::optional<NetworkLoadMetrics> CurlHandle::getNetworkLoadMetrics(const WTF::S
>     if (errorCode != CURLE_OK)
>         return std::nullopt;
> 
>+    NetworkLoadMetrics networkLoadMetrics;
>+
>+    networkLoadMetrics.domainLookupStart = domainLookupStart;
>+    networkLoadMetrics.domainLookupEnd = domainLookupStart + Seconds(nameLookup);
>+    networkLoadMetrics.connectStart = domainLookupStart + Seconds(nameLookup);
>+    networkLoadMetrics.connectEnd = domainLookupStart + Seconds(connect);
>+
>+    if (appConnect > 0.0) {
>+        networkLoadMetrics.secureConnectionStart = domainLookupStart + Seconds(connect);
>+        networkLoadMetrics.connectEnd = domainLookupStart + Seconds(appConnect);
>+    }
>+
>+    networkLoadMetrics.requestStart = networkLoadMetrics.connectEnd;
>+    networkLoadMetrics.responseStart = domainLookupStart + Seconds(startTransfer);
>+
>+    return networkLoadMetrics;
>+}
>+
>+void CurlHandle::addExtraNetworkLoadMetrics(NetworkLoadMetrics& networkLoadMetrics)
>+{
>+    long requestHeaderSize = 0;
>+    curl_off_t requestBodySize = 0;
>+    long responseHeaderSize = 0;
>+    curl_off_t responseBodySize = 0;
>+    long version = 0;
>+    char* ip = nullptr;
>+    long port = 0;
>+
>     // FIXME: Gets total request size not just headers https://bugs.webkit.org/show_bug.cgi?id=188363
>-    errorCode = curl_easy_getinfo(m_handle, CURLINFO_REQUEST_SIZE, &requestHeaderSize);
>+    CURLcode errorCode = curl_easy_getinfo(m_handle, CURLINFO_REQUEST_SIZE, &requestHeaderSize);
>     if (errorCode != CURLE_OK)
>-        return std::nullopt;
>+        return;
> 
>     errorCode = curl_easy_getinfo(m_handle, CURLINFO_SIZE_UPLOAD_T, &requestBodySize);
>     if (errorCode != CURLE_OK)
>-        return std::nullopt;
>+        return;
> 
>     errorCode = curl_easy_getinfo(m_handle, CURLINFO_HEADER_SIZE, &responseHeaderSize);
>     if (errorCode != CURLE_OK)
>-        return std::nullopt;
>+        return;
> 
>     errorCode = curl_easy_getinfo(m_handle, CURLINFO_SIZE_DOWNLOAD_T, &responseBodySize);
>     if (errorCode != CURLE_OK)
>-        return std::nullopt;
>+        return;
> 
>     errorCode = curl_easy_getinfo(m_handle, CURLINFO_PRIMARY_IP, &ip);
>     if (errorCode != CURLE_OK)
>-        return std::nullopt;
>+        return;
> 
>     errorCode = curl_easy_getinfo(m_handle, CURLINFO_PRIMARY_PORT, &port);
>     if (errorCode != CURLE_OK)
>-        return std::nullopt;
>+        return;
> 
>     errorCode = curl_easy_getinfo(m_handle, CURLINFO_HTTP_VERSION, &version);
>     if (errorCode != CURLE_OK)
>-        return std::nullopt;
>-
>-    NetworkLoadMetrics networkLoadMetrics;
>-
>-    networkLoadMetrics.domainLookupStart = domainLookupStart;
>-    networkLoadMetrics.domainLookupEnd = domainLookupStart + Seconds(nameLookup);
>-    networkLoadMetrics.connectStart = domainLookupStart + Seconds(nameLookup);
>-    networkLoadMetrics.connectEnd = domainLookupStart + Seconds(connect);
>-
>-    if (appConnect > 0.0) {
>-        networkLoadMetrics.secureConnectionStart = domainLookupStart + Seconds(connect);
>-        networkLoadMetrics.connectEnd = domainLookupStart + Seconds(appConnect);
>-    }
>-
>-    networkLoadMetrics.requestStart = networkLoadMetrics.connectEnd;
>-    networkLoadMetrics.responseStart = domainLookupStart + Seconds(startTransfer);
>+        return;
> 
>     networkLoadMetrics.requestHeaderBytesSent = requestHeaderSize;
>     networkLoadMetrics.requestBodyBytesSent = requestBodySize;
>@@ -825,8 +831,6 @@ std::optional<NetworkLoadMetrics> CurlHandle::getNetworkLoadMetrics(const WTF::S
>         networkLoadMetrics.protocol = httpVersion11;
>     else if (version == CURL_HTTP_VERSION_2)
>         networkLoadMetrics.protocol = httpVersion2;
>-
>-    return networkLoadMetrics;
> }
> 
> std::optional<CertificateInfo> CurlHandle::certificateInfo() const
>diff --git a/Source/WebCore/platform/network/curl/CurlContext.h b/Source/WebCore/platform/network/curl/CurlContext.h
>index 8a94520e67f..f0b318d1da7 100644
>--- a/Source/WebCore/platform/network/curl/CurlContext.h
>+++ b/Source/WebCore/platform/network/curl/CurlContext.h
>@@ -283,6 +283,7 @@ public:
>     std::optional<long> getProxyAuthAvail();
>     std::optional<long> getHttpVersion();
>     std::optional<NetworkLoadMetrics> getNetworkLoadMetrics(const WTF::Seconds& domainLookupStart);
>+    void addExtraNetworkLoadMetrics(NetworkLoadMetrics&);
> 
>     int sslErrors() const;
>     std::optional<CertificateInfo> certificateInfo() const;
>diff --git a/Source/WebCore/platform/network/curl/CurlRequest.cpp b/Source/WebCore/platform/network/curl/CurlRequest.cpp
>index 2d8aff2c9b7..13ef2e5bddf 100644
>--- a/Source/WebCore/platform/network/curl/CurlRequest.cpp
>+++ b/Source/WebCore/platform/network/curl/CurlRequest.cpp
>@@ -648,11 +648,15 @@ void CurlRequest::updateNetworkLoadMetrics()
> {
>     auto domainLookupStart = m_performStartTime - m_requestStartTime;
> 
>-    if (auto metrics = m_curlHandle->getNetworkLoadMetrics(domainLookupStart))
>+    if (auto metrics = m_curlHandle->getNetworkLoadMetrics(domainLookupStart)) {
>         m_networkLoadMetrics = *metrics;
> 
>-    m_networkLoadMetrics.requestHeaders = m_request.httpHeaderFields();
>-    m_networkLoadMetrics.responseBodyDecodedSize = m_totalReceivedSize;
>+        if (m_captureExtraNetworkLoadMetricsEnabled) {
>+            m_curlHandle->addExtraNetworkLoadMetrics(m_networkLoadMetrics);
>+            m_networkLoadMetrics.requestHeaders = m_request.httpHeaderFields();
>+            m_networkLoadMetrics.responseBodyDecodedSize = m_totalReceivedSize;
>+        }
>+    }
> }
> 
> void CurlRequest::enableDownloadToFile()
>diff --git a/Source/WebCore/platform/network/curl/CurlRequest.h b/Source/WebCore/platform/network/curl/CurlRequest.h
>index 958c777b775..f61c2fc0e84 100644
>--- a/Source/WebCore/platform/network/curl/CurlRequest.h
>+++ b/Source/WebCore/platform/network/curl/CurlRequest.h
>@@ -69,6 +69,7 @@ public:
>     void invalidateClient();
>     WEBCORE_EXPORT void setUserPass(const String&, const String&);
>     void setStartTime(const MonotonicTime& startTime) { m_requestStartTime = startTime; }
>+    void setCaptureExtraNetworkLoadMetricsEnabled(bool enabled) { m_captureExtraNetworkLoadMetricsEnabled = enabled; }
> 
>     void start();
>     void cancel();
>@@ -198,6 +199,7 @@ private:
>     FileSystem::PlatformFileHandle m_downloadFileHandle { FileSystem::invalidPlatformFileHandle };
> 
>     CertificateInfo m_certificateInfo;
>+    bool m_captureExtraNetworkLoadMetricsEnabled { false };
>     NetworkLoadMetrics m_networkLoadMetrics;
>     MonotonicTime m_requestStartTime { MonotonicTime::nan() };
>     MonotonicTime m_performStartTime;
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 766af6b6a5e..c92f4d9d5b1 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,16 @@
>+2018-09-17  Basuke Suzuki  <Basuke.Suzuki@sony.com>
>+
>+        [Curl] Limit capturing extra metrics for Web Inspector when not required.
>+        https://bugs.webkit.org/show_bug.cgi?id=189520
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Respect the value of NetworkDataTask::shouldCaptureExtraNetworkLoadMetrics() to reduce the process
>+        time when they are not needed.
>+
>+        * NetworkProcess/curl/NetworkDataTaskCurl.cpp:
>+        (WebKit::NetworkDataTaskCurl::createCurlRequest):
>+
> 2018-09-17  Woodrow Wang  <woodrow_wang@apple.com>
> 
>         Clear pending resource load statistics' writes after tests
>diff --git a/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp b/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp
>index 183b219b6be..b5036c39104 100644
>--- a/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp
>+++ b/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp
>@@ -136,7 +136,9 @@ Ref<CurlRequest> NetworkDataTaskCurl::createCurlRequest(ResourceRequest&& reques
> 
>     // Creates a CurlRequest in suspended state.
>     // Then, NetworkDataTaskCurl::resume() will be called and communication resumes.
>-    return CurlRequest::create(request, *this, CurlRequest::ShouldSuspend::Yes);
>+    auto curlRequest = CurlRequest::create(request, *this, CurlRequest::ShouldSuspend::Yes);
>+    curlRequest->setCaptureExtraNetworkLoadMetricsEnabled(shouldCaptureExtraNetworkLoadMetrics());
>+    return curlRequest;
> }
> 
> void NetworkDataTaskCurl::curlDidSendData(CurlRequest&, unsigned long long totalBytesSent, unsigned long long totalBytesExpectedToSend)

Flags:

achristensen: review-

Actions: View | Formatted Diff | Diff

Attachments on bug 189520: 349903 | 349959 | 350053