Update WKSystemCookieStore and CookieCreationTimeManager Thread usage.

1- Updates WKSystemCookieStore so it can use wkcookiestore only from
   the main thread.
2- Update the CookieCreationTimeManager to only be used on IO thread
   (aside from creation on Main thread)

Bug: 779106,759229
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ifa0f9e5bb2d2c33c488759467dc8a25403f6e0ed
Reviewed-on: https://chromium-review.googlesource.com/809531
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521954}

ios/net/cookies/cookie_creation_time_manager.h

@@ -8,6 +8,7 @@
 #include <set>
 
 #include "base/containers/hash_tables.h"
+#include "base/memory/weak_ptr.h"
 #include "base/threading/thread_checker.h"
 #include "base/time/time.h"
 
@@ -17,6 +18,8 @@ namespace net {
 
 // The CookieCreationTimeManager allows to get and set the creation time of a
 // NSHTTPCookie.
+// All the methods of CookieCreationTimeManager must be called on the IO thread,
+// except its constructor that can be called from any thread.
 // Creation time data is stored either in the cookie properties (when the cookie
 // is created by the system) or in |creation_times_| (when the cookie is created
 // by CookieStoreIOS). When both are available, |creation_times_| is used,
@@ -37,11 +40,14 @@ class CookieCreationTimeManager {
   void DeleteCreationTime(NSHTTPCookie* cookie);
   // Clears all the creation times.
   void Clear();
+  // Gets base::WeakPtr to the object to be used in sorting.
+  base::WeakPtr<CookieCreationTimeManager> GetWeakPtr();
 
  private:
   base::hash_map<std::string, base::Time> creation_times_;
   std::set<base::Time> unique_times_;
   base::ThreadChecker thread_checker_;
+  base::WeakPtrFactory<CookieCreationTimeManager> weak_factory_;
 };
 
 }  // namespace net

ios/net/cookies/cookie_creation_time_manager.mm

