App Launcher: Webstore results are now highlighted.

We now highlight (in bold) the part of the query that you typed, just as
we do with other types of queries.

Moved AppResult::UpdateFromMatch into SearchResult so it can be used by
any provider.

BUG=481837

Review URL: https://codereview.chromium.org/1110883003

Cr-Commit-Position: refs/heads/master@{#329356}

chrome/browser/ui/app_list/search/app_result.cc

@@ -23,8 +23,6 @@
 #include "extensions/common/extension_icon_set.h"
 #include "extensions/common/manifest_handlers/icons_handler.h"
 #include "ui/app_list/app_list_switches.h"
-#include "ui/app_list/search/tokenized_string.h"
-#include "ui/app_list/search/tokenized_string_match.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/image/image_skia_operations.h"
 
@@ -65,20 +63,6 @@ AppResult::~AppResult() {
   StopObservingExtensionRegistry();
 }
 
-void AppResult::UpdateFromMatch(const TokenizedString& title,
-                                const TokenizedStringMatch& match) {
-  const TokenizedStringMatch::Hits& hits = match.hits();
-
-  Tags tags;
-  tags.reserve(hits.size());
-  for (size_t i = 0; i < hits.size(); ++i)
-    tags.push_back(Tag(Tag::MATCH, hits[i].start(), hits[i].end()));
-
-  set_title(title.text());
-  set_title_tags(tags);
-  set_relevance(match.relevance());
-}
-
 void AppResult::UpdateFromLastLaunched(const base::Time& current_time,
                                        const base::Time& last_launched) {
   base::TimeDelta delta = current_time - last_launched;
@@ -129,6 +113,7 @@ scoped_ptr<SearchResult> AppResult::Duplicate() const {
                     display_type() == DISPLAY_RECOMMENDATION));
   copy->set_title(title());
   copy->set_title_tags(title_tags());
+  copy->set_relevance(relevance());
 
   return copy.Pass();
 }

chrome/browser/ui/app_list/search/app_result.h

@@ -29,8 +29,6 @@ class ExtensionRegistry;
 namespace app_list {
 
 class AppContextMenu;
-class TokenizedString;
-class TokenizedStringMatch;
 
 class AppResult : public SearchResult,
                   public extensions::IconImage::Observer,
@@ -44,9 +42,6 @@ class AppResult : public SearchResult,
             bool is_recommendation);
   ~AppResult() override;
 
-  void UpdateFromMatch(const TokenizedString& title,
-                       const TokenizedStringMatch& match);
-
   void UpdateFromLastLaunched(const base::Time& current_time,
                               const base::Time& last_launched);
 

chrome/browser/ui/app_list/search/webstore/webstore_provider.cc

@@ -178,17 +178,18 @@ scoped_ptr<SearchResult> WebstoreProvider::CreateResult(
   extensions::Manifest::Type item_type = ParseItemType(item_type_string);
 
   // Calculate the relevance score by matching the query with the title. Results
-  // with a match score of 0 are discarded.
-  // TODO(mgiuca): Set the tags to indicate the parts of the title that were
-  // matched.
+  // with a match score of 0 are discarded. This will also be used to set the
+  // title tags (highlighting which parts of the title matched the search
+  // query).
   TokenizedString title(base::UTF8ToUTF16(localized_name));
   TokenizedStringMatch match;
   if (!match.Calculate(query, title))
     return scoped_ptr<SearchResult>();
 
-  return make_scoped_ptr(new WebstoreResult(profile_, app_id, localized_name,
-                                            match.relevance(), icon_url,
-                                            is_paid, item_type, controller_));
+  scoped_ptr<SearchResult> result(new WebstoreResult(
+      profile_, app_id, icon_url, is_paid, item_type, controller_));
+  result->UpdateFromMatch(title, match);
+  return result;
 }
 
 }  // namespace app_list

chrome/browser/ui/app_list/search/webstore/webstore_provider_browsertest.cc

@@ -41,7 +41,7 @@ const char kOneResult[] =
     "\"results\":["
     "  {"
     "    \"id\": \"app1_id\","
-    "    \"localized_name\": \"app1 name\","
+    "    \"localized_name\": \"Fun App\","
     "    \"icon_url\": \"http://host/icon\","
     "    \"is_paid\": false"
     "  }"
@@ -83,32 +83,49 @@ struct ParsedSearchResult {
   size_t num_actions;
 };
 
+// Expected results from a search for "fun" on kOneResult.
 ParsedSearchResult kParsedOneResult[] = {{"app1_id",
-                                          "app1 name",
+                                          "[Fun] App",
                                           "http://host/icon",
                                           false,
                                           Manifest::TYPE_UNKNOWN,
                                           1}};
 
