Changeset 218185 in webkit


Ignore:
Timestamp:
Jun 13, 2017 10:55:25 AM (7 years ago)
Author:
Carlos Garcia Campos
Message:

[GTK] Blob download doesn't work
​https://bugs.webkit.org/show_bug.cgi?id=172442

Reviewed by Carlos Alberto Lopez Perez.

Source/WebKit2:

GTK+ API uses URIs for download destination paths, and passes that URIs to the WebKit internals. But WebKit
expects download destination location to be a local path. This is not a problem for normal downloads, because
the soup backend handles the cases of download destination being a URI and a path. For blob downloads
NetworkDataTaskBlob is used, and it always expects the download destination to be a local path, failing in
FileSystem::openFile() when a URI is passed. We need to keep using local files internally and convert to URIs
only when exposing those paths to the API.

  • NetworkProcess/soup/NetworkDataTaskSoup.cpp:

(WebKit::NetworkDataTaskSoup::download): Stop handling URIs here, we should always expect local files.

  • UIProcess/API/gtk/WebKitDownload.cpp:

(webkitDownloadDecideDestinationWithSuggestedFilename): Convert destination URI to filanme before pasing it to DownloadClient.
(webkitDownloadDestinationCreated): Convert the destination path to a URI before passing it to WebKitDownload::created-destionation signal.

  • UIProcess/API/gtk/WebKitDownloadClient.cpp:
  • UIProcess/API/gtk/WebKitDownloadPrivate.h:

Tools:

Add a unit test to check blob downloads.

  • TestWebKitAPI/Tests/WebKit2Gtk/TestDownloads.cpp:

