- Revision
- 295511
- Author
- yu...@chromium.org
- Date
- 2022-06-13 19:24:27 -0700 (Mon, 13 Jun 2022)
Log Message
[WPE][GTK] REGRESSION (r294381): WPEWebProcess leak after closing browser
https://bugs.webkit.org/show_bug.cgi?id=241353
Reviewed by Alex Christensen.
Do not keep strong reference to WebPageProxy in the async IPC callback, instead
use WeakPtr to let the page be destroyed if necessary, otherwise the page may
keep its process pool alive after the page was closed.
* Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp: exposed a couple of internal
methods for testing the behavior.
(webkitWebViewForceRepaintForTesting):
(webkitSetCachedProcessSuspensionDelayForTesting):
* Source/WebKit/UIProcess/API/glib/WebKitWebViewInternal.h:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::forceRepaint): replaced strong reference in the callback with
a weak one.
* Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitWebContext.cpp:
(testNoWebProcessLeakAfterWebKitWebContextDestroy): new test that makes sure that outstanding
async IPC callbacks are run when page and its context are destroyed.
(beforeAll):
* Tools/TestWebKitAPI/glib/WebKitGLib/WebViewTest.cpp:
(WebViewTest::waitUntilLoadFinished):
* Tools/TestWebKitAPI/glib/WebKitGLib/WebViewTest.h:
Canonical link: https://commits.webkit.org/251516@main
Modified Paths
Diff
Modified: trunk/Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp (295510 => 295511)
--- trunk/Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp 2022-06-14 01:17:18 UTC (rev 295510)
+++ trunk/Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp 2022-06-14 02:24:27 UTC (rev 295511)
@@ -5051,3 +5051,17 @@
webkitWebViewConfigureMediaCapture(webView, WebCore::MediaProducerMediaCaptureKind::Display, state);
}
+
+void webkitWebViewForceRepaintForTesting(WebKitWebView* webView, ForceRepaintCallback callback, gpointer userData)
+{
+ g_return_if_fail(WEBKIT_IS_WEB_VIEW(webView));
+
+ getPage(webView).forceRepaint([callback, userData]() {
+ callback(userData);
+ });
+}
+
+void webkitSetCachedProcessSuspensionDelayForTesting(double seconds)
+{
+ WebKit::WebsiteDataStore::setCachedProcessSuspensionDelayForTesting(Seconds(seconds));
+}
Modified: trunk/Source/WebKit/UIProcess/API/glib/WebKitWebViewInternal.h (295510 => 295511)
--- trunk/Source/WebKit/UIProcess/API/glib/WebKitWebViewInternal.h 2022-06-14 01:17:18 UTC (rev 295510)
+++ trunk/Source/WebKit/UIProcess/API/glib/WebKitWebViewInternal.h 2022-06-14 02:24:27 UTC (rev 295511)
@@ -27,3 +27,6 @@
typedef struct _WebKitWebView WebKitWebView;
WK_EXPORT void webkitWebViewRunJavascriptWithoutForcedUserGestures(WebKitWebView*, const gchar*, GCancellable*, GAsyncReadyCallback, gpointer);
+typedef void (*ForceRepaintCallback) (gpointer userData);
+WK_EXPORT void webkitWebViewForceRepaintForTesting(WebKitWebView*, ForceRepaintCallback, gpointer userData);
+WK_EXPORT void webkitSetCachedProcessSuspensionDelayForTesting(double seconds);
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (295510 => 295511)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2022-06-14 01:17:18 UTC (rev 295510)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2022-06-14 02:24:27 UTC (rev 295511)
@@ -4577,10 +4577,13 @@
m_drawingArea->waitForBackingStoreUpdateOnNextPaint();
- sendWithAsyncReply(Messages::WebPage::ForceRepaint(), [this, protectedThis = Ref { *this }, callback = WTFMove(callback)] () mutable {
- callAfterNextPresentationUpdate([callback = WTFMove(callback)] (auto) mutable {
+ sendWithAsyncReply(Messages::WebPage::ForceRepaint(), [weakThis = WeakPtr { *this }, callback = WTFMove(callback)] () mutable {
+ if (weakThis) {
+ weakThis->callAfterNextPresentationUpdate([callback = WTFMove(callback)] (auto) mutable {
+ callback();
+ });
+ } else
callback();
- });
});
}
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitWebContext.cpp (295510 => 295511)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitWebContext.cpp 2022-06-14 01:17:18 UTC (rev 295510)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitWebContext.cpp 2022-06-14 02:24:27 UTC (rev 295511)
@@ -21,6 +21,7 @@
#include "LoadTrackingTest.h"
#include "WebKitTestServer.h"
+#include "WebKitWebViewInternal.h"
#include <WebCore/SoupVersioning.h>
#include <libsoup/soup.h>
#include <limits.h>
@@ -1043,6 +1044,32 @@
g_assert_cmpstr(WebViewTest::_javascript_ResultToCString(_javascript_Result), ==, "Europe/Berlin, Europe/Berlin, Europe/Berlin, Europe/Berlin");
}
+static void testNoWebProcessLeakAfterWebKitWebContextDestroy(WebViewTest* test, gconstpointer)
+{
+ webkitSetCachedProcessSuspensionDelayForTesting(0);
+ GRefPtr<WebKitWebContext> webContext = adoptGRef(WEBKIT_WEB_CONTEXT(g_object_new(WEBKIT_TYPE_WEB_CONTEXT, nullptr)));
+ GRefPtr<WebKitWebView> webView = Test::adoptView(Test::createWebView(webContext.get()));
+ webkit_web_view_load_uri(webView.get(), kServer->getURIForPath("/").data());
+ test->waitUntilLoadFinished(webView.get());
+ bool didRunForceRepaintCallback = false;
+ webkitWebViewForceRepaintForTesting(webView.get(), [] (gpointer data) {
+ *static_cast<bool*>(data) = true;
+ }, &didRunForceRepaintCallback);
+ webView.clear();
+ // Wait for shutdownPreventingScope created in WebPageProxy::close to be destroyed.
+ while (g_main_context_pending(nullptr))
+ g_main_context_iteration(nullptr, TRUE);
+ webContext.clear();
+ while (!didRunForceRepaintCallback)
+ test->wait(0.1);
+ // At this point page web process should have exited and the callback is expected to have
+ // been invoked during the IPC connection destruction.
+ //
+ // Ideally we'd check that underlying WebProcesPool, and WebProcessProxy have been destroyed too
+ // but there is no public API for that.
+ g_assert_true(didRunForceRepaintCallback);
+}
+
void beforeAll()
{
kServer = new WebKitTestServer();
@@ -1062,6 +1089,7 @@
MemoryPressureTest::add("WebKitWebContext", "memory-pressure", testMemoryPressureSettings);
WebViewTest::add("WebKitWebContext", "timezone", testWebContextTimeZoneOverride);
WebViewTest::add("WebKitWebContext", "timezone-worker", testWebContextTimeZoneOverrideInWorker);
+ WebViewTest::add("WebKitWebContext", "no-web-process-leak", testNoWebProcessLeakAfterWebKitWebContextDestroy);
}
void afterAll()
Modified: trunk/Tools/TestWebKitAPI/glib/WebKitGLib/WebViewTest.cpp (295510 => 295511)
--- trunk/Tools/TestWebKitAPI/glib/WebKitGLib/WebViewTest.cpp 2022-06-14 01:17:18 UTC (rev 295510)
+++ trunk/Tools/TestWebKitAPI/glib/WebKitGLib/WebViewTest.cpp 2022-06-14 02:24:27 UTC (rev 295511)
@@ -195,9 +195,11 @@
g_main_loop_quit(test->m_mainLoop);
}
-void WebViewTest::waitUntilLoadFinished()
+void WebViewTest::waitUntilLoadFinished(WebKitWebView* webView)
{
- g_signal_connect(m_webView, "load-changed", G_CALLBACK(loadChanged), this);
+ if (!webView)
+ webView = m_webView;
+ g_signal_connect(webView, "load-changed", G_CALLBACK(loadChanged), this);
g_main_loop_run(m_mainLoop);
}
Modified: trunk/Tools/TestWebKitAPI/glib/WebKitGLib/WebViewTest.h (295510 => 295511)
--- trunk/Tools/TestWebKitAPI/glib/WebKitGLib/WebViewTest.h 2022-06-14 01:17:18 UTC (rev 295510)
+++ trunk/Tools/TestWebKitAPI/glib/WebKitGLib/WebViewTest.h 2022-06-14 02:24:27 UTC (rev 295511)
@@ -49,7 +49,7 @@
void quitMainLoop();
void quitMainLoopAfterProcessingPendingEvents();
void wait(double seconds);
- void waitUntilLoadFinished();
+ void waitUntilLoadFinished(WebKitWebView* = nullptr);
void waitUntilTitleChangedTo(const char* expectedTitle);
void waitUntilTitleChanged();
void waitUntilIsWebProcessResponsiveChanged();
