Bug 136172

Summary: get rid of WebCore.exp.in
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: WebCore Misc.Assignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: alex.christensen, burg, commit-queue, darin, dbates, mitz, simon.fraser, thorton
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 139936, 141409, 141413, 141491    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
darin: review-
Bring the state of WEBCORE_EXPORT annotations closer to what the exports file specifies darin: review+

Description Alex Christensen 2014-08-22 14:56:49 PDT
IT'S ALIIIIVE!

I'm not going to be able to finish this, so someone else is going to need to polish this off.  This compiles all of open source WebKit.  I haven't tested iOS.
Comment 1 Alex Christensen 2014-08-22 15:03:20 PDT
Created attachment 237001 [details]
Patch
Comment 2 Alex Christensen 2014-08-22 15:25:28 PDT
Created attachment 237002 [details]
Patch
Comment 3 Darin Adler 2014-08-24 11:48:29 PDT
Comment on attachment 237002 [details]
Patch

This needs to be landed in pieces.

1) Land all the parts of this that add in or remove WEBCORE_EXPORT. Except possibly for the ones in npruntime_impl.h since that is an imported header file that we generally wish not to modify. Maybe that’s not a big deal any more, but that feels like an open question to me at this time.

2) Construct the smallest possible “throw the switch” patch, *not* the full “clean up all the remnants” version. Run the nm command before and after this patch to make a diff. If the diff is acceptable, then land that patch.

3) Follow up with the cleanup patch that removes all the remnants of the old system. This step includes deleting the actual export files, and even the sort-export-file script and the code we recently added to check-webkit-style to invoke it.

Someone should come up with another patch for the next piece of (1). As long as it only adds a macro that has no effect, I think it can be a huge patch and we need only review it to make sure we don’t accidentally include other changes.
Comment 4 Alex Christensen 2014-08-27 18:26:09 PDT
Someone with access to the iOS build needs to take it from here. I do not plan to land this myself. Tim?
Comment 5 Darin Adler 2014-09-02 09:32:14 PDT
Landed a big piece of the patch as http://trac.webkit.org/changeset/173176

Didn’t touch WebCoreSystemInterface.h because WEBCORE_EXPORT is defined as empty string, not extern, so it wouldn’t handle those declarations properly.

Also didn’t touch npruntime_impl.h yet, because I am not sure that’s truly an internal WebCore header used only by WebKit; the naming of the header implies that it’s somehow a public header shared with other browsers that implement the Netscape Plug-in API, but I suspect that is not true. Might be nice to rename this header to reduce confusion on that point.
Comment 6 Simon Fraser (smfr) 2014-09-02 11:12:37 PDT
This broke bindings tests.
Comment 7 Alex Christensen 2014-09-02 22:33:02 PDT
bindings tests fixed in http://trac.webkit.org/changeset/173182
Comment 8 Darin Adler 2014-09-03 23:30:25 PDT
Thanks for fixing that. I should not have overlooked the bindings tests!
Comment 9 mitz 2014-12-24 23:21:13 PST
Created attachment 243741 [details]
Bring the state of WEBCORE_EXPORT annotations closer to what the exports file specifies

This patch brings the state of what’s exported when not using the exports file closer to what’s currently being exported. It avoids touching the two headers that Darin was concerned about, npruntime_impl.h and WebCoreSystemInterface.h, instead adjusting the visibility in the implementation files.

The only missing symbols after this patch are ones coming from generated files.
Comment 10 WebKit Commit Bot 2014-12-24 23:23:35 PST
Attachment 243741 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/SuddenTermination.h:36:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebCore/bridge/npruntime.cpp:41:  Bad include order. Mixing system and custom headers.  [build/include_order] [4]
ERROR: Source/WebCore/bridge/NP_jsobject.cpp:49:  Bad include order. Mixing system and custom headers.  [build/include_order] [4]
ERROR: Source/WebCore/platform/graphics/mac/ColorMac.h:42:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 4 in 55 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 mitz 2014-12-25 13:25:56 PST
Committed attachment 243741 [details] as <http://trac.webkit.org/r177739>.
Comment 12 Alex Christensen 2015-03-12 17:30:13 PDT
This was finished in https://bugs.webkit.org/show_bug.cgi?id=141491