Bug 141734

Summary: use WEBCORE_EXPORT on Windows
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: WebCore Misc.Assignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, burg, commit-queue, kling, mmaxfield, ossy, thechad722
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows 7   
Bug Depends on: 141735    
Bug Blocks: 142035    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch achristensen: review+

Description Alex Christensen 2015-02-17 18:48:40 PST
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.
Comment 1 Alex Christensen 2015-02-17 19:00:14 PST
(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.
Comment 2 Brent Fulgham 2015-02-25 16:38:35 PST
*** Bug 141720 has been marked as a duplicate of this bug. ***
Comment 3 Brent Fulgham 2015-02-25 17:02:23 PST
Created attachment 247367 [details]
Patch
Comment 4 WebKit Commit Bot 2015-02-25 17:03:49 PST
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.
Comment 5 Brent Fulgham 2015-02-25 17:12:30 PST
Created attachment 247370 [details]
Patch
Comment 6 WebKit Commit Bot 2015-02-25 17:15:02 PST
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.
Comment 7 Brent Fulgham 2015-02-25 17:31:06 PST
Created attachment 247372 [details]
Patch
Comment 8 WebKit Commit Bot 2015-02-25 17:32:55 PST
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.
Comment 9 Brent Fulgham 2015-02-25 17:45:03 PST
Created attachment 247373 [details]
Patch
Comment 10 WebKit Commit Bot 2015-02-25 17:46:02 PST
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.
Comment 11 Alex Christensen 2015-02-25 17:59:41 PST
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.
Comment 12 Brent Fulgham 2015-02-25 18:00:59 PST
Created attachment 247377 [details]
Patch
Comment 13 WebKit Commit Bot 2015-02-25 18:02:27 PST
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.
Comment 14 Alex Christensen 2015-02-25 18:22:02 PST
r=me once this passes ews
Comment 15 Brent Fulgham 2015-02-25 18:34:36 PST
Created attachment 247386 [details]
Patch
Comment 16 Brent Fulgham 2015-02-25 18:36:54 PST
Created attachment 247387 [details]
Patch
Comment 17 WebKit Commit Bot 2015-02-25 18:39:29 PST
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.
Comment 18 Brent Fulgham 2015-02-25 20:04:29 PST
Committed r180653: <http://trac.webkit.org/changeset/180653>
Comment 19 Myles C. Maxfield 2015-02-25 20:31:35 PST
YYYAAAAAAYYYYYYYYY
Comment 20 Andreas Kling 2015-02-25 22:37:31 PST
OMG is this real life?
Comment 21 Brent Fulgham 2015-02-25 22:43:39 PST
It's happening!

We should all give Alex a big thanks for getting all the parts in place so that this could finally happen.
Comment 22 Csaba Osztrogonác 2015-02-26 03:50:09 PST
(In reply to comment #18)
> Committed r180653: <http://trac.webkit.org/changeset/180653>

It broke the build on Apple's WinCairo buildbot.
Comment 23 Brent Fulgham 2015-02-26 08:54:12 PST
(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...
Comment 24 Brent Fulgham 2015-02-26 09:06:11 PST
Speculative build fix for WinCairo in r180677 <https://trac.webkit.org/changeset/180677>.
Comment 25 Csaba Osztrogonác 2015-02-27 12:18:05 PST
(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.
Comment 26 Brent Fulgham 2015-02-27 13:42:09 PST
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?
Comment 27 Alex Christensen 2015-02-27 14:17:53 PST
http://trac.webkit.org/changeset/180781 should have fixed WinCairo.