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.
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
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 0e97f72..553cc0b 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -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
diff --git a/Source/WebCore/loader/DocumentLoader.cpp b/Source/WebCore/loader/DocumentLoader.cpp
index 6a63023..fba266a 100644
--- a/Source/WebCore/loader/DocumentLoader.cpp
+++ b/Source/WebCore/loader/DocumentLoader.cpp
@@ -2135,6 +2135,12 @@
#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);
diff --git a/Source/WebCore/loader/DocumentLoader.h b/Source/WebCore/loader/DocumentLoader.h
index a85a076..dffed40 100644
--- a/Source/WebCore/loader/DocumentLoader.h
+++ b/Source/WebCore/loader/DocumentLoader.h
@@ -432,6 +432,9 @@
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);
diff --git a/Source/WebCore/loader/ResourceLoader.h b/Source/WebCore/loader/ResourceLoader.h
index 5c5c8df..72f4c7a 100644
--- a/Source/WebCore/loader/ResourceLoader.h
+++ b/Source/WebCore/loader/ResourceLoader.h
@@ -117,6 +117,7 @@
#if USE(QUICK_LOOK)
bool isQuickLookResource() const;
+ virtual void didReceivePreviewResponse(const ResourceResponse&) { };
#endif
const URL& url() const { return m_request.url(); }
diff --git a/Source/WebCore/loader/SubresourceLoader.cpp b/Source/WebCore/loader/SubresourceLoader.cpp
index a5be875..8e30b75 100644
--- a/Source/WebCore/loader/SubresourceLoader.cpp
+++ b/Source/WebCore/loader/SubresourceLoader.cpp
@@ -314,6 +314,15 @@
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)
diff --git a/Source/WebCore/loader/SubresourceLoader.h b/Source/WebCore/loader/SubresourceLoader.h
index dec8d0b..9ecb782 100644
--- a/Source/WebCore/loader/SubresourceLoader.h
+++ b/Source/WebCore/loader/SubresourceLoader.h
@@ -100,6 +100,7 @@
#if USE(QUICK_LOOK)
bool shouldCreatePreviewLoaderForResponse(const ResourceResponse&) const;
+ void didReceivePreviewResponse(const ResourceResponse&) override;
#endif
enum SubresourceLoaderState {
diff --git a/Source/WebCore/loader/cache/CachedRawResource.cpp b/Source/WebCore/loader/cache/CachedRawResource.cpp
index f9a75f3..e7dc633 100644
--- a/Source/WebCore/loader/cache/CachedRawResource.cpp
+++ b/Source/WebCore/loader/cache/CachedRawResource.cpp
@@ -341,4 +341,16 @@
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
diff --git a/Source/WebCore/loader/cache/CachedRawResource.h b/Source/WebCore/loader/cache/CachedRawResource.h
index 6376136..27cddd3 100644
--- a/Source/WebCore/loader/cache/CachedRawResource.h
+++ b/Source/WebCore/loader/cache/CachedRawResource.h
@@ -68,6 +68,10 @@
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;
diff --git a/Source/WebCore/loader/cache/CachedRawResourceClient.h b/Source/WebCore/loader/cache/CachedRawResourceClient.h
index 547c689..c80315e 100644
--- a/Source/WebCore/loader/cache/CachedRawResourceClient.h
+++ b/Source/WebCore/loader/cache/CachedRawResourceClient.h
@@ -49,6 +49,10 @@
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
};
}
diff --git a/Source/WebCore/loader/cache/CachedResource.cpp b/Source/WebCore/loader/cache/CachedResource.cpp
index 8722455..4b16f59 100644
--- a/Source/WebCore/loader/cache/CachedResource.cpp
+++ b/Source/WebCore/loader/cache/CachedResource.cpp
@@ -919,4 +919,14 @@
#endif
+#if USE(QUICK_LOOK)
+
+void CachedResource::previewResponseReceived(const ResourceResponse& response)
+{
+ ASSERT(response.url().protocolIs(QLPreviewProtocol));
+ CachedResource::responseReceived(response);
+}
+
+#endif
+
}
diff --git a/Source/WebCore/loader/cache/CachedResource.h b/Source/WebCore/loader/cache/CachedResource.h
index fc30b0f..7f8cb1f 100644
--- a/Source/WebCore/loader/cache/CachedResource.h
+++ b/Source/WebCore/loader/cache/CachedResource.h
@@ -278,6 +278,10 @@
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*);
diff --git a/Source/WebCore/loader/ios/PreviewLoader.mm b/Source/WebCore/loader/ios/PreviewLoader.mm
index c5ca5c2..2cd3559 100644
--- a/Source/WebCore/loader/ios/PreviewLoader.mm
+++ b/Source/WebCore/loader/ios/PreviewLoader.mm
@@ -143,7 +143,7 @@
if (_shouldDecidePolicyBeforeLoading) {
_hasProcessedResponse = YES;
- _resourceLoader->documentLoader()->setResponse(response);
+ _resourceLoader->didReceivePreviewResponse(response);
return;
}
diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index 77e437f..39fc344 100644
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -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
diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/QuickLook.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/QuickLook.mm
index 3db52f3..a54ea78 100644
--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/QuickLook.mm
+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/QuickLook.mm
@@ -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 @@
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)
diff --git a/Tools/TestWebKitAPI/cocoa/TestWKWebView.h b/Tools/TestWebKitAPI/cocoa/TestWKWebView.h
index 24a5acd..ab25039 100644
--- a/Tools/TestWebKitAPI/cocoa/TestWKWebView.h
+++ b/Tools/TestWebKitAPI/cocoa/TestWKWebView.h
@@ -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;
diff --git a/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm b/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm
index 651b5f2..fa3e9c0 100644
--- a/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm
+++ b/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm
@@ -70,6 +70,18 @@
[self loadRequest:request];
}
+- (void)synchronouslyGoBack
+{
+ [self goBack];
+ [self _test_waitForDidFinishNavigation];
+}
+
+- (void)synchronouslyGoForward
+{
+ [self goForward];
+ [self _test_waitForDidFinishNavigation];
+}
+
- (void)synchronouslyLoadRequest:(NSURLRequest *)request
{
[self loadRequest:request];
