-
Notifications
You must be signed in to change notification settings - Fork 6.7k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[Launcher health] Encapsulate burn-in logic in new BurnInController.
***Old*** Burn-in logic was bundled with and spread throughout the SearchControllerImplNew (SCIN). The SCIN has become large and complex. ***New*** Separate out burn-in logic into a BurnInController (BIC). ***Summary of changes*** This is intended as purely a refactor, with no logic changes. 1) Introduce BIC, which is owned by the SCIN. Largely copy-paste changes: (2 to 5) 2) Move burn-in related members from SCIN to BIC. 3) Move starting logic from SCIN::StartSearch() to BIC::Start(). 4) Move interruption logic from SCIN::StartZeroState to BIC::Stop(). 5) Move result updating logic from SCIN::SetSearchResults() to BIC::UpdateResults(). 6) Adjust plumbing of callback for when burn-in period elapses. Part of the go/launcher-health effort. Test: none added; refactor only Bug: 1279686, 1258415 Change-Id: I7bee528f622bc6f1e8044b30e9272db108a726fe Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3578193 Commit-Queue: Amanda Deacon <amandadeacon@chromium.org> Reviewed-by: Rachel Wong <wrong@chromium.org> Cr-Commit-Position: refs/heads/main@{#1001366}
- Loading branch information
Amanda Deacon
authored and
Chromium LUCI CQ
committed
May 10, 2022
1 parent
998d826
commit f6164b5
Showing
6 changed files
with
197 additions
and
85 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| // Copyright 2022 The Chromium Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| #include "chrome/browser/ui/app_list/search/burnin_controller.h" | ||
|
|
||
| #include "ash/constants/ash_features.h" | ||
| #include "base/containers/flat_set.h" | ||
| #include "base/metrics/field_trial_params.h" | ||
| #include "chrome/browser/ui/app_list/search/chrome_search_result.h" | ||
|
|
||
| namespace app_list { | ||
|
|
||
| BurnInController::BurnInController(BurnInPeriodElapsedCallback callback) | ||
| : burnin_period_elapsed_callback_(std::move(callback)), | ||
| burnin_period_(base::Milliseconds(base::GetFieldTrialParamByFeatureAsInt( | ||
| ash::features::kProductivityLauncher, | ||
| "burnin_length_ms", | ||
| 200))) {} | ||
|
|
||
| bool BurnInController::is_post_burnin() { | ||
| return base::Time::Now() - session_start_ > burnin_period_; | ||
| } | ||
|
|
||
| void BurnInController::Start() { | ||
| burnin_timer_.Start(FROM_HERE, burnin_period_, | ||
| burnin_period_elapsed_callback_); | ||
|
|
||
| session_start_ = base::Time::Now(); | ||
| burnin_iteration_counter_ = 0; | ||
| ids_to_burnin_iteration_.clear(); | ||
| } | ||
|
|
||
| BurnInController::~BurnInController() {} | ||
|
|
||
| void BurnInController::Stop() { | ||
| burnin_timer_.Stop(); | ||
| } | ||
|
|
||
| void BurnInController::UpdateResults(ResultsMap& results, | ||
| CategoriesList& categories, | ||
| ash::AppListSearchResultType result_type) { | ||
| if (is_post_burnin()) | ||
| ++burnin_iteration_counter_; | ||
|
|
||
| // Record the burn-in iteration number for categories we are seeing for the | ||
| // first time in this search. | ||
| base::flat_set<Category> updated_categories; | ||
| for (const auto& result : results[result_type]) | ||
| updated_categories.insert(result->category()); | ||
| for (auto& category : categories) { | ||
| const auto it = updated_categories.find(category.category); | ||
| if (it != updated_categories.end() && category.burnin_iteration == -1) | ||
| category.burnin_iteration = burnin_iteration_counter_; | ||
| } | ||
|
|
||
| // Record-keeping for the burn-in iteration number of individual results. | ||
| const auto it = results.find(result_type); | ||
| DCHECK(it != results.end()); | ||
|
|
||
| for (const auto& result : it->second) { | ||
| const std::string result_id = result->id(); | ||
| if (ids_to_burnin_iteration_.find(result_id) != | ||
| ids_to_burnin_iteration_.end()) { | ||
| // Result has been seen before. Set burnin_iteration, since the result | ||
| // object has changed since last seen. | ||
| result->scoring().burnin_iteration = ids_to_burnin_iteration_[result_id]; | ||
| } else { | ||
| result->scoring().burnin_iteration = burnin_iteration_counter_; | ||
| ids_to_burnin_iteration_[result_id] = burnin_iteration_counter_; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| } // namespace app_list |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| // Copyright 2022 The Chromium Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| #ifndef CHROME_BROWSER_UI_APP_LIST_SEARCH_BURNIN_CONTROLLER_H_ | ||
| #define CHROME_BROWSER_UI_APP_LIST_SEARCH_BURNIN_CONTROLLER_H_ | ||
|
|
||
| #include "ash/public/cpp/app_list/app_list_types.h" | ||
| #include "base/callback.h" | ||
| #include "base/containers/flat_map.h" | ||
| #include "base/time/time.h" | ||
| #include "base/timer/timer.h" | ||
| #include "chrome/browser/ui/app_list/search/ranking/types.h" | ||
| #include "chrome/browser/ui/app_list/search/search_controller.h" | ||
|
|
||
| namespace app_list { | ||
|
|
||
| // Manages operations related to the burn-in period. Owned by the | ||
| // SearchControllerImplNew. | ||
| class BurnInController { | ||
| public: | ||
| using BurnInPeriodElapsedCallback = base::RepeatingCallback<void()>; | ||
|
|
||
| explicit BurnInController(BurnInPeriodElapsedCallback); | ||
| ~BurnInController(); | ||
|
|
||
| BurnInController(const BurnInController&) = delete; | ||
| BurnInController& operator=(const BurnInController&) = delete; | ||
|
|
||
| // True if the burn-in period has elapsed. | ||
| bool is_post_burnin(); | ||
|
|
||
| // Called at the beginning of a query search. Initiates the burn-in/ period. | ||
| void Start(); | ||
|
|
||
| // Called to stop/interrupt a previous call to Start(). | ||
| // | ||
| // Stops the burn-in timer, thereby preventing any now unneeded calls to | ||
| // BurnInPeriodElapsedCallback. Currently, the only use case for this is when | ||
| // a query search is succeeded by a zero-state search. | ||
| void Stop(); | ||
|
|
||
| // Called when new result information has become available. | ||
| // | ||
| // Performs house-keeping related to burn-in iteration numbers for categories | ||
| // and individual results. These are later important for sorting purposes in | ||
| // the SearchControllerImplNew - see further documentation below. | ||
| // | ||
| // Triggers the BurnInPeriodElapsedCallback if it is the first time | ||
| // UpdateResults() has been called since the burn-in period has elapsed. | ||
| void UpdateResults(ResultsMap& results, | ||
| CategoriesList& categories, | ||
| ash::AppListSearchResultType result_type); | ||
|
|
||
| const base::flat_map<std::string, int>& ids_to_burnin_iteration_for_test() { | ||
| return ids_to_burnin_iteration_; | ||
| } | ||
|
|
||
| private: | ||
| // The time when Start was most recently called. | ||
| base::Time session_start_; | ||
|
|
||
| // Called when the burn-in period has elapsed. | ||
| BurnInPeriodElapsedCallback burnin_period_elapsed_callback_; | ||
|
|
||
| // The period of time ("burn-in") to wait before publishing a first collection | ||
| // of search results to the model updater. | ||
| const base::TimeDelta burnin_period_; | ||
|
|
||
| // A timer for the burn-in period. During the burn-in period, results are | ||
| // collected from search providers. Publication of results to the model | ||
| // updater is delayed until the burn-in period has elapsed. | ||
| base::RetainingOneShotTimer burnin_timer_; | ||
|
|
||
| // Counter for burn-in iterations. Useful for query search only. | ||
| // | ||
| // Zero signifies pre-burn-in state. After burn-in period has elapsed, counter | ||
| // is incremented by one each time SetResults() is called. This burn-in | ||
| // iteration number is used for individual results as well as overall | ||
| // categories. | ||
| // | ||
| // This information is useful because it allows for: | ||
| // | ||
| // (1) Results and categories to be ranked by different rules depending on | ||
| // whether the information arrived pre- or post-burn-in. | ||
| // (2) Sorting stability across multiple post-burn-in updates. | ||
| int burnin_iteration_counter_ = 0; | ||
|
|
||
| // Store the ID of each result we encounter in a given query, along with the | ||
| // burn-in iteration during which it arrived. This storage is necessary | ||
| // because: | ||
| // | ||
| // Some providers may return more than once, and on each successive return, | ||
| // the previous results are swapped for new ones within SetResults(). Result | ||
| // meta-information we wish to persist across multiple calls to SetResults | ||
| // must therefore be stored separately. | ||
| base::flat_map<std::string, int> ids_to_burnin_iteration_; | ||
| }; | ||
|
|
||
| } // namespace app_list | ||
|
|
||
| #endif // CHROME_BROWSER_UI_APP_LIST_SEARCH_BURNIN_CONTROLLER_H_ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters