- Revision
- 229666
- Author
- [email protected]
- Date
- 2018-03-16 09:11:54 -0700 (Fri, 16 Mar 2018)
Log Message
URLSchemeHandler.Basic API test fails with async policy delegates
https://bugs.webkit.org/show_bug.cgi?id=183678
Reviewed by Alex Christensen.
Source/WebKit:
The issue is that the client calls _didPerformRedirection / didReceiveResponse / didReceiveData / didFinish
on the URLScheme task one after the one, synchronously. However, redirects and responses can be processed
asynchronously. To address the issue, we now queue operations requested by the client if we're waiting
for an async policy delegate.
* WebProcess/WebPage/WebURLSchemeTaskProxy.cpp:
(WebKit::WebURLSchemeTaskProxy::didPerformRedirection):
(WebKit::WebURLSchemeTaskProxy::didReceiveResponse):
(WebKit::WebURLSchemeTaskProxy::didReceiveData):
(WebKit::WebURLSchemeTaskProxy::didComplete):
(WebKit::WebURLSchemeTaskProxy::processNextPendingTask):
* WebProcess/WebPage/WebURLSchemeTaskProxy.h:
(WebKit::WebURLSchemeTaskProxy::queueTask):
Tools:
Add API test coverage.
* TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-1.mm:
(-[URLSchemeHandlerAsyncNavigationDelegate webView:decidePolicyForNavigationAction:decisionHandler:]):
(-[URLSchemeHandlerAsyncNavigationDelegate webView:decidePolicyForNavigationResponse:decisionHandler:]):
(TEST):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (229665 => 229666)
--- trunk/Source/WebKit/ChangeLog 2018-03-16 15:12:25 UTC (rev 229665)
+++ trunk/Source/WebKit/ChangeLog 2018-03-16 16:11:54 UTC (rev 229666)
@@ -1,3 +1,24 @@
+2018-03-16 Chris Dumez <[email protected]>
+
+ URLSchemeHandler.Basic API test fails with async policy delegates
+ https://bugs.webkit.org/show_bug.cgi?id=183678
+
+ Reviewed by Alex Christensen.
+
+ The issue is that the client calls _didPerformRedirection / didReceiveResponse / didReceiveData / didFinish
+ on the URLScheme task one after the one, synchronously. However, redirects and responses can be processed
+ asynchronously. To address the issue, we now queue operations requested by the client if we're waiting
+ for an async policy delegate.
+
+ * WebProcess/WebPage/WebURLSchemeTaskProxy.cpp:
+ (WebKit::WebURLSchemeTaskProxy::didPerformRedirection):
+ (WebKit::WebURLSchemeTaskProxy::didReceiveResponse):
+ (WebKit::WebURLSchemeTaskProxy::didReceiveData):
+ (WebKit::WebURLSchemeTaskProxy::didComplete):
+ (WebKit::WebURLSchemeTaskProxy::processNextPendingTask):
+ * WebProcess/WebPage/WebURLSchemeTaskProxy.h:
+ (WebKit::WebURLSchemeTaskProxy::queueTask):
+
2018-03-16 Claudio Saavedra <[email protected]>
Suppress GCC warnings by using #include instead of #import
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebURLSchemeTaskProxy.cpp (229665 => 229666)
--- trunk/Source/WebKit/WebProcess/WebPage/WebURLSchemeTaskProxy.cpp 2018-03-16 15:12:25 UTC (rev 229665)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebURLSchemeTaskProxy.cpp 2018-03-16 16:11:54 UTC (rev 229666)
@@ -70,16 +70,23 @@
return;
auto completionHandler = [this, protectedThis = makeRef(*this), originalRequest = request] (ResourceRequest&& request) {
- m_waitingForRedirectCompletionHandler = false;
+ m_waitingForCompletionHandler = false;
// We do not inform the UIProcess of WebKit's new request with the given suggested request.
// We do want to know if WebKit would have generated a request that differs from the suggested request, though.
if (request.url() != originalRequest.url())
WTFLogAlways("Redirected scheme task would have been sent to a different URL.");
+
+ processNextPendingTask();
};
- if (m_waitingForRedirectCompletionHandler)
- WTFLogAlways("Received redirect during previous redirect processing.");
- m_waitingForRedirectCompletionHandler = true;
+ if (m_waitingForCompletionHandler) {
+ WTFLogAlways("Received redirect during previous redirect processing, queuing it.");
+ queueTask([this, protectedThis = makeRef(*this), redirectResponse = WTFMove(redirectResponse), request = WTFMove(request)]() mutable {
+ didPerformRedirection(WTFMove(redirectResponse), WTFMove(request));
+ });
+ return;
+ }
+ m_waitingForCompletionHandler = true;
m_coreLoader->willSendRequest(WTFMove(request), redirectResponse, WTFMove(completionHandler));
}
@@ -86,13 +93,22 @@
void WebURLSchemeTaskProxy::didReceiveResponse(const ResourceResponse& response)
{
- if (m_waitingForRedirectCompletionHandler)
- WTFLogAlways("Received response during redirect processing.");
+ if (m_waitingForCompletionHandler) {
+ WTFLogAlways("Received response during redirect processing, queuing it.");
+ queueTask([this, protectedThis = makeRef(*this), response] {
+ didReceiveResponse(response);
+ });
+ return;
+ }
if (!hasLoader())
return;
- m_coreLoader->didReceiveResponse(response, nullptr);
+ m_waitingForCompletionHandler = true;
+ m_coreLoader->didReceiveResponse(response, [this, protectedThis = makeRef(*this)] {
+ m_waitingForCompletionHandler = false;
+ processNextPendingTask();
+ });
}
void WebURLSchemeTaskProxy::didReceiveData(size_t size, const uint8_t* data)
@@ -100,7 +116,18 @@
if (!hasLoader())
return;
+ if (m_waitingForCompletionHandler) {
+ WTFLogAlways("Received data during response processing, queuing it.");
+ Vector<uint8_t> dataVector;
+ dataVector.append(data, size);
+ queueTask([this, protectedThis = makeRef(*this), dataVector = WTFMove(dataVector)] {
+ didReceiveData(dataVector.size(), dataVector.data());
+ });
+ return;
+ }
+
m_coreLoader->didReceiveData(reinterpret_cast<const char*>(data), size, 0, DataPayloadType::DataPayloadBytes);
+ processNextPendingTask();
}
void WebURLSchemeTaskProxy::didComplete(const ResourceError& error)
@@ -108,6 +135,13 @@
if (!hasLoader())
return;
+ if (m_waitingForCompletionHandler) {
+ queueTask([this, protectedThis = makeRef(*this), error] {
+ didComplete(error);
+ });
+ return;
+ }
+
if (error.isNull())
m_coreLoader->didFinishLoading(NetworkLoadMetrics());
else
@@ -124,4 +158,10 @@
return m_coreLoader;
}
+void WebURLSchemeTaskProxy::processNextPendingTask()
+{
+ if (!m_queuedTasks.isEmpty())
+ m_queuedTasks.takeFirst()();
+}
+
} // namespace WebKit
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebURLSchemeTaskProxy.h (229665 => 229666)
--- trunk/Source/WebKit/WebProcess/WebPage/WebURLSchemeTaskProxy.h 2018-03-16 15:12:25 UTC (rev 229665)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebURLSchemeTaskProxy.h 2018-03-16 16:11:54 UTC (rev 229666)
@@ -26,6 +26,7 @@
#pragma once
#include <WebCore/ResourceRequest.h>
+#include <wtf/Deque.h>
#include <wtf/RefCounted.h>
namespace WebCore {
@@ -61,11 +62,15 @@
WebURLSchemeTaskProxy(WebURLSchemeHandlerProxy&, WebCore::ResourceLoader&);
bool hasLoader();
+ void queueTask(Function<void()>&& task) { m_queuedTasks.append(WTFMove(task)); }
+ void processNextPendingTask();
+
WebURLSchemeHandlerProxy& m_urlSchemeHandler;
RefPtr<WebCore::ResourceLoader> m_coreLoader;
WebCore::ResourceRequest m_request;
unsigned long m_identifier;
- bool m_waitingForRedirectCompletionHandler { };
+ bool m_waitingForCompletionHandler { false };
+ Deque<Function<void()>> m_queuedTasks;
};
} // namespace WebKit
Modified: trunk/Tools/ChangeLog (229665 => 229666)
--- trunk/Tools/ChangeLog 2018-03-16 15:12:25 UTC (rev 229665)
+++ trunk/Tools/ChangeLog 2018-03-16 16:11:54 UTC (rev 229666)
@@ -1,3 +1,17 @@
+2018-03-16 Chris Dumez <[email protected]>
+
+ URLSchemeHandler.Basic API test fails with async policy delegates
+ https://bugs.webkit.org/show_bug.cgi?id=183678
+
+ Reviewed by Alex Christensen.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-1.mm:
+ (-[URLSchemeHandlerAsyncNavigationDelegate webView:decidePolicyForNavigationAction:decisionHandler:]):
+ (-[URLSchemeHandlerAsyncNavigationDelegate webView:decidePolicyForNavigationResponse:decisionHandler:]):
+ (TEST):
+
2018-03-16 Zalan Bujtas <[email protected]>
[LayoutReloaded] Utils.computed* functions should just take node instead of box.
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-1.mm (229665 => 229666)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-1.mm 2018-03-16 15:12:25 UTC (rev 229665)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-1.mm 2018-03-16 16:11:54 UTC (rev 229666)
@@ -99,6 +99,32 @@
@end
+@interface URLSchemeHandlerAsyncNavigationDelegate : NSObject <WKNavigationDelegate, WKUIDelegate>
+@end
+
+@implementation URLSchemeHandlerAsyncNavigationDelegate
+
+- (void)webView:(WKWebView *)webView decidePolicyForNavigationAction:(WKNavigationAction *)navigationAction decisionHandler:(void (^)(WKNavigationActionPolicy))decisionHandler
+{
+ 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(WKNavigationActionPolicyAllow);
+ });
+
+}
+
+- (void)webView:(WKWebView *)webView decidePolicyForNavigationResponse:(WKNavigationResponse *)navigationResponse decisionHandler:(void (^)(WKNavigationResponsePolicy))decisionHandler
+{
+ 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);
+ });
+}
+@end
+
+
static const char mainBytes[] =
"<html>" \
"<img src=''>" \
@@ -126,6 +152,30 @@
EXPECT_EQ([handler.get().stoppedURLs count], 0u);
}
+TEST(URLSchemeHandler, BasicWithAsyncPolicyDelegate)
+{
+ done = false;
+
+ RetainPtr<WKWebViewConfiguration> configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+
+ RetainPtr<SchemeHandler> handler = adoptNS([[SchemeHandler alloc] initWithData:[NSData dataWithBytesNoCopy:(void*)mainBytes length:sizeof(mainBytes) freeWhenDone:NO] mimeType:@"text/html"]);
+ [configuration setURLSchemeHandler:handler.get() forURLScheme:@"testing"];
+
+ RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
+ auto delegate = adoptNS([[URLSchemeHandlerAsyncNavigationDelegate alloc] init]);
+ [webView setNavigationDelegate:delegate.get()];
+
+ NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"testing:main"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+
+ EXPECT_EQ([handler.get().startedURLs count], 2u);
+ EXPECT_TRUE([[handler.get().startedURLs objectAtIndex:0] isEqual:[NSURL URLWithString:@"testing:main"]]);
+ EXPECT_TRUE([[handler.get().startedURLs objectAtIndex:1] isEqual:[NSURL URLWithString:@"testing:image"]]);
+ EXPECT_EQ([handler.get().stoppedURLs count], 0u);
+}
+
TEST(URLSchemeHandler, NoMIMEType)
{
// Since there's no MIMEType, and no NavigationDelegate to tell WebKit to do the load anyways, WebKit will ignore (silently fail) the load.
