Create SendRegistrationPing in UpdateClient

Create a new state and event within the component type. Within the
UpdateEngine and UpdateClient create SendRegistrationPing that calls
for registration event to be sent. The execution path for the
the registration ping is not taken by the code inside the component
updater The change is needed for the code in //chrome/updater.

Bug:1096733

Change-Id: I0f239d8d837eb22dda49165ae04698d896860578
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2290195Reviewed-by: Tommy Li <tommycli@chromium.org>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Commit-Queue: Samantha Sappenfield <t-sasap@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#792810}

chrome/browser/ui/webui/components/components_handler.cc

@@ -134,6 +134,7 @@ base::string16 ComponentsHandler::ServiceStatusToString(
     case update_client::ComponentState::kUpdateError:
       return l10n_util::GetStringUTF16(IDS_COMPONENTS_SVC_STATUS_UPDATE_ERROR);
     case update_client::ComponentState::kUninstalled:  // Fall through.
+    case update_client::ComponentState::kRegistration:
     case update_client::ComponentState::kRun:
     case update_client::ComponentState::kLastStatus:
       return l10n_util::GetStringUTF16(IDS_COMPONENTS_UNKNOWN);

chrome/updater/update_service_in_process.cc

@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/run_loop.h"
 #include "base/sequenced_task_runner.h"
@@ -72,6 +73,7 @@ UpdateService::UpdateState::State ToUpdateState(
       return UpdateService::UpdateState::State::kUpdateError;
 
     case update_client::ComponentState::kUninstalled:
+    case update_client::ComponentState::kRegistration:
     case update_client::ComponentState::kRun:
     case update_client::ComponentState::kLastStatus:
       NOTREACHED();
@@ -156,6 +158,9 @@ void UpdateServiceInProcess::RegisterApp(
 
   persisted_data_->RegisterApp(request);
 
+  update_client_->SendRegistrationPing(request.app_id, request.version,
+                                       base::DoNothing());
+
   // Result of registration. Currently there's no error handling in
   // PersistedData, so we assume success every time, which is why we respond
   // with 0.

components/component_updater/component_installer_unittest.cc

@@ -98,6 +98,13 @@ class MockUpdateClient : public UpdateClient {
     std::move(callback).Run(update_client::Error::NONE);
   }
 
+  void SendRegistrationPing(const std::string& id,
+                            const base::Version& version,
+                            Callback callback) override {
+    DoSendRegistrationPing(id, version);
+    std::move(callback).Run(update_client::Error::NONE);
+  }
+
   MOCK_METHOD1(AddObserver, void(Observer* observer));
   MOCK_METHOD1(RemoveObserver, void(Observer* observer));
   MOCK_METHOD2(DoInstall,
@@ -114,6 +121,8 @@ class MockUpdateClient : public UpdateClient {
                void(const std::string& id,
                     const base::Version& version,
                     int reason));
+  MOCK_METHOD2(DoSendRegistrationPing,
+               void(const std::string& id, const base::Version& version));
 
  private:
   ~MockUpdateClient() override = default;

components/component_updater/component_updater_service_unittest.cc

@@ -88,6 +88,10 @@ class MockUpdateClient : public UpdateClient {
                     const base::Version& version,
                     int reason,
                     Callback callback));
+  MOCK_METHOD3(SendRegistrationPing,
+               void(const std::string& id,
+                    const base::Version& version,
+                    Callback callback));
 
  private:
   ~MockUpdateClient() override = default;

components/update_client/BUILD.gn

@@ -98,6 +98,8 @@ static_library("update_client") {
     "request_sender.cc",
     "request_sender.h",
     "task.h",
+    "task_send_registration_ping.cc",
+    "task_send_registration_ping.h",
     "task_send_uninstall_ping.cc",
     "task_send_uninstall_ping.h",
     "task_traits.h",

components/update_client/component.cc

@@ -361,6 +361,19 @@ void Component::Uninstall(const base::Version& version, int reason) {
   state_ = std::make_unique<StateUninstalled>(this);
 }
 
+void Component::Registration(const base::Version& version) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  DCHECK_EQ(ComponentState::kNew, state());
+
+  crx_component_ = CrxComponent();
+  crx_component_->version = version;
+
+  next_version_ = version;
+
+  state_ = std::make_unique<StateRegistration>(this);
+}
+
 void Component::SetUpdateCheckResult(
     const base::Optional<ProtocolParser::Result>& result,
     ErrorCategory error_category,
@@ -498,6 +511,20 @@ base::Value Component::MakeEventUninstalled() const {
   return event;
 }
 
+base::Value Component::MakeEventRegistration() const {
+  DCHECK(state() == ComponentState::kRegistration);
+  base::Value event(base::Value::Type::DICTIONARY);
+  event.SetKey("eventtype", base::Value(2));
+  event.SetKey("eventresult", base::Value(1));
+  if (error_code())
+    event.SetKey("errorcode", base::Value(error_code()));
+  if (extra_code1())
+    event.SetKey("extracode1", base::Value(extra_code1()));
+  DCHECK(next_version().IsValid());
+  event.SetKey("nextversion", base::Value(next_version().GetString()));
+  return event;
+}
+
 base::Value Component::MakeEventActionRun(bool succeeded,
                                           int error_code,
                                           int extra_code1) const {
@@ -1046,6 +1073,25 @@ void Component::StateUninstalled::DoHandle() {
   EndState();
 }
 
+Component::StateRegistration::StateRegistration(Component* component)
+    : State(component, ComponentState::kRegistration) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+}
+
+Component::StateRegistration::~StateRegistration() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+}
+void Component::StateRegistration::DoHandle() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  auto& component = State::component();
+  DCHECK(component.crx_component());
+
+  component.AppendEvent(component.MakeEventRegistration());
+
+  EndState();
+}
+
 Component::StateRun::StateRun(Component* component)
     : State(component, ComponentState::kRun) {}
 

components/update_client/component.h

@@ -57,6 +57,9 @@ class Component {
   // Sets the uninstall state for this component.
   void Uninstall(const base::Version& cur_version, int reason);
 
+  // Set the registration state for this component.
+  void Registration(const base::Version& cur_version);
+
   // Called by the UpdateEngine when an update check for this component is done.
   void SetUpdateCheckResult(
       const base::Optional<ProtocolParser::Result>& result,
@@ -346,6 +349,18 @@ class Component {
     void DoHandle() override;
   };
 
+  class StateRegistration : public State {
+   public:
+    explicit StateRegistration(Component* component);
+    ~StateRegistration() override;
+
+   private:
+    // State overrides.
+    void DoHandle() override;
+
+    DISALLOW_COPY_AND_ASSIGN(StateRegistration);
+  };
+
   class StateRun : public State {
    public:
     explicit StateRun(Component* component);
@@ -388,6 +403,7 @@ class Component {
   base::Value MakeEventDownloadMetrics(
       const CrxDownloader::DownloadMetrics& download_metrics) const;
   base::Value MakeEventUninstalled() const;
+  base::Value MakeEventRegistration() const;
   base::Value MakeEventActionRun(bool succeeded,
                                  int error_code,
                                  int extra_code1) const;

components/update_client/ping_manager_unittest.cc

@@ -316,6 +316,33 @@ TEST_P(PingManagerTest, SendPing) {
     interceptor->Reset();
   }
 
+  {
+    // Test registrationEvent.
+    Component component(*update_context, "abc");
+    component.crx_component_ = CrxComponent();
+    component.Registration(base::Version("1.2.3.4"));
+    component.AppendEvent(component.MakeEventRegistration());
+
+    EXPECT_TRUE(interceptor->ExpectRequest(std::make_unique<AnyMatch>()));
+    ping_manager_->SendPing(component, MakePingCallback());
+    RunThreads();
+
+    EXPECT_EQ(1, interceptor->GetCount()) << interceptor->GetRequestsAsString();
+    const auto msg = interceptor->GetRequestBody(0);
+
+    const auto root = base::JSONReader::Read(msg);
+    ASSERT_TRUE(root);
+    const auto* request = root->FindKey("request");
+    const auto& app = request->FindKey("app")->GetList()[0];
+    EXPECT_EQ("abc", app.FindKey("appid")->GetString());
+    EXPECT_EQ("1.2.3.4", app.FindKey("version")->GetString());
+    const auto& event = app.FindKey("event")->GetList()[0];
+    EXPECT_EQ(1, event.FindKey("eventresult")->GetInt());
+    EXPECT_EQ(2, event.FindKey("eventtype")->GetInt());
+    EXPECT_EQ("1.2.3.4", event.FindKey("nextversion")->GetString());
+    interceptor->Reset();
+  }
+
   {
     // Test the download metrics.
     Component component(*update_context, "abc");

components/update_client/task_send_registration_ping.cc

+// Copyright 2020 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/task_send_registration_ping.h"
+
+#include <utility>
+
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/threading/sequenced_task_runner_handle.h"
+#include "base/version.h"
+#include "components/update_client/update_client.h"
+#include "components/update_client/update_engine.h"
+
+namespace update_client {
+
+TaskSendRegistrationPing::TaskSendRegistrationPing(
+    scoped_refptr<UpdateEngine> update_engine,
+    const std::string& id,
+    const base::Version& version,
+    Callback callback)
+    : update_engine_(update_engine),
+      id_(id),
+      version_(version),
+      callback_(std::move(callback)) {}
+
+TaskSendRegistrationPing::~TaskSendRegistrationPing() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+}
+
+void TaskSendRegistrationPing::Run() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  if (id_.empty()) {
+    TaskComplete(Error::INVALID_ARGUMENT);
+    return;
+  }
+
+  update_engine_->SendRegistrationPing(
+      id_, version_,
+      base::BindOnce(&TaskSendRegistrationPing::TaskComplete, this));
+}
+
+void TaskSendRegistrationPing::Cancel() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  TaskComplete(Error::UPDATE_CANCELED);
+}
+
+std::vector<std::string> TaskSendRegistrationPing::GetIds() const {
+  return std::vector<std::string>{id_};
+}
+
+void TaskSendRegistrationPing::TaskComplete(Error error) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  base::SequencedTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE,
+      base::BindOnce(&TaskSendRegistrationPing::RunCallback, this, error));
+}
+
+void TaskSendRegistrationPing::RunCallback(Error error) {
+  std::move(callback_).Run(this, error);
+}
+
+}  // namespace update_client

components/update_client/task_send_registration_ping.h

+// Copyright 2020 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_TASK_SEND_REGISTRATION_PING_H_
+#define COMPONENTS_UPDATE_CLIENT_TASK_SEND_REGISTRATION_PING_H_
+
+#include <string>
+#include <vector>
+
+#include "base/callback.h"
+#include "base/memory/ref_counted.h"
+#include "base/sequence_checker.h"
+#include "components/update_client/task.h"
+#include "components/update_client/update_client.h"
+
+namespace base {
+class Version;
+}
+
+namespace update_client {
+
+class UpdateEngine;
+enum class Error;
+
+// Defines a specialized task for sending the registration ping.
+class TaskSendRegistrationPing : public Task {
+ public:
+  using Callback =
+      base::OnceCallback<void(scoped_refptr<Task> task, Error error)>;
+
+  TaskSendRegistrationPing(const TaskSendRegistrationPing&) = delete;
+  TaskSendRegistrationPing& operator=(const TaskSendRegistrationPing&) = delete;
+
+  // |update_engine| is injected here to handle the task.
+  // |id| represents the CRX to send the ping for.
+  // |callback| is posted when the task is done.
+  TaskSendRegistrationPing(scoped_refptr<UpdateEngine> update_engine,
+                           const std::string& id,
+                           const base::Version& version,
+                           Callback callback);
+
+  void Run() override;
+
+  void Cancel() override;
+
+  std::vector<std::string> GetIds() const override;
+
+ private:
+  ~TaskSendRegistrationPing() override;
+
+  // Called when the task has completed either because the task has run or
+  // it has been canceled.
+  void TaskComplete(Error error);
+
+  // Runs the task registration ping callback
+  void RunCallback(Error error);
+
+  SEQUENCE_CHECKER(sequence_checker_);
+  scoped_refptr<UpdateEngine> update_engine_;
+  const std::string id_;
+  const base::Version version_;
+  Callback callback_;
+};
+
+}  // namespace update_client
+
+#endif  // COMPONENTS_UPDATE_CLIENT_TASK_SEND_REGISTRATION_PING_H_

components/update_client/update_client.cc

@@ -26,6 +26,7 @@
 #include "components/update_client/persisted_data.h"
 #include "components/update_client/ping_manager.h"
 #include "components/update_client/protocol_parser.h"
+#include "components/update_client/task_send_registration_ping.h"
 #include "components/update_client/task_send_uninstall_ping.h"
 #include "components/update_client/task_update.h"
 #include "components/update_client/update_checker.h"
@@ -239,6 +240,17 @@ void UpdateClientImpl::SendUninstallPing(const std::string& id,
                      std::move(callback))));
 }
 
