Bug 32359 – [GTK] Don't assume downloads are always synchronous

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Christian Dywan]

The download-requested signal assumes if the download was handled, it can start it right away. This is wrong if the application needs to handle the download asynchronously. In that case, WebKit should leave it up to the API user to start or cancel the download as appropriate.

[Christian Dywan]

Created attachment 44590 [details]
Check if the destination URI is set and clarify the documentation

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44590 [details] without any errors.

[Gustavo Noronha (kov)]

Comment on attachment 44590 [details]
Check if the destination URI is set and clarify the documentation

This works for me. Can you add a unittest, to make sure this works, to testdownload?

[Christian Dywan]

Created attachment 45044 [details]
Check destination URI, clarify docs and test downloads both ways

Updated the patch, to test downloads with having the destination set in the callback, and again with setting it later. The test breaks without the frame loader change.

[WebKit Review Bot]

Attachment 45044 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKit/gtk/tests/testdownload.c:29:  Declaration has space between * and variable name in WebKitDownload* theDownload  [whitespace/declaration] [3]
WebKit/gtk/tests/testdownload.c:137:  Declaration has space between * and variable name in gchar* filename  [whitespace/declaration] [3]
WebKit/gtk/tests/testdownload.c:138:  Extra space before ( in function call  [whitespace/parens] [4]
WebKit/gtk/tests/testdownload.c:148:  Extra space before ( in function call  [whitespace/parens] [4]
WebKit/gtk/tests/testdownload.c:171:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 5

[Christian Dywan]

Created attachment 45046 [details]
Check destination URI, clarify docs and test downloads both ways #2

Bracket and indentation fixed. The globals the bot complains about are correct as far as I can see.

[WebKit Review Bot]

Attachment 45046 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKit/gtk/tests/testdownload.c:29:  Declaration has space between * and variable name in WebKitDownload* theDownload  [whitespace/declaration] [3]
WebKit/gtk/tests/testdownload.c:137:  Declaration has space between * and variable name in gchar* filename  [whitespace/declaration] [3]
Total errors found: 2

[Gustavo Noronha (kov)]

Comment on attachment 45046 [details]
Check destination URI, clarify docs and test downloads both ways #2

Good, but please fix the issues pointed out by the style bot =).

[Gustavo Noronha (kov)]

(In reply to comment #8)
> (From update of attachment 45046 [details])
> Good, but please fix the issues pointed out by the style bot =).

In fact, don't. Please report those as bugs to the bot =P.

[Christian Dywan]

2009-12-17  Christian Dywan  <[e-mail]>

        Reviewed by Gustavo Noronha Silva.

        [GTK] Don't assume downloads are always synchronous
        http://bugs.webkit.org/show_bug.cgi?id=32359

        * tests/testdownload.c:
        (download_requested_cb):
        (set_filename):
        (test_webkit_download_perform):
        (test_webkit_download_synch):
        (test_webkit_download_asynch):
        (main): Test downloads synchronously and asynchronously.
        * webkit/webkitwebview.cpp:
        (webkit_web_view_class_init):
        (webkit_web_view_request_download): Only try to start a requested
        download if the destination URI is set and clarify the documentation.