Changeset 181446 in webkit

Timestamp:

Mar 12, 2015, 11:38:11 AM (11 years ago)

Author:

ap@apple.com

Message:

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/gtk/TestExpectations (modified) (1 diff)
• LayoutTests/platform/mac-wk2/TestExpectations (modified) (1 diff)
• Source/WebKit2/ChangeLog (modified) (1 diff)
• Source/WebKit2/UIProcess/WebCookieManagerProxy.cpp (modified) (2 diffs)
• Source/WebKit2/UIProcess/WebProcessPool.h (modified) (1 diff)
• Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundle.cpp (modified) (1 diff)
• Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePrivate.h (modified) (1 diff)
• Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.cpp (modified) (1 diff)
• Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.h (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp (modified) (1 diff)
• Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp (modified) (1 diff)
• Tools/WebKitTestRunner/TestInvocation.cpp (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/LayoutTests/platform/gtk/TestExpectations

@@ -1558 +1558 @@
 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 ]

trunk/LayoutTests/platform/mac-wk2/TestExpectations

@@ -469 +469 @@
 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.

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebKit2/UIProcess/WebCookieManagerProxy.cpp

@@ -153 +153 @@
 {
 #if PLATFORM(COCOA)
-    persistHTTPCookieAcceptPolicy(policy);
+    if (!processPool()->isUsingTestingNetworkSession())
+        persistHTTPCookieAcceptPolicy(policy);
 #endif
 #if USE(SOUP)
@@ -159 +160 @@
 #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
 }
 

trunk/Source/WebKit2/UIProcess/WebProcessPool.h

@@ -247 +247 @@
 
     void useTestingNetworkSession();
+    bool isUsingTestingNetworkSession() const { return m_shouldUseTestingNetworkSession; }
 
     void allowSpecificHTTPSCertificateForHost(const WebCertificateInfo*, const String& host);

trunk/Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundle.cpp

@@ -84 +84 @@
 }
 
-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)
 {

trunk/Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePrivate.h

@@ -41 +41 @@
 #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);

trunk/Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.cpp

@@ -137 +137 @@
 }
 
-void InjectedBundle::setAlwaysAcceptCookies(bool accept)
-{
-    WebProcess::singleton().supplement<WebCookieManager>()->setHTTPCookieAcceptPolicy(accept ? HTTPCookieAcceptPolicyAlways : HTTPCookieAcceptPolicyOnlyFromMainDocumentDomain);
-}
-
 void InjectedBundle::overrideBoolPreferenceForTestRunner(WebPageGroupProxy* pageGroup, const String& preference, bool enabled)
 {

trunk/Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.h

@@ -98 +98 @@
 
     // TestRunner only SPI
-    void setAlwaysAcceptCookies(bool);
     void overrideBoolPreferenceForTestRunner(WebPageGroupProxy*, const String& preference, bool enabled);
     void overrideXSSAuditorEnabledForTestRunner(WebPageGroupProxy* pageGroup, bool enabled);

trunk/Tools/ChangeLog

@@ +1 @@
+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>
 

trunk/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp

@@ -250 +250 @@
     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);
 

trunk/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp

@@ -602 +602 @@
 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);
 }
 

trunk/Tools/WebKitTestRunner/TestInvocation.cpp

@@ -32 +32 @@
 #include "TestController.h"
 #include <WebKit/WKContextPrivate.h>
+#include <WebKit/WKCookieManager.h>
 #include <WebKit/WKData.h>
 #include <WebKit/WKDictionary.h>
@@ -152 +153 @@
     TestController::singleton().setShouldLogHistoryClientCallbacks(shouldLogHistoryClientCallbacks());
 
+    WKCookieManagerSetHTTPCookieAcceptPolicy(WKContextGetCookieManager(TestController::singleton().context()), kWKHTTPCookieAcceptPolicyOnlyFromMainDocumentDomain);
+
     // FIXME: We should clear out visited links here.
 
@@ -643 +646 @@
         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;