QuickLook.NavigationDelegate API test is failing on iOS with async policy delegates
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Mar 2018 22:54:21 +0000 (22:54 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Mar 2018 22:54:21 +0000 (22:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=183791

Reviewed by Alex Christensen.

Source/WebCore:

Update PreviewLoader to not send data (or call finishFinishLoading) until
the resource response has been processed.

* loader/ios/PreviewLoader.mm:
(-[WebPreviewLoader _sendDidReceiveResponseIfNecessary]):
(-[WebPreviewLoader connection:didReceiveData:lengthReceived:]):
(-[WebPreviewLoader connectionDidFinishLoading:]):

Tools:

Add API test coverage.

* TestWebKitAPI/Tests/WebKitCocoa/QuickLook.mm:
(-[QuickLookAsyncNavigationDelegate webView:decidePolicyForNavigationResponse:decisionHandler:]):
(-[QuickLookAsyncNavigationDelegate _webView:didStartLoadForQuickLookDocumentInMainFrameWithFileName:uti:]):
(-[QuickLookAsyncNavigationDelegate _webView:didFinishLoadForQuickLookDocumentInMainFrame:]):
(-[QuickLookAsyncNavigationDelegate webView:didFinishNavigation:]):
(TEST):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@229776 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/loader/ios/PreviewLoader.mm
Source/WebCore/platform/SharedBuffer.h
Source/WebCore/platform/cocoa/SharedBufferCocoa.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/QuickLook.mm

index 24ebe5f..e71a5ff 100644 (file)
@@ -1,3 +1,18 @@
+2018-03-20  Chris Dumez  <cdumez@apple.com>
+
+        QuickLook.NavigationDelegate API test is failing on iOS with async policy delegates
+        https://bugs.webkit.org/show_bug.cgi?id=183791
+
+        Reviewed by Alex Christensen.
+
+        Update PreviewLoader to not send data (or call finishFinishLoading) until
+        the resource response has been processed.
+
+        * loader/ios/PreviewLoader.mm:
+        (-[WebPreviewLoader _sendDidReceiveResponseIfNecessary]):
+        (-[WebPreviewLoader connection:didReceiveData:lengthReceived:]):
+        (-[WebPreviewLoader connectionDidFinishLoading:]):
+
 2018-03-20  Antoine Quint  <graouts@apple.com>
 
         [Web Animations] Update the timing model when pending tasks schedule changes
index 9c31260..95334cc 100644 (file)
@@ -48,6 +48,10 @@ using namespace WebCore;
     std::unique_ptr<PreviewConverter> _converter;
     RetainPtr<NSMutableArray> _bufferedDataArray;
     BOOL _hasSentDidReceiveResponse;
+    BOOL _hasProcessedResponse;
+    RefPtr<SharedBuffer> _bufferedData;
+    long long _lengthReceived;
+    BOOL _needsToCallDidFinishLoading;
 }
 
 - (instancetype)initWithResourceLoader:(ResourceLoader&)resourceLoader resourceResponse:(const ResourceResponse&)resourceResponse;
@@ -130,7 +134,26 @@ static PreviewLoaderClient& emptyClient()
     _resourceLoader->documentLoader()->setPreviewConverter(WTFMove(_converter));
 
     _hasSentDidReceiveResponse = YES;
-    _resourceLoader->didReceiveResponse(response, nullptr);
+    _hasProcessedResponse = NO;
+    _resourceLoader->didReceiveResponse(response, [self, retainedSelf = retainPtr(self)] {
+        _hasProcessedResponse = YES;
+
+        if (_resourceLoader->reachedTerminalState())
+            return;
+
+        if (auto bufferedData = WTFMove(_bufferedData)) {
+            _resourceLoader->didReceiveData(bufferedData->data(), bufferedData->size(), _lengthReceived, DataPayloadBytes);
+            _lengthReceived = 0;
+        }
+
+        if (_resourceLoader->reachedTerminalState())
+            return;
+
+        if (_needsToCallDidFinishLoading) {
+            _needsToCallDidFinishLoading = NO;
+            _resourceLoader->didFinishLoading(NetworkLoadMetrics { });
+        }
+    });
 }
 
 - (void)connection:(NSURLConnection *)connection didReceiveData:(NSData *)data lengthReceived:(long long)lengthReceived
@@ -141,10 +164,23 @@ static PreviewLoaderClient& emptyClient()
     
     [self _sendDidReceiveResponseIfNecessary];
 
+    auto dataLength = data.length;
+
     // QuickLook code sends us a nil data at times. The check below is the same as the one in
     // ResourceHandleMac.cpp added for a different bug.
