[cros search service] Add sync API for local search service

As majority of out clients will be calling the service in-process,
we are working to towards using C++ impl directly to avoid mojo
overhead. We will add mojo to support out-of-process overhead.

This cl is the first step towards that (see bug below).
Specifically, this cl
1. Allow LocalSearchServiceProxy to return impl directly.
2. Add sync functions to IndexImpl.
3. Add C++ structs that are equivalent to the mojo structs
(Data, Result etc). The mojo structs will be removed later.

Following the cl, the existing user of the service (settings)
will update their function to use impl directly and will make
sync calls.

In the following cl, we will remove the mojo interface and
rename relevant functions. Settings will make changes accordingly
but this two-step change should be simpler than changing everything
in one cl.

Bug: 1064424, 1018613
Change-Id: I1200779483146a0344f0b15191918c0471b04c9e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2114470
Commit-Queue: Jia Meng <jiameng@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753415}

chrome/browser/local_search_service/local_search_service_proxy.cc

@@ -16,11 +16,25 @@ LocalSearchServiceProxy::~LocalSearchServiceProxy() = default;
 
 mojom::LocalSearchService* LocalSearchServiceProxy::GetLocalSearchService() {
   if (!local_search_service_impl_) {
-    local_search_service_impl_ = std::make_unique<LocalSearchServiceImpl>();
-    local_search_service_impl_->BindReceiver(
-        remote_.BindNewPipeAndPassReceiver());
+    CreateLocalSearchServiceAndBind();
   }
   return remote_.get();
 }
 