+void UpdateClientImpl::SendRegistrationPing(const std::string& id,
+                                            const base::Version& version,
+                                            Callback callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  RunTask(base::MakeRefCounted<TaskSendRegistrationPing>(
+      update_engine_.get(), id, version,
+      base::BindOnce(&UpdateClientImpl::OnTaskComplete, this,
+                     std::move(callback))));
+}
+
 scoped_refptr<UpdateClient> UpdateClientFactory(
     scoped_refptr<Configurator> config) {
   return base::MakeRefCounted<UpdateClientImpl>(

components/update_client/update_client.h

@@ -160,6 +160,7 @@ enum class ComponentState {
   kUpToDate,
   kUpdateError,
   kUninstalled,
+  kRegistration,
   kRun,
   kLastStatus
 };
@@ -436,6 +437,14 @@ class UpdateClient : public base::RefCountedThreadSafe<UpdateClient> {
                                  int reason,
                                  Callback callback) = 0;
 
+  // Sends a registration ping for the CRX identified by |id| and |version|.
+  // The current implementation of this function only sends a best-effort,
+  // fire-and-forget ping. It has no other side effects regarding installs or
+  // updates done through an instance of this class.
+  virtual void SendRegistrationPing(const std::string& id,
+                                    const base::Version& version,
+                                    Callback callback) = 0;
+
   // Returns status details about a CRX update. The function returns true in
   // case of success and false in case of errors, such as |id| was
   // invalid or not known.

components/update_client/update_client_internal.h

@@ -53,6 +53,9 @@ class UpdateClientImpl : public UpdateClient {
                          const base::Version& version,
                          int reason,
                          Callback callback) override;
+  void SendRegistrationPing(const std::string& id,
+                            const base::Version& version,
+                            Callback callback) override;
 
  private:
   ~UpdateClientImpl() override;

components/update_client/update_client_unittest.cc

@@ -3252,6 +3252,69 @@ TEST_F(UpdateClientTest, SendUninstallPing) {
   RunThreads();
 }
 
+TEST_F(UpdateClientTest, SendRegistrationPing) {
+  class CompletionCallbackMock {
+   public:
+    static void Callback(base::OnceClosure quit_closure, Error error) {
+      std::move(quit_closure).Run();
+    }
+  };
+
+  class MockUpdateChecker : public UpdateChecker {
+   public:
+    static std::unique_ptr<UpdateChecker> Create(
+        scoped_refptr<Configurator> config,
+        PersistedData* metadata) {
+      return std::make_unique<MockUpdateChecker>();
+    }
+
+    void CheckForUpdates(
+        const std::string& session_id,
+        const std::vector<std::string>& ids_to_check,
+        const IdToComponentPtrMap& components,
+        const base::flat_map<std::string, std::string>& additional_attributes,
+        bool enabled_component_updates,
+        UpdateCheckCallback update_check_callback) override {
+      NOTREACHED();
+    }
+  };
+
+  class MockCrxDownloader : public CrxDownloader {
+   public:
+    MockCrxDownloader() : CrxDownloader(nullptr) {}
+
+   private:
+    ~MockCrxDownloader() override = default;
+
+    void DoStartDownload(const GURL& url) override { EXPECT_TRUE(false); }
+  };
+
+  class MockPingManager : public MockPingManagerImpl {
+   public:
+    explicit MockPingManager(scoped_refptr<Configurator> config)
+        : MockPingManagerImpl(config) {}
+
+   protected:
+    ~MockPingManager() override {
+      const auto ping_data = MockPingManagerImpl::ping_data();
+      EXPECT_EQ(1u, ping_data.size());
+      EXPECT_EQ("jebgalgnebhfojomionfpkfelancnnkf", ping_data[0].id);
+      EXPECT_EQ(base::Version("1.2.3.4"), ping_data[0].next_version);
+    }
+  };
+
+  scoped_refptr<UpdateClient> update_client =
+      base::MakeRefCounted<UpdateClientImpl>(
+          config(), base::MakeRefCounted<MockPingManager>(config()),
+          &MockUpdateChecker::Create);
+
+  update_client->SendRegistrationPing(
+      "jebgalgnebhfojomionfpkfelancnnkf", base::Version("1.2.3.4"),
+      base::BindOnce(&CompletionCallbackMock::Callback, quit_closure()));
+
+  RunThreads();
+}
+
 TEST_F(UpdateClientTest, RetryAfter) {
   class DataCallbackMock {
    public:

components/update_client/update_engine.cc

@@ -430,4 +430,34 @@ void UpdateEngine::SendUninstallPing(const std::string& id,
       base::BindOnce(&UpdateEngine::HandleComponent, this, update_context));
 }
 
+void UpdateEngine::SendRegistrationPing(const std::string& id,
+                                        const base::Version& version,
+                                        Callback callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  const auto update_context = base::MakeRefCounted<UpdateContext>(
+      config_, false, std::vector<std::string>{id},
+      UpdateClient::CrxDataCallback(), UpdateClient::CrxStateChangeCallback(),
+      UpdateEngine::NotifyObserversCallback(), std::move(callback),
+      metadata_.get());
+  DCHECK(!update_context->session_id.empty());
+
+  const auto result = update_contexts_.insert(
+      std::make_pair(update_context->session_id, update_context));
+  DCHECK(result.second);
+
+  DCHECK(update_context);
+  DCHECK_EQ(1u, update_context->ids.size());
+  DCHECK_EQ(1u, update_context->components.count(id));
+  const auto& component = update_context->components.at(id);
+
+  component->Registration(version);
+
+  update_context->component_queue.push(id);
+
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE,
+      base::BindOnce(&UpdateEngine::HandleComponent, this, update_context));
+}
+
 }  // namespace update_client

components/update_client/update_engine.h

@@ -68,6 +68,10 @@ class UpdateEngine : public base::RefCountedThreadSafe<UpdateEngine> {
                          int reason,
                          Callback update_callback);
 
+  void SendRegistrationPing(const std::string& id,
+                            const base::Version& version,
+                            Callback update_callback);
+
  private:
   friend class base::RefCountedThreadSafe<UpdateEngine>;
   ~UpdateEngine();

extensions/browser/updater/update_service_unittest.cc

@@ -110,7 +110,9 @@ class FakeUpdateClient : public update_client::UpdateClient {
                          update_client::Callback callback) override {
     uninstall_pings_.emplace_back(id, version, reason);
   }
-
+  void SendRegistrationPing(const std::string& id,
+                            const base::Version& version,
+                            update_client::Callback Callback) override {}
   void FireEvent(Observer::Events event, const std::string& extension_id) {
     for (Observer* observer : observers_)
       observer->OnEvent(event, extension_id);