dlcservice: Expose DLC progress with Observers

For users of DlcserviceClient, the possibliy to observe the
download/install of a DLC is added.

Bug: b:143098235
Test: autoninja -C out/Default chromeos_unittests && ./out/Default/chromeos_unittests
Change-Id: I846f05664e1ff90626ac79af4dcc4e3dbfd7ce23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1874815
Commit-Queue: Jae Hoon Kim <kimjae@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708828}

chromeos/dbus/dlcservice/dlcservice_client.cc

@@ -105,6 +105,16 @@ class DlcserviceClientImpl : public DlcserviceClient {
 
   ~DlcserviceClientImpl() override = default;
 
+  void AddObserver(Observer* obs) override { observers_.AddObserver(obs); }
+
+  void RemoveObserver(Observer* obs) override {
+    observers_.RemoveObserver(obs);
+  }
+
+  void NotifyProgressUpdateForTest(double progress) override {
+    NotifyProgressUpdate(progress);
+  }
+
   void Install(const dlcservice::DlcModuleList& dlc_module_list,
                InstallCallback callback) override {
     if (!service_available_) {
@@ -190,6 +200,12 @@ class DlcserviceClientImpl : public DlcserviceClient {
   }
 
  private:
+  // Notify to observers the progress of DLC(s) download/install.
+  void NotifyProgressUpdate(double progress) {
+    for (Observer& obs : observers_)
+      obs.OnProgressUpdate(progress);
+  }
+
   void OnInstallStatus(dbus::Signal* signal) {
     if (!install_callback_holder_.has_value())
       return;
@@ -212,11 +228,18 @@ class DlcserviceClientImpl : public DlcserviceClient {
         VLOG(1) << "DLC(s) install successful.";
         SendSignal();
         break;
-      case dlcservice::Status::RUNNING:
-        VLOG(2) << "Install in progress: " << install_status.progress();
+      case dlcservice::Status::RUNNING: {
+        const double progress = install_status.progress();
+        VLOG(2) << "Install in progress: " << progress;
+        // TODO(kimjae): Currently, update_engine delegate's two actions.
+        // Specifically the Download action and Post Install Runner action,
+        // which means that progress will go from 0 -> 1 two complete cycles.
+        // This can be easily handled either here or inside dlcservice daemon.
+        NotifyProgressUpdate(progress);
         // Need to return here since we don't want to try starting another
         // pending install from the queue (would waste time checking).
         return;
+      }
       case dlcservice::Status::FAILED:
         LOG(ERROR) << "Failed to install with error code: "
                    << install_status.error_code();
@@ -308,6 +331,9 @@ class DlcserviceClientImpl : public DlcserviceClient {
   // available.
   std::vector<base::OnceClosure> pending_tasks_;
 
+  // The list holding observers that want to listen in on certain events.
+  base::ObserverList<Observer> observers_;
+
   // Note: This should remain the last member so it'll be destroyed and
   // invalidate its weak pointers before any other members are destroyed.
   base::WeakPtrFactory<DlcserviceClientImpl> weak_ptr_factory_{this};

chromeos/dbus/dlcservice/dlcservice_client.h

@@ -48,6 +48,19 @@ class COMPONENT_EXPORT(DLCSERVICE_CLIENT) DlcserviceClient {
       const std::string& err,
       const dlcservice::DlcModuleList& dlc_module_list)>;
 
+  class Observer : public base::CheckedObserver {
+   public:
+    // Observers should not dictate the finalization of install based purely on
+    // what's notified, but based on the callback passed to |Install()|.
+    virtual void OnProgressUpdate(double progress) = 0;
+  };
+
+  virtual void AddObserver(Observer* obs) = 0;
+
+  virtual void RemoveObserver(Observer* obs) = 0;
+
+  virtual void NotifyProgressUpdateForTest(double progress) = 0;
+
   virtual void Install(const dlcservice::DlcModuleList& dlc_module_list,
                        InstallCallback callback) = 0;
 

chromeos/dbus/dlcservice/dlcservice_client_unittest.cc

@@ -4,8 +4,12 @@
 
 #include "chromeos/dbus/dlcservice/dlcservice_client.h"
 
+#include <algorithm>
+#include <atomic>
+#include <functional>
 #include <memory>
 #include <utility>
+#include <vector>
 
 #include "base/bind.h"
 #include "base/run_loop.h"
@@ -23,6 +27,22 @@ using ::testing::WithArg;
 
 namespace chromeos {
 
+namespace {
+
+class TestObserver : public DlcserviceClient::Observer {
+ public:
+  using FuncType = base::RepeatingCallback<void()>;
+  TestObserver(FuncType action) : action_(action) {}
+  void OnProgressUpdate(double progress) override { action_.Run(); }
+
+ private:
+  FuncType action_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestObserver);
+};
+
+}  // namespace
+
 class DlcserviceClientTest : public testing::Test {
  public:
   void SetUp() override {
@@ -36,18 +56,18 @@ class DlcserviceClientTest : public testing::Test {
         mock_bus_.get(), dlcservice::kDlcServiceServiceName,
         dbus::ObjectPath(dlcservice::kDlcServiceServicePath));
 
-    EXPECT_CALL(
+    ON_CALL(
         *mock_bus_.get(),
         GetObjectProxy(dlcservice::kDlcServiceServiceName,
                        dbus::ObjectPath(dlcservice::kDlcServiceServicePath)))
-        .WillOnce(Return(mock_proxy_.get()));
+        .WillByDefault(Return(mock_proxy_.get()));
 
-    EXPECT_CALL(*mock_proxy_.get(),
-                DoConnectToSignal(dlcservice::kDlcServiceInterface, _, _, _))
-        .WillOnce(Invoke(this, &DlcserviceClientTest::ConnectToSignal));
+    ON_CALL(*mock_proxy_.get(),
+            DoConnectToSignal(dlcservice::kDlcServiceInterface, _, _, _))
+        .WillByDefault(Invoke(this, &DlcserviceClientTest::ConnectToSignal));
 
-    EXPECT_CALL(*mock_proxy_.get(), DoCallMethodWithErrorResponse(_, _, _))
-        .WillOnce(
+    ON_CALL(*mock_proxy_.get(), DoCallMethodWithErrorResponse(_, _, _))
+        .WillByDefault(
             Invoke(this, &DlcserviceClientTest::CallMethodWithErrorResponse));
 
     DlcserviceClient::Initialize(mock_bus_.get());
@@ -159,4 +179,30 @@ TEST_F(DlcserviceClientTest, InstallFailureTest) {
   base::RunLoop().RunUntilIdle();
 }
 
+TEST_F(DlcserviceClientTest, ObserverTest) {
+  std::atomic<size_t> counter{0};
+  TestObserver::FuncType action = base::BindRepeating(
+      [](decltype(counter)* counter) { ++*counter; }, &counter);
+  constexpr size_t kNumObs = 100;
+  std::vector<std::unique_ptr<TestObserver>> obss;
+  obss.reserve(kNumObs);
+  std::generate_n(std::back_inserter(obss), kNumObs,
+                  [action] { return std::make_unique<TestObserver>(action); });
+
+  // Phase 0: Check initial value.
+  EXPECT_EQ(0u, counter.load());
+
+  // Phase 1: Add observers and check updated value.
+  for (auto& obs : obss)
+    client_->AddObserver(obs.get());
+  client_->NotifyProgressUpdateForTest(0.);
+  EXPECT_EQ(kNumObs, counter.load());
+
+  // Phase 2: Remove observers and check unchanged value.
+  for (auto& obs : obss)
+    client_->RemoveObserver(obs.get());
+  client_->NotifyProgressUpdateForTest(0.);
+  EXPECT_EQ(kNumObs, counter.load());
+}
+
 }  // namespace chromeos

chromeos/dbus/dlcservice/fake_dlcservice_client.cc

@@ -13,6 +13,18 @@ FakeDlcserviceClient::FakeDlcserviceClient() = default;
 
 FakeDlcserviceClient::~FakeDlcserviceClient() = default;
 
+void FakeDlcserviceClient::AddObserver(Observer* obs) {
+  VLOG(1) << "Requesting to add observer.";
+}
+
+void FakeDlcserviceClient::RemoveObserver(Observer* obs) {
+  VLOG(1) << "Requesting to remove observer.";
+}
+
+void FakeDlcserviceClient::NotifyProgressUpdateForTest(double progress) {
+  NOTREACHED();
+}
+
 void FakeDlcserviceClient::Install(
     const dlcservice::DlcModuleList& dlc_module_list,
     InstallCallback callback) {

chromeos/dbus/dlcservice/fake_dlcservice_client.h

@@ -24,6 +24,9 @@ class COMPONENT_EXPORT(DLCSERVICE_CLIENT) FakeDlcserviceClient
   ~FakeDlcserviceClient() override;
 
   // DlcserviceClient:
+  void AddObserver(Observer* obs) override;
+  void RemoveObserver(Observer* obs) override;
+  void NotifyProgressUpdateForTest(double progress) override;
   void Install(const dlcservice::DlcModuleList& dlc_module_list,
                InstallCallback callback) override;
   void Uninstall(const std::string& dlc_id,