Web Automation: cookie-related commands don't work correctly

https://bugs.webkit.org/show_bug.cgi?id=171713 <rdar://problem/29829930> Reviewed by Alexey Proskuryakov. Commands that use WebCookieManager directly should complete when the manager's completion handler is called. Otherwise, this will race with subsequent accesses to cookies via the web process (document.cookie). Also, these commands need to use the active browsing context's session ID. They currently use the process pool's storage session, which is wrong since we specially configure automation instances with an ephemeral store. * UIProcess/Automation/WebAutomationSession.cpp: (WebKit::WebAutomationSession::addSingleCookie): (WebKit::WebAutomationSession::deleteAllCookies): Canonical link: https://commits.webkit.org/188641@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@216261 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · May 5, 2017 · 8105b13 · 8105b13
1 parent 79826a6
commit 8105b13
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 4 deletions.
diff --git a/Source/WebKit2/ChangeLog b/Source/WebKit2/ChangeLog
@@ -1,3 +1,23 @@
+2017-05-05  Brian Burg  <bburg@apple.com>
+
+        Web Automation: cookie-related commands don't work correctly
+        https://bugs.webkit.org/show_bug.cgi?id=171713
+        <rdar://problem/29829930>
+
+        Reviewed by Alexey Proskuryakov.
+
+        Commands that use WebCookieManager directly should complete when
+        the manager's completion handler is called. Otherwise, this will race
+        with subsequent accesses to cookies via the web process (document.cookie).
+
+        Also, these commands need to use the active browsing context's session ID.
+        They currently use the process pool's storage session, which is wrong
+        since we specially configure automation instances with an ephemeral store.
+
+        * UIProcess/Automation/WebAutomationSession.cpp:
+        (WebKit::WebAutomationSession::addSingleCookie):
+        (WebKit::WebAutomationSession::deleteAllCookies):
+
 2017-05-05  Chris Dumez  <cdumez@apple.com>
 
         Rename webProcessDidCrashWithReason callback to webProcessDidTerminate and stop calling webProcessDidCrash for client terminations

diff --git a/Source/WebKit2/UIProcess/Automation/WebAutomationSession.cpp b/Source/WebKit2/UIProcess/Automation/WebAutomationSession.cpp
@@ -819,9 +819,12 @@ void WebAutomationSession::addSingleCookie(ErrorString& errorString, const Strin
 
     // FIXME: Using activeURL here twice is basically saying "this is always in the context of the main document"
     // which probably isn't accurate.
-    cookieManager->setCookies(WebCore::SessionID::defaultSessionID(), { cookie }, activeURL, activeURL, [](CallbackBase::Error){});
-
-    callback->sendSuccess();
+    cookieManager->setCookies(page->websiteDataStore().sessionID(), { cookie }, activeURL, activeURL, [callback = callback.copyRef()](CallbackBase::Error error) {
+        if (error == CallbackBase::Error::None)
+            callback->sendSuccess();
+        else
+            callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_NAME(InternalError));
+    });
 }
 
 void WebAutomationSession::deleteAllCookies(ErrorString& errorString, const String& browsingContextHandle)
@@ -834,7 +837,7 @@ void WebAutomationSession::deleteAllCookies(ErrorString& errorString, const Stri
     ASSERT(activeURL.isValid());
 
     WebCookieManagerProxy* cookieManager = m_processPool->supplement<WebCookieManagerProxy>();
-    cookieManager->deleteCookiesForHostname(WebCore::SessionID::defaultSessionID(), activeURL.host());
+    cookieManager->deleteCookiesForHostname(page->websiteDataStore().sessionID(), activeURL.host());
 }
 
 #if USE(APPKIT) || PLATFORM(GTK)