Bug 141734 – use WEBCORE_EXPORT on Windows

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

[Alex Christensen]

Step 1: Add a few more WEBCORE_EXPORT macros.
Step 2: Make the switch in PlatformExportMacros.h and WebKitCommon.props.
Step 3: Remove WebKitExports.def.in and the associated build step.

[Alex Christensen]

(In reply to comment #0)
> Step 1: Add a few more WEBCORE_EXPORT macros.
> Step 2: Make the switch in PlatformExportMacros.h and WebKitCommon.props.
And in WebKitPrefix.h.
> Step 3: Remove WebKitExports.def.in and the associated build step.

[Brent Fulgham]

*** Bug 141720 has been marked as a duplicate of this bug. ***

[Brent Fulgham]

Created attachment 247367 [details]
Patch

[WebKit Commit Bot]

Attachment 247367 [details] did not pass style-queue:


ERROR: Source/WebCore/testing/js/WebCoreTestSupportPrefix.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebCore/testing/js/WebCoreTestSupportPrefix.h:126:  "windows.h" already included at Source/WebCore/testing/js/WebCoreTestSupportPrefix.h:105  [build/include] [4]
Total errors found: 2 in 46 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brent Fulgham]

Created attachment 247370 [details]
Patch

[WebKit Commit Bot]

Attachment 247370 [details] did not pass style-queue:


ERROR: Source/WebCore/testing/js/WebCoreTestSupportPrefix.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebCore/testing/js/WebCoreTestSupportPrefix.h:126:  "windows.h" already included at Source/WebCore/testing/js/WebCoreTestSupportPrefix.h:105  [build/include] [4]
ERROR: Tools/DumpRenderTree/win/DumpRenderTreePrefix.cpp:28:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 3 in 53 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brent Fulgham]

Created attachment 247372 [details]
Patch

[WebKit Commit Bot]

Attachment 247372 [details] did not pass style-queue:


ERROR: Source/WebCore/testing/js/WebCoreTestSupportPrefix.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebCore/testing/js/WebCoreTestSupportPrefix.h:126:  "windows.h" already included at Source/WebCore/testing/js/WebCoreTestSupportPrefix.h:105  [build/include] [4]
ERROR: Tools/DumpRenderTree/win/DumpRenderTreePrefix.cpp:28:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 3 in 53 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brent Fulgham]

Created attachment 247373 [details]
Patch

[WebKit Commit Bot]

Attachment 247373 [details] did not pass style-queue:


ERROR: Source/WebCore/testing/js/WebCoreTestSupportPrefix.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebCore/testing/js/WebCoreTestSupportPrefix.h:126:  "windows.h" already included at Source/WebCore/testing/js/WebCoreTestSupportPrefix.h:105  [build/include] [4]
ERROR: Tools/DumpRenderTree/win/DumpRenderTreePrefix.cpp:28:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 3 in 53 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alex Christensen]

Comment on attachment 247373 [details]
Patch

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

> Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters:7228
> +    <ClCompile Include="..\platform\cf\CoreMediaSoftLink.cpp" />

These should have filters.

> Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters:15177
> +    <ClInclude Include="..\Modules\streams\ReadableStream.h" />

Filters.

> Source/WebCore/WebCore.vcxproj/WebCoreTestSupport.vcxproj:179
> +      <ForceSymbolReferences>

What does this do?

