[QuickLook] FrameLoaderClient should return the new QuickLookHandleCl…

…ient it creates

https://bugs.webkit.org/show_bug.cgi?id=167625

Reviewed by Andreas Kling.

Source/WebCore:

QuickLookHandleClients were being created by QuickLookHandle calling
FrameLoaderClient::didCreateQuickLookHandle() then having the FrameLoaderClient create a new
QuickLookHandleClient and set it using QuickLookHandle::setClient().

Since all FrameLoaderClient::didCreateQuickLookHandle() does is create a new
QuickLookHandleClient, this patch changes the FrameLoaderClient function to return the new
QuickLookHandleClient directly and removes the API from QuickLookHandle to set a client.
This also removes previewFileName() and previewUTI() from QuickLookHandle and instead passes
these as arguments to the FrameLoaderClient.

No change in behavior. Covered by existing tests.

* loader/EmptyClients.cpp: Added an implementation of createQuickLookHandleClient() that
returns nullptr.
* loader/FrameLoaderClient.h: Declared createQuickLookHandleClient() and removed
didCreateQuickLookHandle().
* loader/ios/QuickLook.h: Removed setClient(), previewFileName(), and previewUTI().
* loader/ios/QuickLook.mm: Removed testingOrEmptyClient().
(-[WebPreviewLoader initWithResourceLoader:resourceResponse:quickLookHandle:]): Set _client
to testingClient() if it exists, otherwise set it to the client returned by
FrameLoaderClient::createQuickLookHandleClient() it non-null, otherwise set it to
emptyClient().
(testingOrEmptyClient): Deleted.
(-[WebPreviewLoader setClient:]): Deleted.
(-[WebPreviewLoader converter]): Deleted.
(WebCore::QuickLookHandle::setClient): Deleted.
(WebCore::QuickLookHandle::previewFileName): Deleted.
(WebCore::QuickLookHandle::previewUTI): Deleted.

Source/WebKit/mac:

* WebCoreSupport/WebFrameLoaderClient.h: Declared createQuickLookHandleClient().
* WebCoreSupport/WebFrameLoaderClient.mm:
(WebFrameLoaderClient::createQuickLookHandleClient): Renamed from didCreateQuickLookHandle().
(WebFrameLoaderClient::didCreateQuickLookHandle): Renamed to createQuickLookHandleClient().

Source/WebKit2:

* WebProcess/WebCoreSupport/WebFrameLoaderClient.h: Declared createQuickLookHandleClient().
* WebProcess/WebCoreSupport/ios/WebFrameLoaderClientIOS.mm:
(WebKit::WebFrameLoaderClient::createQuickLookHandleClient): Renamed from didCreateQuickLookHandle().
(WebKit::WebFrameLoaderClient::didCreateQuickLookHandle): Renamed to createQuickLookHandleClient().
* WebProcess/WebCoreSupport/ios/WebQuickLookHandleClient.cpp:
(WebKit::WebQuickLookHandleClient::WebQuickLookHandleClient): Set m_fileName and m_uti from
the constructor arguments instead of from handle.
* WebProcess/WebCoreSupport/ios/WebQuickLookHandleClient.h:


Canonical link: https://commits.webkit.org/184642@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@211402 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog

