Revert "[Files Ranking] Add a new Dolphin model for zero-state ranking"

This reverts commit ae792bea.

Reason for revert: Broke build
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8913531942624945040/+/steps/compile/0/stdout

Original change's description:
> [Files Ranking] Add a new Dolphin model for zero-state ranking
> 
> This CL adds a new Dolphin model to the search result ranker for zero
> state files ranking. Additionally, it:
> 
> 1. Adds a new Finch flag to guard the construction of the model.
> 2. Adds/renames some getters/setters for consistent naming: the app
>    model is an AppSearchResultRanker (Roselle) object, and the non-app
>    model is a SearchResultRanker (Dolphin) object.
> 
> This CL also makes a small change to the SearchController's members:
> the Mixer is moved above the SearchProviders. This is because, in
> future CLs, a search provider will hold a pointer for a model owned
> (indirectly) by the Mixer, and this ensures the model outlives the
> provider.
> 
> Bug: 959679
> Change-Id: Ib101463c61ed79657e380870eeb749e08188a131
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1604387
> Reviewed-by: Jenny Zhang <jennyz@chromium.org>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Reviewed-by: Jia Meng <jiameng@chromium.org>
> Commit-Queue: Tony Yeoman <tby@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#659273}

TBR=xiyuan@chromium.org,jennyz@chromium.org,jiameng@chromium.org,tby@chromium.org

Change-Id: Icebce6483275b23d55621d75bff1b85a7dbf1038
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 959679
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1609587Reviewed-by: Alexei Filippov <alph@chromium.org>
Commit-Queue: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659278}

ash/public/cpp/app_list/app_list_features.cc

@@ -33,8 +33,6 @@ const base::Feature kEnableZeroStateAppsRanker{
     "EnableZeroStateAppsRanker", base::FEATURE_ENABLED_BY_DEFAULT};
 const base::Feature kEnableQueryBasedMixedTypesRanker{
     "EnableQueryBasedMixedTypesRanker", base::FEATURE_DISABLED_BY_DEFAULT};
