Bug 110136 – [WK2][Soup] Add entry point for network process

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

[Balazs Kelemen]

Add the entry point what will be used by the executable.

[Balazs Kelemen]

Created attachment 188912 [details]
Patch

[Build Bot]

Comment on attachment 188912 [details]
Patch

Attachment 188912 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16625201

New failing tests:
media/video-controls-captions-trackmenu.html

[Kwang Yul Seo]

Created attachment 205779 [details]
Patch

[Kwang Yul Seo]

(In reply to comment #3)
I made two changes to the original patch:
1. Include unix/NetworkMainUnix.cpp here.
2. Wrap NetworkProcessMain declaration with extern "C" to prevent name mangling in C++.

[Carlos Garcia Campos]

Comment on attachment 205779 [details]
Patch

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

> Source/WebKit2/NetworkProcess/unix/NetworkProcessMainUnix.cpp:56
> +#if PLATFORM(GTK)
> +    gtk_init(&argc, &argv);
> +#endif

Why do we need GTK+ in the network process?

[Kwang Yul Seo]

(In reply to comment #5)
> Why do we need GTK+ in the network process?
Not needed. I will change it to use Glib event loop.

[Kwang Yul Seo]

(In reply to comment #6)
Because Gtk is already using glib-only run loop implementation, it seems we can simply drop gtk_init() here.

[Kwang Yul Seo]

Created attachment 205987 [details]
Patch

[Brady Eidson]

Comment on attachment 205987 [details]
Patch

Seems fine.  Once a platform knowledgeable reviewer signs off, so will I.

[Kwang Yul Seo]

CC'ing knowledgeable reviewer Gustavo Gustavo Noronha Silva.

[Gustavo Noronha (kov)]

Comment on attachment 205987 [details]
Patch

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

> Source/WebKit2/NetworkProcess/unix/NetworkProcessMainUnix.cpp:51
> +    // FIXME: Event loop is not initialized correctly for EFL. For EFL we should should either
> +    // initialize ecore or use the glib-only RunLoop implementation in the network process.

Why is it not initialized correctly? I guess you need to call ecore_main_loop_glib_integrate() before calling InitializeWebKit2?

> Source/WebKit2/NetworkProcess/unix/NetworkProcessMainUnix.cpp:63
> +    errno = 0;
> +    long int socket = strtol(argv[1], 0, 10);
> +    if (errno)
> +        return 1;
> +
> +    WebKit::ChildProcessInitializationParameters parameters;
> +    parameters.connectionIdentifier = int(socket);

On other entry points we use atoi() to go straight to int and avoid this error checking and conversion dance. I would prefer if we kept this consistent.

> Source/WebKit2/unix/NetworkMainUnix.cpp:2
> + * Copyright (C) 2013 University of Szeged. All rights reserved.

This is trivial enough that I don't think you need to import the copyright.

[Kwang Yul Seo]

Created attachment 206822 [details]
Patch

[Kwang Yul Seo]

(In reply to comment #11)
> (From update of attachment 205987 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=205987&action=review
> 
> Why is it not initialized correctly? I guess you need to call ecore_main_loop_glib_integrate() before calling InitializeWebKit2?

Done. I will check if this is correct with one of EFL maintainers.

> On other entry points we use atoi() to go straight to int and avoid this error checking and conversion dance. I would prefer if we kept this consistent.

Done.
 
> > Source/WebKit2/unix/NetworkMainUnix.cpp:2
> > + * Copyright (C) 2013 University of Szeged. All rights reserved.
> 
> This is trivial enough that I don't think you need to import the copyright.

This copyright line is from Balazs Kelemen's original patch. I kept the license intact as he is the original author.

[WebKit Commit Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/NetworkProcess/unix/NetworkProcessMainUnix.cpp', u'Source/WebKit2/NetworkProcess/unix/NetworkProcessMainUnix.h', u'Source/WebKit2/unix/NetworkMainUnix.cpp']" exit_code: 1
Source/WebKit2/NetworkProcess/unix/NetworkProcessMainUnix.cpp:45:  Do not use 'using namespace WebCore;'.  [build/using_namespace] [4]
Total errors found: 1 in 4 files


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

[Kwang Yul Seo]

(In reply to comment #14)
> If any of these errors are false positives, please file a bug against check-webkit-style.

I filed Bug 118755 to fix this.

[Balazs Kelemen]

(In reply to comment #11)
> (From update of attachment 205987 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=205987&action=review
> 
> > Source/WebKit2/NetworkProcess/unix/NetworkProcessMainUnix.cpp:51
> > +    // FIXME: Event loop is not initialized correctly for EFL. For EFL we should should either
> > +    // initialize ecore or use the glib-only RunLoop implementation in the network process.
> 
> Why is it not initialized correctly? I guess you need to call ecore_main_loop_glib_integrate() before calling InitializeWebKit2?
 
When I was working on this I was told that it is unfortunate that efl is using it's own event loop with glib integration instead of just using glib. For the latter to work one needs to add a build target (a static lib) that includes the glib implementation of RunLoop (RunLoopGtk) and it's dependencies and use it for the network process. Similarly it could be used for the plugin process.

I think it does not belong to this work so it is fine to land this now and postpone the problem.

[Kwang Yul Seo]

Comment on attachment 206822 [details]
Patch

Clearing flags on attachment: 206822

Committed r153106: <http://trac.webkit.org/changeset/153106>

[Kwang Yul Seo]

All reviewed patches have been landed.  Closing bug.