[WK2] Add C API to retrieve the originating page of a WKDownload
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Sep 2017 22:49:58 +0000 (22:49 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Sep 2017 22:49:58 +0000 (22:49 +0000)
https://bugs.webkit.org/show_bug.cgi?id=176557
<rdar://problem/34314776>

Reviewed by Alex Christensen.

Source/WebKit:

Add C API to retrieve the original page of a WKDownload. This is the C API equivalent to
_WKDownload.originatingWebView which already exists in ObjC. The pointer is weak so as
to not keep alive the page for the duration of the download.

* UIProcess/API/C/WKDownload.cpp:
(WKDownloadGetOriginatingPage):
* UIProcess/API/C/WKDownload.h:
* UIProcess/API/Cocoa/_WKDownload.mm:
(-[_WKDownload originatingWebView]):
* UIProcess/API/Cocoa/_WKDownloadInternal.h:
* UIProcess/Downloads/DownloadProxy.cpp:
(WebKit::DownloadProxy::originatingPage const):
(WebKit::DownloadProxy::setOriginatingPage):
* UIProcess/Downloads/DownloadProxy.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::receivedPolicyDecision):
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::download):
(WebKit::WebProcessPool::resumeDownload):
(WebKit::WebProcessPool::createDownloadProxy):
* UIProcess/WebProcessPool.h:
* UIProcess/ios/PageClientImplIOS.mm:
(WebKit::PageClientImpl::handleDownloadRequest):
* UIProcess/mac/PageClientImplMac.mm:
(WebKit::PageClientImpl::handleDownloadRequest):

Tools:

Add layout test coverage for both the ObjC and C API.

* TestWebKitAPI/Tests/WebKit/mac/ContextMenuDownload.mm:
(TestWebKitAPI::decideDestinationWithSuggestedFilename):
(TestWebKitAPI::TEST):
* TestWebKitAPI/Tests/WebKitCocoa/Download.mm:
(-[RedirectedDownloadDelegate _downloadDidStart:]):
(TEST):

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

16 files changed:
Source/WTF/wtf/WeakPtr.h
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/C/WKDownload.cpp
Source/WebKit/UIProcess/API/C/WKDownload.h
Source/WebKit/UIProcess/API/Cocoa/_WKDownload.mm
Source/WebKit/UIProcess/API/Cocoa/_WKDownloadInternal.h
Source/WebKit/UIProcess/Downloads/DownloadProxy.cpp
Source/WebKit/UIProcess/Downloads/DownloadProxy.h
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebProcessPool.cpp
Source/WebKit/UIProcess/WebProcessPool.h
Source/WebKit/UIProcess/ios/PageClientImplIOS.mm
Source/WebKit/UIProcess/mac/PageClientImplMac.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKit/mac/ContextMenuDownload.mm
Tools/TestWebKitAPI/Tests/WebKitCocoa/Download.mm

index f15160b..29cc056 100644 (file)
@@ -92,6 +92,7 @@ class WeakPtr {
     WTF_MAKE_FAST_ALLOCATED;
 public:
     WeakPtr() : m_ref(WeakReference<T>::create(nullptr)) { }
+    WeakPtr(std::nullptr_t) : m_ref(WeakReference<T>::create(nullptr)) { }
     WeakPtr(const WeakPtr& o) : m_ref(o.m_ref.copyRef()) { }
     template<typename U> WeakPtr(const WeakPtr<U>& o) : m_ref(o.m_ref.copyRef()) { }
 
index 0b582fa..5a62597 100644 (file)
@@ -1,3 +1,37 @@
+2017-09-07  Chris Dumez  <cdumez@apple.com>
+
+        [WK2] Add C API to retrieve the originating page of a WKDownload
+        https://bugs.webkit.org/show_bug.cgi?id=176557
+        <rdar://problem/34314776>
+
+        Reviewed by Alex Christensen.
+
+        Add C API to retrieve the original page of a WKDownload. This is the C API equivalent to
+        _WKDownload.originatingWebView which already exists in ObjC. The pointer is weak so as
+        to not keep alive the page for the duration of the download.
+
+        * UIProcess/API/C/WKDownload.cpp:
+        (WKDownloadGetOriginatingPage):
+        * UIProcess/API/C/WKDownload.h:
+        * UIProcess/API/Cocoa/_WKDownload.mm:
+        (-[_WKDownload originatingWebView]):
+        * UIProcess/API/Cocoa/_WKDownloadInternal.h:
+        * UIProcess/Downloads/DownloadProxy.cpp:
+        (WebKit::DownloadProxy::originatingPage const):
+        (WebKit::DownloadProxy::setOriginatingPage):
+        * UIProcess/Downloads/DownloadProxy.h:
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::receivedPolicyDecision):
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::download):
+        (WebKit::WebProcessPool::resumeDownload):
+        (WebKit::WebProcessPool::createDownloadProxy):
+        * UIProcess/WebProcessPool.h:
+        * UIProcess/ios/PageClientImplIOS.mm:
+        (WebKit::PageClientImpl::handleDownloadRequest):
+        * UIProcess/mac/PageClientImplMac.mm:
+        (WebKit::PageClientImpl::handleDownloadRequest):
+
 2017-09-07  Alex Christensen  <achristensen@webkit.org>
 
         Clean up Geolocation request handling