-ParsedSearchResult kParsedThreeResults[] = {{"app1_id",
-                                             "Mystery App",
+// Expected results from a search for "app" on kThreeResults.
+ParsedSearchResult kParsedThreeResultsApp[] = {
+    {"app1_id",
+     "Mystery [App]",
      "http://host/icon1",
      true,
      Manifest::TYPE_PLATFORM_APP,
      1},
     {"app2_id",
-                                             "App Mystère",
+     "[App] Mystère",
      "http://host/icon2",
      false,
      Manifest::TYPE_HOSTED_APP,
      1},
     {"app3_id",
-                                             "Mistero App",
+     "Mistero [App]",
      "http://host/icon3",
      false,
      Manifest::TYPE_LEGACY_PACKAGED_APP,
      1}};
 
+// Expected results from a search for "myst" on kThreeResults.
+ParsedSearchResult kParsedThreeResultsMyst[] = {{"app1_id",
+                                                 "[Myst]ery App",
+                                                 "http://host/icon1",
+                                                 true,
+                                                 Manifest::TYPE_PLATFORM_APP,
+                                                 1},
+                                                {"app2_id",
+                                                 "App [Myst]ère",
+                                                 "http://host/icon2",
+                                                 false,
+                                                 Manifest::TYPE_HOSTED_APP,
+                                                 1}};
+
 }  // namespace
 
 class WebstoreProviderTest : public InProcessBrowserTest {
@@ -183,7 +200,8 @@ class WebstoreProviderTest : public InProcessBrowserTest {
                     expected_results[i].id).spec(),
                 result->id());
       EXPECT_EQ(std::string(expected_results[i].title),
-                base::UTF16ToUTF8(result->title()));
+                app_list::SearchResult::TagsDebugString(
+                    base::UTF16ToUTF8(result->title()), result->title_tags()));
 
       // Ensure the number of action buttons is appropriate for the item type.
       EXPECT_EQ(expected_results[i].num_actions, result->actions().size());
@@ -265,18 +283,18 @@ IN_PROC_BROWSER_TEST_F(WebstoreProviderTest, Basic) {
       {"bad json", "invalid json", "bad json", nullptr, 0},
       // Good results. Note that the search term appears in all of the result
       // titles.
-      {"app1", kOneResult, "app1 name", kParsedOneResult, 1},
+      {"fun", kOneResult, "Fun App", kParsedOneResult, 1},
       {"app",
        kThreeResults,
        "Mystery App,App Mystère,Mistero App",
-       kParsedThreeResults,
+       kParsedThreeResultsApp,
        3},
       // Search where one of the results does not include the query term. Only
       // the results with a title matching the query should be selected.
       {"myst",
        kThreeResults,
        "Mystery App,App Mystère",
-       kParsedThreeResults,
+       kParsedThreeResultsMyst,
        2},
   };
 
@@ -317,16 +335,16 @@ IN_PROC_BROWSER_TEST_F(WebstoreProviderTest, NoSearchForSensitiveData) {
 }
 
 IN_PROC_BROWSER_TEST_F(WebstoreProviderTest, NoSearchForShortQueries) {
-  RunQueryAndVerify("a", kOneResult, nullptr, 0);
-  RunQueryAndVerify("ap", kOneResult, nullptr, 0);
-  RunQueryAndVerify("app", kOneResult, kParsedOneResult, 1);
+  RunQueryAndVerify("f", kOneResult, nullptr, 0);
+  RunQueryAndVerify("fu", kOneResult, nullptr, 0);
+  RunQueryAndVerify("fun", kOneResult, kParsedOneResult, 1);
 }
 
 IN_PROC_BROWSER_TEST_F(WebstoreProviderTest, SearchCache) {
-  RunQueryAndVerify("app", kOneResult, kParsedOneResult, 1);
+  RunQueryAndVerify("fun", kOneResult, kParsedOneResult, 1);
 
   // No result is provided but the provider gets the result from the cache.
-  RunQueryAndVerify("app", "", kParsedOneResult, 1);
+  RunQueryAndVerify("fun", "", kParsedOneResult, 1);
 }
 
 }  // namespace test

chrome/browser/ui/app_list/search/webstore/webstore_result.cc

