Skip to content

Commit

Permalink
[Launcher FA] Plumbing for FederatedServiceController.
Browse files Browse the repository at this point in the history
Dependency injection, to facilitate mocking out of controller during
unit testing.

Test: existing
Bug: b/262611120
Change-Id: Ia3a546d39749721a0a60f084ab8531430ae9ebe2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4170142
Commit-Queue: Amanda Deacon <amandadeacon@chromium.org>
Reviewed-by: Lauren Commeignes <laurencom@chromium.org>
Reviewed-by: Xinglong Luan <alanlxl@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1099006}
  • Loading branch information
Amanda Deacon authored and Chromium LUCI CQ committed Jan 31, 2023
1 parent 2be4352 commit 49ef586
Show file tree
Hide file tree
Showing 11 changed files with 54 additions and 18 deletions.
2 changes: 2 additions & 0 deletions ash/system/federated/federated_service_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#ifndef ASH_SYSTEM_FEDERATED_FEDERATED_SERVICE_CONTROLLER_H_
#define ASH_SYSTEM_FEDERATED_FEDERATED_SERVICE_CONTROLLER_H_

#include "ash/ash_export.h"

namespace ash::federated {

// Controller class for the federated service.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ class SpokenFeedbackAppListSearchTest
std::unique_ptr<app_list::SearchController> search_controller =
std::make_unique<app_list::SearchController>(
app_list_client->GetModelUpdaterForTest(), app_list_client, nullptr,
browser()->profile());
browser()->profile(), nullptr);
search_controller->Initialize();
// Disable ranking, which may override the explicitly set relevance scores
// and best match status of results.
Expand Down
5 changes: 4 additions & 1 deletion chrome/browser/ash/app_list/app_list_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
#include "ash/public/cpp/app_list/app_list_types.h"
#include "ash/public/cpp/new_window_delegate.h"
#include "ash/public/cpp/tablet_mode.h"
#include "ash/shell.h"
#include "ash/system/federated/federated_service_controller_impl.h"
#include "base/functional/bind.h"
#include "base/metrics/histogram_functions.h"
#include "base/metrics/histogram_macros.h"
Expand Down Expand Up @@ -500,7 +502,8 @@ void AppListClientImpl::SetProfile(Profile* new_profile) {

void AppListClientImpl::SetUpSearchUI() {
search_controller_ = app_list::CreateSearchController(
profile_, current_model_updater_, this, GetNotifier());
profile_, current_model_updater_, this, GetNotifier(),
ash::Shell::Get()->federated_service_controller());

// Refresh the results used for the suggestion chips with empty query.
// This fixes crbug.com/999287.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,13 @@ bool IsLoggingEnabled() {
} // namespace

FederatedMetricsManager::FederatedMetricsManager(
ash::AppListNotifier* notifier) {
ash::AppListNotifier* notifier,
ash::federated::FederatedServiceController* controller)
: controller_(controller) {
// TODO(b/262611120): Implement.
if (!controller_) {
// TODO(b/262611120): Metrics.
}
}

FederatedMetricsManager::~FederatedMetricsManager() = default;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include <vector>

#include "ash/public/cpp/app_list/app_list_notifier.h"
#include "ash/system/federated/federated_service_controller.h"
#include "base/memory/raw_ptr.h"
#include "base/scoped_observation.h"

