Change document.cookie setter to use a sync IPC call.

This is needed to avoid race conditions with setting a cookie and making a request right away.
Currently it works when cookies and request handling are dispatched on the same thread and
associated mojo interfaces are used. However this will stop working if non-associated interfaces
are used, which will be the case when the cookie and loading interfaces live in different processes.
More background here:
https://docs.google.com/a/chromium.org/document/d/1eg1ohplfFGrlz5gFNgbJflRdIljPqCj58p4Zfb81QGM

We don't anticipate this having a perceptible perf impact, but added UMA stats to confirm. Cookie
getters are already sync, and this new sync call replies right when the IPC is dispatched (as
opposed to when the cookie database is updated).

BUG=721395

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: Icb7afe358a9fa969cd6fed1c2c5730fa9a18a2ec
Reviewed-on: https://chromium-review.googlesource.com/742345
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513123}

content/browser/frame_host/render_frame_message_filter.cc

@@ -442,7 +442,9 @@ void RenderFrameMessageFilter::OnRenderProcessGone() {
 void RenderFrameMessageFilter::SetCookie(int32_t render_frame_id,
                                          const GURL& url,
                                          const GURL& site_for_cookies,
-                                         const std::string& cookie) {
+                                         const std::string& cookie,
+                                         SetCookieCallback callback) {
+  std::move(callback).Run();
   ChildProcessSecurityPolicyImpl* policy =
       ChildProcessSecurityPolicyImpl::GetInstance();
   if (!policy->CanAccessDataForOrigin(render_process_id_, url)) {

content/browser/frame_host/render_frame_message_filter.h

@@ -127,7 +127,8 @@ class CONTENT_EXPORT RenderFrameMessageFilter
   void SetCookie(int32_t render_frame_id,
                  const GURL& url,
                  const GURL& site_for_cookies,
-                 const std::string& cookie) override;
+                 const std::string& cookie,
+                 SetCookieCallback callback) override;
   void GetCookies(int render_frame_id,
                   const GURL& url,
                   const GURL& site_for_cookies,

content/browser/frame_host/render_frame_message_filter_browsertest.cc

@@ -4,6 +4,8 @@
 
 #include <string>
 
+#include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/test/histogram_tester.h"
@@ -234,10 +236,11 @@ IN_PROC_BROWSER_TEST_F(RenderFrameMessageFilterBrowserTest,
       ->PostTask(FROM_HERE, base::BindOnce(
                                 [](RenderFrameHost* frame) {
                                   GetFilterForProcess(frame->GetProcess())
-                                      ->SetCookie(frame->GetRoutingID(),
-                                                  GURL("https://baz.com/"),
-                                                  GURL("https://baz.com/"),
-                                                  "pwn=ed");
+                                      ->SetCookie(
+                                          frame->GetRoutingID(),
+                                          GURL("https://baz.com/"),
+                                          GURL("https://baz.com/"), "pwn=ed",
+                                          base::BindOnce(&base::DoNothing));
                                 },
                                 main_frame));
 

content/common/render_frame_message_filter.mojom

@@ -7,10 +7,13 @@ module content.mojom;
 import "url/mojo/url.mojom";
 
 interface RenderFrameMessageFilter {
-  // Sets a cookie. The cookie is set asynchronously, but will be available to
-  // any subsequent GetCookies() request.
+  // Sets a cookie. Returns after the cookie write request has been scheduled on
+  // the IO thread (the database is modified asynchronously). This ensures that
+  // network requests issued after the cookie setter call are processed after
+  // the cookie change is committed, and therefore see the change.
+  [Sync]
   SetCookie(int32 render_frame_id, url.mojom.Url url,
-      url.mojom.Url first_party_for_cookies, string cookie);
+      url.mojom.Url first_party_for_cookies, string cookie) => ();
 
   // Used to get cookies for the given URL. This may block waiting for a
   // previous SetCookie message to be processed.

content/renderer/renderer_webcookiejar_impl.cc

@@ -4,6 +4,8 @@
 
 #include "content/renderer/renderer_webcookiejar_impl.h"
 
+#include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/common/frame_messages.h"
 #include "content/public/renderer/content_renderer_client.h"
@@ -21,7 +23,8 @@ void RendererWebCookieJarImpl::SetCookie(const WebURL& url,
   std::string value_utf8 =
       value.Utf8(WebString::UTF8ConversionMode::kStrictReplacingErrorsWithFFFD);
   RenderThreadImpl::current()->render_frame_message_filter()->SetCookie(
-      sender_->GetRoutingID(), url, site_for_cookies, value_utf8);
+      sender_->GetRoutingID(), url, site_for_cookies, value_utf8,
+      base::BindOnce(&base::DoNothing));
 }
 
 WebString RendererWebCookieJarImpl::Cookies(const WebURL& url,

third_party/WebKit/Source/core/loader/CookieJar.cpp

@@ -61,6 +61,7 @@ void SetCookies(Document* document,
   WebCookieJar* cookie_jar = ToCookieJar(document);
   if (!cookie_jar)
     return;
+  SCOPED_BLINK_UMA_HISTOGRAM_TIMER("Blink.CookieJar.SyncCookiesSetTime");
   cookie_jar->SetCookie(url, document->SiteForCookies(), cookie_string);
 }
 

tools/metrics/histograms/histograms.xml

@@ -6572,6 +6572,12 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   </summary>
 </histogram>
 
+<histogram name="Blink.CookieJar.SyncCookiesSetTime" units="microseconds">
+  <owner>kinuko@chromium.org</owner>
+  <owner>dcheng@chromium.org</owner>
+  <summary>Microseconds per sync IPC call to set cookies.</summary>
+</histogram>
+
 <histogram name="Blink.CookieJar.SyncCookiesTime" units="microseconds">
   <owner>kinuko@chromium.org</owner>
   <owner>dcheng@chromium.org</owner>