testRunner.setAlwaysAcceptCookies does not work with NetworkProcess

https://bugs.webkit.org/show_bug.cgi?id=138687

Source/WebKit2:

Reviewed by Carlos Garcia Campos.

* UIProcess/WebCookieManagerProxy.cpp:
(WebKit::WebCookieManagerProxy::setHTTPCookieAcceptPolicy): Don't change system-wide
cookie accept policy when in testing mode. Also, update the policy in WebProcesses too,
as we still do some networking there.

* UIProcess/WebProcessPool.h:
* WebProcess/InjectedBundle/API/c/WKBundle.cpp:
(WKBundleSetAlwaysAcceptCookies): Deleted.
* WebProcess/InjectedBundle/API/c/WKBundlePrivate.h:
* WebProcess/InjectedBundle/InjectedBundle.cpp:
(WebKit::InjectedBundle::setAlwaysAcceptCookies): Deleted.
* WebProcess/InjectedBundle/InjectedBundle.h:
Remove WKBundleSetAlwaysAcceptCookies SPI, as WKTR now uses an API in UI process.

Tools:

Reviewed by Carlos Garcia Campos.

Always change cookie policy from UI process, as changing it from WebProcess doesn't
affect other processes.

The implementation is imperfect, because the change happens asynchronously, but I
don't expect it to be a problem in practice.

* WebKitTestRunner/InjectedBundle/InjectedBundle.cpp:
(WTR::InjectedBundle::beginTesting):
* WebKitTestRunner/InjectedBundle/TestRunner.cpp:
(WTR::TestRunner::setAlwaysAcceptCookies):
* WebKitTestRunner/TestInvocation.cpp:
(WTR::TestInvocation::invoke):
(WTR::TestInvocation::didReceiveSynchronousMessageFromInjectedBundle):

LayoutTests:

Unreviewed (only code changes were reviewed).

* platform/gtk/TestExpectations:
* platform/mac-wk2/TestExpectations:
Unskip tests that used to be affected.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@181446 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 78039dc..6a71a28 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2015-03-12  Alexey Proskuryakov  <ap@apple.com>
+
+        testRunner.setAlwaysAcceptCookies does not work with NetworkProcess
+        https://bugs.webkit.org/show_bug.cgi?id=138687
+
+        Unreviewed (only code changes were reviewed).
+
+        * platform/gtk/TestExpectations:
+        * platform/mac-wk2/TestExpectations:
+        Unskip tests that used to be affected.
+
 2015-03-12  Simon Fraser  <simon.fraser@apple.com>
 
         Clean up after JoePeck.

diff --git a/LayoutTests/platform/gtk/TestExpectations b/LayoutTests/platform/gtk/TestExpectations
index 33ed934..50d7828 100644 (file)
--- a/LayoutTests/platform/gtk/TestExpectations
+++ b/LayoutTests/platform/gtk/TestExpectations
@@ -1557,9 +1557,6 @@ webkit.org/b/139357 http/tests/media/video-accept-encoding.html [ Failure ]
 webkit.org/b/139357 fast/dom/call-a-constructor-as-a-function.html [ Failure ]
 webkit.org/b/139357 js/dom/constructor-length.html [ Failure ]
 
-# testRunner.setAlwaysAcceptCookies does not work with NetworkProcess
-webkit.org/b/138687 http/tests/xmlhttprequest/cross-origin-cookie-storage.html [ Skip ]
-
 # PingLoader is still in WebProcess. As a result, this test fails and also fails to delete its cookie, breaking many other cookie tests.
 webkit.org/b/138688 http/tests/navigation/ping-cookie.html [ Skip ]
 

diff --git a/LayoutTests/platform/mac-wk2/TestExpectations b/LayoutTests/platform/mac-wk2/TestExpectations
index f50051d..864a991 100644 (file)
--- a/LayoutTests/platform/mac-wk2/TestExpectations
+++ b/LayoutTests/platform/mac-wk2/TestExpectations
@@ -468,10 +468,6 @@ storage/domstorage/localstorage/private-browsing-affects-storage.html [ Skip ]
 storage/domstorage/sessionstorage/private-browsing-affects-storage.html [ Skip ]
 storage/websql/private-browsing-noread-nowrite.html [ Skip ]
 
-# testRunner.setAlwaysAcceptCookies does not work with NetworkProcess
-webkit.org/b/138687 http/tests/cookies/third-party-cookie-relaxing.html [ Skip ]
-webkit.org/b/138687 http/tests/xmlhttprequest/cross-origin-cookie-storage.html [ Skip ]
-
 # FIXME: Needs bugzilla (<rdar://problem/12829991>)
 # PPT: PingLoader is still in WebProcess. As a result, this test fails and also fails to delete its cookie, breaking many other cookie tests.
 webkit.org/b/138688 http/tests/navigation/ping-cookie.html [ Skip ]

