Skip to content

Commit

Permalink
Modify the app metrics code to use the non mojom LaunchSource.
Browse files Browse the repository at this point in the history
BUG=1253250

Change-Id: I86b9f0da4b6b179030f5a75fedd0f89d86602a89
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3755184
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Nancy Wang <nancylingwang@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1024531}
  • Loading branch information
Nancy Wang authored and Chromium LUCI CQ committed Jul 15, 2022
1 parent f6bb11d commit 3c7cc7d
Show file tree
Hide file tree
Showing 14 changed files with 125 additions and 128 deletions.
7 changes: 3 additions & 4 deletions chrome/browser/apps/app_service/app_service_proxy_ash.cc
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ void AppServiceProxyAsh::OnAppRegistryCacheWillBeDestroyed(
void AppServiceProxyAsh::RecordAppPlatformMetrics(
Profile* profile,
const apps::AppUpdate& update,
apps::mojom::LaunchSource launch_source,
apps::LaunchSource launch_source,
apps::LaunchContainer container) {
RecordAppLaunchMetrics(profile, update.AppType(), update.AppId(),
launch_source, container);
Expand All @@ -593,9 +593,8 @@ void AppServiceProxyAsh::PerformPostUninstallTasks(
}

void AppServiceProxyAsh::PerformPostLaunchTasks(
apps::mojom::LaunchSource launch_source) {
if (apps_util::IsHumanLaunch(
ConvertMojomLaunchSourceToLaunchSource(launch_source))) {
apps::LaunchSource launch_source) {
if (apps_util::IsHumanLaunch(launch_source)) {
ash::full_restore::FullRestoreService::MaybeCloseNotification(profile_);
}
}
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/apps/app_service/app_service_proxy_ash.h
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,11 @@ class AppServiceProxyAsh : public AppServiceProxyBase,
void OnAppRegistryCacheWillBeDestroyed(
apps::AppRegistryCache* cache) override;

void PerformPostLaunchTasks(apps::mojom::LaunchSource launch_source) override;
void PerformPostLaunchTasks(apps::LaunchSource launch_source) override;

void RecordAppPlatformMetrics(Profile* profile,
const apps::AppUpdate& update,
apps::mojom::LaunchSource launch_source,
apps::LaunchSource launch_source,
apps::LaunchContainer container) override;

void InitAppPlatformMetrics();
Expand Down
37 changes: 21 additions & 16 deletions chrome/browser/apps/app_service/app_service_proxy_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -278,22 +278,24 @@ AppServiceProxyBase::LoadIconFromIconKey(AppType app_type,

void AppServiceProxyBase::Launch(const std::string& app_id,
int32_t event_flags,
apps::mojom::LaunchSource launch_source,
apps::mojom::LaunchSource mojom_launch_source,
apps::mojom::WindowInfoPtr window_info) {
if (app_service_.is_connected()) {
app_registry_cache_.ForOneApp(
app_id, [this, event_flags, launch_source,
app_id, [this, event_flags, mojom_launch_source,
&window_info](const apps::AppUpdate& update) {
if (MaybeShowLaunchPreventionDialog(update)) {
return;
}

apps::LaunchSource launch_source =
ConvertMojomLaunchSourceToLaunchSource(mojom_launch_source);
RecordAppLaunch(update.AppId(), launch_source);
RecordAppPlatformMetrics(profile_, update, launch_source,
apps::LaunchContainer::kLaunchContainerNone);

app_service_->Launch(ConvertAppTypeToMojomAppType(update.AppType()),
update.AppId(), event_flags, launch_source,
update.AppId(), event_flags, mojom_launch_source,
std::move(window_info));

PerformPostLaunchTasks(launch_source);
Expand All @@ -304,30 +306,32 @@ void AppServiceProxyBase::Launch(const std::string& app_id,
void AppServiceProxyBase::LaunchAppWithFiles(
const std::string& app_id,
int32_t event_flags,
apps::mojom::LaunchSource launch_source,
apps::mojom::LaunchSource mojom_launch_source,
apps::mojom::FilePathsPtr file_paths) {
if (app_service_.is_connected()) {
app_registry_cache_.ForOneApp(
app_id, [this, event_flags, launch_source,
app_id, [this, event_flags, mojom_launch_source,
&file_paths](const apps::AppUpdate& update) {
if (MaybeShowLaunchPreventionDialog(update)) {
return;
}

apps::LaunchSource launch_source =
ConvertMojomLaunchSourceToLaunchSource(mojom_launch_source);
RecordAppPlatformMetrics(profile_, update, launch_source,
apps::LaunchContainer::kLaunchContainerNone);

// TODO(crbug/1117655): File manager records metrics for apps it
// launched. So we only record launches from other places. We should
// eventually move those metrics here, after AppService supports all
// app types launched by file manager.
if (launch_source != apps::mojom::LaunchSource::kFromFileManager) {
if (launch_source != apps::LaunchSource::kFromFileManager) {
RecordAppLaunch(update.AppId(), launch_source);
}

app_service_->LaunchAppWithFiles(
ConvertAppTypeToMojomAppType(update.AppType()), update.AppId(),
event_flags, launch_source, std::move(file_paths));
event_flags, mojom_launch_source, std::move(file_paths));

PerformPostLaunchTasks(launch_source);
});
Expand All @@ -338,13 +342,13 @@ void AppServiceProxyBase::LaunchAppWithIntent(
const std::string& app_id,
int32_t event_flags,
apps::mojom::IntentPtr intent,
apps::mojom::LaunchSource launch_source,
apps::mojom::LaunchSource mojom_launch_source,
apps::mojom::WindowInfoPtr window_info,
apps::mojom::Publisher::LaunchAppWithIntentCallback callback) {
CHECK(intent);
if (app_service_.is_connected()) {
app_registry_cache_.ForOneApp(
app_id, [this, event_flags, &intent, launch_source, &window_info,
app_id, [this, event_flags, &intent, mojom_launch_source, &window_info,
callback = std::move(callback)](
const apps::AppUpdate& update) mutable {
if (MaybeShowLaunchPreventionDialog(update)) {
Expand All @@ -353,19 +357,21 @@ void AppServiceProxyBase::LaunchAppWithIntent(
return;
}

apps::LaunchSource launch_source =
ConvertMojomLaunchSourceToLaunchSource(mojom_launch_source);
// TODO(crbug/1117655): File manager records metrics for apps it
// launched. So we only record launches from other places. We should
// eventually move those metrics here, after AppService supports all
// app types launched by file manager.
if (launch_source != apps::mojom::LaunchSource::kFromFileManager) {
if (launch_source != apps::LaunchSource::kFromFileManager) {
RecordAppLaunch(update.AppId(), launch_source);
}
RecordAppPlatformMetrics(profile_, update, launch_source,
apps::LaunchContainer::kLaunchContainerNone);

app_service_->LaunchAppWithIntent(
ConvertAppTypeToMojomAppType(update.AppType()), update.AppId(),
event_flags, std::move(intent), launch_source,
event_flags, std::move(intent), mojom_launch_source,
std::move(window_info), std::move(callback));

PerformPostLaunchTasks(launch_source);
Expand Down Expand Up @@ -401,13 +407,12 @@ void AppServiceProxyBase::LaunchAppWithParams(AppLaunchParams&& params,
std::move(callback).Run(LaunchResult());
return;
}
auto launch_source =
ConvertLaunchSourceToMojomLaunchSource(params.launch_source);
auto launch_source = params.launch_source;
// TODO(crbug/1117655): File manager records metrics for apps it
// launched. So we only record launches from other places. We should
// eventually move those metrics here, after AppService supports all
// app types launched by file manager.
if (launch_source != apps::mojom::LaunchSource::kFromFileManager) {
if (launch_source != apps::LaunchSource::kFromFileManager) {
RecordAppLaunch(update.AppId(), launch_source);
}

Expand Down Expand Up @@ -866,12 +871,12 @@ apps::mojom::IntentFilterPtr AppServiceProxyBase::FindBestMatchingMojomFilter(
}

void AppServiceProxyBase::PerformPostLaunchTasks(
apps::mojom::LaunchSource launch_source) {}
apps::LaunchSource launch_source) {}

void AppServiceProxyBase::RecordAppPlatformMetrics(
Profile* profile,
const apps::AppUpdate& update,
apps::mojom::LaunchSource launch_source,
apps::LaunchSource launch_source,
apps::LaunchContainer container) {}

void AppServiceProxyBase::PerformPostUninstallTasks(
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/apps/app_service/app_service_proxy_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -389,11 +389,11 @@ class AppServiceProxyBase : public KeyedService,
apps::mojom::IntentFilterPtr FindBestMatchingMojomFilter(
const apps::mojom::IntentPtr& intent);

virtual void PerformPostLaunchTasks(apps::mojom::LaunchSource launch_source);
virtual void PerformPostLaunchTasks(apps::LaunchSource launch_source);

virtual void RecordAppPlatformMetrics(Profile* profile,
const apps::AppUpdate& update,
apps::mojom::LaunchSource launch_source,
apps::LaunchSource launch_source,
apps::LaunchContainer container);

virtual void PerformPostUninstallTasks(
Expand Down
16 changes: 6 additions & 10 deletions chrome/browser/apps/app_service/browser_app_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,13 @@ content::WebContents* LaunchAppWithParamsImpl(
web_app_launch_manager->OpenApplication(std::move(params));

if (!SessionID::IsValidValue(restore_id)) {
RecordAppLaunchMetrics(
profile, apps::AppType::kWeb, app_id,
ConvertLaunchSourceToMojomLaunchSource(launch_source), container);
RecordAppLaunchMetrics(profile, apps::AppType::kWeb, app_id,
launch_source, container);
return web_contents;
}

RecordAppLaunchMetrics(profile, apps::AppType::kWeb, app_id,
apps::mojom::LaunchSource::kFromFullRestore,
container);
apps::LaunchSource::kFromFullRestore, container);

int session_id = apps::GetSessionIdForRestoreFromWebContents(web_contents);
if (!SessionID::IsValidValue(session_id)) {
Expand All @@ -94,7 +92,7 @@ content::WebContents* LaunchAppWithParamsImpl(
// restore file.
if (SessionID::IsValidValue(params.restore_id)) {
RecordAppLaunchMetrics(profile, apps::AppType::kChromeApp, params.app_id,
apps::mojom::LaunchSource::kFromFullRestore,
apps::LaunchSource::kFromFullRestore,
params.container);

apps::AppLaunchParams params_for_restore(
Expand All @@ -109,10 +107,8 @@ content::WebContents* LaunchAppWithParamsImpl(
std::move(params_for_restore.intent));
full_restore::SaveAppLaunchInfo(profile->GetPath(), std::move(launch_info));
} else {
RecordAppLaunchMetrics(
profile, apps::AppType::kChromeApp, params.app_id,
ConvertLaunchSourceToMojomLaunchSource(params.launch_source),
params.container);
RecordAppLaunchMetrics(profile, apps::AppType::kChromeApp, params.app_id,
params.launch_source, params.container);
}
#endif

Expand Down
13 changes: 6 additions & 7 deletions chrome/browser/apps/app_service/metrics/app_platform_metrics.cc
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ apps::AppTypeNameV2 GetAppTypeNameV2(Profile* profile,

// Records the number of times Chrome OS apps are launched grouped by the launch
// source.
void RecordAppLaunchSource(apps::mojom::LaunchSource launch_source) {
void RecordAppLaunchSource(apps::LaunchSource launch_source) {
base::UmaHistogramEnumeration("Apps.AppLaunchSource", launch_source);
}

Expand Down Expand Up @@ -325,7 +325,7 @@ const std::set<apps::AppTypeName>& GetAppTypeNameSet() {
void RecordAppLaunchMetrics(Profile* profile,
AppType app_type,
const std::string& app_id,
apps::mojom::LaunchSource launch_source,
apps::LaunchSource launch_source,
apps::LaunchContainer container) {
if (app_type == AppType::kUnknown) {
return;
Expand Down Expand Up @@ -632,11 +632,10 @@ void AppPlatformMetrics::OnTwoHours() {
RecordAppsUsageTimeUkm();
}

void AppPlatformMetrics::RecordAppLaunchUkm(
AppType app_type,
const std::string& app_id,
apps::mojom::LaunchSource launch_source,
apps::LaunchContainer container) {
void AppPlatformMetrics::RecordAppLaunchUkm(AppType app_type,
const std::string& app_id,
apps::LaunchSource launch_source,
apps::LaunchContainer container) {
if (app_type == AppType::kUnknown || !ShouldRecordUkm(profile_)) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const std::set<apps::AppTypeName>& GetAppTypeNameSet();
void RecordAppLaunchMetrics(Profile* profile,
AppType app_type,
const std::string& app_id,
apps::mojom::LaunchSource launch_source,
apps::LaunchSource launch_source,
apps::LaunchContainer container);

class AppPlatformMetrics : public apps::AppRegistryCache::Observer,
Expand Down Expand Up @@ -127,7 +127,7 @@ class AppPlatformMetrics : public apps::AppRegistryCache::Observer,
// Records UKM when launching an app.
void RecordAppLaunchUkm(AppType app_type,
const std::string& app_id,
apps::mojom::LaunchSource launch_source,
apps::LaunchSource launch_source,
apps::LaunchContainer container);

// Records UKM when uninstalling an app.
Expand Down

0 comments on commit 3c7cc7d

Please sign in to comment.