+LocalSearchServiceImpl* LocalSearchServiceProxy::GetLocalSearchServiceImpl() {
+  if (!local_search_service_impl_) {
+    // 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

@@ -21,8 +21,10 @@ namespace local_search_service {
 
 class LocalSearchServiceImpl;
 
-// This class owns an implementation of LocalSearchService, but it only exposes
-// LocalSearchService through the mojo interface by returning a remote.
+// 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.
@@ -37,7 +39,12 @@ class LocalSearchServiceProxy : public KeyedService {
   // 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_;
 

chrome/services/local_search_service/index_impl.cc

@@ -16,7 +16,7 @@ namespace local_search_service {
 
 namespace {
 
-using Hits = std::vector<mojom::RangePtr>;
+using Hits = std::vector<local_search_service::Range>;
 
 void TokenizeSearchTags(
     const std::vector<base::string16>& search_tags,
@@ -54,8 +54,10 @@ bool IsItemRelevant(
                          partial_match_penalty_rate)) {
       *relevance_score = match.relevance();
       for (const auto& hit : match.hits()) {
-        mojom::RangePtr range = mojom::Range::New(hit.start(), hit.end());
-        hits->push_back(std::move(range));
+        local_search_service::Range range;
+        range.start = hit.start();
+        range.end = hit.end();
+        hits->push_back(range);
       }
       return true;
     }
@@ -63,13 +65,21 @@ bool IsItemRelevant(
   return false;
 }
 
-// Compares mojom::ResultPtr by |score|.
-bool CompareResultPtr(const mojom::ResultPtr& r1, const mojom::ResultPtr& r2) {
-  return r1->score > r2->score;
+// Compares Results by |score|.
+bool CompareResults(const local_search_service::Result& r1,
+                    const local_search_service::Result& r2) {
+  return r1.score > r2.score;
 }
 
 }  // namespace
 
+local_search_service::Data::Data() = default;
+local_search_service::Data::Data(const Data& data) = default;
+local_search_service::Data::~Data() = default;
+local_search_service::Result::Result() = default;
+local_search_service::Result::Result(const Result& result) = default;
+local_search_service::Result::~Result() = default;
+
 IndexImpl::IndexImpl() = default;
 
 IndexImpl::~IndexImpl() = default;
@@ -79,31 +89,57 @@ void IndexImpl::BindReceiver(mojo::PendingReceiver<mojom::Index> receiver) {
 }
 
 void IndexImpl::GetSize(GetSizeCallback callback) {
-  std::move(callback).Run(data_.size());
+  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) {
-  for (const auto& item : data) {
-    const auto& id = item->id;
-    // Keys shouldn't be empty.
-    if (id.empty())
+  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) {
+    const auto& id = item.id;
+    DCHECK(!id.empty());
+
     // If a key already exists, it will overwrite earlier data.
     data_[id] = std::vector<std::unique_ptr<TokenizedString>>();
-    TokenizeSearchTags(item->search_tags, &data_[id]);
+    TokenizeSearchTags(item.search_tags, &data_[id]);
   }
-  std::move(callback).Run();
 }
 
 void IndexImpl::Delete(const std::vector<std::string>& ids,
                        DeleteCallback callback) {
-  uint32_t num_deleted = 0u;
   for (const auto& id : ids) {
-    // Keys shouldn't be empty.
     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) {
+    DCHECK(!id.empty());
 
     const auto& it = data_.find(id);
     if (it != data_.end()) {
@@ -112,32 +148,89 @@ void IndexImpl::Delete(const std::vector<std::string>& ids,
       ++num_deleted;
     }
   }
-  std::move(callback).Run(num_deleted);
+  return num_deleted;
 }
 
 void IndexImpl::Find(const base::string16& query,
                      int32_t max_latency_in_ms,
                      int32_t max_results,
                      FindCallback callback) {
-  if (query.empty()) {
-    std::move(callback).Run(mojom::ResponseStatus::EMPTY_QUERY, base::nullopt);
+  std::vector<local_search_service::Result> results;
+  const auto response = Find(query, max_latency_in_ms, 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,
+    int32_t max_latency_in_ms,
+    int32_t max_results,
+    std::vector<local_search_service::Result>* results) {
+  DCHECK(results);
+  results->clear();
+  if (query.empty()) {
+    return local_search_service::ResponseStatus::kEmptyQuery;
+  }
   if (data_.empty()) {
-    std::move(callback).Run(mojom::ResponseStatus::EMPTY_INDEX, base::nullopt);
-    return;
+    return local_search_service::ResponseStatus::kEmptyIndex;
   }
 
-  std::vector<mojom::ResultPtr> results = GetSearchResults(query);
-  std::move(callback).Run(mojom::ResponseStatus::SUCCESS, std::move(results));
+  *results = GetSearchResults(query);
+  return local_search_service::ResponseStatus::kSuccess;
 }
 
 void IndexImpl::SetSearchParams(mojom::SearchParamsPtr search_params,
                                 SetSearchParamsCallback callback) {
-  search_params_ = std::move(search_params);
+  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;
+}
+
 void IndexImpl::GetSearchParamsForTesting(double* relevance_threshold,
                                           double* partial_match_penalty_rate,
                                           bool* use_prefix_only,
@@ -149,36 +242,36 @@ void IndexImpl::GetSearchParamsForTesting(double* relevance_threshold,
   DCHECK(use_weighted_ratio);
   DCHECK(use_edit_distance);
 
-  *relevance_threshold = search_params_->relevance_threshold;
-  *partial_match_penalty_rate = search_params_->partial_match_penalty_rate;
-  *use_prefix_only = search_params_->use_prefix_only;
-  *use_weighted_ratio = search_params_->use_weighted_ratio;
-  *use_edit_distance = search_params_->use_edit_distance;
+  *relevance_threshold = search_params_.relevance_threshold;
+  *partial_match_penalty_rate = search_params_.partial_match_penalty_rate;
+  *use_prefix_only = search_params_.use_prefix_only;
+  *use_weighted_ratio = search_params_.use_weighted_ratio;
+  *use_edit_distance = search_params_.use_edit_distance;
 }
 
-std::vector<mojom::ResultPtr> IndexImpl::GetSearchResults(
+std::vector<local_search_service::Result> IndexImpl::GetSearchResults(
     const base::string16& query) const {
-  std::vector<mojom::ResultPtr> results;
+  std::vector<local_search_service::Result> results;
   const TokenizedString tokenized_query(query);
 
   for (const auto& item : data_) {
     double relevance_score = 0.0;
     Hits hits;
     if (IsItemRelevant(
-            tokenized_query, item.second, search_params_->relevance_threshold,
-            search_params_->use_prefix_only, search_params_->use_weighted_ratio,
-            search_params_->use_edit_distance,
-            search_params_->partial_match_penalty_rate, &relevance_score,
+            tokenized_query, item.second, search_params_.relevance_threshold,
+            search_params_.use_prefix_only, search_params_.use_weighted_ratio,
+            search_params_.use_edit_distance,
+            search_params_.partial_match_penalty_rate, &relevance_score,
             &hits)) {
-      mojom::ResultPtr result = mojom::Result::New();
-      result->id = item.first;
-      result->score = relevance_score;
-      result->hits = std::move(hits);
-      results.push_back(std::move(result));
+      local_search_service::Result result;
+      result.id = item.first;
+      result.score = relevance_score;
+      result.hits = hits;
+      results.push_back(result);
     }
   }
 
-  std::sort(results.begin(), results.end(), CompareResultPtr);
+  std::sort(results.begin(), results.end(), CompareResults);
   return results;
 }
 

chrome/services/local_search_service/index_impl.h

@@ -24,10 +24,70 @@ class TokenizedString;
 
 namespace local_search_service {
 
+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.
+  std::string id;
+
+  // Data item will be matched between its search tags and query term.
+  std::vector<base::string16> search_tags;
+  Data();
+  Data(const Data& data);
+  ~Data();
+};
+
+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;
+};
+
+// A numeric range used to represent the start and end position.
+struct Range {
+  uint32_t start;
+  uint32_t end;
+};
+
+// 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.
+  std::string id;
+  // Relevance score, in the range of [0,1].
+  double score;
+  // Matching ranges.
+  std::vector<Range> hits;
+
+  Result();
+  Result(const Result& result);
+  ~Result();
+};
+
+// Status of the search attempt.
+// More will be added later.
+enum class ResponseStatus {
+  kUnknownError = 0,
+  // Query is empty.
+  kEmptyQuery = 1,
+  // Index is empty (i.e. no data).
+  kEmptyIndex = 2,
+  // Search operation is successful. But there could be no matching item and
+  // result list is empty.
+  kSuccess = 3
+};
+
 // 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();
@@ -36,21 +96,33 @@ class IndexImpl : public mojom::Index {
   void BindReceiver(mojo::PendingReceiver<mojom::Index> receiver);
 
   // mojom::Index overrides.
+  // Also included the synchronous versions for in-process clients.
   void GetSize(GetSizeCallback callback) override;
+  uint64_t GetSize();
 
+  // 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);
 
+  // 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);
 
   void Find(const base::string16& query,
             int32_t max_latency_in_ms,
             int32_t max_results,
             FindCallback callback) override;
+  local_search_service::ResponseStatus Find(
+      const base::string16& query,
+      int32_t max_latency_in_ms,
+      int32_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,
                                  double* partial_match_penalty_rate,
@@ -60,7 +132,7 @@ class IndexImpl : public mojom::Index {
 
  private:
   // Returns all search results for a given query.
-  std::vector<mojom::ResultPtr> GetSearchResults(
+  std::vector<local_search_service::Result> GetSearchResults(
       const base::string16& query) const;
 
   // A map from key to tokenized search-tags.
@@ -69,7 +141,7 @@ class IndexImpl : public mojom::Index {
   mojo::ReceiverSet<mojom::Index> receivers_;
 
   // Search parameters.
-  mojom::SearchParamsPtr search_params_ = mojom::SearchParams::New();
+  local_search_service::SearchParams search_params_;
 
   DISALLOW_COPY_AND_ASSIGN(IndexImpl);
 };

chrome/services/local_search_service/local_search_service_impl.cc

@@ -24,13 +24,28 @@ void LocalSearchServiceImpl::BindReceiver(
 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;
 
   DCHECK(it != indices_.end());
   DCHECK(it->second);
-  it->second->BindReceiver(std::move(index));
+
+  return it->second.get();
 }
 
 }  // namespace local_search_service

chrome/services/local_search_service/local_search_service_impl.h

@@ -20,9 +20,12 @@ namespace local_search_service {
 
 class IndexImpl;
 
+enum class IndexId { kCrosSettings = 0 };
+
 // 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();
@@ -34,10 +37,13 @@ class LocalSearchServiceImpl : public mojom::LocalSearchService {
   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<mojom::LocalSearchService::IndexId, std::unique_ptr<IndexImpl>>
-      indices_;
+  std::map<local_search_service::IndexId, std::unique_ptr<IndexImpl>> indices_;
 
   DISALLOW_COPY_AND_ASSIGN(LocalSearchServiceImpl);
 };