Cleanup: Remove HttpPostProviderFactor::Init()

It didn't actually initialize anything, it was just used to pass in the
user agent string. Might as well pass that into the ctor, and remove
some plumbing.

The original reason for this two-step initialization was apparently to
move construction of the Sync user agent string off the UI thread.
However, (a) even before this CL, the user agent string was in fact
constructed on the UI thread, and (b) nothing in MakeUserAgentForSync
looks like it could actually block.

Bug: 951350
Change-Id: I691f85468988d27187dbfc9088020e10a875fd79
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1913258
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715254}

components/browser_sync/test_http_bridge_factory.cc

@@ -30,8 +30,6 @@ TestHttpBridgeFactory::TestHttpBridgeFactory() {}
 
 TestHttpBridgeFactory::~TestHttpBridgeFactory() {}
 
-void TestHttpBridgeFactory::Init(const std::string& user_agent) {}
-
 syncer::HttpPostProviderInterface* TestHttpBridgeFactory::Create() {
   return new TestHttpBridge();
 }

components/browser_sync/test_http_bridge_factory.h

@@ -42,7 +42,6 @@ class TestHttpBridgeFactory : public syncer::HttpPostProviderFactory {
   ~TestHttpBridgeFactory() override;
 
   // syncer::HttpPostProviderFactory:
-  void Init(const std::string& user_agent) override;
   syncer::HttpPostProviderInterface* Create() override;
   void Destroy(syncer::HttpPostProviderInterface* http) override;
 };

components/sync/driver/glue/sync_engine_backend.cc

