Bug 123803 – [WIN] Add a wrapper for setting the default settings

• Status: NEW
• Component: New Bugs

[Patrick R. Gansterer]

[WIN] Add a wrapper for setting the default settings

[Patrick R. Gansterer]

Created attachment 216040 [details]
Patch

[Brent Fulgham]

Comment on attachment 216040 [details]
Patch

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

This change is likely to introduce stability problems as versions of CoreFoundation or WebKit are changed out independently.

> Source/WebKit/win/WebPreferences.cpp:116
> +        RetainPtr<CFStringRef> cfValue = adoptCF(CFStringCreateWithCStringNoCopy(0, value, kCFStringEncodingASCII, 0));

This doesn't properly duplicate the behavior of the CFSTR macro.  CFSTR(a) maps to __CFStringMakeConstantString(a), which builds a cached string value internally so that subsequent calls get the 'same' object back.

We definitely don't want to use "CFStringCreateWithCStringNoCopy" because we can have allocator differences between the C++ RTL and whatever was used to build the specific version of CoreFoundation used with WebKit.  It would be better to change this to CFStringCreateWithCString (allowing a copy to be made by the CoreFoundation allocator), and better still to use the __CFStringMakeConstantString method call.

[Patrick R. Gansterer]

Created attachment 216236 [details]
Patch

[Patrick R. Gansterer]

Comment on attachment 216236 [details]
Patch

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

> Source/WebKit/win/WebPreferences.cpp:116
> +        RetainPtr<CFStringRef> cfValue = adoptCF(CFStringCreateWithCStringNoCopy(0, value, kCFStringEncodingASCII, kCFAllocatorNull));

I added the missing kCFAllocatorNull, since all strings are compile-time-strings without allocation.

[Alex Christensen]

I'd do something like this to mimic existing behavior without a setInternal function:

    void set(const char* key, const char* value)
    {
		CFDictionaryAddValue(m_dictionary.get(), __CFStringMakeConstantString(key), __CFStringMakeConstantString(value));
    }

    void set(const char* key, bool value)
    {
		CFDictionaryAddValue(m_dictionary.get(), __CFStringMakeConstantString(key), value ? kCFBooleanTrue : kCFBooleanFalse);
    }

    void set(const char* key, const String& value)
    {
		CFDictionaryAddValue(m_dictionary.get(), __CFStringMakeConstantString(key), value.createCFString().get());
    }

[Brady Eidson]

Comment on attachment 216236 [details]
Patch

Assuming that patches for review since 2013 are stale, r-