Files.app: split query into AND conditioned keywords in metadata search.

components/drive/search_metadata.cc

Index: components/drive/search_metadata.cc
diff --git a/components/drive/search_metadata.cc b/components/drive/search_metadata.cc
index 381e81fb0db9260cbd4f4a3778abfcae8c63ddfd..5e2a7d8fe7272f8d20dc044d796131937c5272f1 100644
--- a/components/drive/search_metadata.cc
+++ b/components/drive/search_metadata.cc
@@ -9,7 +9,9 @@
 
 #include "base/bind.h"
 #include "base/i18n/string_search.h"
+#include "base/memory/scoped_vector.h"
 #include "base/metrics/histogram.h"
+#include "base/strings/string_split.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "components/drive/drive_api_util.h"
@@ -141,12 +143,13 @@ class HiddenEntryClassifier {
 
 // Used to implement SearchMetadata.
 // Adds entry to the result when appropriate.
-// In particular, if |query| is non-null, only adds files with the name matching
-// the query.
+// In particular, if size of |queries| is larger than 0, only adds files with
+// the name matching the query.
 FileError MaybeAddEntryToResult(
     ResourceMetadata* resource_metadata,
     ResourceMetadata::Iterator* it,
-    base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents* query,
+    const ScopedVector<
+        base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents>& queries,
     const SearchMetadataPredicate& predicate,
     size_t at_most_num_matches,
     HiddenEntryClassifier* hidden_entry_classifier,
@@ -168,7 +171,8 @@ FileError MaybeAddEntryToResult(
   // contain |query| to match the query.
   std::string highlighted;
   if (!predicate.Run(entry) ||
-      (query && !FindAndHighlight(entry.base_name(), query, &highlighted)))
+      (queries.size() > 0 &&
+       !FindAndHighlight(entry.base_name(), queries, &highlighted)))
     return FILE_ERROR_OK;
 
   // Hidden entry should not be returned.
@@ -194,8 +198,17 @@ FileError SearchMetadataOnBlockingPool(ResourceMetadata* resource_metadata,
                       ResultCandidateComparator> result_candidates;
 
   // Prepare data structure for searching.
-  base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents query(
-      base::UTF8ToUTF16(query_text));
+  std::vector<base::string16> keywords =
+      base::SplitString(base::UTF8ToUTF16(query_text), base::string16(1, ' '),

[hashimoto, 2015/09/02 03:24:11]

IIUC this code doesn't support whitespace characters other than ASCII space,
like IDEOGRAPHIC SPACE.
(Chromium has a list of whitespace characters
https://code.google.com/p/chromium/codesearch#chromium/src/base/strings/strin...)

Is this intended behavior?
Do we have a (future) plan to support other whitespace characters?

[yawano, 2015/09/04 09:33:55]

No. This should be able to handle other types of whitespace characters. I fixed
it. Thank you!

+                        base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+
+  ScopedVector<base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents>
+      queries;
+  for (const auto& keyword : keywords) {
+    queries.push_back(
+        new base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents(
+            keyword));
+  }
 
   // Prepare an object to filter out hidden entries.
   ResourceEntry mydrive;
@@ -210,9 +223,8 @@ FileError SearchMetadataOnBlockingPool(ResourceMetadata* resource_metadata,
   scoped_ptr<ResourceMetadata::Iterator> it = resource_metadata->GetIterator();
   for (; !it->IsAtEnd(); it->Advance()) {
     FileError error = MaybeAddEntryToResult(
-        resource_metadata, it.get(), query_text.empty() ? NULL : &query,
-        predicate, at_most_num_matches, &hidden_entry_classifier,
-        &result_candidates);
+        resource_metadata, it.get(), queries, predicate, at_most_num_matches,
+        &hidden_entry_classifier, &result_candidates);
     if (error != FILE_ERROR_OK)
       return error;
   }
@@ -308,26 +320,43 @@ bool MatchesType(int options, const ResourceEntry& entry) {
 
 bool FindAndHighlight(
     const std::string& text,
-    base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents* query,
+    const ScopedVector<
+        base::i18n::FixedPatternStringSearchIgnoringCaseAndAccents>& queries,
     std::string* highlighted_text) {
-  DCHECK(query);
   DCHECK(highlighted_text);
   highlighted_text->clear();
 
-  base::string16 text16 = base::UTF8ToUTF16(text);
   size_t match_start = 0;
   size_t match_length = 0;
-  if (!query->Search(text16, &match_start, &match_length))
-    return false;
 
-  base::string16 pre = text16.substr(0, match_start);
-  base::string16 match = text16.substr(match_start, match_length);
-  base::string16 post = text16.substr(match_start + match_length);
-  highlighted_text->append(net::EscapeForHTML(base::UTF16ToUTF8(pre)));
-  highlighted_text->append("<b>");
-  highlighted_text->append(net::EscapeForHTML(base::UTF16ToUTF8(match)));
-  highlighted_text->append("</b>");
-  highlighted_text->append(net::EscapeForHTML(base::UTF16ToUTF8(post)));
+  base::string16 text16 = base::UTF8ToUTF16(text);
+  std::vector<bool> highlights(text16.size(), false);
+  for (auto* query : queries) {
+    if (!query->Search(text16, &match_start, &match_length))
+      return false;
+
+    int match_end = static_cast<int>(match_start + match_length);

[hashimoto, 2015/09/02 03:24:11]

You don't have to static_cast if you use size_t instead of int.

[yawano, 2015/09/04 09:33:55]

Done.

+    for (int i = static_cast<int>(match_start); i < match_end; i++) {

[hashimoto, 2015/09/02 03:24:11]

++i instead of i++

[yawano, 2015/09/04 09:33:55]

Done.

+      highlights[i] = true;
+    }
+  }
+
+  // Generate highlighted text.
+  bool highlighting = false;
+  for (int i = 0; i < static_cast<int>(text16.size()); i++) {

[hashimoto, 2015/09/02 03:24:11]

Use size_t, ++i

[yawano, 2015/09/04 09:33:55]

Done.

+    if (highlighting != highlights[i]) {
+      highlighted_text->append(highlighting ? "</b>" : "<b>");
+      highlighting = highlights[i];
+    }
+
+    // TODO(yawano): Optimize this. Append as a chunk.
+    highlighted_text->append(
+        net::EscapeForHTML(base::UTF16ToUTF8(text16.substr(i, 1))));

[hashimoto, 2015/09/02 03:24:11]

Does this code work with a string which contains surrogate pairs?
Please add a test for the case if we haven't had one yet.

[yawano, 2015/09/04 09:33:55]

No, this hasn't worked. Fixed and added unit test.

+  }
+
+  if (highlighting)
+    highlighted_text->append("</b>");
+
   return true;
 }