-const base::Feature kEnableZeroStateMixedTypesRanker{
-    "EnableZeroStateMixedTypesRanker", base::FEATURE_DISABLED_BY_DEFAULT};
 const base::Feature kEnableAppReinstallZeroState{
     "EnableAppReinstallZeroState", base::FEATURE_DISABLED_BY_DEFAULT};
 const base::Feature kEnableEmbeddedAssistantUI{
@@ -92,10 +90,6 @@ bool IsQueryBasedMixedTypesEnabled() {
   return base::FeatureList::IsEnabled(kEnableQueryBasedMixedTypesRanker);
 }
 
-bool IsZeroStateMixedTypesRankerEnabled() {
-  return base::FeatureList::IsEnabled(kEnableZeroStateMixedTypesRanker);
-}
-
 bool IsAppReinstallZeroStateEnabled() {
   return base::FeatureList::IsEnabled(kEnableAppReinstallZeroState);
 }

ash/public/cpp/app_list/app_list_features.h

@@ -53,9 +53,6 @@ ASH_PUBLIC_EXPORT extern const base::Feature kEnableZeroStateAppsRanker;
 // Enable an model that ranks query based non-apps result.
 ASH_PUBLIC_EXPORT extern const base::Feature kEnableQueryBasedMixedTypesRanker;
 
-// Enable a model that ranks zero-state files and recent queries.
-ASH_PUBLIC_EXPORT extern const base::Feature kEnableZeroStateMixedTypesRanker;
-
 // Enables the feature to include a single reinstallation candidate in
 // zero-state.
 ASH_PUBLIC_EXPORT extern const base::Feature kEnableAppReinstallZeroState;
@@ -80,7 +77,6 @@ bool ASH_PUBLIC_EXPORT IsAdaptiveResultRankerEnabled();
 bool ASH_PUBLIC_EXPORT IsQueryBasedAppsRankerEnabled();
 bool ASH_PUBLIC_EXPORT IsZeroStateAppsRankerEnabled();
 bool ASH_PUBLIC_EXPORT IsQueryBasedMixedTypesRankerEnabled();
-bool ASH_PUBLIC_EXPORT IsZeroStateMixedTypesRankerEnabled();
 bool ASH_PUBLIC_EXPORT IsAppReinstallZeroStateEnabled();
 bool ASH_PUBLIC_EXPORT IsEmbeddedAssistantUIEnabled();
 bool ASH_PUBLIC_EXPORT IsAppGridGhostEnabled();

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

@@ -156,24 +156,19 @@ void Mixer::RemoveDuplicates(SortedResults* results) {
 }
 
 void Mixer::FetchResults(const base::string16& query) {
-  if (non_app_ranker_)
-    non_app_ranker_->FetchRankings(query);
+  if (ranker_)
+    ranker_->FetchRankings(query);
   for (const auto& group : groups_)
-    group->FetchResults(non_app_ranker_.get());
+    group->FetchResults(ranker_.get());
 }
 
-void Mixer::SetNonAppSearchResultRanker(
-    std::unique_ptr<SearchResultRanker> ranker) {
-  non_app_ranker_ = std::move(ranker);
-}
-
-SearchResultRanker* Mixer::GetNonAppSearchResultRanker() {
-  return non_app_ranker_.get();
+void Mixer::SetSearchResultRanker(std::unique_ptr<SearchResultRanker> ranker) {
+  ranker_ = std::move(ranker);
 }
 
 void Mixer::Train(const std::string& id, RankingItemType type) {
-  if (non_app_ranker_)
-    non_app_ranker_->Train(id, type);
+  if (ranker_)
+    ranker_->Train(id, type);
 }
 
 }  // namespace app_list

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

@@ -50,13 +50,9 @@ class Mixer {
   // Collects the results, sorts and publishes them.
   void MixAndPublish(size_t num_max_results, const base::string16& query);
 
-  // Sets a SearchResultRanker to re-rank non-app search results before they are
+  // Sets a SearchResultRanker to re-rank search results before they are
   // published.
-  void SetNonAppSearchResultRanker(std::unique_ptr<SearchResultRanker> ranker);
-
-  // Get a pointer to the SearchResultRanker owned by this object used for all
-  // non-app ranking.
-  SearchResultRanker* GetNonAppSearchResultRanker();
+  void SetSearchResultRanker(std::unique_ptr<SearchResultRanker> ranker);
 
   // Handle a training signal.
   void Train(const std::string& id, RankingItemType type);
@@ -92,7 +88,7 @@ class Mixer {
   Groups groups_;
 
   // Adaptive models used for re-ranking search results.
-  std::unique_ptr<SearchResultRanker> non_app_ranker_;
+  std::unique_ptr<SearchResultRanker> ranker_;
 
   DISALLOW_COPY_AND_ASSIGN(Mixer);
 };

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

@@ -51,13 +51,10 @@ SearchController::SearchController(AppListModelUpdater* model_updater,
                                    AppListControllerDelegate* list_controller,
                                    Profile* profile)
     : mixer_(std::make_unique<Mixer>(model_updater)),
-      app_ranker_(std::make_unique<AppSearchResultRanker>(
+      ranker_(std::make_unique<AppSearchResultRanker>(
           profile->GetPath(),
           chromeos::ProfileHelper::IsEphemeralUserProfile(profile))),
-      list_controller_(list_controller) {
-  mixer_->SetNonAppSearchResultRanker(
-      std::make_unique<SearchResultRanker>(profile));
-}
+      list_controller_(list_controller) {}
 
 SearchController::~SearchController() {}
 
@@ -153,12 +150,9 @@ ChromeSearchResult* SearchController::GetResultByTitleForTest(
   return nullptr;
 }
 
-AppSearchResultRanker* SearchController::GetAppSearchResultRanker() {
-  return app_ranker_.get();
-}
-
-SearchResultRanker* SearchController::GetNonAppSearchResultRanker() {
-  return mixer_->GetNonAppSearchResultRanker();
+void SearchController::SetSearchResultRanker(
+    std::unique_ptr<SearchResultRanker> ranker) {
+  mixer_->SetSearchResultRanker(std::move(ranker));
 }
 
 void SearchController::Train(const std::string& id, RankingItemType type) {
@@ -182,4 +176,8 @@ void SearchController::Train(const std::string& id, RankingItemType type) {
   mixer_->Train(id, type);
 }
 
+AppSearchResultRanker* SearchController::GetSearchResultRanker() {
+  return ranker_.get();
+}
+
 }  // namespace app_list

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

@@ -54,16 +54,16 @@ class SearchController {
   ChromeSearchResult* FindSearchResult(const std::string& result_id);
   ChromeSearchResult* GetResultByTitleForTest(const std::string& title);
 
+  // Sets a SearchResultRanker to re-rank search results before they are
+  // published. The Mixer owned by the SearchController will take ownership of
+  // |ranker|.
+  void SetSearchResultRanker(std::unique_ptr<SearchResultRanker> ranker);
+
   // Sends training signal to each |providers_|
   void Train(const std::string& id, RankingItemType type);
 
-  // Gets the search result ranker owned by this object that is used for ranking
-  // apps.
-  AppSearchResultRanker* GetAppSearchResultRanker();
-
-  // Gets the search result ranker owned by the Mixer that is used for all
-  // other ranking.
-  SearchResultRanker* GetNonAppSearchResultRanker();
+  // Get the app search result ranker owned by this object.
+  AppSearchResultRanker* GetSearchResultRanker();
 
  private:
   // Invoked when the search results are changed.
@@ -77,10 +77,10 @@ class SearchController {
   // The query associated with the most recent search.
   base::string16 last_query_;
 
-  std::unique_ptr<Mixer> mixer_;
   using Providers = std::vector<std::unique_ptr<SearchProvider>>;
   Providers providers_;
-  std::unique_ptr<AppSearchResultRanker> app_ranker_;
+  std::unique_ptr<Mixer> mixer_;
+  std::unique_ptr<AppSearchResultRanker> ranker_;
   AppListControllerDelegate* list_controller_;
 
   DISALLOW_COPY_AND_ASSIGN(SearchController);

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

@@ -75,8 +75,7 @@ std::unique_ptr<SearchController> CreateSearchController(
   std::unique_ptr<SearchController> controller =
       std::make_unique<SearchController>(model_updater, list_controller,
                                          profile);
-
-  AppSearchResultRanker* app_ranker = controller->GetAppSearchResultRanker();
+  AppSearchResultRanker* ranker = controller->GetSearchResultRanker();
 
   // Add mixer groups. There are four main groups: answer card, apps
   // and omnibox. Each group has a "soft" maximum number of results. However, if
@@ -95,7 +94,7 @@ std::unique_ptr<SearchController> CreateSearchController(
   controller->AddProvider(apps_group_id, std::make_unique<AppSearchProvider>(
                                              profile, list_controller,
                                              base::DefaultClock::GetInstance(),
-                                             model_updater, app_ranker));
+                                             model_updater, ranker));
   controller->AddProvider(omnibox_group_id, std::make_unique<OmniboxProvider>(
                                                 profile, list_controller));
   if (app_list_features::IsAnswerCardEnabled()) {
@@ -156,7 +155,7 @@ std::unique_ptr<SearchController> CreateSearchController(
     controller->AddProvider(
         app_shortcut_group_id,
         std::make_unique<ArcAppShortcutsSearchProvider>(
-            kMaxAppShortcutResults, profile, list_controller, app_ranker));
+            kMaxAppShortcutResults, profile, list_controller, ranker));
   }
 
   // TODO(https://crbug.com/921429): Put feature switch in ash/public/app_list/
@@ -169,6 +168,9 @@ std::unique_ptr<SearchController> CreateSearchController(
         std::make_unique<CrostiniRepositorySearchProvider>(profile));
   }
 
+  controller->SetSearchResultRanker(
+      std::make_unique<SearchResultRanker>(profile));
+
   return controller;
 }
 

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

@@ -49,9 +49,7 @@ Model ModelForType(RankingItemType type) {
 
 }  // namespace
 
-SearchResultRanker::SearchResultRanker(Profile* profile)
-    : enable_zero_state_mixed_types_(
-          app_list_features::IsZeroStateMixedTypesRankerEnabled()) {
+SearchResultRanker::SearchResultRanker(Profile* profile) {
   if (app_list_features::IsAdaptiveResultRankerEnabled()) {
     RecurrenceRankerConfigProto config;
     config.set_min_seconds_between_saves(240u);
@@ -75,40 +73,18 @@ SearchResultRanker::SearchResultRanker(Profile* profile)
   }
 
   profile_ = profile;
-  if (enable_zero_state_mixed_types_) {
-    if (auto* notifier =
-            file_manager::file_tasks::FileTasksNotifier::GetForProfile(
-                profile_)) {
-      notifier->AddObserver(this);
-    }
-
-    RecurrenceRankerConfigProto config;
-    config.set_min_seconds_between_saves(240u);
-    auto* predictor = config.mutable_zero_state_frecency_predictor();
-    predictor->set_target_limit(base::GetFieldTrialParamByFeatureAsInt(
-        app_list_features::kEnableZeroStateMixedTypesRanker, "target_limit",
-        200));
-    predictor->set_decay_coeff(base::GetFieldTrialParamByFeatureAsDouble(
-        app_list_features::kEnableZeroStateMixedTypesRanker, "decay_coeff",
-        0.8f));
-    auto* fallback = config.mutable_fallback_predictor();
-    fallback->set_target_limit(200);
-    fallback->set_decay_coeff(0.8);
-
-    zero_state_mixed_types_ranker_ = std::make_unique<RecurrenceRanker>(
-        profile->GetPath().AppendASCII("zero_state_mixed_types_ranker.proto"),
-        config, chromeos::ProfileHelper::IsEphemeralUserProfile(profile));
-  }
+  if (auto* notifier =
+          file_manager::file_tasks::FileTasksNotifier::GetForProfile(profile_))
+    notifier->AddObserver(this);
+  /*file_tasks_observer_.Add(
+      file_manager::file_tasks::FileTasksNotifierFactory::GetInstance()
+          ->GetForProfile(profile));*/
 }
 
 SearchResultRanker::~SearchResultRanker() {
-  if (enable_zero_state_mixed_types_) {
-    if (auto* notifier =
-            file_manager::file_tasks::FileTasksNotifier::GetForProfile(
-                profile_)) {
-      notifier->RemoveObserver(this);
-    }
-  }
+  if (auto* notifier =
+          file_manager::file_tasks::FileTasksNotifier::GetForProfile(profile_))
+    notifier->RemoveObserver(this);
 }
 
 void SearchResultRanker::FetchRankings(const base::string16& query) {
@@ -165,15 +141,7 @@ void SearchResultRanker::Train(const std::string& id, RankingItemType type) {
 
 void SearchResultRanker::OnFilesOpened(
     const std::vector<FileOpenEvent>& file_opens) {
-  if (enable_zero_state_mixed_types_) {
-    DCHECK(zero_state_mixed_types_ranker_);
-    for (const auto& file_open : file_opens)
-      zero_state_mixed_types_ranker_->Record(file_open.path.value());
-  }
-}
-
-RecurrenceRanker* SearchResultRanker::get_zero_state_mixed_types_ranker() {
-  return zero_state_mixed_types_ranker_.get();
+  // TODO(959679): route file open events to a model as training signals.
 }
 
 }  // namespace app_list

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

@@ -51,8 +51,6 @@ class SearchResultRanker : file_manager::file_tasks::FileTasksObserver {
   // file_manager::file_tasks::FileTaskObserver:
   void OnFilesOpened(const std::vector<FileOpenEvent>& file_opens) override;
 
-  RecurrenceRanker* get_zero_state_mixed_types_ranker();
-
  private:
   // Records the time of the last call to FetchRankings() and is used to
   // limit the number of queries to the models within a short timespan.
@@ -70,15 +68,13 @@ class SearchResultRanker : file_manager::file_tasks::FileTasksObserver {
   // affect apps.
   std::unique_ptr<RecurrenceRanker> results_list_group_ranker_;
 
-  // Ranks files and previous queries for launcher zero-state.
-  std::unique_ptr<RecurrenceRanker> zero_state_mixed_types_ranker_;
-
   // TODO(931149): Move the AppSearchResultRanker instance and associated logic
   // to here.
 
   Profile* profile_;
-
-  const bool enable_zero_state_mixed_types_;
+  /*ScopedObserver<file_manager::file_tasks::FileTasksNotifier,
+                 file_manager::file_tasks::FileTasksObserver>
+      file_tasks_observer_{this};*/
 };
 
 }  // namespace app_list