[webkit-reviews] review denied: [Bug 38256] [GTK] Random failure on 'testdownload' unit test : [Attachment 55972] Make sure the set_filename function is called after handling the 'download-requested' signal for the asynchronous test

[bugzilla-daemon at webkit.org]

Xan Lopez <xan.lopez at gmail.com> has denied Mario Sanchez Prada
<msanchez at igalia.com>'s request for review:
Bug 38256: [GTK] Random failure on 'testdownload' unit test
https://bugs.webkit.org/show_bug.cgi?id=38256

Attachment 55972: Make sure the set_filename function is called after handling
the 'download-requested' signal for the asynchronous test
https://bugs.webkit.org/attachment.cgi?id=55972&action=review

------- Additional Comments from Xan Lopez <xan.lopez at gmail.com>
>--- a/WebKit/gtk/ChangeLog
>+++ b/WebKit/gtk/ChangeLog
>@@ -1,3 +1,21 @@
>+2010-05-06  Mario Sanchez Prada  <msanchez at igalia.com>
>+
>+	  Reviewed by NOBODY (OOPS!).
>+
>+	  [GTK] Random failure on 'testdownload' unit test
>+	  https://bugs.webkit.org/show_bug.cgi?id=38256
>+
>+	  Make sure the set_filename function is called after
>+	  handling the 'download-requested' signal for the
>+	  asynchronous test.
>+
>+	  * tests/testdownload.c:
>+	  (set_filename):
>+	  (handle_download_requested_cb):
>+	  (download_requested_cb):
>+	  (download_requested_asynch_cb):
>+	  (test_webkit_download_perform):
>+

As commented, it would be nice to add here the explanation in comment #15,
since you already wrote it.

>+	      g_idle_add((GSourceFunc)set_filename,
g_strdup(temporaryFilename));

Mmm, do you really need to dup the string?

>+	  } else {
>+	      gchar *uri = g_filename_to_uri(temporaryFilename, NULL, NULL);
>+	      if (uri)
>+		  webkit_download_set_destination_uri(download, uri);
>+	      g_free(uri);
>+	  }
>     }
> 

>+    GCallback dr_cb = NULL;

downloadRequestCallback? or downloadRequestCB? or something not 'dr_cb' :D