[Webkit-unassigned] [Bug 136322] [SOUP] WebKitDownload cannot overwrite existing file

[bugzilla-daemon at webkit.org]

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


Carlos Garcia Campos <cgarcia at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #237352|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #6 from Carlos Garcia Campos <cgarcia at igalia.com>  2014-08-29 09:51:54 PST ---
(From update of attachment 237352)
View in context: https://bugs.webkit.org/attachment.cgi?id=237352&action=review

Thanks, I have a few minor comments.

> Source/WebKit2/Shared/Downloads/soup/DownloadSoup.cpp:91
> -        m_destinationURI = m_download->decideDestinationWithSuggestedFilename(suggestedFilename, overwrite);
> +        m_destinationURI = m_download->decideDestinationWithSuggestedFilename(suggestedFilename, m_overwriteDestination);

Nit: I would use m_allowOverwrite for consistency with the IPC message param name.

> Source/WebKit2/Shared/Downloads/soup/DownloadSoup.cpp:140
> -        if (!g_file_move(m_intermediateFile.get(), destinationFile.get(), G_FILE_COPY_NONE, nullptr, nullptr, nullptr, &error.outPtr())) {
> +        if (!g_file_move(m_intermediateFile.get(), destinationFile.get(), flags, nullptr, nullptr, nullptr, &error.outPtr())) {

I think you can add the if directly here, without the local variable 

if (!g_file_move(m_intermediateFile.get(), destinationFile.get(), m_allowOverwrite ? G_FILE_COPY_OVERWRITE : G_FILE_COPY_NONE, nullptr, nullptr, nullptr, &error.outPtr())) {

> Source/WebKit2/Shared/Downloads/soup/DownloadSoup.cpp:200
> +    bool m_overwriteDestination = false;

Initialize the member in the constructor.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.