Flush Channel IDs when Cookies get saved to a persistent backend

chrome/browser/profiles/profile_impl_io_data.cc

Index: chrome/browser/profiles/profile_impl_io_data.cc
diff --git a/chrome/browser/profiles/profile_impl_io_data.cc b/chrome/browser/profiles/profile_impl_io_data.cc
index 8eb31578fc4518f17fc7f0bbfd1de7c74d4e5643..005d4c2f79a7bb3c0e44eb94f9d1fd2f40465c55 100644
--- a/chrome/browser/profiles/profile_impl_io_data.cc
+++ b/chrome/browser/profiles/profile_impl_io_data.cc
@@ -490,6 +490,21 @@ void ProfileImplIOData::InitializeInternal(
 
   main_context->set_proxy_service(proxy_service());
 
+  scoped_refptr<base::SequencedTaskRunner> background_task_runner =

[mattm, 2017/05/17 23:28:09]

may want to add comment or different naming to convey why the cookie store and
channel id store use the same task runner, and that this isn't just a background
task runner any other random thing should use (unless it also needs to be
serialized with the cookie/channel id tasks for some reason)

[nharper, 2017/05/18 00:45:53]

Done.

+      base::CreateSequencedTaskRunnerWithTraits(
+          {base::MayBlock(), base::TaskPriority::BACKGROUND,
+           base::TaskShutdownBehavior::BLOCK_SHUTDOWN});
+
+  // Set up server bound cert service.
+  DCHECK(!lazy_params_->channel_id_path.empty());
+  scoped_refptr<QuotaPolicyChannelIDStore> channel_id_db =
+      new QuotaPolicyChannelIDStore(lazy_params_->channel_id_path,
+                                    background_task_runner,
+                                    lazy_params_->special_storage_policy.get());
+  main_context_storage->set_channel_id_service(
+      base::MakeUnique<net::ChannelIDService>(
+          new net::DefaultChannelIDStore(channel_id_db.get())));
+
   // Set up cookie store.
   DCHECK(!lazy_params_->cookie_path.empty());
 
@@ -498,21 +513,11 @@ void ProfileImplIOData::InitializeInternal(
       lazy_params_->special_storage_policy.get(),
       profile_params->cookie_monster_delegate.get());
   cookie_config.crypto_delegate = cookie_config::GetCookieCryptoDelegate();
+  cookie_config.channel_id_service = main_context->channel_id_service();

[mattm, 2017/05/17 23:28:08]

Should this also be done in
android_webview/browser/net/aw_url_request_context_getter.cc ? (I have no idea
what's going on with AwCookieStoreWrapper)

ios/chrome/browser/browser_state/chrome_browser_state_impl_io_data.mm and
ios/chrome/browser/browser_state/off_the_record_chrome_browser_state_io_data.mm?
(Dunno if it matters for iOS these days)

[nharper, 2017/05/18 00:45:53]

android webview has a CookieManager::GetCookieStore() method which returns a
singleton net::CookieStore* - doing this in android would require changing how
the CookieManager constructs its singleton. I don't think it's worth it to mess
with that right now.
I could wire it up for iOS - its cookie_util::CookieStoreConfig and
cookie_util::CreateCookieStore are pretty similar to the same struct/function in
the content namespace. I don't think it's worth doing though, given that it's
not used for much.

+  cookie_config.background_task_runner = background_task_runner;
   main_context_storage->set_cookie_store(
       content::CreateCookieStore(cookie_config));
 
-  // Set up server bound cert service.
-  DCHECK(!lazy_params_->channel_id_path.empty());
-  scoped_refptr<QuotaPolicyChannelIDStore> channel_id_db =
-      new QuotaPolicyChannelIDStore(
-          lazy_params_->channel_id_path,
-          base::CreateSequencedTaskRunnerWithTraits(
-              {base::MayBlock(), base::TaskPriority::BACKGROUND}),
-          lazy_params_->special_storage_policy.get());
-  main_context_storage->set_channel_id_service(
-      base::MakeUnique<net::ChannelIDService>(
-          new net::DefaultChannelIDStore(channel_id_db.get())));
-
   main_context->cookie_store()->SetChannelIDServiceID(

[mattm, 2017/05/17 23:28:09]

Seems like this could be obviated as ChannelIDService passed in the
cookie_config now, or are there some differences in how they are used? 

[nharper, 2017/05/18 00:45:53]

They should get merged together - I'd prefer to postpone that to a future CL
that brings the two stores closer to being one instead of piling more things
onto this CL. (The SetChannelIDServiceID method is used for debugging/metrics,
while the c'tor arg is used for synchronizing Flushing, so there is a
difference, but it's always the same ChannelIDService/Store.)

       main_context->channel_id_service()->GetUniqueID());
 
@@ -585,6 +590,7 @@ void ProfileImplIOData::
   cookie_config.crypto_delegate = cookie_config::GetCookieCryptoDelegate();
   // Enable cookies for chrome-extension URLs.
   cookie_config.cookieable_schemes.push_back(extensions::kExtensionScheme);
+  cookie_config.channel_id_service = extensions_context->channel_id_service();
   extensions_cookie_store_ = content::CreateCookieStore(cookie_config);
   extensions_context->set_cookie_store(extensions_cookie_store_.get());
   if (extensions_context->channel_id_service()) {
@@ -639,28 +645,33 @@ net::URLRequestContext* ProfileImplIOData::InitializeAppRequestContext(
 
   std::unique_ptr<net::CookieStore> cookie_store;
   scoped_refptr<net::SQLiteChannelIDStore> channel_id_db;
+  scoped_refptr<base::SequencedTaskRunner> background_task_runner =

[mattm, 2017/05/17 23:28:09]

same

[nharper, 2017/05/18 00:45:53]

Done.

+      base::CreateSequencedTaskRunnerWithTraits(
+          {base::MayBlock(), base::TaskPriority::BACKGROUND,
+           base::TaskShutdownBehavior::BLOCK_SHUTDOWN});
   if (partition_descriptor.in_memory) {
-    cookie_store = content::CreateCookieStore(content::CookieStoreConfig());
-  } else {
+    cookie_path = base::FilePath();
+  }
+  content::CookieStoreConfig cookie_config(
+      cookie_path, content::CookieStoreConfig::EPHEMERAL_SESSION_COOKIES,
+      nullptr, nullptr);
+  if (!partition_descriptor.in_memory) {
     // Use an app-specific cookie store.
     DCHECK(!cookie_path.empty());
 
     // TODO(creis): We should have a cookie delegate for notifying the cookie
     // extensions API, but we need to update it to understand isolated apps
     // first.
-    content::CookieStoreConfig cookie_config(
-        cookie_path, content::CookieStoreConfig::EPHEMERAL_SESSION_COOKIES,
-        nullptr, nullptr);
     cookie_config.crypto_delegate = cookie_config::GetCookieCryptoDelegate();
-    cookie_store = content::CreateCookieStore(cookie_config);
-    channel_id_db = new net::SQLiteChannelIDStore(
-        channel_id_path,
-        base::CreateSequencedTaskRunnerWithTraits(
-            {base::MayBlock(), base::TaskPriority::BACKGROUND}));
+    channel_id_db =
+        new net::SQLiteChannelIDStore(channel_id_path, background_task_runner);
   }
   std::unique_ptr<net::ChannelIDService> channel_id_service(
       new net::ChannelIDService(
           new net::DefaultChannelIDStore(channel_id_db.get())));
+  cookie_config.channel_id_service = channel_id_service.get();
+  cookie_config.background_task_runner = background_task_runner;
+  cookie_store = content::CreateCookieStore(cookie_config);
   cookie_store->SetChannelIDServiceID(channel_id_service->GetUniqueID());
 
   // Build a new HttpNetworkSession that uses the new ChannelIDService.