Bug 217239

Summary: Unify linkedOnOrAfter duplicate code and use SPI/WebCoreTestSupport instead of NSUserDefaults
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: New BugsAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, ews-watchlist, hi, joepeck, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 68089    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch darin: review+

Alex Christensen
Reported 2020-10-02 11:11:29 PDT
Unify linkedOnOrAfter duplicate code and use SPI/WebCoreTestSupport instead of NSUserDefaults
Attachments
Patch (90.36 KB, patch)
2020-10-02 11:16 PDT, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Patch (88.44 KB, patch)
2020-10-02 11:20 PDT, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Patch (88.50 KB, patch)
2020-10-02 12:25 PDT, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Patch (82.93 KB, patch)
2020-10-03 16:55 PDT, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Patch (79.38 KB, patch)
2020-10-03 17:06 PDT, Alex Christensen 		darin: review+
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Alex Christensen
Comment 1 2020-10-02 11:16:52 PDT
Created attachment 410345 [details] Patch
Alex Christensen
Comment 2 2020-10-02 11:20:39 PDT
Created attachment 410346 [details] Patch
Alex Christensen
Comment 3 2020-10-02 12:25:01 PDT
Created attachment 410354 [details] Patch
Darin Adler
Comment 4 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.
Alex Christensen
Comment 5 2020-10-03 16:55:41 PDT
Created attachment 410446 [details] Patch
Alex Christensen
Comment 6 2020-10-03 17:06:58 PDT
Created attachment 410447 [details] Patch
Darin Adler
Comment 7 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.
Alex Christensen
Comment 8 2020-10-05 07:58:21 PDT
Done http://trac.webkit.org/r267966
Radar WebKit Bug Importer
Comment 9 2020-10-05 07:59:20 PDT
<rdar://problem/69954414>
Note You need to log in before you can comment on or make changes to this bug.