[CrOS Settings] Add synchronous support for SearchHandler

(1) Change internal implementation to be synchronous, now that
    LocalSearchService exposes a synchronous API.
(2) Add a synchronous public API for SearchHandler in anticipation for
    use by in-process clients (e.g., upcoming Launcher usage).

Bug: 1063505, 1059099
Change-Id: If1110a4f43158ca17b387d766dac58cc87ba08f3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2130681
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#755197}

chrome/browser/ui/BUILD.gn

@@ -2073,7 +2073,7 @@ jumbo_static_library("ui") {
       "//chrome/browser/ui/webui/chromeos/machine_learning:mojo_bindings",
       "//chrome/browser/ui/webui/settings/chromeos/search:mojo_bindings",
       "//chrome/services/file_util/public/cpp",
-      "//chrome/services/local_search_service/public/mojom",
+      "//chrome/services/local_search_service",
       "//chromeos",
       "//chromeos/assistant:buildflags",
       "//chromeos/audio",

chrome/browser/ui/webui/settings/chromeos/search/search_handler.cc

@@ -8,6 +8,7 @@
 #include "chrome/browser/ui/webui/settings/chromeos/os_settings_localized_strings_provider.h"
 #include "chrome/browser/ui/webui/settings/chromeos/search/search_concept.h"
 #include "chrome/browser/ui/webui/settings/chromeos/search/search_result_icon.mojom.h"
+#include "chrome/services/local_search_service/local_search_service_impl.h"
 #include "chrome/services/local_search_service/public/mojom/types.mojom.h"
 #include "ui/base/l10n/l10n_util.h"
 
@@ -15,7 +16,6 @@ namespace chromeos {
 namespace settings {
 namespace {
 
-const int32_t kLocalSearchServiceMaxLatencyMs = 3000;
 const int32_t kLocalSearchServiceMaxResults = 10;
 
 }  // namespace
@@ -25,13 +25,10 @@ const size_t SearchHandler::kNumMaxResults = 5;
 
 SearchHandler::SearchHandler(
     OsSettingsLocalizedStringsProvider* strings_provider,
-    local_search_service::mojom::LocalSearchService* local_search_service)
-    : strings_provider_(strings_provider) {
-  DCHECK(strings_provider_);
-  local_search_service->GetIndex(
-      local_search_service::mojom::LocalSearchService::IndexId::CROS_SETTINGS,
-      index_remote_.BindNewPipeAndPassReceiver());
-}
+    local_search_service::LocalSearchServiceImpl* local_search_service)
+    : strings_provider_(strings_provider),
+      index_(local_search_service->GetIndexImpl(
+          local_search_service::IndexId::kCrosSettings)) {}
 
 SearchHandler::~SearchHandler() = default;
 
@@ -40,43 +37,33 @@ void SearchHandler::BindInterface(
   receivers_.Add(this, std::move(pending_receiver));
 }
 
-void SearchHandler::Search(const base::string16& query,
-                           SearchCallback callback) {
-  index_remote_->Find(
-      query, kLocalSearchServiceMaxLatencyMs, kLocalSearchServiceMaxResults,
-      base::BindOnce(&SearchHandler::OnLocalSearchServiceResults,
-                     base::Unretained(this), std::move(callback)));
-}
-
-void SearchHandler::OnLocalSearchServiceResults(
-    SearchCallback callback,
-    local_search_service::mojom::ResponseStatus response_status,
-    base::Optional<std::vector<local_search_service::mojom::ResultPtr>>
-        results) {
-  switch (response_status) {
-    case local_search_service::mojom::ResponseStatus::UNKNOWN_ERROR:
-    case local_search_service::mojom::ResponseStatus::EMPTY_QUERY:
-    case local_search_service::mojom::ResponseStatus::EMPTY_INDEX:
-      LOG(ERROR) << "Cannot search; LocalSearchService returned "
-                 << response_status << ". Returning empty results array.";
-      std::move(callback).Run({});
-      return;
-    case local_search_service::mojom::ResponseStatus::SUCCESS:
-      DCHECK(results);
-      ReturnSuccessfulResults(std::move(callback), std::move(results));
-      return;
+std::vector<mojom::SearchResultPtr> SearchHandler::Search(
+    const base::string16& query) {
+  std::vector<local_search_service::Result> local_search_service_results;
+  local_search_service::ResponseStatus response_status = index_->Find(
+      query, kLocalSearchServiceMaxResults, &local_search_service_results);
+
+  if (response_status != local_search_service::ResponseStatus::kSuccess) {
+    LOG(ERROR) << "Cannot search; LocalSearchService returned "
+               << static_cast<int>(response_status)
+               << ". Returning empty results array.";
+    return {};
   }
 
-  NOTREACHED() << "Invalid response status: " << response_status << ".";
+  return GenerateSearchResultsArray(local_search_service_results);
+}
+
+void SearchHandler::Search(const base::string16& query,
+                           SearchCallback callback) {
+  std::move(callback).Run(Search(query));
 }
 
-void SearchHandler::ReturnSuccessfulResults(
-    SearchCallback callback,
-    base::Optional<std::vector<local_search_service::mojom::ResultPtr>>
-        results) {
+std::vector<mojom::SearchResultPtr> SearchHandler::GenerateSearchResultsArray(
+    const std::vector<local_search_service::Result>&
+        local_search_service_results) {
   std::vector<mojom::SearchResultPtr> search_results;
-  for (const auto& result : *results) {
-    mojom::SearchResultPtr result_ptr = ResultToSearchResult(*result);
+  for (const auto& result : local_search_service_results) {
+    mojom::SearchResultPtr result_ptr = ResultToSearchResult(result);
     if (result_ptr)
       search_results.push_back(std::move(result_ptr));
 
@@ -85,11 +72,11 @@ void SearchHandler::ReturnSuccessfulResults(
       break;
   }
 
-  std::move(callback).Run(std::move(search_results));
+  return search_results;
 }
 
 mojom::SearchResultPtr SearchHandler::ResultToSearchResult(
-    const local_search_service::mojom::Result& result) {
+    const local_search_service::Result& result) {
   int message_id;
 
   // The result's ID is expected to be a stringified int.
@@ -112,8 +99,8 @@ mojom::SearchResultPtr SearchHandler::ResultToSearchResult(
 
 void SearchHandler::Shutdown() {
   strings_provider_ = nullptr;
+  index_ = nullptr;
   receivers_.Clear();
-  index_remote_.reset();
 }
 
 }  // namespace settings

chrome/browser/ui/webui/settings/chromeos/search/search_handler.h

@@ -9,12 +9,16 @@
 
 #include "base/optional.h"
 #include "chrome/browser/ui/webui/settings/chromeos/search/search.mojom.h"
-#include "chrome/services/local_search_service/public/mojom/local_search_service.mojom.h"
+#include "chrome/services/local_search_service/index_impl.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
 #include "mojo/public/cpp/bindings/remote.h"
 
+namespace local_search_service {
+class LocalSearchServiceImpl;
+}  // namespace local_search_service
+
 namespace chromeos {
 namespace settings {
 
@@ -35,7 +39,7 @@ class SearchHandler : public mojom::SearchHandler, public KeyedService {
 
   SearchHandler(
       OsSettingsLocalizedStringsProvider* strings_provider,
-      local_search_service::mojom::LocalSearchService* local_search_service);
+      local_search_service::LocalSearchServiceImpl* local_search_service);
   ~SearchHandler() override;
 
   SearchHandler(const SearchHandler& other) = delete;
@@ -44,6 +48,9 @@ class SearchHandler : public mojom::SearchHandler, public KeyedService {
   void BindInterface(
       mojo::PendingReceiver<mojom::SearchHandler> pending_receiver);
 
+  // Synchronous search implementation (for in-process clients).
+  std::vector<mojom::SearchResultPtr> Search(const base::string16& query);
+
   // mojom::SearchHandler:
   void Search(const base::string16& query, SearchCallback callback) override;
 
@@ -51,22 +58,17 @@ class SearchHandler : public mojom::SearchHandler, public KeyedService {
   // KeyedService:
   void Shutdown() override;
 
-  void OnLocalSearchServiceResults(
-      SearchCallback callback,
-      local_search_service::mojom::ResponseStatus response_status,
-      base::Optional<std::vector<local_search_service::mojom::ResultPtr>>
-          results);
-  void ReturnSuccessfulResults(
-      SearchCallback callback,
-      base::Optional<std::vector<local_search_service::mojom::ResultPtr>>
-          results);
+  std::vector<mojom::SearchResultPtr> GenerateSearchResultsArray(
+      const std::vector<local_search_service::Result>&
+          local_search_service_results);
   mojom::SearchResultPtr ResultToSearchResult(
-      const local_search_service::mojom::Result& result);
+      const local_search_service::Result& result);
 
   OsSettingsLocalizedStringsProvider* strings_provider_;
+  local_search_service::IndexImpl* index_;
+
   // Note: Expected to have multiple clients, so a ReceiverSet is used.
   mojo::ReceiverSet<mojom::SearchHandler> receivers_;
-  mojo::Remote<local_search_service::mojom::Index> index_remote_;
 };
 
 }  // namespace settings

chrome/browser/ui/webui/settings/chromeos/search/search_handler_factory.cc

@@ -45,7 +45,7 @@ KeyedService* SearchHandlerFactory::BuildServiceInstanceFor(
       OsSettingsLocalizedStringsProviderFactory::GetForProfile(profile),
       local_search_service::LocalSearchServiceProxyFactory::GetForProfile(
           Profile::FromBrowserContext(profile))
-          ->GetLocalSearchService());
+          ->GetLocalSearchServiceImpl());
 }
 
 bool SearchHandlerFactory::ServiceIsNULLWhileTesting() const {