diff --git a/Source/WebKit2/ChangeLog b/Source/WebKit2/ChangeLog
index 451af24..534f204 100644 (file)
--- a/Source/WebKit2/ChangeLog
+++ b/Source/WebKit2/ChangeLog
@@ -1,3 +1,24 @@
+2015-03-12  Alexey Proskuryakov  <ap@apple.com>
+
+        testRunner.setAlwaysAcceptCookies does not work with NetworkProcess
+        https://bugs.webkit.org/show_bug.cgi?id=138687
+
+        Reviewed by Carlos Garcia Campos.
+
+        * UIProcess/WebCookieManagerProxy.cpp:
+        (WebKit::WebCookieManagerProxy::setHTTPCookieAcceptPolicy): Don't change system-wide
+        cookie accept policy when in testing mode. Also, update the policy in WebProcesses too,
+        as we still do some networking there.
+
+        * UIProcess/WebProcessPool.h:
+        * WebProcess/InjectedBundle/API/c/WKBundle.cpp:
+        (WKBundleSetAlwaysAcceptCookies): Deleted.
+        * WebProcess/InjectedBundle/API/c/WKBundlePrivate.h:
+        * WebProcess/InjectedBundle/InjectedBundle.cpp:
+        (WebKit::InjectedBundle::setAlwaysAcceptCookies): Deleted.
+        * WebProcess/InjectedBundle/InjectedBundle.h:
+        Remove WKBundleSetAlwaysAcceptCookies SPI, as WKTR now uses an API in UI process.
+
 2015-03-12  Anders Carlsson  <andersca@apple.com>
 
         WKErrorDomain should be declared nonnull

diff --git a/Source/WebKit2/UIProcess/WebCookieManagerProxy.cpp b/Source/WebKit2/UIProcess/WebCookieManagerProxy.cpp
index 5c96627..7849402 100644 (file)
--- a/Source/WebKit2/UIProcess/WebCookieManagerProxy.cpp
+++ b/Source/WebKit2/UIProcess/WebCookieManagerProxy.cpp
@@ -152,13 +152,21 @@ void WebCookieManagerProxy::cookiesDidChange()
 void WebCookieManagerProxy::setHTTPCookieAcceptPolicy(HTTPCookieAcceptPolicy policy)
 {
 #if PLATFORM(COCOA)
-    persistHTTPCookieAcceptPolicy(policy);
+    if (!processPool()->isUsingTestingNetworkSession())
+        persistHTTPCookieAcceptPolicy(policy);
 #endif
 #if USE(SOUP)
     processPool()->setInitialHTTPCookieAcceptPolicy(policy);
 #endif
 
-    processPool()->sendToNetworkingProcessRelaunchingIfNecessary(Messages::WebCookieManager::SetHTTPCookieAcceptPolicy(policy));
+    // The policy is not sent to newly created processes (only Soup does that via setInitialHTTPCookieAcceptPolicy()). This is not a serious problem, because:
+    // - When testing, we only have one WebProcess and one NetworkProcess, and WebKitTestRunner never restarts them;
+    // - When not testing, Cocoa has the policy persisted, and thus new processes use it (even for ephemeral sessions).
+    processPool()->sendToAllProcesses(Messages::WebCookieManager::SetHTTPCookieAcceptPolicy(policy));
+#if ENABLE(NETWORK_PROCESS)
+    if (processPool()->usesNetworkProcess())
+        processPool()->sendToNetworkingProcess(Messages::WebCookieManager::SetHTTPCookieAcceptPolicy(policy));
+#endif
 }
 
 void WebCookieManagerProxy::getHTTPCookieAcceptPolicy(std::function<void (HTTPCookieAcceptPolicy, CallbackBase::Error)> callbackFunction)

diff --git a/Source/WebKit2/UIProcess/WebProcessPool.h b/Source/WebKit2/UIProcess/WebProcessPool.h
index 2f65559..0960531 100644 (file)
--- a/Source/WebKit2/UIProcess/WebProcessPool.h
+++ b/Source/WebKit2/UIProcess/WebProcessPool.h
@@ -246,6 +246,7 @@ public:
     void setCookieStorageDirectory(const String& dir) { m_overrideCookieStorageDirectory = dir; }
 
     void useTestingNetworkSession();
