Diff
Modified: trunk/Source/WebCore/ChangeLog (222755 => 222756)
--- trunk/Source/WebCore/ChangeLog 2017-10-02 23:33:00 UTC (rev 222755)
+++ trunk/Source/WebCore/ChangeLog 2017-10-02 23:34:18 UTC (rev 222756)
@@ -1,3 +1,53 @@
+2017-10-02 Basuke Suzuki <[email protected]>
+
+ [Curl] Implement missing async method in RecourceHandle and make it actually async
+ https://bugs.webkit.org/show_bug.cgi?id=173964
+
+ Reviewed by Alex Christensen.
+
+ * platform/network/ResourceHandle.cpp:
+ (WebCore::ResourceHandle::continueWillSendRequest): Deleted.
+ (WebCore::ResourceHandle::continueDidReceiveResponse): Deleted.
+ (WebCore::ResourceHandle::continueCanAuthenticateAgainstProtectionSpace): Deleted.
+ * platform/network/curl/CurlRequest.cpp:
+ (WebCore::CurlRequest::start):
+ (WebCore::CurlRequest::cancel):
+ (WebCore::CurlRequest::suspend):
+ (WebCore::CurlRequest::resume):
+ (WebCore::CurlRequest::didReceiveHeader):
+ (WebCore::CurlRequest::didReceiveData):
+ (WebCore::CurlRequest::didCompleteTransfer):
+ (WebCore::CurlRequest::didCancelTransfer):
+ (WebCore::CurlRequest::finalizeTransfer):
+ (WebCore::CurlRequest::invokeDidReceiveResponseForFile):
+ (WebCore::CurlRequest::invokeDidReceiveResponse):
+ (WebCore::CurlRequest::completeDidReceiveResponse):
+ (WebCore::CurlRequest::setRequestPaused):
+ (WebCore::CurlRequest::setCallbackPaused):
+ (WebCore::CurlRequest::pausedStatusChanged):
+ (WebCore::CurlRequest::setPaused): Deleted.
+ * platform/network/curl/CurlRequest.h:
+ (WebCore::CurlRequest::needToInvokeDidReceiveResponse const):
+ (WebCore::CurlRequest::isPaused const):
+ * platform/network/curl/ResourceHandleCurl.cpp:
+ (WebCore::ResourceHandle::receivedRequestToContinueWithoutCredential):
+ (WebCore::ResourceHandle::continueDidReceiveResponse):
+ (WebCore::ResourceHandle::platformContinueSynchronousDidReceiveResponse):
+ * platform/network/curl/ResourceHandleCurlDelegate.cpp:
+ (WebCore::ResourceHandleCurlDelegate::curlDidReceiveResponse):
+ (WebCore::ResourceHandleCurlDelegate::continueDidReceiveResponse):
+ (WebCore::ResourceHandleCurlDelegate::platformContinueSynchronousDidReceiveResponse):
+ (WebCore::ResourceHandleCurlDelegate::continueAfterDidReceiveResponse):
+ (WebCore::ResourceHandleCurlDelegate::shouldRedirectAsGET):
+ * platform/network/curl/ResourceHandleCurlDelegate.h:
+ * platform/network/curl/ResourceResponseCurl.cpp:
+ (WebCore::ResourceResponse::shouldRedirect):
+ (WebCore::ResourceResponse::isMovedPermanently const):
+ (WebCore::ResourceResponse::isFound const):
+ (WebCore::ResourceResponse::isSeeOther const):
+ (WebCore::ResourceResponse::isNotModified const):
+ (WebCore::ResourceResponse::isUnauthorized const):
+
2017-10-02 Ryosuke Niwa <[email protected]>
PasteImage tests are failing on debug builds
Modified: trunk/Source/WebCore/platform/network/ResourceHandle.cpp (222755 => 222756)
--- trunk/Source/WebCore/platform/network/ResourceHandle.cpp 2017-10-02 23:33:00 UTC (rev 222755)
+++ trunk/Source/WebCore/platform/network/ResourceHandle.cpp 2017-10-02 23:34:18 UTC (rev 222756)
@@ -170,27 +170,7 @@
}
}
-#if !PLATFORM(COCOA) && !USE(CFURLCONNECTION) && !USE(SOUP) && !USE(CURL)
-// ResourceHandle never uses async client calls on these platforms yet.
-void ResourceHandle::continueWillSendRequest(ResourceRequest&&)
-{
- notImplemented();
-}
-
-void ResourceHandle::continueDidReceiveResponse()
-{
- notImplemented();
-}
-
-#if USE(PROTECTION_SPACE_AUTH_CALLBACK)
-void ResourceHandle::continueCanAuthenticateAgainstProtectionSpace(bool)
-{
- notImplemented();
-}
-#endif
-#endif
-
-#if !USE(SOUP)
+#if !USE(SOUP) && !USE(CURL)
void ResourceHandle::platformContinueSynchronousDidReceiveResponse()
{
// Do nothing.
Modified: trunk/Source/WebCore/platform/network/curl/CurlRequest.cpp (222755 => 222756)
--- trunk/Source/WebCore/platform/network/curl/CurlRequest.cpp 2017-10-02 23:33:00 UTC (rev 222755)
+++ trunk/Source/WebCore/platform/network/curl/CurlRequest.cpp 2017-10-02 23:34:18 UTC (rev 222756)
@@ -56,6 +56,15 @@
void CurlRequest::start(bool isSyncRequest)
{
+ // The pausing of transfer does not work with protocols, like file://.
+ // Therefore, PAUSE can not be done in didReceiveData().
+ // It means that the same logic as http:// can not be used.
+ // In the file scheme, invokeDidReceiveResponse() is done first.
+ // Then StartWithJobManager is called with completeDidReceiveResponse and start transfer with libcurl.
+
+ // http : didReceiveHeader => didReceiveData[PAUSE] => invokeDidReceiveResponse => (MainThread)curlDidReceiveResponse => completeDidReceiveResponse[RESUME] => didReceiveData
+ // file : invokeDidReceiveResponseForFile => (MainThread)curlDidReceiveResponse => completeDidReceiveResponse => didReceiveData
+
ASSERT(isMainThread());
m_isSyncRequest = isSyncRequest;
@@ -66,8 +75,8 @@
// For asynchronous, use CurlJobManager. Curl processes runs on sub thread.
if (url.isLocalFile())
invokeDidReceiveResponseForFile(url);
-
- startWithJobManager();
+ else
+ startWithJobManager();
} else {
// For synchronous, does not use CurlJobManager. Curl processes runs on main thread.
// curl_easy_perform blocks until the transfer is finished.
@@ -101,7 +110,8 @@
if (!m_isSyncRequest)
CurlJobManager::singleton().cancel(this);
- setPaused(false);
+ setRequestPaused(false);
+ setCallbackPaused(false);
}
void CurlRequest::suspend()
@@ -108,7 +118,7 @@
{
ASSERT(isMainThread());
- setPaused(true);
+ setRequestPaused(true);
}
void CurlRequest::resume()
@@ -115,7 +125,7 @@
{
ASSERT(isMainThread());
- setPaused(false);
+ setRequestPaused(false);
}
/* `this` is protected inside this method. */
@@ -288,7 +298,8 @@
if (auto metrics = m_curlHandle->getNetworkLoadMetrics())
m_networkLoadMetrics = *metrics;
- invokeDidReceiveResponse();
+ // Response will send at didReceiveData() or didCompleteTransfer()
+ // to receive continueDidRceiveResponse() for asynchronously.
return receiveBytes;
}
@@ -300,6 +311,19 @@
if (m_cancelled)
return 0;
+ if (needToInvokeDidReceiveResponse()) {
+ if (!m_isSyncRequest) {
+ // For asynchronous, pause until completeDidReceiveResponse() is called.
+ setCallbackPaused(true);
+ invokeDidReceiveResponse(Action::ReceiveData);
+ return CURL_WRITEFUNC_PAUSE;
+ }
+
+ // For synchronous, completeDidReceiveResponse() is called in invokeDidReceiveResponse().
+ // In this case, pause is unnecessary.
+ invokeDidReceiveResponse(Action::None);
+ }
+
auto receiveBytes = buffer->size();
if (receiveBytes) {
@@ -320,30 +344,42 @@
}
if (result == CURLE_OK) {
- if (auto metrics = m_curlHandle->getNetworkLoadMetrics())
- m_networkLoadMetrics = *metrics;
+ if (needToInvokeDidReceiveResponse()) {
+ // Processing of didReceiveResponse() has not been completed. (For example, HEAD method)
+ // When completeDidReceiveResponse() is called, didCompleteTransfer() will be called again.
- callDelegate([this](CurlRequestDelegate* delegate) {
- if (delegate)
- delegate->curlDidComplete();
- });
+ m_finishedResultCode = result;
+ invokeDidReceiveResponse(Action::FinishTransfer);
+ } else {
+ if (auto metrics = m_curlHandle->getNetworkLoadMetrics())
+ m_networkLoadMetrics = *metrics;
+
+ finalizeTransfer();
+ callDelegate([this](CurlRequestDelegate* delegate) {
+ if (delegate)
+ delegate->curlDidComplete();
+ });
+ }
} else {
auto resourceError = ResourceError::httpError(result, m_request.url());
if (m_sslVerifier.sslErrors())
resourceError.setSslErrors(m_sslVerifier.sslErrors());
+ finalizeTransfer();
callDelegate([this, error = resourceError.isolatedCopy()](CurlRequestDelegate* delegate) {
if (delegate)
delegate->curlDidFailWithError(error);
});
}
-
- m_formDataStream = nullptr;
- m_curlHandle = nullptr;
}
void CurlRequest::didCancelTransfer()
{
+ finalizeTransfer();
+}
+
+void CurlRequest::finalizeTransfer()
+{
m_formDataStream = nullptr;
m_curlHandle = nullptr;
}
@@ -454,14 +490,21 @@
if (!m_isSyncRequest) {
// DidReceiveResponse must not be called immediately
CurlJobManager::singleton().callOnJobThread([protectedThis = makeRef(*this)]() {
- protectedThis->invokeDidReceiveResponse();
+ protectedThis->invokeDidReceiveResponse(Action::StartTransfer);
});
- } else
- invokeDidReceiveResponse();
+ } else {
+ // For synchronous, completeDidReceiveResponse() is called in platformContinueSynchronousDidReceiveResponse().
+ invokeDidReceiveResponse(Action::None);
+ }
}
-void CurlRequest::invokeDidReceiveResponse()
+void CurlRequest::invokeDidReceiveResponse(Action behaviorAfterInvoke)
{
+ ASSERT(!m_didNotifyResponse);
+
+ m_didNotifyResponse = true;
+ m_actionAfterInvoke = behaviorAfterInvoke;
+
callDelegate([this, response = m_response.isolatedCopy()](CurlRequestDelegate* delegate) {
if (delegate)
delegate->curlDidReceiveResponse(response);
@@ -468,21 +511,69 @@
});
}
-void CurlRequest::setPaused(bool paused)
+void CurlRequest::completeDidReceiveResponse()
{
+ ASSERT(isMainThread());
+ ASSERT(m_didNotifyResponse);
+ ASSERT(!m_didReturnFromNotify);
+
if (m_cancelled)
return;
- if (paused == m_isPaused)
+ m_didReturnFromNotify = true;
+
+ if (m_actionAfterInvoke == Action::ReceiveData) {
+ // Resume transfer
+ setCallbackPaused(false);
+ } else if (m_actionAfterInvoke == Action::StartTransfer) {
+ // Start transfer for file scheme
+ startWithJobManager();
+ } else if (m_actionAfterInvoke == Action::FinishTransfer) {
+ // Keep the calling thread of didCompleteTransfer()
+ if (!m_isSyncRequest) {
+ CurlJobManager::singleton().callOnJobThread([protectedThis = makeRef(*this), finishedResultCode = m_finishedResultCode]() {
+ protectedThis->didCompleteTransfer(finishedResultCode);
+ });
+ } else
+ didCompleteTransfer(m_finishedResultCode);
+ }
+}
+
+void CurlRequest::setRequestPaused(bool paused)
+{
+ auto wasPaused = isPaused();
+
+ m_isPausedOfRequest = paused;
+
+ if (isPaused() == wasPaused)
return;
- m_isPaused = paused;
+ pausedStatusChanged();
+}
- if (!m_curlHandle)
+void CurlRequest::setCallbackPaused(bool paused)
+{
+ auto wasPaused = isPaused();
+
+ m_isPausedOfCallback = paused;
+
+ if (isPaused() == wasPaused)
return;
+ // In this case, PAUSE will be executed within didReceiveData(). Change pause state and return.
+ if (paused)
+ return;
+
+ pausedStatusChanged();
+}
+
+void CurlRequest::pausedStatusChanged()
+{
+ if (m_cancelled || !m_curlHandle)
+ return;
+
if (!m_isSyncRequest && isMainThread()) {
- CurlJobManager::singleton().callOnJobThread([protectedThis = makeRef(*this), paused = m_isPaused]() {
+ CurlJobManager::singleton().callOnJobThread([protectedThis = makeRef(*this), paused = isPaused()]() {
if (protectedThis->m_cancelled)
return;
@@ -493,8 +584,8 @@
}
});
} else {
- auto error = m_curlHandle->pause(m_isPaused ? CURLPAUSE_ALL : CURLPAUSE_CONT);
- if ((error != CURLE_OK) && !m_isPaused)
+ auto error = m_curlHandle->pause(isPaused() ? CURLPAUSE_ALL : CURLPAUSE_CONT);
+ if ((error != CURLE_OK) && !isPaused())
cancel();
}
}
Modified: trunk/Source/WebCore/platform/network/curl/CurlRequest.h (222755 => 222756)
--- trunk/Source/WebCore/platform/network/curl/CurlRequest.h 2017-10-02 23:33:00 UTC (rev 222755)
+++ trunk/Source/WebCore/platform/network/curl/CurlRequest.h 2017-10-02 23:34:18 UTC (rev 222756)
@@ -56,9 +56,19 @@
bool isSyncRequest() { return m_isSyncRequest; }
+ // Processing for DidReceiveResponse
+ void completeDidReceiveResponse();
+
NetworkLoadMetrics getNetworkLoadMetrics() { return m_networkLoadMetrics.isolatedCopy(); }
private:
+ enum class Action {
+ None,
+ ReceiveData,
+ StartTransfer,
+ FinishTransfer
+ };
+
void retain() override { ref(); }
void release() override { deref(); }
CURL* handle() override { return m_curlHandle ? m_curlHandle->handle() : nullptr; }
@@ -76,6 +86,7 @@
size_t didReceiveData(Ref<SharedBuffer>&&);
void didCompleteTransfer(CURLcode) override;
void didCancelTransfer() override;
+ void finalizeTransfer();
// For POST and PUT method
void resolveBlobReferences(ResourceRequest&);
@@ -83,10 +94,14 @@
void setupPUT(ResourceRequest&);
void setupFormData(ResourceRequest&, bool);
- // Processing for DidResourceResponse
+ // Processing for DidReceiveResponse
+ bool needToInvokeDidReceiveResponse() const { return !m_didNotifyResponse || !m_didReturnFromNotify; }
void invokeDidReceiveResponseForFile(URL&);
- void invokeDidReceiveResponse();
- void setPaused(bool);
+ void invokeDidReceiveResponse(Action);
+ void setRequestPaused(bool);
+ void setCallbackPaused(bool);
+ void pausedStatusChanged();
+ bool isPaused() const { return m_isPausedOfRequest || m_isPausedOfCallback; };
// Callback functions for curl
static CURLcode willSetupSslCtxCallback(CURL*, void*, void*);
@@ -111,8 +126,14 @@
CurlSSLVerifier m_sslVerifier;
CurlResponse m_response;
- bool m_isPaused { false };
+ bool m_didNotifyResponse { false };
+ bool m_didReturnFromNotify { false };
+ Action m_actionAfterInvoke { Action::None };
+ CURLcode m_finishedResultCode { CURLE_OK };
+ bool m_isPausedOfRequest { false };
+ bool m_isPausedOfCallback { false };
+
NetworkLoadMetrics m_networkLoadMetrics;
};
Modified: trunk/Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp (222755 => 222756)
--- trunk/Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp 2017-10-02 23:33:00 UTC (rev 222755)
+++ trunk/Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp 2017-10-02 23:34:18 UTC (rev 222756)
@@ -213,10 +213,10 @@
if (challenge != d->m_currentWebChallenge)
return;
- if (d->m_delegate)
- d->m_delegate->setAuthentication("", "");
+ clearAuthentication();
- clearAuthentication();
+ auto protectedThis = makeRef(*this);
+ didReceiveResponse(ResourceResponse(d->m_response));
}
void ResourceHandle::receivedCancellation(const AuthenticationChallenge& challenge)
@@ -259,9 +259,20 @@
void ResourceHandle::continueDidReceiveResponse()
{
- notImplemented();
+ ASSERT(isMainThread());
+
+ if (d->m_delegate)
+ d->m_delegate->continueDidReceiveResponse();
}
+void ResourceHandle::platformContinueSynchronousDidReceiveResponse()
+{
+ ASSERT(isMainThread());
+
+ if (d->m_delegate)
+ d->m_delegate->platformContinueSynchronousDidReceiveResponse();
+}
+
void ResourceHandle::continueWillSendRequest(ResourceRequest&& request)
{
ASSERT(isMainThread());
Modified: trunk/Source/WebCore/platform/network/curl/ResourceHandleCurlDelegate.cpp (222755 => 222756)
--- trunk/Source/WebCore/platform/network/curl/ResourceHandleCurlDelegate.cpp 2017-10-02 23:33:00 UTC (rev 222755)
+++ trunk/Source/WebCore/platform/network/curl/ResourceHandleCurlDelegate.cpp 2017-10-02 23:34:18 UTC (rev 222756)
@@ -227,7 +227,9 @@
}
CurlCacheManager::getInstance().didReceiveResponse(*m_handle, response());
- m_handle->client()->didReceiveResponse(m_handle, ResourceResponse(response()));
+
+ auto protectedThis = makeRef(*m_handle);
+ m_handle->didReceiveResponse(ResourceResponse(response()));
}
}
@@ -276,9 +278,34 @@
m_handle->client()->didFail(m_handle, resourceError);
}
+void ResourceHandleCurlDelegate::continueDidReceiveResponse()
+{
+ ASSERT(isMainThread());
+
+ continueAfterDidReceiveResponse();
+}
+
+void ResourceHandleCurlDelegate::platformContinueSynchronousDidReceiveResponse()
+{
+ ASSERT(isMainThread());
+
+ continueAfterDidReceiveResponse();
+}
+
+void ResourceHandleCurlDelegate::continueAfterDidReceiveResponse()
+{
+ ASSERT(isMainThread());
+
+ // continueDidReceiveResponse might cancel the load.
+ if (cancelledOrClientless() || !m_curlRequest)
+ return;
+
+ m_curlRequest->completeDidReceiveResponse();
+}
+
bool ResourceHandleCurlDelegate::shouldRedirectAsGET(const ResourceRequest& request, bool crossOrigin)
{
- if ((request.httpMethod() == "GET") || (request.httpMethod() == "HEAD"))
+ if (request.httpMethod() == "GET" || request.httpMethod() == "HEAD")
return false;
if (!request.url().protocolIsInHTTPFamily())
Modified: trunk/Source/WebCore/platform/network/curl/ResourceHandleCurlDelegate.h (222755 => 222756)
--- trunk/Source/WebCore/platform/network/curl/ResourceHandleCurlDelegate.h 2017-10-02 23:33:00 UTC (rev 222755)
+++ trunk/Source/WebCore/platform/network/curl/ResourceHandleCurlDelegate.h 2017-10-02 23:34:18 UTC (rev 222756)
@@ -55,6 +55,9 @@
void dispatchSynchronousJob();
+ void continueDidReceiveResponse();
+ void platformContinueSynchronousDidReceiveResponse();
+
void continueWillSendRequest(ResourceRequest&&);
private:
@@ -71,6 +74,8 @@
void curlDidComplete() override;
void curlDidFailWithError(const ResourceError&) override;
+ void continueAfterDidReceiveResponse();
+
bool shouldRedirectAsGET(const ResourceRequest&, bool crossOrigin);
void willSendRequest();
void continueAfterWillSendRequest(ResourceRequest&&);
Modified: trunk/Source/WebCore/platform/network/curl/ResourceResponseCurl.cpp (222755 => 222756)
--- trunk/Source/WebCore/platform/network/curl/ResourceResponseCurl.cpp 2017-10-02 23:33:00 UTC (rev 222755)
+++ trunk/Source/WebCore/platform/network/curl/ResourceResponseCurl.cpp 2017-10-02 23:34:18 UTC (rev 222756)
@@ -136,7 +136,7 @@
bool ResourceResponse::shouldRedirect()
{
auto statusCode = httpStatusCode();
- if ((statusCode < 300) || (400 <= statusCode))
+ if (statusCode < 300 || 400 <= statusCode)
return false;
// Some 3xx status codes aren't actually redirects.
@@ -151,27 +151,27 @@
bool ResourceResponse::isMovedPermanently() const
{
- return (httpStatusCode() == 301);
+ return httpStatusCode() == 301;
}
bool ResourceResponse::isFound() const
{
- return (httpStatusCode() == 302);
+ return httpStatusCode() == 302;
}
bool ResourceResponse::isSeeOther() const
{
- return (httpStatusCode() == 303);
+ return httpStatusCode() == 303;
}
bool ResourceResponse::isNotModified() const
{
- return (httpStatusCode() == 304);
+ return httpStatusCode() == 304;
}
bool ResourceResponse::isUnauthorized() const
{
- return (httpStatusCode() == 401);
+ return httpStatusCode() == 401;
}
}