index abf93f1..9d5d6d9 100644 (file)
@@ -30,6 +30,7 @@
 #include "APIURLRequest.h"
 #include "DownloadProxy.h"
 #include "WKAPICast.h"
+#include "WebPageProxy.h"
 
 using namespace WebKit;
 
@@ -57,3 +58,8 @@ void WKDownloadCancel(WKDownloadRef download)
 {
     return toImpl(download)->cancel();
 }
+
+WKPageRef WKDownloadGetOriginatingPage(WKDownloadRef download)
+{
+    return toAPI(toImpl(download)->originatingPage());
+}
index 5dbea7e..d6da408 100644 (file)
@@ -42,6 +42,7 @@ WK_EXPORT uint64_t WKDownloadGetID(WKDownloadRef download);
 WK_EXPORT WKURLRequestRef WKDownloadCopyRequest(WKDownloadRef download);
 WK_EXPORT WKDataRef WKDownloadGetResumeData(WKDownloadRef download);
 WK_EXPORT void WKDownloadCancel(WKDownloadRef download);
+WK_EXPORT WKPageRef WKDownloadGetOriginatingPage(WKDownloadRef download);
 
 #ifdef __cplusplus
 }
index 06255e0..131c32f 100644 (file)
 #if WK_API_ENABLED
 
 #import "DownloadProxy.h"
+#import "WKWebViewInternal.h"
 #import "WeakObjCPtr.h"
 
 @implementation _WKDownload {
     API::ObjectStorage<WebKit::DownloadProxy> _download;
-    WebKit::WeakObjCPtr<WKWebView> _originatingWebView;
 }
 
 - (void)dealloc
 
 - (WKWebView *)originatingWebView
 {
-    return _originatingWebView.getAutoreleased();
-}
+    if (auto* originatingPage = _download->originatingPage())
+        return [[fromWebPageProxy(*originatingPage) retain] autorelease];
+    return nil;
 
-- (void)setOriginatingWebView:(WKWebView *)originatingWebView
-{
-    _originatingWebView = originatingWebView;
 }
 
 #pragma mark WKObject protocol implementation
index 2fb76c5..0a226c3 100644 (file)
@@ -30,7 +30,6 @@
 #import "WKObject.h"
 
 @interface _WKDownload () <WKObject>
-@property (nonatomic, weak) WKWebView *originatingWebView;
 @end
 
 #endif // WK_API_ENABLED
index 06a38de..c2618a2 100644 (file)
@@ -92,6 +92,16 @@ void DownloadProxy::processDidClose()
     m_processPool->downloadClient().processDidCrash(m_processPool.get(), this);
 }
 
+WebPageProxy* DownloadProxy::originatingPage() const
+{
+    return m_originatingPage.get();
+}
+
+void DownloadProxy::setOriginatingPage(WebPageProxy* page)
+{
+    m_originatingPage = page ? page->createWeakPtr() : nullptr;
+}
+
 void DownloadProxy::didStart(const ResourceRequest& request, const String& suggestedFilename)
 {
     m_request = request;
index ea16af7..49797f3 100644 (file)
@@ -33,6 +33,7 @@
 #include <WebCore/ResourceRequest.h>
 #include <wtf/Forward.h>
 #include <wtf/Ref.h>
+#include <wtf/WeakPtr.h>
 
 namespace API {
 class Data;
@@ -69,6 +70,9 @@ public:
     void didReceiveDownloadProxyMessage(IPC::Connection&, IPC::Decoder&);
     void didReceiveSyncDownloadProxyMessage(IPC::Connection&, IPC::Decoder&, std::unique_ptr<IPC::Encoder>&);
 
+    WebPageProxy* originatingPage() const;
+    void setOriginatingPage(WebPageProxy*);
+
 private:
     explicit DownloadProxy(DownloadProxyMap&, WebProcessPool&, const WebCore::ResourceRequest&);
 
@@ -103,6 +107,8 @@ private:
     RefPtr<API::Data> m_resumeData;
     WebCore::ResourceRequest m_request;
     String m_suggestedFilename;
+
+    WeakPtr<WebPageProxy> m_originatingPage;
 };
 
 } // namespace WebKit
