Modified: trunk/Source/WebKit2/ChangeLog (206788 => 206789)
--- trunk/Source/WebKit2/ChangeLog 2016-10-04 22:26:44 UTC (rev 206788)
+++ trunk/Source/WebKit2/ChangeLog 2016-10-04 22:41:01 UTC (rev 206789)
@@ -1,3 +1,29 @@
+2016-10-04 Anders Carlsson <ander...@apple.com>
+
+ Properly kill web processes in the launching state
+ https://bugs.webkit.org/show_bug.cgi?id=162938
+
+ Reviewed by Tim Horton.
+
+ * UIProcess/Launcher/ProcessLauncher.h:
+ Add m_xpcConnection member.
+
+ * UIProcess/Launcher/mac/ProcessLauncherMac.mm:
+ (WebKit::ProcessLauncher::launchProcess):
+ Store the XPC connection in the m_xpcConnection member variable.
+ In the reply handler, handle m_xpcConnection being null.
+
+ (WebKit::ProcessLauncher::platformInvalidate):
+ Cancel and kill the connection.
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::terminateProcess):
+ Get rid of an assertion.
+
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::requestTermination):
+ Handle the launching state.
+
2016-10-04 Tim Horton <timothy_hor...@apple.com>
Fix the build.
Modified: trunk/Source/WebKit2/UIProcess/Launcher/ProcessLauncher.h (206788 => 206789)
--- trunk/Source/WebKit2/UIProcess/Launcher/ProcessLauncher.h 2016-10-04 22:26:44 UTC (rev 206788)
+++ trunk/Source/WebKit2/UIProcess/Launcher/ProcessLauncher.h 2016-10-04 22:41:01 UTC (rev 206789)
@@ -86,6 +86,10 @@
Client* m_client;
+#if PLATFORM(COCOA)
+ OSObjectPtr<xpc_connection_t> m_xpcConnection;
+#endif
+
WeakPtrFactory<ProcessLauncher> m_weakPtrFactory;
const LaunchOptions m_launchOptions;
bool m_isLaunching;
Modified: trunk/Source/WebKit2/UIProcess/Launcher/mac/ProcessLauncherMac.mm (206788 => 206789)
--- trunk/Source/WebKit2/UIProcess/Launcher/mac/ProcessLauncherMac.mm 2016-10-04 22:26:44 UTC (rev 206788)
+++ trunk/Source/WebKit2/UIProcess/Launcher/mac/ProcessLauncherMac.mm 2016-10-04 22:41:01 UTC (rev 206789)
@@ -96,11 +96,13 @@
void ProcessLauncher::launchProcess()
{
- auto connection = adoptOSObject(xpc_connection_create(serviceName(m_launchOptions), dispatch_get_main_queue()));
+ ASSERT(!m_xpcConnection);
+ m_xpcConnection = adoptOSObject(xpc_connection_create(serviceName(m_launchOptions), dispatch_get_main_queue()));
+
uuid_t uuid;
uuid_generate(uuid);
- xpc_connection_set_oneshot_instance(connection.get(), uuid);
+ xpc_connection_set_oneshot_instance(m_xpcConnection.get(), uuid);
// Inherit UI process localization. It can be different from child process default localization:
// 1. When the application and system frameworks simply have different localized resources available, we should match the application.
@@ -130,12 +132,12 @@
xpc_dictionary_set_value(initializationMessage.get(), "OverrideLanguages", languages.get());
}
- xpc_connection_set_bootstrap(connection.get(), initializationMessage.get());
+ xpc_connection_set_bootstrap(m_xpcConnection.get(), initializationMessage.get());
if (shouldLeakBoost(m_launchOptions)) {
auto preBootstrapMessage = adoptOSObject(xpc_dictionary_create(nullptr, nullptr, 0));
xpc_dictionary_set_string(preBootstrapMessage.get(), "message-name", "pre-bootstrap");
- xpc_connection_send_message(connection.get(), preBootstrapMessage.get());
+ xpc_connection_send_message(m_xpcConnection.get(), preBootstrapMessage.get());
}
// Create the listening port.
@@ -180,7 +182,7 @@
xpc_dictionary_set_value(bootstrapMessage.get(), "extra-initialization-data", extraInitializationData.get());
auto weakProcessLauncher = m_weakPtrFactory.createWeakPtr();
- xpc_connection_set_event_handler(connection.get(), [weakProcessLauncher, listeningPort](xpc_object_t event) {
+ xpc_connection_set_event_handler(m_xpcConnection.get(), [weakProcessLauncher, listeningPort](xpc_object_t event) {
ASSERT(xpc_get_type(event) == XPC_TYPE_ERROR);
auto processLauncher = weakProcessLauncher.get();
@@ -195,22 +197,32 @@
// And the receive right.
mach_port_mod_refs(mach_task_self(), listeningPort, MACH_PORT_RIGHT_RECEIVE, -1);
+
+ processLauncher->m_xpcConnection = nullptr;
+
processLauncher->didFinishLaunchingProcess(0, IPC::Connection::Identifier());
});
- xpc_connection_resume(connection.get());
+ xpc_connection_resume(m_xpcConnection.get());
ref();
- xpc_connection_send_message_with_reply(connection.get(), bootstrapMessage.get(), dispatch_get_main_queue(), ^(xpc_object_t reply) {
+ xpc_connection_send_message_with_reply(m_xpcConnection.get(), bootstrapMessage.get(), dispatch_get_main_queue(), ^(xpc_object_t reply) {
// Errors are handled in the event handler.
if (xpc_get_type(reply) != XPC_TYPE_ERROR) {
ASSERT(xpc_get_type(reply) == XPC_TYPE_DICTIONARY);
ASSERT(!strcmp(xpc_dictionary_get_string(reply, "message-name"), "process-finished-launching"));
+ if (!m_xpcConnection) {
+ // The process was terminated.
+ didFinishLaunchingProcess(0, IPC::Connection::Identifier());
+ return;
+ }
+
// The process has finished launching, grab the pid from the connection.
- pid_t processIdentifier = xpc_connection_get_pid(connection.get());
+ pid_t processIdentifier = xpc_connection_get_pid(m_xpcConnection.get());
- didFinishLaunchingProcess(processIdentifier, IPC::Connection::Identifier(listeningPort, connection));
+ didFinishLaunchingProcess(processIdentifier, IPC::Connection::Identifier(listeningPort, m_xpcConnection));
+ m_xpcConnection = nullptr;
}
deref();
@@ -233,6 +245,12 @@
void ProcessLauncher::platformInvalidate()
{
+ if (!m_xpcConnection)
+ return;
+
+ xpc_connection_cancel(m_xpcConnection.get());
+ xpc_connection_kill(m_xpcConnection.get(), SIGKILL);
+ m_xpcConnection = nullptr;
}
} // namespace WebKit
Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp (206788 => 206789)
--- trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp 2016-10-04 22:26:44 UTC (rev 206788)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp 2016-10-04 22:41:01 UTC (rev 206789)
@@ -2351,11 +2351,6 @@
void WebPageProxy::terminateProcess()
{
- // requestTermination() is a no-op for launching processes, so we get into an inconsistent state by calling resetStateAfterProcessExited().
- // FIXME: A client can terminate the page at any time, so we should do something more meaningful than assert and fall apart in release builds.
- // See also <https://bugs.webkit.org/show_bug.cgi?id=136012>.
- ASSERT(m_process->state() != WebProcessProxy::State::Launching);
-
// NOTE: This uses a check of m_isValid rather than calling isValid() since
// we want this to run even for pages being closed or that already closed.
if (!m_isValid)
Modified: trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp (206788 => 206789)
--- trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp 2016-10-04 22:26:44 UTC (rev 206788)
+++ trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp 2016-10-04 22:41:01 UTC (rev 206789)
@@ -768,7 +768,7 @@
void WebProcessProxy::requestTermination()
{
- if (state() != State::Running)
+ if (state() == State::Terminated)
return;
ChildProcessProxy::terminate();
Modified: trunk/Tools/ChangeLog (206788 => 206789)
--- trunk/Tools/ChangeLog 2016-10-04 22:26:44 UTC (rev 206788)
+++ trunk/Tools/ChangeLog 2016-10-04 22:41:01 UTC (rev 206789)
@@ -1,3 +1,13 @@
+2016-10-04 Anders Carlsson <ander...@apple.com>
+
+ Properly kill web processes in the launching state
+ https://bugs.webkit.org/show_bug.cgi?id=162938
+
+ Reviewed by Tim Horton.
+
+ * TestWebKitAPI/Tests/WebKit2/TerminateTwice.cpp:
+ Enable this test again.
+
2016-10-04 Alex Christensen <achristen...@webkit.org>
URLParser: query-only URLs relative to file URLs should just add a query
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit2/TerminateTwice.cpp (206788 => 206789)
--- trunk/Tools/TestWebKitAPI/Tests/WebKit2/TerminateTwice.cpp 2016-10-04 22:26:44 UTC (rev 206788)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2/TerminateTwice.cpp 2016-10-04 22:41:01 UTC (rev 206789)
@@ -32,9 +32,6 @@
namespace TestWebKitAPI {
-// Disabled in debug mode while investigating <https://bugs.webkit.org/show_bug.cgi?id=136012>.
-#ifdef NDEBUG
-
static bool loaded;
static void didFinishLoadForFrame(WKPageRef page, WKFrameRef frame, WKTypeRef userData, const void* clientInfo)
@@ -68,8 +65,6 @@
WKPageTerminate(webView.page());
}
-#endif
-
} // namespace TestWebKitAPI
#endif
