Change web context getter to use WKHTTPSystemCookieStore

Change RequestContextGetter to use CookieStoreIOS for web_view and
for web shell.
This is needed so i can remove NSHTTPCookieStorage and CookieStoreIOSPersistent
and also to remove CookieMonster entirely from Chrome for iOS

Bug: 989554
Change-Id: I2e2bf61a6638b47796c90c7012d1ac689960316e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1822418
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Reviewed-by: Hiroshi Ichikawa <ichikawa@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#700355}

ios/web/shell/shell_browser_state.mm

@@ -23,7 +23,8 @@ ShellBrowserState::ShellBrowserState() : BrowserState() {
   CHECK(base::PathService::Get(base::DIR_APP_DATA, &path_));
 
   request_context_getter_ = new ShellURLRequestContextGetter(
-      GetStatePath(), base::CreateSingleThreadTaskRunner({web::WebThread::IO}));
+      GetStatePath(), this,
+      base::CreateSingleThreadTaskRunner({web::WebThread::IO}));
 }
 
 ShellBrowserState::~ShellBrowserState() {

ios/web/shell/shell_url_request_context_getter.h

@@ -21,14 +21,18 @@ class ProxyConfigService;
 class TransportSecurityPersister;
 class URLRequestContext;
 class URLRequestContextStorage;
+class SystemCookieStore;
 }
 
 namespace web {
 
+class BrowserState;
+
 class ShellURLRequestContextGetter : public net::URLRequestContextGetter {
  public:
   ShellURLRequestContextGetter(
       const base::FilePath& base_path,
+      web::BrowserState* browser_state,
       const scoped_refptr<base::SingleThreadTaskRunner>& network_task_runner);
 
   // net::URLRequestContextGetter implementation.
@@ -49,6 +53,12 @@ class ShellURLRequestContextGetter : public net::URLRequestContextGetter {
   std::unique_ptr<net::NetLog> net_log_;
   std::unique_ptr<net::TransportSecurityPersister>
       transport_security_persister_;
+  // SystemCookieStore must be created on UI thread in
+  // ShellURLRequestContextGetter's constructor. Later the ownership is passed
+  // to net::URLRequestContextStorage on IO thread. |system_cookie_store_| is
+  // created in constructor and cleared in GetURLRequestContext() where
+  // net::URLRequestContextStorage is lazily created.
+  std::unique_ptr<net::SystemCookieStore> system_cookie_store_;
 
   DISALLOW_COPY_AND_ASSIGN(ShellURLRequestContextGetter);
 };

ios/web/shell/shell_url_request_context_getter.mm

@@ -13,7 +13,8 @@
 #include "base/memory/ref_counted.h"
 #include "base/path_service.h"
 #include "base/task/post_task.h"
-#import "ios/net/cookies/cookie_store_ios_persistent.h"
+#import "ios/net/cookies/cookie_store_ios.h"
+#include "ios/web/public/browsing_data/system_cookie_store_util.h"
 #import "ios/web/public/web_client.h"
 #include "net/base/cache_type.h"
 #include "net/base/network_delegate_impl.h"
@@ -21,7 +22,6 @@
 #include "net/cert/ct_policy_enforcer.h"
 #include "net/cert/multi_log_ct_verifier.h"
 #include "net/dns/host_resolver.h"
-#include "net/extras/sqlite/sqlite_persistent_cookie_store.h"
 #include "net/http/http_auth_handler_factory.h"
 #include "net/http/http_cache.h"
 #include "net/http/http_network_session.h"
@@ -47,12 +47,14 @@ namespace web {
 
 ShellURLRequestContextGetter::ShellURLRequestContextGetter(
     const base::FilePath& base_path,
+    web::BrowserState* browser_state,
     const scoped_refptr<base::SingleThreadTaskRunner>& network_task_runner)
     : base_path_(base_path),
       network_task_runner_(network_task_runner),
       proxy_config_service_(
           new net::ProxyConfigServiceIOS(NO_TRAFFIC_ANNOTATION_YET)),
-      net_log_(new net::NetLog()) {}
+      net_log_(new net::NetLog()),
+      system_cookie_store_(web::CreateSystemCookieStore(browser_state)) {}
 
 ShellURLRequestContextGetter::~ShellURLRequestContextGetter() {}
 
@@ -68,24 +70,10 @@ net::URLRequestContext* ShellURLRequestContextGetter::GetURLRequestContext() {
 
     storage_.reset(
         new net::URLRequestContextStorage(url_request_context_.get()));
-
-    // Setup the cookie store.
-    base::FilePath cookie_path;
-    bool cookie_path_found =
-        base::PathService::Get(base::DIR_APP_DATA, &cookie_path);
-    DCHECK(cookie_path_found);
-    cookie_path = cookie_path.Append("WebShell").Append("Cookies");
-    scoped_refptr<net::CookieMonster::PersistentCookieStore> persistent_store =
-        new net::SQLitePersistentCookieStore(
-            cookie_path, network_task_runner_,
-            base::CreateSequencedTaskRunner({base::ThreadPool(),
-                                             base::MayBlock(),
-                                             base::TaskPriority::BEST_EFFORT}),
-            true, nullptr);
-    std::unique_ptr<net::CookieStoreIOS> cookie_store(
-        new net::CookieStoreIOSPersistent(persistent_store.get(),
-                                          net_log_.get()));
-    storage_->set_cookie_store(std::move(cookie_store));
+    // Using std::move on a |system_cookie_store_| resets it to null as it's a
+    // unique_ptr, so |system_cookie_store_| will not be a dangling pointer.
+    storage_->set_cookie_store(std::make_unique<net::CookieStoreIOS>(
+        std::move(system_cookie_store_), net_log_.get()));
 
     std::string user_agent =
         web::GetWebClient()->GetUserAgent(web::UserAgentType::MOBILE);

ios/web_view/internal/autofill/cwv_autofill_controller_unittest.mm

@@ -24,6 +24,7 @@
 #import "ios/web/public/test/fakes/fake_web_frame.h"
 #import "ios/web/public/test/fakes/fake_web_frames_manager.h"
 #import "ios/web/public/test/fakes/test_web_state.h"
+#include "ios/web/public/test/scoped_testing_web_client.h"
 #include "ios/web/public/test/web_task_environment.h"
 #include "ios/web/public/web_client.h"
 #import "ios/web_view/internal/autofill/cwv_autofill_suggestion_internal.h"
@@ -56,10 +57,9 @@ NSString* const kTestFieldValue = @"FieldValue";
 class CWVAutofillControllerTest : public TestWithLocaleAndResources {
  protected:
   CWVAutofillControllerTest()
-      : task_environment_(web::WebTaskEnvironment::IO_MAINLOOP),
+      : web_client_(std::make_unique<web::WebClient>()),
+        task_environment_(web::WebTaskEnvironment::IO_MAINLOOP),
         browser_state_(/*off_the_record=*/false) {
-    web::SetWebClient(&web_client_);
-
     test_web_state_.SetBrowserState(&browser_state_);
     CRWTestJSInjectionReceiver* injectionReceiver =
         [[CRWTestJSInjectionReceiver alloc] init];
@@ -92,7 +92,7 @@ class CWVAutofillControllerTest : public TestWithLocaleAndResources {
     test_web_state_.OnWebFrameDidBecomeAvailable(frame_ptr);
   }
 
-  web::WebClient web_client_;
+  web::ScopedTestingWebClient web_client_;
   web::WebTaskEnvironment task_environment_;
   ios_web_view::WebViewBrowserState browser_state_;
   web::TestWebState test_web_state_;

ios/web_view/internal/cwv_web_view_configuration_unittest.mm

@@ -6,7 +6,9 @@
 
 #include <memory>
 
+#include "ios/web/public/test/scoped_testing_web_client.h"
 #include "ios/web/public/test/web_task_environment.h"
+#include "ios/web/public/web_client.h"
 #include "ios/web_view/internal/web_view_browser_state.h"
 #include "ios/web_view/test/test_with_locale_and_resources.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -20,7 +22,11 @@ namespace ios_web_view {
 
 class CWVWebViewConfigurationTest : public TestWithLocaleAndResources {
  protected:
+  CWVWebViewConfigurationTest()
+      : web_client_(std::make_unique<web::WebClient>()) {}
+
   web::WebTaskEnvironment task_environment_;
+  web::ScopedTestingWebClient web_client_;
 };
 
 // Test CWVWebViewConfiguration initialization.

ios/web_view/internal/sync/cwv_sync_controller_unittest.mm

@@ -21,7 +21,9 @@
 #include "components/sync/driver/sync_service_observer.h"
 #include "google_apis/gaia/google_service_auth_error.h"
 #import "ios/web/public/test/fakes/test_web_state.h"
+#include "ios/web/public/test/scoped_testing_web_client.h"
 #include "ios/web/public/test/web_task_environment.h"
+#include "ios/web/public/web_client.h"
 #include "ios/web_view/internal/app/application_context.h"
 #include "ios/web_view/internal/signin/ios_web_view_signin_client.h"
 #include "ios/web_view/internal/signin/web_view_device_accounts_provider_impl.h"
@@ -67,7 +69,9 @@ std::unique_ptr<KeyedService> BuildMockSyncService(web::BrowserState* context) {
 
 class CWVSyncControllerTest : public TestWithLocaleAndResources {
  protected:
-  CWVSyncControllerTest() : browser_state_(/*off_the_record=*/false) {
+  CWVSyncControllerTest()
+      : web_client_(std::make_unique<web::WebClient>()),
+        browser_state_(/*off_the_record=*/false) {
     // Clear account info before each test.
     PrefService* pref_service = browser_state_.GetPrefs();
     pref_service->ClearPref(prefs::kGoogleServicesAccountId);
@@ -114,6 +118,7 @@ class CWVSyncControllerTest : public TestWithLocaleAndResources {
   }
 
   web::WebTaskEnvironment task_environment_;
+  web::ScopedTestingWebClient web_client_;
   ios_web_view::WebViewBrowserState browser_state_;
   std::unique_ptr<autofill::TestPersonalDataManager> personal_data_manager_;
   CWVSyncController* sync_controller_ = nil;

ios/web_view/internal/translate/cwv_translation_controller_unittest.mm

@@ -14,7 +14,9 @@
 #import "ios/web/public/deprecated/crw_test_js_injection_receiver.h"
 #import "ios/web/public/test/fakes/test_navigation_manager.h"
 #import "ios/web/public/test/fakes/test_web_state.h"
+#include "ios/web/public/test/scoped_testing_web_client.h"
 #include "ios/web/public/test/web_task_environment.h"
+#include "ios/web/public/web_client.h"
 #import "ios/web_view/internal/translate/cwv_translation_language_internal.h"
 #import "ios/web_view/internal/translate/fake_web_view_translate_client.h"
 #include "ios/web_view/internal/web_view_browser_state.h"
@@ -40,7 +42,9 @@ NSString* const kTestPageHost = @"www.chromium.org";
 
 class CWVTranslationControllerTest : public TestWithLocaleAndResources {
  protected:
-  CWVTranslationControllerTest() : browser_state_(/*off_the_record=*/false) {
+  CWVTranslationControllerTest()
+      : web_client_(std::make_unique<web::WebClient>()),
+        browser_state_(/*off_the_record=*/false) {
     web_state_.SetBrowserState(&browser_state_);
     auto test_navigation_manager =
         std::make_unique<web::TestNavigationManager>();
@@ -71,6 +75,7 @@ class CWVTranslationControllerTest : public TestWithLocaleAndResources {
   }
 
   web::WebTaskEnvironment task_environment_;
+  web::ScopedTestingWebClient web_client_;
   WebViewBrowserState browser_state_;
   std::unique_ptr<FakeWebViewTranslateClient> translate_client_;
   web::TestWebState web_state_;

ios/web_view/internal/web_view_browser_state.mm

@@ -89,7 +89,7 @@ WebViewBrowserState::WebViewBrowserState(
   CHECK(base::PathService::Get(base::DIR_APP_DATA, &path_));
 
   request_context_getter_ = new WebViewURLRequestContextGetter(
-      GetStatePath(), ApplicationContext::GetInstance()->GetNetLog(),
+      GetStatePath(), this, ApplicationContext::GetInstance()->GetNetLog(),
       base::CreateSingleThreadTaskRunner({web::WebThread::IO}));
 
   // Initialize prefs.

ios/web_view/internal/web_view_url_request_context_getter.h

@@ -20,6 +20,11 @@ class ProxyConfigService;
 class TransportSecurityPersister;
 class URLRequestContext;
 class URLRequestContextStorage;
+class SystemCookieStore;
+}  // namespace net
+
+namespace web {
+class BrowserState;
 }
 
 namespace ios_web_view {
@@ -29,6 +34,7 @@ class WebViewURLRequestContextGetter : public net::URLRequestContextGetter {
  public:
   WebViewURLRequestContextGetter(
       const base::FilePath& base_path,
+      web::BrowserState* browser_state,
       net::NetLog* net_log,
       const scoped_refptr<base::SingleThreadTaskRunner>& network_task_runner);
 
@@ -55,6 +61,12 @@ class WebViewURLRequestContextGetter : public net::URLRequestContextGetter {
   std::unique_ptr<net::URLRequestContextStorage> storage_;
   std::unique_ptr<net::TransportSecurityPersister>
       transport_security_persister_;
+  // SystemCookieStore must be created on UI thread in
+  // WebViewURLRequestContextGetter's constructor. Later the ownership is passed
+  // to net::URLRequestContextStorage on IO thread. |system_cookie_store_| is
+  // created in constructor and cleared in GetURLRequestContext() where
+  // net::URLRequestContextStorage is lazily created.
+  std::unique_ptr<net::SystemCookieStore> system_cookie_store_;
 
   // Used to ensure GetURLRequestContext() returns nullptr during shut down.
   bool is_shutting_down_;

ios/web_view/internal/web_view_url_request_context_getter.mm

@@ -12,7 +12,8 @@
 #include "base/memory/ref_counted.h"
 #include "base/path_service.h"
 #include "base/task/post_task.h"
-#import "ios/net/cookies/cookie_store_ios_persistent.h"
+#import "ios/net/cookies/cookie_store_ios.h"
+#include "ios/web/public/browsing_data/system_cookie_store_util.h"
 #import "ios/web/public/web_client.h"
 #include "net/base/cache_type.h"
 #include "net/base/network_delegate_impl.h"
@@ -20,7 +21,6 @@
 #include "net/cert/ct_policy_enforcer.h"
 #include "net/cert/multi_log_ct_verifier.h"
 #include "net/dns/host_resolver.h"
-#include "net/extras/sqlite/sqlite_persistent_cookie_store.h"
 #include "net/http/http_auth_handler_factory.h"
 #include "net/http/http_cache.h"
 #include "net/http/http_network_session.h"
@@ -46,6 +46,7 @@ namespace ios_web_view {
 
 WebViewURLRequestContextGetter::WebViewURLRequestContextGetter(
     const base::FilePath& base_path,
+    web::BrowserState* browser_state,
     net::NetLog* net_log,
     const scoped_refptr<base::SingleThreadTaskRunner>& network_task_runner)
     : base_path_(base_path),
@@ -53,6 +54,7 @@ WebViewURLRequestContextGetter::WebViewURLRequestContextGetter(
       network_task_runner_(network_task_runner),
       proxy_config_service_(
           new net::ProxyConfigServiceIOS(NO_TRAFFIC_ANNOTATION_YET)),
+      system_cookie_store_(web::CreateSystemCookieStore(browser_state)),
       is_shutting_down_(false) {}
 
 WebViewURLRequestContextGetter::~WebViewURLRequestContextGetter() = default;
@@ -73,23 +75,10 @@ net::URLRequestContext* WebViewURLRequestContextGetter::GetURLRequestContext() {
 
     storage_.reset(
         new net::URLRequestContextStorage(url_request_context_.get()));
-
-    // Setup the cookie store.
-    base::FilePath cookie_path;
-    bool cookie_path_found =
-        base::PathService::Get(base::DIR_APP_DATA, &cookie_path);
-    DCHECK(cookie_path_found);
-    cookie_path = cookie_path.Append("ChromeWebViewCookies");
-    scoped_refptr<net::CookieMonster::PersistentCookieStore> persistent_store =
-        new net::SQLitePersistentCookieStore(
-            cookie_path, network_task_runner_,
-            base::CreateSequencedTaskRunner({base::ThreadPool(),
-                                             base::MayBlock(),
-                                             base::TaskPriority::BEST_EFFORT}),
-            true, nullptr);
-    std::unique_ptr<net::CookieStoreIOS> cookie_store(
-        new net::CookieStoreIOSPersistent(persistent_store.get(), net_log_));
-    storage_->set_cookie_store(std::move(cookie_store));
+    // Using std::move on a |system_cookie_store_| resets it to null as it's a
+    // unique_ptr, so |system_cookie_store_| will not be a dangling pointer.
+    storage_->set_cookie_store(std::make_unique<net::CookieStoreIOS>(
+        std::move(system_cookie_store_), net_log_));
 
     web::WebClient* web_client = web::GetWebClient();
     DCHECK(web_client);