Allow injection of protocol factory in update_client through Configurator.

This is a mechanical change. Its purpose is allowing different wire
protocol parsers and serializers to be injected in the UpdateChecker and
PingManager instances in the UpdateClient using the Configuration instance.

The current code hardcodes an instance of ProtocolHandlerXm. There is work
in progress to support JSON.

Bug: 881076
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I40cb1605f134a5d3f168a511094b2b265bc50f25
Reviewed-on: https://chromium-review.googlesource.com/c/1289510Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Minh Nguyen <mxnguyen@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601179}

chrome/browser/component_updater/chrome_component_updater_configurator.cc

@@ -25,6 +25,7 @@
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "components/update_client/activity_data_service.h"
+#include "components/update_client/protocol_handler.h"
 #include "components/update_client/update_query_params.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/service_manager_connection.h"
@@ -73,6 +74,8 @@ class ChromeConfigurator : public update_client::Configurator {
   bool IsPerUserInstall() const override;
   std::vector<uint8_t> GetRunActionKeyHash() const override;
   std::string GetAppGuid() const override;
+  std::unique_ptr<update_client::ProtocolHandlerFactory>
+  GetProtocolHandlerFactory() const override;
 
  private:
   friend class base::RefCountedThreadSafe<ChromeConfigurator>;
@@ -221,6 +224,11 @@ std::string ChromeConfigurator::GetAppGuid() const {
 #endif
 }
 
+std::unique_ptr<update_client::ProtocolHandlerFactory>
+ChromeConfigurator::GetProtocolHandlerFactory() const {
+  return configurator_impl_.GetProtocolHandlerFactory();
+}
+
 }  // namespace
 
 void RegisterPrefsForChromeComponentUpdaterConfigurator(

chrome/browser/extensions/updater/chrome_update_client_config.cc

@@ -17,6 +17,7 @@
 #include "chrome/common/channel_info.h"
 #include "components/prefs/pref_service.h"
 #include "components/update_client/activity_data_service.h"
+#include "components/update_client/protocol_handler.h"
 #include "components/update_client/update_query_params.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
@@ -215,6 +216,11 @@ std::string ChromeUpdateClientConfig::GetAppGuid() const {
   return impl_.GetAppGuid();
 }
 
+std::unique_ptr<update_client::ProtocolHandlerFactory>
+ChromeUpdateClientConfig::GetProtocolHandlerFactory() const {
+  return impl_.GetProtocolHandlerFactory();
+}
+
 ChromeUpdateClientConfig::~ChromeUpdateClientConfig() {}
 
 // static

chrome/browser/extensions/updater/chrome_update_client_config.h

@@ -23,6 +23,7 @@ class BrowserContext;
 
 namespace update_client {
 class ActivityDataService;
+class ProtocolHandlerFactory;
 }
 
 namespace extensions {
@@ -65,6 +66,8 @@ class ChromeUpdateClientConfig : public update_client::Configurator {
   bool IsPerUserInstall() const override;
   std::vector<uint8_t> GetRunActionKeyHash() const override;
   std::string GetAppGuid() const override;
+  std::unique_ptr<update_client::ProtocolHandlerFactory>
+  GetProtocolHandlerFactory() const override;
 
  protected:
   friend class base::RefCountedThreadSafe<ChromeUpdateClientConfig>;

components/component_updater/configurator_impl.cc

@@ -16,6 +16,7 @@
 #include "components/component_updater/component_updater_switches.h"
 #include "components/component_updater/component_updater_url_constants.h"
 #include "components/update_client/command_line_config_policy.h"
+#include "components/update_client/protocol_handler.h"
 #include "components/update_client/utils.h"
 #include "components/version_info/version_info.h"
 
@@ -130,4 +131,9 @@ std::string ConfiguratorImpl::GetAppGuid() const {
   return {};
 }
 
+std::unique_ptr<update_client::ProtocolHandlerFactory>
+ConfiguratorImpl::GetProtocolHandlerFactory() const {
+  return std::make_unique<update_client::ProtocolHandlerFactoryXml>();
+}
+
 }  // namespace component_updater

components/component_updater/configurator_impl.h

@@ -7,6 +7,7 @@
 
 #include <stdint.h>
 
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -20,6 +21,7 @@ class Version;
 
 namespace update_client {
 class CommandLineConfigPolicy;
+class ProtocolHandlerFactory;
 }
 
 namespace component_updater {
@@ -89,6 +91,11 @@ class ConfiguratorImpl {
   // an empty string if this brand does not integrate with Google Update.
   std::string GetAppGuid() const;
 
+  // Returns the class factory to create protocol parser and protocol
+  // serializer object instances.
+  std::unique_ptr<update_client::ProtocolHandlerFactory>
+  GetProtocolHandlerFactory() const;
+
  private:
   base::flat_map<std::string, std::string> extra_info_;
   const bool background_downloads_enabled_;

components/update_client/BUILD.gn

@@ -32,6 +32,8 @@ static_library("update_client") {
     "ping_manager.h",
     "protocol_definition.cc",
     "protocol_definition.h",
+    "protocol_handler.cc",
+    "protocol_handler.h",
     "protocol_parser.cc",
     "protocol_parser.h",
     "protocol_parser_xml.cc",

components/update_client/configurator.h

@@ -27,6 +27,7 @@ class Connector;
 namespace update_client {
 
 class ActivityDataService;
+class ProtocolHandlerFactory;
 
 // Controls the component updater behavior.
 // TODO(sorin): this class will be split soon in two. One class controls
@@ -150,6 +151,11 @@ class Configurator : public base::RefCountedThreadSafe<Configurator> {
   // an empty string if this brand does not integrate with Google Update.
   virtual std::string GetAppGuid() const = 0;
 
+  // Returns the class factory to create protocol parser and protocol
+  // serializer object instances.
+  virtual std::unique_ptr<ProtocolHandlerFactory> GetProtocolHandlerFactory()
+      const = 0;
+
  protected:
   friend class base::RefCountedThreadSafe<Configurator>;
 

components/update_client/ping_manager.cc

@@ -19,6 +19,7 @@
 #include "components/update_client/component.h"
 #include "components/update_client/configurator.h"
 #include "components/update_client/protocol_definition.h"
+#include "components/update_client/protocol_handler.h"
 #include "components/update_client/protocol_serializer.h"
 #include "components/update_client/request_sender.h"
 #include "components/update_client/utils.h"
@@ -93,12 +94,13 @@ void PingSender::SendPing(const Component& component, Callback callback) {
   request_sender_ = std::make_unique<RequestSender>(config_);
   request_sender_->Send(
       urls, {},
-      ProtocolSerializer::Create()->Serialize(MakeProtocolRequest(
-          component.session_id(), config_->GetProdId(),
-          config_->GetBrowserVersion().GetString(), config_->GetLang(),
-          config_->GetChannel(), config_->GetOSLongName(),
-          config_->GetDownloadPreference(), config_->ExtraRequestParams(),
-          nullptr, std::move(apps))),
+      config_->GetProtocolHandlerFactory()->CreateSerializer()->Serialize(
+          MakeProtocolRequest(
+              component.session_id(), config_->GetProdId(),
+              config_->GetBrowserVersion().GetString(), config_->GetLang(),
+              config_->GetChannel(), config_->GetOSLongName(),
+              config_->GetDownloadPreference(), config_->ExtraRequestParams(),
+              nullptr, std::move(apps))),
       false, base::BindOnce(&PingSender::SendPingComplete, this));
 }
 

components/update_client/protocol_handler.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/update_client/protocol_handler.h"
+#include "components/update_client/protocol_parser_xml.h"
+#include "components/update_client/protocol_serializer_xml.h"
+
+namespace update_client {
+
+std::unique_ptr<ProtocolParser> ProtocolHandlerFactoryXml::CreateParser()
+    const {
+  return std::make_unique<ProtocolParserXml>();
+}
+
+std::unique_ptr<ProtocolSerializer>
+ProtocolHandlerFactoryXml::CreateSerializer() const {
+  return std::make_unique<ProtocolSerializerXml>();
+}
+
+}  // namespace update_client

components/update_client/protocol_handler.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_UPDATE_CLIENT_PROTOCOL_HANDLER_H_
+#define COMPONENTS_UPDATE_CLIENT_PROTOCOL_HANDLER_H_
+
+#include <memory>
+
+#include "base/macros.h"
+
+namespace update_client {
+
+class ProtocolParser;
+class ProtocolSerializer;
+
+class ProtocolHandlerFactory {
+ public:
+  virtual ~ProtocolHandlerFactory() = default;
+  virtual std::unique_ptr<ProtocolParser> CreateParser() const = 0;
+  virtual std::unique_ptr<ProtocolSerializer> CreateSerializer() const = 0;
+
+ protected:
+  ProtocolHandlerFactory() = default;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ProtocolHandlerFactory);
+};
+
+class ProtocolHandlerFactoryXml final : public ProtocolHandlerFactory {
+ public:
+  // Overrides for ProtocolHandlerFactory.
+  std::unique_ptr<ProtocolParser> CreateParser() const override;
+  std::unique_ptr<ProtocolSerializer> CreateSerializer() const override;
+};
+
+}  // namespace update_client
+
+#endif  // COMPONENTS_UPDATE_CLIENT_PROTOCOL_HANDLER_H_

components/update_client/test_configurator.cc

@@ -12,6 +12,7 @@
 #include "components/services/patch/patch_service.h"
 #include "components/services/unzip/unzip_service.h"
 #include "components/update_client/activity_data_service.h"
+#include "components/update_client/protocol_handler.h"
 #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
 #include "services/service_manager/public/cpp/connector.h"
 #include "services/service_manager/public/cpp/service.h"
@@ -201,4 +202,9 @@ std::string TestConfigurator::GetAppGuid() const {
   return app_guid_;
 }
 
+std::unique_ptr<ProtocolHandlerFactory>
+TestConfigurator::GetProtocolHandlerFactory() const {
+  return std::make_unique<ProtocolHandlerFactoryXml>();
+}
+
 }  // namespace update_client

components/update_client/test_configurator.h

@@ -33,6 +33,7 @@ class TestConnectorFactory;
 namespace update_client {
 
 class ActivityDataService;
+class ProtocolHandlerFactory;
 
 #define POST_INTERCEPT_SCHEME "https"
 #define POST_INTERCEPT_HOSTNAME "localhost2"
@@ -100,6 +101,8 @@ class TestConfigurator : public Configurator {
   bool IsPerUserInstall() const override;
   std::vector<uint8_t> GetRunActionKeyHash() const override;
   std::string GetAppGuid() const override;
+  std::unique_ptr<ProtocolHandlerFactory> GetProtocolHandlerFactory()
+      const override;
 
   void SetBrand(const std::string& brand);
   void SetOnDemandTime(int seconds);

components/update_client/update_checker.cc

@@ -25,6 +25,7 @@
 #include "components/update_client/configurator.h"
 #include "components/update_client/persisted_data.h"
 #include "components/update_client/protocol_definition.h"
+#include "components/update_client/protocol_handler.h"
 #include "components/update_client/protocol_serializer.h"
 #include "components/update_client/request_sender.h"
 #include "components/update_client/task_traits.h"
@@ -216,7 +217,8 @@ void UpdateCheckerImpl::CheckForUpdatesHelper(
       BuildUpdateCheckExtraRequestHeaders(config_->GetProdId(),
                                           config_->GetBrowserVersion(),
                                           ids_checked_, is_foreground),
-      ProtocolSerializer::Create()->Serialize(request),
+      config_->GetProtocolHandlerFactory()->CreateSerializer()->Serialize(
+          request),
       config_->EnabledCupSigning(),
       base::BindOnce(&UpdateCheckerImpl::OnRequestSenderComplete,
                      base::Unretained(this)));
@@ -234,7 +236,7 @@ void UpdateCheckerImpl::OnRequestSenderComplete(
     return;
   }
 
-  auto parser = ProtocolParser::Create();
+  auto parser = config_->GetProtocolHandlerFactory()->CreateParser();
   if (!parser->Parse(response)) {
     VLOG(1) << "Parse failed " << parser->errors();
     UpdateCheckFailed(ErrorCategory::kUpdateCheck,

ios/chrome/browser/component_updater/ios_component_updater_configurator.cc

@@ -15,6 +15,7 @@
 #include "components/component_updater/component_updater_command_line_config_policy.h"
 #include "components/component_updater/configurator_impl.h"
 #include "components/update_client/activity_data_service.h"
+#include "components/update_client/protocol_handler.h"
 #include "components/update_client/update_query_params.h"
 #include "ios/chrome/browser/application_context.h"
 #include "ios/chrome/browser/google/google_brand.h"
@@ -57,6 +58,8 @@ class IOSConfigurator : public update_client::Configurator {
   bool IsPerUserInstall() const override;
   std::vector<uint8_t> GetRunActionKeyHash() const override;
   std::string GetAppGuid() const override;
+  std::unique_ptr<update_client::ProtocolHandlerFactory>
+  GetProtocolHandlerFactory() const override;
 
  private:
   friend class base::RefCountedThreadSafe<IOSConfigurator>;
@@ -180,6 +183,11 @@ std::string IOSConfigurator::GetAppGuid() const {
   return configurator_impl_.GetAppGuid();
 }
 
+std::unique_ptr<update_client::ProtocolHandlerFactory>
+IOSConfigurator::GetProtocolHandlerFactory() const {
+  return configurator_impl_.GetProtocolHandlerFactory();
+}
+
 }  // namespace
 
 scoped_refptr<update_client::Configurator> MakeIOSComponentUpdaterConfigurator(