index 087cf7d..d2977f8 100644 (file)
@@ -2277,7 +2277,7 @@ void WebPageProxy::receivedPolicyDecision(PolicyAction action, WebFrameProxy& fr
     DownloadID downloadID = { };
     if (action == PolicyDownload) {
         // Create a download proxy.
-        DownloadProxy* download = m_process->processPool().createDownloadProxy(m_decidePolicyForResponseRequest);
+        auto* download = m_process->processPool().createDownloadProxy(m_decidePolicyForResponseRequest, this);
         downloadID = download->downloadID();
         handleDownloadRequest(download);
         m_decidePolicyForResponseRequest = { };
index 8116536..1bf1fbc 100644 (file)
@@ -995,7 +995,7 @@ void WebProcessPool::pageRemovedFromProcess(WebPageProxy& page)
 
 DownloadProxy* WebProcessPool::download(WebPageProxy* initiatingPage, const ResourceRequest& request, const String& suggestedFilename)
 {
-    DownloadProxy* downloadProxy = createDownloadProxy(request);
+    auto* downloadProxy = createDownloadProxy(request, initiatingPage);
     PAL::SessionID sessionID = initiatingPage ? initiatingPage->sessionID() : PAL::SessionID::defaultSessionID();
 
     if (initiatingPage)
@@ -1018,7 +1018,7 @@ DownloadProxy* WebProcessPool::download(WebPageProxy* initiatingPage, const Reso
 
 DownloadProxy* WebProcessPool::resumeDownload(const API::Data* resumeData, const String& path)
 {
-    DownloadProxy* downloadProxy = createDownloadProxy(ResourceRequest());
+    auto* downloadProxy = createDownloadProxy(ResourceRequest(), nullptr);
 
     SandboxExtension::Handle sandboxExtensionHandle;
     if (!path.isEmpty())
@@ -1206,9 +1206,11 @@ void WebProcessPool::setDefaultRequestTimeoutInterval(double timeoutInterval)
     sendToAllProcesses(Messages::WebProcess::SetDefaultRequestTimeoutInterval(timeoutInterval));
 }
 
-DownloadProxy* WebProcessPool::createDownloadProxy(const ResourceRequest& request)
+DownloadProxy* WebProcessPool::createDownloadProxy(const ResourceRequest& request, WebPageProxy* originatingPage)
 {
-    return ensureNetworkProcess().createDownloadProxy(request);
+    auto downloadProxy = ensureNetworkProcess().createDownloadProxy(request);
+    downloadProxy->setOriginatingPage(originatingPage);
+    return downloadProxy;
 }
 
 void WebProcessPool::addMessageReceiver(IPC::StringReference messageReceiverName, IPC::MessageReceiver& messageReceiver)
index 45cb88f..c3cb985 100644 (file)
@@ -232,7 +232,7 @@ public:
     void setEnhancedAccessibility(bool);
     
     // Downloads.
-    DownloadProxy* createDownloadProxy(const WebCore::ResourceRequest&);
+    DownloadProxy* createDownloadProxy(const WebCore::ResourceRequest&, WebPageProxy* originatingPage);
     API::DownloadClient& downloadClient() { return *m_downloadClient; }
 
     API::LegacyContextHistoryClient& historyClient() { return *m_historyClient; }
index dad7247..d8cf367 100644 (file)
@@ -260,11 +260,8 @@ void PageClientImpl::didCommitLoadForMainFrame(const String& mimeType, bool useC
     [m_contentView _didCommitLoadForMainFrame];
 }
 
-void PageClientImpl::handleDownloadRequest(DownloadProxy* download)
+void PageClientImpl::handleDownloadRequest(DownloadProxy*)
 {
-    ASSERT_ARG(download, download);
-    ASSERT([download->wrapper() isKindOfClass:[_WKDownload class]]);
-    [static_cast<_WKDownload *>(download->wrapper()) setOriginatingWebView:m_webView];
 }
 
 void PageClientImpl::didChangeContentSize(const WebCore::IntSize&)
index 7d7a1e1..66101da 100644 (file)
@@ -273,13 +273,8 @@ void PageClientImpl::didFinishLoadingDataForCustomContentProvider(const String&
 {
 }
 
-void PageClientImpl::handleDownloadRequest(DownloadProxy* download)
+void PageClientImpl::handleDownloadRequest(DownloadProxy*)
 {
-    ASSERT_ARG(download, download);
-#if WK_API_ENABLED
-    ASSERT([download->wrapper() isKindOfClass:[_WKDownload class]]);
-    [static_cast<_WKDownload *>(download->wrapper()) setOriginatingWebView:m_webView];
-#endif
 }
 
 void PageClientImpl::didChangeContentSize(const WebCore::IntSize& newSize)
index feffb3f..3af7893 100644 (file)
@@ -1,3 +1,20 @@
+2017-09-07  Chris Dumez  <cdumez@apple.com>
+
+        [WK2] Add C API to retrieve the originating page of a WKDownload
+        https://bugs.webkit.org/show_bug.cgi?id=176557
+        <rdar://problem/34314776>
+
+        Reviewed by Alex Christensen.
+
+        Add layout test coverage for both the ObjC and C API.
+
+        * TestWebKitAPI/Tests/WebKit/mac/ContextMenuDownload.mm:
+        (TestWebKitAPI::decideDestinationWithSuggestedFilename):
+        (TestWebKitAPI::TEST):
+        * TestWebKitAPI/Tests/WebKitCocoa/Download.mm:
+        (-[RedirectedDownloadDelegate _downloadDidStart:]):
+        (TEST):
+
 2017-09-07  Filip Pizlo  <fpizlo@apple.com>
 
         WSL Node.prototype.visit should probably do memoization
index cfbc11f..5dc5473 100644 (file)
 #include <WebKit/WKPageContextMenuClient.h>
 #include <WebKit/WKPreferencesPrivate.h>
 #include <WebKit/WKRetainPtr.h>
+#include <WebKit/_WKDownload.h>
 
 namespace TestWebKitAPI {
 
 static bool didFinishLoad;
 static bool didDecideDownloadDestination;
+static WKPageRef expectedOriginatingPage;
 
 static void didFinishLoadForFrame(WKPageRef, WKFrameRef, WKTypeRef, const void*)
 {
@@ -68,6 +70,8 @@ static WKStringRef decideDestinationWithSuggestedFilename(WKContextRef, WKDownlo
     WKDownloadCancel(download);
     didDecideDownloadDestination = true;
 
+    EXPECT_EQ(expectedOriginatingPage, WKDownloadGetOriginatingPage(download));
+
     return Util::toWK("/tmp/WebKitAPITest/ContextMenuDownload").leakRef();
 }
 
@@ -100,6 +104,7 @@ TEST(WebKit, ContextMenuDownloadHTMLDownloadAttribute)
 
     WKRetainPtr<WKURLRef> url(AdoptWK, Util::createURLForResource("link-with-download-attribute", "html"));
 
+    expectedOriginatingPage = webView.page();
     WKPageLoadURL(webView.page(), url.get());
     Util::run(&didFinishLoad);
 
@@ -146,6 +151,7 @@ TEST(WebKit, ContextMenuDownloadHTMLDownloadAttributeWithSlashes)
 
     WKRetainPtr<WKURLRef> url(AdoptWK, Util::createURLForResource("link-with-download-attribute-with-slashes", "html"));
 
+    expectedOriginatingPage = webView.page();
     WKPageLoadURL(webView.page(), url.get());
     Util::run(&didFinishLoad);
 
index 4762069..0867998 100644 (file)
@@ -49,6 +49,7 @@ static bool isDone;
 static bool hasReceivedRedirect;
 static bool hasReceivedResponse;
 static NSURL *sourceURL = [[NSBundle mainBundle] URLForResource:@"simple" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
+static WKWebView* expectedOriginatingWebView;
 
 @interface DownloadDelegate : NSObject <_WKDownloadDelegate>
 @end
@@ -449,6 +450,12 @@ TEST(_WKDownload, DownloadRequestBlobURL)
     String _destinationPath;
 }
 
+- (void)_downloadDidStart:(_WKDownload *)download
+{
+    EXPECT_NOT_NULL(download);
+    EXPECT_EQ(expectedOriginatingWebView, download.originatingWebView);
+}
+
 - (NSString *)_download:(_WKDownload *)download decideDestinationWithSuggestedFilename:(NSString *)filename allowOverwrite:(BOOL *)allowOverwrite
 {
     WebCore::PlatformFileHandle fileHandle;
@@ -492,6 +499,7 @@ TEST(_WKDownload, RedirectedDownload)
 
     [webView synchronouslyLoadHTMLString:@"<a style='display: block; height: 100%; width: 100%' href='http://redirect/?pass'>test</a>"];
 
+    expectedOriginatingWebView = webView.get();
     NSPoint clickPoint = NSMakePoint(100, 100);
     [[webView hitTest:clickPoint] mouseDown:[NSEvent mouseEventWithType:NSEventTypeRightMouseDown location:clickPoint modifierFlags:0 timestamp:0 windowNumber:[window windowNumber] context:nil eventNumber:0 clickCount:1 pressure:1]];
     [[webView hitTest:clickPoint] mouseUp:[NSEvent mouseEventWithType:NSEventTypeRightMouseUp location:clickPoint modifierFlags:0 timestamp:0 windowNumber:[window windowNumber] context:nil eventNumber:0 clickCount:1 pressure:1]];