REGRESSION (r243682): Quick Look previews loaded from the memory cache render with...
authoraestes@apple.com <aestes@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 14 Oct 2019 22:50:48 +0000 (22:50 +0000)
committeraestes@apple.com <aestes@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 14 Oct 2019 22:50:48 +0000 (22:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=202935
<rdar://problem/54318133>

Reviewed by Tim Horton.

Source/WebCore:

When loading a Quick Look preview after deciding content policy, PreviewLoader would update
DocumentLoader with the preview response (the response that contains the preview's
Content-Type). It would not update the CachedResource representing the preview main
resource, however, which caches the underlying ResourceResponse in m_response.

When loading from the memory cache, it's the CachedResource's response that's used to
synthesize DocumentLoader::responseReceived. When loading Quick Look previews *before*
deciding content policy, this response would be the preview response, but as described
above, when loading after deciding content policy it's the underlying response.

This patch updates a Quick Look preview's CachedResource with the preview response along
with updating DocumentLoader so that there is not a mismatch between the resource's content
type and its underlying data.

Added a new API test.

* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::previewResponseReceived):
* loader/DocumentLoader.h:
* loader/ResourceLoader.h:
(WebCore::ResourceLoader::didReceivePreviewResponse):
* loader/SubresourceLoader.cpp:
(WebCore::SubresourceLoader::didReceivePreviewResponse):
* loader/SubresourceLoader.h:
* loader/cache/CachedRawResource.cpp:
(WebCore::CachedRawResource::previewResponseReceived):
* loader/cache/CachedRawResource.h:
* loader/cache/CachedRawResourceClient.h:
(WebCore::CachedRawResourceClient::previewResponseReceived):
* loader/cache/CachedResource.cpp:
(WebCore::CachedResource::previewResponseReceived):
* loader/cache/CachedResource.h:
* loader/ios/PreviewLoader.mm:
(-[WebPreviewLoader _loadPreviewIfNeeded]):

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/QuickLook.mm:
(TEST):
* TestWebKitAPI/cocoa/TestWKWebView.h:
* TestWebKitAPI/cocoa/TestWKWebView.mm:
(-[WKWebView synchronouslyGoBack]):
(-[WKWebView synchronouslyGoForward]):

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

16 files changed:
Source/WebCore/ChangeLog
Source/WebCore/loader/DocumentLoader.cpp
Source/WebCore/loader/DocumentLoader.h
Source/WebCore/loader/ResourceLoader.h
Source/WebCore/loader/SubresourceLoader.cpp
Source/WebCore/loader/SubresourceLoader.h
Source/WebCore/loader/cache/CachedRawResource.cpp
Source/WebCore/loader/cache/CachedRawResource.h
Source/WebCore/loader/cache/CachedRawResourceClient.h
Source/WebCore/loader/cache/CachedResource.cpp
Source/WebCore/loader/cache/CachedResource.h
Source/WebCore/loader/ios/PreviewLoader.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/QuickLook.mm
Tools/TestWebKitAPI/cocoa/TestWKWebView.h
Tools/TestWebKitAPI/cocoa/TestWKWebView.mm