@@ -49,7 +49,8 @@ std::string GetCookieUniqueID(NSHTTPCookie* cookie) {
 
 namespace net {
 
-CookieCreationTimeManager::CookieCreationTimeManager() {
+CookieCreationTimeManager::CookieCreationTimeManager() : weak_factory_(this) {
+  DETACH_FROM_THREAD(thread_checker_);
 }
 
 CookieCreationTimeManager::~CookieCreationTimeManager() {
@@ -122,4 +123,9 @@ void CookieCreationTimeManager::Clear() {
   unique_times_.clear();
 }
 
+base::WeakPtr<CookieCreationTimeManager>
+CookieCreationTimeManager::GetWeakPtr() {
+  return weak_factory_.GetWeakPtr();
+}
+
 }  // namespace net

ios/net/cookies/system_cookie_store.h

@@ -74,8 +74,8 @@ class SystemCookieStore {
   base::WeakPtr<SystemCookieStore> GetWeakPtr();
 
  protected:
-  // Compares cookies based on the path lengths and the creation times, as per
-  // RFC6265.
+  // Compares cookies based on the path lengths and the creation times provided
+  // by a non null creation time manager |context|, as per RFC6265.
   static NSInteger CompareCookies(id a, id b, void* context);
 
   // Internal cookie stores doesn't store creation time. This object is used

ios/net/cookies/system_cookie_store.mm

@@ -36,6 +36,7 @@ base::WeakPtr<SystemCookieStore> SystemCookieStore::GetWeakPtr() {
 
 // protected static
 NSInteger SystemCookieStore::CompareCookies(id a, id b, void* context) {
+  DCHECK(context);
   NSHTTPCookie* cookie_a = static_cast<NSHTTPCookie*>(a);
   NSHTTPCookie* cookie_b = static_cast<NSHTTPCookie*>(b);
   // Compare path lengths first.

ios/web/net/cookies/wk_http_system_cookie_store.mm

@@ -65,71 +65,95 @@ void WKHTTPSystemCookieStore::GetCookiesForURLAsync(
     SystemCookieCallbackForCookies callback) {
   __block SystemCookieCallbackForCookies shared_callback = std::move(callback);
   GURL block_url = url;
-  [cookie_store_ getAllCookies:^(NSArray<NSHTTPCookie*>* cookies) {
-    NSMutableArray* result = [NSMutableArray array];
-    for (NSHTTPCookie* cookie in cookies) {
-      if (ShouldIncludeForRequestUrl(cookie, block_url)) {
-        [result addObject:cookie];
-      }
-    }
-    RunSystemCookieCallbackForCookies(std::move(shared_callback), result);
-  }];
+  web::WebThread::PostTask(
+      web::WebThread::UI, FROM_HERE, base::BindBlockArc(^{
+        [cookie_store_ getAllCookies:^(NSArray<NSHTTPCookie*>* cookies) {
+          NSMutableArray* result = [NSMutableArray array];
+          for (NSHTTPCookie* cookie in cookies) {
+            if (ShouldIncludeForRequestUrl(cookie, block_url)) {
+              [result addObject:cookie];
+            }
+          }
+          RunSystemCookieCallbackForCookies(std::move(shared_callback), result);
+        }];
+      }));
 }
 
 void WKHTTPSystemCookieStore::GetAllCookiesAsync(
     SystemCookieCallbackForCookies callback) {
   __block SystemCookieCallbackForCookies shared_callback = std::move(callback);
-  [cookie_store_ getAllCookies:^(NSArray<NSHTTPCookie*>* cookies) {
-    RunSystemCookieCallbackForCookies(std::move(shared_callback), cookies);
-  }];
+  web::WebThread::PostTask(
+      web::WebThread::UI, FROM_HERE, base::BindBlockArc(^{
+        [cookie_store_ getAllCookies:^(NSArray<NSHTTPCookie*>* cookies) {
+          RunSystemCookieCallbackForCookies(std::move(shared_callback),
+                                            cookies);
+        }];
+      }));
 }
 
 void WKHTTPSystemCookieStore::DeleteCookieAsync(NSHTTPCookie* cookie,
                                                 SystemCookieCallback callback) {
   __block SystemCookieCallback shared_callback = std::move(callback);
-  [cookie_store_ deleteCookie:cookie
-            completionHandler:^{
-              creation_time_manager_->DeleteCreationTime(cookie);
-              RunSystemCookieCallbackOnIOThread(std::move(shared_callback));
-            }];
+  NSHTTPCookie* block_cookie = cookie;
+  web::WebThread::PostTask(
+      web::WebThread::UI, FROM_HERE, base::BindBlockArc(^{
+        [cookie_store_ deleteCookie:block_cookie
+                  completionHandler:^{
+                    RunSystemCookieCallbackOnIOThread(base::BindBlockArc(^{
+                      creation_time_manager_->DeleteCreationTime(block_cookie);
+                      if (!shared_callback.is_null())
+                        std::move(shared_callback).Run();
+                    }));
+                  }];
+      }));
 }
 void WKHTTPSystemCookieStore::SetCookieAsync(
     NSHTTPCookie* cookie,
     const base::Time* optional_creation_time,
     SystemCookieCallback callback) {
   __block SystemCookieCallback shared_callback = std::move(callback);
-  [cookie_store_ setCookie:cookie
-         completionHandler:^{
-           // Set creation time as soon as possible
-           base::Time cookie_time = base::Time::Now();
-           if (optional_creation_time && !optional_creation_time->is_null())
-             cookie_time = *optional_creation_time;
-
-           creation_time_manager_->SetCreationTime(
-               cookie,
-               creation_time_manager_->MakeUniqueCreationTime(cookie_time));
-           RunSystemCookieCallbackOnIOThread(std::move(shared_callback));
-         }];
+  NSHTTPCookie* block_cookie = cookie;
+  base::Time cookie_time = base::Time::Now();
+  if (optional_creation_time && !optional_creation_time->is_null())
+    cookie_time = *optional_creation_time;
+
+  web::WebThread::PostTask(
+      web::WebThread::UI, FROM_HERE, base::BindBlockArc(^{
+        [cookie_store_ setCookie:block_cookie
+               completionHandler:^{
+                 RunSystemCookieCallbackOnIOThread(base::BindBlockArc(^{
+                   creation_time_manager_->SetCreationTime(
+                       block_cookie,
+                       creation_time_manager_->MakeUniqueCreationTime(
+                           cookie_time));
+                   if (!shared_callback.is_null())
+                     std::move(shared_callback).Run();
+                 }));
+               }];
+      }));
 }
 
 void WKHTTPSystemCookieStore::ClearStoreAsync(SystemCookieCallback callback) {
   __block SystemCookieCallback shared_callback = std::move(callback);
-  [cookie_store_ getAllCookies:^(NSArray<NSHTTPCookie*>* cookies) {
-
-    scoped_refptr<CallbackCounter> callback_counter =
-        new CallbackCounter(base::BindBlockArc(^{
-          creation_time_manager_->Clear();
-          RunSystemCookieCallbackOnIOThread(std::move(shared_callback));
-        }));
-    // Add callback for each cookie
-    callback_counter->IncrementCount(cookies.count);
-    for (NSHTTPCookie* cookie in cookies) {
-      [cookie_store_ deleteCookie:cookie
-                completionHandler:^{
-                  callback_counter->DecrementCount();
-                }];
-    }
-  }];
+  web::WebThread::PostTask(
+      web::WebThread::UI, FROM_HERE, base::BindBlockArc(^{
+        [cookie_store_ getAllCookies:^(NSArray<NSHTTPCookie*>* cookies) {
+          scoped_refptr<CallbackCounter> callback_counter =
+              new CallbackCounter(base::BindRepeating(
+                  &RunSystemCookieCallbackOnIOThread, base::BindBlockArc(^{
+                    creation_time_manager_->Clear();
+                    std::move(shared_callback).Run();
+                  })));
+          // Add callback for each cookie
+          callback_counter->IncrementCount(cookies.count);
+          for (NSHTTPCookie* cookie in cookies) {
+            [cookie_store_ deleteCookie:cookie
+                      completionHandler:^{
+                        callback_counter->DecrementCount();
+                      }];
+          }
+        }];
+      }));
 }
 
 NSHTTPCookieAcceptPolicy WKHTTPSystemCookieStore::GetCookieAcceptPolicy() {
@@ -145,11 +169,21 @@ void WKHTTPSystemCookieStore::RunSystemCookieCallbackForCookies(
     NSArray<NSHTTPCookie*>* cookies) {
   if (callback.is_null())
     return;
-  NSArray* result = [static_cast<NSArray*>(cookies)
-      sortedArrayUsingFunction:CompareCookies
-                       context:creation_time_manager_.get()];
-  RunSystemCookieCallbackOnIOThread(
-      base::BindOnce(std::move(callback), result));
+  base::WeakPtr<net::CookieCreationTimeManager> weak_time_manager =
+      creation_time_manager_->GetWeakPtr();
+  NSArray<NSHTTPCookie*>* block_cookies = cookies;
+  __block net::SystemCookieStore::SystemCookieCallbackForCookies
+      shared_callback = std::move(callback);
+  RunSystemCookieCallbackOnIOThread(base::BindBlockArc(^{
+    if (weak_time_manager) {
+      NSArray* result = [static_cast<NSArray*>(block_cookies)
+          sortedArrayUsingFunction:CompareCookies
+                           context:weak_time_manager.get()];
+      std::move(shared_callback).Run(result);
+    } else {
+      std::move(shared_callback).Run(block_cookies);
+    }
+  }));
 }
 
 }  // namespace web