@@ -1,3 +1,39 @@
+2017-01-30  Andy Estes  <aestes@apple.com>
+
+        [QuickLook] FrameLoaderClient should return the new QuickLookHandleClient it creates
+        https://bugs.webkit.org/show_bug.cgi?id=167625
+
+        Reviewed by Andreas Kling.
+
+        QuickLookHandleClients were being created by QuickLookHandle calling
+        FrameLoaderClient::didCreateQuickLookHandle() then having the FrameLoaderClient create a new
+        QuickLookHandleClient and set it using QuickLookHandle::setClient().
+
+        Since all FrameLoaderClient::didCreateQuickLookHandle() does is create a new
+        QuickLookHandleClient, this patch changes the FrameLoaderClient function to return the new
+        QuickLookHandleClient directly and removes the API from QuickLookHandle to set a client.
+        This also removes previewFileName() and previewUTI() from QuickLookHandle and instead passes
+        these as arguments to the FrameLoaderClient.
+
+        No change in behavior. Covered by existing tests.
+
+        * loader/EmptyClients.cpp: Added an implementation of createQuickLookHandleClient() that
+        returns nullptr.
+        * loader/FrameLoaderClient.h: Declared createQuickLookHandleClient() and removed
+        didCreateQuickLookHandle().
+        * loader/ios/QuickLook.h: Removed setClient(), previewFileName(), and previewUTI().
+        * loader/ios/QuickLook.mm: Removed testingOrEmptyClient().
+        (-[WebPreviewLoader initWithResourceLoader:resourceResponse:quickLookHandle:]): Set _client
+        to testingClient() if it exists, otherwise set it to the client returned by
+        FrameLoaderClient::createQuickLookHandleClient() it non-null, otherwise set it to
+        emptyClient().
+        (testingOrEmptyClient): Deleted.
+        (-[WebPreviewLoader setClient:]): Deleted.
+        (-[WebPreviewLoader converter]): Deleted.
+        (WebCore::QuickLookHandle::setClient): Deleted.
+        (WebCore::QuickLookHandle::previewFileName): Deleted.
+        (WebCore::QuickLookHandle::previewUTI): Deleted.
+
 2017-01-30  Jer Noble  <jer.noble@apple.com>
 
         NULL-deref crash in TextTrack::removeCue()

Source/WebCore/loader/EmptyClients.cpp

@@ -67,6 +67,10 @@
 #include "CompiledContentExtension.h"
 #endif
 