@@ -357,9 +357,6 @@ void SyncEngineBackend::DoInitialize(SyncEngine::InitParams params) {
   args.enable_local_sync_backend = params.enable_local_sync_backend;
   args.local_sync_backend_folder = params.local_sync_backend_folder;
   args.post_factory = std::move(params.http_factory_getter).Run();
-  // Finish initializing the HttpBridgeFactory.  We do this here because
-  // building the user agent may block on some platforms.
-  args.post_factory->Init(params.sync_user_agent);
   registrar_->GetWorkers(&args.workers);
   args.encryption_observer_proxy = std::move(params.encryption_observer_proxy);
   args.extensions_activity = params.extensions_activity.get();

components/sync/driver/glue/sync_engine_impl_unittest.cc

@@ -225,8 +225,8 @@ class SyncEngineImplTest : public testing::Test {
     SyncEngine::HttpPostProviderFactoryGetter
         http_post_provider_factory_getter =
             base::BindOnce(&NetworkResources::GetHttpPostProviderFactory,
-                           base::Unretained(network_resources_.get()), nullptr,
-                           base::DoNothing());
+                           base::Unretained(network_resources_.get()), "",
+                           nullptr, base::DoNothing());
 
     SyncEngine::InitParams params;
     params.sync_task_runner = sync_thread_.task_runner();

components/sync/driver/profile_sync_service.cc

@@ -294,6 +294,7 @@ SyncEngine::HttpPostProviderFactoryGetter
 ProfileSyncService::MakeHttpPostProviderFactoryGetter() {
   return base::BindOnce(&NetworkResources::GetHttpPostProviderFactory,
                         base::Unretained(network_resources_.get()),
+                        MakeUserAgentForSync(channel_),
                         url_loader_factory_->Clone(),
                         network_time_update_callback_);
 }
@@ -484,7 +485,6 @@ void ProfileSyncService::StartUpSlowEngineComponents() {
   params.extensions_activity = sync_client_->GetExtensionsActivity();
   params.event_handler = GetJsEventHandler();
   params.service_url = sync_service_url_;
-  params.sync_user_agent = MakeUserAgentForSync(channel_);
   params.http_factory_getter = MakeHttpPostProviderFactoryGetter();
   params.authenticated_account_id = GetAuthenticatedAccountInfo().account_id;
   DCHECK(!params.authenticated_account_id.empty() || IsLocalSyncEnabled());

components/sync/engine/net/http_bridge.cc

@@ -52,10 +52,12 @@ base::LazyInstance<scoped_refptr<base::SequencedTaskRunner>>::Leaky
 }  // namespace
 
 HttpBridgeFactory::HttpBridgeFactory(
+    const std::string& user_agent,
     std::unique_ptr<network::SharedURLLoaderFactoryInfo>
         url_loader_factory_info,
     const NetworkTimeUpdateCallback& network_time_update_callback)
-    : network_time_update_callback_(network_time_update_callback) {
+    : user_agent_(user_agent),
+      network_time_update_callback_(network_time_update_callback) {
   // Some tests pass null'ed out url_loader_factory_info instances.
   if (url_loader_factory_info) {
     url_loader_factory_ = network::SharedURLLoaderFactory::Create(
@@ -65,10 +67,6 @@ HttpBridgeFactory::HttpBridgeFactory(
 
 HttpBridgeFactory::~HttpBridgeFactory() = default;
 
-void HttpBridgeFactory::Init(const std::string& user_agent) {
-  user_agent_ = user_agent;
-}
-
 HttpPostProviderInterface* HttpBridgeFactory::Create() {
   DCHECK(url_loader_factory_);
 

components/sync/engine/net/http_bridge.h

@@ -196,19 +196,19 @@ class HttpBridge : public base::RefCountedThreadSafe<HttpBridge>,
 class HttpBridgeFactory : public HttpPostProviderFactory {
  public:
   HttpBridgeFactory(
+      const std::string& user_agent,
       std::unique_ptr<network::SharedURLLoaderFactoryInfo>
           url_loader_factory_info,
       const NetworkTimeUpdateCallback& network_time_update_callback);
   ~HttpBridgeFactory() override;
 
   // HttpPostProviderFactory:
-  void Init(const std::string& user_agent) override;
   HttpPostProviderInterface* Create() override;
   void Destroy(HttpPostProviderInterface* http) override;
 
  private:
   // The user agent to use in all requests.
-  std::string user_agent_;
+  const std::string user_agent_;
 
   // The URL loader factory used for making all requests.
   scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;

components/sync/engine/net/http_bridge_network_resources.cc

@@ -4,6 +4,7 @@
 
 #include "components/sync/engine/net/http_bridge_network_resources.h"
 
+#include <string>
 #include <utility>
 
 #include "components/sync/engine/net/http_bridge.h"
@@ -16,10 +17,12 @@ HttpBridgeNetworkResources::~HttpBridgeNetworkResources() {}
 
 std::unique_ptr<HttpPostProviderFactory>
 HttpBridgeNetworkResources::GetHttpPostProviderFactory(
+    const std::string& user_agent,
     std::unique_ptr<network::SharedURLLoaderFactoryInfo>
         url_loader_factory_info,
     const NetworkTimeUpdateCallback& network_time_update_callback) {
-  return std::make_unique<HttpBridgeFactory>(std::move(url_loader_factory_info),
+  return std::make_unique<HttpBridgeFactory>(user_agent,
+                                             std::move(url_loader_factory_info),
                                              network_time_update_callback);
 }
 

components/sync/engine/net/http_bridge_network_resources.h

@@ -6,6 +6,7 @@
 #define COMPONENTS_SYNC_ENGINE_NET_HTTP_BRIDGE_NETWORK_RESOURCES_H_
 
 #include <memory>
+#include <string>
 
 #include "components/sync/engine/net/network_resources.h"
 #include "components/sync/engine/net/network_time_update_callback.h"
@@ -24,6 +25,7 @@ class HttpBridgeNetworkResources : public NetworkResources {
 
   // NetworkResources
   std::unique_ptr<HttpPostProviderFactory> GetHttpPostProviderFactory(
+      const std::string& user_agent,
       std::unique_ptr<network::SharedURLLoaderFactoryInfo>
           url_loader_factory_info,
       const NetworkTimeUpdateCallback& network_time_update_callback) override;

components/sync/engine/net/http_post_provider_factory.h

@@ -21,8 +21,6 @@ class HttpPostProviderFactory {
  public:
   virtual ~HttpPostProviderFactory() {}
 
-  virtual void Init(const std::string& user_agent) = 0;
-
   // Obtain a new HttpPostProviderInterface instance, owned by caller.
   virtual HttpPostProviderInterface* Create() = 0;
 

components/sync/engine/net/network_resources.h

@@ -6,6 +6,7 @@
 #define COMPONENTS_SYNC_ENGINE_NET_NETWORK_RESOURCES_H_
 
 #include <memory>
+#include <string>
 
 #include "components/sync/engine/net/network_time_update_callback.h"
 
@@ -22,6 +23,7 @@ class NetworkResources {
   virtual ~NetworkResources() {}
 
   virtual std::unique_ptr<HttpPostProviderFactory> GetHttpPostProviderFactory(
+      const std::string& user_agent,
       std::unique_ptr<network::SharedURLLoaderFactoryInfo>
           url_loader_factory_info,
       const NetworkTimeUpdateCallback& network_time_update_callback) = 0;

components/sync/engine/sync_engine.h

@@ -27,14 +27,12 @@
 #include "components/sync/engine/sync_backend_registrar.h"
 #include "components/sync/engine/sync_credentials.h"
 #include "components/sync/engine/sync_manager_factory.h"
-
-class GURL;
+#include "url/gurl.h"
 
 namespace syncer {
 
 class HttpPostProviderFactory;
 class SyncEngineHost;
-class SyncManagerFactory;
 class UnrecoverableErrorHandler;
 
 // The interface into the sync engine, which is the part of sync that performs
@@ -61,7 +59,6 @@ class SyncEngine : public ModelTypeConfigurer {
     scoped_refptr<ExtensionsActivity> extensions_activity;
     WeakHandle<JsEventHandler> event_handler;
     GURL service_url;
-    std::string sync_user_agent;
     SyncEngine::HttpPostProviderFactoryGetter http_factory_getter;
     CoreAccountId authenticated_account_id;
     std::string invalidator_client_id;

components/sync/engine_impl/net/sync_server_connection_manager_unittest.cc

@@ -55,7 +55,6 @@ class BlockingHttpPost : public HttpPostProviderInterface {
 class BlockingHttpPostFactory : public HttpPostProviderFactory {
  public:
   ~BlockingHttpPostFactory() override {}
-  void Init(const std::string& user_agent) override {}
 
   HttpPostProviderInterface* Create() override {
     return new BlockingHttpPost();
@@ -157,7 +156,6 @@ class FailingHttpPostFactory : public HttpPostProviderFactory {
   explicit FailingHttpPostFactory(int net_error_code)
       : net_error_code_(net_error_code) {}
   ~FailingHttpPostFactory() override {}
-  void Init(const std::string& user_agent) override {}
 
   HttpPostProviderInterface* Create() override {
     return new FailingHttpPost(net_error_code_);

components/sync/engine_impl/sync_manager_impl_unittest.cc

@@ -880,7 +880,6 @@ class TestHttpPostProviderInterface : public HttpPostProviderInterface {
 class TestHttpPostProviderFactory : public HttpPostProviderFactory {
  public:
   ~TestHttpPostProviderFactory() override {}
-  void Init(const std::string& user_agent) override {}
   HttpPostProviderInterface* Create() override {
     return new TestHttpPostProviderInterface();
   }

components/sync/test/fake_server/fake_server_http_post_provider.cc

@@ -26,8 +26,6 @@ FakeServerHttpPostProviderFactory::FakeServerHttpPostProviderFactory(
 
 FakeServerHttpPostProviderFactory::~FakeServerHttpPostProviderFactory() {}
 
-void FakeServerHttpPostProviderFactory::Init(const std::string& user_agent) {}
-
 syncer::HttpPostProviderInterface* FakeServerHttpPostProviderFactory::Create() {
   FakeServerHttpPostProvider* http =
       new FakeServerHttpPostProvider(fake_server_, fake_server_task_runner_);

components/sync/test/fake_server/fake_server_http_post_provider.h

@@ -90,7 +90,6 @@ class FakeServerHttpPostProviderFactory
   ~FakeServerHttpPostProviderFactory() override;
 
   // HttpPostProviderFactory:
-  void Init(const std::string& user_agent) override;
   syncer::HttpPostProviderInterface* Create() override;
   void Destroy(syncer::HttpPostProviderInterface* http) override;
 

components/sync/test/fake_server/fake_server_network_resources.cc

@@ -4,15 +4,14 @@
 
 #include "components/sync/test/fake_server/fake_server_network_resources.h"
 
+#include <string>
+
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/sync/engine/net/http_post_provider_factory.h"
 #include "components/sync/test/fake_server/fake_server.h"
 #include "components/sync/test/fake_server/fake_server_http_post_provider.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 
-using syncer::HttpPostProviderFactory;
-using syncer::NetworkTimeUpdateCallback;
-
 namespace fake_server {
 
 FakeServerNetworkResources::FakeServerNetworkResources(
@@ -24,9 +23,10 @@ FakeServerNetworkResources::~FakeServerNetworkResources() {}
 
 std::unique_ptr<syncer::HttpPostProviderFactory>
 FakeServerNetworkResources::GetHttpPostProviderFactory(
+    const std::string& user_agent,
     std::unique_ptr<network::SharedURLLoaderFactoryInfo>
         url_loader_factory_info,
-    const NetworkTimeUpdateCallback& network_time_update_callback) {
+    const syncer::NetworkTimeUpdateCallback& network_time_update_callback) {
   return std::make_unique<FakeServerHttpPostProviderFactory>(
       fake_server_, fake_server_task_runner_);
 }

components/sync/test/fake_server/fake_server_network_resources.h

@@ -6,6 +6,7 @@
 #define COMPONENTS_SYNC_TEST_FAKE_SERVER_FAKE_SERVER_NETWORK_RESOURCES_H_
 
 #include <memory>
+#include <string>
 
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
@@ -30,6 +31,7 @@ class FakeServerNetworkResources : public syncer::NetworkResources {
 
   // NetworkResources
   std::unique_ptr<syncer::HttpPostProviderFactory> GetHttpPostProviderFactory(
+      const std::string& user_agent,
       std::unique_ptr<network::SharedURLLoaderFactoryInfo>
           url_loader_factory_info,
       const syncer::NetworkTimeUpdateCallback& network_time_update_callback)