Revert "[cros search service] Remove mojom and async API"

This reverts commit 0fbb4553.

Reason for revert: Broke the chromeos-amd64-generic-rel build: https://ci.chromium.org/p/chromium/builders/ci/chromeos-amd64-generic-rel/41896?

Original change's description:
> [cros search service] Remove mojom and async API
> 
> This cl removes all mojo and async APIs without moving files or
> changing file names (which will be done in later cls).
> 
> Bug: 1064424,1018613
> Change-Id: Ie92967aa37723e31557487aeb96bc9eed3458b39
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2151376
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Colin Blundell <blundell@chromium.org>
> Commit-Queue: Jia Meng <jiameng@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#759848}

TBR=dcheng@chromium.org,blundell@chromium.org,jiameng@chromium.org

Change-Id: Ica10760e53a1a95a0e25cd4b8c6370043e0adec1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1064424, 1018613
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2153172Reviewed-by: Meredith Lane <meredithl@chromium.org>
Commit-Queue: Meredith Lane <meredithl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759877}

chrome/browser/local_search_service/DEPS

@@ -7,8 +7,6 @@ specific_include_rules = {
     "+chrome/services/local_search_service/local_search_service_impl.h",
   ],
   "local_search_service_proxy_unittest.cc": [
-    "+chrome/services/local_search_service/index_impl.h",
-    "+chrome/services/local_search_service/local_search_service_impl.h",
     "+chrome/services/local_search_service/test_utils.h",
   ],
 }

chrome/browser/local_search_service/local_search_service_proxy.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/local_search_service/local_search_service_proxy.h"
 
+#include "base/bind.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/services/local_search_service/local_search_service_impl.h"
 
@@ -13,11 +14,27 @@ LocalSearchServiceProxy::LocalSearchServiceProxy(Profile* profile) {}
 
 LocalSearchServiceProxy::~LocalSearchServiceProxy() = default;
 
+mojom::LocalSearchService* LocalSearchServiceProxy::GetLocalSearchService() {
+  if (!local_search_service_impl_) {
+    CreateLocalSearchServiceAndBind();
+  }
+  return remote_.get();
+}
+
 LocalSearchServiceImpl* LocalSearchServiceProxy::GetLocalSearchServiceImpl() {
   if (!local_search_service_impl_) {
-    local_search_service_impl_ = std::make_unique<LocalSearchServiceImpl>();
+    // Need to bind |remote_| even if a client asks for the implementation
+    // directly.
+    CreateLocalSearchServiceAndBind();
   }
   return local_search_service_impl_.get();
 }
 
+void LocalSearchServiceProxy::CreateLocalSearchServiceAndBind() {
+  DCHECK(!local_search_service_impl_);
+  local_search_service_impl_ = std::make_unique<LocalSearchServiceImpl>();
+  local_search_service_impl_->BindReceiver(
+      remote_.BindNewPipeAndPassReceiver());
+}
+
 }  // namespace local_search_service

chrome/browser/local_search_service/local_search_service_proxy.h

@@ -9,7 +9,11 @@
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "chrome/services/local_search_service/public/mojom/local_search_service.mojom.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"
 
 class Profile;
 