> Source/WebCore/WebCore.vcxproj/WebCoreTestSupportCommon.props:10
> +      <AdditionalIncludeDirectories>$(ProjectDir)..;$(ProjectDir)..\Modules\mediacontrols;$(ProjectDir)..\Modules\mediastream;$(ProjectDir)..\Modules\encryptedmedia;$(ProjectDir)..\Modules\filesystem;$(ProjectDir)..\Modules\gamepad;$(ProjectDir)..\Modules\geolocation;$(ProjectDir)..\Modules\indexeddb;$(ProjectDir)..\Modules\mediasource;$(ProjectDir)..\Modules\navigatorcontentutils;$(ProjectDir)..\Modules\plugins;$(ProjectDir)..\Modules\speech;$(ProjectDir)..\Modules\proximity;$(ProjectDir)..\Modules\quota;$(ProjectDir)..\Modules\notifications;$(ProjectDir)..\Modules\streams;$(ProjectDir)..\Modules\webdatabase;$(ProjectDir)..\Modules\websockets;$(ProjectDir)..\accessibility;$(ProjectDir)..\accessibility\win;$(ProjectDir)..\bridge;$(ProjectDir)..\bridge\c;$(ProjectDir)..\bridge\jsc;$(ProjectDir)..\css;$(ProjectDir)..\cssjit;$(ProjectDir)..\editing;$(ProjectDir)..\fileapi;$(ProjectDir)..\rendering;$(ProjectDir)..\rendering\line;$(ProjectDir)..\rendering\mathml;$(ProjectDir)..\rendering\shapes;$(ProjectDir)..\rendering\style;$(ProjectDir)..\rendering\svg;$(ProjectDir)..\bindings;$(ProjectDir)..\bindings\generic;$(ProjectDir)..\bindings\js;$(ProjectDir)..\bindings\js\specialization;$(ProjectDir)..\dom;$(ProjectDir)..\dom\default;$(ProjectDir)..\history;$(ProjectDir)..\html;$(ProjectDir)..\html\canvas;$(ProjectDir)..\html\forms;$(ProjectDir)..\html\parser;$(ProjectDir)..\html\shadow;$(ProjectDir)..\html\track;$(ProjectDir)..\inspector;$(ProjectDir)..\loader;$(ProjectDir)..\loader\appcache;$(ProjectDir)..\loader\archive;$(ProjectDir)..\loader\archive\cf;$(ProjectDir)..\loader\cache;$(ProjectDir)..\loader\icon;$(ProjectDir)..\mathml;$(ProjectDir)..\page;$(ProjectDir)..\page\animation;$(ProjectDir)..\page\scrolling;$(ProjectDir)..\page\win;$(ProjectDir)..\platform;$(ProjectDir)..\platform\animation;$(ProjectDir)..\platform\audio;$(ProjectDir)..\platform\mock;$(ProjectDir)..\platform\sql;$(ProjectDir)..\platform\win;$(ProjectDir)..\platform\network;$(ProjectDir)..\platform\network\win;$(ProjectDir)..\platform\cf;$(ProjectDir)..\platform\graphics;$(ProjectDir)..\platform\graphics\ca;$(ProjectDir)..\platform\graphics\cpu\arm\filters;$(ProjectDir)..\platform\graphics\filters;$(ProjectDir)..\platform\graphics\filters\arm;$(ProjectDir)..\platform\graphics\opentype;$(ProjectDir)..\platform\graphics\transforms;$(ProjectDir)..\platform\text;$(ProjectDir)..\platform\text\icu;$(ProjectDir)..\platform\text\transcoder;$(ProjectDir)..\platform\graphics\win;$(ProjectDir)..\xml;$(ProjectDir)..\xml\parser;$(ConfigurationBuildDir)\obj$(PlatformArchitecture)\WebCore\DerivedSources;$(ProjectDir)..\plugins;$(ProjectDir)..\plugins\win;$(ProjectDir)..\replay;$(ProjectDir)..\svg\animation;$(ProjectDir)..\svg\graphics;$(ProjectDir)..\svg\properties;$(ProjectDir)..\svg\graphics\filters;$(ProjectDir)..\svg;$(ProjectDir)..\testing;$(ProjectDir)..\crypto;$(ProjectDir)..\crypto\keys;$(ProjectDir)..\wml;$(ProjectDir)..\storage;$(ProjectDir)..\style;$(ProjectDir)..\websockets;$(ProjectDir)..\workers;$(ConfigurationBuildDir)\include;$(ConfigurationBuildDir)\include\private;$(ConfigurationBuildDir)\include\JavaScriptCore;$(ConfigurationBuildDir)\include\private\JavaScriptCore;$(ProjectDir)..\ForwardingHeaders;$(ProjectDir)..\platform\graphics\gpu;$(ProjectDir)..\platform\graphics\egl;$(ProjectDir)..\platform\graphics\surfaces;$(ProjectDir)..\platform\graphics\surfaces\egl;$(ProjectDir)..\platform\graphics\opengl;$(WebKit_Libraries)\include;$(WebKit_Libraries)\include\private;$(WebKit_Libraries)\include\private\JavaScriptCore;$(WebKit_Libraries)\include\sqlite;$(WebKit_Libraries)\include\JavaScriptCore;$(WebKit_Libraries)\include\zlib;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>

