Skip to content
Permalink
Browse files
[GTK][WPE] Leak checker is not working in WebKitGLib web process tests
https://bugs.webkit.org/show_bug.cgi?id=183404

Reviewed by Michael Catanzaro.

Source/WebKit:

Add private helper for testing to do a garbage collection when the page is closing.

* WebProcess/InjectedBundle/API/glib/WebKitWebExtension.cpp:
(webkitWebExtensionSetGarbageCollectOnPageDestroy):
* WebProcess/InjectedBundle/API/glib/WebKitWebExtensionPrivate.h:
* WebProcess/InjectedBundle/API/gtk/DOM/WebKitDOMClientRectList.h: Fix annotation of
webkit_dom_client_rect_list_item(), it should be transfer full.

Tools:

This might have regressed when we started to use the JSC garbage collector timers. The thing is that we expect
that the WebProcessTester object that we expose to JavaScript is released when the web frame is destroyed, but
that's no longer the case. On window object cleared a GC is scheduled, but JSC timers do the actual garbage
collection later. In the case of tests that never happens because the web process finishes quickly after the
test. We need to force a garbage collection at some point when the web page is destroyed. We can't use the
WebKitWebPage destroy signal, since we are also checking that WebKitWebPage isn't leaked. The
API::InjectedBundle::Client::willDestroyPage() always happen when the page is closed, even if WebKitWebPage is
still alive, so we can force the GC at that point. The only problem is that the frame is detached right after
that point, so we can't check WebKitFrame leaks. The only frame in the tests is the main one, so we can assume
that if WebKitWebPage is released, the frame is too.

* TestWebKitAPI/Tests/WebKitGLib/FrameTest.cpp:
(WebKitFrameTest::testMainFrame): Stop checking we don't leak WebKitFrame.
(WebKitFrameTest::testURI): Ditto.
(WebKitFrameTest::testJavaScriptContext): Ditto.
* TestWebKitAPI/Tests/WebKitGLib/TestFrame.cpp:
(testWebKitFrameMainFrame): Use new WebViewTest::runWebProcessTest() API.
(testWebKitFrameURI): Ditto.
(testWebKitFrameJavaScriptContext): Ditto.
(webkitFrameTestRun): Deleted.
* TestWebKitAPI/Tests/WebKitGLib/WebProcessTest.cpp:
(runTest): We no longer need the special case for dom-cache test.
(windowObjectClearedCallback): Only expose test runner object to JavaScript when loading tests.
(webkit_web_extension_initialize): Call webkitWebExtensionSetGarbageCollectOnPageDestroy() to ensure a garbage
collection is performed when the page is closing.
* TestWebKitAPI/Tests/WebKitGtk/DOMClientRectTest.cpp:
(WebKitDOMClientRectTest::testDivClientRectsPositionAndLength): Fix memory leak,
webkit_dom_client_rect_list_item() returns a full reference.
* TestWebKitAPI/Tests/WebKitGtk/TestAutocleanups.cpp:
(testWebProcessAutocleanups):Use new WebViewTest::runWebProcessTest() API.
* TestWebKitAPI/Tests/WebKitGtk/TestDOMClientRect.cpp:
(testWebKitDOMClientRectDivBoundingClientRectPosition): Use new WebViewTest::runWebProcessTest() API.
(testWebKitDOMClientRectDivClientRectsPositionAndLength): Use new WebViewTest::runWebProcessTest() API.
(prepareDOMForClientRectPositionTests): Deleted.
* TestWebKitAPI/Tests/WebKitGtk/TestDOMNode.cpp:
(testWebKitDOMNodeHierarchyNavigation): Use new WebViewTest::runWebProcessTest() API.
(testWebKitDOMNodeInsertion): Ditto.
(testWebKitDOMNodeTagNamesNodeList): Ditto.
(testWebKitDOMNodeTagNamesHTMLCollection): Ditto.
(testWebKitDOMObjectCache): We no longer need to run the test several times, since runWebProcessTest() loads
about blank after every test.
(prepareDOMForTagNamesTests): Deleted.
* TestWebKitAPI/Tests/WebKitGtk/TestDOMNodeFilter.cpp:
(testWebKitDOMNodeFilterTreeWalker): Use new WebViewTest::runWebProcessTest() API.
(testWebKitDOMNodeFilterNodeIterator): Ditto.
(runTest): Deleted.
* TestWebKitAPI/Tests/WebKitGtk/TestDOMXPathNSResolver.cpp:
(testWebKitDOMXPathNSResolverNative): Use new WebViewTest::runWebProcessTest() API.
(testWebKitDOMXPathNSResolverCustom): Ditto.
* TestWebKitAPI/Tests/WebKitGtk/TestEditor.cpp:
(testWebKitWebEditorSelectionChanged): Use new WebViewTest::runWebProcessTest() API.
* TestWebKitAPI/glib/WebKitGLib/WebViewTest.cpp:
(WebViewTest::runWebProcessTest): It now receives the contents, so it automatically loads the view using
"webprocess://test" as base URI, used to detect tests in the web process. It also loads about:blank after every
test to ensure that window object is cleared.
* TestWebKitAPI/glib/WebKitGLib/WebViewTest.h:

Canonical link: https://commits.webkit.org/199102@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@229395 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
carlosgcampos committed Mar 8, 2018
1 parent de6e8f2 commit f3d58eacd4632dbc2d574b419701034ee05e899c
@@ -1,3 +1,18 @@
2018-03-07 Carlos Garcia Campos <cgarcia@igalia.com>

[GTK][WPE] Leak checker is not working in WebKitGLib web process tests
https://bugs.webkit.org/show_bug.cgi?id=183404

Reviewed by Michael Catanzaro.

Add private helper for testing to do a garbage collection when the page is closing.

* WebProcess/InjectedBundle/API/glib/WebKitWebExtension.cpp:
(webkitWebExtensionSetGarbageCollectOnPageDestroy):
* WebProcess/InjectedBundle/API/glib/WebKitWebExtensionPrivate.h:
* WebProcess/InjectedBundle/API/gtk/DOM/WebKitDOMClientRectList.h: Fix annotation of
webkit_dom_client_rect_list_item(), it should be transfer full.

2018-03-07 Youenn Fablet <youenn@apple.com>

Match unsupported plugins based on domains and not origin
@@ -26,6 +26,7 @@
#include "WebKitWebExtensionPrivate.h"
#include "WebKitWebPagePrivate.h"
#include "WebProcess.h"
#include <WebCore/GCController.h>
#include <wtf/HashMap.h>
#include <wtf/glib/GRefPtr.h>
#include <wtf/glib/WTFGType.h>
@@ -118,6 +119,9 @@ typedef HashMap<WebPage*, GRefPtr<WebKitWebPage> > WebPageMap;

struct _WebKitWebExtensionPrivate {
WebPageMap pages;
#if ENABLE(DEVELOPER_MODE)
bool garbageCollectOnPageDestroy;
#endif
};

static guint signals[LAST_SIGNAL] = { 0, };
@@ -162,6 +166,10 @@ class WebExtensionInjectedBundleClient final : public API::InjectedBundle::Clien
void willDestroyPage(InjectedBundle&, WebPage& page) override
{
m_extension->priv->pages.remove(&page);
#if ENABLE(DEVELOPER_MODE)
if (m_extension->priv->garbageCollectOnPageDestroy)
WebCore::GCController::singleton().garbageCollectNow();
#endif
}

void didReceiveMessage(InjectedBundle&, const String& messageName, API::Object* messageBody) override
@@ -192,6 +200,13 @@ WebKitWebExtension* webkitWebExtensionCreate(InjectedBundle* bundle)
return extension;
}

void webkitWebExtensionSetGarbageCollectOnPageDestroy(WebKitWebExtension* extension)
{
#if ENABLE(DEVELOPER_MODE)
extension->priv->garbageCollectOnPageDestroy = true;
#endif
}

/**
* webkit_web_extension_get_page:
* @extension: a #WebKitWebExtension
@@ -17,12 +17,10 @@
* Boston, MA 02110-1301, USA.
*/

#ifndef WebKitWebExtensionPrivate_h
#define WebKitWebExtensionPrivate_h
#pragma once

#include "InjectedBundle.h"
#include "WebKitWebExtension.h"

WebKitWebExtension* webkitWebExtensionCreate(WebKit::InjectedBundle*);