namespace app_list {
Expand All @@ -19,7 +21,9 @@ class FederatedMetricsManager : ash::AppListNotifier::Observer {
using Result = ash::AppListNotifier::Result;
using Location = ash::AppListNotifier::Location;

explicit FederatedMetricsManager(ash::AppListNotifier* notifier);
FederatedMetricsManager(
ash::AppListNotifier* notifier,
ash::federated::FederatedServiceController* controller);
~FederatedMetricsManager() override;

FederatedMetricsManager(const FederatedMetricsManager&) = delete;
Expand All @@ -37,6 +41,7 @@ class FederatedMetricsManager : ash::AppListNotifier::Observer {
private:
base::ScopedObservation<ash::AppListNotifier, ash::AppListNotifier::Observer>
observation_{this};
const raw_ptr<ash::federated::FederatedServiceController> controller_;
};

} // namespace app_list
Expand Down
17 changes: 10 additions & 7 deletions chrome/browser/ash/app_list/search/search_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,17 @@ void ClearNonZeroStateResults(ResultsMap& results) {

} // namespace

SearchController::SearchController(AppListModelUpdater* model_updater,
AppListControllerDelegate* list_controller,
ash::AppListNotifier* notifier,
Profile* profile)
SearchController::SearchController(
AppListModelUpdater* model_updater,
AppListControllerDelegate* list_controller,
ash::AppListNotifier* notifier,
Profile* profile,
ash::federated::FederatedServiceController* federated_service_controller)
: profile_(profile),
model_updater_(model_updater),
list_controller_(list_controller),
notifier_(notifier) {}
notifier_(notifier),
federated_service_controller_(federated_service_controller) {}

SearchController::~SearchController() = default;

Expand All @@ -66,8 +69,8 @@ void SearchController::Initialize() {
std::make_unique<SearchMetricsManager>(profile_, notifier_);
session_metrics_manager_ =
std::make_unique<SearchSessionMetricsManager>(profile_, notifier_);
federated_metrics_manager_ =
std::make_unique<FederatedMetricsManager>(notifier_);
federated_metrics_manager_ = std::make_unique<FederatedMetricsManager>(
notifier_, federated_service_controller_);
app_search_data_source_ = std::make_unique<AppSearchDataSource>(
profile_, list_controller_, base::DefaultClock::GetInstance());
}
Expand Down
11 changes: 10 additions & 1 deletion chrome/browser/ash/app_list/search/search_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ class Profile;

namespace ash {
class AppListNotifier;

namespace federated {
class FederatedServiceController;
} // namespace federated

} // namespace ash

namespace app_list {
Expand All @@ -53,7 +58,9 @@ class SearchController {
SearchController(AppListModelUpdater* model_updater,
AppListControllerDelegate* list_controller,
ash::AppListNotifier* notifier,
Profile* profile);
Profile* profile,
ash::federated::FederatedServiceController*
federated_service_controller_);
virtual ~SearchController();

SearchController(const SearchController&) = delete;
Expand Down Expand Up @@ -209,6 +216,8 @@ class SearchController {
const raw_ptr<AppListModelUpdater> model_updater_;
const raw_ptr<AppListControllerDelegate> list_controller_;
const raw_ptr<ash::AppListNotifier> notifier_;
const raw_ptr<ash::federated::FederatedServiceController>
federated_service_controller_;

base::ObserverList<Observer> observer_list_;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,11 @@ std::unique_ptr<SearchController> CreateSearchController(
Profile* profile,
AppListModelUpdater* model_updater,
AppListControllerDelegate* list_controller,
ash::AppListNotifier* notifier) {
ash::AppListNotifier* notifier,
ash::federated::FederatedServiceController* federated_service_controller) {
auto controller = std::make_unique<SearchController>(
model_updater, list_controller, notifier, profile);
model_updater, list_controller, notifier, profile,
federated_service_controller);
controller->Initialize();

// Add search providers.
Expand Down
10 changes: 8 additions & 2 deletions chrome/browser/ash/app_list/search/search_controller_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@ class Profile;

namespace ash {
class AppListNotifier;
}

namespace federated {
class FederatedServiceController;
} // namespace federated

} // namespace ash

namespace app_list {

Expand All @@ -25,7 +30,8 @@ std::unique_ptr<SearchController> CreateSearchController(
Profile* profile,
AppListModelUpdater* model_updater,
AppListControllerDelegate* list_controller,
ash::AppListNotifier* notifier);
ash::AppListNotifier* notifier,
ash::federated::FederatedServiceController* federated_service_controller);

} // namespace app_list

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ class SearchControllerTest : public testing::Test {
search_controller_ = std::make_unique<SearchController>(
/*model_updater=*/&model_updater_,
/*list_controller=*/&list_controller_,
/*notifier=*/nullptr, &profile_);
/*notifier=*/nullptr, &profile_,
/*federated_service_controller_*/ nullptr);
search_controller_->Initialize();

auto ranker_manager = std::make_unique<TestRankerManager>(&profile_);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
namespace app_list {

TestSearchController::TestSearchController()
: SearchController(nullptr, nullptr, nullptr, nullptr) {}
: SearchController(nullptr, nullptr, nullptr, nullptr, nullptr) {}

TestSearchController::~TestSearchController() = default;

Expand Down

0 comments on commit 49ef586

Please sign in to comment.