Skip to content

Commit

Permalink
Merge r225043 - [GTK][WPE] webkit_cookie_manager_delete_all_cookies d…
Browse files Browse the repository at this point in the history
…oesn'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):
  • Loading branch information
carlosgcampos committed Dec 18, 2017
1 parent c3d630f commit f1ba975
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 12 deletions.
22 changes: 22 additions & 0 deletions Source/WebKit/ChangeLog
@@ -1,3 +1,25 @@
2017-11-20 Carlos Garcia Campos <cgarcia@igalia.com>

[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 <beidson@apple.com>

Try to avoid creating the default WKWebsiteDataStore until its actually needed.
Expand Down
6 changes: 6 additions & 0 deletions Source/WebKit/NetworkProcess/NetworkProcess.cpp
Expand Up @@ -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);
}
Expand Down
1 change: 1 addition & 0 deletions Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp
Expand Up @@ -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);

Expand Down
14 changes: 14 additions & 0 deletions Tools/ChangeLog
@@ -1,3 +1,17 @@
2017-11-20 Carlos Garcia Campos <cgarcia@igalia.com>

[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 <beidson@apple.com>

Try to avoid creating the default WKWebsiteDataStore until its actually needed.
Expand Down
51 changes: 39 additions & 12 deletions Tools/TestWebKitAPI/Tests/WebKitGLib/TestCookieManager.cpp
Expand Up @@ -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);
Expand Down Expand Up @@ -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<char> m_cookiesTextFile;
GUniquePtr<char> m_cookiesSQLiteFile;
};
Expand Down Expand Up @@ -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<char> cookiesFileContents(g_strdup_printf(cookiesFileFormat, expires, expires));
GUniquePtr<char> 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)
Expand Down Expand Up @@ -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 = "<html><body><p>No cookies</p></body></html>";
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);
}
Expand All @@ -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);
}

Expand Down

0 comments on commit f1ba975

Please sign in to comment.