index 0e97f72..553cc0b 100644 (file)
@@ -1,3 +1,46 @@
+2019-10-14  Andy Estes  <aestes@apple.com>
+
+        REGRESSION (r243682): Quick Look previews loaded from the memory cache render with the wrong content type
+        https://bugs.webkit.org/show_bug.cgi?id=202935
+        <rdar://problem/54318133>
+
+        Reviewed by Tim Horton.
+
+        When loading a Quick Look preview after deciding content policy, PreviewLoader would update
+        DocumentLoader with the preview response (the response that contains the preview's
+        Content-Type). It would not update the CachedResource representing the preview main
+        resource, however, which caches the underlying ResourceResponse in m_response.
+
+        When loading from the memory cache, it's the CachedResource's response that's used to
+        synthesize DocumentLoader::responseReceived. When loading Quick Look previews *before*
+        deciding content policy, this response would be the preview response, but as described
+        above, when loading after deciding content policy it's the underlying response.
+
+        This patch updates a Quick Look preview's CachedResource with the preview response along
+        with updating DocumentLoader so that there is not a mismatch between the resource's content
+        type and its underlying data.
+
+        Added a new API test.
+
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::previewResponseReceived):
+        * loader/DocumentLoader.h:
+        * loader/ResourceLoader.h:
+        (WebCore::ResourceLoader::didReceivePreviewResponse):
+        * loader/SubresourceLoader.cpp:
+        (WebCore::SubresourceLoader::didReceivePreviewResponse):
+        * loader/SubresourceLoader.h:
+        * loader/cache/CachedRawResource.cpp:
+        (WebCore::CachedRawResource::previewResponseReceived):
+        * loader/cache/CachedRawResource.h:
+        * loader/cache/CachedRawResourceClient.h:
+        (WebCore::CachedRawResourceClient::previewResponseReceived):
+        * loader/cache/CachedResource.cpp:
+        (WebCore::CachedResource::previewResponseReceived):
+        * loader/cache/CachedResource.h:
+        * loader/ios/PreviewLoader.mm:
+        (-[WebPreviewLoader _loadPreviewIfNeeded]):
+
 2019-10-14  Youenn Fablet  <youenn@apple.com>
 
         A response body promise should be rejected in case of a failure happening after the HTTP response
index 6a63023..fba266a 100644 (file)
@@ -2135,6 +2135,12 @@ bool DocumentLoader::isAlwaysOnLoggingAllowed() const
 
 #if USE(QUICK_LOOK)
 
+void DocumentLoader::previewResponseReceived(CachedResource& resource, const ResourceResponse& response)
+{
+    ASSERT_UNUSED(resource, m_mainResource == &resource);
+    m_response = response;
+}
+
 void DocumentLoader::setPreviewConverter(std::unique_ptr<PreviewConverter>&& previewConverter)
 {
     m_previewConverter = WTFMove(previewConverter);
index a85a076..dffed40 100644 (file)
@@ -432,6 +432,9 @@ private:
     WEBCORE_EXPORT void responseReceived(CachedResource&, const ResourceResponse&, CompletionHandler<void()>&&) override;
     WEBCORE_EXPORT void dataReceived(CachedResource&, const char* data, int length) override;
     WEBCORE_EXPORT void notifyFinished(CachedResource&) override;
+#if USE(QUICK_LOOK)
+    WEBCORE_EXPORT void previewResponseReceived(CachedResource&, const ResourceResponse&) override;
+#endif
 
     void responseReceived(const ResourceResponse&, CompletionHandler<void()>&&);
     void dataReceived(const char* data, int length);
index 5c5c8df..72f4c7a 100644 (file)
@@ -117,6 +117,7 @@ public:
 
 #if USE(QUICK_LOOK)
     bool isQuickLookResource() const;
+    virtual void didReceivePreviewResponse(const ResourceResponse&) { };
 #endif
 
     const URL& url() const { return m_request.url(); }
index a5be875..8e30b75 100644 (file)
@@ -314,6 +314,15 @@ bool SubresourceLoader::shouldCreatePreviewLoaderForResponse(const ResourceRespo
     return PreviewConverter::supportsMIMEType(response.mimeType());
 }
 
+void SubresourceLoader::didReceivePreviewResponse(const ResourceResponse& response)
+{
+    ASSERT(m_state == Initialized);
+    ASSERT(!response.isNull());
+    ASSERT(m_resource);
+    m_resource->previewResponseReceived(response);
+    ResourceLoader::didReceivePreviewResponse(response);
+}
+
 #endif
 
 void SubresourceLoader::didReceiveResponse(const ResourceResponse& response, CompletionHandler<void()>&& policyCompletionHandler)
index dec8d0b..9ecb782 100644 (file)
@@ -100,6 +100,7 @@ private:
 
 #if USE(QUICK_LOOK)
     bool shouldCreatePreviewLoaderForResponse(const ResourceResponse&) const;
+    void didReceivePreviewResponse(const ResourceResponse&) override;
 #endif
 
     enum SubresourceLoaderState {
index f9a75f3..e7dc633 100644 (file)
@@ -341,4 +341,16 @@ void CachedRawResource::clear()
         m_loader->clearResourceData();
 }
 
+#if USE(QUICK_LOOK)
+void CachedRawResource::previewResponseReceived(const ResourceResponse& response)
+{
+    CachedResourceHandle<CachedRawResource> protectedThis(this);
+    CachedResource::previewResponseReceived(response);
+    CachedResourceClientWalker<CachedRawResourceClient> w(m_clients);
+    while (CachedRawResourceClient* c = w.next())
+        c->previewResponseReceived(*this, m_response);
+}
+
+#endif
+
 } // namespace WebCore
