Bug 203876 – WEBCORE_TESTSUPPORT_EXPORT should be used only for WebCoreTestSupport, not for WebCore

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Fujii Hironori]

I'm going to fix WEBCORE_TESTSUPPORT_EXPORT mis-use.

[Fujii Hironori]

Created attachment 382882 [details]
Patch

[Ross Kirsling]

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 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]

Created attachment 382996 [details]
Patch

[Fujii Hironori]

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 on attachment 382996 [details]
Patch

Clearing flags on attachment: 382996

Committed r252166: <https://trac.webkit.org/changeset/252166>

[Fujii Hironori]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/56968155>

[Darin Adler]

Should add a comment somewhere explaining this?

[Fujii Hironori]

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]

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]

(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