Make CrxComponent an optional member in the CrxUpdateItem.

Since the instance of CrxComponent is provided by the clients of the
update_client::UpdateClient, it is possible that instance is missing in
some cases, such as when a CRX is uninstalled. That leads to corner
cases such as calls to UpdateClient::GetCrxUpdateState returning an
error, because an instance of CrxUpdateItem could not be returned.

This is a mechanical change to replace the std::unique_ptr wrapper of
the CrxComponent with a base::Optional wrapper, so that CrxUpdateItem
could have correct value semantics.

BUG=869663

Change-Id: I6aa805c2784af5ed959c8777f724322763404cb0
Reviewed-on: https://chromium-review.googlesource.com/1173018
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Minh Nguyen <mxnguyen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583913}

chrome/browser/ui/webui/components_ui.cc

@@ -183,12 +183,14 @@ std::unique_ptr<base::ListValue> ComponentsUI::LoadComponents() {
   for (size_t j = 0; j < component_ids.size(); ++j) {
     update_client::CrxUpdateItem item;
     if (cus->GetComponentDetails(component_ids[j], &item)) {
-      std::unique_ptr<base::DictionaryValue> component_entry(
-          new base::DictionaryValue());
+      auto component_entry = std::make_unique<base::DictionaryValue>();
       component_entry->SetString("id", component_ids[j]);
-      component_entry->SetString("name", item.component.name);
-      component_entry->SetString("version", item.component.version.GetString());
       component_entry->SetString("status", ServiceStatusToString(item.state));
+      if (item.component) {
+        component_entry->SetString("name", item.component->name);
+        component_entry->SetString("version",
+                                   item.component->version.GetString());
+      }
       component_list->Append(std::move(component_entry));
     }
   }
@@ -266,8 +268,8 @@ void ComponentsUI::OnEvent(Events event, const std::string& id) {
     if (event == Events::COMPONENT_UPDATED) {
       auto* component_updater = g_browser_process->component_updater();
       update_client::CrxUpdateItem item;
-      if (component_updater->GetComponentDetails(id, &item))
-        parameters.SetString("version", item.component.version.GetString());
+      if (component_updater->GetComponentDetails(id, &item) && item.component)
+        parameters.SetString("version", item.component->version.GetString());
     }
     parameters.SetString("id", id);
   }

components/component_updater/component_installer_unittest.cc

@@ -311,7 +311,8 @@ TEST_F(ComponentInstallerTest, RegisterComponent) {
 
   CrxUpdateItem item;
   EXPECT_TRUE(component_updater()->GetComponentDetails(id, &item));
-  const CrxComponent& component(item.component);
+  ASSERT_TRUE(item.component);
+  const CrxComponent& component = *item.component;
 
   update_client::InstallerAttributes expected_attrs;
   expected_attrs["ap"] = "fake-ap";

components/component_updater/component_updater_service.cc

@@ -98,7 +98,6 @@ void CrxUpdateService::Start() {
       base::TimeDelta::FromSeconds(config_->NextCheckDelay()),
       base::Bind(base::IgnoreResult(&CrxUpdateService::CheckForUpdates),
                  base::Unretained(this)),
-      // TODO: Stop component update if requested.
       base::DoNothing());
 }
 
@@ -200,7 +199,7 @@ std::unique_ptr<ComponentInfo> CrxUpdateService::GetComponentForMimeType(
   const auto it = component_ids_by_mime_type_.find(mime_type);
   if (it == component_ids_by_mime_type_.end())
     return nullptr;
-  auto* const component = GetComponent(it->second);
+  const auto component = GetComponent(it->second);
   if (!component)
     return nullptr;
   return std::make_unique<ComponentInfo>(
@@ -224,11 +223,13 @@ OnDemandUpdater& CrxUpdateService::GetOnDemandUpdater() {
   return *this;
 }
 
-const CrxComponent* CrxUpdateService::GetComponent(
+base::Optional<CrxComponent> CrxUpdateService::GetComponent(
     const std::string& id) const {
   DCHECK(thread_checker_.CalledOnValidThread());
-  const auto it(components_.find(id));
-  return it != components_.end() ? &(it->second) : nullptr;
+  const auto it = components_.find(id);
+  if (it != components_.end())
+    return it->second;
+  return base::nullopt;
 }
 
 const CrxUpdateItem* CrxUpdateService::GetComponentState(
@@ -328,7 +329,7 @@ bool CrxUpdateService::CheckForUpdates(
   for (const auto id : components_order_) {
     DCHECK(components_.find(id) != components_.end());
 
-    auto* component(GetComponent(id));
+    const auto component = GetComponent(id);
     if (!component || component->requires_network_encryption)
       secure_ids.push_back(id);
     else
@@ -392,16 +393,12 @@ bool CrxUpdateService::GetComponentDetails(const std::string& id,
   return false;
 }
 
-std::vector<std::unique_ptr<CrxComponent>> CrxUpdateService::GetCrxComponents(
+std::vector<base::Optional<CrxComponent>> CrxUpdateService::GetCrxComponents(
     const std::vector<std::string>& ids) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  std::vector<std::unique_ptr<CrxComponent>> components;
-  for (const auto& id : ids) {
-    const auto* registered_component = GetComponent(id);
-    components.push_back(registered_component ? std::make_unique<CrxComponent>(
-                                                    *registered_component)
-                                              : nullptr);
-  }
+  std::vector<base::Optional<CrxComponent>> components;
+  for (const auto& id : ids)
+    components.push_back(GetComponent(id));
   return components;
 }
 
@@ -420,7 +417,7 @@ void CrxUpdateService::OnUpdateComplete(Callback callback,
 
   for (const auto id : components_pending_unregistration_) {
     if (!update_client_->IsUpdating(id)) {
-      const auto* component = GetComponent(id);
+      const auto component = GetComponent(id);
       if (component)
         DoUnregisterComponent(*component);
     }
@@ -451,16 +448,16 @@ void CrxUpdateService::OnEvent(Events event, const std::string& id) {
     return;
 
   // Update the state of the item.
-  auto it = component_states_.find(id);
-  DCHECK(it != component_states_.end());
-  it->second = update_item;
+  const auto it = component_states_.find(id);
+  if (it != component_states_.end())
+    it->second = update_item;
 
   // Update the component registration with the new version.
   if (event == Observer::Events::COMPONENT_UPDATED) {
-    auto* component(const_cast<CrxComponent*>(GetComponent(id)));
-    if (component) {
-      component->version = update_item.next_version;
-      component->fingerprint = update_item.next_fp;
+    const auto it = components_.find(id);
+    if (it != components_.end()) {
+      it->second.version = update_item.next_version;
+      it->second.fingerprint = update_item.next_fp;
     }
   }
 }

components/component_updater/component_updater_service_internal.h

@@ -12,6 +12,7 @@
 
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/optional.h"
 #include "base/threading/thread_checker.h"
 #include "components/component_updater/update_scheduler.h"
 
@@ -77,11 +78,11 @@ class CrxUpdateService : public ComponentUpdateService,
 
   bool DoUnregisterComponent(const CrxComponent& component);
 
-  const CrxComponent* GetComponent(const std::string& id) const;
+  base::Optional<CrxComponent> GetComponent(const std::string& id) const;
 
   const CrxUpdateItem* GetComponentState(const std::string& id) const;
 
-  std::vector<std::unique_ptr<CrxComponent>> GetCrxComponents(
+  std::vector<base::Optional<CrxComponent>> GetCrxComponents(
       const std::vector<std::string>& ids);
   void OnUpdateComplete(Callback callback,
                         const base::TimeTicks& start_time,

components/update_client/action_runner.cc

@@ -44,7 +44,7 @@ void ActionRunner::Run(Callback run_complete) {
 
 void ActionRunner::Unpack(
     std::unique_ptr<service_manager::Connector> connector) {
-  const auto& installer = component_.crx_component()->installer;
+  const auto installer = component_.crx_component()->installer;
 
   base::FilePath file_path;
   installer->GetInstalledFile(component_.action_run(), &file_path);

components/update_client/component.cc

@@ -256,7 +256,7 @@ void Component::Uninstall(const base::Version& version, int reason) {
 
   DCHECK_EQ(ComponentState::kNew, state());
 
-  crx_component_ = std::make_unique<CrxComponent>();
+  crx_component_ = CrxComponent();
   crx_component_->version = version;
 
   previous_version_ = version;

components/update_client/component.h

@@ -73,9 +73,11 @@ class Component {
 
   std::string id() const { return id_; }
 
-  const CrxComponent* crx_component() const { return crx_component_.get(); }
-  void set_crx_component(std::unique_ptr<CrxComponent> crx_component) {
-    crx_component_ = std::move(crx_component);
+  const base::Optional<CrxComponent>& crx_component() const {
+    return crx_component_;
+  }
+  void set_crx_component(const CrxComponent& crx_component) {
+    crx_component_ = crx_component;
   }
 
   const base::Version& previous_version() const { return previous_version_; }
@@ -371,7 +373,7 @@ class Component {
   base::ThreadChecker thread_checker_;
 
   const std::string id_;
-  std::unique_ptr<CrxComponent> crx_component_;
+  base::Optional<CrxComponent> crx_component_;
 
   // The status of the updatecheck response.
   std::string status_;

components/update_client/crx_update_item.h

@@ -11,6 +11,7 @@
 #include <vector>
 
 #include "base/memory/weak_ptr.h"
+#include "base/optional.h"
 #include "base/time/time.h"
 #include "base/version.h"
 #include "components/update_client/crx_downloader.h"
@@ -27,7 +28,12 @@ struct CrxUpdateItem {
   ComponentState state;
 
   std::string id;
-  CrxComponent component;
+
+  // The value of this data member is provided to the |UpdateClient| by the
+  // caller by responding to the |CrxDataCallback|. If the caller can't
+  // provide this value, for instance, in cases where the CRX was uninstalled,
+  // then the |component| member will not be present.
+  base::Optional<CrxComponent> component;
 
   // Time when an update check for this CRX has happened.
   base::TimeTicks last_check;

components/update_client/ping_manager_unittest.cc

@@ -112,7 +112,7 @@ TEST_F(PingManagerTest, SendPing) {
 
   {
     Component component(*update_context, "abc");
-    component.crx_component_ = std::make_unique<CrxComponent>();
+    component.crx_component_ = CrxComponent();
     component.state_ = std::make_unique<Component::StateUpdated>(&component);
     component.previous_version_ = base::Version("1.0");
     component.next_version_ = base::Version("2.0");
@@ -145,7 +145,7 @@ TEST_F(PingManagerTest, SendPing) {
   {
     // Test eventresult="0" is sent for failed updates.
     Component component(*update_context, "abc");
-    component.crx_component_ = std::make_unique<CrxComponent>();
+    component.crx_component_ = CrxComponent();
     component.state_ =
         std::make_unique<Component::StateUpdateError>(&component);
     component.previous_version_ = base::Version("1.0");
@@ -169,7 +169,7 @@ TEST_F(PingManagerTest, SendPing) {
   {
     // Test the error values and the fingerprints.
     Component component(*update_context, "abc");
-    component.crx_component_ = std::make_unique<CrxComponent>();
+    component.crx_component_ = CrxComponent();
     component.state_ =
         std::make_unique<Component::StateUpdateError>(&component);
     component.previous_version_ = base::Version("1.0");
@@ -206,7 +206,7 @@ TEST_F(PingManagerTest, SendPing) {
   {
     // Test an invalid |next_version| is not serialized.
     Component component(*update_context, "abc");
-    component.crx_component_ = std::make_unique<CrxComponent>();
+    component.crx_component_ = CrxComponent();
     component.state_ =
         std::make_unique<Component::StateUpdateError>(&component);
     component.previous_version_ = base::Version("1.0");
@@ -230,7 +230,7 @@ TEST_F(PingManagerTest, SendPing) {
     // Test a valid |previouversion| and |next_version| = base::Version("0")
     // are serialized correctly under <event...> for uninstall.
     Component component(*update_context, "abc");
-    component.crx_component_ = std::make_unique<CrxComponent>();
+    component.crx_component_ = CrxComponent();
     component.Uninstall(base::Version("1.2.3.4"), 0);
     component.AppendEvent(BuildUninstalledEventElement(component));
 
@@ -251,7 +251,7 @@ TEST_F(PingManagerTest, SendPing) {
   {
     // Test the download metrics.
     Component component(*update_context, "abc");
-    component.crx_component_ = std::make_unique<CrxComponent>();
+    component.crx_component_ = CrxComponent();
     component.state_ = std::make_unique<Component::StateUpdated>(&component);
     component.previous_version_ = base::Version("1.0");
     component.next_version_ = base::Version("2.0");
@@ -309,7 +309,7 @@ TEST_F(PingManagerTest, RequiresEncryption) {
   const auto update_context = MakeMockUpdateContext();
 
   Component component(*update_context, "abc");
-  component.crx_component_ = std::make_unique<CrxComponent>();
+  component.crx_component_ = CrxComponent();
 
   // The default value for |requires_network_encryption| is true.
   EXPECT_TRUE(component.crx_component_->requires_network_encryption);

components/update_client/protocol_builder.cc

@@ -336,7 +336,7 @@ std::string BuildUpdateCheckRequest(
     DCHECK_EQ(1u, components.count(id));
     const auto& component = *components.at(id);
     const auto& component_id = component.id();
-    const auto* crx_component = component.crx_component();
+    const auto crx_component = component.crx_component();
 
     DCHECK(crx_component);
 

components/update_client/update_checker_unittest.cc

@@ -227,17 +227,16 @@ scoped_refptr<UpdateContext> UpdateCheckerTest::MakeMockUpdateContext() const {
 }
 
 std::unique_ptr<Component> UpdateCheckerTest::MakeComponent() const {
-  std::unique_ptr<CrxComponent> crx_component =
-      std::make_unique<CrxComponent>();
-  crx_component->name = "test_jebg";
-  crx_component->pk_hash.assign(jebg_hash, jebg_hash + base::size(jebg_hash));
-  crx_component->installer = nullptr;
-  crx_component->version = base::Version("0.9");
-  crx_component->fingerprint = "fp1";
+  CrxComponent crx_component;
+  crx_component.name = "test_jebg";
+  crx_component.pk_hash.assign(jebg_hash, jebg_hash + base::size(jebg_hash));
+  crx_component.installer = nullptr;
+  crx_component.version = base::Version("0.9");
+  crx_component.fingerprint = "fp1";
 
   auto component = std::make_unique<Component>(*update_context_, kUpdateItemId);
   component->state_ = std::make_unique<Component::StateNew>(component.get());
-  component->crx_component_ = std::move(crx_component);
+  component->crx_component_ = crx_component;
 
   return component;
 }
@@ -621,7 +620,7 @@ TEST_F(UpdateCheckerTest, UpdateCheckInstallSource) {
   components[kUpdateItemId] = MakeComponent();
 
   auto& component = components[kUpdateItemId];
-  auto* crx_component = const_cast<CrxComponent*>(component->crx_component());
+  auto crx_component = component->crx_component();
 
   EXPECT_TRUE(post_interceptor_->ExpectRequest(
       std::make_unique<PartialMatch>("updatecheck"),
@@ -652,6 +651,7 @@ TEST_F(UpdateCheckerTest, UpdateCheckInstallSource) {
   update_context_->is_foreground = false;
   crx_component->install_source = "webstore";
   crx_component->install_location = "external";
+  component->set_crx_component(*crx_component);
   EXPECT_TRUE(post_interceptor_->ExpectRequest(
       std::make_unique<PartialMatch>("updatecheck"),
       test_file("updatecheck_reply_1.xml")));
@@ -668,6 +668,7 @@ TEST_F(UpdateCheckerTest, UpdateCheckInstallSource) {
   update_context_->is_foreground = true;
   crx_component->install_source = "sideload";
   crx_component->install_location = "policy";
+  component->set_crx_component(*crx_component);
   EXPECT_TRUE(post_interceptor_->ExpectRequest(
       std::make_unique<PartialMatch>("updatecheck"),
       test_file("updatecheck_reply_1.xml")));
@@ -689,7 +690,7 @@ TEST_F(UpdateCheckerTest, ComponentDisabled) {
   components[kUpdateItemId] = MakeComponent();
 
   auto& component = components[kUpdateItemId];
-  auto* crx_component = const_cast<CrxComponent*>(component->crx_component());
+  auto crx_component = component->crx_component();
 
   EXPECT_TRUE(post_interceptor_->ExpectRequest(
       std::make_unique<PartialMatch>("updatecheck"),
@@ -705,6 +706,7 @@ TEST_F(UpdateCheckerTest, ComponentDisabled) {
   EXPECT_THAT(body0, testing::Not(testing::HasSubstr("<disabled")));
 
   crx_component->disabled_reasons = {};
+  component->set_crx_component(*crx_component);
   update_checker_ = UpdateChecker::Create(config_, metadata_.get());
   EXPECT_TRUE(post_interceptor_->ExpectRequest(
       std::make_unique<PartialMatch>("updatecheck"),
@@ -720,6 +722,7 @@ TEST_F(UpdateCheckerTest, ComponentDisabled) {
   EXPECT_THAT(body1, testing::Not(testing::HasSubstr("<disabled")));
 
   crx_component->disabled_reasons = {0};
+  component->set_crx_component(*crx_component);
   EXPECT_TRUE(post_interceptor_->ExpectRequest(
       std::make_unique<PartialMatch>("updatecheck"),
       test_file("updatecheck_reply_1.xml")));
@@ -734,6 +737,7 @@ TEST_F(UpdateCheckerTest, ComponentDisabled) {
   EXPECT_THAT(body2, testing::HasSubstr(R"(<disabled reason="0")"));
 
   crx_component->disabled_reasons = {1};
+  component->set_crx_component(*crx_component);
   update_checker_ = UpdateChecker::Create(config_, metadata_.get());
   EXPECT_TRUE(post_interceptor_->ExpectRequest(
       std::make_unique<PartialMatch>("updatecheck"),
@@ -749,6 +753,7 @@ TEST_F(UpdateCheckerTest, ComponentDisabled) {
   EXPECT_THAT(body3, testing::HasSubstr(R"(<disabled reason="1")"));
 
   crx_component->disabled_reasons = {4, 8, 16};
+  component->set_crx_component(*crx_component);
   update_checker_ = UpdateChecker::Create(config_, metadata_.get());
   EXPECT_TRUE(post_interceptor_->ExpectRequest(
       std::make_unique<PartialMatch>("updatecheck"),
@@ -766,6 +771,7 @@ TEST_F(UpdateCheckerTest, ComponentDisabled) {
   EXPECT_THAT(body4, testing::HasSubstr(R"(<disabled reason="16")"));
 
   crx_component->disabled_reasons = {0, 4, 8, 16};
+  component->set_crx_component(*crx_component);
   update_checker_ = UpdateChecker::Create(config_, metadata_.get());
   EXPECT_TRUE(post_interceptor_->ExpectRequest(
       std::make_unique<PartialMatch>("updatecheck"),
@@ -792,14 +798,14 @@ TEST_F(UpdateCheckerTest, UpdateCheckUpdateDisabled) {
   components[kUpdateItemId] = MakeComponent();
 
   auto& component = components[kUpdateItemId];
+  auto crx_component = component->crx_component();
 
   // Tests the scenario where:
   //  * the component does not support group policies.
   //  * the component updates are disabled.
   // Expects the group policy to be ignored and the update check to not
   // include the "updatedisabled" attribute.
-  EXPECT_FALSE(component->crx_component_
-                   ->supports_group_policy_enable_component_updates);
+  EXPECT_FALSE(crx_component->supports_group_policy_enable_component_updates);
 
   EXPECT_TRUE(post_interceptor_->ExpectRequest(
       std::make_unique<PartialMatch>("updatecheck"),
@@ -818,8 +824,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckUpdateDisabled) {
   //  * the component supports group policies.
   //  * the component updates are disabled.
   // Expects the update check to include the "updatedisabled" attribute.
-  component->crx_component_->supports_group_policy_enable_component_updates =
-      true;
+  crx_component->supports_group_policy_enable_component_updates = true;
+  component->set_crx_component(*crx_component);
   update_checker_ = UpdateChecker::Create(config_, metadata_.get());
   EXPECT_TRUE(post_interceptor_->ExpectRequest(
       std::make_unique<PartialMatch>("updatecheck"),
@@ -839,8 +845,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckUpdateDisabled) {
   //  * the component does not support group policies.
   //  * the component updates are enabled.
   // Expects the update check to not include the "updatedisabled" attribute.
-  component->crx_component_->supports_group_policy_enable_component_updates =
-      false;
+  crx_component->supports_group_policy_enable_component_updates = false;
+  component->set_crx_component(*crx_component);
   update_checker_ = UpdateChecker::Create(config_, metadata_.get());
   EXPECT_TRUE(post_interceptor_->ExpectRequest(
       std::make_unique<PartialMatch>("updatecheck"),
@@ -859,8 +865,8 @@ TEST_F(UpdateCheckerTest, UpdateCheckUpdateDisabled) {
   //  * the component supports group policies.
   //  * the component updates are enabled.
   // Expects the update check to not include the "updatedisabled" attribute.
-  component->crx_component_->supports_group_policy_enable_component_updates =
-      true;
+  crx_component->supports_group_policy_enable_component_updates = true;
+  component->set_crx_component(*crx_component);
   update_checker_ = UpdateChecker::Create(config_, metadata_.get());
   EXPECT_TRUE(post_interceptor_->ExpectRequest(
       std::make_unique<PartialMatch>("updatecheck"),

components/update_client/update_client.cc

@@ -37,21 +37,15 @@
 namespace update_client {
 
 CrxUpdateItem::CrxUpdateItem() : state(ComponentState::kNew) {}
-
-CrxUpdateItem::~CrxUpdateItem() {
-}
-
+CrxUpdateItem::~CrxUpdateItem() = default;
 CrxUpdateItem::CrxUpdateItem(const CrxUpdateItem& other) = default;
 
 CrxComponent::CrxComponent()
     : allows_background_download(true),
       requires_network_encryption(true),
       supports_group_policy_enable_component_updates(false) {}
-
 CrxComponent::CrxComponent(const CrxComponent& other) = default;
-
-CrxComponent::~CrxComponent() {
-}
+CrxComponent::~CrxComponent() = default;
 
 // It is important that an instance of the UpdateClient binds an unretained
 // pointer to itself. Otherwise, a life time circular dependency between this

components/update_client/update_client.h

@@ -8,12 +8,12 @@
 #include <stdint.h>
 
 #include <map>
-#include <memory>
 #include <string>
 #include <vector>
 
 #include "base/callback_forward.h"
 #include "base/memory/ref_counted.h"
+#include "base/optional.h"
 #include "base/version.h"
 #include "components/update_client/update_client_errors.h"
 
@@ -211,7 +211,6 @@ class CrxInstaller : public base::RefCountedThreadSafe<CrxInstaller> {
 // may be used in the update checks requests.
 using InstallerAttributes = std::map<std::string, std::string>;
 
-// TODO(sorin): this structure will be refactored soon.
 struct CrxComponent {
   CrxComponent();
   CrxComponent(const CrxComponent& other);
@@ -275,7 +274,7 @@ using Callback = base::OnceCallback<void(Error error)>;
 class UpdateClient : public base::RefCounted<UpdateClient> {
  public:
   using CrxDataCallback =
-      base::OnceCallback<std::vector<std::unique_ptr<CrxComponent>>(
+      base::OnceCallback<std::vector<base::Optional<CrxComponent>>(
           const std::vector<std::string>& ids)>;
 
   // Defines an interface to observe the UpdateClient. It provides

components/update_client/update_engine.cc

@@ -11,6 +11,7 @@
 #include "base/guid.h"
 #include "base/location.h"
 #include "base/logging.h"
+#include "base/optional.h"
 #include "base/stl_util.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/prefs/pref_service.h"
@@ -103,7 +104,7 @@ void UpdateEngine::Update(bool is_foreground,
 
   // Calls out to get the corresponding CrxComponent data for the CRXs in this
   // update context.
-  std::vector<std::unique_ptr<CrxComponent>> crx_components =
+  const auto crx_components =
       std::move(update_context->crx_data_callback).Run(update_context->ids);
   DCHECK_EQ(update_context->ids.size(), crx_components.size());
 
@@ -112,12 +113,12 @@ void UpdateEngine::Update(bool is_foreground,
 
     DCHECK(update_context->components[id]->state() == ComponentState::kNew);
 
-    auto& crx_component = crx_components[i];
+    const auto crx_component = crx_components[i];
     if (crx_component) {
       // This component can be checked for updates.
       DCHECK_EQ(id, GetCrxComponentID(*crx_component));
       auto& component = update_context->components[id];
-      component->set_crx_component(std::move(crx_component));
+      component->set_crx_component(*crx_component);
       component->set_previous_version(component->crx_component()->version);
       component->set_previous_fp(component->crx_component()->fingerprint);
       update_context->components_to_check_for_updates.push_back(id);
@@ -379,7 +380,7 @@ bool UpdateEngine::GetUpdateState(const std::string& id,
   for (const auto& context : update_contexts_) {
     const auto& components = context.second->components;
     const auto it = components.find(id);
-    if (it != components.end() && it->second->crx_component()) {
+    if (it != components.end()) {
       *update_item = it->second->GetCrxUpdateItem();
       return true;
     }

extensions/browser/updater/update_data_provider.cc

@@ -72,24 +72,25 @@ void UpdateDataProvider::Shutdown() {
   browser_context_ = nullptr;
 }
 
-std::vector<std::unique_ptr<update_client::CrxComponent>>
+std::vector<base::Optional<update_client::CrxComponent>>
 UpdateDataProvider::GetData(bool install_immediately,
                             const ExtensionUpdateDataMap& update_crx_component,
                             const std::vector<std::string>& ids) {
-  std::vector<std::unique_ptr<update_client::CrxComponent>> data;
+  std::vector<base::Optional<update_client::CrxComponent>> data;
   if (!browser_context_)
     return data;
   const ExtensionRegistry* registry = ExtensionRegistry::Get(browser_context_);
   const ExtensionPrefs* extension_prefs = ExtensionPrefs::Get(browser_context_);
   for (const auto& id : ids) {
     const Extension* extension = registry->GetInstalledExtension(id);
-    data.push_back(extension ? std::make_unique<update_client::CrxComponent>()
-                             : nullptr);
+    data.push_back(extension
+                       ? base::make_optional<update_client::CrxComponent>()
+                       : base::nullopt);
     if (!extension)
       continue;
     DCHECK_NE(0u, update_crx_component.count(id));
     const ExtensionUpdateData& extension_data = update_crx_component.at(id);
-    update_client::CrxComponent* crx_component = data.back().get();
+    auto& crx_component = data.back();
     std::string pubkey_bytes;
     base::Base64Decode(extension->public_key(), &pubkey_bytes);
     crx_component->pk_hash.resize(crypto::kSHA256Length, 0);

extensions/browser/updater/update_data_provider.h

@@ -13,6 +13,7 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/optional.h"
 #include "extensions/browser/updater/extension_installer.h"
 #include "extensions/browser/updater/extension_update_data.h"
 
@@ -47,7 +48,7 @@ class UpdateDataProvider : public base::RefCounted<UpdateDataProvider> {
   // done.
   void Shutdown();
 
-  std::vector<std::unique_ptr<update_client::CrxComponent>> GetData(
+  std::vector<base::Optional<update_client::CrxComponent>> GetData(
       bool install_immediately,
       const ExtensionUpdateDataMap& update_info,
       const std::vector<std::string>& ids);

extensions/browser/updater/update_data_provider_unittest.cc

@@ -14,6 +14,7 @@
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
+#include "base/optional.h"
 #include "base/run_loop.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/update_client/update_client.h"
@@ -383,13 +384,13 @@ TEST_F(UpdateDataProviderTest,
                              {kExtensionId1, kExtensionId2});
 
   ASSERT_EQ(2UL, data.size());
-  ASSERT_NE(nullptr, data[0]);
+  ASSERT_NE(base::nullopt, data[0]);
   EXPECT_EQ(version, data[0]->version.GetString());
   EXPECT_NE(nullptr, data[0]->installer.get());
   EXPECT_EQ(0UL, data[0]->disabled_reasons.size());
   EXPECT_EQ("other", data[0]->install_location);
 
-  EXPECT_EQ(nullptr, data[1]);
+  EXPECT_EQ(base::nullopt, data[1]);
 }
 
 TEST_F(UpdateDataProviderTest, GetData_MultipleExtensions_CorruptExtension) {

extensions/browser/updater/update_service_unittest.cc

@@ -10,6 +10,7 @@
 
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
+#include "base/optional.h"
 #include "base/run_loop.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
@@ -49,7 +50,7 @@ class FakeUpdateClient : public update_client::UpdateClient {
 
   // Returns the data we've gotten from the CrxDataCallback for ids passed to
   // the Update function.
-  std::vector<std::unique_ptr<update_client::CrxComponent>>* data() {
+  std::vector<base::Optional<update_client::CrxComponent>>* data() {
     return &data_;
   }
 
@@ -123,7 +124,7 @@ class FakeUpdateClient : public update_client::UpdateClient {
   friend class base::RefCounted<FakeUpdateClient>;
   ~FakeUpdateClient() override {}
 
-  std::vector<std::unique_ptr<update_client::CrxComponent>> data_;
+  std::vector<base::Optional<update_client::CrxComponent>> data_;
   std::vector<UninstallPing> uninstall_pings_;
   std::vector<Observer*> observers_;