-    if (auto dataLength = data.length)
+    if (!dataLength)
+        return;
+
+    if (_hasProcessedResponse) {
         _resourceLoader->didReceiveData(reinterpret_cast<const char*>(data.bytes), dataLength, lengthReceived, DataPayloadBytes);
+        return;
+    }
+
+    if (!_bufferedData)
+        _bufferedData = SharedBuffer::create(data);
+    else
+        _bufferedData->append(data);
+    _lengthReceived += lengthReceived;
 }
 
 - (void)connectionDidFinishLoading:(NSURLConnection *)connection
@@ -155,8 +191,12 @@ static PreviewLoaderClient& emptyClient()
     
     ASSERT(_hasSentDidReceiveResponse);
 
-    NetworkLoadMetrics emptyMetrics;
-    _resourceLoader->didFinishLoading(emptyMetrics);
+    if (!_hasProcessedResponse) {
+        _needsToCallDidFinishLoading = YES;
+        return;
+    }
+
+    _resourceLoader->didFinishLoading(NetworkLoadMetrics { });
 }
 
 static inline bool isQuickLookPasswordError(NSError *error)
index fc023ee..88b84ac 100644 (file)
@@ -66,6 +66,7 @@ public:
     RetainPtr<NSData> createNSData() const;
     RetainPtr<NSArray> createNSDataArray() const;
     static Ref<SharedBuffer> create(NSData *);
+    void append(NSData *);
 #endif
 #if USE(CF)
     RetainPtr<CFDataRef> createCFData() const;
index 911efac..594d110 100644 (file)
@@ -88,6 +88,11 @@ Ref<SharedBuffer> SharedBuffer::create(NSData *nsData)
     return adoptRef(*new SharedBuffer((CFDataRef)nsData));
 }
 
+void SharedBuffer::append(NSData *nsData)
+{
+    return append((CFDataRef)nsData);
+}
+
 RetainPtr<NSData> SharedBuffer::createNSData() const
 {
     return adoptNS((NSData *)createCFData().leakRef());
index ed6993d..b84ddcc 100644 (file)
@@ -1,3 +1,19 @@
+2018-03-20  Chris Dumez  <cdumez@apple.com>
+
+        QuickLook.NavigationDelegate API test is failing on iOS with async policy delegates
+        https://bugs.webkit.org/show_bug.cgi?id=183791
+
+        Reviewed by Alex Christensen.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/QuickLook.mm:
+        (-[QuickLookAsyncNavigationDelegate webView:decidePolicyForNavigationResponse:decisionHandler:]):
+        (-[QuickLookAsyncNavigationDelegate _webView:didStartLoadForQuickLookDocumentInMainFrameWithFileName:uti:]):
+        (-[QuickLookAsyncNavigationDelegate _webView:didFinishLoadForQuickLookDocumentInMainFrame:]):
+        (-[QuickLookAsyncNavigationDelegate webView:didFinishNavigation:]):
+        (TEST):
+
 2018-03-20  Tim Horton  <timothy_horton@apple.com>
 
         Add and adopt WK_PLATFORM_NAME and adjust default feature defines
index 58f8243..b9f5d3b 100644 (file)
@@ -74,6 +74,41 @@ static NSURLRequest * const pagesDocumentRequest = [[NSURLRequest requestWithURL
 
 @end
 
+@interface QuickLookAsyncNavigationDelegate : NSObject <WKNavigationDelegatePrivate>
+@end
+
+@implementation QuickLookAsyncNavigationDelegate
+
+- (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);
+    });
+}
+
+- (void)_webView:(WKWebView *)webView didStartLoadForQuickLookDocumentInMainFrameWithFileName:(NSString *)fileName uti:(NSString *)uti
+{
+    EXPECT_WK_STREQ(expectedFileName, fileName);
+    EXPECT_WK_STREQ(expectedUTI, uti);
+    didStartQuickLookLoad = true;
+}
+
+- (void)_webView:(WKWebView *)webView didFinishLoadForQuickLookDocumentInMainFrame:(NSData *)documentData
+{
+    EXPECT_EQ(expectedFileSize, documentData.length);
+    didFinishQuickLookLoad = true;
+}
+
+- (void)webView:(WKWebView *)webView didFinishNavigation:(WKNavigation *)navigation
+{
+    isDone = true;
+}
+
+@end
+
 static void runTest(Class navigationDelegateClass, NSURLRequest *request)
 {
     auto webView = adoptNS([[WKWebView alloc] init]);
@@ -92,6 +127,13 @@ TEST(QuickLook, NavigationDelegate)
     EXPECT_TRUE(didFinishQuickLookLoad);
 }
 
+TEST(QuickLook, AsyncNavigationDelegate)
+{
+    runTest([QuickLookAsyncNavigationDelegate class], pagesDocumentRequest);
+    EXPECT_TRUE(didStartQuickLookLoad);
+    EXPECT_TRUE(didFinishQuickLookLoad);
+}
+
 @interface QuickLookDecidePolicyDelegate : NSObject <WKNavigationDelegate>
 @end