From f1ba975f1bc887178cbb27780e32fcbf65820b23 Mon Sep 17 00:00:00 2001 From: Carlos Garcia Campos Date: Mon, 18 Dec 2017 10:54:33 +0000 Subject: [PATCH] Merge r225043 - [GTK][WPE] webkit_cookie_manager_delete_all_cookies doesn't delete the cookies if called before a web process is running https://bugs.webkit.org/show_bug.cgi?id=175265 Reviewed by Michael Catanzaro. Source/WebKit: This is what happens: 1- We create our WebKitWebContext that creates its WebProcessPool. 2- We set a persistent cookies storage. 3- We ask the website data store to delete all cookies, but since website data store is a web process observer and we haven't spawned any web process yet, it creates a new WebProcessPool with the default configuration (no persistent cookies) and sends the message to delete the cookies there. 4- The network process of the second process pool does nothing because it doesn't have cookies at all. We need to set the primary data store of the WebProcessPool when WebKitWebContext is constructed to ensure that one is used before the web process is launched. * UIProcess/API/glib/WebKitWebContext.cpp: (webkitWebContextConstructed): Tools: Add test case. * TestWebKitAPI/Tests/WebKitGLib/TestCookieManager.cpp: (testCookieManagerPersistentStorageDeleteAll): (serverCallback): (beforeAll): --- Source/WebKit/ChangeLog | 22 ++++++++ .../WebKit/NetworkProcess/NetworkProcess.cpp | 6 +++ .../UIProcess/API/glib/WebKitWebContext.cpp | 1 + Tools/ChangeLog | 14 +++++ .../Tests/WebKitGLib/TestCookieManager.cpp | 51 ++++++++++++++----- 5 files changed, 82 insertions(+), 12 deletions(-) diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog index 677cfef1fca2..4d7b36111060 100644 --- a/Source/WebKit/ChangeLog +++ b/Source/WebKit/ChangeLog @@ -1,3 +1,25 @@ +2017-11-20 Carlos Garcia Campos + + [GTK][WPE] webkit_cookie_manager_delete_all_cookies doesn't delete the cookies if called before a web process is running + https://bugs.webkit.org/show_bug.cgi?id=175265 + + Reviewed by Michael Catanzaro. + + This is what happens: + + 1- We create our WebKitWebContext that creates its WebProcessPool. + 2- We set a persistent cookies storage. + 3- We ask the website data store to delete all cookies, but since website data store is a web process observer + and we haven't spawned any web process yet, it creates a new WebProcessPool with the default configuration + (no persistent cookies) and sends the message to delete the cookies there. + 4- The network process of the second process pool does nothing because it doesn't have cookies at all. + + We need to set the primary data store of the WebProcessPool when WebKitWebContext is constructed to ensure that + one is used before the web process is launched. + + * UIProcess/API/glib/WebKitWebContext.cpp: + (webkitWebContextConstructed): + 2017-09-10 Brady Eidson Try to avoid creating the default WKWebsiteDataStore until its actually needed. diff --git a/Source/WebKit/NetworkProcess/NetworkProcess.cpp b/Source/WebKit/NetworkProcess/NetworkProcess.cpp index 1be38a272180..d3ce1d9faed4 100644 --- a/Source/WebKit/NetworkProcess/NetworkProcess.cpp +++ b/Source/WebKit/NetworkProcess/NetworkProcess.cpp @@ -242,6 +242,12 @@ void NetworkProcess::initializeNetworkProcess(NetworkProcessCreationParameters&& if (parameters.shouldUseTestingNetworkSession) NetworkStorageSession::switchToNewTestingSession(); +#if USE(NETWORK_SESSION) + // Ensure default session is created. + NetworkSession::defaultSession(); +#endif + + for (auto& supplement : m_supplements.values()) supplement->initialize(parameters); } diff --git a/Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp b/Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp index 3a0b4f377eaa..bee6232dec2a 100644 --- a/Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp +++ b/Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp @@ -339,6 +339,7 @@ static void webkitWebContextConstructed(GObject* object) if (!priv->websiteDataManager) priv->websiteDataManager = adoptGRef(webkitWebsiteDataManagerCreate(websiteDataStoreConfigurationForWebProcessPoolConfiguration(configuration))); + priv->processPool->setPrimaryDataStore(webkitWebsiteDataManagerGetDataStore(priv->websiteDataManager.get())); webkitWebsiteDataManagerAddProcessPool(priv->websiteDataManager.get(), *priv->processPool); diff --git a/Tools/ChangeLog b/Tools/ChangeLog index 86df0632c0b2..5bfa102981da 100644 --- a/Tools/ChangeLog +++ b/Tools/ChangeLog @@ -1,3 +1,17 @@ +2017-11-20 Carlos Garcia Campos + + [GTK][WPE] webkit_cookie_manager_delete_all_cookies doesn't delete the cookies if called before a web process is running + https://bugs.webkit.org/show_bug.cgi?id=175265 + + Reviewed by Michael Catanzaro. + + Add test case. + + * TestWebKitAPI/Tests/WebKitGLib/TestCookieManager.cpp: + (testCookieManagerPersistentStorageDeleteAll): + (serverCallback): + (beforeAll): + 2017-09-10 Brady Eidson Try to avoid creating the default WKWebsiteDataStore until its actually needed. diff --git a/Tools/TestWebKitAPI/Tests/WebKitGLib/TestCookieManager.cpp b/Tools/TestWebKitAPI/Tests/WebKitGLib/TestCookieManager.cpp index 1be8cc0d0604..911a624a0472 100644 --- a/Tools/TestWebKitAPI/Tests/WebKitGLib/TestCookieManager.cpp +++ b/Tools/TestWebKitAPI/Tests/WebKitGLib/TestCookieManager.cpp @@ -40,17 +40,13 @@ class CookieManagerTest: public WebViewTest { static void cookiesChangedCallback(WebKitCookieManager*, CookieManagerTest* test) { test->m_cookiesChanged = true; - if (test->m_finishLoopWhenCookiesChange) + if (test->m_finishLoopWhenCookiesChange && !(--test->m_cookiesExpectedToChangeCount)) g_main_loop_quit(test->m_mainLoop); } CookieManagerTest() : WebViewTest() , m_cookieManager(webkit_web_context_get_cookie_manager(webkit_web_view_get_context(m_webView))) - , m_acceptPolicy(WEBKIT_COOKIE_POLICY_ACCEPT_NO_THIRD_PARTY) - , m_domains(0) - , m_cookiesChanged(false) - , m_finishLoopWhenCookiesChange(false) { g_assert(webkit_website_data_manager_get_cookie_manager(webkit_web_context_get_website_data_manager(webkit_web_view_get_context(m_webView))) == m_cookieManager); g_signal_connect(m_cookieManager, "changed", G_CALLBACK(cookiesChangedCallback), this); @@ -162,19 +158,21 @@ class CookieManagerTest: public WebViewTest { G_GNUC_END_IGNORE_DEPRECATIONS; } - void waitUntilCookiesChanged() + void waitUntilCookiesChanged(int cookiesExpectedToChangeCount = 1) { m_cookiesChanged = false; + m_cookiesExpectedToChangeCount = cookiesExpectedToChangeCount; m_finishLoopWhenCookiesChange = true; g_main_loop_run(m_mainLoop); m_finishLoopWhenCookiesChange = false; } - WebKitCookieManager* m_cookieManager; - WebKitCookieAcceptPolicy m_acceptPolicy; - char** m_domains; - bool m_cookiesChanged; - bool m_finishLoopWhenCookiesChange; + WebKitCookieManager* m_cookieManager { nullptr }; + WebKitCookieAcceptPolicy m_acceptPolicy { WEBKIT_COOKIE_POLICY_ACCEPT_NO_THIRD_PARTY }; + char** m_domains { nullptr }; + bool m_cookiesChanged { false }; + int m_cookiesExpectedToChangeCount { 0 }; + bool m_finishLoopWhenCookiesChange { false }; GUniquePtr m_cookiesTextFile; GUniquePtr m_cookiesSQLiteFile; }; @@ -300,6 +298,31 @@ static void testCookieManagerPersistentStorage(CookieManagerTest* test, gconstpo g_assert_cmpint(g_strv_length(test->getDomains()), ==, 0); } +static void testCookieManagerPersistentStorageDeleteAll(CookieManagerTest* test, gconstpointer) +{ + // This checks that we can remove all the cookies of an existing file before a web process is created. + // See bug https://bugs.webkit.org/show_bug.cgi?id=175265. + static const char cookiesFileFormat[] = "127.0.0.1\tFALSE\t/\tFALSE\t%ld\tfoo\tbar\nlocalhost\tFALSE\t/\tFALSE\t%ld\tbaz\tqux\n"; + time_t expires = time(nullptr) + 60; + GUniquePtr cookiesFileContents(g_strdup_printf(cookiesFileFormat, expires, expires)); + GUniquePtr cookiesFile(g_build_filename(Test::dataDirectory(), "cookies.txt", nullptr)); + g_assert(g_file_set_contents(cookiesFile.get(), cookiesFileContents.get(), -1, nullptr)); + + test->setPersistentStorage(WEBKIT_COOKIE_PERSISTENT_STORAGE_TEXT); + test->deleteAllCookies(); + // Changed signal is emitted for every deleted cookie, twice in this case. + test->waitUntilCookiesChanged(2); + + // Ensure the web process is created and load something without cookies. + test->m_cookiesChanged = false; + test->loadURI(kServer->getURIForPath("/no-cookies.html").data()); + test->waitUntilLoadFinished(); + g_assert(!test->m_cookiesChanged); + char** domains = test->getDomains(); + g_assert(domains); + g_assert_cmpint(g_strv_length(domains), ==, 0); +} + static void ephemeralViewloadChanged(WebKitWebView* webView, WebKitLoadEvent loadEvent, WebViewTest* test) { if (loadEvent != WEBKIT_LOAD_FINISHED) @@ -366,7 +389,10 @@ static void serverCallback(SoupServer* server, SoupMessage* message, const char* soup_message_body_append(message->response_body, SOUP_MEMORY_TAKE, indexHtml, strlen(indexHtml)); } else if (g_str_equal(path, "/image.png")) soup_message_headers_replace(message->response_headers, "Set-Cookie", "baz=qux; Max-Age=60"); - else + else if (g_str_equal(path, "/no-cookies.html")) { + static const char* indexHtml = "

No cookies

"; + soup_message_body_append(message->response_body, SOUP_MEMORY_STATIC, indexHtml, strlen(indexHtml)); + } else soup_message_set_status(message, SOUP_STATUS_NOT_FOUND); soup_message_body_complete(message->response_body); } @@ -380,6 +406,7 @@ void beforeAll() CookieManagerTest::add("WebKitCookieManager", "delete-cookies", testCookieManagerDeleteCookies); CookieManagerTest::add("WebKitCookieManager", "cookies-changed", testCookieManagerCookiesChanged); CookieManagerTest::add("WebKitCookieManager", "persistent-storage", testCookieManagerPersistentStorage); + CookieManagerTest::add("WebKitCookieManager", "persistent-storage-delete-all", testCookieManagerPersistentStorageDeleteAll); CookieManagerTest::add("WebKitCookieManager", "ephemeral", testCookieManagerEphemeral); }