[M103 merge] libassistant: Not load lib if DLC not installed

If the installation of DLC lib failed, do not load the library.

Bug: b:232852357
Test: Manual

(cherry picked from commit 906e146)

Change-Id: I616b103a12e5ace2ca5d8b41be3187881b2f3e05
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3645156
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Commit-Queue: Tao Wu <wutao@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1004617}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3656333
Cr-Commit-Position: refs/branch-heads/5060@{#142}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}

chromeos/services/assistant/assistant_manager_service_impl.cc

@@ -46,6 +46,7 @@
 #include "chromeos/services/libassistant/public/mojom/speech_recognition_observer.mojom.h"
 #include "chromeos/strings/grit/chromeos_strings.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
 #include "ui/accessibility/mojom/ax_assistant_structure.mojom.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "url/gurl.h"
@@ -251,11 +252,11 @@ void AssistantManagerServiceImpl::Start(const absl::optional<UserInfo>& user,
   // rootfs if installabtion failed. No error handling needed.
   auto* client = chromeos::DlcserviceClient::Get();
   if (!client) {
-    InitAssistant(user, /*dlc_path=*/std::string());
+    InitAssistant(user);
     return;
   }
 
-  DVLOG(1) << "Install libassistant.so from DLC";
+  DVLOG(1) << "Installing libassistant.so from DLC";
   dlcservice::InstallRequest install_request;
   install_request.set_id(kLibassistantDlcId);
   client->Install(
@@ -470,18 +471,20 @@ void AssistantManagerServiceImpl::OnStateChanged(
 void AssistantManagerServiceImpl::OnInstallDlcComplete(
     const absl::optional<UserInfo>& user,
     const chromeos::DlcserviceClient::InstallResult& result) {
-  DVLOG(1) << "Installed libassistant.so from DLC";
-  std::string dlc_path;
+  absl::optional<std::string> dlc_path;
   if (result.error == dlcservice::kErrorNone) {
+    DVLOG(3) << "Installed libassistant.so from DLC";
     dlc_path = result.root_path;
+  } else {
+    DVLOG(1) << "Failed to install libassistant.so from DLC: " << result.error;
   }
 
   InitAssistant(user, dlc_path);
 }
 
 void AssistantManagerServiceImpl::InitAssistant(
     const absl::optional<UserInfo>& user,
-    const std::string& dlc_path) {
+    const absl::optional<std::string>& dlc_path) {
   DCHECK(!IsServiceStarted());
 
   auto bootup_config = bootup_config_.Clone();

chromeos/services/assistant/assistant_manager_service_impl.h

@@ -194,9 +194,10 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl
       const absl::optional<UserInfo>& user,
       const chromeos::DlcserviceClient::InstallResult& result);
 
-  // `dlc_path` is where the DLC libassistant.so mounted.
-  void InitAssistant(const absl::optional<UserInfo>& user,
-                     const std::string& dlc_path);
+  // Optional `dlc_path`, where the DLC libassistant.so is mounted.
+  void InitAssistant(
+      const absl::optional<UserInfo>& user,
+      const absl::optional<std::string>& dlc_path = absl::nullopt);
   void OnServiceStarted();
   void OnServiceRunning();
   bool IsServiceStarted() const;

chromeos/services/libassistant/libassistant_factory.h

@@ -8,6 +8,8 @@
 #include <memory>
 #include <string>
 
+#include "third_party/abseil-cpp/absl/types/optional.h"
+
 namespace assistant_client {
 class AssistantManager;
 class AssistantManagerInternal;
@@ -29,7 +31,8 @@ class LibassistantFactory {
   UnwrapAssistantManagerInternal(
       assistant_client::AssistantManager* assistant_manager) = 0;
 
-  virtual void LoadLibassistantLibraryFromDlc(const std::string& root_path) = 0;
+  virtual void LoadLibassistantLibraryFromDlc(
+      const absl::optional<std::string>& root_path) = 0;
 };
 
 }  // namespace libassistant

chromeos/services/libassistant/libassistant_service.cc

@@ -40,7 +40,7 @@ class LibassistantFactoryImpl : public LibassistantFactory {
   // LibassistantFactory implementation:
   std::unique_ptr<assistant_client::AssistantManager> CreateAssistantManager(
       const std::string& lib_assistant_config) override {
-    if (!dlc_library_.is_valid()) {
+    if (!IsDlcLibraryValid()) {
       return base::WrapUnique(assistant_client::AssistantManager::Create(
           platform_api_, lib_assistant_config));
     }
@@ -55,7 +55,7 @@ class LibassistantFactoryImpl : public LibassistantFactory {
 
   assistant_client::AssistantManagerInternal* UnwrapAssistantManagerInternal(
       assistant_client::AssistantManager* assistant_manager) override {
-    if (!dlc_library_.is_valid()) {
+    if (!IsDlcLibraryValid()) {
       return assistant_client::UnwrapAssistantManagerInternal(
           assistant_manager);
     }
@@ -68,14 +68,21 @@ class LibassistantFactoryImpl : public LibassistantFactory {
     return assistant_manager_internal;
   }
 
-  void LoadLibassistantLibraryFromDlc(const std::string& dlc_path) override {
-    base::FilePath path = GetLibassisantPath(dlc_path);
+  void LoadLibassistantLibraryFromDlc(
+      const absl::optional<std::string>& dlc_path) override {
+    if (!dlc_path.has_value()) {
+      DVLOG(3) << "libassistant DLC is not mounted.";
+      dlc_library_.reset();
+      return;
+    }
+
+    base::FilePath path = GetLibassisantPath(dlc_path.value());
     dlc_library_ = base::ScopedNativeLibrary(path);
-    if (!dlc_library_.is_valid()) {
-      LOG(ERROR) << "Failed to load libassistant shared library from: " << path
-                 << ", error: " << dlc_library_.GetError()->ToString();
+    if (IsDlcLibraryValid()) {
+      DVLOG(3) << "Loaded libassistant shared library from: " << path;
     } else {
-      DVLOG(1) << "Loaded libassistant shared library from: " << path;
+      DVLOG(1) << "Failed to load libassistant shared library from: " << path
+               << ", error: " << dlc_library_.GetError()->ToString();
     }
   }
 
@@ -93,6 +100,8 @@ class LibassistantFactoryImpl : public LibassistantFactory {
         c_entrypoint);
   }
 
+  bool IsDlcLibraryValid() const { return dlc_library_.is_valid(); }
+
   assistant_client::PlatformApi* const platform_api_;
   base::ScopedNativeLibrary dlc_library_;
 };

chromeos/services/libassistant/public/mojom/service_controller.mojom

@@ -66,7 +66,6 @@ struct BootupConfig {
   bool dark_mode_enabled;
   bool hotword_enabled;
 
-  // The root path where the DLC library mounted.
-  // Empty if library is not mounted.
-  string dlc_path;
+  // Optional root path where the DLC library mounted.
+  string? dlc_path;
 };

chromeos/services/libassistant/test_support/fake_libassistant_factory.cc

@@ -44,7 +44,7 @@ FakeLibassistantFactory::UnwrapAssistantManagerInternal(
 }
 
 void FakeLibassistantFactory::LoadLibassistantLibraryFromDlc(
-    const std::string& root_path) {}
+    const absl::optional<std::string>& root_path) {}
 
 assistant::FakeAssistantManager& FakeLibassistantFactory::assistant_manager() {
   if (current_assistant_manager_)

chromeos/services/libassistant/test_support/fake_libassistant_factory.h

@@ -34,7 +34,8 @@ class FakeLibassistantFactory : public LibassistantFactory {
       const std::string& libassistant_config) override;
   assistant_client::AssistantManagerInternal* UnwrapAssistantManagerInternal(
       assistant_client::AssistantManager* assistant_manager) override;
-  void LoadLibassistantLibraryFromDlc(const std::string& root_path) override;
+  void LoadLibassistantLibraryFromDlc(
+      const absl::optional<std::string>& root_path) override;
 
   std::string libassistant_config() const { return libassistant_config_; }