Bug 217239 – Unify linkedOnOrAfter duplicate code and use SPI/WebCoreTestSupport instead of NSUserDefaults

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

[Alex Christensen]

Unify linkedOnOrAfter duplicate code and use SPI/WebCoreTestSupport instead of NSUserDefaults

[Alex Christensen]

Created attachment 410345 [details]
Patch

[Alex Christensen]

Created attachment 410346 [details]
Patch

[Alex Christensen]

Created attachment 410354 [details]
Patch

[Darin Adler]

Comment on attachment 410354 [details]
Patch

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

Great idea to unify and clean this up.

review- because this doesn’t yet have the correct code to do the right thing inside the web process.

> Source/WebCore/platform/cocoa/VersionChecks.mm:58
> +    return sdkVersionAsInteger && dyld_get_program_sdk_version() >= sdkVersionAsInteger;

I think this needs to call applicationSDKVersion() from RuntimeApplicationChecksCocoa.h instead of dyld_get_program_sdk_version(), otherwise it will give the wrong answer when run inside the web process.

> Source/WebCore/testing/js/WebCoreTestSupport.cpp:150
> +    WebCore::setLinkedOnOrAfterEverythingForTesting();

How about this instead?

    setApplicationSDKVersion(std::numeric_limits<uint32_t>::max());

Then we don’t need to add WebCore::setLinkedOnOrAfterEverythingForTesting. Doing something similar for the SDK version that is passed cross-process could reduce the change in modern WebKit. We’d change the WebProcessPool code that sets uiProcessSDKVersion to also call applicationSDKVersion() instead of dyld_get_program_sdk_version().

> Tools/DumpRenderTree/mac/DumpRenderTree.mm:1045
> +    WebCoreTestSupport::setLinkedOnOrAfterEverythingForTesting();

I think we should move this out of this function into DumpRenderTreeMain.

> Tools/TestWebKitAPI/Configurations/TestWTF.xcconfig:30
> +GCC_PREPROCESSOR_DEFINITIONS = $(inherited) BUILDING_TEST_WTF=1

No need for the "=1" here.

> Tools/TestWebKitAPI/ios/mainIOS.mm:44
> +#ifndef BUILDING_TEST_WTF
> +    [WKProcessPool _setLinkedOnOrAfterEverythingForTesting];
> +#endif

Can we put this after the NSUserDefaults call?

> Tools/WebKitTestRunner/mac/main.mm:50
> +    [WKProcessPool _setLinkedOnOrAfterEverythingForTesting];

We should move this out of this function into main.

[Alex Christensen]

Created attachment 410446 [details]
Patch

[Alex Christensen]

Created attachment 410447 [details]
Patch

[Darin Adler]

Comment on attachment 410447 [details]
Patch

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

> Tools/TestWebKitAPI/mac/mainMac.mm:55
> +#ifndef BUILDING_TEST_WTF
> +    [WKProcessPool _setLinkedOnOrAfterEverythingForTesting];
> +#endif

This doesn’t really belong, here, between the dictionary used for NSUserDefaults, and the method call for NSUserDefaults. I'd move it down a little bit, until after the NSUserDefaults code.

[Alex Christensen]

Done
http://trac.webkit.org/r267966

[Radar WebKit Bug Importer]

<rdar://problem/69954414>