Bug 203876

Summary: WEBCORE_TESTSUPPORT_EXPORT should be used only for WebCoreTestSupport, not for WebCore
Product: WebKit Reporter: Fujii Hironori <fujii.hironori>
Component: WebCore Misc.Assignee: Fujii Hironori <fujii.hironori>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, calvaris, cdumez, darin, don.olmstead, eric.carlson, esprehn+autocc, ews-watchlist, glenn, hta, jer.noble, kondapallykalyan, philipj, pvollan, ross.kirsling, sergio, tommyw, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Fujii Hironori
Reported 2019-11-05 18:46:08 PST
I'm going to fix WEBCORE_TESTSUPPORT_EXPORT mis-use.
Attachments
Patch (11.46 KB, patch)
2019-11-05 18:56 PST, Fujii Hironori 		no flags
DetailsFormatted DiffDiff
Patch (11.45 KB, patch)
2019-11-06 18:17 PST, Fujii Hironori 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Fujii Hironori
Comment 1 2019-11-05 18:56:20 PST
Created attachment 382882 [details] Patch
Ross Kirsling
Comment 2 2019-11-06 12:18:50 PST
Comment on attachment 382882 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=382882&action=review Makes sense and looks like EWS is happy with it. > Source/WebCore/ChangeLog:3 > + [Win] WEBCORE_TESTSUPPORT_EXPORT should be used only for WebCoreTestSupport, but for WebCore Nit: "not" for WebCore
Fujii Hironori
Comment 3 2019-11-06 14:50:18 PST
Comment on attachment 382882 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=382882&action=review >> Source/WebCore/ChangeLog:3 >> + [Win] WEBCORE_TESTSUPPORT_EXPORT should be used only for WebCoreTestSupport, but for WebCore > > Nit: "not" for WebCore Will fix. > Source/WebKit/config.h:-40 > -#define WEBCORE_TESTSUPPORT_EXPORT WTF_EXPORT_DECLARATION I should remove WEBCORE_TESTSUPPORT_EXPORT definition in webcore not only for windows, but also for linux and mac in order to avoid build breakage only for windows.
Fujii Hironori
Comment 4 2019-11-06 18:17:01 PST
Created attachment 382996 [details] Patch
Fujii Hironori
Comment 5 2019-11-06 18:20:09 PST
Comment on attachment 382882 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=382882&action=review >> Source/WebKit/config.h:-40 >> -#define WEBCORE_TESTSUPPORT_EXPORT WTF_EXPORT_DECLARATION > > I should remove WEBCORE_TESTSUPPORT_EXPORT definition in webcore not only for windows, but also for linux and mac in order to avoid build breakage only for windows. Umm, it's not easy to do that. WebCore and WebCoreTestSupport shares WebCore/config.h which includes WebCore/platform/PlatformExportMacros.h. I will do it if Windows-only build breakage will happen frequently.
Fujii Hironori
Comment 6 2019-11-06 18:29:52 PST
Comment on attachment 382996 [details] Patch Clearing flags on attachment: 382996 Committed r252166: <https://trac.webkit.org/changeset/252166>
Fujii Hironori
Comment 7 2019-11-06 18:29:56 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 8 2019-11-06 18:30:17 PST
<rdar://problem/56968155>
Darin Adler
Comment 9 2020-02-05 21:31:04 PST
Should add a comment somewhere explaining this?
Fujii Hironori
Comment 10 2020-02-05 23:01:24 PST
OK, filed: Bug 207323 – WEBCORE_TESTSUPPORT_EXPORT shouldn't be defined for compiling WebCore to avoid misuse But, this is not simple task. (Comment 5)
Fujii Hironori
Comment 11 2020-02-06 07:38:27 PST
Rethinking comment 9, Bug 207323 approach can't be the solution. Mac port folks can miuse WTF_EXPORT_PRIVATE and JS_EXPORT_PRIVATE in WebCore because their definitions are same. check-webkit-style seems the best place to prevent the misuse.
Fujii Hironori
Comment 12 2020-02-07 04:16:52 PST
(In reply to Fujii Hironori from comment #11) Filed: Bug 207380 – check-webkit-style: Check if *_EXPORT and *_EXPORT_PRIVATE macros are used under corresponding directories
Note You need to log in before you can comment on or make changes to this bug.