+    bool isUsingTestingNetworkSession() const { return m_shouldUseTestingNetworkSession; }
 
     void allowSpecificHTTPSCertificateForHost(const WebCertificateInfo*, const String& host);
 

diff --git a/Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundle.cpp b/Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundle.cpp
index 16144da..6b97e91 100644 (file)
--- a/Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundle.cpp
+++ b/Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundle.cpp
@@ -83,11 +83,6 @@ size_t WKBundleGetJavaScriptObjectsCount(WKBundleRef bundleRef)
     return toImpl(bundleRef)->javaScriptObjectsCount();
 }
 
-void WKBundleSetAlwaysAcceptCookies(WKBundleRef bundleRef, bool accept)
-{
-    toImpl(bundleRef)->setAlwaysAcceptCookies(accept);
-}
-
 void WKBundleAddUserScript(WKBundleRef bundleRef, WKBundlePageGroupRef pageGroupRef, WKBundleScriptWorldRef scriptWorldRef, WKStringRef sourceRef, WKURLRef urlRef, WKArrayRef whitelistRef, WKArrayRef blacklistRef, _WKUserScriptInjectionTime injectionTimeRef, WKUserContentInjectedFrames injectedFramesRef)
 {
     toImpl(bundleRef)->addUserScript(toImpl(pageGroupRef), toImpl(scriptWorldRef), toWTFString(sourceRef), toWTFString(urlRef), toImpl(whitelistRef), toImpl(blacklistRef), toUserScriptInjectionTime(injectionTimeRef), toUserContentInjectedFrames(injectedFramesRef));

diff --git a/Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePrivate.h b/Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePrivate.h
index b8d6717..091f8fd 100644 (file)
--- a/Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePrivate.h
+++ b/Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePrivate.h
@@ -40,8 +40,7 @@
 extern "C" {
 #endif
 
-// TestRunner only SPI
-WK_EXPORT void WKBundleSetAlwaysAcceptCookies(WKBundleRef bundle, bool);
+// TestRunner only SPIs.
 // Will make WebProcess ignore this preference until a preferences change notification, only for WebKitTestRunner use.
 WK_EXPORT void WKBundleOverrideBoolPreferenceForTestRunner(WKBundleRef bundle, WKBundlePageGroupRef pageGroup, WKStringRef preference, bool enabled);
 WK_EXPORT void WKBundleSetAllowUniversalAccessFromFileURLs(WKBundleRef bundle, WKBundlePageGroupRef pageGroup, bool enabled);

diff --git a/Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.cpp b/Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.cpp
index 0f9406c..e8af3be 100644 (file)
--- a/Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.cpp
+++ b/Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.cpp
@@ -136,11 +136,6 @@ WebConnection* InjectedBundle::webConnectionToUIProcess() const
     return WebProcess::singleton().webConnectionToUIProcess();
 }
 
-void InjectedBundle::setAlwaysAcceptCookies(bool accept)
-{
-    WebProcess::singleton().supplement<WebCookieManager>()->setHTTPCookieAcceptPolicy(accept ? HTTPCookieAcceptPolicyAlways : HTTPCookieAcceptPolicyOnlyFromMainDocumentDomain);
-}
-
 void InjectedBundle::overrideBoolPreferenceForTestRunner(WebPageGroupProxy* pageGroup, const String& preference, bool enabled)
 {
     const HashSet<Page*>& pages = PageGroup::pageGroup(pageGroup->identifier())->pages();

diff --git a/Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.h b/Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.h
index 9b52693..8834658 100644 (file)
--- a/Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.h
+++ b/Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.h
@@ -97,7 +97,6 @@ public:
     WebConnection* webConnectionToUIProcess() const;
 
     // TestRunner only SPI
-    void setAlwaysAcceptCookies(bool);
     void overrideBoolPreferenceForTestRunner(WebPageGroupProxy*, const String& preference, bool enabled);
     void overrideXSSAuditorEnabledForTestRunner(WebPageGroupProxy* pageGroup, bool enabled);
     void setAllowUniversalAccessFromFileURLs(WebPageGroupProxy*, bool);

diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index 2ec3dee..55dd766 100644 (file)
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,24 @@
+2015-03-12  Alexey Proskuryakov  <ap@apple.com>
+
+        testRunner.setAlwaysAcceptCookies does not work with NetworkProcess
+        https://bugs.webkit.org/show_bug.cgi?id=138687
+
+        Reviewed by Carlos Garcia Campos.
+
+        Always change cookie policy from UI process, as changing it from WebProcess doesn't
+        affect other processes.
+
+        The implementation is imperfect, because the change happens asynchronously, but I
+        don't expect it to be a problem in practice.
+
+        * WebKitTestRunner/InjectedBundle/InjectedBundle.cpp:
+        (WTR::InjectedBundle::beginTesting):
+        * WebKitTestRunner/InjectedBundle/TestRunner.cpp:
+        (WTR::TestRunner::setAlwaysAcceptCookies):
+        * WebKitTestRunner/TestInvocation.cpp:
+        (WTR::TestInvocation::invoke):
+        (WTR::TestInvocation::didReceiveSynchronousMessageFromInjectedBundle):
+
 2015-03-12  Csaba Osztrogonác  <ossy@webkit.org>
 
         [EFL] Enable building EFL 1.13 with NEON support

diff --git a/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp b/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp
index 615716c..d57081b 100644 (file)
--- a/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp
+++ b/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp
@@ -249,7 +249,6 @@ void InjectedBundle::beginTesting(WKDictionaryRef settings)
     WKBundleSetAllowFileAccessFromFileURLs(m_bundle, m_pageGroup, true);
     WKBundleSetPluginsEnabled(m_bundle, m_pageGroup, true);
     WKBundleSetPopupBlockingEnabled(m_bundle, m_pageGroup, false);
-    WKBundleSetAlwaysAcceptCookies(m_bundle, false); // FIXME: Do this from UI process, so that Networking process gets the preference, too.
     WKBundleSetSerialLoadingEnabled(m_bundle, false);
 
     WKBundleRemoveAllUserContent(m_bundle, m_pageGroup);

diff --git a/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp b/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp
index 569886c..285f59b 100644 (file)
--- a/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp
+++ b/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp
@@ -601,7 +601,11 @@ void TestRunner::overridePreference(JSStringRef preference, JSStringRef value)
 
 void TestRunner::setAlwaysAcceptCookies(bool accept)
 {
-    WKBundleSetAlwaysAcceptCookies(InjectedBundle::singleton().bundle(), accept);
+    WKRetainPtr<WKStringRef> messageName(AdoptWK, WKStringCreateWithUTF8CString("SetAlwaysAcceptCookies"));
+
+    WKRetainPtr<WKBooleanRef> messageBody(AdoptWK, WKBooleanCreate(accept));
+
+    WKBundlePostSynchronousMessage(InjectedBundle::singleton().bundle(), messageName.get(), messageBody.get(), 0);
 }
 
 double TestRunner::preciseTime()

diff --git a/Tools/WebKitTestRunner/TestInvocation.cpp b/Tools/WebKitTestRunner/TestInvocation.cpp
index eca9c10..712b6ff 100644 (file)
--- a/Tools/WebKitTestRunner/TestInvocation.cpp
+++ b/Tools/WebKitTestRunner/TestInvocation.cpp
@@ -31,6 +31,7 @@
 #include "StringFunctions.h"
 #include "TestController.h"
 #include <WebKit/WKContextPrivate.h>
+#include <WebKit/WKCookieManager.h>
 #include <WebKit/WKData.h>
 #include <WebKit/WKDictionary.h>
 #include <WebKit/WKInspector.h>
@@ -151,6 +152,8 @@ void TestInvocation::invoke()
 
     TestController::singleton().setShouldLogHistoryClientCallbacks(shouldLogHistoryClientCallbacks());
 
+    WKCookieManagerSetHTTPCookieAcceptPolicy(WKContextGetCookieManager(TestController::singleton().context()), kWKHTTPCookieAcceptPolicyOnlyFromMainDocumentDomain);
+
     // FIXME: We should clear out visited links here.
 
     WKRetainPtr<WKStringRef> messageName = adoptWK(WKStringCreateWithUTF8CString("BeginTest"));
@@ -642,6 +645,15 @@ WKRetainPtr<WKTypeRef> TestInvocation::didReceiveSynchronousMessageFromInjectedB
 #endif
         return result;
     }
+
+    if (WKStringIsEqualToUTF8CString(messageName, "SetAlwaysAcceptCookies")) {
+        WKBooleanRef accept = static_cast<WKBooleanRef>(messageBody);
+        WKHTTPCookieAcceptPolicy policy = WKBooleanGetValue(accept) ? kWKHTTPCookieAcceptPolicyAlways : kWKHTTPCookieAcceptPolicyOnlyFromMainDocumentDomain;
+        // FIXME: This updates the policy in WebProcess and in NetworkProcess asynchronously, which might break some tests' expectations.
+        WKCookieManagerSetHTTPCookieAcceptPolicy(WKContextGetCookieManager(TestController::singleton().context()), policy);
+        return 0;
+    }
+
     ASSERT_NOT_REACHED();
     return 0;
 }