Make usesNetworkProcess always true
Created attachment 266114 [details] Patch
Created attachment 266116 [details] Patch
Created attachment 266117 [details] Patch
Created attachment 266119 [details] Patch
Comment on attachment 266119 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=266119&action=review > Source/WebKit2/Shared/WebProcessCreationParameters.cpp:-100 > encoder << cookiePersistentStoragePath; > encoder << cookiePersistentStorageType; > encoder.encodeEnum(cookieAcceptPolicy); > - encoder << ignoreTLSErrors; I think we can remove all other network parameters as well. At least diskCacheDirectory, cookiePersistentStoragePath, cookiePersistentStorageType, cookieAcceptPolicy and urlSchemesRegisteredForCustomProtocols > Source/WebKit2/Shared/Network/CustomProtocols/Cocoa/CustomProtocolManagerCocoa.mm:140 > void CustomProtocolManager::initialize(const WebProcessCreationParameters& parameters) > { > - ASSERT(parameters.urlSchemesRegisteredForCustomProtocols.isEmpty() || !parameters.usesNetworkProcess); > - if (parameters.usesNetworkProcess) { > - m_childProcess->parentProcessConnection()->removeWorkQueueMessageReceiver(Messages::CustomProtocolManager::messageReceiverName()); > - m_messageQueue = nullptr; > - return; > - } > - > - [NSURLProtocol registerClass:[WKCustomProtocol class]]; > - > - for (size_t i = 0; i < parameters.urlSchemesRegisteredForCustomProtocols.size(); ++i) > - registerScheme(parameters.urlSchemesRegisteredForCustomProtocols[i]); > + ASSERT(parameters.urlSchemesRegisteredForCustomProtocols.isEmpty()); > + m_childProcess->parentProcessConnection()->removeWorkQueueMessageReceiver(Messages::CustomProtocolManager::messageReceiverName()); > + m_messageQueue = nullptr; > } I think we should remove this entirely and make CustomProtocolManager a NetworkProcessSupplement only.
Comment on attachment 266119 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=266119&action=review >> Source/WebKit2/Shared/WebProcessCreationParameters.cpp:-100 >> - encoder << ignoreTLSErrors; > > I think we can remove all other network parameters as well. At least diskCacheDirectory, cookiePersistentStoragePath, cookiePersistentStorageType, cookieAcceptPolicy and urlSchemesRegisteredForCustomProtocols Should return and delete even more of this unused stuff. > Source/WebKit2/UIProcess/WebProcessPool.cpp:484 > + if (networkProcess()) { I don't understand why this file mixes networkProcess() and m_networkProcess. > Source/WebKit2/UIProcess/WebProcessPool.cpp:1011 > + ASSERT(m_networkProcess); Assertion seems like overkill with nearly no value. > Source/WebKit2/UIProcess/WebProcessPool.cpp:1012 > + return m_networkProcess->createDownloadProxy(request); Should just use the return value of ensureNetworkProcess.
Created attachment 266141 [details] Patch
(In reply to comment #7) > Created attachment 266141 [details] > Patch I addressed comments. I plan to land this after Thanksgiving. I'll be removing more code after that, too.
Comment on attachment 266141 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=266141&action=review > Source/WebKit2/Shared/Network/CustomProtocols/soup/CustomProtocolManagerSoup.cpp:54 > void CustomProtocolManager::initialize(const WebProcessCreationParameters& parameters) > { > - ASSERT(parameters.urlSchemesRegisteredForCustomProtocols.isEmpty() || !parameters.usesNetworkProcess); > - if (parameters.usesNetworkProcess) { > - m_childProcess->parentProcessConnection()->removeWorkQueueMessageReceiver(Messages::CustomProtocolManager::messageReceiverName()); > - m_messageQueue = nullptr; > - return; > - } > - for (size_t i = 0; i < parameters.urlSchemesRegisteredForCustomProtocols.size(); ++i) > - registerScheme(parameters.urlSchemesRegisteredForCustomProtocols[i]); > + ASSERT(parameters.urlSchemesRegisteredForCustomProtocols.isEmpty()); > + m_childProcess->parentProcessConnection()->removeWorkQueueMessageReceiver(Messages::CustomProtocolManager::messageReceiverName()); > + m_messageQueue = nullptr; > } This should be removed entirely as well, please do it before landing.
Comment on attachment 266141 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=266141&action=review > Source/WebKit2/UIProcess/efl/WebProcessPoolEfl.cpp:97 > parameters.urlSchemesRegisteredForCustomProtocols = supplement<WebSoupCustomProtocolRequestManager>()->registeredSchemesForCustomProtocols(); > supplement<WebCookieManagerProxy>()->getCookiePersistentStorage(parameters.cookiePersistentStoragePath, parameters.cookiePersistentStorageType); > parameters.cookieAcceptPolicy = m_initialHTTPCookieAcceptPolicy; > - parameters.ignoreTLSErrors = m_ignoreTLSErrors; > parameters.diskCacheDirectory = m_configuration->diskCacheDirectory(); > } I think these lines should be removed too to fix the EFL build. ../../Source/WebKit2/UIProcess/efl/WebProcessPoolEfl.cpp:93:16: error: 'struct WebKit::WebProcessCreationParameters' has no member named 'urlSchemesRegisteredForCustomProtocols' ../../Source/WebKit2/UIProcess/efl/WebProcessPoolEfl.cpp:94:80: error: 'struct WebKit::WebProcessCreationParameters' has no member named 'cookiePersistentStoragePath' ../../Source/WebKit2/UIProcess/efl/WebProcessPoolEfl.cpp:94:120: error: 'struct WebKit::WebProcessCreationParameters' has no member named 'cookiePersistentStorageType' ../../Source/WebKit2/UIProcess/efl/WebProcessPoolEfl.cpp:95:16: error: 'struct WebKit::WebProcessCreationParameters' has no member named 'cookieAcceptPolicy' ../../Source/WebKit2/UIProcess/efl/WebProcessPoolEfl.cpp:96:16: error: 'struct WebKit::WebProcessCreationParameters' has no member named 'diskCacheDirectory'
Created attachment 266244 [details] EFL buildfix This small patch fixed the EFL build for me, feel free to pick it up.
(In reply to comment #11) > Created attachment 266244 [details] > EFL buildfix > > This small patch fixed the EFL build for me, feel free to pick it up. But unfortunately tests are still failing with timeout as I mentioned it previously: https://bugs.webkit.org/show_bug.cgi?id=151418#c18
(In reply to comment #12) > (In reply to comment #11) > > Created attachment 266244 [details] > > EFL buildfix > > > > This small patch fixed the EFL build for me, feel free to pick it up. > > But unfortunately tests are still failing with timeout as I mentioned > it previously: https://bugs.webkit.org/show_bug.cgi?id=151418#c18 It is a 15 months old regression, see bug137692 for details. It means that EFL port didn't test enabled NetworkProcess at all nowadays. It's not so good. :-/
(In reply to comment #13) > (In reply to comment #12) > > (In reply to comment #11) > > > Created attachment 266244 [details] > > > EFL buildfix > > > > > > This small patch fixed the EFL build for me, feel free to pick it up. > > > > But unfortunately tests are still failing with timeout as I mentioned > > it previously: https://bugs.webkit.org/show_bug.cgi?id=151418#c18 > > It is a 15 months old regression, see bug137692 for details. It means that > EFL > port didn't test enabled NetworkProcess at all nowadays. It's not so good. > :-/ Looking at the patch proposed, it seems to me that it could be an integer overflow in a timer delay due to the std::chrono::milliseconds::max(). We had similar issues with std::chrono::microseconds::max(). The proposed patch seems to be more a workaround than a real fix.
Carlos and Ossy, thanks for looking into the build failures. http://trac.webkit.org/changeset/192796
(In reply to comment #15) > Carlos and Ossy, thanks for looking into the build failures. > http://trac.webkit.org/changeset/192796 It made Networking completely broken on EFL - bug137692 . And it made many http tests timeout on GTK too: - before: https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Tests%29/builds/12445 - after: https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Tests%29/builds/12446
We never tested the network process until now, so the GTK tests are probably only now catching bugs that have always existed. Let's not block on those.
(In reply to comment #17) > We never tested the network process until now, so the GTK tests are probably > only now catching bugs that have always existed. Let's not block on those. This is not true, we have run the layout tests with the network process enabled for long time, since r175776, see bug #138428
(In reply to comment #16) > (In reply to comment #15) > > Carlos and Ossy, thanks for looking into the build failures. > > http://trac.webkit.org/changeset/192796 > > It made Networking completely broken on EFL - bug137692 . > And it made many http tests timeout on GTK too: > - before: > https://build.webkit.org/builders/GTK%20Linux%2064- > bit%20Release%20%28Tests%29/builds/12445 > - after: > https://build.webkit.org/builders/GTK%20Linux%2064- > bit%20Release%20%28Tests%29/builds/12446 GTK+ failures were because r192796 removed the m_ignoreTLSErrors initialization from WebProcessPool by mistake. Those tests use HTTPS and loads failed due to invalid certificate issues. It should be fixed now in r192885
<rdar://problem/24558287>