Refactor the UpdateEngine and its actions in the component updater.

components/update_client/update_checker.cc

Index: components/update_client/update_checker.cc
diff --git a/components/update_client/update_checker.cc b/components/update_client/update_checker.cc
index 1459d561204f1f419ee5be819142afe0c6e69dbc..91b07886f66ca3b8ecd4512d79ccc5b650d76011 100644
--- a/components/update_client/update_checker.cc
+++ b/components/update_client/update_checker.cc
@@ -15,11 +15,12 @@
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/threading/thread_checker.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "components/update_client/component.h"
 #include "components/update_client/configurator.h"
-#include "components/update_client/crx_update_item.h"
 #include "components/update_client/persisted_data.h"
 #include "components/update_client/request_sender.h"
 #include "components/update_client/update_client.h"
@@ -48,9 +49,10 @@ update_client::InstallerAttributes SanitizeInstallerAttributes(
 }
 
 // Returns true if at least one item requires network encryption.
-bool IsEncryptionRequired(const IdToCrxUpdateItemMap& items) {
-  for (const auto& item : items) {
-    if (item.second->component.requires_network_encryption)
+bool IsEncryptionRequired(const IdToComponentPtrMap& components) {
+  for (const auto& item : components) {
+    const auto& component = item.second;
+    if (component->crx_component().requires_network_encryption)
       return true;
   }
   return false;
@@ -72,31 +74,36 @@ bool IsEncryptionRequired(const IdToCrxUpdateItemMap& items) {
 //    </app>
 std::string BuildUpdateCheckRequest(
     const Configurator& config,
-    const IdToCrxUpdateItemMap& items,
+    const std::vector<std::string>& ids_checked,
+    const IdToComponentPtrMap& components,
     PersistedData* metadata,
     const std::string& additional_attributes,
     bool enabled_component_updates,
     const std::unique_ptr<UpdaterState::Attributes>& updater_state_attributes) {
   const std::string brand(SanitizeBrand(config.GetBrand()));
   std::string app_elements;
-  for (const auto& item_pair : items) {
-    const CrxUpdateItem* item = item_pair.second.get();
+  for (const auto& id : ids_checked) {
+    DCHECK_EQ(1u, components.count(id));
+    const Component& component = *components.at(id);
+
     const update_client::InstallerAttributes installer_attributes(
-        SanitizeInstallerAttributes(item->component.installer_attributes));
+        SanitizeInstallerAttributes(
+            component.crx_component().installer_attributes));
     std::string app("<app ");
-    base::StringAppendF(&app, "appid=\"%s\" version=\"%s\"", item->id.c_str(),
-                        item->component.version.GetString().c_str());
+    base::StringAppendF(&app, "appid=\"%s\" version=\"%s\"",
+                        component.id().c_str(),
+                        component.crx_component().version.GetString().c_str());
     if (!brand.empty())
       base::StringAppendF(&app, " brand=\"%s\"", brand.c_str());
-    if (item->on_demand)
+    if (component.on_demand())
       base::StringAppendF(&app, " installsource=\"ondemand\"");
     for (const auto& attr : installer_attributes) {
       base::StringAppendF(&app, " %s=\"%s\"", attr.first.c_str(),
                           attr.second.c_str());
     }
-    const std::string cohort = metadata->GetCohort(item->id);
-    const std::string cohort_name = metadata->GetCohortName(item->id);
-    const std::string cohort_hint = metadata->GetCohortHint(item->id);
+    const std::string cohort = metadata->GetCohort(component.id());
+    const std::string cohort_name = metadata->GetCohortName(component.id());
+    const std::string cohort_hint = metadata->GetCohortHint(component.id());
     if (!cohort.empty())
       base::StringAppendF(&app, " cohort=\"%s\"", cohort.c_str());
     if (!cohort_name.empty())
@@ -106,21 +113,22 @@ std::string BuildUpdateCheckRequest(
     base::StringAppendF(&app, ">");
 
     base::StringAppendF(&app, "<updatecheck");
-    if (item->component.supports_group_policy_enable_component_updates &&
+    if (component.crx_component()
+            .supports_group_policy_enable_component_updates &&
         !enabled_component_updates) {
       base::StringAppendF(&app, " updatedisabled=\"true\"");
     }
     base::StringAppendF(&app, "/>");
 
     base::StringAppendF(&app, "<ping rd=\"%d\" ping_freshness=\"%s\"/>",
-                        metadata->GetDateLastRollCall(item->id),
-                        metadata->GetPingFreshness(item->id).c_str());
-    if (!item->component.fingerprint.empty()) {
+                        metadata->GetDateLastRollCall(component.id()),
+                        metadata->GetPingFreshness(component.id()).c_str());
+    if (!component.crx_component().fingerprint.empty()) {
       base::StringAppendF(&app,
                           "<packages>"
                           "<package fp=\"%s\"/>"
                           "</packages>",
-                          item->component.fingerprint.c_str());
+                          component.crx_component().fingerprint.c_str());
     }
     base::StringAppendF(&app, "</app>");
     app_elements.append(app);
@@ -143,26 +151,33 @@ class UpdateCheckerImpl : public UpdateChecker {
 
   // Overrides for UpdateChecker.
   bool CheckForUpdates(
-      const IdToCrxUpdateItemMap& items_to_check,
+      const std::vector<std::string>& ids_checked,
+      const IdToComponentPtrMap& components,
       const std::string& additional_attributes,
       bool enabled_component_updates,
       const UpdateCheckCallback& update_check_callback) override;
 
  private:
   void ReadUpdaterStateAttributes();
-  void CheckForUpdatesHelper(const IdToCrxUpdateItemMap& items_to_check,
+  void CheckForUpdatesHelper(const IdToComponentPtrMap& components,
                              const std::string& additional_attributes,
                              bool enabled_component_updates);
+  void OnRequestSenderComplete(const IdToComponentPtrMap& components,
+                               int error,
+                               const std::string& response,
+                               int retry_after_sec);
+  void UpdateCheckSucceeded(const IdToComponentPtrMap& components,
+                            const UpdateResponse::Results& results,
+                            int retry_after_sec);
+  void UpdateCheckFailed(const IdToComponentPtrMap& components,
+                         int error,
+                         int retry_after_sec);
 
-  void OnRequestSenderComplete(
-      std::unique_ptr<std::vector<std::string>> ids_checked,
-      int error,
-      const std::string& response,
-      int retry_after_sec);
   base::ThreadChecker thread_checker_;
 
   const scoped_refptr<Configurator> config_;
-  PersistedData* metadata_;
+  PersistedData* metadata_ = nullptr;
+  std::vector<std::string> ids_checked_;
   UpdateCheckCallback update_check_callback_;
   std::unique_ptr<UpdaterState::Attributes> updater_state_attributes_;
   std::unique_ptr<RequestSender> request_sender_;
@@ -179,19 +194,22 @@ UpdateCheckerImpl::~UpdateCheckerImpl() {
 }
 
 bool UpdateCheckerImpl::CheckForUpdates(
-    const IdToCrxUpdateItemMap& items_to_check,
+    const std::vector<std::string>& ids_checked,
+    const IdToComponentPtrMap& components,
     const std::string& additional_attributes,
     bool enabled_component_updates,
     const UpdateCheckCallback& update_check_callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
+  ids_checked_ = ids_checked;
   update_check_callback_ = update_check_callback;
 
   return config_->GetSequencedTaskRunner()->PostTaskAndReply(
-      FROM_HERE, base::Bind(&UpdateCheckerImpl::ReadUpdaterStateAttributes,
-                            base::Unretained(this)),
+      FROM_HERE,
+      base::Bind(&UpdateCheckerImpl::ReadUpdaterStateAttributes,
+                 base::Unretained(this)),
       base::Bind(&UpdateCheckerImpl::CheckForUpdatesHelper,
-                 base::Unretained(this), base::ConstRef(items_to_check),
+                 base::Unretained(this), base::ConstRef(components),
                  additional_attributes, enabled_component_updates));
 }
 
@@ -202,66 +220,95 @@ void UpdateCheckerImpl::ReadUpdaterStateAttributes() {
 }
 
 void UpdateCheckerImpl::CheckForUpdatesHelper(
-    const IdToCrxUpdateItemMap& items_to_check,
+    const IdToComponentPtrMap& components,
     const std::string& additional_attributes,
     bool enabled_component_updates) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   auto urls(config_->UpdateUrl());
-  if (IsEncryptionRequired(items_to_check))
+  if (IsEncryptionRequired(components))
     RemoveUnsecureUrls(&urls);
 
-  std::unique_ptr<std::vector<std::string>> ids_checked(
-      new std::vector<std::string>());
-  for (const auto& item : items_to_check)
-    ids_checked->push_back(item.second->id);
-  request_sender_.reset(new RequestSender(config_));
+  request_sender_ = base::MakeUnique<RequestSender>(config_);
   request_sender_->Send(
       config_->EnabledCupSigning(),
-      BuildUpdateCheckRequest(*config_, items_to_check, metadata_,
+      BuildUpdateCheckRequest(*config_, ids_checked_, components, metadata_,
                               additional_attributes, enabled_component_updates,
                               updater_state_attributes_),
-      urls, base::Bind(&UpdateCheckerImpl::OnRequestSenderComplete,
-                       base::Unretained(this), base::Passed(&ids_checked)));
+      urls,
+      base::Bind(&UpdateCheckerImpl::OnRequestSenderComplete,
+                 base::Unretained(this), base::ConstRef(components)));
 }
 
 void UpdateCheckerImpl::OnRequestSenderComplete(
-    std::unique_ptr<std::vector<std::string>> ids_checked,
+    const IdToComponentPtrMap& components,
     int error,
     const std::string& response,
     int retry_after_sec) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  if (!error) {
-    UpdateResponse update_response;
-    if (update_response.Parse(response)) {
-      int daynum = update_response.results().daystart_elapsed_days;
-      if (daynum != UpdateResponse::kNoDaystart)
-        metadata_->SetDateLastRollCall(*ids_checked, daynum);
-      for (const auto& result : update_response.results().list) {
-        auto entry = result.cohort_attrs.find(UpdateResponse::Result::kCohort);
-        if (entry != result.cohort_attrs.end())
-          metadata_->SetCohort(result.extension_id, entry->second);
-        entry = result.cohort_attrs.find(UpdateResponse::Result::kCohortName);
-        if (entry != result.cohort_attrs.end())
-          metadata_->SetCohortName(result.extension_id, entry->second);
-        entry = result.cohort_attrs.find(UpdateResponse::Result::kCohortHint);
-        if (entry != result.cohort_attrs.end())
-          metadata_->SetCohortHint(result.extension_id, entry->second);
-      }
-      base::ThreadTaskRunnerHandle::Get()->PostTask(
-          FROM_HERE, base::Bind(update_check_callback_, error,
-                                update_response.results(), retry_after_sec));
-      return;
-    }
+  if (error) {
+    VLOG(1) << "RequestSender failed " << error;
+    UpdateCheckFailed(components, error, retry_after_sec);
+    return;
+  }
 
-    error = -1;
+  UpdateResponse update_response;
+  if (!update_response.Parse(response)) {
     VLOG(1) << "Parse failed " << update_response.errors();
+    UpdateCheckFailed(components, -1, retry_after_sec);
+    return;
+  }
+
+  DCHECK_EQ(0, error);
+  UpdateCheckSucceeded(components, update_response.results(), retry_after_sec);
+}
+
+void UpdateCheckerImpl::UpdateCheckSucceeded(
+    const IdToComponentPtrMap& components,
+    const UpdateResponse::Results& results,
+    int retry_after_sec) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  const int daynum = results.daystart_elapsed_days;
+  if (daynum != UpdateResponse::kNoDaystart)
+    metadata_->SetDateLastRollCall(ids_checked_, daynum);
+  for (const auto& result : results.list) {
+    auto entry = result.cohort_attrs.find(UpdateResponse::Result::kCohort);
+    if (entry != result.cohort_attrs.end())
+      metadata_->SetCohort(result.extension_id, entry->second);
+    entry = result.cohort_attrs.find(UpdateResponse::Result::kCohortName);
+    if (entry != result.cohort_attrs.end())
+      metadata_->SetCohortName(result.extension_id, entry->second);
+    entry = result.cohort_attrs.find(UpdateResponse::Result::kCohortHint);
+    if (entry != result.cohort_attrs.end())
+      metadata_->SetCohortHint(result.extension_id, entry->second);
+  }
+
+  for (const auto& result : results.list) {
+    const auto& id = result.extension_id;
+    const auto it = components.find(id);
+    if (it != components.end())
+      it->second->SetParseResult(result);
+  }
+
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::Bind(update_check_callback_, 0, retry_after_sec));
+}
+
+void UpdateCheckerImpl::UpdateCheckFailed(const IdToComponentPtrMap& components,
+                                          int error,
+                                          int retry_after_sec) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_NE(0, error);
+  for (const auto& item : components) {
+    DCHECK(item.second);
+    Component& component = *item.second;
+    component.set_update_check_error(error);
   }
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::Bind(update_check_callback_, error,
-                            UpdateResponse::Results(), retry_after_sec));
+      FROM_HERE, base::Bind(update_check_callback_, error, retry_after_sec));
 }
 
 }  // namespace
@@ -269,8 +316,7 @@ void UpdateCheckerImpl::OnRequestSenderComplete(
 std::unique_ptr<UpdateChecker> UpdateChecker::Create(
     const scoped_refptr<Configurator>& config,
     PersistedData* persistent) {
-  return std::unique_ptr<UpdateChecker>(
-      new UpdateCheckerImpl(config, persistent));
+  return base::MakeUnique<UpdateCheckerImpl>(config, persistent);
 }
 
 }  // namespace update_client