Skip to content
Permalink
Browse files
[WPE][GTK] REGRESSION (r294381): WPEWebProcess leak after closing bro…
…wser

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
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295511 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
yury-s committed Jun 14, 2022
1 parent 9440b41 commit 364ed4fd60ade51f1f382a73075bdee471ab0b17
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 6 deletions.
@@ -5051,3 +5051,17 @@ void webkit_web_view_set_display_capture_state(WebKitWebView* webView, WebKitMed

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));
}
@@ -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);
@@ -4577,10 +4577,13 @@ void WebPageProxy::forceRepaint(CompletionHandler<void()>&& callback)

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();
});
});
}

@@ -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 @@ static void testWebContextTimeZoneOverrideInWorker(WebViewTest* test, gconstpoin
g_assert_cmpstr(WebViewTest::javascriptResultToCString(javascriptResult), ==, "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 @@ void beforeAll()
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()
@@ -195,9 +195,11 @@ static void loadChanged(WebKitWebView* webView, WebKitLoadEvent loadEvent, WebVi
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);
}

@@ -49,7 +49,7 @@ class WebViewTest: public Test {
void quitMainLoop();
void quitMainLoopAfterProcessingPendingEvents();
void wait(double seconds);
void waitUntilLoadFinished();
void waitUntilLoadFinished(WebKitWebView* = nullptr);
void waitUntilTitleChangedTo(const char* expectedTitle);
void waitUntilTitleChanged();
void waitUntilIsWebProcessResponsiveChanged();

0 comments on commit 364ed4f

Please sign in to comment.