Changeset 206355 in webkit

Timestamp:

Sep 24, 2016 6:00:02 PM (8 years ago)

Author:

Chris Dumez

Message:

[WK2] anchor.download attribute value is ignored by NETWORK_SESSION code path
​https://bugs.webkit.org/show_bug.cgi?id=162531

Reviewed by Darin Adler.

Source/WebKit2:

Pipe the suggested filename through in the NETWORK_SESSION code path so
that it is used when deciding the destination path.

• NetworkProcess/Downloads/DownloadManager.cpp:

(WebKit::DownloadManager::startDownload):

• NetworkProcess/Downloads/PendingDownload.cpp:

(WebKit::PendingDownload::PendingDownload):

• NetworkProcess/Downloads/PendingDownload.h:
• NetworkProcess/NetworkDataTask.h:
• NetworkProcess/NetworkLoad.cpp:

(WebKit::NetworkLoad::setSuggestedFilename):

• NetworkProcess/NetworkLoad.h:
• NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:

(WebKit::NetworkDataTask::suggestedFilename):
(WebKit::NetworkDataTask::setSuggestedFilename):

• NetworkProcess/cocoa/NetworkSessionCocoa.mm:

(-[WKNetworkSessionDelegate URLSession:dataTask:didBecomeDownloadTask:]):

Tools:

Update decideDestinationWithSuggestedFilename() to return a actual file path
instead of nullptr. The NETWORK_SESSION code path cancels the download when
decideDestinationWithSuggestedFilename() returns an empty file path, unlike
the !NETWORK_SESSION code path that just continue downloading without
explicitely setting a destination path. This lead to different test results
for NETWORK_SESSION and !NETWORK_SESSION code paths. Returns a valid
destination path makes more sense and leads to consistent results.

• WebKitTestRunner/TestController.cpp:

(WTR::TestController::decideDestinationWithSuggestedFilename):

LayoutTests:

Unskip tests that are now passing.

