[webkit-reviews] review requested: [Bug 56855] [GTK] Provide a way in DRT to check the platform name : [Attachment 95452] Patch proposal
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue May 31 10:45:12 PDT 2011
Mario Sanchez Prada <msanchez at igalia.com> has asked for review:
Bug 56855: [GTK] Provide a way in DRT to check the platform name
https://bugs.webkit.org/show_bug.cgi?id=56855
Attachment 95452: Patch proposal
https://bugs.webkit.org/attachment.cgi?id=95452&action=review
------- Additional Comments from Mario Sanchez Prada <msanchez at igalia.com>
(In reply to comment #12)
> (From update of attachment 88635 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=88635&action=review
>
> You should add a test that shows this is working.
Well I haven't done it since it's a patch for a new functionality in DRT, not
in WebKit, so I thought it wouldn't be needed since its functionality would be
tested anytime it was used in a LayoutTest :-)
> You should also explain why this is needed.
See comment #8 and comment #10. It's all in there :-)
> > Tools/DumpRenderTree/LayoutTestController.cpp:2093
> > + return JSValueMakeString(context, controller->platformName());
>
> This is leaking the JSStringRef returned by platformName. platformName should
return a JSRetainPtr<JSStringRef> instead.
Thanks for the catch. Attaching a new patch fixing these issues.
More information about the webkit-reviews
mailing list