Attachment 350053 Details for Bug 189520 – PATCH

[patch] PATCH

189520.diff (text/plain), 13.10 KB, created by Basuke Suzuki on 2018-09-18 14:13:48 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Basuke Suzuki

Created: 2018-09-18 14:13:48 PDT

Size: 13.10 KB

patch

obsolete

>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 1b12cd68d4f..cf663259952 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,27 @@
>+2018-09-18  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 Alex Christensen.
>+
>+        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::CurlRequest):
>+        (WebCore::CurlRequest::updateNetworkLoadMetrics):
>+        * platform/network/curl/CurlRequest.h:
>+        (WebCore::CurlRequest::create):
>+        * platform/network/curl/ResourceHandleCurl.cpp:
>+        (WebCore::ResourceHandle::createCurlRequest):
>+
> 2018-09-17  Frederic Wang  <fwang@igalia.com>
> 
>         Build error in ImageBufferCG when compiled with IOSurfacePool
>diff --git a/Source/WebCore/platform/network/curl/CurlContext.cpp b/Source/WebCore/platform/network/curl/CurlContext.cpp
>index 9c87807a0b0..5c8ad589f3c 100644
>--- a/Source/WebCore/platform/network/curl/CurlContext.cpp
>+++ b/Source/WebCore/platform/network/curl/CurlContext.cpp
>@@ -733,13 +733,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;
>@@ -760,49 +753,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;
>@@ -821,8 +827,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 7d9711caf37..373260179b8 100644
>--- a/Source/WebCore/platform/network/curl/CurlContext.h
>+++ b/Source/WebCore/platform/network/curl/CurlContext.h
>@@ -282,6 +282,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 7f11234f195..46542a7f761 100644
>--- a/Source/WebCore/platform/network/curl/CurlRequest.cpp
>+++ b/Source/WebCore/platform/network/curl/CurlRequest.cpp
>@@ -38,13 +38,14 @@
> 
> namespace WebCore {
> 
>-CurlRequest::CurlRequest(const ResourceRequest&request, CurlRequestClient* client, bool shouldSuspend, bool enableMultipart, MessageQueue<Function<void()>>* messageQueue)
>+CurlRequest::CurlRequest(const ResourceRequest&request, CurlRequestClient* client, ShouldSuspend shouldSuspend, EnableMultipart enableMultipart, CaptureNetworkLoadMetrics captureExtraMetrics, MessageQueue<Function<void()>>* messageQueue)
>     : m_request(request.isolatedCopy())
>     , m_client(client)
>-    , m_shouldSuspend(shouldSuspend)
>-    , m_enableMultipart(enableMultipart)
>+    , m_shouldSuspend(shouldSuspend == ShouldSuspend::Yes)
>+    , m_enableMultipart(enableMultipart == EnableMultipart::Yes)
>     , m_formDataStream(m_request.httpBody())
>     , m_messageQueue(messageQueue)
>+    , m_captureExtraMetrics(captureExtraMetrics == CaptureNetworkLoadMetrics::Extended)
> {
>     ASSERT(isMainThread());
> }
>@@ -672,11 +673,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_captureExtraMetrics) {
>+            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 88ee2eefd32..fdc4b590c14 100644
>--- a/Source/WebCore/platform/network/curl/CurlRequest.h
>+++ b/Source/WebCore/platform/network/curl/CurlRequest.h
>@@ -60,9 +60,14 @@ public:
>         Yes = true
>     };
> 
>-    static Ref<CurlRequest> create(const ResourceRequest& request, CurlRequestClient& client, ShouldSuspend shouldSuspend = ShouldSuspend::No, EnableMultipart enableMultipart = EnableMultipart::No, MessageQueue<Function<void()>>* messageQueue = nullptr)
>+    enum class CaptureNetworkLoadMetrics : uint8_t {
>+        Basic,
>+        Extended
>+    };
>+
>+    static Ref<CurlRequest> create(const ResourceRequest& request, CurlRequestClient& client, ShouldSuspend shouldSuspend = ShouldSuspend::No, EnableMultipart enableMultipart = EnableMultipart::No, CaptureNetworkLoadMetrics captureMetrics = CaptureNetworkLoadMetrics::Basic, MessageQueue<Function<void()>>* messageQueue = nullptr)
>     {
>-        return adoptRef(*new CurlRequest(request, &client, shouldSuspend == ShouldSuspend::Yes, enableMultipart == EnableMultipart::Yes, messageQueue));
>+        return adoptRef(*new CurlRequest(request, &client, shouldSuspend, enableMultipart, captureMetrics, messageQueue));
>     }
> 
>     virtual ~CurlRequest() = default;
>@@ -104,7 +109,7 @@ private:
>         FinishTransfer
>     };
> 
>-    CurlRequest(const ResourceRequest&, CurlRequestClient*, bool, bool, MessageQueue<Function<void()>>*);
>+    CurlRequest(const ResourceRequest&, CurlRequestClient*, ShouldSuspend, EnableMultipart, CaptureNetworkLoadMetrics, MessageQueue<Function<void()>>*);
> 
>     void retain() override { ref(); }
>     void release() override { deref(); }
>@@ -201,6 +206,7 @@ private:
>     FileSystem::PlatformFileHandle m_downloadFileHandle { FileSystem::invalidPlatformFileHandle };
> 
>     CertificateInfo m_certificateInfo;
>+    bool m_captureExtraMetrics;
>     NetworkLoadMetrics m_networkLoadMetrics;
>     MonotonicTime m_requestStartTime { MonotonicTime::nan() };
>     MonotonicTime m_performStartTime;
>diff --git a/Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp b/Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp
>index a4bceb827a5..5df36764142 100644
>--- a/Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp
>+++ b/Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp
>@@ -154,7 +154,7 @@ Ref<CurlRequest> ResourceHandle::createCurlRequest(ResourceRequest&& request, Re
>     }
> 
>     CurlRequest::ShouldSuspend shouldSuspend = d->m_defersLoading ? CurlRequest::ShouldSuspend::Yes : CurlRequest::ShouldSuspend::No;
>-    auto curlRequest = CurlRequest::create(request, *delegate(), shouldSuspend, CurlRequest::EnableMultipart::Yes, d->m_messageQueue);
>+    auto curlRequest = CurlRequest::create(request, *delegate(), shouldSuspend, CurlRequest::EnableMultipart::Yes, CurlRequest::CaptureNetworkLoadMetrics::Basic, d->m_messageQueue);
>     
>     return curlRequest;
> }
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 59d20dbf9b9..297568396f8 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,16 @@
>+2018-09-18  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 Alex Christensen.
>+
>+        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  Basuke Suzuki  <Basuke.Suzuki@sony.com>
> 
>         [Curl] Respond with requested authentication scheme for authentication challenge.
>diff --git a/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp b/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp
>index 4d5046745cf..3cbf335e117 100644
>--- a/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp
>+++ b/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp
>@@ -138,7 +138,8 @@ 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);
>+    const auto captureMetrics = shouldCaptureExtraNetworkLoadMetrics() ? CurlRequest::CaptureNetworkLoadMetrics::Extended : CurlRequest::CaptureNetworkLoadMetrics::Basic;
>+    return CurlRequest::create(request, *this, CurlRequest::ShouldSuspend::Yes, CurlRequest::EnableMultipart::No, captureMetrics);
> }
> 
> void NetworkDataTaskCurl::curlDidSendData(CurlRequest&, unsigned long long totalBytesSent, unsigned long long totalBytesExpectedToSend)

Actions: View | Formatted Diff | Diff

Attachments on bug 189520: 349903 | 349959 | 350053