@@ -61,15 +61,12 @@ namespace app_list {
 
 WebstoreResult::WebstoreResult(Profile* profile,
                                const std::string& app_id,
-                               const std::string& localized_name,
-                               double relevance,
                                const GURL& icon_url,
                                bool is_paid,
                                extensions::Manifest::Type item_type,
                                AppListControllerDelegate* controller)
     : profile_(profile),
       app_id_(app_id),
-      localized_name_(localized_name),
       icon_url_(icon_url),
       is_paid_(is_paid),
       item_type_(item_type),
@@ -78,9 +75,7 @@ WebstoreResult::WebstoreResult(Profile* profile,
       extension_registry_(NULL),
       weak_factory_(this) {
   set_id(extensions::Extension::GetBaseURLFromExtensionId(app_id_).spec());
-  set_relevance(relevance);
 
-  set_title(base::UTF8ToUTF16(localized_name_));
   SetDefaultDetails();
 
   InitAndStartObserving();
@@ -128,9 +123,12 @@ void WebstoreResult::InvokeAction(int action_index, int event_flags) {
 }
 
 scoped_ptr<SearchResult> WebstoreResult::Duplicate() const {
-  return scoped_ptr<SearchResult>(
-      new WebstoreResult(profile_, app_id_, localized_name_, relevance(),
-                         icon_url_, is_paid_, item_type_, controller_));
+  scoped_ptr<SearchResult> copy(new WebstoreResult(
+      profile_, app_id_, icon_url_, is_paid_, item_type_, controller_));
+  copy->set_title(title());
+  copy->set_title_tags(title_tags());
+  copy->set_relevance(relevance());
+  return copy;
 }
 
 void WebstoreResult::InitAndStartObserving() {

chrome/browser/ui/app_list/search/webstore/webstore_result.h

@@ -32,8 +32,6 @@ class WebstoreResult : public SearchResult,
  public:
   WebstoreResult(Profile* profile,
                  const std::string& app_id,
-                 const std::string& localized_name,
-                 double relevance,
                  const GURL& icon_url,
                  bool is_paid,
                  extensions::Manifest::Type item_type,
@@ -82,7 +80,6 @@ class WebstoreResult : public SearchResult,
 
   Profile* profile_;
   const std::string app_id_;
-  const std::string localized_name_;
   const GURL icon_url_;
   const bool is_paid_;
   extensions::Manifest::Type item_type_;

ui/app_list/search_result.cc

@@ -4,7 +4,11 @@
 
 #include "ui/app_list/search_result.h"
 
+#include <map>
+
 #include "ui/app_list/app_list_constants.h"
+#include "ui/app_list/search/tokenized_string.h"
+#include "ui/app_list/search/tokenized_string_match.h"
 #include "ui/app_list/search_result_observer.h"
 
 namespace app_list {
@@ -99,6 +103,20 @@ void SearchResult::RemoveObserver(SearchResultObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
+void SearchResult::UpdateFromMatch(const TokenizedString& title,
+                                   const TokenizedStringMatch& match) {
+  const TokenizedStringMatch::Hits& hits = match.hits();
+
+  Tags tags;
+  tags.reserve(hits.size());
+  for (size_t i = 0; i < hits.size(); ++i)
+    tags.push_back(Tag(Tag::MATCH, hits[i].start(), hits[i].end()));
+
+  set_title(title.text());
+  set_title_tags(tags);
+  set_relevance(match.relevance());
+}
+
 void SearchResult::Open(int event_flags) {
 }
 
@@ -109,4 +127,33 @@ ui::MenuModel* SearchResult::GetContextMenuModel() {
   return NULL;
 }
 
+// static
+std::string SearchResult::TagsDebugString(const std::string& text,
+                                          const Tags& tags) {
+  std::string result = text;
+
+  // Build a table of delimiters to insert.
+  std::map<size_t, std::string> inserts;
+  for (const auto& tag : tags) {
+    if (tag.styles & Tag::URL)
+      inserts[tag.range.start()].push_back('{');
+    if (tag.styles & Tag::MATCH)
+      inserts[tag.range.start()].push_back('[');
+    if (tag.styles & Tag::DIM) {
+      inserts[tag.range.start()].push_back('<');
+      inserts[tag.range.end()].push_back('>');
+    }
+    if (tag.styles & Tag::MATCH)
+      inserts[tag.range.end()].push_back(']');
+    if (tag.styles & Tag::URL)
+      inserts[tag.range.end()].push_back('}');
+  }
+
+  // Insert the delimiters (in reverse order, to preserve indices).
+  for (auto it = inserts.rbegin(); it != inserts.rend(); ++it)
+    result.insert(it->first, it->second);
+
+  return result;
+}
+
 }  // namespace app_list

ui/app_list/search_result.h

@@ -22,6 +22,8 @@ class MenuModel;
 namespace app_list {
 
 class SearchResultObserver;
+class TokenizedString;
+class TokenizedStringMatch;
 
 // SearchResult consists of an icon, title text and details text. Title and
 // details text can have tagged ranges that are displayed differently from
@@ -135,6 +137,11 @@ class APP_LIST_EXPORT SearchResult {
   void AddObserver(SearchResultObserver* observer);
   void RemoveObserver(SearchResultObserver* observer);
 
+  // Updates the result's relevance score, and sets its title and title tags,
+  // based on a string match result.
+  void UpdateFromMatch(const TokenizedString& title,
+                       const TokenizedStringMatch& match);
+
   // TODO(mukai): Remove this method and really simplify the ownership of
   // SearchResult. Ideally, SearchResult will be copyable.
   virtual scoped_ptr<SearchResult> Duplicate() const = 0;
@@ -147,6 +154,10 @@ class APP_LIST_EXPORT SearchResult {
   // Note the returned menu model is owned by this item.
   virtual ui::MenuModel* GetContextMenuModel();
 
+  // Returns a string showing |text| marked up with brackets indicating the
+  // tag positions in |tags|. Useful for debugging and testing.
+  static std::string TagsDebugString(const std::string& text, const Tags& tags);
+
  protected:
   void set_id(const std::string& id) { id_ = id; }
   void set_voice_result(bool voice_result) { voice_result_ = voice_result; }