(testBlobDownload):
(beforeAll):

Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit2/ChangeLog

    r218180 r218185  
     12017-06-13  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        [GTK] Blob download doesn't work
     4        https://bugs.webkit.org/show_bug.cgi?id=172442
     5
     6        Reviewed by Carlos Alberto Lopez Perez.
     7
     8        GTK+ API uses URIs for download destination paths, and passes that URIs to the WebKit internals. But WebKit
     9        expects download destination location to be a local path. This is not a problem for normal downloads, because
     10        the soup backend handles the cases of download destination being a URI and a path. For blob downloads
     11        NetworkDataTaskBlob is used, and it always expects the download destination to be a local path, failing in
     12        FileSystem::openFile() when a URI is passed. We need to keep using local files internally and convert to URIs
     13        only when exposing those paths to the API.
     14
     15        * NetworkProcess/soup/NetworkDataTaskSoup.cpp:
     16        (WebKit::NetworkDataTaskSoup::download): Stop handling URIs here, we should always expect local files.
     17        * UIProcess/API/gtk/WebKitDownload.cpp:
     18        (webkitDownloadDecideDestinationWithSuggestedFilename): Convert destination URI to filanme before pasing it to DownloadClient.
     19        (webkitDownloadDestinationCreated): Convert the destination path to a URI before passing it to WebKitDownload::created-destionation signal.
     20        * UIProcess/API/gtk/WebKitDownloadClient.cpp:
     21        * UIProcess/API/gtk/WebKitDownloadPrivate.h:
     22
    1232017-06-13  Wenson Hsieh  <wenson_hsieh@apple.com>
    224

  • trunk/Source/WebKit2/NetworkProcess/soup/NetworkDataTaskSoup.cpp

    r216111 r218185  
    891891    }
    892892
    893     if (g_path_is_absolute(m_pendingDownloadLocation.utf8().data()))
    894         m_downloadDestinationFile = adoptGRef(g_file_new_for_path(m_pendingDownloadLocation.utf8().data()));
    895     else
    896         m_downloadDestinationFile = adoptGRef(g_file_new_for_uri(m_pendingDownloadLocation.utf8().data()));
     893    CString downloadDestinationPath = m_pendingDownloadLocation.utf8();
     894    m_downloadDestinationFile = adoptGRef(g_file_new_for_path(downloadDestinationPath.data()));
    897895    GRefPtr<GFileOutputStream> outputStream;
    898896    GUniqueOutPtr<GError> error;
    … …  
    906904    }
    907905
    908     GUniquePtr<char> downloadDestinationURI(g_file_get_uri(m_downloadDestinationFile.get()));
    909     GUniquePtr<char> intermediateURI(g_strdup_printf("%s.wkdownload", downloadDestinationURI.get()));
    910     m_downloadIntermediateFile = adoptGRef(g_file_new_for_uri(intermediateURI.get()));
    911     outputStream = adoptGRef(g_file_replace(m_downloadIntermediateFile.get(), 0, TRUE, G_FILE_CREATE_NONE, 0, &error.outPtr()));
     906    GUniquePtr<char> intermediatePath(g_strdup_printf("%s.wkdownload", downloadDestinationPath.data()));
     907    m_downloadIntermediateFile = adoptGRef(g_file_new_for_path(intermediatePath.get()));
     908    outputStream = adoptGRef(g_file_replace(m_downloadIntermediateFile.get(), nullptr, TRUE, G_FILE_CREATE_NONE, nullptr, &error.outPtr()));
    912909    if (!outputStream) {
    913910        didFailDownload(downloadDestinationError(m_response, error->message));
    … …  
    920917    auto* downloadPtr = download.get();
    921918    downloadManager.dataTaskBecameDownloadTask(m_pendingDownloadID, WTFMove(download));
    922     downloadPtr->didCreateDestination(String::fromUTF8(downloadDestinationURI.get()));
     919    downloadPtr->didCreateDestination(m_pendingDownloadLocation);
    923920
    924921    ASSERT(!m_client);

  • trunk/Source/WebKit2/UIProcess/API/gtk/WebKitDownload.cpp

    r217960 r218185  
    3131#include <wtf/glib/GRefPtr.h>
    3232#include <wtf/glib/GUniquePtr.h>
     33#include <wtf/text/CString.h>
    3334
    3435using namespace WebKit;
    … …  
    402403}
    403404
    404 CString webkitDownloadDecideDestinationWithSuggestedFilename(WebKitDownload* download, const CString& suggestedFilename, bool& allowOverwrite)
     405String webkitDownloadDecideDestinationWithSuggestedFilename(WebKitDownload* download, const CString& suggestedFilename, bool& allowOverwrite)
    405406{
    406407    if (download->priv->isCancelled)
    407         return "";
     408        return emptyString();
    408409    gboolean returnValue;
    409410    g_signal_emit(download, signals[DECIDE_DESTINATION], 0, suggestedFilename.data(), &returnValue);
    410411    allowOverwrite = download->priv->allowOverwrite;
    411     return download->priv->destinationURI;
    412 }
    413 
    414 void webkitDownloadDestinationCreated(WebKitDownload* download, const CString& destinationURI)
     412    GUniquePtr<char> destinationPath(g_filename_from_uri(download->priv->destinationURI.data(), nullptr, nullptr));
     413    if (!destinationPath)
     414        return emptyString();
     415    return String::fromUTF8(destinationPath.get());
     416}
     417
     418void webkitDownloadDestinationCreated(WebKitDownload* download, const String& destinationPath)
    415419{
    416420    if (download->priv->isCancelled)
    417421        return;
    418     g_signal_emit(download, signals[CREATED_DESTINATION], 0, destinationURI.data(), nullptr);
     422    GUniquePtr<char> destinationURI(g_filename_to_uri(destinationPath.utf8().data(), nullptr, nullptr));
     423    ASSERT(destinationURI);
     424    g_signal_emit(download, signals[CREATED_DESTINATION], 0, destinationURI.get());
    419425}
    420426

  • trunk/Source/WebKit2/UIProcess/API/gtk/WebKitDownloadClient.cpp

    r212345 r218185  
    7878    {
    7979        GRefPtr<WebKitDownload> download = webkitWebContextGetOrCreateDownload(downloadProxy);
    80         return String::fromUTF8(webkitDownloadDecideDestinationWithSuggestedFilename(download.get(), filename.utf8(), allowOverwrite));
     80        return webkitDownloadDecideDestinationWithSuggestedFilename(download.get(), filename.utf8(), allowOverwrite);
    8181    }
    8282
    … …  
    8484    {
    8585        GRefPtr<WebKitDownload> download = webkitWebContextGetOrCreateDownload(downloadProxy);
    86         webkitDownloadDestinationCreated(download.get(), path.utf8());
     86        webkitDownloadDestinationCreated(download.get(), path);
    8787    }
    8888

  • trunk/Source/WebKit2/UIProcess/API/gtk/WebKitDownloadPrivate.h

    r208882 r218185  
    2525#include <WebCore/ResourceError.h>
    2626#include <WebCore/ResourceRequest.h>
    27 #include <wtf/text/CString.h>
    2827
    2928WebKitDownload* webkitDownloadCreate(WebKit::DownloadProxy*);
    … …  
    3534void webkitDownloadCancelled(WebKitDownload*);
    3635void webkitDownloadFinished(WebKitDownload*);
    37 CString webkitDownloadDecideDestinationWithSuggestedFilename(WebKitDownload*, const CString& suggestedFilename, bool& allowOverwrite);
    38 void webkitDownloadDestinationCreated(WebKitDownload*, const CString& destinationURI);
     36String webkitDownloadDecideDestinationWithSuggestedFilename(WebKitDownload*, const CString& suggestedFilename, bool& allowOverwrite);
     37void webkitDownloadDestinationCreated(WebKitDownload*, const String& destinationPath);
    3938
    4039#endif // WebKitDownloadPrivate_h

  • trunk/Tools/ChangeLog

    r218184 r218185  
     12017-06-13  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        [GTK] Blob download doesn't work
     4        https://bugs.webkit.org/show_bug.cgi?id=172442
     5
     6        Reviewed by Carlos Alberto Lopez Perez.
     7
     8        Add a unit test to check blob downloads.
     9
     10        * TestWebKitAPI/Tests/WebKit2Gtk/TestDownloads.cpp:
     11        (testBlobDownload):
     12        (beforeAll):
     13
    1142017-06-13  Carlos Garcia Campos  <cgarcia@igalia.com>
    215

  • trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestDownloads.cpp

    r213075 r218185  
    728728}
    729729
     730static void testBlobDownload(WebViewDownloadTest* test, gconstpointer)
     731{
     732    test->showInWindowAndWaitUntilMapped();
     733
     734    static const char* linkBlobHTML =
     735        "<html><body>"
     736        "<a id='downloadLink' style='position:absolute; left:1; top:1' download='foo.pdf'>Download Me</a>"
     737        "<script>"
     738        "  blob = new Blob(['Hello world'], {type: 'text/plain'});"
     739        "  document.getElementById('downloadLink').href = window.URL.createObjectURL(blob);"
     740        "</script>"
     741        "</body></html>";
     742    test->loadHtml(linkBlobHTML, kServer->getURIForPath("/").data());
     743    test->waitUntilLoadFinished();
     744
     745    g_idle_add([](gpointer userData) -> gboolean {
     746        auto* test = static_cast<WebViewDownloadTest*>(userData);
     747        test->clickMouseButton(1, 1, 1);
     748        return FALSE;
     749    }, test);
     750    test->waitUntilDownloadStarted();
     751
     752    g_assert(test->m_webView == webkit_download_get_web_view(test->m_download.get()));
     753    test->waitUntilDownloadFinished();
     754
     755    GRefPtr<GFile> downloadFile = adoptGRef(g_file_new_for_uri(webkit_download_get_destination(test->m_download.get())));
     756    GRefPtr<GFileInfo> downloadFileInfo = adoptGRef(g_file_query_info(downloadFile.get(), G_FILE_ATTRIBUTE_STANDARD_SIZE, static_cast<GFileQueryInfoFlags>(0), nullptr, nullptr));
     757    GUniquePtr<char> downloadPath(g_file_get_path(downloadFile.get()));
     758    GUniqueOutPtr<char> downloadContents;
     759    gsize downloadContentsLength;
     760    g_assert(g_file_get_contents(downloadPath.get(), &downloadContents.outPtr(), &downloadContentsLength, nullptr));
     761    g_assert_cmpint(g_file_info_get_size(downloadFileInfo.get()), ==, downloadContentsLength);
     762    g_assert_cmpstr(downloadContents.get(), ==, "Hello world");
     763    g_file_delete(downloadFile.get(), nullptr, nullptr);
     764}
     765
    730766void beforeAll()
    731767{
    … …  
    744780    DownloadTest::add("Downloads", "mime-type", testDownloadMIMEType);
    745781    WebViewDownloadTest::add("Downloads", "contex-menu-download-actions", testContextMenuDownloadActions);
     782    WebViewDownloadTest::add("Downloads", "blob-download", testBlobDownload);
    746783}
    747784
Note: See TracChangeset for help on using the changeset viewer.