Commit 8b5b9bd4 authored by David Black's avatar David Black Committed by Commit Bot

Populate search results from conversation starters cache.

On creation, AssistantSearchProvider will sync its results with the
cache of Assistant conversation starters. It will also observe changes
to the conversation starters cache to keep itself in sync.

Bug: b:153165835
Change-Id: I678bb6c8b8d80148c18c4970107872089d02dd12
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2140210
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Reviewed-by: default avatarXiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759114}
parent 234d0d56
...@@ -111,6 +111,7 @@ component("app_list") { ...@@ -111,6 +111,7 @@ component("app_list") {
"//ash/assistant/util", "//ash/assistant/util",
"//ash/keyboard/ui", "//ash/keyboard/ui",
"//ash/public/cpp/app_list/vector_icons", "//ash/public/cpp/app_list/vector_icons",
"//ash/public/cpp/vector_icons",
"//ash/resources/vector_icons", "//ash/resources/vector_icons",
"//ash/strings", "//ash/strings",
"//base", "//base",
......
...@@ -24,6 +24,7 @@ ...@@ -24,6 +24,7 @@
#include "ash/public/cpp/app_list/app_list_config.h" #include "ash/public/cpp/app_list/app_list_config.h"
#include "ash/public/cpp/app_list/app_list_features.h" #include "ash/public/cpp/app_list/app_list_features.h"
#include "ash/public/cpp/app_list/vector_icons/vector_icons.h" #include "ash/public/cpp/app_list/vector_icons/vector_icons.h"
#include "ash/public/cpp/vector_icons/vector_icons.h"
#include "ash/public/cpp/wallpaper_types.h" #include "ash/public/cpp/wallpaper_types.h"
#include "ash/resources/vector_icons/vector_icons.h" #include "ash/resources/vector_icons/vector_icons.h"
#include "base/bind.h" #include "base/bind.h"
......
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
#include "ash/public/cpp/app_list/app_list_features.h" #include "ash/public/cpp/app_list/app_list_features.h"
#include "ash/public/cpp/app_list/app_list_metrics.h" #include "ash/public/cpp/app_list/app_list_metrics.h"
#include "ash/public/cpp/app_list/vector_icons/vector_icons.h" #include "ash/public/cpp/app_list/vector_icons/vector_icons.h"
#include "ash/public/cpp/vector_icons/vector_icons.h"
#include "ash/resources/vector_icons/vector_icons.h" #include "ash/resources/vector_icons/vector_icons.h"
#include "ash/strings/grit/ash_strings.h" #include "ash/strings/grit/ash_strings.h"
#include "base/bind.h" #include "base/bind.h"
......
...@@ -9,7 +9,7 @@ ...@@ -9,7 +9,7 @@
#include "ash/assistant/ui/assistant_ui_constants.h" #include "ash/assistant/ui/assistant_ui_constants.h"
#include "ash/assistant/ui/assistant_view_delegate.h" #include "ash/assistant/ui/assistant_view_delegate.h"
#include "ash/public/cpp/assistant/proactive_suggestions.h" #include "ash/public/cpp/assistant/proactive_suggestions.h"
#include "ash/resources/vector_icons/vector_icons.h" #include "ash/public/cpp/vector_icons/vector_icons.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "chromeos/services/assistant/public/features.h" #include "chromeos/services/assistant/public/features.h"
#include "net/base/escape.h" #include "net/base/escape.h"
......
...@@ -9,6 +9,7 @@ aggregate_vector_icons("ash_public_vector_icons") { ...@@ -9,6 +9,7 @@ aggregate_vector_icons("ash_public_vector_icons") {
icon_directory = "." icon_directory = "."
icons = [ icons = [
"assistant.icon",
"notification_assistant.icon", "notification_assistant.icon",
"notification_supervised_user.icon", "notification_supervised_user.icon",
"notification_warning.icon", "notification_warning.icon",
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "ash/quick_answers/ui/quick_answers_view.h" #include "ash/quick_answers/ui/quick_answers_view.h"
#include "ash/public/cpp/assistant/assistant_interface_binder.h" #include "ash/public/cpp/assistant/assistant_interface_binder.h"
#include "ash/public/cpp/vector_icons/vector_icons.h"
#include "ash/quick_answers/quick_answers_ui_controller.h" #include "ash/quick_answers/quick_answers_ui_controller.h"
#include "ash/resources/vector_icons/vector_icons.h" #include "ash/resources/vector_icons/vector_icons.h"
#include "ash/shell.h" #include "ash/shell.h"
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "ash/quick_answers/ui/user_consent_view.h" #include "ash/quick_answers/ui/user_consent_view.h"
#include "ash/public/cpp/vector_icons/vector_icons.h"
#include "ash/quick_answers/quick_answers_ui_controller.h" #include "ash/quick_answers/quick_answers_ui_controller.h"
#include "ash/resources/vector_icons/vector_icons.h" #include "ash/resources/vector_icons/vector_icons.h"
#include "ash/shell.h" #include "ash/shell.h"
......
...@@ -10,7 +10,6 @@ aggregate_vector_icons("ash_vector_icons") { ...@@ -10,7 +10,6 @@ aggregate_vector_icons("ash_vector_icons") {
icons = [ icons = [
"always_show_shelf.icon", "always_show_shelf.icon",
"assistant.icon",
"auto_hide.icon", "auto_hide.icon",
"autoclick.icon", "autoclick.icon",
"autoclick_close.icon", "autoclick_close.icon",
......
...@@ -4014,12 +4014,11 @@ jumbo_static_library("ui") { ...@@ -4014,12 +4014,11 @@ jumbo_static_library("ui") {
"views/plugin_vm/plugin_vm_installer_view.h", "views/plugin_vm/plugin_vm_installer_view.h",
] ]
deps += [ deps += [
# TODO(wutao): Put new icons resources to ash/public/cpp/vector_icons/
# when UX provides them.
"//ash/app_list", "//ash/app_list",
"//ash/assistant/model",
"//ash/public/cpp", "//ash/public/cpp",
"//ash/public/cpp/app_list/vector_icons", "//ash/public/cpp/app_list/vector_icons",
"//ash/resources/vector_icons", "//ash/public/cpp/vector_icons",
"//chrome/browser/chromeos/crostini:crostini_installer_types_mojom", "//chrome/browser/chromeos/crostini:crostini_installer_types_mojom",
"//chrome/browser/ui/app_list/search/cros_action_history:cros_action_proto", "//chrome/browser/ui/app_list/search/cros_action_history:cros_action_proto",
"//chrome/browser/ui/app_list/search/search_result_ranker:app_launch_event_logger_proto", "//chrome/browser/ui/app_list/search/search_result_ranker:app_launch_event_logger_proto",
......
include_rules = [
"+ash/assistant/model",
"+ash/public/cpp/vector_icons",
]
...@@ -4,29 +4,44 @@ ...@@ -4,29 +4,44 @@
#include "chrome/browser/ui/app_list/search/assistant_search_provider.h" #include "chrome/browser/ui/app_list/search/assistant_search_provider.h"
#include "ash/assistant/model/assistant_suggestions_model.h"
#include "ash/public/cpp/app_list/app_list_config.h"
#include "ash/public/cpp/app_list/app_list_metrics.h" #include "ash/public/cpp/app_list/app_list_metrics.h"
#include "ash/public/cpp/app_list/app_list_types.h" #include "ash/public/cpp/app_list/app_list_types.h"
#include "ash/public/cpp/assistant/controller/assistant_suggestions_controller.h"
#include "ash/public/cpp/vector_icons/vector_icons.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/unguessable_token.h" #include "base/unguessable_token.h"
#include "chrome/browser/ui/app_list/search/chrome_search_result.h" #include "chrome/browser/ui/app_list/search/chrome_search_result.h"
#include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
#include "ui/gfx/color_palette.h"
#include "ui/gfx/paint_vector_icon.h"
namespace app_list { namespace app_list {
namespace { namespace {
// Aliases.
using AssistantSuggestion = chromeos::assistant::mojom::AssistantSuggestion;
// Constants.
constexpr char kIdPrefix[] = "googleassistant://"; constexpr char kIdPrefix[] = "googleassistant://";
// AssistantSearchResult ------------------------------------------------------- // AssistantSearchResult -------------------------------------------------------
// TODO(b:153165835): Populate metadata from an actual conversation starter.
class AssistantSearchResult : public ChromeSearchResult { class AssistantSearchResult : public ChromeSearchResult {
public: public:
AssistantSearchResult() { explicit AssistantSearchResult(
set_id(kIdPrefix + base::UnguessableToken::Create().ToString()); const AssistantSuggestion* conversation_starter) {
set_id(kIdPrefix + conversation_starter->id.ToString());
SetDisplayIndex(ash::SearchResultDisplayIndex::kFirstIndex); SetDisplayIndex(ash::SearchResultDisplayIndex::kFirstIndex);
SetDisplayType(ash::SearchResultDisplayType::kChip); SetDisplayType(ash::SearchResultDisplayType::kChip);
SetResultType(ash::AppListSearchResultType::kAssistantChip); SetResultType(ash::AppListSearchResultType::kAssistantChip);
SetTitle(base::UTF8ToUTF16("AssistantSearchResult")); SetTitle(base::UTF8ToUTF16(conversation_starter->text));
SetChipIcon(gfx::CreateVectorIcon(
ash::kAssistantIcon,
ash::AppListConfig::instance().suggestion_chip_icon_dimension(),
gfx::kPlaceholderColor));
} }
AssistantSearchResult(const AssistantSearchResult&) = delete; AssistantSearchResult(const AssistantSearchResult&) = delete;
...@@ -48,15 +63,28 @@ class AssistantSearchResult : public ChromeSearchResult { ...@@ -48,15 +63,28 @@ class AssistantSearchResult : public ChromeSearchResult {
// AssistantSearchProvider ----------------------------------------------------- // AssistantSearchProvider -----------------------------------------------------
AssistantSearchProvider::AssistantSearchProvider() { AssistantSearchProvider::AssistantSearchProvider() {
// TODO(b:153165835): Populate results from conversation starters cache. // Synchronize our initial state w/ that of the Assistant suggestions model.
// NOTE: We'll only create a result if we meet a confidence score threshold. OnConversationStartersChanged(ash::AssistantSuggestionsController::Get()
SearchProvider::Results results; ->GetModel()
results.push_back(std::make_unique<AssistantSearchResult>()); ->GetConversationStarters());
SwapResults(&results);
// Observe the Assistant suggestions model to receive updates.
ash::AssistantSuggestionsController::Get()->AddModelObserver(this);
} }
AssistantSearchProvider::~AssistantSearchProvider() = default; AssistantSearchProvider::~AssistantSearchProvider() {
ash::AssistantSuggestionsController::Get()->RemoveModelObserver(this);
}
// TODO(b:153167085): Implement refresh logic to fetch fresh results. // TODO(b:153466226): Only create a result if confidence score threshold is met.
void AssistantSearchProvider::OnConversationStartersChanged(
const std::vector<const AssistantSuggestion*>& conversation_starters) {
SearchProvider::Results results;
if (!conversation_starters.empty()) {
const AssistantSuggestion* starter = conversation_starters.front();
results.push_back(std::make_unique<AssistantSearchResult>(starter));
}
SwapResults(&results);
}
} // namespace app_list } // namespace app_list
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#ifndef CHROME_BROWSER_UI_APP_LIST_SEARCH_ASSISTANT_SEARCH_PROVIDER_H_ #ifndef CHROME_BROWSER_UI_APP_LIST_SEARCH_ASSISTANT_SEARCH_PROVIDER_H_
#define CHROME_BROWSER_UI_APP_LIST_SEARCH_ASSISTANT_SEARCH_PROVIDER_H_ #define CHROME_BROWSER_UI_APP_LIST_SEARCH_ASSISTANT_SEARCH_PROVIDER_H_
#include "ash/assistant/model/assistant_suggestions_model_observer.h"
#include "chrome/browser/ui/app_list/search/search_provider.h" #include "chrome/browser/ui/app_list/search/search_provider.h"
namespace app_list { namespace app_list {
...@@ -13,15 +14,22 @@ namespace app_list { ...@@ -13,15 +14,22 @@ namespace app_list {
// NOTE: This is currently only used to provide a single search result when // NOTE: This is currently only used to provide a single search result when
// launcher chip integration is enabled from Assistant's internal cache of // launcher chip integration is enabled from Assistant's internal cache of
// conversation starters. // conversation starters.
class AssistantSearchProvider : public SearchProvider { class AssistantSearchProvider : public SearchProvider,
public ash::AssistantSuggestionsModelObserver {
public: public:
AssistantSearchProvider(); AssistantSearchProvider();
AssistantSearchProvider(const AssistantSearchProvider&) = delete; AssistantSearchProvider(const AssistantSearchProvider&) = delete;
AssistantSearchProvider& operator=(const AssistantSearchProvider&) = delete; AssistantSearchProvider& operator=(const AssistantSearchProvider&) = delete;
~AssistantSearchProvider() override; ~AssistantSearchProvider() override;
private:
// SearchProvider: // SearchProvider:
void Start(const base::string16& query) override {} void Start(const base::string16& query) override {}
// ash::AssistantSuggestionsModelObserver:
void OnConversationStartersChanged(
const std::vector<const AssistantSuggestion*>& conversation_starters)
override;
}; };
} // namespace app_list } // namespace app_list
......
...@@ -2,13 +2,143 @@ ...@@ -2,13 +2,143 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#include "chrome/browser/ui/app_list/search/assistant_search_provider.h" #include <vector>
#include "ash/assistant/model/assistant_suggestions_model.h"
#include "ash/public/cpp/app_list/app_list_config.h"
#include "ash/public/cpp/assistant/controller/assistant_suggestions_controller.h"
#include "ash/public/cpp/vector_icons/vector_icons.h"
#include "base/strings/utf_string_conversions.h"
#include "base/unguessable_token.h"
#include "chrome/browser/ui/app_list/app_list_test_util.h" #include "chrome/browser/ui/app_list/app_list_test_util.h"
#include "chrome/browser/ui/app_list/search/assistant_search_provider.h"
#include "chrome/browser/ui/app_list/search/chrome_search_result.h" #include "chrome/browser/ui/app_list/search/chrome_search_result.h"
#include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
#include "ui/gfx/color_palette.h"
#include "ui/gfx/paint_vector_icon.h"
namespace app_list { namespace app_list {
namespace test { namespace test {
// Aliases.
using AssistantSuggestion = chromeos::assistant::mojom::AssistantSuggestion;
using AssistantSuggestionPtr =
chromeos::assistant::mojom::AssistantSuggestionPtr;
// Expectations ----------------------------------------------------------------
class Expect {
public:
explicit Expect(const ChromeSearchResult& result) : r_(result) {
EXPECT_EQ(r_.display_index(), ash::SearchResultDisplayIndex::kFirstIndex);
EXPECT_EQ(r_.display_type(), ash::SearchResultDisplayType::kChip);
EXPECT_EQ(r_.result_type(), ash::AppListSearchResultType::kAssistantChip);
EXPECT_EQ(r_.GetSearchResultType(), ash::SearchResultType::ASSISTANT);
EXPECT_TRUE(r_.chip_icon().BackedBySameObjectAs(gfx::CreateVectorIcon(
ash::kAssistantIcon,
ash::AppListConfig::instance().suggestion_chip_icon_dimension(),
gfx::kPlaceholderColor)));
}
Expect(const Expect&) = delete;
Expect& operator=(const Expect&) = delete;
~Expect() = default;
Expect& Matches(const AssistantSuggestion& starter) {
EXPECT_EQ(r_.id(), "googleassistant://" + starter.id.ToString());
EXPECT_EQ(r_.title(), base::UTF8ToUTF16(starter.text));
return *this;
}
private:
const ChromeSearchResult& r_;
};
// ConversationStarterBuilder --------------------------------------------------
class ConversationStarterBuilder {
public:
ConversationStarterBuilder() = default;
ConversationStarterBuilder(const ConversationStarterBuilder&) = delete;
ConversationStarterBuilder& operator=(const ConversationStarterBuilder&) =
delete;
~ConversationStarterBuilder() = default;
AssistantSuggestionPtr Build() {
DCHECK(!id_.is_empty());
DCHECK(!text_.empty());
AssistantSuggestionPtr conversation_starter = AssistantSuggestion::New();
conversation_starter->id = id_;
conversation_starter->text = text_;
return conversation_starter;
}
ConversationStarterBuilder& WithId(const base::UnguessableToken& id) {
id_ = id;
return *this;
}
ConversationStarterBuilder& WithText(const std::string& text) {
text_ = text;
return *this;
}
private:
base::UnguessableToken id_;
std::string text_;
};
// TestAssistantSuggestionsController ------------------------------------------
class TestAssistantSuggestionsController
: public ash::AssistantSuggestionsController {
public:
TestAssistantSuggestionsController() {
SetConversationStarter(ConversationStarterBuilder()
.WithId(base::UnguessableToken::Create())
.WithText("Initial result")
.Build());
}
TestAssistantSuggestionsController(
const TestAssistantSuggestionsController&) = delete;
TestAssistantSuggestionsController& operator=(
const TestAssistantSuggestionsController&) = delete;
~TestAssistantSuggestionsController() override = default;
// ash::AssistantSuggestionsController:
const ash::AssistantSuggestionsModel* GetModel() const override {
return &model_;
}
void AddModelObserver(
ash::AssistantSuggestionsModelObserver* observer) override {
model_.AddObserver(observer);
}
void RemoveModelObserver(
ash::AssistantSuggestionsModelObserver* observer) override {
model_.RemoveObserver(observer);
}
void ClearConversationStarters() { SetConversationStarters({}); }
void SetConversationStarter(AssistantSuggestionPtr conversation_starter) {
std::vector<AssistantSuggestionPtr> conversation_starters;
conversation_starters.push_back(std::move(conversation_starter));
SetConversationStarters(std::move(conversation_starters));
}
void SetConversationStarters(
std::vector<AssistantSuggestionPtr> conversation_starters) {
model_.SetConversationStarters(std::move(conversation_starters));
}
private:
ash::AssistantSuggestionsModel model_;
};
// AssistantSearchProviderTest ------------------------------------------------- // AssistantSearchProviderTest -------------------------------------------------
class AssistantSearchProviderTest : public AppListTestBase { class AssistantSearchProviderTest : public AppListTestBase {
...@@ -21,20 +151,46 @@ class AssistantSearchProviderTest : public AppListTestBase { ...@@ -21,20 +151,46 @@ class AssistantSearchProviderTest : public AppListTestBase {
AssistantSearchProvider& search_provider() { return search_provider_; } AssistantSearchProvider& search_provider() { return search_provider_; }
TestAssistantSuggestionsController& suggestions_controller() {
return suggestions_controller_;
}
private: private:
TestAssistantSuggestionsController suggestions_controller_;
AssistantSearchProvider search_provider_; AssistantSearchProvider search_provider_;
}; };
// Tests ----------------------------------------------------------------------- // Tests -----------------------------------------------------------------------
TEST_F(AssistantSearchProviderTest, ShouldHaveAnInitialChipResult) { TEST_F(AssistantSearchProviderTest, ShouldHaveAnInitialResult) {
std::vector<const AssistantSuggestion*> conversation_starters =
suggestions_controller().GetModel()->GetConversationStarters();
ASSERT_EQ(1u, conversation_starters.size());
ASSERT_EQ(1u, search_provider().results().size());
const ChromeSearchResult& result = *search_provider().results().at(0);
Expect(result).Matches(*conversation_starters.front());
}
TEST_F(AssistantSearchProviderTest, ShouldClearResultsDynamically) {
EXPECT_EQ(1u, search_provider().results().size());
suggestions_controller().ClearConversationStarters();
EXPECT_TRUE(search_provider().results().empty());
}
TEST_F(AssistantSearchProviderTest, ShouldUpdateResultsDynamically) {
AssistantSuggestionPtr update = ConversationStarterBuilder()
.WithId(base::UnguessableToken::Create())
.WithText("Updated result")
.Build();
suggestions_controller().SetConversationStarter(update->Clone());
ASSERT_EQ(1u, search_provider().results().size()); ASSERT_EQ(1u, search_provider().results().size());
const ChromeSearchResult& result = *search_provider().results().at(0); const ChromeSearchResult& result = *search_provider().results().at(0);
EXPECT_EQ(ash::SearchResultDisplayIndex::kFirstIndex, result.display_index()); Expect(result).Matches(*update);
EXPECT_EQ(ash::SearchResultDisplayType::kChip, result.display_type());
EXPECT_EQ(ash::AppListSearchResultType::kAssistantChip, result.result_type());
EXPECT_EQ(ash::SearchResultType::ASSISTANT, result.GetSearchResultType());
} }
} // namespace test } // namespace test
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment