Bug 118239 - [GTK] Make DRT obbey testRunner's addURLToRedirect
Summary: [GTK] Make DRT obbey testRunner's addURLToRedirect
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Pena
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-01 06:22 PDT by Simon Pena
Modified: 2013-07-17 08:33 PDT (History)
3 users (show)

See Also:

Attachments
Patch (3.90 KB, patch)
2013-07-01 06:28 PDT, Simon Pena 		no flags Details | Formatted Diff | Diff
Patch (3.79 KB, patch)
2013-07-16 01:37 PDT, Simon Pena 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Pena 2013-07-01 06:22:40 PDT 
[GTK] Make DRT obbey testRunner's addURLToRedirect
Comment 1 Simon Pena 2013-07-01 06:28:23 PDT 
Created attachment 205805 [details]
Patch
Comment 2 Gustavo Noronha (kov) 2013-07-15 09:03:35 PDT 
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.
Comment 3 Simon Pena 2013-07-16 01:37:21 PDT 
Created attachment 206740 [details]
Patch
Comment 4 Simon Pena 2013-07-16 01:38:54 PDT 
(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.
Comment 5 Gustavo Noronha (kov) 2013-07-17 08:10:51 PDT 
Comment on attachment 206740 [details]
Patch

thanks!
Comment 6 WebKit Commit Bot 2013-07-17 08:33:24 PDT 
Comment on attachment 206740 [details]
Patch

Clearing flags on attachment: 206740

Committed r152782: <http://trac.webkit.org/changeset/152782>
Comment 7 WebKit Commit Bot 2013-07-17 08:33:27 PDT 
All reviewed patches have been landed.  Closing bug.