index 6376136..27cddd3 100644 (file)
@@ -68,6 +68,10 @@ private:
 
     Optional<SharedBufferDataView> calculateIncrementalDataChunk(const SharedBuffer*) const;
     void notifyClientsDataWasReceived(const char* data, unsigned length);
+    
+#if USE(QUICK_LOOK)
+    void previewResponseReceived(const ResourceResponse&) final;
+#endif
 
     unsigned long m_identifier;
     bool m_allowEncodedDataReplacement;
index 547c689..c80315e 100644 (file)
@@ -49,6 +49,10 @@ public:
     virtual void dataReceived(CachedResource&, const char* /* data */, int /* length */) { }
     virtual void redirectReceived(CachedResource&, ResourceRequest&& request, const ResourceResponse&, CompletionHandler<void(ResourceRequest&&)>&& completionHandler) { completionHandler(WTFMove(request)); }
     virtual void finishedTimingForWorkerLoad(CachedResource&, const ResourceTiming&) { }
+
+#if USE(QUICK_LOOK)
+    virtual void previewResponseReceived(CachedResource&, const ResourceResponse&) { };
+#endif
 };
 
 }
index 8722455..4b16f59 100644 (file)
@@ -919,4 +919,14 @@ void CachedResource::tryReplaceEncodedData(SharedBuffer& newBuffer)
 
 #endif
 
+#if USE(QUICK_LOOK)
+
+void CachedResource::previewResponseReceived(const ResourceResponse& response)
+{
+    ASSERT(response.url().protocolIs(QLPreviewProtocol));
+    CachedResource::responseReceived(response);
+}
+
+#endif
+
 }
index fc30b0f..7f8cb1f 100644 (file)
@@ -278,6 +278,10 @@ public:
     void setOriginalRequest(std::unique_ptr<ResourceRequest>&& originalRequest) { m_originalRequest = WTFMove(originalRequest); }
     const std::unique_ptr<ResourceRequest>& originalRequest() const { return m_originalRequest; }
 
+#if USE(QUICK_LOOK)
+    virtual void previewResponseReceived(const ResourceResponse&);
+#endif
+
 protected:
     // CachedResource constructor that may be used when the CachedResource can already be filled with response data.
     CachedResource(const URL&, Type, const PAL::SessionID&, const CookieJar*);
index c5ca5c2..2cd3559 100644 (file)
@@ -143,7 +143,7 @@ static PreviewLoaderClient& emptyClient()
 
     if (_shouldDecidePolicyBeforeLoading) {
         _hasProcessedResponse = YES;
-        _resourceLoader->documentLoader()->setResponse(response);
+        _resourceLoader->didReceivePreviewResponse(response);
         return;
     }
 
index 77e437f..39fc344 100644 (file)
@@ -1,3 +1,18 @@
+2019-10-14  Andy Estes  <aestes@apple.com>
+
+        REGRESSION (r243682): Quick Look previews loaded from the memory cache render with the wrong content type
+        https://bugs.webkit.org/show_bug.cgi?id=202935
+        <rdar://problem/54318133>
+
+        Reviewed by Tim Horton.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/QuickLook.mm:
+        (TEST):
+        * TestWebKitAPI/cocoa/TestWKWebView.h:
+        * TestWebKitAPI/cocoa/TestWKWebView.mm:
+        (-[WKWebView synchronouslyGoBack]):
+        (-[WKWebView synchronouslyGoForward]):
+
 2019-10-14  Alex Christensen  <achristensen@webkit.org>
 
         REGRESSION: [iOS 13?] TestWebKitAPI.SharedBufferTest.tryCreateArrayBufferLargeSegments is failing