@@ -17,8 +21,10 @@ namespace local_search_service {
 
 class LocalSearchServiceImpl;
 
-// TODO(jiameng): the next cl will remove this class completely because the
-// factory will return LocalSearchService (that will be a KeyedService).
+// This class owns an implementation of LocalSearchService.
+// It exposes LocalSearchService through the mojo interface by returning a
+// remote. However, in-process clients can request implementation ptr directly.
+// TODO(jiameng): the next cl will remove mojo and will provide impl directly.
 class LocalSearchServiceProxy : public KeyedService {
  public:
   // Profile isn't required, hence can be nullptr in tests.
@@ -28,10 +34,19 @@ class LocalSearchServiceProxy : public KeyedService {
   LocalSearchServiceProxy(const LocalSearchServiceProxy&) = delete;
   LocalSearchServiceProxy& operator=(const LocalSearchServiceProxy&) = delete;
 
+  // Clients should call this function to get a remote to LocalSearchService.
+  // This function returns to the caller a pointer to |remote_|, which is bound
+  // to |local_search_service_impl_|.
+  mojom::LocalSearchService* GetLocalSearchService();
+
+  // For in-process clients, it could be more efficient to get the
+  // implementation ptr directly.
   LocalSearchServiceImpl* GetLocalSearchServiceImpl();
 
  private:
+  void CreateLocalSearchServiceAndBind();
   std::unique_ptr<LocalSearchServiceImpl> local_search_service_impl_;
+  mojo::Remote<mojom::LocalSearchService> remote_;
 
   base::WeakPtrFactory<LocalSearchServiceProxy> weak_ptr_factory_{this};
 };

chrome/browser/local_search_service/local_search_service_proxy_unittest.cc

@@ -7,35 +7,41 @@
 #include <utility>
 #include <vector>
 
+#include "base/callback.h"
+#include "base/test/task_environment.h"
 #include "chrome/browser/local_search_service/local_search_service_proxy.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/types.mojom.h"
 #include "chrome/services/local_search_service/test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace local_search_service {
 
-class LocalSearchServiceProxyTest : public testing::Test {};
+class LocalSearchServiceProxyTest : public testing::Test {
+ protected:
+  base::test::SingleThreadTaskEnvironment task_environment_;
+};
 
 TEST_F(LocalSearchServiceProxyTest, Basic) {
   LocalSearchServiceProxy service_proxy(nullptr);
-  LocalSearchServiceImpl* const service =
-      service_proxy.GetLocalSearchServiceImpl();
-  DCHECK(service);
+  mojom::LocalSearchService* const service_remote =
+      service_proxy.GetLocalSearchService();
+  ASSERT_TRUE(service_remote);
 
-  IndexImpl* const index = service->GetIndexImpl(IndexId::kCrosSettings);
-  DCHECK(index);
+  mojo::Remote<mojom::Index> index_remote;
+  service_remote->GetIndex(mojom::LocalSearchService::IndexId::CROS_SETTINGS,
+                           index_remote.BindNewPipeAndPassReceiver());
+  CHECK(index_remote.is_connected());
 
-  EXPECT_EQ(index->GetSize(), 0u);
+  GetSizeAndCheck(index_remote.get(), 0u);
 
   // Register the following data to the search index, the map is id to
   // search-tags.
   const std::map<std::string, std::vector<std::string>> data_to_register = {
       {"id1", {"tag1a", "tag1b"}}, {"id2", {"tag2a", "tag2b"}}};
-  std::vector<Data> data = CreateTestData(data_to_register);
+  std::vector<mojom::DataPtr> data = CreateTestData(data_to_register);
   EXPECT_EQ(data.size(), 2u);
-  index->AddOrUpdate(data);
-  EXPECT_EQ(index->GetSize(), 2u);
+  AddOrUpdateAndCheck(index_remote.get(), std::move(data));
+  GetSizeAndCheck(index_remote.get(), 2u);
 }
 
 }  // namespace local_search_service

chrome/services/local_search_service/BUILD.gn

@@ -10,7 +10,11 @@ source_set("local_search_service") {
     "local_search_service_impl.h",
   ]
 
-  public_deps = [ "//base" ]
+  public_deps = [
+    "//base",
+    "//chrome/services/local_search_service/public/mojom",
+    "//mojo/public/cpp/bindings",
+  ]
 }
 
 source_set("test_utils") {
@@ -23,6 +27,8 @@ source_set("test_utils") {
 
   public_deps = [
     "//base",
+    "//chrome/services/local_search_service/public/mojom",
+    "//mojo/public/cpp/bindings",
     "//testing/gtest",
   ]
 }

chrome/services/local_search_service/OWNERS

 jiameng@chromium.org
 tby@chromium.org
 thanhdng@chromium.org
-wrong@chromium.org

chrome/services/local_search_service/index_impl.cc

@@ -6,6 +6,7 @@
 
 #include <utility>
 
+#include "base/bind.h"
 #include "base/optional.h"
 #include "base/strings/string_util.h"
 #include "chrome/common/string_matching/fuzzy_tokenized_string_match.h"
@@ -86,10 +87,36 @@ IndexImpl::IndexImpl() = default;
 
 IndexImpl::~IndexImpl() = default;
 
+void IndexImpl::BindReceiver(mojo::PendingReceiver<mojom::Index> receiver) {
+  receivers_.Add(this, std::move(receiver));
+}
+
+void IndexImpl::GetSize(GetSizeCallback callback) {
+  const uint64_t size = GetSize();
+  std::move(callback).Run(size);
+}
+
 uint64_t IndexImpl::GetSize() {
   return data_.size();
 }
 
+void IndexImpl::AddOrUpdate(std::vector<mojom::DataPtr> data,
+                            AddOrUpdateCallback callback) {
+  std::vector<local_search_service::Data> data_in;
+  for (const auto& d : data) {
+    if (d->id.empty())
+      receivers_.ReportBadMessage("Empty ID in updated data");
+
+    local_search_service::Data d_in;
+    d_in.id = d->id;
+    d_in.search_tags = d->search_tags;
+    data_in.push_back(d_in);
+  }
+
+  AddOrUpdate(data_in);
+  std::move(callback).Run();
+}
+
 void IndexImpl::AddOrUpdate(
     const std::vector<local_search_service::Data>& data) {
   for (const auto& item : data) {
@@ -102,6 +129,16 @@ void IndexImpl::AddOrUpdate(
   }
 }
 
+void IndexImpl::Delete(const std::vector<std::string>& ids,
+                       DeleteCallback callback) {
+  for (const auto& id : ids) {
+    if (id.empty())
+      receivers_.ReportBadMessage("Empty ID in deleted data");
+  }
+  const uint32_t num_deleted = Delete(ids);
+  std::move(callback).Run(num_deleted);
+}
+
 uint32_t IndexImpl::Delete(const std::vector<std::string>& ids) {
   uint32_t num_deleted = 0u;
   for (const auto& id : ids) {
@@ -117,6 +154,53 @@ uint32_t IndexImpl::Delete(const std::vector<std::string>& ids) {
   return num_deleted;
 }
 
+void IndexImpl::Find(const base::string16& query,
+                     int32_t max_latency_in_ms,
+                     int32_t max_results,
+                     FindCallback callback) {
+  std::vector<local_search_service::Result> results;
+  // TODO(jiameng): |max_latency| isn't supported yet. We're
+  // temporarily ignoring it before the next cl removes the async call.
+  const auto response =
+      Find(query, max_results < 0 ? 0u : max_results, &results);
+
+  mojom::ResponseStatus mresponse = mojom::ResponseStatus::UNKNOWN_ERROR;
+  switch (response) {
+    case local_search_service::ResponseStatus::kEmptyQuery:
+      mresponse = mojom::ResponseStatus::EMPTY_QUERY;
+      break;
+    case local_search_service::ResponseStatus::kEmptyIndex:
+      mresponse = mojom::ResponseStatus::EMPTY_INDEX;
+      break;
+    case local_search_service::ResponseStatus::kSuccess:
+      mresponse = mojom::ResponseStatus::SUCCESS;
+      break;
+    default:
+      break;
+  }
+
+  if (mresponse != mojom::ResponseStatus::SUCCESS) {
+    std::move(callback).Run(mresponse, base::nullopt);
+    return;
+  }
+
+  std::vector<mojom::ResultPtr> mresults;
+  for (const auto& r : results) {
+    mojom::ResultPtr mr = mojom::Result::New();
+    mr->id = r.id;
+    mr->score = r.score;
+    std::vector<mojom::RangePtr> mhits;
+    for (const auto& hit : r.hits) {
+      mojom::RangePtr range = mojom::Range::New(hit.start, hit.end);
+      mhits.push_back(std::move(range));
+    }
+    mr->hits = std::move(mhits);
+    mresults.push_back(std::move(mr));
+  }
+
+  std::move(callback).Run(mojom::ResponseStatus::SUCCESS, std::move(mresults));
+}
+
 local_search_service::ResponseStatus IndexImpl::Find(
     const base::string16& query,
     uint32_t max_results,
@@ -134,6 +218,19 @@ local_search_service::ResponseStatus IndexImpl::Find(
   return local_search_service::ResponseStatus::kSuccess;
 }
 
+void IndexImpl::SetSearchParams(mojom::SearchParamsPtr search_params,
+                                SetSearchParamsCallback callback) {
+  local_search_service::SearchParams search_params_in;
+  search_params_in.relevance_threshold = search_params->relevance_threshold;
+  search_params_in.partial_match_penalty_rate =
+      search_params->partial_match_penalty_rate;
+  search_params_in.use_prefix_only = search_params->use_prefix_only;
+  search_params_in.use_weighted_ratio = search_params->use_weighted_ratio;
+  search_params_in.use_edit_distance = search_params->use_edit_distance;
+  SetSearchParams(search_params_in);
+  std::move(callback).Run();
+}
+
 void IndexImpl::SetSearchParams(
     const local_search_service::SearchParams& search_params) {
   search_params_ = search_params;

chrome/services/local_search_service/index_impl.h

@@ -11,8 +11,14 @@
 #include <vector>
 
 #include "base/macros.h"
-#include "base/memory/weak_ptr.h"
 #include "base/strings/string16.h"
+#include "chrome/services/local_search_service/public/mojom/local_search_service.mojom.h"
+#include "chrome/services/local_search_service/public/mojom/types.mojom.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
+#include "mojo/public/cpp/bindings/receiver_set.h"
+#include "mojo/public/cpp/bindings/remote.h"
+#include "mojo/public/cpp/bindings/remote_set.h"
 
 class TokenizedString;
 
@@ -77,36 +83,47 @@ enum class ResponseStatus {
   kSuccess = 3
 };
 
-// A local search service Index.
-// It has a registry of searchable data, which can be updated. It also runs a
-// synchronous search function to find matching items for a given query.
-class IndexImpl {
+// Actual implementation of a local search service Index.
+// It has a registry of searchable data, which can be updated. It also runs an
+// asynchronous search function to find matching items for a given query, and
+// returns results via a callback.
+// In-process clients can choose to call synchronous versions of these
+// functions.
+// TODO(jiameng): all async calls will be deleted in the next cl.
+class IndexImpl : public mojom::Index {
  public:
   IndexImpl();
-  ~IndexImpl();
+  ~IndexImpl() override;
 
-  IndexImpl(const IndexImpl&) = delete;
-  IndexImpl& operator=(const IndexImpl&) = delete;
+  void BindReceiver(mojo::PendingReceiver<mojom::Index> receiver);
 
-  // Returns number of data items.
+  // mojom::Index overrides.
+  // Also included the synchronous versions for in-process clients.
+  void GetSize(GetSizeCallback callback) override;
   uint64_t GetSize();
 
-  // Adds or updates data.
   // IDs of data should not be empty.
+  void AddOrUpdate(std::vector<mojom::DataPtr> data,
+                   AddOrUpdateCallback callback) override;
   void AddOrUpdate(const std::vector<local_search_service::Data>& data);
 
-  // Deletes data with |ids| and returns number of items deleted.
-  // If an id doesn't exist in the Index, no operation will be done.
   // IDs should not be empty.
+  void Delete(const std::vector<std::string>& ids,
+              DeleteCallback callback) override;
   uint32_t Delete(const std::vector<std::string>& ids);
 
-  // Returns matching results for a given query.
+  void Find(const base::string16& query,
+            int32_t max_latency_in_ms,
+            int32_t max_results,
+            FindCallback callback) override;
   // Zero |max_results| means no max.
   local_search_service::ResponseStatus Find(
       const base::string16& query,
       uint32_t max_results,
       std::vector<local_search_service::Result>* results);
 
+  void SetSearchParams(mojom::SearchParamsPtr search_params,
+                       SetSearchParamsCallback callback) override;
   void SetSearchParams(const local_search_service::SearchParams& search_params);
 
   void GetSearchParamsForTesting(double* relevance_threshold,
@@ -124,10 +141,12 @@ class IndexImpl {
   // A map from key to tokenized search-tags.
   std::map<std::string, std::vector<std::unique_ptr<TokenizedString>>> data_;
 
+  mojo::ReceiverSet<mojom::Index> receivers_;
+
   // Search parameters.
   local_search_service::SearchParams search_params_;
 
-  base::WeakPtrFactory<IndexImpl> weak_ptr_factory_{this};
+  DISALLOW_COPY_AND_ASSIGN(IndexImpl);
 };
 
 }  // namespace local_search_service

chrome/services/local_search_service/index_impl_unittest.cc

@@ -9,7 +9,14 @@
 #include <utility>
 #include <vector>
 
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/optional.h"
+#include "base/run_loop.h"
+#include "base/test/task_environment.h"
 #include "chrome/services/local_search_service/index_impl.h"
+#include "chrome/services/local_search_service/public/mojom/local_search_service.mojom-test-utils.h"
+#include "chrome/services/local_search_service/public/mojom/types.mojom.h"
 #include "chrome/services/local_search_service/test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -23,18 +30,31 @@ constexpr bool kDefaultUsePrefixOnly = false;
 constexpr bool kDefaultUseWeightedRatio = true;
 constexpr bool kDefaultUseEditDistance = false;
 
+void SetSearchParamsAndCheck(mojom::Index* index,
+                             mojom::SearchParamsPtr search_params) {
+  DCHECK(index);
+  mojom::IndexAsyncWaiter(index).SetSearchParams(std::move(search_params));
+}
+
 }  // namespace
 
 class IndexImplTest : public testing::Test {
+ public:
+  IndexImplTest() {
+    index_impl_.BindReceiver(index_remote_.BindNewPipeAndPassReceiver());
+  }
+
  protected:
+  base::test::SingleThreadTaskEnvironment task_environment_;
   IndexImpl index_impl_;
+  mojo::Remote<mojom::Index> index_remote_;
 };
 
 TEST_F(IndexImplTest, SetSearchParams) {
   {
     // No params are specified so default values are used.
-    const SearchParams search_params;
-    index_impl_.SetSearchParams(search_params);
+    mojom::SearchParamsPtr search_params = mojom::SearchParams::New();
+    SetSearchParamsAndCheck(index_remote_.get(), std::move(search_params));
 
     // Initialize them to values different from default.
     double relevance_threshold = kDefaultRelevanceThreshold * 2.0;
@@ -57,12 +77,12 @@ TEST_F(IndexImplTest, SetSearchParams) {
 
   {
     // Params are specified and are used.
-    const SearchParams search_params = {
+    mojom::SearchParamsPtr search_params = mojom::SearchParams::New(
         kDefaultRelevanceThreshold / 2, kDefaultPartialMatchPenaltyRate / 2,
         !kDefaultUsePrefixOnly, !kDefaultUseWeightedRatio,
-        !kDefaultUseEditDistance};
+        !kDefaultUseEditDistance);
 
-    index_impl_.SetSearchParams(search_params);
+    SetSearchParamsAndCheck(index_remote_.get(), std::move(search_params));
 
     // Initialize them to default values.
     double relevance_threshold = kDefaultRelevanceThreshold;
@@ -87,49 +107,51 @@ TEST_F(IndexImplTest, SetSearchParams) {
 TEST_F(IndexImplTest, RelevanceThreshold) {
   const std::map<std::string, std::vector<std::string>> data_to_register = {
       {"id1", {"Clash Of Clan"}}, {"id2", {"famous"}}};
-  std::vector<Data> data = CreateTestData(data_to_register);
-  index_impl_.AddOrUpdate(data);
-  EXPECT_EQ(index_impl_.GetSize(), 2u);
+  std::vector<mojom::DataPtr> data = CreateTestData(data_to_register);
+  AddOrUpdateAndCheck(index_remote_.get(), std::move(data));
+  GetSizeAndCheck(index_remote_.get(), 2u);
   {
-    SearchParams search_params;
-    search_params.relevance_threshold = 0.0;
-    index_impl_.SetSearchParams(search_params);
+    mojom::SearchParamsPtr search_params = mojom::SearchParams::New();
+    search_params->relevance_threshold = 0.0;
+    SetSearchParamsAndCheck(index_remote_.get(), std::move(search_params));
 
-    FindAndCheck(&index_impl_, "CC",
-                 /*max_results=*/-1, ResponseStatus::kSuccess, {"id1", "id2"});
+    FindAndCheck(index_remote_.get(), "CC", /*max_latency_in_ms=*/-1,
+                 /*max_results=*/-1, mojom::ResponseStatus::SUCCESS,
+                 {"id1", "id2"});
   }
   {
-    SearchParams search_params;
-    search_params.relevance_threshold = 0.3;
-    index_impl_.SetSearchParams(search_params);
+    mojom::SearchParamsPtr search_params = mojom::SearchParams::New();
+    search_params->relevance_threshold = 0.3;
+    SetSearchParamsAndCheck(index_remote_.get(), std::move(search_params));
 
-    FindAndCheck(&index_impl_, "CC",
-                 /*max_results=*/-1, ResponseStatus::kSuccess, {"id1"});
+    FindAndCheck(index_remote_.get(), "CC", /*max_latency_in_ms=*/-1,
+                 /*max_results=*/-1, mojom::ResponseStatus::SUCCESS, {"id1"});
   }
   {
-    SearchParams search_params;
-    search_params.relevance_threshold = 0.9;
-    index_impl_.SetSearchParams(search_params);
+    mojom::SearchParamsPtr search_params = mojom::SearchParams::New();
+    search_params->relevance_threshold = 0.9;
+    SetSearchParamsAndCheck(index_remote_.get(), std::move(search_params));
 
-    FindAndCheck(&index_impl_, "CC",
-                 /*max_results=*/-1, ResponseStatus::kSuccess, {});
+    FindAndCheck(index_remote_.get(), "CC", /*max_latency_in_ms=*/-1,
+                 /*max_results=*/-1, mojom::ResponseStatus::SUCCESS, {});
   }
 }
 
 TEST_F(IndexImplTest, MaxResults) {
   const std::map<std::string, std::vector<std::string>> data_to_register = {
       {"id1", {"Clash Of Clan"}}, {"id2", {"famous"}}};
-  std::vector<Data> data = CreateTestData(data_to_register);
-  index_impl_.AddOrUpdate(data);
-  EXPECT_EQ(index_impl_.GetSize(), 2u);
-  SearchParams search_params;
-  search_params.relevance_threshold = 0.0;
-  index_impl_.SetSearchParams(search_params);
-
-  FindAndCheck(&index_impl_, "CC",
-               /*max_results=*/-1, ResponseStatus::kSuccess, {"id1", "id2"});
-  FindAndCheck(&index_impl_, "CC",
-               /*max_results=*/1, ResponseStatus::kSuccess, {"id1"});
+  std::vector<mojom::DataPtr> data = CreateTestData(data_to_register);
+  AddOrUpdateAndCheck(index_remote_.get(), std::move(data));
+  GetSizeAndCheck(index_remote_.get(), 2u);
+  mojom::SearchParamsPtr search_params = mojom::SearchParams::New();
+  search_params->relevance_threshold = 0.0;
+  SetSearchParamsAndCheck(index_remote_.get(), std::move(search_params));
+
+  FindAndCheck(index_remote_.get(), "CC", /*max_latency_in_ms=*/-1,
+               /*max_results=*/-1, mojom::ResponseStatus::SUCCESS,
+               {"id1", "id2"});
+  FindAndCheck(index_remote_.get(), "CC", /*max_latency_in_ms=*/-1,
+               /*max_results=*/1, mojom::ResponseStatus::SUCCESS, {"id1"});
 }
 
 }  // namespace local_search_service

chrome/services/local_search_service/local_search_service_impl.cc

@@ -6,7 +6,9 @@
 
 #include <utility>
 
+#include "base/bind.h"
 #include "chrome/services/local_search_service/index_impl.h"
+#include "chrome/services/local_search_service/public/mojom/types.mojom.h"
 
 namespace local_search_service {
 
@@ -14,8 +16,28 @@ LocalSearchServiceImpl::LocalSearchServiceImpl() = default;
 
 LocalSearchServiceImpl::~LocalSearchServiceImpl() = default;
 
+void LocalSearchServiceImpl::BindReceiver(
+    mojo::PendingReceiver<mojom::LocalSearchService> receiver) {
+  receivers_.Add(this, std::move(receiver));
+}
+
+void LocalSearchServiceImpl::GetIndex(
+    mojom::LocalSearchService::IndexId index_id,
+    mojo::PendingReceiver<mojom::Index> index) {
+  if (index_id != mojom::LocalSearchService::IndexId::CROS_SETTINGS)
+    return;
+
+  auto* it = IndexLookupOrCreate(local_search_service::IndexId::kCrosSettings);
+  it->BindReceiver(std::move(index));
+}
+
 IndexImpl* LocalSearchServiceImpl::GetIndexImpl(
     local_search_service::IndexId index_id) {
+  return IndexLookupOrCreate(index_id);
+}
+
+IndexImpl* LocalSearchServiceImpl::IndexLookupOrCreate(
+    local_search_service::IndexId index_id) {
   auto it = indices_.find(index_id);
   if (it == indices_.end())
     it = indices_.emplace(index_id, std::make_unique<IndexImpl>()).first;

chrome/services/local_search_service/local_search_service_impl.h

@@ -9,6 +9,12 @@
 #include <memory>
 
 #include "base/macros.h"
+#include "chrome/services/local_search_service/public/mojom/local_search_service.mojom.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
+#include "mojo/public/cpp/bindings/receiver_set.h"
+#include "mojo/public/cpp/bindings/remote.h"
+#include "mojo/public/cpp/bindings/remote_set.h"
 
 namespace local_search_service {
 
@@ -16,19 +22,30 @@ class IndexImpl;
 
 enum class IndexId { kCrosSettings = 0 };
 
-// LocalSearchServiceImpl creates and owns content-specific Indices. Clients can
-// call it |GetIndexImpl| method to get an Index for a given index id.
-class LocalSearchServiceImpl {
+// Actual implementation of LocalSearchService.
+// It creates and owns content-specific Indices. Clients can call it |GetIndex|
+// method to get an Index for a given index id.
+// In-process clients can call |GetIndexImpl| directly.
+class LocalSearchServiceImpl : public mojom::LocalSearchService {
  public:
   LocalSearchServiceImpl();
-  ~LocalSearchServiceImpl();
-  LocalSearchServiceImpl(const LocalSearchServiceImpl&) = delete;
-  LocalSearchServiceImpl& operator=(const LocalSearchServiceImpl&) = delete;
+  ~LocalSearchServiceImpl() override;
 
+  void BindReceiver(mojo::PendingReceiver<mojom::LocalSearchService> receiver);
+
+  // mojom::LocalSearchService overrides.
+  void GetIndex(mojom::LocalSearchService::IndexId index_id,
+                mojo::PendingReceiver<mojom::Index> index) override;
+
+  // Only to be used by in-process clients.
   IndexImpl* GetIndexImpl(local_search_service::IndexId index_id);
 
  private:
+  IndexImpl* IndexLookupOrCreate(local_search_service::IndexId index_id);
+  mojo::ReceiverSet<mojom::LocalSearchService> receivers_;
   std::map<local_search_service::IndexId, std::unique_ptr<IndexImpl>> indices_;
+
+  DISALLOW_COPY_AND_ASSIGN(LocalSearchServiceImpl);
 };
 
 }  // namespace local_search_service

chrome/services/local_search_service/local_search_service_impl_unittest.cc

@@ -9,98 +9,119 @@
 #include <utility>
 #include <vector>
 
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/optional.h"
+#include "base/run_loop.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/task_environment.h"
 #include "chrome/services/local_search_service/local_search_service_impl.h"
+#include "chrome/services/local_search_service/public/mojom/types.mojom.h"
 #include "chrome/services/local_search_service/test_utils.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
+#include "mojo/public/cpp/bindings/receiver_set.h"
+#include "mojo/public/cpp/bindings/remote.h"
+#include "mojo/public/cpp/bindings/remote_set.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace local_search_service {
 
 class LocalSearchServiceImplTest : public testing::Test {
+ public:
+  LocalSearchServiceImplTest() {
+    service_impl_.BindReceiver(service_remote_.BindNewPipeAndPassReceiver());
+  }
+
  protected:
+  base::test::SingleThreadTaskEnvironment task_environment_;
   LocalSearchServiceImpl service_impl_;
+  mojo::Remote<mojom::LocalSearchService> service_remote_;
 };
 
 // Tests a query that results in an exact match. We do not aim to test the
 // algorithm used in the search, but exact match should always be returned.
 TEST_F(LocalSearchServiceImplTest, ResultFound) {
-  IndexImpl* const index = service_impl_.GetIndexImpl(IndexId::kCrosSettings);
-  DCHECK(index);
+  mojo::Remote<mojom::Index> index_remote;
+  service_remote_->GetIndex(mojom::LocalSearchService::IndexId::CROS_SETTINGS,
+                            index_remote.BindNewPipeAndPassReceiver());
 
-  EXPECT_EQ(index->GetSize(), 0u);
+  GetSizeAndCheck(index_remote.get(), 0u);
 
   // Register the following data to the search index, the map is id to
   // search-tags.
   const std::map<std::string, std::vector<std::string>> data_to_register = {
       {"id1", {"id1", "tag1a", "tag1b"}}, {"xyz", {"xyz"}}};
-  std::vector<Data> data = CreateTestData(data_to_register);
+  std::vector<mojom::DataPtr> data = CreateTestData(data_to_register);
   EXPECT_EQ(data.size(), 2u);
 
-  index->AddOrUpdate(data);
-  EXPECT_EQ(index->GetSize(), 2u);
+  AddOrUpdateAndCheck(index_remote.get(), std::move(data));
+  GetSizeAndCheck(index_remote.get(), 2u);
 
   // Find result with query "id1". It returns an exact match.
-  FindAndCheck(index, "id1",
-               /*max_results=*/-1, ResponseStatus::kSuccess, {"id1"});
+  FindAndCheck(index_remote.get(), "id1", /*max_latency_in_ms=*/-1,
+               /*max_results=*/-1, mojom::ResponseStatus::SUCCESS, {"id1"});
 }
 
 // Tests a query that results in no match. We do not aim to test the algorithm
 // used in the search, but a query too different from the item should have no
 // result returned.
 TEST_F(LocalSearchServiceImplTest, ResultNotFound) {
-  IndexImpl* const index = service_impl_.GetIndexImpl(IndexId::kCrosSettings);
-  DCHECK(index);
+  mojo::Remote<mojom::Index> index_remote;
+  service_remote_->GetIndex(mojom::LocalSearchService::IndexId::CROS_SETTINGS,
+                            index_remote.BindNewPipeAndPassReceiver());
 
-  EXPECT_EQ(index->GetSize(), 0u);
+  GetSizeAndCheck(index_remote.get(), 0u);
 
   // Register the following data to the search index, the map is id to
   // search-tags.
   const std::map<std::string, std::vector<std::string>> data_to_register = {
       {"id1", {"id1", "tag1a", "tag1b"}}, {"id2", {"id2", "tag2a", "tag2b"}}};
-  std::vector<Data> data = CreateTestData(data_to_register);
+  std::vector<mojom::DataPtr> data = CreateTestData(data_to_register);
   EXPECT_EQ(data.size(), 2u);
 
-  index->AddOrUpdate(data);
-  EXPECT_EQ(index->GetSize(), 2u);
+  AddOrUpdateAndCheck(index_remote.get(), std::move(data));
+  GetSizeAndCheck(index_remote.get(), 2u);
 
   // Find result with query "xyz". It returns no match.
-  FindAndCheck(index, "xyz",
-               /*max_results=*/-1, ResponseStatus::kSuccess, {});
+  FindAndCheck(index_remote.get(), "xyz", /*max_latency_in_ms=*/-1,
+               /*max_results=*/-1, mojom::ResponseStatus::SUCCESS, {});
 }
 
 TEST_F(LocalSearchServiceImplTest, UpdateData) {
-  IndexImpl* const index = service_impl_.GetIndexImpl(IndexId::kCrosSettings);
-  DCHECK(index);
+  mojo::Remote<mojom::Index> index_remote;
+  service_remote_->GetIndex(mojom::LocalSearchService::IndexId::CROS_SETTINGS,
+                            index_remote.BindNewPipeAndPassReceiver());
 
-  EXPECT_EQ(index->GetSize(), 0u);
+  GetSizeAndCheck(index_remote.get(), 0u);
 
   // Register the following data to the search index, the map is id to
   // search-tags.
   const std::map<std::string, std::vector<std::string>> data_to_register = {
       {"id1", {"tag1a", "tag1b"}}, {"id2", {"tag2a", "tag2b"}}};
-  std::vector<Data> data = CreateTestData(data_to_register);
+  std::vector<mojom::DataPtr> data = CreateTestData(data_to_register);
   EXPECT_EQ(data.size(), 2u);
 
-  index->AddOrUpdate(data);
-  EXPECT_EQ(index->GetSize(), 2u);
+  AddOrUpdateAndCheck(index_remote.get(), std::move(data));
+  GetSizeAndCheck(index_remote.get(), 2u);
 
   // Delete "id1" and "id10" from the index. Since "id10" doesn't exist, only
   // one item is deleted.
-  EXPECT_EQ(index->Delete({"id1", "id10"}), 1u);
-  EXPECT_EQ(index->GetSize(), 1u);
+  DeleteAndCheck(index_remote.get(), {"id1", "id10"}, 1u);
+  GetSizeAndCheck(index_remote.get(), 1u);
 
   // Add "id3" to the index.
-  Data data_id3("id3",
-                std::vector<base::string16>({base::UTF8ToUTF16("tag3a")}));
-  std::vector<Data> data_to_update;
-  data_to_update.push_back(data_id3);
-  index->AddOrUpdate(data_to_update);
-  EXPECT_EQ(index->GetSize(), 2u);
-
-  FindAndCheck(index, "id3",
-               /*max_results=*/-1, ResponseStatus::kSuccess, {"id3"});
-  FindAndCheck(index, "id1",
-               /*max_results=*/-1, ResponseStatus::kSuccess, {});
+  mojom::DataPtr data_id3 = mojom::Data::New(
+      "id3", std::vector<base::string16>({base::UTF8ToUTF16("tag3a")}));
+  std::vector<mojom::DataPtr> data_to_update;
+  data_to_update.push_back(std::move(data_id3));
+  AddOrUpdateAndCheck(index_remote.get(), std::move(data_to_update));
+  GetSizeAndCheck(index_remote.get(), 2u);
+
+  FindAndCheck(index_remote.get(), "id3", /*max_latency_in_ms=*/-1,
+               /*max_results=*/-1, mojom::ResponseStatus::SUCCESS, {"id3"});
+  FindAndCheck(index_remote.get(), "id1", /*max_latency_in_ms=*/-1,
+               /*max_results=*/-1, mojom::ResponseStatus::SUCCESS, {});
 }
 }  // namespace local_search_service

chrome/services/local_search_service/public/mojom/BUILD.gn

+# Copyright 2019 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.
+
+import("//mojo/public/tools/bindings/mojom.gni")
+
+mojom("mojom") {
+  sources = [
+    "local_search_service.mojom",
+    "types.mojom",
+  ]
+
+  public_deps = [ "//mojo/public/mojom/base" ]
+}

chrome/services/local_search_service/public/mojom/OWNERS

+per-file *.mojom=set noparent
+per-file *.mojom=file://ipc/SECURITY_OWNERS

chrome/services/local_search_service/public/mojom/local_search_service.mojom

+// Copyright 2019 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.
+
+module local_search_service.mojom;
+
+import "chrome/services/local_search_service/public/mojom/types.mojom";
+import "mojo/public/mojom/base/string16.mojom";
+
+// LocalSearchService will run as a singleton service so that clients
+// can request Indices from it. LocalSearchService will be responsible for
+// creating Indices, but data update will be the responsibility of
+// clients. Runs in the browser process as a KeyedService.
+interface LocalSearchService {
+  // Identifies a globally unique Index of related search data.
+  enum IndexId {
+    CROS_SETTINGS = 0
+  };
+
+  // A client can call this function to get the Index for |index_id|. If
+  // the Index isn't created when this function is called, LocalSearchService
+  // will create one.
+  // Note, there should be one primary client that is the owner of the
+  // data and can read/write the data to the Index. The other clients
+  // should only use the Index for query search.
+  GetIndex(IndexId index_id, pending_receiver<Index> index);
+};
+
+// An Index can serve multiple clients, but only one client (the primary
+// client) that owns the data should be allowed to modify the Index.
+interface Index {
+  // Returns number of data items.
+  GetSize() => (uint64 num_items);
+
+  // Adds or updates data and callbacks upon completion.
+  // Only the primary client should be allowed to do this operation.
+  AddOrUpdate(array<Data> data) => ();
+
+  // Deletes data with |ids| and returns number of items deleted.
+  // If an id doesn't exist in the Index, no operation will be done.
+  // Only the primary client should be allowed to do this operation.
+  Delete(array<string> ids) => (uint32 num_deleted);
+
+  // Takes an asynchronous search request call and returns results and status
+  // code via a callback.
+  // |max_latency_in_ms| = -1 means there is no maximum latency.
+  Find(mojo_base.mojom.String16 query, int32 max_latency_in_ms,
+       int32 max_results) => (ResponseStatus status, array<Result>? results);
+
+  // Sets search parameters. Clients should not need to set the parameters
+  // explicitly. They can use default values provided by the Index. However,
+  // this will be useful for experimental purposes.
+  SetSearchParams(SearchParams search_params) => ();
+};

chrome/services/local_search_service/public/mojom/types.mojom

+// Copyright 2019 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.
+
+module local_search_service.mojom;
+
+import "mojo/public/mojom/base/values.mojom";
+import "mojo/public/mojom/base/string16.mojom";
+
+// A numeric range used to represent the start and end position.
+struct Range {
+  uint32 start;
+  uint32 end;
+};
+
+// Every Data is one item to be added to an Index. It contains an id, search
+// tags that will be used for matching against a query.
+struct Data {
+  // Identifier of the data item, should be unique across the registry. Clients
+  // will decide what ids to use, they could be paths, urls or any opaque string
+  // identifiers.
+  // Ideally IDs should persist across sessions, but this is not strictly
+  // required now because data is not persisted across sessions.
+  string id;
+
+  // Data item will be matched between its search tags and query term.
+  array<mojo_base.mojom.String16> search_tags;
+};
+
+// Result is one item that matches a given query. It contains the id of the item
+// and its matching score.
+struct Result {
+  // Id of the data.
+  string id;
+  // Relevance score, in the range of [0,1].
+  double score;
+  // Matching ranges.
+  array<Range> hits;
+};
+
+// Status of the search attempt.
+// More will be added later.
+enum ResponseStatus {
+  UNKNOWN_ERROR,
+  // Query is empty.
+  EMPTY_QUERY,
+  // Index is empty (i.e. no data).
+  EMPTY_INDEX,
+  // Search operation is successful. But there could be no matching item and
+  // result list is empty.
+  SUCCESS
+};
+
+struct SearchParams {
+  double relevance_threshold = 0.3;
+  double partial_match_penalty_rate = 0.9;
+  bool use_prefix_only = false;
+  bool use_weighted_ratio = true;
+  bool use_edit_distance = false;
+};

chrome/services/local_search_service/test_utils.cc

@@ -6,6 +6,7 @@
 
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/services/local_search_service/public/mojom/local_search_service.mojom-test-utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace local_search_service {
@@ -23,37 +24,63 @@ std::vector<base::string16> MultiUTF8ToUTF16(
 
 }  // namespace
 
-std::vector<Data> CreateTestData(
+std::vector<mojom::DataPtr> CreateTestData(
     const std::map<std::string, std::vector<std::string>>& input) {
-  std::vector<Data> output;
+  std::vector<mojom::DataPtr> output;
   for (const auto& item : input) {
     const std::vector<base::string16> tags = MultiUTF8ToUTF16(item.second);
-    const Data data(item.first, tags);
-    output.push_back(data);
+    mojom::DataPtr data = mojom::Data::New(item.first, tags);
+    output.push_back(std::move(data));
   }
   return output;
 }
 
-void FindAndCheck(IndexImpl* index,
+void GetSizeAndCheck(mojom::Index* index, uint64_t expected_num_items) {
+  DCHECK(index);
+  uint64_t num_items = 0;
+  mojom::IndexAsyncWaiter(index).GetSize(&num_items);
+  EXPECT_EQ(num_items, expected_num_items);
+}
+
+void AddOrUpdateAndCheck(mojom::Index* index,
+                         std::vector<mojom::DataPtr> data) {
+  DCHECK(index);
+  mojom::IndexAsyncWaiter(index).AddOrUpdate(std::move(data));
+}
+
+void DeleteAndCheck(mojom::Index* index,
+                    const std::vector<std::string>& ids,
+                    uint32_t expected_num_deleted) {
+  DCHECK(index);
+  uint32_t num_deleted = 0u;
+  mojom::IndexAsyncWaiter(index).Delete(ids, &num_deleted);
+  EXPECT_EQ(num_deleted, expected_num_deleted);
+}
+
+void FindAndCheck(mojom::Index* index,
                   std::string query,
+                  int32_t max_latency_in_ms,
                   int32_t max_results,
-                  ResponseStatus expected_status,
+                  mojom::ResponseStatus expected_status,
                   const std::vector<std::string>& expected_result_ids) {
   DCHECK(index);
 
-  std::vector<Result> results;
-  auto status = index->Find(base::UTF8ToUTF16(query), max_results, &results);
+  mojom::IndexAsyncWaiter async_waiter(index);
+  mojom::ResponseStatus status = mojom::ResponseStatus::UNKNOWN_ERROR;
+  base::Optional<std::vector<::local_search_service::mojom::ResultPtr>> results;
+  async_waiter.Find(base::UTF8ToUTF16(query), max_latency_in_ms, max_results,
+                    &status, &results);
 
   EXPECT_EQ(status, expected_status);
 
-  if (!results.empty()) {
+  if (results) {
     // If results are returned, check size and values match the expected.
-    EXPECT_EQ(results.size(), expected_result_ids.size());
-    for (size_t i = 0; i < results.size(); ++i) {
-      EXPECT_EQ(results[i].id, expected_result_ids[i]);
+    EXPECT_EQ(results->size(), expected_result_ids.size());
+    for (size_t i = 0; i < results->size(); ++i) {
+      EXPECT_EQ((*results)[i]->id, expected_result_ids[i]);
       // Scores should be non-increasing.
-      if (i < results.size() - 1) {
-        EXPECT_GE(results[i].score, results[i + 1].score);
+      if (i < results->size() - 1) {
+        EXPECT_GE((*results)[i]->score, (*results)[i + 1]->score);
       }
     }
     return;

chrome/services/local_search_service/test_utils.h

@@ -10,21 +10,36 @@
 #include <utility>
 #include <vector>
 
-#include "chrome/services/local_search_service/index_impl.h"
+#include "chrome/services/local_search_service/public/mojom/local_search_service.mojom-test-utils.h"
+#include "chrome/services/local_search_service/public/mojom/types.mojom.h"
 
 namespace local_search_service {
 
 // Creates test data to be registered to the index. |input| is a map from
 // id to search-tags.
-std::vector<Data> CreateTestData(
+std::vector<mojom::DataPtr> CreateTestData(
     const std::map<std::string, std::vector<std::string>>& input);
 
+// The following helper functions call the async functions and check results.
+// Gets the number of items in |index| and verifies it is |expected_num_items|.
+void GetSizeAndCheck(mojom::Index* index, uint64_t expected_num_items);
+
+// Adds items from |data| to |index|.
+void AddOrUpdateAndCheck(mojom::Index* index, std::vector<mojom::DataPtr> data);
+
+// Deletes items with |ids| from |index| and verifies number deleted is
+// |expected_num_deleted|.
+void DeleteAndCheck(mojom::Index* index,
+                    const std::vector<std::string>& ids,
+                    uint32_t expected_num_deleted);
+
 // Finds item for |query| from |index| and checks their ids are those in
 // |expected_result_ids|.
-void FindAndCheck(IndexImpl* index,
+void FindAndCheck(mojom::Index* index,
                   std::string query,
+                  int32_t max_latency_in_ms,
                   int32_t max_results,
-                  ResponseStatus expected_status,
+                  mojom::ResponseStatus expected_status,
                   const std::vector<std::string>& expected_result_ids);
 }  // namespace local_search_service