[CrOS Settings] Synchronous search in OsSettingsLocalizedStringsProvider

Update OsSettingsLocalizedStringsProvider to use the synchronous API
provided by LocalSearchService.

Bug: 1063505
Change-Id: I569215fd0149bd9912a82a66ba2c8d0eb509c4e1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2140960Reviewed-by: Jia Meng <jiameng@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757923}

chrome/browser/ui/webui/settings/chromeos/os_settings_localized_strings_provider.cc

@@ -50,7 +50,7 @@
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/grit/locale_settings.h"
-#include "chrome/services/local_search_service/public/mojom/types.mojom.h"
+#include "chrome/services/local_search_service/local_search_service_impl.h"
 #include "chromeos/constants/chromeos_features.h"
 #include "chromeos/constants/chromeos_switches.h"
 #include "chromeos/services/assistant/public/features.h"
@@ -78,9 +78,9 @@ namespace chromeos {
 namespace settings {
 namespace {
 
-std::vector<local_search_service::mojom::DataPtr> ConceptVectorToDataPtrVector(
+std::vector<local_search_service::Data> ConceptVectorToDataVector(
     const std::vector<SearchConcept>& tags_group) {
-  std::vector<local_search_service::mojom::DataPtr> data_list;
+  std::vector<local_search_service::Data> data_list;
 
   for (const auto& concept : tags_group) {
     std::vector<base::string16> search_tags;
@@ -99,8 +99,8 @@ std::vector<local_search_service::mojom::DataPtr> ConceptVectorToDataPtrVector(
 
     // Note: A stringified version of the canonical tag message ID is used as
     // the identifier for this search data.
-    data_list.push_back(local_search_service::mojom::Data::New(
-        base::NumberToString(concept.canonical_message_id), search_tags));
+    data_list.emplace_back(base::NumberToString(concept.canonical_message_id),
+                           search_tags);
   }
 
   return data_list;
@@ -2071,11 +2071,9 @@ void AddPageVisibilityStrings(content::WebUIDataSource* html_source) {
 
 OsSettingsLocalizedStringsProvider::OsSettingsLocalizedStringsProvider(
     Profile* profile,
-    local_search_service::mojom::LocalSearchService* local_search_service) {
-  local_search_service->GetIndex(
-      local_search_service::mojom::LocalSearchService::IndexId::CROS_SETTINGS,
-      index_remote_.BindNewPipeAndPassReceiver());
-
+    local_search_service::LocalSearchServiceImpl* local_search_service)
+    : index_(local_search_service->GetIndexImpl(
+          local_search_service::IndexId::kCrosSettings)) {
   // Add per-page string providers.
   // TODO(khorimoto): Add providers for the remaining pages.
   per_page_providers_.push_back(
@@ -2135,13 +2133,16 @@ OsSettingsLocalizedStringsProvider::GetCanonicalTagMetadata(
 }
 
 void OsSettingsLocalizedStringsProvider::Shutdown() {
-  index_remote_.reset();
+  index_ = nullptr;
 }
 
 void OsSettingsLocalizedStringsProvider::AddSearchTags(
     const std::vector<SearchConcept>& tags_group) {
-  index_remote_->AddOrUpdate(ConceptVectorToDataPtrVector(tags_group),
-                             /*callback=*/base::DoNothing());
+  // Note: Can be null after Shutdown().
+  if (!index_)
+    return;
+
+  index_->AddOrUpdate(ConceptVectorToDataVector(tags_group));
 
   // Add each concept to the map. Note that it is safe to take the address of
   // each concept because all concepts are allocated via static
@@ -2152,13 +2153,17 @@ void OsSettingsLocalizedStringsProvider::AddSearchTags(
 
 void OsSettingsLocalizedStringsProvider::RemoveSearchTags(
     const std::vector<SearchConcept>& tags_group) {
+  // Note: Can be null after Shutdown().
+  if (!index_)
+    return;
+
   std::vector<std::string> ids;
   for (const auto& concept : tags_group) {
     canonical_id_to_metadata_map_.erase(concept.canonical_message_id);
     ids.push_back(base::NumberToString(concept.canonical_message_id));
   }
 
-  index_remote_->Delete(ids, /*callback=*/base::DoNothing());
+  index_->Delete(ids);
 }
 
 }  // namespace settings

chrome/browser/ui/webui/settings/chromeos/os_settings_localized_strings_provider.h

@@ -10,7 +10,7 @@
 #include <vector>
 
 #include "chrome/browser/ui/webui/settings/chromeos/os_settings_per_page_strings_provider.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/remote.h"
 
@@ -20,6 +20,10 @@ namespace content {
 class WebUIDataSource;
 }  // namespace content
 
+namespace local_search_service {
+class LocalSearchServiceImpl;
+}  // namespace local_search_service
+
 namespace chromeos {
 namespace settings {
 
@@ -52,7 +56,7 @@ class OsSettingsLocalizedStringsProvider
  public:
   OsSettingsLocalizedStringsProvider(
       Profile* profile,
-      local_search_service::mojom::LocalSearchService* local_search_service);
+      local_search_service::LocalSearchServiceImpl* local_search_service);
   OsSettingsLocalizedStringsProvider(
       const OsSettingsLocalizedStringsProvider& other) = delete;
   OsSettingsLocalizedStringsProvider& operator=(
@@ -81,7 +85,7 @@ class OsSettingsLocalizedStringsProvider
 
   std::vector<std::unique_ptr<OsSettingsPerPageStringsProvider>>
       per_page_providers_;
-  mojo::Remote<local_search_service::mojom::Index> index_remote_;
+  local_search_service::IndexImpl* index_;
   std::unordered_map<int, const SearchConcept*> canonical_id_to_metadata_map_;
 };
 

chrome/browser/ui/webui/settings/chromeos/os_settings_localized_strings_provider_factory.cc

@@ -48,7 +48,7 @@ OsSettingsLocalizedStringsProviderFactory::BuildServiceInstanceFor(
       profile,
       local_search_service::LocalSearchServiceProxyFactory::GetForProfile(
           Profile::FromBrowserContext(profile))
-          ->GetLocalSearchService());
+          ->GetLocalSearchServiceImpl());
 }
 
 bool OsSettingsLocalizedStringsProviderFactory::ServiceIsNULLWhileTesting()

chrome/browser/ui/webui/settings/chromeos/os_settings_localized_strings_provider_unittest.cc

@@ -8,8 +8,8 @@
 #include "chrome/browser/ui/webui/settings/chromeos/search/search_concept.h"
 #include "chrome/common/webui_url_constants.h"
 #include "chrome/grit/generated_resources.h"
+#include "chrome/services/local_search_service/index_impl.h"
 #include "chrome/services/local_search_service/local_search_service_impl.h"
-#include "chrome/services/local_search_service/public/mojom/local_search_service.mojom-test-utils.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chrome/test/base/testing_profile_manager.h"
@@ -37,9 +37,8 @@ class OsSettingsLocalizedStringsProviderTest : public testing::Test {
         profile_manager_.CreateTestingProfile("TestingProfile"),
         &local_search_service_);
 
-    local_search_service_.GetIndex(
-        local_search_service::mojom::LocalSearchService::IndexId::CROS_SETTINGS,
-        index_remote_.BindNewPipeAndPassReceiver());
+    index_ = local_search_service_.GetIndexImpl(
+        local_search_service::IndexId::kCrosSettings);
 
     // Allow asynchronous networking code to complete (networking functionality
     // is tested below).
@@ -49,7 +48,7 @@ class OsSettingsLocalizedStringsProviderTest : public testing::Test {
   content::BrowserTaskEnvironment task_environment_;
   TestingProfileManager profile_manager_;
   chromeos::network_config::CrosNetworkConfigTestHelper network_config_helper_;
-  mojo::Remote<local_search_service::mojom::Index> index_remote_;
+  local_search_service::IndexImpl* index_;
   local_search_service::LocalSearchServiceImpl local_search_service_;
   std::unique_ptr<OsSettingsLocalizedStringsProvider> provider_;
 };
@@ -58,9 +57,7 @@ class OsSettingsLocalizedStringsProviderTest : public testing::Test {
 // verifies that when the provider starts up, it adds *some* strings without
 // checking the exact number. It also checks one specific canonical tag.
 TEST_F(OsSettingsLocalizedStringsProviderTest, WifiTags) {
-  uint64_t initial_num_items = 0;
-  local_search_service::mojom::IndexAsyncWaiter(index_remote_.get())
-      .GetSize(&initial_num_items);
+  uint64_t initial_num_items = index_->GetSize();
   EXPECT_GT(initial_num_items, 0u);
 
   const SearchConcept* network_settings_concept =
@@ -82,9 +79,7 @@ TEST_F(OsSettingsLocalizedStringsProviderTest, WifiTags) {
       "/device/stub_eth_device", shill::kTypeEthernet, "stub_eth_device");
   base::RunLoop().RunUntilIdle();
 
-  uint64_t num_items_after_adding_ethernet = 0;
-  local_search_service::mojom::IndexAsyncWaiter(index_remote_.get())
-      .GetSize(&num_items_after_adding_ethernet);
+  uint64_t num_items_after_adding_ethernet = index_->GetSize();
   EXPECT_GT(num_items_after_adding_ethernet, initial_num_items);
 
   ethernet_settings_concept =

chrome/services/local_search_service/index_impl.cc

@@ -73,6 +73,9 @@ bool CompareResults(const local_search_service::Result& r1,
 
 }  // namespace
 
+local_search_service::Data::Data(const std::string& id,
+                                 const std::vector<base::string16>& search_tags)
+    : id(id), search_tags(search_tags) {}
 local_search_service::Data::Data() = default;
 local_search_service::Data::Data(const Data& data) = default;
 local_search_service::Data::~Data() = default;

chrome/services/local_search_service/index_impl.h

@@ -34,6 +34,8 @@ struct Data {
 
   // Data item will be matched between its search tags and query term.
   std::vector<base::string16> search_tags;
+
+  Data(const std::string& id, const std::vector<base::string16>& search_tags);
   Data();
   Data(const Data& data);
   ~Data();