#endif // WebKitWebExtensionPrivate_h
void webkitWebExtensionSetGarbageCollectOnPageDestroy(WebKitWebExtension*);
@@ -68,7 +68,7 @@ webkit_dom_client_rect_list_get_length(WebKitDOMClientRectList* self);
*
* Returns the #WebKitDOMClientRect object that @self contains at @index.
*
* Returns: (transfer none): A #WebKitDOMClientRect
* Returns: (transfer full): A #WebKitDOMClientRect
*
* Since: 2.18
**/
@@ -1,3 +1,67 @@
2018-03-07 Carlos Garcia Campos <cgarcia@igalia.com>

[GTK][WPE] Leak checker is not working in WebKitGLib web process tests
https://bugs.webkit.org/show_bug.cgi?id=183404

Reviewed by Michael Catanzaro.

This might have regressed when we started to use the JSC garbage collector timers. The thing is that we expect
that the WebProcessTester object that we expose to JavaScript is released when the web frame is destroyed, but
that's no longer the case. On window object cleared a GC is scheduled, but JSC timers do the actual garbage
collection later. In the case of tests that never happens because the web process finishes quickly after the
test. We need to force a garbage collection at some point when the web page is destroyed. We can't use the
WebKitWebPage destroy signal, since we are also checking that WebKitWebPage isn't leaked. The
API::InjectedBundle::Client::willDestroyPage() always happen when the page is closed, even if WebKitWebPage is
still alive, so we can force the GC at that point. The only problem is that the frame is detached right after
that point, so we can't check WebKitFrame leaks. The only frame in the tests is the main one, so we can assume
that if WebKitWebPage is released, the frame is too.

* TestWebKitAPI/Tests/WebKitGLib/FrameTest.cpp:
(WebKitFrameTest::testMainFrame): Stop checking we don't leak WebKitFrame.
(WebKitFrameTest::testURI): Ditto.
(WebKitFrameTest::testJavaScriptContext): Ditto.
* TestWebKitAPI/Tests/WebKitGLib/TestFrame.cpp:
(testWebKitFrameMainFrame): Use new WebViewTest::runWebProcessTest() API.
(testWebKitFrameURI): Ditto.
(testWebKitFrameJavaScriptContext): Ditto.
(webkitFrameTestRun): Deleted.
* TestWebKitAPI/Tests/WebKitGLib/WebProcessTest.cpp:
(runTest): We no longer need the special case for dom-cache test.
(windowObjectClearedCallback): Only expose test runner object to JavaScript when loading tests.
(webkit_web_extension_initialize): Call webkitWebExtensionSetGarbageCollectOnPageDestroy() to ensure a garbage
collection is performed when the page is closing.
* TestWebKitAPI/Tests/WebKitGtk/DOMClientRectTest.cpp:
(WebKitDOMClientRectTest::testDivClientRectsPositionAndLength): Fix memory leak,
webkit_dom_client_rect_list_item() returns a full reference.
* TestWebKitAPI/Tests/WebKitGtk/TestAutocleanups.cpp:
(testWebProcessAutocleanups):Use new WebViewTest::runWebProcessTest() API.
* TestWebKitAPI/Tests/WebKitGtk/TestDOMClientRect.cpp:
(testWebKitDOMClientRectDivBoundingClientRectPosition): Use new WebViewTest::runWebProcessTest() API.
(testWebKitDOMClientRectDivClientRectsPositionAndLength): Use new WebViewTest::runWebProcessTest() API.
(prepareDOMForClientRectPositionTests): Deleted.
* TestWebKitAPI/Tests/WebKitGtk/TestDOMNode.cpp:
(testWebKitDOMNodeHierarchyNavigation): Use new WebViewTest::runWebProcessTest() API.
(testWebKitDOMNodeInsertion): Ditto.
(testWebKitDOMNodeTagNamesNodeList): Ditto.
(testWebKitDOMNodeTagNamesHTMLCollection): Ditto.
(testWebKitDOMObjectCache): We no longer need to run the test several times, since runWebProcessTest() loads
about blank after every test.
(prepareDOMForTagNamesTests): Deleted.
* TestWebKitAPI/Tests/WebKitGtk/TestDOMNodeFilter.cpp:
(testWebKitDOMNodeFilterTreeWalker): Use new WebViewTest::runWebProcessTest() API.
(testWebKitDOMNodeFilterNodeIterator): Ditto.
(runTest): Deleted.
* TestWebKitAPI/Tests/WebKitGtk/TestDOMXPathNSResolver.cpp:
(testWebKitDOMXPathNSResolverNative): Use new WebViewTest::runWebProcessTest() API.
(testWebKitDOMXPathNSResolverCustom): Ditto.
* TestWebKitAPI/Tests/WebKitGtk/TestEditor.cpp:
(testWebKitWebEditorSelectionChanged): Use new WebViewTest::runWebProcessTest() API.
* TestWebKitAPI/glib/WebKitGLib/WebViewTest.cpp:
(WebViewTest::runWebProcessTest): It now receives the contents, so it automatically loads the view using
"webprocess://test" as base URI, used to detect tests in the web process. It also loads about:blank after every
test to ensure that window object is cleared.
* TestWebKitAPI/glib/WebKitGLib/WebViewTest.h:

2018-03-07 Youenn Fablet <youenn@apple.com>

Match unsupported plugins based on domains and not origin
@@ -31,7 +31,6 @@ class WebKitFrameTest : public WebProcessTest {
{
WebKitFrame* frame = webkit_web_page_get_main_frame(page);
g_assert(WEBKIT_IS_FRAME(frame));
assertObjectIsDeletedWhenTestFinishes(G_OBJECT(frame));
g_assert(webkit_frame_is_main_frame(frame));

return true;
@@ -41,7 +40,6 @@ class WebKitFrameTest : public WebProcessTest {
{
WebKitFrame* frame = webkit_web_page_get_main_frame(page);
g_assert(WEBKIT_IS_FRAME(frame));
assertObjectIsDeletedWhenTestFinishes(G_OBJECT(frame));
g_assert_cmpstr(webkit_web_page_get_uri(page), ==, webkit_frame_get_uri(frame));

return true;
@@ -51,7 +49,6 @@ class WebKitFrameTest : public WebProcessTest {
{
WebKitFrame* frame = webkit_web_page_get_main_frame(page);
g_assert(WEBKIT_IS_FRAME(frame));
assertObjectIsDeletedWhenTestFinishes(G_OBJECT(frame));
g_assert(webkit_frame_get_javascript_global_context(frame));

return true;
@@ -21,28 +21,19 @@

#include "WebViewTest.h"

static void webkitFrameTestRun(WebViewTest* test, const char* testName)
{
static const char* testHTML = "<html><body></body></html>";
test->loadHtml(testHTML, 0);
test->waitUntilLoadFinished();

g_assert(test->runWebProcessTest("WebKitFrame", testName));
}

static void testWebKitFrameMainFrame(WebViewTest* test, gconstpointer)
{
webkitFrameTestRun(test, "main-frame");
g_assert(test->runWebProcessTest("WebKitFrame", "main-frame"));
}

static void testWebKitFrameURI(WebViewTest* test, gconstpointer)
{
webkitFrameTestRun(test, "uri");
g_assert(test->runWebProcessTest("WebKitFrame", "uri"));
}

static void testWebKitFrameJavaScriptContext(WebViewTest* test, gconstpointer)
{
webkitFrameTestRun(test, "javascript-context");
g_assert(test->runWebProcessTest("WebKitFrame", "javascript-context"));
}

void beforeAll()
@@ -20,6 +20,7 @@
#include "config.h"
#include "WebProcessTest.h"

#include "WebKitWebExtensionPrivate.h"
#include <JavaScriptCore/JSRetainPtr.h>
#include <gio/gio.h>
#include <wtf/HashSet.h>
@@ -64,14 +65,7 @@ static JSValueRef runTest(JSContextRef context, JSObjectRef function, JSObjectRe

WebKitWebPage* webPage = WEBKIT_WEB_PAGE(JSObjectGetPrivate(thisObject));
g_assert(WEBKIT_IS_WEB_PAGE(webPage));
// Test /WebKitDOMNode/dom-cache is an exception, because it's called 3 times, so
// the WebPage is destroyed after the third time.
if (g_str_equal(testPath.get(), "WebKitDOMNode/dom-cache")) {
static unsigned domCacheTestRunCount = 0;
if (++domCacheTestRunCount == 3)
WebProcessTest::assertObjectIsDeletedWhenTestFinishes(G_OBJECT(webPage));
} else
WebProcessTest::assertObjectIsDeletedWhenTestFinishes(G_OBJECT(webPage));
WebProcessTest::assertObjectIsDeletedWhenTestFinishes(G_OBJECT(webPage));

std::unique_ptr<WebProcessTest> test = WebProcessTest::create(String::fromUTF8(testPath.get()));
return JSValueMakeBoolean(context, test->runTest(g_strrstr(testPath.get(), "/") + 1, webPage));
@@ -100,6 +94,9 @@ static void webProcessTestRunnerFinalize(JSObjectRef object)

static void windowObjectClearedCallback(WebKitScriptWorld* world, WebKitWebPage* webPage, WebKitFrame* frame, WebKitWebExtension* extension)
{
if (g_strcmp0(webkit_web_page_get_uri(webPage), "webprocess://test"))
return;

JSGlobalContextRef context = webkit_frame_get_javascript_context_for_script_world(frame, world);
JSObjectRef globalObject = JSContextGetGlobalObject(context);

@@ -117,5 +114,6 @@ static void windowObjectClearedCallback(WebKitScriptWorld* world, WebKitWebPage*

extern "C" void webkit_web_extension_initialize(WebKitWebExtension* extension)
{
webkitWebExtensionSetGarbageCollectOnPageDestroy(extension);
g_signal_connect(webkit_script_world_get_default(), "window-object-cleared", G_CALLBACK(windowObjectClearedCallback), extension);
}
@@ -74,13 +74,14 @@ class WebKitDOMClientRectTest : public WebProcessTest {

g_assert_cmpuint(webkit_dom_client_rect_list_get_length(clientRectList.get()), ==, 1);

WebKitDOMClientRect* clientRect = webkit_dom_client_rect_list_item(clientRectList.get(), 0);
g_assert(WEBKIT_DOM_IS_CLIENT_RECT(clientRect));
assertObjectIsDeletedWhenTestFinishes(G_OBJECT(clientRect));
checkClientRectPosition(clientRect);
GRefPtr<WebKitDOMClientRect> clientRect = adoptGRef(webkit_dom_client_rect_list_item(clientRectList.get(), 0));
g_assert(WEBKIT_DOM_IS_CLIENT_RECT(clientRect.get()));
assertObjectIsDeletedWhenTestFinishes(G_OBJECT(clientRect.get()));
checkClientRectPosition(clientRect.get());

// Getting the clientRect twice should return the same pointer.
g_assert(webkit_dom_client_rect_list_item(clientRectList.get(), 0) == webkit_dom_client_rect_list_item(clientRectList.get(), 0));
GRefPtr<WebKitDOMClientRect> clientRect2 = adoptGRef(webkit_dom_client_rect_list_item(clientRectList.get(), 0));
g_assert(clientRect.get() == clientRect2.get());

return true;
}
@@ -44,10 +44,6 @@ static void testUIProcessAutocleanups(WebViewTest* test, gconstpointer)

static void testWebProcessAutocleanups(WebViewTest* test, gconstpointer)
{
static const char* testHTML = "<html><body></body></html>";
test->loadHtml(testHTML, nullptr);
test->waitUntilLoadFinished();

g_assert(test->runWebProcessTest("Autocleanups", "web-process-autocleanups"));
}

@@ -22,27 +22,20 @@
#include "WebViewTest.h"
#include <webkit2/webkit2.h>

static void prepareDOMForClientRectPositionTests(WebViewTest* test)
{
static const char* testHTML = "<html><head></head><body>"
"<style>"
" #rect { position: fixed; top: -25px; left: -50px; width: 100px; height: 200px; }"
"</style>"
"<div id=rect></div></body></html>";
test->loadHtml(testHTML, nullptr);
test->waitUntilLoadFinished();
}
static const char* testHTML = "<html><head></head><body>"
"<style>"
" #rect { position: fixed; top: -25px; left: -50px; width: 100px; height: 200px; }"
"</style>"
"<div id=rect></div></body></html>";

static void testWebKitDOMClientRectDivBoundingClientRectPosition(WebViewTest* test, gconstpointer)
{
prepareDOMForClientRectPositionTests(test);
g_assert(test->runWebProcessTest("WebKitDOMClientRect", "div-bounding-client-rect-position"));
g_assert(test->runWebProcessTest("WebKitDOMClientRect", "div-bounding-client-rect-position", testHTML));
}

static void testWebKitDOMClientRectDivClientRectsPositionAndLength(WebViewTest* test, gconstpointer)
{
prepareDOMForClientRectPositionTests(test);
g_assert(test->runWebProcessTest("WebKitDOMClientRect", "div-client-rects-position-and-length"));
g_assert(test->runWebProcessTest("WebKitDOMClientRect", "div-client-rects-position-and-length", testHTML));
}

void beforeAll()
@@ -26,61 +26,39 @@
static void testWebKitDOMNodeHierarchyNavigation(WebViewTest* test, gconstpointer)
{
static const char* testHTML = "<html><head><title>This is the title</title></head><body><p>1</p><p>2</p><p>3</p></body></html>";
test->loadHtml(testHTML, 0);
test->waitUntilLoadFinished();

g_assert(test->runWebProcessTest("WebKitDOMNode", "hierarchy-navigation"));
g_assert(test->runWebProcessTest("WebKitDOMNode", "hierarchy-navigation", testHTML));
}

static void testWebKitDOMNodeInsertion(WebViewTest* test, gconstpointer)
{
static const char* testHTML = "<html><body></body></html>";
test->loadHtml(testHTML, 0);
test->waitUntilLoadFinished();

g_assert(test->runWebProcessTest("WebKitDOMNode", "insertion"));
}

static void prepareDOMForTagNamesTests(WebViewTest* test)
{
static const char* testHTML = "<html><head></head><body>"
"<video id='video' preload='none'>"
" <source src='movie.ogg' type='video/ogg'>"
" Your browser does not support the video tag."
"</video>"
"<video id='video2' preload='none'>"
" <source src='movie.ogg' type='video/ogg'>"
" Your browser does not support the video tag."
"</video>"
"<input type='hidden' id='test' name='finish' value='false'></body></html>";
test->loadHtml(testHTML, nullptr);
test->waitUntilLoadFinished();
}
static const char* tagNamesTestHTML = "<html><head></head><body>"
"<video id='video' preload='none'>"
" <source src='movie.ogg' type='video/ogg'>"
" Your browser does not support the video tag."
"</video>"
"<video id='video2' preload='none'>"
" <source src='movie.ogg' type='video/ogg'>"
" Your browser does not support the video tag."
"</video>"
"<input type='hidden' id='test' name='finish' value='false'></body></html>";

static void testWebKitDOMNodeTagNamesNodeList(WebViewTest* test, gconstpointer)
{
prepareDOMForTagNamesTests(test);
g_assert(test->runWebProcessTest("WebKitDOMNode", "tag-names-node-list"));
g_assert(test->runWebProcessTest("WebKitDOMNode", "tag-names-node-list", tagNamesTestHTML));
}

static void testWebKitDOMNodeTagNamesHTMLCollection(WebViewTest* test, gconstpointer)
{
prepareDOMForTagNamesTests(test);
g_assert(test->runWebProcessTest("WebKitDOMNode", "tag-names-html-collection"));
g_assert(test->runWebProcessTest("WebKitDOMNode", "tag-names-html-collection", tagNamesTestHTML));
}

static void testWebKitDOMObjectCache(WebViewTest* test, gconstpointer)
{
static const char* testHTML = "<html><body><div id='container'><p>DOM Cache test</p><a id='link href='#'>link</a></div></body></html>";

// Run the test 3 times to make sure the DOM objects are correctly released when the
// document is detached from the frame for every new document created.
for (unsigned i = 0; i < 3; ++i) {
test->loadHtml(testHTML, nullptr);
test->waitUntilLoadFinished();

g_assert(test->runWebProcessTest("WebKitDOMNode", "dom-cache"));
}
g_assert(test->runWebProcessTest("WebKitDOMNode", "dom-cache", testHTML));
}


0 comments on commit f3d58ea

Please sign in to comment.