[webkit-reviews] review denied: [Bug 20534] DumpRenderTree needs a way to override settings on a per-test basis : [Attachment 23484] Improved possible fix to bug 20534

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has denied 's request for review:
Bug 20534: DumpRenderTree needs a way to override settings on a per-test basis
https://bugs.webkit.org/show_bug.cgi?id=20534

Attachment 23484: Improved possible fix to bug 20534
https://bugs.webkit.org/attachment.cgi?id=23484&action=edit

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
I think the new functions added to WebPreferences should have some explicit
comments near their declarations stating more precisely what their purpose is
(i.e., these are for DRT only).

+    CFStringRef keyref = CFStringCreateWithCharactersNoCopy(0,
(UniChar*)_wcsdup(key), (CFIndex)_tcslen(key), kCFAllocatorMalloc);

This string is being leaked.
Passing in kCFAllocatorMalloc here is wrong. This will cause CF to call
free(key) when keyref is finalized.

+	setStringValue(keyref, value);

Indentation here looks funny.

+	 BSTR val = stringValueForKey(static_cast<CFStringRef>(keys[i]));

val is being leaked here. It also isn't used at all.

+    wstring wKey = jsStringRefToWString(key);
+    wstring wFlag = jsStringRefToWString(flag);
+
+    BSTR keyBSTR = SysAllocStringLen((OLECHAR*)wKey.c_str(), wKey.length());
+    BSTR flagBSTR = SysAllocStringLen((OLECHAR*)wFlag.c_str(),
wFlag.length());

You can use JSStringCopyBSTR here (from JSStringRefBSTR.h) instead.

r- so that the leaks and free() problem above can be fixed.