Bug 110136

Created attachment 188912 [details] Patch

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

Created attachment 205779 [details] Patch

(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++.

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?

(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.

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

Created attachment 205987 [details] Patch

Comment on attachment 205987 [details] Patch Seems fine. Once a platform knowledgeable reviewer signs off, so will I.

CC'ing knowledgeable reviewer Gustavo Gustavo Noronha Silva.

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.

Created attachment 206822 [details] Patch

(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.

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.

(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.

(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.

Comment on attachment 206822 [details] Patch Clearing flags on attachment: 206822 Committed r153106: <http://trac.webkit.org/changeset/153106>

All reviewed patches have been landed. Closing bug.