Bug 131024 – [UNIX] Reorganize and cleanup main functions of GTK and EFL ports

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Carlos Garcia Campos]

We currently have main functions in Source/WebKit2/gtk/MainGtk.cpp and Source/WebKit2/gtk/MainEfl.cpp for the WebProcess and Source/WebKit2/unix/NetworkMainUnix.cpp and Source/WebKit2/unix/PluginMainUnix.cpp for the network and plugin processes. We should move main functions to WebProcess, NetworkProcess and PluginProcess dirs. Also, in some cases like the NetworkProcess we share the main function, but we need a lot of ifdefs for the platform specific initializations, and the common code is indeed common to all other processes. We could move the common code to Share and use it for all the child processes, with an interface to add the platform specific initializations.

[Carlos Garcia Campos]

Created attachment 228260 [details]
Patch

[Carlos Garcia Campos]

hmm, I think this approach doesn't work, I'm trying a new one

[Carlos Garcia Campos]

Created attachment 228267 [details]
Updated patch

Slightly different approach to ensure that main() functions only depend on a public symbol in the libs like current code does.

[WebKit Commit Bot]

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


ERROR: Source/WebKit2/NetworkProcess/efl/NetworkProcessMainEfl.cpp:33:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebKit2/NetworkProcess/gtk/NetworkProcessMainGtk.cpp:33:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebKit2/WebProcess/efl/WebProcessMainEfl.cpp:29:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit2/WebProcess/efl/WebProcessMainEfl.cpp:30:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebKit2/WebProcess/gtk/WebProcessMainGtk.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit2/WebProcess/gtk/WebProcessMainGtk.cpp:31:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 6 in 16 files


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

[Carlos Garcia Campos]

Created attachment 228269 [details]
Try to fix EFL build

Use WK_EXPORT instead of WTF_EXPORT

[WebKit Commit Bot]

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


ERROR: Source/WebKit2/NetworkProcess/efl/NetworkProcessMainEfl.cpp:33:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebKit2/NetworkProcess/gtk/NetworkProcessMainGtk.cpp:33:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebKit2/WebProcess/efl/WebProcessMainEfl.cpp:29:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit2/WebProcess/efl/WebProcessMainEfl.cpp:30:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebKit2/WebProcess/gtk/WebProcessMainGtk.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit2/WebProcess/gtk/WebProcessMainGtk.cpp:31:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 6 in 16 files


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

[Gustavo Noronha (kov)]

Comment on attachment 228269 [details]
Try to fix EFL build

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

I'm unsure how much complexity and code this saves, it doesn't look like a lot, but I you're probably a better judge of that than me. One thing that bothers me is that in theory we could use these same implementations for Gtk on Windows, say, but I guess Unix is still sensible as a name since we'd be using GNU libraries even on windows, so r=me.

> Source/WebKit2/ChangeLog:8
> +        Move main function implementation files to <process-dir>/EntryPoint/unix/ProcessNameMain.cpp

add <> around ProcessName to be clearer?

[Carlos Garcia Campos]

(In reply to comment #7)
> (From update of attachment 228269 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=228269&action=review

Thanks for the review.

> I'm unsure how much complexity and code this saves, it doesn't look like a lot, but I you're probably a better judge of that than me. One thing that bothers me is that in theory we could use these same implementations for Gtk on Windows, say, but I guess Unix is still sensible as a name since we'd be using GNU libraries even on windows, so r=me.

Well, this is more about keeping consistency than saving code. When I moved the NEWS file to Source/WebKit2/gtk I noticed there was a MainGtk.cpp file there. And I thought Main what? it ended up being the main function of the WebProcess. When we only had one process, it probably was less confusing, but still misplaced, it should be in Source/WebKit2/WebProcess/gtk at least. Then I looked at the different main functions, and noticed that in some cases we were sharing the implementation with EFL (but with a full of ifdefs) and in others every port had its own file. So, I took a look at what apple did for the entry points, and inspired by the mac implementation I reorganized the "unix" entry points.

> > Source/WebKit2/ChangeLog:8
> > +        Move main function implementation files to <process-dir>/EntryPoint/unix/ProcessNameMain.cpp
> 
> add <> around ProcessName to be clearer?

Ok

[Carlos Garcia Campos]

Created attachment 231951 [details]
Rebased patch for landing

[WebKit Commit Bot]

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


ERROR: Source/WebKit2/NetworkProcess/efl/NetworkProcessMainEfl.cpp:33:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebKit2/NetworkProcess/gtk/NetworkProcessMainGtk.cpp:33:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebKit2/WebProcess/efl/WebProcessMainEfl.cpp:29:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit2/WebProcess/efl/WebProcessMainEfl.cpp:30:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/WebKit2/WebProcess/gtk/WebProcessMainGtk.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit2/WebProcess/gtk/WebProcessMainGtk.cpp:31:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 6 in 16 files


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

[Carlos Garcia Campos]

Committed r169255: <http://trac.webkit.org/changeset/169255>