Diff
Modified: trunk/Source/WTF/ChangeLog (228851 => 228852)
--- trunk/Source/WTF/ChangeLog 2018-02-21 00:51:06 UTC (rev 228851)
+++ trunk/Source/WTF/ChangeLog 2018-02-21 01:08:28 UTC (rev 228852)
@@ -1,3 +1,19 @@
+2018-02-20 Chris Dumez <[email protected]>
+
+ Provisional load may get committed before receiving the decidePolicyForNavigationResponse response
+ https://bugs.webkit.org/show_bug.cgi?id=182720
+ <rdar://problem/37515204>
+
+ Reviewed by Alex Christensen.
+
+ Add convenience CompletionHandlerCallingScope class which calls its CompletionHandler
+ when destroyed, and provides a release() methods to manually call the completionHandler.
+
+ * wtf/CompletionHandler.h:
+ (WTF::CompletionHandlerCallingScope::CompletionHandlerCallingScope):
+ (WTF::CompletionHandlerCallingScope::~CompletionHandlerCallingScope):
+ (WTF::CompletionHandlerCallingScope::CompletionHandler<void):
+
2018-02-20 Tim Horton <[email protected]>
Always inline soft linking functions to work around a clang bug
Modified: trunk/Source/WTF/wtf/CompletionHandler.h (228851 => 228852)
--- trunk/Source/WTF/wtf/CompletionHandler.h 2018-02-21 00:51:06 UTC (rev 228851)
+++ trunk/Source/WTF/wtf/CompletionHandler.h 2018-02-21 01:08:28 UTC (rev 228852)
@@ -64,6 +64,28 @@
mutable WTF::Function<Out(In...)> m_function;
};
+class CompletionHandlerCallingScope {
+public:
+ CompletionHandlerCallingScope(CompletionHandler<void()>&& completionHandler)
+ : m_completionHandler(WTFMove(completionHandler))
+ { }
+
+ ~CompletionHandlerCallingScope()
+ {
+ if (m_completionHandler)
+ m_completionHandler();
+ }
+
+ CompletionHandlerCallingScope(CompletionHandlerCallingScope&&) = default;
+ CompletionHandlerCallingScope& operator=(CompletionHandlerCallingScope&&) = default;
+
+ CompletionHandler<void()> release() { return WTFMove(m_completionHandler); }
+
+private:
+ CompletionHandler<void()> m_completionHandler;
+};
+
} // namespace WTF
using WTF::CompletionHandler;
+using WTF::CompletionHandlerCallingScope;
Modified: trunk/Source/WebCore/ChangeLog (228851 => 228852)
--- trunk/Source/WebCore/ChangeLog 2018-02-21 00:51:06 UTC (rev 228851)
+++ trunk/Source/WebCore/ChangeLog 2018-02-21 01:08:28 UTC (rev 228852)
@@ -1,5 +1,40 @@
2018-02-20 Chris Dumez <[email protected]>
+ Provisional load may get committed before receiving the decidePolicyForNavigationResponse response
+ https://bugs.webkit.org/show_bug.cgi?id=182720
+ <rdar://problem/37515204>
+
+ Reviewed by Alex Christensen.
+
+ Wait for the policy response from the client after receiving a resource response,
+ before sending the NetworkResourceLoader::ContinueDidReceiveResponse IPC back to
+ the NetworkProcess. Otherwise, the network process may start sending us data and
+ we may end up committing the provisional load before receiving the policy decision
+ fron the client.
+
+ Change is covered by new API test.
+
+ * loader/DocumentLoader.cpp:
+ (WebCore::DocumentLoader::responseReceived):
+ * loader/NetscapePlugInStreamLoader.cpp:
+ (WebCore::NetscapePlugInStreamLoader::didReceiveResponse):
+ * loader/NetscapePlugInStreamLoader.h:
+ * loader/ResourceLoader.cpp:
+ (WebCore::ResourceLoader::deliverResponseAndData):
+ (WebCore::ResourceLoader::loadDataURL):
+ (WebCore::ResourceLoader::didReceiveResponse):
+ (WebCore::ResourceLoader::didReceiveResponseAsync):
+ * loader/ResourceLoader.h:
+ * loader/SubresourceLoader.cpp:
+ (WebCore::SubresourceLoader::didReceiveResponse):
+ (WebCore::SubresourceLoader::didReceiveResponsePolicy):
+ (WebCore::SubresourceLoader::willCancel):
+ * loader/SubresourceLoader.h:
+ * loader/ios/PreviewLoader.mm:
+ (-[WebPreviewLoader _sendDidReceiveResponseIfNecessary]):
+
+2018-02-20 Chris Dumez <[email protected]>
+
Crash under JSC::JSCell::toNumber(JSC::ExecState*)
https://bugs.webkit.org/show_bug.cgi?id=182984
<rdar://problem/37694346>
Modified: trunk/Source/WebCore/loader/DocumentLoader.cpp (228851 => 228852)
--- trunk/Source/WebCore/loader/DocumentLoader.cpp 2018-02-21 00:51:06 UTC (rev 228851)
+++ trunk/Source/WebCore/loader/DocumentLoader.cpp 2018-02-21 01:08:28 UTC (rev 228852)
@@ -783,7 +783,12 @@
}
#endif
+ if (auto* mainResourceLoader = this->mainResourceLoader())
+ mainResourceLoader->markInAsyncResponsePolicyCheck();
frameLoader()->checkContentPolicy(m_response, [this, protectedThis = makeRef(*this)](PolicyAction policy) {
+ if (auto* mainResourceLoader = this->mainResourceLoader())
+ mainResourceLoader->didReceiveResponsePolicy();
+
continueAfterContentPolicy(policy);
});
}
Modified: trunk/Source/WebCore/loader/NetscapePlugInStreamLoader.cpp (228851 => 228852)
--- trunk/Source/WebCore/loader/NetscapePlugInStreamLoader.cpp 2018-02-21 00:51:06 UTC (rev 228851)
+++ trunk/Source/WebCore/loader/NetscapePlugInStreamLoader.cpp 2018-02-21 01:08:28 UTC (rev 228852)
@@ -97,9 +97,10 @@
});
}
-void NetscapePlugInStreamLoader::didReceiveResponse(const ResourceResponse& response)
+void NetscapePlugInStreamLoader::didReceiveResponse(const ResourceResponse& response, CompletionHandler<void()>&& policyCompletionHandler)
{
Ref<NetscapePlugInStreamLoader> protectedThis(*this);
+ CompletionHandlerCallingScope completionHandlerCaller(WTFMove(policyCompletionHandler));
m_client->didReceiveResponse(this, response);
@@ -107,21 +108,21 @@
if (!m_client)
return;
- ResourceLoader::didReceiveResponse(response);
+ ResourceLoader::didReceiveResponse(response, [this, protectedThis = WTFMove(protectedThis), response, completionHandlerCaller = WTFMove(completionHandlerCaller)]() mutable {
+ // Don't continue if the stream is cancelled
+ if (!m_client)
+ return;
- // Don't continue if the stream is cancelled
- if (!m_client)
- return;
+ if (!response.isHTTP())
+ return;
- if (!response.isHTTP())
- return;
-
- if (m_client->wantsAllStreams())
- return;
+ if (m_client->wantsAllStreams())
+ return;
- // Status code can be null when serving from a Web archive.
- if (response.httpStatusCode() && (response.httpStatusCode() < 100 || response.httpStatusCode() >= 400))
- cancel(frameLoader()->client().fileDoesNotExistError(response));
+ // Status code can be null when serving from a Web archive.
+ if (response.httpStatusCode() && (response.httpStatusCode() < 100 || response.httpStatusCode() >= 400))
+ cancel(frameLoader()->client().fileDoesNotExistError(response));
+ });
}
void NetscapePlugInStreamLoader::didReceiveData(const char* data, unsigned length, long long encodedDataLength, DataPayloadType dataPayloadType)
Modified: trunk/Source/WebCore/loader/NetscapePlugInStreamLoader.h (228851 => 228852)
--- trunk/Source/WebCore/loader/NetscapePlugInStreamLoader.h 2018-02-21 00:51:06 UTC (rev 228851)
+++ trunk/Source/WebCore/loader/NetscapePlugInStreamLoader.h 2018-02-21 01:08:28 UTC (rev 228852)
@@ -60,7 +60,7 @@
void init(ResourceRequest&&, CompletionHandler<void(bool)>&&) override;
void willSendRequest(ResourceRequest&&, const ResourceResponse& redirectResponse, CompletionHandler<void(ResourceRequest&&)>&& callback) override;
- void didReceiveResponse(const ResourceResponse&) override;
+ void didReceiveResponse(const ResourceResponse&, CompletionHandler<void()>&& policyCompletionHandler) override;
void didReceiveData(const char*, unsigned, long long encodedDataLength, DataPayloadType) override;
void didReceiveBuffer(Ref<SharedBuffer>&&, long long encodedDataLength, DataPayloadType) override;
void didFinishLoading(const NetworkLoadMetrics&) override;
Modified: trunk/Source/WebCore/loader/ResourceLoader.cpp (228851 => 228852)
--- trunk/Source/WebCore/loader/ResourceLoader.cpp 2018-02-21 00:51:06 UTC (rev 228851)
+++ trunk/Source/WebCore/loader/ResourceLoader.cpp 2018-02-21 01:08:28 UTC (rev 228852)
@@ -169,21 +169,20 @@
void ResourceLoader::deliverResponseAndData(const ResourceResponse& response, RefPtr<SharedBuffer>&& buffer)
{
- Ref<ResourceLoader> protectedThis(*this);
-
- didReceiveResponse(response);
- if (reachedTerminalState())
- return;
-
- if (buffer) {
- unsigned size = buffer->size();
- didReceiveBuffer(buffer.releaseNonNull(), size, DataPayloadWholeResource);
+ didReceiveResponse(response, [this, protectedThis = makeRef(*this), buffer = WTFMove(buffer)]() mutable {
if (reachedTerminalState())
return;
- }
- NetworkLoadMetrics emptyMetrics;
- didFinishLoading(emptyMetrics);
+ if (buffer) {
+ unsigned size = buffer->size();
+ didReceiveBuffer(buffer.releaseNonNull(), size, DataPayloadWholeResource);
+ if (reachedTerminalState())
+ return;
+ }
+
+ NetworkLoadMetrics emptyMetrics;
+ didFinishLoading(emptyMetrics);
+ });
}
void ResourceLoader::start()
@@ -251,14 +250,14 @@
if (auto* scheduledPairs = m_frame->page()->scheduledRunLoopPairs())
scheduleContext.scheduledPairs = *scheduledPairs;
#endif
- DataURLDecoder::decode(url, scheduleContext, [protectedThis = makeRef(*this), url](auto decodeResult) {
- if (protectedThis->reachedTerminalState())
+ DataURLDecoder::decode(url, scheduleContext, [this, protectedThis = makeRef(*this), url](auto decodeResult) mutable {
+ if (this->reachedTerminalState())
return;
if (!decodeResult) {
protectedThis->didFail(ResourceError(errorDomainWebKitInternal, 0, url, "Data URL decoding failed"));
return;
}
- if (protectedThis->wasCancelled())
+ if (this->wasCancelled())
return;
auto& result = decodeResult.value();
auto dataSize = result.data ? result.data->size() : 0;
@@ -268,15 +267,15 @@
dataResponse.setHTTPStatusText(ASCIILiteral("OK"));
dataResponse.setHTTPHeaderField(HTTPHeaderName::ContentType, result.contentType);
dataResponse.setSource(ResourceResponse::Source::Network);
- protectedThis->didReceiveResponse(dataResponse);
+ this->didReceiveResponse(dataResponse, [this, protectedThis = WTFMove(protectedThis), dataSize, data = "" mutable {
+ if (!this->reachedTerminalState() && dataSize)
+ this->didReceiveBuffer(WTFMove(data), dataSize, DataPayloadWholeResource);
- if (!protectedThis->reachedTerminalState() && dataSize)
- protectedThis->didReceiveBuffer(result.data.releaseNonNull(), dataSize, DataPayloadWholeResource);
-
- if (!protectedThis->reachedTerminalState()) {
- NetworkLoadMetrics emptyMetrics;
- protectedThis->didFinishLoading(emptyMetrics);
- }
+ if (!this->reachedTerminalState()) {
+ NetworkLoadMetrics emptyMetrics;
+ this->didFinishLoading(emptyMetrics);
+ }
+ });
});
}
@@ -459,9 +458,10 @@
FrameLoader::reportAuthenticationChallengeBlocked(m_frame.get(), m_request.url(), ASCIILiteral("it is a cross-origin request"));
}
-void ResourceLoader::didReceiveResponse(const ResourceResponse& r)
+void ResourceLoader::didReceiveResponse(const ResourceResponse& r, CompletionHandler<void()>&& policyCompletionHandler)
{
ASSERT(!m_reachedTerminalState);
+ CompletionHandlerCallingScope completionHandlerCaller(WTFMove(policyCompletionHandler));
// Protect this in this delegate method since the additional processing can do
// anything including possibly derefing this; one example of this is Radar 3266216.
@@ -660,8 +660,7 @@
completionHandler();
return;
}
- didReceiveResponse(response);
- completionHandler();
+ didReceiveResponse(response, WTFMove(completionHandler));
}
void ResourceLoader::didReceiveData(ResourceHandle*, const char* data, unsigned length, int encodedDataLength)
Modified: trunk/Source/WebCore/loader/ResourceLoader.h (228851 => 228852)
--- trunk/Source/WebCore/loader/ResourceLoader.h 2018-02-21 00:51:06 UTC (rev 228851)
+++ trunk/Source/WebCore/loader/ResourceLoader.h 2018-02-21 01:08:28 UTC (rev 228852)
@@ -101,7 +101,7 @@
virtual void willSendRequest(ResourceRequest&&, const ResourceResponse& redirectResponse, CompletionHandler<void(ResourceRequest&&)>&& callback);
virtual void didSendData(unsigned long long bytesSent, unsigned long long totalBytesToBeSent);
- virtual void didReceiveResponse(const ResourceResponse&);
+ virtual void didReceiveResponse(const ResourceResponse&, CompletionHandler<void()>&& policyCompletionHandler);
virtual void didReceiveData(const char*, unsigned, long long encodedDataLength, DataPayloadType);
virtual void didReceiveBuffer(Ref<SharedBuffer>&&, long long encodedDataLength, DataPayloadType);
virtual void didFinishLoading(const NetworkLoadMetrics&);
Modified: trunk/Source/WebCore/loader/SubresourceLoader.cpp (228851 => 228852)
--- trunk/Source/WebCore/loader/SubresourceLoader.cpp 2018-02-21 00:51:06 UTC (rev 228851)
+++ trunk/Source/WebCore/loader/SubresourceLoader.cpp 2018-02-21 01:08:28 UTC (rev 228852)
@@ -288,11 +288,13 @@
#endif
-void SubresourceLoader::didReceiveResponse(const ResourceResponse& response)
+void SubresourceLoader::didReceiveResponse(const ResourceResponse& response, CompletionHandler<void()>&& policyCompletionHandler)
{
ASSERT(!response.isNull());
ASSERT(m_state == Initialized);
+ CompletionHandlerCallingScope completionHandlerCaller(WTFMove(policyCompletionHandler));
+
#if USE(QUICK_LOOK)
if (shouldCreatePreviewLoaderForResponse(response)) {
m_previewLoader = PreviewLoader::create(*this, response);
@@ -335,7 +337,7 @@
if (m_frame && m_frame->page())
m_frame->page()->diagnosticLoggingClient().logDiagnosticMessageWithResult(DiagnosticLoggingKeys::cachedResourceRevalidationKey(), emptyString(), DiagnosticLoggingResultPass, ShouldSample::Yes);
if (!reachedTerminalState())
- ResourceLoader::didReceiveResponse(revalidationResponse);
+ ResourceLoader::didReceiveResponse(revalidationResponse, [completionHandlerCaller = WTFMove(completionHandlerCaller)] { });
return;
}
// Did not get 304 response, continue as a regular resource load.
@@ -356,38 +358,51 @@
if (reachedTerminalState())
return;
- ResourceLoader::didReceiveResponse(response);
- if (reachedTerminalState())
- return;
+ bool isResponseMultipart = response.isMultipart();
+ ResourceLoader::didReceiveResponse(response, [this, protectedThis = WTFMove(protectedThis), isResponseMultipart, completionHandlerCaller = WTFMove(completionHandlerCaller)]() mutable {
+ if (reachedTerminalState())
+ return;
- // FIXME: Main resources have a different set of rules for multipart than images do.
- // Hopefully we can merge those 2 paths.
- if (response.isMultipart() && m_resource->type() != CachedResource::MainResource) {
- m_loadingMultipartContent = true;
+ // FIXME: Main resources have a different set of rules for multipart than images do.
+ // Hopefully we can merge those 2 paths.
+ if (isResponseMultipart && m_resource->type() != CachedResource::MainResource) {
+ m_loadingMultipartContent = true;
- // We don't count multiParts in a CachedResourceLoader's request count
- m_requestCountTracker = std::nullopt;
- if (!m_resource->isImage()) {
- cancel();
- return;
+ // We don't count multiParts in a CachedResourceLoader's request count
+ m_requestCountTracker = std::nullopt;
+ if (!m_resource->isImage()) {
+ cancel();
+ return;
+ }
}
- }
- auto* buffer = resourceData();
- if (m_loadingMultipartContent && buffer && buffer->size()) {
- // The resource data will change as the next part is loaded, so we need to make a copy.
- m_resource->finishLoading(buffer->copy().ptr());
- clearResourceData();
- // Since a subresource loader does not load multipart sections progressively, data was delivered to the loader all at once.
- // After the first multipart section is complete, signal to delegates that this load is "finished"
- NetworkLoadMetrics emptyMetrics;
- m_documentLoader->subresourceLoaderFinishedLoadingOnePart(this);
- didFinishLoadingOnePart(emptyMetrics);
- }
+ auto* buffer = resourceData();
+ if (m_loadingMultipartContent && buffer && buffer->size()) {
+ // The resource data will change as the next part is loaded, so we need to make a copy.
+ m_resource->finishLoading(buffer->copy().ptr());
+ clearResourceData();
+ // Since a subresource loader does not load multipart sections progressively, data was delivered to the loader all at once.
+ // After the first multipart section is complete, signal to delegates that this load is "finished"
+ NetworkLoadMetrics emptyMetrics;
+ m_documentLoader->subresourceLoaderFinishedLoadingOnePart(this);
+ didFinishLoadingOnePart(emptyMetrics);
+ }
- checkForHTTPStatusCodeError();
+ checkForHTTPStatusCodeError();
+
+ if (m_inAsyncResponsePolicyCheck)
+ m_policyForResponseCompletionHandler = completionHandlerCaller.release();
+ });
}
+void SubresourceLoader::didReceiveResponsePolicy()
+{
+ ASSERT(m_inAsyncResponsePolicyCheck);
+ m_inAsyncResponsePolicyCheck = false;
+ if (auto completionHandler = WTFMove(m_policyForResponseCompletionHandler))
+ completionHandler();
+}
+
void SubresourceLoader::didReceiveData(const char* data, unsigned length, long long encodedDataLength, DataPayloadType dataPayloadType)
{
#if USE(QUICK_LOOK)
@@ -659,6 +674,9 @@
ASSERT(!reachedTerminalState());
LOG(ResourceLoading, "Cancelled load of '%s'.\n", m_resource->url().string().latin1().data());
+ if (auto policyForResponseCompletionHandler = WTFMove(m_policyForResponseCompletionHandler))
+ policyForResponseCompletionHandler();
+
Ref<SubresourceLoader> protectedThis(*this);
#if PLATFORM(IOS)
m_state = m_state == Uninitialized ? CancelledWhileInitializing : Finishing;
Modified: trunk/Source/WebCore/loader/SubresourceLoader.h (228851 => 228852)
--- trunk/Source/WebCore/loader/SubresourceLoader.h 2018-02-21 00:51:06 UTC (rev 228851)
+++ trunk/Source/WebCore/loader/SubresourceLoader.h 2018-02-21 01:08:28 UTC (rev 228852)
@@ -30,6 +30,7 @@
#include "FrameLoaderTypes.h"
#include "ResourceLoader.h"
+#include <wtf/CompletionHandler.h>
#include <wtf/text/WTFString.h>
namespace WebCore {
@@ -60,6 +61,9 @@
unsigned redirectCount() const { return m_redirectCount; }
+ void markInAsyncResponsePolicyCheck() { m_inAsyncResponsePolicyCheck = true; }
+ void didReceiveResponsePolicy();
+
private:
SubresourceLoader(Frame&, CachedResource&, const ResourceLoaderOptions&);
@@ -67,7 +71,7 @@
void willSendRequestInternal(ResourceRequest&&, const ResourceResponse& redirectResponse, CompletionHandler<void(ResourceRequest&&)>&&) override;
void didSendData(unsigned long long bytesSent, unsigned long long totalBytesToBeSent) override;
- void didReceiveResponse(const ResourceResponse&) override;
+ void didReceiveResponse(const ResourceResponse&, CompletionHandler<void()>&& policyCompletionHandler) override;
void didReceiveData(const char*, unsigned, long long encodedDataLength, DataPayloadType) override;
void didReceiveBuffer(Ref<SharedBuffer>&&, long long encodedDataLength, DataPayloadType) override;
void didFinishLoading(const NetworkLoadMetrics&) override;
@@ -124,8 +128,10 @@
SubresourceLoaderState m_state;
std::optional<RequestCountTracker> m_requestCountTracker;
RefPtr<SecurityOrigin> m_origin;
+ CompletionHandler<void()> m_policyForResponseCompletionHandler;
unsigned m_redirectCount { 0 };
bool m_loadingMultipartContent { false };
+ bool m_inAsyncResponsePolicyCheck { false };
};
}
Modified: trunk/Source/WebCore/loader/ios/PreviewLoader.mm (228851 => 228852)
--- trunk/Source/WebCore/loader/ios/PreviewLoader.mm 2018-02-21 00:51:06 UTC (rev 228851)
+++ trunk/Source/WebCore/loader/ios/PreviewLoader.mm 2018-02-21 01:08:28 UTC (rev 228852)
@@ -130,7 +130,7 @@
_resourceLoader->documentLoader()->setPreviewConverter(WTFMove(_converter));
_hasSentDidReceiveResponse = YES;
- _resourceLoader->didReceiveResponse(response);
+ _resourceLoader->didReceiveResponse(response, nullptr);
}
- (void)connection:(NSURLConnection *)connection didReceiveData:(NSData *)data lengthReceived:(long long)lengthReceived
Modified: trunk/Source/WebKit/ChangeLog (228851 => 228852)
--- trunk/Source/WebKit/ChangeLog 2018-02-21 00:51:06 UTC (rev 228851)
+++ trunk/Source/WebKit/ChangeLog 2018-02-21 01:08:28 UTC (rev 228852)
@@ -1,3 +1,18 @@
+2018-02-20 Chris Dumez <[email protected]>
+
+ Provisional load may get committed before receiving the decidePolicyForNavigationResponse response
+ https://bugs.webkit.org/show_bug.cgi?id=182720
+ <rdar://problem/37515204>
+
+ Reviewed by Alex Christensen.
+
+ * WebProcess/Network/WebResourceLoader.cpp:
+ (WebKit::WebResourceLoader::didReceiveResponse):
+ * WebProcess/Storage/ServiceWorkerClientFetch.cpp:
+ (WebKit::ServiceWorkerClientFetch::didReceiveResponse):
+ * WebProcess/WebPage/WebURLSchemeTaskProxy.cpp:
+ (WebKit::WebURLSchemeTaskProxy::didReceiveResponse):
+
2018-02-20 Matt Lewis <[email protected]>
Unreviewed, rolling out r228829.
Modified: trunk/Source/WebKit/WebProcess/Network/WebResourceLoader.cpp (228851 => 228852)
--- trunk/Source/WebKit/WebProcess/Network/WebResourceLoader.cpp 2018-02-21 00:51:06 UTC (rev 228851)
+++ trunk/Source/WebKit/WebProcess/Network/WebResourceLoader.cpp 2018-02-21 01:08:28 UTC (rev 228852)
@@ -107,19 +107,21 @@
LOG(Network, "(WebProcess) WebResourceLoader::didReceiveResponse for '%s'. Status %d.", m_coreLoader->url().string().latin1().data(), response.httpStatusCode());
RELEASE_LOG_IF_ALLOWED("didReceiveResponse: (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ", status = %d)", m_trackingParameters.pageID, m_trackingParameters.frameID, m_trackingParameters.resourceID, response.httpStatusCode());
- Ref<WebResourceLoader> protect(*this);
+ Ref<WebResourceLoader> protectedThis(*this);
if (m_coreLoader->documentLoader()->applicationCacheHost().maybeLoadFallbackForResponse(m_coreLoader.get(), response))
return;
- m_coreLoader->didReceiveResponse(response);
+ CompletionHandler<void()> policyDecisionCompletionHandler;
+ if (needsContinueDidReceiveResponseMessage) {
+ policyDecisionCompletionHandler = [this, protectedThis = WTFMove(protectedThis)] {
+ // If m_coreLoader becomes null as a result of the didReceiveResponse callback, we can't use the send function().
+ if (m_coreLoader)
+ send(Messages::NetworkResourceLoader::ContinueDidReceiveResponse());
+ };
+ }
- // If m_coreLoader becomes null as a result of the didReceiveResponse callback, we can't use the send function().
- if (!m_coreLoader)
- return;
-
- if (needsContinueDidReceiveResponseMessage)
- send(Messages::NetworkResourceLoader::ContinueDidReceiveResponse());
+ m_coreLoader->didReceiveResponse(response, WTFMove(policyDecisionCompletionHandler));
}
void WebResourceLoader::didReceiveData(const IPC::DataReference& data, int64_t encodedDataLength)
Modified: trunk/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp (228851 => 228852)
--- trunk/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp 2018-02-21 00:51:06 UTC (rev 228851)
+++ trunk/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp 2018-02-21 01:08:28 UTC (rev 228852)
@@ -141,9 +141,10 @@
if (response.url().isNull())
response.setURL(m_loader->request().url());
- m_loader->didReceiveResponse(response);
- if (auto callback = WTFMove(m_callback))
- callback(Result::Succeeded);
+ m_loader->didReceiveResponse(response, [this, protectedThis = WTFMove(protectedThis)] {
+ if (auto callback = WTFMove(m_callback))
+ callback(Result::Succeeded);
+ });
});
}
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebURLSchemeTaskProxy.cpp (228851 => 228852)
--- trunk/Source/WebKit/WebProcess/WebPage/WebURLSchemeTaskProxy.cpp 2018-02-21 00:51:06 UTC (rev 228851)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebURLSchemeTaskProxy.cpp 2018-02-21 01:08:28 UTC (rev 228852)
@@ -93,7 +93,7 @@
if (!hasLoader())
return;
- m_coreLoader->didReceiveResponse(response);
+ m_coreLoader->didReceiveResponse(response, nullptr);
}
void WebURLSchemeTaskProxy::didReceiveData(size_t size, const uint8_t* data)
Modified: trunk/Tools/ChangeLog (228851 => 228852)
--- trunk/Tools/ChangeLog 2018-02-21 00:51:06 UTC (rev 228851)
+++ trunk/Tools/ChangeLog 2018-02-21 01:08:28 UTC (rev 228852)
@@ -1,3 +1,17 @@
+2018-02-20 Chris Dumez <[email protected]>
+
+ Provisional load may get committed before receiving the decidePolicyForNavigationResponse response
+ https://bugs.webkit.org/show_bug.cgi?id=182720
+ <rdar://problem/37515204>
+
+ Reviewed by Alex Christensen.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/AsyncPolicyForNavigationResponse.mm:
+ (-[TestAsyncNavigationDelegate webView:decidePolicyForNavigationResponse:decisionHandler:]):
+ (TestWebKitAPI::TEST):
+
2018-02-20 Nan Wang <[email protected]>
AX: AOM: Dispatch accessibleclick event
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/AsyncPolicyForNavigationResponse.mm (228851 => 228852)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/AsyncPolicyForNavigationResponse.mm 2018-02-21 00:51:06 UTC (rev 228851)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/AsyncPolicyForNavigationResponse.mm 2018-02-21 01:08:28 UTC (rev 228852)
@@ -31,6 +31,7 @@
#import <wtf/RetainPtr.h>
#if WK_API_ENABLED
+static bool shouldAccept = true;
static bool navigationComplete = false;
static bool navigationFailed = false;
@@ -66,7 +67,7 @@
int64_t deferredWaitTime = 100 * NSEC_PER_MSEC;
dispatch_time_t when = dispatch_time(DISPATCH_TIME_NOW, deferredWaitTime);
dispatch_after(when, dispatch_get_main_queue(), ^{
- decisionHandler(WKNavigationResponsePolicyAllow);
+ decisionHandler(shouldAccept ? WKNavigationResponsePolicyAllow : WKNavigationResponsePolicyCancel);
});
}
@end
@@ -82,6 +83,9 @@
[webView setNavigationDelegate:delegate.get()];
[webView setUIDelegate:delegate.get()];
+ shouldAccept = true;
+ navigationFailed = false;
+ navigationComplete = false;
[webView loadRequest:[NSURLRequest requestWithURL:testURL.get()]];
TestWebKitAPI::Util::run(&navigationComplete);
@@ -88,6 +92,24 @@
EXPECT_FALSE(navigationFailed);
}
+TEST(WebKit, PolicyForNavigationResponseCancelAsynchronously)
+{
+ RetainPtr<NSURL> testURL = [[NSBundle mainBundle] URLForResource:@"simple" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
+
+ auto webView = adoptNS([[WKWebView alloc] init]);
+ auto delegate = adoptNS([[TestAsyncNavigationDelegate alloc] init]);
+ [webView setNavigationDelegate:delegate.get()];
+ [webView setUIDelegate:delegate.get()];
+
+ shouldAccept = false;
+ navigationFailed = false;
+ navigationComplete = false;
+ [webView loadRequest:[NSURLRequest requestWithURL:testURL.get()]];
+ TestWebKitAPI::Util::run(&navigationComplete);
+
+ EXPECT_TRUE(navigationFailed);
+}
+
} // namespace TestWebKitAPI
#endif // WK_API_ENABLED
