Bug 118239 – [GTK] Make DRT obbey testRunner's addURLToRedirect

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Simon Pena]

[GTK] Make DRT obbey testRunner's addURLToRedirect

[Simon Pena]

Created attachment 205805 [details]
Patch

[Gustavo Noronha (kov)]

Comment on attachment 205805 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=205805&action=review

r- because of the leak

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1305
> +    if (destination.length())

Checking for length() here feels wrong to me, there is empty(), but it looks like the Mac port does the same so *shrug*.

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1306
> +        webkit_network_request_set_uri(request, strdup(destination.c_str()));

Should not need to strdup the string here. Did you have trouble with the c_str() being destroyed early or something? In any case, you are leaking the result here since set_uri does not take ownership of the passed string.

[Simon Pena]

Created attachment 206740 [details]
Patch

[Simon Pena]

(In reply to comment #2)
> (From update of attachment 205805 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=205805&action=review
> 
> r- because of the leak
> 
> > Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1305
> > +    if (destination.length())
> 
> Checking for length() here feels wrong to me, there is empty(), but it looks like the Mac port does the same so *shrug*.
> 
> > Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1306
> > +        webkit_network_request_set_uri(request, strdup(destination.c_str()));
> 
> Should not need to strdup the string here. Did you have trouble with the c_str() being destroyed early or something? In any case, you are leaking the result here since set_uri does not take ownership of the passed string.

Thanks for catching that! I failed to clear it after checking a different thing. I also used empty(), since it's true that it feels more natural.

[Gustavo Noronha (kov)]

Comment on attachment 206740 [details]
Patch

thanks!

[WebKit Commit Bot]

Comment on attachment 206740 [details]
Patch

Clearing flags on attachment: 206740

Committed r152782: <http://trac.webkit.org/changeset/152782>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.