• platform/mac/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/mac/TestExpectations (modified) (1 diff)
• Source/WebKit2/ChangeLog (modified) (1 diff)
• Source/WebKit2/NetworkProcess/Downloads/DownloadManager.cpp (modified) (1 diff)
• Source/WebKit2/NetworkProcess/Downloads/PendingDownload.cpp (modified) (1 diff)
• Source/WebKit2/NetworkProcess/Downloads/PendingDownload.h (modified) (1 diff)
• Source/WebKit2/NetworkProcess/NetworkDataTask.h (modified) (2 diffs)
• Source/WebKit2/NetworkProcess/NetworkLoad.cpp (modified) (1 diff)
• Source/WebKit2/NetworkProcess/NetworkLoad.h (modified) (1 diff)
• Source/WebKit2/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm (modified) (1 diff)
• Source/WebKit2/NetworkProcess/cocoa/NetworkSessionCocoa.mm (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/WebKitTestRunner/TestController.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-09-24  Chris Dumez  <cdumez@apple.com>
+
+        [WK2] anchor.download attribute value is ignored by NETWORK_SESSION code path
+        https://bugs.webkit.org/show_bug.cgi?id=162531
+
+        Reviewed by Darin Adler.
+
+        Unskip tests that are now passing.
+
+        * platform/mac/TestExpectations:
+
 2016-09-23  Zalan Bujtas  <zalan@apple.com>
 

trunk/LayoutTests/platform/mac/TestExpectations

@@ -1384 +1384 @@
 [ Sierra+ ] fast/dom/HTMLAnchorElement/anchor-download.html [ Skip ]
 [ Sierra+ ] fast/dom/HTMLAnchorElement/anchor-nodownload-set.html [ Skip ]
-[ Sierra+ ] http/tests/security/anchor-download-allow-data.html [ Skip ]
-[ Sierra+ ] http/tests/security/anchor-download-allow-sameorigin.html [ Skip ]
 [ Sierra+ ] http/tests/security/anchor-download-allow-blob.html [ Skip ]
 

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2016-09-24  Chris Dumez  <cdumez@apple.com>
+
+        [WK2] anchor.download attribute value is ignored by NETWORK_SESSION code path
+        https://bugs.webkit.org/show_bug.cgi?id=162531
+
+        Reviewed by Darin Adler.
+
+        Pipe the suggested filename through in the NETWORK_SESSION code path so
+        that it is used when deciding the destination path.
+
+        * NetworkProcess/Downloads/DownloadManager.cpp:
+        (WebKit::DownloadManager::startDownload):
+        * NetworkProcess/Downloads/PendingDownload.cpp:
+        (WebKit::PendingDownload::PendingDownload):
+        * NetworkProcess/Downloads/PendingDownload.h:
+        * NetworkProcess/NetworkDataTask.h:
+        * NetworkProcess/NetworkLoad.cpp:
+        (WebKit::NetworkLoad::setSuggestedFilename):
+        * NetworkProcess/NetworkLoad.h:
+        * NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:
+        (WebKit::NetworkDataTask::suggestedFilename):
+        (WebKit::NetworkDataTask::setSuggestedFilename):
+        * NetworkProcess/cocoa/NetworkSessionCocoa.mm:
+        (-[WKNetworkSessionDelegate URLSession:dataTask:didBecomeDownloadTask:]):
+
 2016-09-23  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Source/WebKit2/NetworkProcess/Downloads/DownloadManager.cpp

@@ -55 +55 @@
     parameters.request = request;
     parameters.clientCredentialPolicy = ClientCredentialPolicy::MayAskClientForCredentials;
-    m_pendingDownloads.add(downloadID, std::make_unique<PendingDownload>(WTFMove(parameters), downloadID, *networkSession));
+    m_pendingDownloads.add(downloadID, std::make_unique<PendingDownload>(WTFMove(parameters), downloadID, *networkSession, suggestedName));
 #else
     auto download = std::make_unique<Download>(*this, downloadID, request, suggestedName);

trunk/Source/WebKit2/NetworkProcess/Downloads/PendingDownload.cpp

@@ -39 +39 @@
 namespace WebKit {
 
-PendingDownload::PendingDownload(NetworkLoadParameters&& parameters, DownloadID downloadID, NetworkSession& networkSession)
+PendingDownload::PendingDownload(NetworkLoadParameters&& parameters, DownloadID downloadID, NetworkSession& networkSession, const String& suggestedName)
     : m_networkLoad(std::make_unique<NetworkLoad>(*this, WTFMove(parameters), networkSession))
 {
     m_networkLoad->setPendingDownloadID(downloadID);
     m_networkLoad->setPendingDownload(*this);
+    m_networkLoad->setSuggestedFilename(suggestedName);
 }
 

trunk/Source/WebKit2/NetworkProcess/Downloads/PendingDownload.h

@@ -46 +46 @@
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    PendingDownload(NetworkLoadParameters&&, DownloadID, NetworkSession&);
+    PendingDownload(NetworkLoadParameters&&, DownloadID, NetworkSession&, const String& suggestedName);
 
     void continueWillSendRequest(WebCore::ResourceRequest&&);

trunk/Source/WebKit2/NetworkProcess/NetworkDataTask.h

@@ -128 +128 @@
     WebCore::ResourceRequest currentRequest();
     String suggestedFilename();
+    void setSuggestedFilename(const String&);
     void willPerformHTTPRedirection(WebCore::ResourceResponse&&, WebCore::ResourceRequest&&, RedirectCompletionHandler&&);
     void transferSandboxExtensionToDownload(Download&);
@@ -165 +166 @@
     RetainPtr<NSURLSessionDataTask> m_task;
 #endif
+    String m_suggestedFilename;
 };
 

trunk/Source/WebKit2/NetworkProcess/NetworkLoad.cpp

@@ -202 +202 @@
 }
 
