Refactor the UpdateEngine and its actions in the component updater.

components/update_client/component.h

Index: components/update_client/component.h
diff --git a/components/update_client/component.h b/components/update_client/component.h
new file mode 100644
index 0000000000000000000000000000000000000000..6e3ef0deb86e452709f9d77bceb601a31c768fa0
--- /dev/null
+++ b/components/update_client/component.h
@@ -0,0 +1,437 @@
+// Copyright 2017 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_COMPONENT_H_
+#define COMPONENTS_UPDATE_CLIENT_COMPONENT_H_
+
+#include <map>
+#include <memory>
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "base/callback.h"
+#include "base/files/file_path.h"
+#include "base/gtest_prod_util.h"
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/threading/thread_checker.h"
+#include "base/time/time.h"
+#include "base/version.h"
+#include "components/update_client/component_unpacker.h"
+#include "components/update_client/crx_downloader.h"
+#include "components/update_client/update_client.h"
+#include "components/update_client/update_response.h"
+#include "url/gurl.h"
+
+namespace base {
+class SingleThreadTaskRunner;
+}
+
+namespace update_client {
+
+struct CrxUpdateItem;
+struct UpdateContext;
+
+// Describes a CRX component managed by the UpdateEngine. A |Component| can
+// only exists if it is associated with an UpdateContext.

[waffles, 2017/04/24 18:57:22]

exists → exist

or "Each |Component| is associated with an UpdateContext."

[Sorin Jianu, 2017/04/24 23:26:45]

Done.

+class Component {
+ public:
+  using Events = UpdateClient::Observer::Events;
+
+  using CallbackHandleComplete = base::Callback<void()>;
+
+  Component(const UpdateContext& update_context, const std::string& id);
+  ~Component();
+
+  // Handles the current state of the component and makes it transition
+  // to the next component state before |callback| is invoked.
+  void Handle(CallbackHandleComplete callback);
+
+  CrxUpdateItem GetCrxUpdateItem() const;
+
+  // Called by the UpdateChecker to set the update response for this component.
+  void SetParseResult(const UpdateResponse::Result& result);
+
+  // Sets the uninstall state for this component.
+  void Uninstall(const base::Version& cur_version, int reason);
+
+  // Called by the UpdateEngine when an update check for this component is done.
+  void UpdateCheckComplete() const;
+
+  // Returns true if the component has reached a final state and no further
+  // handling and state transitions are possible.
+  bool IsHandled() const { return state_->IsFinal(); }
+
+  // Returns true if an update is available for this component, meaning that
+  // the update server has return a response containing an update.
+  bool IsUpdateAvailable() const { return is_update_available_; }
+
+  // Returns true if a ping must be sent back to the server. As a general rule,
+  // a ping is sent only for server responses containing instructions to update.
+  bool CanPing() const {
+    return IsUpdateAvailable() || state() == ComponentState::kUninstalled;
+  }
+
+  base::TimeDelta GetUpdateDuration() const;
+
+  ComponentState state() const { return state_->state(); }
+
+  std::string id() const { return id_; }
+
+  const 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_; }
+  void set_previous_version(const base::Version& previous_version) {
+    previous_version_ = previous_version;
+  }
+
+  const base::Version& next_version() const { return next_version_; }
+
+  std::string previous_fp() const { return previous_fp_; }
+  void set_previous_fp(const std::string& previous_fp) {
+    previous_fp_ = previous_fp;
+  }
+
+  std::string next_fp() const { return next_fp_; }
+  void set_next_fp(const std::string& next_fp) { next_fp_ = next_fp; }
+
+  int update_check_error() const { return update_check_error_; }
+  void set_update_check_error(int update_check_error) {
+    update_check_error_ = update_check_error;
+  }
+
+  // Returns the time when processing of an update for this component has
+  // begun, once the update has been discovered. Returns a null TimeTicks object
+  // if the handling of an update has not happened.
+  // base::TimeTicks update_begin() const { return update_begin_; }
+
+  bool on_demand() const { return on_demand_; }
+  void set_on_demand(bool on_demand) { on_demand_ = on_demand; }
+
+  const std::vector<CrxDownloader::DownloadMetrics>& download_metrics() const {
+    return download_metrics_;
+  }
+
+  const std::vector<GURL>& crx_diffurls() const { return crx_diffurls_; }
+
+  bool diff_update_failed() const { return !!diff_error_code_; }
+
+  int error_category() const { return error_category_; }
+  int error_code() const { return error_code_; }
+  int extra_code1() const { return extra_code1_; }
+  int diff_error_category() const { return diff_error_category_; }
+  int diff_error_code() const { return diff_error_code_; }
+  int diff_extra_code1() const { return diff_extra_code1_; }
+
+ private:
+  friend class FakePingManagerImpl;
+  friend class UpdateCheckerTest;
+
+  FRIEND_TEST_ALL_PREFIXES(PingManagerTest, SendPing);
+  FRIEND_TEST_ALL_PREFIXES(PingManagerTest, RequiresEncryption);
+  FRIEND_TEST_ALL_PREFIXES(UpdateCheckerTest, UpdateCheckCupError);
+  FRIEND_TEST_ALL_PREFIXES(UpdateCheckerTest, UpdateCheckError);
+  FRIEND_TEST_ALL_PREFIXES(UpdateCheckerTest, UpdateCheckInvalidAp);
+  FRIEND_TEST_ALL_PREFIXES(UpdateCheckerTest,
+                           UpdateCheckRequiresEncryptionError);
+  FRIEND_TEST_ALL_PREFIXES(UpdateCheckerTest, UpdateCheckSuccess);
+  FRIEND_TEST_ALL_PREFIXES(UpdateCheckerTest, UpdateCheckUpdateDisabled);
+
+  // Describes an abstraction for implementing the behavior of a component and
+  // the transition from one state to another.
+  class State {
+   public:
+    using CallbackNextState =
+        base::Callback<void(std::unique_ptr<State> next_state)>;
+
+    State(Component* component, ComponentState state);

[waffles, 2017/04/24 18:57:22]

Should this be protected?

[Sorin Jianu, 2017/04/24 23:26:45]

It seems ok to be public. The inner base class is private, and it is abstract.
The outer class or its friends can't instantiate the inner base class anyway.

+    virtual ~State();
+
+    // Handles the current state and initiates a transition to a new state.
+    // The transition to the new state is non-blocking and it is completed
+    // by the outer component, after the current state is fully handled.
+    void Handle(CallbackNextState callback);
+
+    ComponentState state() const { return state_; }
+
+    bool IsFinal() const { return is_final_; }
+
+   protected:
+    // Initiates the transition to the new state.
+    void TransitionState(std::unique_ptr<State> new_state);
+
+    Component& component() { return component_; }
+    const Component& component() const { return component_; }
+
+    CallbackNextState callback() const { return callback_; }
+
+    base::ThreadChecker thread_checker_;
+
+    const ComponentState state_;
+
+   private:
+    virtual void DoHandle() = 0;
+
+    Component& component_;
+    CallbackNextState callback_;
+
+    bool is_final_ = false;
+  };
+
+  class StateNew : public State {
+   public:
+    explicit StateNew(Component* component);
+    ~StateNew() override;
+
+   private:
+    // State overrides.
+    void DoHandle() override;
+
+    DISALLOW_COPY_AND_ASSIGN(StateNew);
+  };
+
+  class StateChecking : public State {
+   public:
+    explicit StateChecking(Component* component);
+    ~StateChecking() override;
+
+   private:
+    // State overrides.
+    void DoHandle() override;
+
+    void UpdateCheckComplete();
+
+    DISALLOW_COPY_AND_ASSIGN(StateChecking);
+  };
+
+  class StateUpdateError : public State {
+   public:
+    explicit StateUpdateError(Component* component);
+    ~StateUpdateError() override;
+
+   private:
+    // State overrides.
+    void DoHandle() override;
+
+    DISALLOW_COPY_AND_ASSIGN(StateUpdateError);
+  };
+
+  class StateCanUpdate : public State {
+   public:
+    explicit StateCanUpdate(Component* component);
+    ~StateCanUpdate() override;
+
+   private:
+    // State overrides.
+    void DoHandle() override;
+    bool CanTryDiffUpdate() const;
+
+    DISALLOW_COPY_AND_ASSIGN(StateCanUpdate);
+  };
+
+  class StateUpToDate : public State {
+   public:
+    explicit StateUpToDate(Component* component);
+    ~StateUpToDate() override;
+
+   private:
+    // State overrides.
+    void DoHandle() override;
+
+    DISALLOW_COPY_AND_ASSIGN(StateUpToDate);
+  };
+
+  class StateDownloadingDiff : public State {
+   public:
+    explicit StateDownloadingDiff(Component* component);
+    ~StateDownloadingDiff() override;
+
+   private:
+    // State overrides.
+    void DoHandle() override;
+
+    // Called when progress is being made downloading a CRX. The progress may
+    // not monotonically increase due to how the CRX downloader switches between
+    // different downloaders and fallback urls.
+    void DownloadProgress(const std::string& id,
+                          const CrxDownloader::Result& download_result);
+
+    void DownloadComplete(const std::string& id,
+                          const CrxDownloader::Result& download_result);
+
+    // Downloads updates for one CRX id only.
+    std::unique_ptr<CrxDownloader> crx_downloader_;
+
+    DISALLOW_COPY_AND_ASSIGN(StateDownloadingDiff);
+  };
+
+  class StateDownloading : public State {
+   public:
+    explicit StateDownloading(Component* component);
+    ~StateDownloading() override;
+
+   private:
+    // State overrides.
+    void DoHandle() override;
+
+    // Called when progress is being made downloading a CRX. The progress may
+    // not monotonically increase due to how the CRX downloader switches between
+    // different downloaders and fallback urls.
+    void DownloadProgress(const std::string& id,
+                          const CrxDownloader::Result& download_result);
+
+    void DownloadComplete(const std::string& id,
+                          const CrxDownloader::Result& download_result);
+
+    // Downloads updates for one CRX id only.
+    std::unique_ptr<CrxDownloader> crx_downloader_;
+
+    DISALLOW_COPY_AND_ASSIGN(StateDownloading);
+  };
+
+  class StateUpdatingDiff : public State {
+   public:
+    explicit StateUpdatingDiff(Component* component);
+    ~StateUpdatingDiff() override;
+
+   private:
+    // State overrides.
+    void DoHandle() override;
+
+    void InstallComplete(int error_category, int error_code, int extra_code1);
+
+    DISALLOW_COPY_AND_ASSIGN(StateUpdatingDiff);
+  };
+
+  class StateUpdating : public State {
+   public:
+    explicit StateUpdating(Component* component);
+    ~StateUpdating() override;
+
+   private:
+    // State overrides.
+    void DoHandle() override;
+
+    void InstallComplete(int error_category, int error_code, int extra_code1);
+
+    // Posts replies back to the main thread.
+    scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
+
+    // Unpacks one CRX.
+    scoped_refptr<ComponentUnpacker> unpacker_;
+
+    base::FilePath unpack_path_;
+
+    DISALLOW_COPY_AND_ASSIGN(StateUpdating);
+  };
+
+  class StateUpdated : public State {
+   public:
+    explicit StateUpdated(Component* component);
+    ~StateUpdated() override;
+
+   private:
+    // State overrides.
+    void DoHandle() override;
+
+    DISALLOW_COPY_AND_ASSIGN(StateUpdated);
+  };
+
+  class StateUninstalled : public State {
+   public:
+    explicit StateUninstalled(Component* component);
+    ~StateUninstalled() override;
+
+   private:
+    // State overrides.
+    void DoHandle() override;
+
+    DISALLOW_COPY_AND_ASSIGN(StateUninstalled);
+  };
+
+  // Returns true is the update payload for this component can be downloaded
+  // by a downloader which can do bandwidth throttling on the client side.
+  bool CanDoBackgroundDownload() const;
+
+  void AppendDownloadMetrics(
+      const std::vector<CrxDownloader::DownloadMetrics>& download_metrics);
+
+  // Changes the component state and notifies the caller of the |Handle|
+  // function that the handling of this component state is complete.
+  void ChangeState(std::unique_ptr<State> next_state);
+
+  // Notifies registered observers about changes in the state of the component.
+  void NotifyObservers(Events event) const;
+
+  base::ThreadChecker thread_checker_;
+
+  const std::string id_;
+  CrxComponent crx_component_;
+
+  std::string status_;
+
+  // Time when an update check for this CRX has happened.
+  base::TimeTicks last_check_;
+
+  // Time when the update of this CRX has begun.
+  base::TimeTicks update_begin_;
+
+  // A component can be made available for download from several urls.
+  std::vector<GURL> crx_urls_;
+  std::vector<GURL> crx_diffurls_;
+
+  // The cryptographic hash values for the component payload.
+  std::string hash_sha256_;
+  std::string hashdiff_sha256_;
+
+  // The from/to version and fingerprint values.
+  base::Version previous_version_;
+  base::Version next_version_;
+  std::string previous_fp_;
+  std::string next_fp_;
+
+  // True if the update check response for this component includes an update.
+  bool is_update_available_ = false;
+
+  // True if the current update check cycle is on-demand.
+  bool on_demand_ = false;
+
+  // The error reported by the update checker.
+  int update_check_error_ = 0;
+
+  base::FilePath crx_path_;
+
+  // The error information for full and differential updates.
+  // The |error_category| contains a hint about which module in the component
+  // updater generated the error. The |error_code| constains the error and
+  // the |extra_code1| usually contains a system error, but it can contain
+  // any extended information that is relevant to either the category or the
+  // error itself.
+  int error_category_ = 0;
+  int error_code_ = 0;
+  int extra_code1_ = 0;
+  int diff_error_category_ = 0;
+  int diff_error_code_ = 0;
+  int diff_extra_code1_ = 0;
+
+  std::vector<CrxDownloader::DownloadMetrics> download_metrics_;
+
+  CallbackHandleComplete callback_handle_complete_;
+  std::unique_ptr<State> state_;
+  const UpdateContext& update_context_;
+
+  base::Closure update_check_complete_;
+
+  DISALLOW_COPY_AND_ASSIGN(Component);
+};
+
+using IdToComponentPtrMap = std::map<std::string, std::unique_ptr<Component>>;
+
+}  // namespace update_client
+
+#endif  // COMPONENTS_UPDATE_CLIENT_COMPONENT_H_