Commit b312d7fe authored by Paul Dyson's avatar Paul Dyson Committed by Commit Bot

move AppLaunchEventLogger to SearchResultRanker.

1. Move the AppLaunchEventLogger member variable from
AppListClientImpl to SearchResultRanker.

2. Move the app_launcher_event_logger* files from
chrome/browser/ui/app_list to
chrome/browser/ui/app_list/search/search_result_ranker.

Bug: 899123
Change-Id: I329d69b00facd3321335ba1cfbc2b86296339aff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1705459
Commit-Queue: Paul Dyson <pdyson@chromium.org>
Reviewed-by: default avatarcalamity <calamity@chromium.org>
Reviewed-by: default avatarCharles . <charleszhao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680321}
parent 6a5da7ce
......@@ -3341,8 +3341,6 @@ jumbo_split_static_library("ui") {
"app_list/app_context_menu.cc",
"app_list/app_context_menu.h",
"app_list/app_context_menu_delegate.h",
"app_list/app_launch_event_logger.cc",
"app_list/app_launch_event_logger.h",
"app_list/app_list_client_impl.cc",
"app_list/app_list_client_impl.h",
"app_list/app_list_controller_delegate.cc",
......@@ -3420,6 +3418,8 @@ jumbo_split_static_library("ui") {
"app_list/search/search_resource_manager.h",
"app_list/search/search_result_ranker/app_launch_data.cc",
"app_list/search/search_result_ranker/app_launch_data.h",
"app_list/search/search_result_ranker/app_launch_event_logger.cc",
"app_list/search/search_result_ranker/app_launch_event_logger.h",
"app_list/search/search_result_ranker/app_launch_predictor.cc",
"app_list/search/search_result_ranker/app_launch_predictor.h",
"app_list/search/search_result_ranker/app_list_launch_metrics_provider.cc",
......@@ -3457,8 +3457,8 @@ jumbo_split_static_library("ui") {
"//ash/app_list",
"//ash/public/cpp/app_list/vector_icons",
"//ash/resources/vector_icons",
"//chrome/browser/ui:app_launch_event_logger_proto",
"//chrome/browser/ui/app_list/search/logging:search_ranking_event_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_predictor_proto",
"//chrome/browser/ui/app_list/search/search_result_ranker:app_list_launch_recorder_proto",
"//chrome/browser/ui/app_list/search/search_result_ranker:recurrence_ranker_proto",
......@@ -4013,9 +4013,3 @@ if (is_android) {
]
}
}
proto_library("app_launch_event_logger_proto") {
sources = [
"app_list/app_launch_event_logger.proto",
]
}
......@@ -111,16 +111,11 @@ void AppListClientImpl::OpenSearchResult(const std::string& result_id,
app_launch_data.id = result_id;
app_launch_data.ranking_item_type =
app_list::RankingItemTypeFromSearchResult(*result);
app_launch_data.launch_type = launch_type;
app_launch_data.launched_from = launched_from;
// Send training signal to search controller.
search_controller_->Train(std::move(app_launch_data));
if (launch_type == ash::AppListLaunchType::kAppSearchResult) {
// Log the AppResult (either in the search result page, or in chip form in
// AppsGridView) to the UKM system.
app_launch_event_logger_.OnSuggestionChipOrSearchBoxClicked(
result_id, suggestion_index, static_cast<int>(launched_from));
}
RecordSearchResultOpenTypeHistogram(
launched_from, result->GetSearchResultType(), IsTabletMode());
......@@ -198,11 +193,10 @@ void AppListClientImpl::ActivateItem(int profile_id,
app_launch_data.id = id;
app_launch_data.ranking_item_type =
app_list::RankingItemTypeFromChromeAppListItem(*item);
app_launch_data.launched_from = ash::AppListLaunchedFrom::kLaunchedFromGrid;
search_controller_->Train(std::move(app_launch_data));
}
app_launch_event_logger_.OnGridClicked(id);
requested_model_updater->ActivateChromeItem(id, event_flags);
// Suspect that |id| may be destructed after calling ActivateChromeItem. Add
......
......@@ -20,7 +20,6 @@
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/scoped_observer.h"
#include "chrome/browser/ui/app_list/app_launch_event_logger.h"
#include "chrome/browser/ui/app_list/app_list_controller_delegate.h"
#include "chrome/browser/ui/app_list/search/logging/search_ranking_event_logger.h"
#include "components/search_engines/template_url_service.h"
......@@ -192,7 +191,6 @@ class AppListClientImpl
bool app_list_target_visibility_ = false;
bool app_list_visible_ = false;
app_list::AppLaunchEventLogger app_launch_event_logger_;
app_list::SearchRankingEventLogger search_ranking_event_logger_;
base::WeakPtrFactory<AppListClientImpl> weak_ptr_factory_{this};
......
......@@ -4,6 +4,12 @@
import("//third_party/protobuf/proto_library.gni")
proto_library("app_launch_event_logger_proto") {
sources = [
"app_launch_event_logger.proto",
]
}
proto_library("app_launch_predictor_proto") {
sources = [
"app_launch_predictor.proto",
......
......@@ -25,7 +25,7 @@ struct AppLaunchData {
ash::AppListLaunchedFrom launched_from =
ash::AppListLaunchedFrom::kLaunchedFromShelf;
// The type of app launched.
ash::AppListLaunchType launched_type = ash::AppListLaunchType::kSearchResult;
ash::AppListLaunchType launch_type = ash::AppListLaunchType::kSearchResult;
// The index of the suggested app launched, if applicable.
int suggestion_index = 0;
// User's search query string, if relevant.
......
......@@ -2,7 +2,7 @@
// 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/app_launch_event_logger.h"
#include "chrome/browser/ui/app_list/search/search_result_ranker/app_launch_event_logger.h"
#include <cmath>
#include <utility>
......
......@@ -2,8 +2,8 @@
// 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_APP_LAUNCH_EVENT_LOGGER_H_
#define CHROME_BROWSER_UI_APP_LIST_APP_LAUNCH_EVENT_LOGGER_H_
#ifndef CHROME_BROWSER_UI_APP_LIST_SEARCH_SEARCH_RESULT_RANKER_APP_LAUNCH_EVENT_LOGGER_H_
#define CHROME_BROWSER_UI_APP_LIST_SEARCH_SEARCH_RESULT_RANKER_APP_LAUNCH_EVENT_LOGGER_H_
#include <memory>
#include <string>
......@@ -14,7 +14,7 @@
#include "base/memory/weak_ptr.h"
#include "base/sequenced_task_runner.h"
#include "base/values.h"
#include "chrome/browser/ui/app_list/app_launch_event_logger.pb.h"
#include "chrome/browser/ui/app_list/search/search_result_ranker/app_launch_event_logger.pb.h"
#include "extensions/browser/extension_registry.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
......@@ -150,4 +150,4 @@ class AppLaunchEventLogger {
} // namespace app_list
#endif // CHROME_BROWSER_UI_APP_LIST_APP_LAUNCH_EVENT_LOGGER_H_
#endif // CHROME_BROWSER_UI_APP_LIST_SEARCH_SEARCH_RESULT_RANKER_APP_LAUNCH_EVENT_LOGGER_H_
......@@ -2,7 +2,7 @@
// 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/app_launch_event_logger.h"
#include "chrome/browser/ui/app_list/search/search_result_ranker/app_launch_event_logger.h"
#include <memory>
......
......@@ -254,6 +254,19 @@ void SearchResultRanker::Rank(Mixer::SortedResults* results) {
}
void SearchResultRanker::Train(const AppLaunchData& app_launch_data) {
if (app_launch_data.launched_from ==
ash::AppListLaunchedFrom::kLaunchedFromGrid) {
// Log the AppResult from the grid to the UKM system.
app_launch_event_logger_.OnGridClicked(app_launch_data.id);
} else if (app_launch_data.launch_type ==
ash::AppListLaunchType::kAppSearchResult) {
// Log the AppResult (either in the search result page, or in chip form in
// AppsGridView) to the UKM system.
app_launch_event_logger_.OnSuggestionChipOrSearchBoxClicked(
app_launch_data.id, app_launch_data.suggestion_index,
static_cast<int>(app_launch_data.launched_from));
}
if (ModelForType(app_launch_data.ranking_item_type) == Model::MIXED_TYPES) {
if (results_list_group_ranker_) {
results_list_group_ranker_->Record(base::NumberToString(
......
......@@ -20,6 +20,7 @@
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/app_list/search/mixer.h"
#include "chrome/browser/ui/app_list/search/search_result_ranker/app_launch_data.h"
#include "chrome/browser/ui/app_list/search/search_result_ranker/app_launch_event_logger.h"
#include "chrome/browser/ui/app_list/search/search_result_ranker/recurrence_ranker_util.h"
namespace app_list {
......@@ -108,6 +109,9 @@ class SearchResultRanker : file_manager::file_tasks::FileTasksObserver {
// Testing-only closure to inform tests once a JSON config has been parsed.
base::OnceClosure json_config_parsed_for_testing_;
// Logs launch events and stores feature data for aggregated model.
app_list::AppLaunchEventLogger app_launch_event_logger_;
// TODO(931149): Move the AppSearchResultRanker instance and associated logic
// to here.
......
......@@ -4959,7 +4959,6 @@ test("unit_tests") {
if (enable_app_list) {
sources += [
"../browser/ui/app_list/app_context_menu_unittest.cc",
"../browser/ui/app_list/app_launch_event_logger_unittest.cc",
"../browser/ui/app_list/app_list_syncable_service_unittest.cc",
"../browser/ui/app_list/app_list_test_util.cc",
"../browser/ui/app_list/app_list_test_util.h",
......@@ -4981,6 +4980,7 @@ test("unit_tests") {
"../browser/ui/app_list/search/arc/arc_app_shortcuts_search_provider_unittest.cc",
"../browser/ui/app_list/search/arc/arc_playstore_search_provider_unittest.cc",
"../browser/ui/app_list/search/launcher_search/launcher_search_icon_image_loader_unittest.cc",
"../browser/ui/app_list/search/search_result_ranker/app_launch_event_logger_unittest.cc",
"../browser/ui/app_list/search/search_result_ranker/app_launch_predictor_unittest.cc",
"../browser/ui/app_list/search/search_result_ranker/app_list_launch_metrics_provider_unittest.cc",
"../browser/ui/app_list/search/search_result_ranker/app_search_result_ranker_unittest.cc",
......
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