XmppClient: switch to NetworkServiceAsyncSocket

... and adjust clients to pass in the proper configuration for that.
Rather than using URLRequestContextGetter-based ChromeAsyncSocket,
which may not always work right with network service on.

Bug: 875032
Change-Id: I3f2c19f35c97d770eb63da94755f9d1d42b58bc9
Reviewed-on: https://chromium-review.googlesource.com/c/1315948
Reviewed-by: Lei Zhang <[email protected]>
Reviewed-by: Tatiana Gornak <[email protected]>
Reviewed-by: Maksim Ivanov <[email protected]>
Reviewed-by: Nicolas Zea <[email protected]>
Reviewed-by: John Abd-El-Malek <[email protected]>
Commit-Queue: Maks Orlovich <[email protected]>
Cr-Commit-Position: refs/heads/master@{#612395}

chrome/browser/chromeos/policy/affiliated_invalidation_service_provider_impl.cc

@@ -8,6 +8,7 @@
 
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/task/post_task.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_process_platform_part_chromeos.h"
 #include "chrome/browser/chrome_notification_types.h"
@@ -32,13 +33,43 @@
 #include "components/invalidation/public/invalidator_state.h"
 #include "components/policy/core/common/cloud/cloud_policy_constants.h"
 #include "components/user_manager/user.h"
+#include "content/public/browser/browser_task_traits.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/network_service_instance.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_service.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
+#include "services/network/public/mojom/network_context.mojom.h"
 
 namespace policy {
 
+namespace {
+
+// Runs on UI thread.
+void RequestProxyResolvingSocketFactoryOnUIThread(
+    base::WeakPtr<invalidation::TiclInvalidationService> owner,
+    network::mojom::ProxyResolvingSocketFactoryRequest request) {
+  if (!owner)
+    return;
+  if (g_browser_process->system_network_context_manager()) {
+    g_browser_process->system_network_context_manager()
+        ->GetContext()
+        ->CreateProxyResolvingSocketFactory(std::move(request));
+  }
+}
+
+// Runs on IO thread.
+void RequestProxyResolvingSocketFactory(
+    base::WeakPtr<invalidation::TiclInvalidationService> owner,
+    network::mojom::ProxyResolvingSocketFactoryRequest request) {
+  base::PostTaskWithTraits(
+      FROM_HERE, {content::BrowserThread::UI},
+      base::BindOnce(&RequestProxyResolvingSocketFactoryOnUIThread, owner,
+                     std::move(request)));
+}
+
+}  // namespace
+
 class AffiliatedInvalidationServiceProviderImpl::InvalidationServiceObserver
     : public syncer::InvalidationHandler {
  public:
@@ -323,7 +354,9 @@ AffiliatedInvalidationServiceProviderImpl::FindConnectedInvalidationService() {
             std::unique_ptr<invalidation::TiclSettingsProvider>(
                 new TiclDeviceSettingsProvider),
             g_browser_process->gcm_driver(),
-            g_browser_process->system_request_context(),
+            base::BindRepeating(&RequestProxyResolvingSocketFactory),
+            base::CreateSingleThreadTaskRunnerWithTraits(
+                {content::BrowserThread::IO}),
             std::move(url_loader_factory),
             content::GetNetworkConnectionTracker());
     device_invalidation_service_->Init(

chrome/browser/invalidation/deprecated_profile_invalidation_provider_factory.cc

@@ -7,6 +7,7 @@
 #include <memory>
 #include <utility>
 
+#include "base/task/post_task.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/gcm/gcm_profile_service_factory.h"
@@ -28,6 +29,8 @@
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_registry.h"
+#include "content/public/browser/browser_task_traits.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/network_service_instance.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/common/service_manager_connection.h"
@@ -52,6 +55,36 @@
 
 namespace invalidation {
 
+namespace {
+
+#if !defined(OS_ANDROID)
+void RequestProxyResolvingSocketFactoryOnUIThread(
+    Profile* profile,
+    base::WeakPtr<TiclInvalidationService> service,
+    network::mojom::ProxyResolvingSocketFactoryRequest request) {
+  if (!service)
+    return;
+  network::mojom::NetworkContext* network_context =
+      content::BrowserContext::GetDefaultStoragePartition(profile)
+          ->GetNetworkContext();
+  network_context->CreateProxyResolvingSocketFactory(std::move(request));
+}
+
+// A thread-safe wrapper to request a ProxyResolvingSocketFactoryPtr.
+void RequestProxyResolvingSocketFactory(
+    Profile* profile,
+    base::WeakPtr<TiclInvalidationService> service,
+    network::mojom::ProxyResolvingSocketFactoryRequest request) {
+  base::PostTaskWithTraits(
+      FROM_HERE, {content::BrowserThread::UI},
+      base::BindOnce(&RequestProxyResolvingSocketFactoryOnUIThread, profile,
+                     std::move(service), std::move(request)));
+}
+
+#endif
+
+}  // namespace
+
 // static
 ProfileInvalidationProvider*
 DeprecatedProfileInvalidationProviderFactory::GetForProfile(Profile* profile) {
@@ -134,7 +167,9 @@ DeprecatedProfileInvalidationProviderFactory::BuildServiceInstanceFor(
           GetUserAgent(), identity_provider.get(),
           std::make_unique<TiclProfileSettingsProvider>(profile->GetPrefs()),
           gcm::GCMProfileServiceFactory::GetForProfile(profile)->driver(),
-          profile->GetRequestContext(),
+          base::BindRepeating(&RequestProxyResolvingSocketFactory, profile),
+          base::CreateSingleThreadTaskRunnerWithTraits(
+              {content::BrowserThread::IO}),
           content::BrowserContext::GetDefaultStoragePartition(profile)
               ->GetURLLoaderFactoryForBrowserProcess(),
           content::GetNetworkConnectionTracker());

chrome/browser/sync/test/integration/DEPS

@@ -0,0 +1,3 @@
+include_rules = [
+  "+jingle/glue/network_service_config_test_util.h",
+]

chrome/browser/sync/test/integration/sync_test.cc

@@ -84,6 +84,7 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/test_browser_thread.h"
 #include "google_apis/gaia/gaia_urls.h"
+#include "jingle/glue/network_service_config_test_util.h"
 #include "net/base/escape.h"
 #include "net/base/load_flags.h"
 #include "net/base/network_change_notifier.h"
@@ -181,9 +182,12 @@ std::unique_ptr<KeyedService> BuildP2PProfileInvalidationProvider(
     content::BrowserContext* context,
     syncer::P2PNotificationTarget notification_target) {
   Profile* profile = static_cast<Profile*>(context);
+  auto config_helper =
+      std::make_unique<jingle_glue::NetworkServiceConfigTestUtil>(
+          profile->GetRequestContext());
   return std::make_unique<invalidation::ProfileInvalidationProvider>(
       std::make_unique<invalidation::P2PInvalidationService>(
-          profile->GetRequestContext(), content::GetNetworkConnectionTracker(),
+          std::move(config_helper), content::GetNetworkConnectionTracker(),
           notification_target),
       std::make_unique<invalidation::ProfileIdentityProvider>(
           IdentityManagerFactory::GetForProfile(profile)));

chrome/service/cloud_print/cloud_print_proxy_backend.cc

@@ -166,6 +166,17 @@ class CloudPrintProxyBackend::Core
 
   CloudPrintTokenStore* GetTokenStore();
 
+  // Runs on Core thread.
+  static void RequestProxyResolvingSocketFactoryOnCoreThread(
+      base::WeakPtr<CloudPrintProxyBackend::Core> owner,
+      network::mojom::ProxyResolvingSocketFactoryRequest request);
+
+  // Runs on IO thread.
+  static void RequestProxyResolvingSocketFactory(
+      scoped_refptr<base::SingleThreadTaskRunner> core_runner,
+      base::WeakPtr<CloudPrintProxyBackend::Core> owner,
+      network::mojom::ProxyResolvingSocketFactoryRequest request);
+
   // Our parent CloudPrintProxyBackend
   CloudPrintProxyBackend* const backend_;
 
@@ -204,6 +215,8 @@ class CloudPrintProxyBackend::Core
   std::string robot_email_;
   std::unique_ptr<CloudPrintTokenStore> token_store_;
 
+  base::WeakPtrFactory<Core> weak_ptr_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(Core);
 };
 
@@ -288,7 +301,8 @@ CloudPrintProxyBackend::Core::Core(
       job_poll_scheduled_(false),
       enable_job_poll_(enable_job_poll),
       xmpp_ping_scheduled_(false),
-      pending_xmpp_pings_(0) {
+      pending_xmpp_pings_(0),
+      weak_ptr_factory_(this) {
   settings_.CopyFrom(settings);
 }
 
@@ -432,8 +446,14 @@ void CloudPrintProxyBackend::Core::InitNotifications(
 
   pending_xmpp_pings_ = 0;
   notifier::NotifierOptions notifier_options;
-  notifier_options.request_context_getter =
-      g_service_process->GetServiceURLRequestContextGetter();
+  notifier_options.network_config.task_runner =
+      g_service_process->io_task_runner();
+  notifier_options.network_config.get_proxy_resolving_socket_factory_callback =
+      base::BindRepeating(&Core::RequestProxyResolvingSocketFactory,
+                          backend_->core_thread_.task_runner(),
+                          // This needs to use weak pointers since the callback
+                          // is repeatable and a ref would result in a cycle.
+                          weak_ptr_factory_.GetWeakPtr());
   notifier_options.auth_mechanism = "X-OAUTH2";
   notifier_options.try_ssltcp_first = true;
   notifier_options.xmpp_host_port = net::HostPortPair::FromString(
@@ -466,6 +486,7 @@ void CloudPrintProxyBackend::Core::DoShutdown() {
   notifications_enabled_ = false;
   notifications_enabled_since_ = base::TimeTicks();
   token_store_.reset();
+  weak_ptr_factory_.InvalidateWeakPtrs();
   url_loader_factory_owner_.reset();
 
   DestroyAuthAndConnector();
@@ -577,6 +598,32 @@ CloudPrintTokenStore* CloudPrintProxyBackend::Core::GetTokenStore() {
   return token_store_.get();
 }
 
+// static
+void CloudPrintProxyBackend::Core::
+    RequestProxyResolvingSocketFactoryOnCoreThread(
+        base::WeakPtr<CloudPrintProxyBackend::Core> owner,
+        network::mojom::ProxyResolvingSocketFactoryRequest request) {
+  if (!owner)
+    return;
+  DCHECK(owner->CurrentlyOnCoreThread());
+  owner->GetURLLoaderFactory();  // initialize |url_loader_factory_owner_|
+  owner->url_loader_factory_owner_->GetNetworkContext()
+      ->CreateProxyResolvingSocketFactory(std::move(request));
+}
+
+// static
+void CloudPrintProxyBackend::Core::RequestProxyResolvingSocketFactory(
+    scoped_refptr<base::SingleThreadTaskRunner> core_runner,
+    base::WeakPtr<CloudPrintProxyBackend::Core> owner,
+    network::mojom::ProxyResolvingSocketFactoryRequest request) {
+  DCHECK(g_service_process->io_task_runner()->BelongsToCurrentThread());
+  // This runs on IO thread; should not dereference |owner|.
+  core_runner->PostTask(
+      FROM_HERE,
+      base::BindOnce(&Core::RequestProxyResolvingSocketFactoryOnCoreThread,
+                     std::move(owner), std::move(request)));
+}
+
 void CloudPrintProxyBackend::Core::NotifyAuthenticated(
     const std::string& robot_oauth_refresh_token,
     const std::string& robot_email,

components/invalidation/impl/DEPS

@@ -10,6 +10,7 @@ include_rules = [
   "+google_apis/gaia",
   "+google_apis/gcm",
   "+jni",
+  "+jingle/glue/network_service_config_test_util.h",
   "+jingle/notifier",
   "+net/base/backoff_entry.h",
   "+net/base/ip_endpoint.h",

components/invalidation/impl/non_blocking_invalidator_unittest.cc

@@ -16,6 +16,7 @@
 #include "components/invalidation/impl/invalidation_state_tracker.h"
 #include "components/invalidation/impl/invalidator_test_template.h"
 #include "google/cacheinvalidation/types.pb.h"
+#include "jingle/glue/network_service_config_test_util.h"
 #include "jingle/notifier/base/fake_base_task.h"
 #include "net/url_request/url_request_test_util.h"
 #include "services/network/test/test_network_connection_tracker.h"
@@ -40,10 +41,12 @@ class NonBlockingInvalidatorTestDelegate {
     base::Thread::Options options;
     options.message_loop_type = base::MessageLoop::TYPE_IO;
     io_thread_.StartWithOptions(options);
-    request_context_getter_ =
-        new net::TestURLRequestContextGetter(io_thread_.task_runner());
+    net_config_helper_ =
+        std::make_unique<jingle_glue::NetworkServiceConfigTestUtil>(
+            base::MakeRefCounted<net::TestURLRequestContextGetter>(
+                io_thread_.task_runner()));
     notifier::NotifierOptions notifier_options;
-    notifier_options.request_context_getter = request_context_getter_;
+    net_config_helper_->FillInNetworkConfig(&notifier_options.network_config);
     notifier_options.network_connection_tracker =
         network::TestNetworkConnectionTracker::GetInstance();
     NetworkChannelCreator network_channel_creator =
@@ -52,7 +55,7 @@ class NonBlockingInvalidatorTestDelegate {
         network_channel_creator, invalidator_client_id,
         UnackedInvalidationsMap(), initial_state,
         invalidation_state_tracker.get(), "fake_client_info",
-        request_context_getter_->GetNetworkTaskRunner()));
+        notifier_options.network_config.task_runner));
   }
 
   Invalidator* GetInvalidator() {
@@ -61,7 +64,7 @@ class NonBlockingInvalidatorTestDelegate {
 
   void DestroyInvalidator() {
     invalidator_.reset();
-    request_context_getter_ = nullptr;
+    net_config_helper_ = nullptr;
     io_thread_.Stop();
     base::RunLoop().RunUntilIdle();
   }
@@ -85,7 +88,7 @@ class NonBlockingInvalidatorTestDelegate {
  private:
   base::MessageLoop message_loop_;
   base::Thread io_thread_;
-  scoped_refptr<net::URLRequestContextGetter> request_context_getter_;
+  std::unique_ptr<jingle_glue::NetworkServiceConfigTestUtil> net_config_helper_;
   std::unique_ptr<NonBlockingInvalidator> invalidator_;
 };
 

components/invalidation/impl/p2p_invalidation_service.cc

@@ -9,23 +9,20 @@
 #include "base/command_line.h"
 #include "components/invalidation/impl/invalidation_service_util.h"
 #include "components/invalidation/impl/p2p_invalidator.h"
+#include "jingle/glue/network_service_config_test_util.h"
 #include "jingle/notifier/base/notifier_options.h"
 #include "jingle/notifier/listener/push_client.h"
-#include "net/url_request/url_request_context_getter.h"
-
-namespace net {
-class URLRequestContextGetter;
-}
 
 namespace invalidation {
 
 P2PInvalidationService::P2PInvalidationService(
-    const scoped_refptr<net::URLRequestContextGetter>& request_context,
+    std::unique_ptr<jingle_glue::NetworkServiceConfigTestUtil> config_helper,
     network::NetworkConnectionTracker* network_connection_tracker,
-    syncer::P2PNotificationTarget notification_target) {
+    syncer::P2PNotificationTarget notification_target)
+    : config_helper_(std::move(config_helper)) {
   notifier::NotifierOptions notifier_options =
       ParseNotifierOptions(*base::CommandLine::ForCurrentProcess());
-  notifier_options.request_context_getter = request_context;
+  config_helper_->FillInNetworkConfig(&notifier_options.network_config);
   notifier_options.network_connection_tracker = network_connection_tracker;
   invalidator_id_ = GenerateInvalidatorClientId();
   invalidator_.reset(new syncer::P2PInvalidator(

components/invalidation/impl/p2p_invalidation_service.h

@@ -15,8 +15,8 @@
 #include "components/invalidation/public/invalidation_service.h"
 #include "components/keyed_service/core/keyed_service.h"
 
-namespace net {
-class URLRequestContextGetter;
+namespace jingle_glue {
+class NetworkServiceConfigTestUtil;
 }
 
 namespace network {
@@ -37,7 +37,7 @@ class InvalidationLogger;
 class P2PInvalidationService : public InvalidationService {
  public:
   P2PInvalidationService(
-      const scoped_refptr<net::URLRequestContextGetter>& request_context,
+      std::unique_ptr<jingle_glue::NetworkServiceConfigTestUtil> config_helper,
       network::NetworkConnectionTracker* network_connection_tracker,
       syncer::P2PNotificationTarget notification_target);
   ~P2PInvalidationService() override;
@@ -63,6 +63,7 @@ class P2PInvalidationService : public InvalidationService {
 
  private:
   std::unique_ptr<syncer::P2PInvalidator> invalidator_;
+  std::unique_ptr<jingle_glue::NetworkServiceConfigTestUtil> config_helper_;
   std::string invalidator_id_;
 
   SEQUENCE_CHECKER(sequence_checker_);