[Webkit-unassigned] [Bug 136423] [SOUP] Race condition when downloading a file due to the intermediate temporary file

[bugzilla-daemon at webkit.org]

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


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

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




--- Comment #3 from Carlos Garcia Campos <cgarcia at igalia.com>  2014-09-02 01:24:46 PST ---
(From update of attachment 237459)
View in context: https://bugs.webkit.org/attachment.cgi?id=237459&action=review

Looks good to me in general, I have just a few comments

> Source/WebKit2/Shared/Downloads/soup/DownloadSoup.cpp:67
> +        if (m_createdDestination) {
> +            ASSERT(m_destinationFile);
> +            g_file_delete(m_destinationFile.get(), nullptr, nullptr);
> +        }

I think we can use m_destinationFile to know if the destination was successfully created.

> Source/WebKit2/Shared/Downloads/soup/DownloadSoup.cpp:115
> +            outputStream = adoptGRef(g_file_replace(m_destinationFile.get(), nullptr, false, G_FILE_CREATE_NONE, nullptr, &error.outPtr()));

g_file_replace expects a gboolean, so better use FALSE, instead of false, even though it doesn't really matter.

> Source/WebKit2/Shared/Downloads/soup/DownloadSoup.cpp:121
> +        if (!outputStream) {
> +            downloadFailed(platformDownloadDestinationError(response, error->message));
> +            return;
> +        }

And here you could m_destinationFile.reset() and you don't need the m_createdDestination bool member

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