[WK2][UNIX] Free standard output obtained from g_spawn_sync() with g_free
Created attachment 206471 [details] Patch
Comment on attachment 206471 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=206471&action=review I'm not sure that g_free is necessarily better than free, but a GOwnptr probably makes sense. :) > Source/WebKit2/UIProcess/Plugins/unix/PluginProcessProxyUnix.cpp:73 > - char* stdOut = 0; > + gchar* stdOut = 0; Why is this necessary? They are the same type and in WebKit code we typically use the non-Glib versions. > Source/WebKit2/UIProcess/Plugins/unix/PluginProcessProxyUnix.cpp:91 > + g_free(stdOut); Why not switch to using GOwnPtr here?
(In reply to comment #2) > (From update of attachment 206471 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=206471&action=review > > I'm not sure that g_free is necessarily better than free, but a GOwnptr probably makes sense. :) > > > Source/WebKit2/UIProcess/Plugins/unix/PluginProcessProxyUnix.cpp:73 > > - char* stdOut = 0; > > + gchar* stdOut = 0; > > Why is this necessary? They are the same type and in WebKit code we typically use the non-Glib versions. > Good point. We should keep using char, then. > > Source/WebKit2/UIProcess/Plugins/unix/PluginProcessProxyUnix.cpp:91 > > + g_free(stdOut); > > Why not switch to using GOwnPtr here? Yeah, I thought of using it, but since this code is also shared with EFL, I wasn't sure it would be okay. I am going to upload an updated version using GOwnPtr. Thanks for the review.
Comment on attachment 206471 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=206471&action=review >>> Source/WebKit2/UIProcess/Plugins/unix/PluginProcessProxyUnix.cpp:91 >>> + g_free(stdOut); >> >> Why not switch to using GOwnPtr here? > > Yeah, I thought of using it, but since this code is also shared with EFL, I wasn't sure it would be okay. I am going to upload an updated version using GOwnPtr. Thanks for the review. Yes, it is fine for the EFL port. This file uses glib already (e.g. g_spawn_sync()).
Created attachment 206474 [details] Patch
Comment on attachment 206474 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=206474&action=review Looks good except for the include line. > Source/WebKit2/UIProcess/Plugins/unix/PluginProcessProxyUnix.cpp:39 > +#include <GOwnPtr.h> I think you want <wtf/gobject/GOwnPr.h> here.
Comment on attachment 206474 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=206474&action=review >> Source/WebKit2/UIProcess/Plugins/unix/PluginProcessProxyUnix.cpp:39 >> +#include <GOwnPtr.h> > > I think you want <wtf/gobject/GOwnPr.h> here. My bad, this time I did only an EFL build here. I am finishing the Gtk build with the include fix and once it finishes I will reupload and ask for cq. Thanks.
Created attachment 206476 [details] Patch Include fixed.
Comment on attachment 206474 [details] Patch Attachment 206474 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/962174 New failing tests: svg/batik/masking/maskRegions.svg
Created attachment 206478 [details] Archive of layout-test-results from webkit-ews-06 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-06 Port: mac-mountainlion Platform: Mac OS X 10.8.3
Comment on attachment 206476 [details] Patch Clearing flags on attachment: 206476 Committed r152575: <http://trac.webkit.org/changeset/152575>