I don't think it's a good idea to duplicate these lists of include directories.  Import a property sheet that already has it.

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:222
> +my %testSupportClasses = (

Put one of the bindings tests names in here so we can test that this works, then update that bindings test.

> Source/WebCore/page/DOMWindow.cpp:84
> +#include "Profile.h"

I had to include a few extra things like this, too.  I'm not sure why.

[Brent Fulgham]

Created attachment 247377 [details]
Patch

[WebKit Commit Bot]

Attachment 247377 [details] did not pass style-queue:


ERROR: Source/WebCore/testing/js/WebCoreTestSupportPrefix.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebCore/testing/js/WebCoreTestSupportPrefix.h:126:  "windows.h" already included at Source/WebCore/testing/js/WebCoreTestSupportPrefix.h:105  [build/include] [4]
ERROR: Tools/DumpRenderTree/win/DumpRenderTreePrefix.cpp:28:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 3 in 53 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alex Christensen]

r=me once this passes ews

[Brent Fulgham]

Created attachment 247386 [details]
Patch

[Brent Fulgham]

Created attachment 247387 [details]
Patch

[WebKit Commit Bot]

Attachment 247387 [details] did not pass style-queue:


ERROR: Source/WebCore/testing/js/WebCoreTestSupportPrefix.h:126:  "windows.h" already included at Source/WebCore/testing/js/WebCoreTestSupportPrefix.h:105  [build/include] [4]
ERROR: Source/WebCore/testing/js/WebCoreTestSupportPrefix.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Tools/DumpRenderTree/win/DumpRenderTreePrefix.cpp:28:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 3 in 59 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brent Fulgham]

Committed r180653: <http://trac.webkit.org/changeset/180653>

[Myles C. Maxfield]

YYYAAAAAAYYYYYYYYY

[Andreas Kling]

OMG is this real life?

[Brent Fulgham]

It's happening!

We should all give Alex a big thanks for getting all the parts in place so that this could finally happen.

[Csaba Osztrogonác]

(In reply to comment #18)
> Committed r180653: <http://trac.webkit.org/changeset/180653>

It broke the build on Apple's WinCairo buildbot.

[Brent Fulgham]

(In reply to comment #22)
> (In reply to comment #18)
> > Committed r180653: <http://trac.webkit.org/changeset/180653>
> 
> It broke the build on Apple's WinCairo buildbot.

It looks like WinCairo builds a TestWebKitAPI file that the Apple Windows port does not. Patch coming...

[Brent Fulgham]

Speculative build fix for WinCairo in r180677 <https://trac.webkit.org/changeset/180677>.

[Csaba Osztrogonác]

(In reply to comment #24)
> Speculative build fix for WinCairo in r180677
> <https://trac.webkit.org/changeset/180677>.

The build still fails with the same error message.

[Brent Fulgham]

Alex: My proposed fix didn't seem to correct the problem. If you have access to that machine can you see what's going on?

[Alex Christensen]

http://trac.webkit.org/changeset/180781 should have fixed WinCairo.