Bug 217239 - Unify linkedOnOrAfter duplicate code and use SPI/WebCoreTestSupport instead of NSUserDefaults
Summary: Unify linkedOnOrAfter duplicate code and use SPI/WebCoreTestSupport instead o...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks: 68089
  Show dependency treegraph
 
Reported: 2020-10-02 11:11 PDT by Alex Christensen
Modified: 2020-10-05 07:59 PDT (History)
5 users (show)

See Also:


Attachments
Patch (90.36 KB, patch)
2020-10-02 11:16 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (88.44 KB, patch)
2020-10-02 11:20 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (88.50 KB, patch)
2020-10-02 12:25 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (82.93 KB, patch)
2020-10-03 16:55 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (79.38 KB, patch)
2020-10-03 17:06 PDT, Alex Christensen
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2020-10-02 11:11:29 PDT
Unify linkedOnOrAfter duplicate code and use SPI/WebCoreTestSupport instead of NSUserDefaults
Comment 1 Alex Christensen 2020-10-02 11:16:52 PDT
Created attachment 410345 [details]
Patch
Comment 2 Alex Christensen 2020-10-02 11:20:39 PDT
Created attachment 410346 [details]
Patch
Comment 3 Alex Christensen 2020-10-02 12:25:01 PDT
Created attachment 410354 [details]
Patch
Comment 4 Darin Adler 2020-10-03 14:38:39 PDT
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.
Comment 5 Alex Christensen 2020-10-03 16:55:41 PDT
Created attachment 410446 [details]
Patch
Comment 6 Alex Christensen 2020-10-03 17:06:58 PDT
Created attachment 410447 [details]
Patch
Comment 7 Darin Adler 2020-10-04 13:14:00 PDT
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.
Comment 8 Alex Christensen 2020-10-05 07:58:21 PDT
Done
http://trac.webkit.org/r267966
Comment 9 Radar WebKit Bug Importer 2020-10-05 07:59:20 PDT
<rdar://problem/69954414>