index 3db52f3..a54ea78 100644 (file)
@@ -29,6 +29,8 @@
 
 #import "PlatformUtilities.h"
 #import "Test.h"
+#import "TestProtocol.h"
+#import "TestWKWebView.h"
 #import <CoreServices/CoreServices.h>
 #import <WebCore/WebCoreThread.h>
 #import <WebKit/WKNavigationDelegatePrivate.h>
@@ -478,4 +480,30 @@ TEST(QuickLook, LegacyQuickLookContent)
     EXPECT_NULL(mainFrame.dataSource._quickLookContent);
 }
 
+TEST(QuickLook, LoadFromMemoryCache)
+{
+    [TestProtocol registerWithScheme:@"http"];
+    TestProtocol.additionalResponseHeaders = @{ @"Cache-Control" : @"max-age=3600" };
+
+    auto webView = adoptNS([[TestWKWebView alloc] init]);
+    [webView synchronouslyLoadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"http://bundle-file/simple.html"]]];
+
+    NSString * const contentTypeJavaScript = @"document.contentType";
+    NSString * const simpleMIMEType = @"text/html";
+    EXPECT_WK_STREQ(simpleMIMEType, [webView stringByEvaluatingJavaScript:contentTypeJavaScript]);
+
+    NSURL * const pagesDocumentHTTPURL = [NSURL URLWithString:@"http://bundle-file/pages.pages"];
+    [webView synchronouslyLoadRequest:[NSURLRequest requestWithURL:pagesDocumentHTTPURL]];
+    EXPECT_WK_STREQ(pagesDocumentPreviewMIMEType, [webView stringByEvaluatingJavaScript:contentTypeJavaScript]);
+
+    [webView synchronouslyGoBack];
+    EXPECT_WK_STREQ(simpleMIMEType, [webView stringByEvaluatingJavaScript:contentTypeJavaScript]);
+
+    [webView synchronouslyLoadRequest:[NSURLRequest requestWithURL:pagesDocumentHTTPURL]];
+    EXPECT_WK_STREQ(pagesDocumentPreviewMIMEType, [webView stringByEvaluatingJavaScript:contentTypeJavaScript]);
+
+    TestProtocol.additionalResponseHeaders = nil;
+    [TestProtocol unregister];
+}
+
 #endif // PLATFORM(IOS_FAMILY)
index 24a5acd..ab25039 100644 (file)
@@ -50,6 +50,8 @@
 @interface WKWebView (TestWebKitAPI)
 @property (nonatomic, readonly) NSArray<NSString *> *tagsInBody;
 - (void)loadTestPageNamed:(NSString *)pageName;
+- (void)synchronouslyGoBack;
+- (void)synchronouslyGoForward;
 - (void)synchronouslyLoadHTMLString:(NSString *)html;
 - (void)synchronouslyLoadHTMLString:(NSString *)html baseURL:(NSURL *)url;
 - (void)synchronouslyLoadRequest:(NSURLRequest *)request;
index 651b5f2..fa3e9c0 100644 (file)
@@ -70,6 +70,18 @@ SOFT_LINK_CLASS(UIKit, UIWindow)
     [self loadRequest:request];
 }
 
+- (void)synchronouslyGoBack
+{
+    [self goBack];
+    [self _test_waitForDidFinishNavigation];
+}
+
+- (void)synchronouslyGoForward
+{
+    [self goForward];
+    [self _test_waitForDidFinishNavigation];
+}
+
 - (void)synchronouslyLoadRequest:(NSURLRequest *)request
 {
     [self loadRequest:request];