+void NetworkLoad::setSuggestedFilename(const String& suggestedName)
+{
+    if (!m_task)
+        return;
+
+    m_task->setSuggestedFilename(suggestedName);
+}
+
 void NetworkLoad::setPendingDownload(PendingDownload& pendingDownload)
 {

trunk/Source/WebKit2/NetworkProcess/NetworkLoad.h

@@ -67 +67 @@
     void convertTaskToDownload(DownloadID, const WebCore::ResourceRequest&, const WebCore::ResourceResponse&);
     void setPendingDownloadID(DownloadID);
+    void setSuggestedFilename(const String&);
     void setPendingDownload(PendingDownload&);
     DownloadID pendingDownloadID() { return m_task->pendingDownloadID(); }

trunk/Source/WebKit2/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm

@@ -372 +372 @@
 String NetworkDataTask::suggestedFilename()
 {
+    if (!m_suggestedFilename.isEmpty())
+        return m_suggestedFilename;
     return m_task.get().response.suggestedFilename;
+}
+
+void NetworkDataTask::setSuggestedFilename(const String& suggestedName)
+{
+    m_suggestedFilename = suggestedName;
 }
 

trunk/Source/WebKit2/NetworkProcess/cocoa/NetworkSessionCocoa.mm

@@ -298 +298 @@
         auto downloadID = networkDataTask->pendingDownloadID();
         auto& downloadManager = WebKit::NetworkProcess::singleton().downloadManager();
-        auto download = std::make_unique<WebKit::Download>(downloadManager, downloadID, downloadTask, _session->sessionID());
+        auto download = std::make_unique<WebKit::Download>(downloadManager, downloadID, downloadTask, _session->sessionID(), networkDataTask->suggestedFilename());
         networkDataTask->transferSandboxExtensionToDownload(*download);
         ASSERT(WebCore::fileExists(networkDataTask->pendingDownloadLocation()));

trunk/Tools/ChangeLog

@@ +1 @@
+2016-09-24  Chris Dumez  <cdumez@apple.com>
+
+        [WK2] anchor.download attribute value is ignored by NETWORK_SESSION code path
+        https://bugs.webkit.org/show_bug.cgi?id=162531
+
+        Reviewed by Darin Adler.
+
+        Update decideDestinationWithSuggestedFilename() to return a actual file path
+        instead of nullptr. The NETWORK_SESSION code path cancels the download when
+        decideDestinationWithSuggestedFilename() returns an empty file path, unlike
+        the !NETWORK_SESSION code path that just continue downloading without
+        explicitely setting a destination path. This lead to different test results
+        for NETWORK_SESSION and !NETWORK_SESSION code paths. Returns a valid
+        destination path makes more sense and leads to consistent results.
+
+        * WebKitTestRunner/TestController.cpp:
+        (WTR::TestController::decideDestinationWithSuggestedFilename):
+
 2016-09-23  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Tools/WebKitTestRunner/TestController.cpp

@@ -1674 +1674 @@
 WKStringRef TestController::decideDestinationWithSuggestedFilename(WKContextRef, WKDownloadRef, WKStringRef filename, bool*& allowOverwrite)
 {
+    String suggestedFilename = toWTFString(filename);
+
     StringBuilder builder;
     builder.append("Downloading URL with suggested filename \"");
-    builder.append(toWTFString(filename));
+    builder.append(suggestedFilename);
     builder.append("\"\n");
 
     m_currentInvocation->outputText(builder.toString());
 
-    return nullptr;
+    const char* dumpRenderTreeTemp = libraryPathForTesting();
+    if (!dumpRenderTreeTemp)
+        return nullptr;
+
+    *allowOverwrite = true;
+    String temporaryFolder = String::fromUTF8(dumpRenderTreeTemp);
+    if (suggestedFilename.isEmpty())
+        suggestedFilename = "Unknown";
+
+    return toWK(temporaryFolder + "/" + suggestedFilename).leakRef();
 }