+#if USE(QUICK_LOOK)
+#include "QuickLookHandleClient.h"
+#endif
+
 namespace WebCore {
 
 class UserMessageHandlerDescriptor;
@@ -441,6 +445,10 @@ class EmptyFrameLoaderClient final : public FrameLoaderClient {
 
     bool isEmptyFrameLoaderClient() final { return true; }
     void prefetchDNS(const String&) final { }
+
+#if USE(QUICK_LOOK)
+    RefPtr<QuickLookHandleClient> createQuickLookHandleClient(const String&, const String&) final { return nullptr; }
+#endif
 };
 
 class EmptyFrameNetworkingContext final : public FrameNetworkingContext {

Source/WebCore/loader/FrameLoaderClient.h

@@ -84,7 +84,7 @@ class Page;
 class PluginViewBase;
 class PolicyChecker;
 class ProtectionSpace;
-class QuickLookHandle;
+class QuickLookHandleClient;
 class RTCPeerConnectionHandler;
 class ResourceError;
 class ResourceHandle;
@@ -340,7 +340,7 @@ class WEBCORE_EXPORT FrameLoaderClient {
     virtual bool isEmptyFrameLoaderClient() { return false; }
 
 #if USE(QUICK_LOOK)
-    virtual void didCreateQuickLookHandle(QuickLookHandle&) { }
+    virtual RefPtr<QuickLookHandleClient> createQuickLookHandleClient(const String&, const String&) = 0;
 #endif
 
 #if ENABLE(CONTENT_FILTERING)

Source/WebCore/loader/ios/QuickLook.h

@@ -63,12 +63,8 @@ class QuickLookHandle {
     bool didFinishLoading();
     void didFail();
 
-    WEBCORE_EXPORT void setClient(Ref<QuickLookHandleClient>&&);
     WEBCORE_EXPORT static void setClientForTesting(RefPtr<QuickLookHandleClient>&&);
 
-    WEBCORE_EXPORT String previewFileName() const;
-    WEBCORE_EXPORT String previewUTI() const;
-
 private:
     friend std::unique_ptr<QuickLookHandle> std::make_unique<QuickLookHandle>(ResourceLoader&, const ResourceResponse&);
     QuickLookHandle(ResourceLoader&, const ResourceResponse&);

Source/WebCore/loader/ios/QuickLook.mm

@@ -134,13 +134,6 @@ static void addQLPreviewConverterWithFileForURL(NSURL *url, id converter, NSStri
     return emptyClient.get();
 }
 
-static QuickLookHandleClient& testingOrEmptyClient()
-{
-    if (testingClient())
-        return *testingClient();
-    return emptyClient();
-}
-
 @interface WebPreviewLoader : NSObject {
     RefPtr<ResourceLoader> _resourceLoader;
     ResourceResponse _response;
@@ -152,13 +145,10 @@ @interface WebPreviewLoader : NSObject {
 }
 
 - (instancetype)initWithResourceLoader:(ResourceLoader&)resourceLoader resourceResponse:(const ResourceResponse&)resourceResponse quickLookHandle:(QuickLookHandle&)quickLookHandle;
-- (void)setClient:(Ref<QuickLookHandleClient>&&)client;
 - (void)appendDataArray:(NSArray<NSData *> *)dataArray;
 - (void)finishedAppending;
 - (void)failed;
 
-@property (nonatomic, readonly) PreviewConverter* converter;
-
 @end
 
 @implementation WebPreviewLoader
@@ -172,20 +162,20 @@ - (instancetype)initWithResourceLoader:(ResourceLoader&)resourceLoader resourceR
     _resourceLoader = &resourceLoader;
     _response = resourceResponse;
     _handle = &quickLookHandle;
-    _client = &testingOrEmptyClient();
     _converter = std::make_unique<PreviewConverter>(self, _response);
     _bufferedDataArray = adoptNS([[NSMutableArray alloc] init]);
 
+    if (testingClient())
+        _client = testingClient();
+    else if (auto client = resourceLoader.frameLoader()->client().createQuickLookHandleClient(_converter->previewFileName(), _converter->previewUTI()))
+        _client = WTFMove(client);
+    else
+        _client = &emptyClient();
+
     LOG(Network, "WebPreviewConverter created with preview file name \"%s\".", _converter->previewFileName().utf8().data());
     return self;
 }
 
-- (void)setClient:(Ref<QuickLookHandleClient>&&)client
-{
-    if (!testingClient())
-        _client = WTFMove(client);
-}
-
 - (void)appendDataArray:(NSArray<NSData *> *)dataArray
 {
     LOG(Network, "WebPreviewConverter appending data array with count %ld.", dataArray.count);
@@ -208,11 +198,6 @@ - (void)failed
     _client->didFail();
 }
 
-- (PreviewConverter*)converter
-{
-    return _converter.get();
-}
-
 - (void)_sendDidReceiveResponseIfNecessary
 {
     if (_hasSentDidReceiveResponse)
@@ -326,7 +311,6 @@ - (void)connection:(NSURLConnection *)connection didFailWithError:(NSError *)err
 QuickLookHandle::QuickLookHandle(ResourceLoader& loader, const ResourceResponse& response)
     : m_previewLoader { adoptNS([[WebPreviewLoader alloc] initWithResourceLoader:loader resourceResponse:response quickLookHandle:*this]) }
 {
-    loader.frameLoader()->client().didCreateQuickLookHandle(*this);
 }
 
 QuickLookHandle::~QuickLookHandle()
@@ -378,26 +362,11 @@ - (void)connection:(NSURLConnection *)connection didFailWithError:(NSError *)err
     m_previewLoader = nullptr;
 }
 
-void QuickLookHandle::setClient(Ref<QuickLookHandleClient>&& client)
-{
-    [m_previewLoader setClient:WTFMove(client)];
-}
-
 void QuickLookHandle::setClientForTesting(RefPtr<QuickLookHandleClient>&& client)
 {
     testingClient() = WTFMove(client);
 }
 
-String QuickLookHandle::previewFileName() const
-{
-    return [m_previewLoader converter]->previewFileName();
-}
-
-String QuickLookHandle::previewUTI() const
-{
-    return [m_previewLoader converter]->previewUTI();
-}
-
 }
 
 #endif // USE(QUICK_LOOK)

Source/WebKit/mac/ChangeLog

@@ -1,3 +1,15 @@
+2017-01-30  Andy Estes  <aestes@apple.com>
+
+        [QuickLook] FrameLoaderClient should return the new QuickLookHandleClient it creates
+        https://bugs.webkit.org/show_bug.cgi?id=167625
+
+        Reviewed by Andreas Kling.
+
+        * WebCoreSupport/WebFrameLoaderClient.h: Declared createQuickLookHandleClient().
+        * WebCoreSupport/WebFrameLoaderClient.mm:
+        (WebFrameLoaderClient::createQuickLookHandleClient): Renamed from didCreateQuickLookHandle().
+        (WebFrameLoaderClient::didCreateQuickLookHandle): Renamed to createQuickLookHandleClient().
+
 2017-01-30  Chris Dumez  <cdumez@apple.com>
 
         Drop legacy Attributes.isId attribute

Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.h

@@ -246,7 +246,7 @@ class WebFrameLoaderClient : public WebCore::FrameLoaderClient {
     bool shouldPaintBrokenImage(const WebCore::URL&) const final;
 
 #if USE(QUICK_LOOK)
-    void didCreateQuickLookHandle(WebCore::QuickLookHandle&) final;
+    RefPtr<WebCore::QuickLookHandleClient> createQuickLookHandleClient(const String& fileName, const String& uti) final;
 #endif
 
 #if ENABLE(CONTENT_FILTERING)

Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm

@@ -2225,7 +2225,7 @@ static bool shouldBlockWebGL()
 #endif
 
 #if USE(QUICK_LOOK)
-void WebFrameLoaderClient::didCreateQuickLookHandle(WebCore::QuickLookHandle& handle)
+RefPtr<QuickLookHandleClient> WebFrameLoaderClient::createQuickLookHandleClient(const String& fileName, const String& uti)
 {
     class QuickLookDocumentWriter : public WebCore::QuickLookHandleClient {
     public:
@@ -2263,14 +2263,14 @@ void didFail() override
     };
 
     if (![m_webFrame webView].preferences.quickLookDocumentSavingEnabled)
-        return;
+        return nullptr;
 
-    NSString *filePath = createTemporaryFileForQuickLook(handle.previewFileName());
+    NSString *filePath = createTemporaryFileForQuickLook(fileName);
     if (!filePath)
-        return;
+        return nullptr;
 
-    [m_webFrame provisionalDataSource]._quickLookContent = @{ WebQuickLookFileNameKey : filePath, WebQuickLookUTIKey : handle.previewUTI() };
-    handle.setClient(adoptRef(*new QuickLookDocumentWriter(filePath)));
+    [m_webFrame provisionalDataSource]._quickLookContent = @{ WebQuickLookFileNameKey : filePath, WebQuickLookUTIKey : uti };
+    return adoptRef(*new QuickLookDocumentWriter(filePath));
 }
 #endif
 

Source/WebKit2/ChangeLog

@@ -1,3 +1,19 @@
+2017-01-30  Andy Estes  <aestes@apple.com>
+
+        [QuickLook] FrameLoaderClient should return the new QuickLookHandleClient it creates
+        https://bugs.webkit.org/show_bug.cgi?id=167625
+
+        Reviewed by Andreas Kling.
+
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.h: Declared createQuickLookHandleClient().
+        * WebProcess/WebCoreSupport/ios/WebFrameLoaderClientIOS.mm:
+        (WebKit::WebFrameLoaderClient::createQuickLookHandleClient): Renamed from didCreateQuickLookHandle().
+        (WebKit::WebFrameLoaderClient::didCreateQuickLookHandle): Renamed to createQuickLookHandleClient().
+        * WebProcess/WebCoreSupport/ios/WebQuickLookHandleClient.cpp:
+        (WebKit::WebQuickLookHandleClient::WebQuickLookHandleClient): Set m_fileName and m_uti from
+        the constructor arguments instead of from handle.
+        * WebProcess/WebCoreSupport/ios/WebQuickLookHandleClient.h:
+
 2017-01-30  Anders Carlsson  <andersca@apple.com>
 
         Add some more crash reporter information to diagnose a failed mach_msg

Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.h

@@ -245,7 +245,7 @@ class WebFrameLoaderClient final : public WebCore::FrameLoaderClient {
     void forcePageTransitionIfNeeded() final;
 
 #if USE(QUICK_LOOK)
-    void didCreateQuickLookHandle(WebCore::QuickLookHandle&) final;
+    RefPtr<WebCore::QuickLookHandleClient> createQuickLookHandleClient(const String& fileName, const String& uti) final;
 #endif
 
 #if ENABLE(CONTENT_FILTERING)

Source/WebKit2/WebProcess/WebCoreSupport/ios/WebFrameLoaderClientIOS.mm

@@ -66,16 +66,16 @@
 }
 
 #if USE(QUICK_LOOK)
-void WebFrameLoaderClient::didCreateQuickLookHandle(WebCore::QuickLookHandle& handle)
+RefPtr<QuickLookHandleClient> WebFrameLoaderClient::createQuickLookHandleClient(const String& fileName, const String& uti)
 {
     if (!m_frame->isMainFrame())
-        return;
+        return nullptr;
 
     WebPage* webPage = m_frame->page();
     if (!webPage)
-        return;
+        return nullptr;
 
-    handle.setClient(WebQuickLookHandleClient::create(handle, webPage->pageID()));
+    return WebQuickLookHandleClient::create(fileName, uti, webPage->pageID());
 }
 #endif
 

Source/WebKit2/WebProcess/WebCoreSupport/ios/WebQuickLookHandleClient.cpp

@@ -44,10 +44,10 @@ static PasswordCallbackMap& passwordCallbacks()
     return callbackMap.get();
 }
 
-WebQuickLookHandleClient::WebQuickLookHandleClient(const WebCore::QuickLookHandle& handle, uint64_t pageID)
-    : m_fileName(handle.previewFileName())
-    , m_uti(handle.previewUTI())
-    , m_pageID(pageID)
+WebQuickLookHandleClient::WebQuickLookHandleClient(const String& fileName, const String& uti, uint64_t pageID)
+    : m_fileName { fileName }
+    , m_uti { uti }
+    , m_pageID { pageID }
 {
     WebProcess::singleton().send(Messages::WebPageProxy::DidStartLoadForQuickLookDocumentInMainFrame(m_fileName, m_uti), m_pageID);
 }

Source/WebKit2/WebProcess/WebCoreSupport/ios/WebQuickLookHandleClient.h

@@ -41,16 +41,16 @@ class WebFrame;
 
 class WebQuickLookHandleClient final : public WebCore::QuickLookHandleClient {
 public:
-    static Ref<WebQuickLookHandleClient> create(const WebCore::QuickLookHandle& handle, uint64_t pageID)
+    static Ref<WebQuickLookHandleClient> create(const String& fileName, const String& uti, uint64_t pageID)
     {
-        return adoptRef(*new WebQuickLookHandleClient(handle, pageID));
+        return adoptRef(*new WebQuickLookHandleClient(fileName, uti, pageID));
     }
     ~WebQuickLookHandleClient();
 
     static void didReceivePassword(const String&, uint64_t pageID);
 
 private:
-    WebQuickLookHandleClient(const WebCore::QuickLookHandle&, uint64_t pageID);
+    WebQuickLookHandleClient(const String& fileName, const String& uti, uint64_t pageID);
     void didReceiveDataArray(CFArrayRef) override;
     void didFinishLoading() override;
     void didFail() override;