Skip to content

Commit

Permalink
Using the new policy for app usage telemetry reporting
Browse files Browse the repository at this point in the history 
This change updates metric reporting components and the app usage
observer to use the new user policy for app usage telemetry reporting.

Bug: b:260803048
Change-Id: I42fd6796b5b174d2dd1777512859645d02dad2ff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4546329
Reviewed-by: Ahmed Nasr <anasr@google.com>
Commit-Queue: Vignesh Shenvi <vshenvi@google.com>
Reviewed-by: Leonid Baraz <lbaraz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1146668}
  • Loading branch information
Vignesh Shenvi authored and Chromium LUCI CQ committed May 19, 2023
1 parent 3838160 commit 7026514
Show file tree
Hide file tree
Showing 11 changed files with 184 additions and 259 deletions.
31 changes: 12 additions & 19 deletions chrome/browser/ash/policy/reporting/metrics_reporting/apps/app_events_observer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ void AppEventsObserver::OnAppInstalled(const std::string& app_id,
::apps::InstallReason app_install_reason,
::apps::InstallTime app_install_time) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (!IsAppTypeAllowed(app_type)) {
DCHECK(reporting_settings_);
if (!::ash::reporting::IsAppTypeAllowed(
app_type, reporting_settings_.get(),
::ash::reporting::kReportAppInventory)) {
return;
}

Expand Down Expand Up @@ -98,7 +101,10 @@ void AppEventsObserver::OnAppLaunched(const std::string& app_id,
::apps::AppType app_type,
::apps::LaunchSource app_launch_source) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (!IsAppTypeAllowed(app_type)) {
DCHECK(reporting_settings_);
if (!::ash::reporting::IsAppTypeAllowed(
app_type, reporting_settings_.get(),
::ash::reporting::kReportAppInventory)) {
return;
}

Expand All @@ -123,7 +129,10 @@ void AppEventsObserver::OnAppUninstalled(
::apps::AppType app_type,
::apps::UninstallSource app_uninstall_source) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (!IsAppTypeAllowed(app_type)) {
DCHECK(reporting_settings_);
if (!::ash::reporting::IsAppTypeAllowed(
app_type, reporting_settings_.get(),
::ash::reporting::kReportAppInventory)) {
return;
}

Expand All @@ -148,20 +157,4 @@ void AppEventsObserver::OnAppPlatformMetricsDestroyed() {
observer_.Reset();
}

bool AppEventsObserver::IsAppTypeAllowed(::apps::AppType app_type) const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(reporting_settings_);
const base::Value::List* allowed_app_types = nullptr;
if (!reporting_settings_->GetList(::ash::reporting::kReportAppInventory,
&allowed_app_types)) {
// Policy likely not set. Disallow app inventory reporting regardless of app
// type.
return false;
}
const absl::optional<std::string> app_category =
::ash::reporting::GetAppReportingCategoryForType(app_type);
return app_category.has_value() &&
base::Contains(*allowed_app_types, app_category.value());
}

} // namespace reporting
4 changes: 0 additions & 4 deletions chrome/browser/ash/policy/reporting/metrics_reporting/apps/app_events_observer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,6 @@ class AppEventsObserver : public MetricEventObserver,
// ::apps::AppPlatformMetrics::Observer:
void OnAppPlatformMetricsDestroyed() override;

// Helper that determines if the specified app type is allowlisted for app
// inventory event reporting.
bool IsAppTypeAllowed(::apps::AppType app_type) const;

SEQUENCE_CHECKER(sequence_checker_);

// Retriever that retrieves the `AppPlatformMetrics` component so the
Expand Down
8 changes: 4 additions & 4 deletions chrome/browser/ash/policy/reporting/metrics_reporting/apps/app_usage_observer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
#include "base/time/time.h"
#include "chrome/browser/apps/app_service/metrics/app_platform_metrics.h"
#include "chrome/browser/ash/policy/reporting/metrics_reporting/apps/app_platform_metrics_retriever.h"
#include "chrome/browser/ash/policy/reporting/metrics_reporting/metric_reporting_prefs.h"
#include "chrome/browser/chromeos/reporting/metric_default_utils.h"
#include "chrome/browser/profiles/profile.h"
#include "chromeos/ash/components/settings/cros_settings_names.h"
#include "components/prefs/scoped_user_pref_update.h"
#include "components/services/app_service/public/cpp/app_launch_util.h"
#include "components/services/app_service/public/cpp/app_types.h"
Expand Down Expand Up @@ -76,9 +76,9 @@ void AppUsageObserver::OnAppUsage(const std::string& app_id,
const base::UnguessableToken& instance_id,
base::TimeDelta running_time) {
DCHECK(reporting_settings_);
auto is_enabled = metrics::kReportDeviceAppInfoDefaultValue;
reporting_settings_->GetBoolean(::ash::kReportDeviceAppInfo, &is_enabled);
if (!profile_ || !is_enabled) {
if (!profile_ ||
!::ash::reporting::IsAppTypeAllowed(app_type, reporting_settings_.get(),
::ash::reporting::kReportAppUsage)) {
return;
}

Expand Down
104 changes: 66 additions & 38 deletions chrome/browser/ash/policy/reporting/metrics_reporting/apps/app_usage_observer_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,17 @@
#include "chrome/browser/ash/policy/reporting/metrics_reporting/apps/app_usage_observer.h"

#include <memory>
#include <string>
#include <vector>

#include "base/json/values_util.h"
#include "base/memory/raw_ptr.h"
#include "base/time/time.h"
#include "base/unguessable_token.h"
#include "base/values.h"
#include "chrome/browser/apps/app_service/metrics/app_platform_metrics.h"
#include "chrome/browser/apps/app_service/metrics/app_platform_metrics_service_test_base.h"
#include "chromeos/ash/components/settings/cros_settings_names.h"
#include "chrome/browser/ash/policy/reporting/metrics_reporting/metric_reporting_prefs.h"
#include "components/reporting/metrics/fakes/fake_reporting_settings.h"
#include "components/reporting/proto/synced/metric_data.pb.h"
#include "components/services/app_service/public/cpp/app_types.h"
Expand Down Expand Up @@ -108,6 +111,15 @@ class AppUsageObserverTest : public ::apps::AppPlatformMetricsServiceTestBase {
Eq(expected_usage_time));
}

void SetAllowedAppReportingTypes(const std::vector<std::string>& app_types) {
base::Value::List allowed_app_types;
for (const auto& app_type : app_types) {
allowed_app_types.Append(app_type);
}
reporting_settings_.SetList(::ash::reporting::kReportAppUsage,
std::move(allowed_app_types));
}

// Fake reporting settings component used by the test.
test::FakeReportingSettings reporting_settings_;

Expand All @@ -116,43 +128,59 @@ class AppUsageObserverTest : public ::apps::AppPlatformMetricsServiceTestBase {
};

TEST_F(AppUsageObserverTest, PersistAppUsageDataInPrefStore) {
reporting_settings_.SetBoolean(::ash::kReportDeviceAppInfo, true);
SetAllowedAppReportingTypes({::ash::reporting::kAppCategoryAndroidApps});

// Create a new window for the app and simulate app usage.
static constexpr base::TimeDelta kAppUsageDuration = base::Minutes(2);
const auto window = std::make_unique<::aura::Window>(nullptr);
window->Init(::ui::LAYER_NOT_DRAWN);
::aura::Window window(nullptr);
window.Init(::ui::LAYER_NOT_DRAWN);
const base::UnguessableToken& kInstanceId = base::UnguessableToken::Create();
SimulateAppUsageForInstance(window.get(), kInstanceId, kAppUsageDuration);
SimulateAppUsageForInstance(&window, kInstanceId, kAppUsageDuration);

// Verify data is persisted in the pref store.
ASSERT_THAT(GetPrefService()->GetDict(::apps::kAppUsageTime).size(), Eq(1UL));
VerifyAppUsageDataInPrefStoreForInstance(kInstanceId, kAppUsageDuration);
}

TEST_F(AppUsageObserverTest, ShouldNotPersistMicrosecondUsageData) {
reporting_settings_.SetBoolean(::ash::kReportDeviceAppInfo, true);
SetAllowedAppReportingTypes({::ash::reporting::kAppCategoryAndroidApps});

// Create a new window for the app and simulate insignificant app usage.
static constexpr base::TimeDelta kAppUsageDuration = base::Microseconds(200);
const auto window = std::make_unique<::aura::Window>(nullptr);
window->Init(::ui::LAYER_NOT_DRAWN);
::aura::Window window(nullptr);
window.Init(::ui::LAYER_NOT_DRAWN);
const base::UnguessableToken& kInstanceId = base::UnguessableToken::Create();
SimulateAppUsageForInstance(window.get(), kInstanceId, kAppUsageDuration);
SimulateAppUsageForInstance(&window, kInstanceId, kAppUsageDuration);

// Verify data is not persisted in the pref store.
ASSERT_TRUE(GetPrefService()->GetDict(::apps::kAppUsageTime).empty());
}

TEST_F(AppUsageObserverTest, ShouldNotPersistAppUsageDataIfSettingDisabled) {
reporting_settings_.SetBoolean(::ash::kReportDeviceAppInfo, false);
TEST_F(AppUsageObserverTest, ShouldNotPersistAppUsageDataIfSettingUnset) {
// Create a new window for the app and simulate app usage.
static constexpr base::TimeDelta kAppUsageDuration = base::Minutes(2);
::aura::Window window(nullptr);
window.Init(::ui::LAYER_NOT_DRAWN);
const base::UnguessableToken& kInstanceId = base::UnguessableToken::Create();
SimulateAppUsageForInstance(&window, kInstanceId, kAppUsageDuration);

// Verify there is no data persisted to the pref store.
const auto& usage_dict_pref =
GetPrefService()->GetDict(::apps::kAppUsageTime);
ASSERT_TRUE(usage_dict_pref.empty());
}

TEST_F(AppUsageObserverTest, ShouldNotPersistAppUsageDataIfAppTypeDisallowed) {
// Set policy to enable reporting for a different app type than the one being
// used.
SetAllowedAppReportingTypes({::ash::reporting::kAppCategoryPWA});

// Create a new window for the app and simulate app usage.
static constexpr base::TimeDelta kAppUsageDuration = base::Minutes(2);
const auto window = std::make_unique<::aura::Window>(nullptr);
window->Init(::ui::LAYER_NOT_DRAWN);
::aura::Window window(nullptr);
window.Init(::ui::LAYER_NOT_DRAWN);
const base::UnguessableToken& kInstanceId = base::UnguessableToken::Create();
SimulateAppUsageForInstance(window.get(), kInstanceId, kAppUsageDuration);
SimulateAppUsageForInstance(&window, kInstanceId, kAppUsageDuration);

// Verify there is no data persisted to the pref store.
const auto& usage_dict_pref =
Expand All @@ -161,21 +189,21 @@ TEST_F(AppUsageObserverTest, ShouldNotPersistAppUsageDataIfSettingDisabled) {
}

TEST_F(AppUsageObserverTest, ShouldAggregateAppUsageDataOnSubsequentUsage) {
reporting_settings_.SetBoolean(::ash::kReportDeviceAppInfo, true);
SetAllowedAppReportingTypes({::ash::reporting::kAppCategoryAndroidApps});

// Create a new window for the app and simulate app usage.
static constexpr base::TimeDelta kAppUsageDuration = base::Minutes(2);
const auto window = std::make_unique<::aura::Window>(nullptr);
window->Init(::ui::LAYER_NOT_DRAWN);
::aura::Window window(nullptr);
window.Init(::ui::LAYER_NOT_DRAWN);
const base::UnguessableToken& kInstanceId = base::UnguessableToken::Create();
SimulateAppUsageForInstance(window.get(), kInstanceId, kAppUsageDuration);
SimulateAppUsageForInstance(&window, kInstanceId, kAppUsageDuration);

// Verify data is persisted to the pref store.
ASSERT_THAT(GetPrefService()->GetDict(::apps::kAppUsageTime).size(), Eq(1UL));
VerifyAppUsageDataInPrefStoreForInstance(kInstanceId, kAppUsageDuration);

// Simulate additional app usage.
SimulateAppUsageForInstance(window.get(), kInstanceId, kAppUsageDuration);
SimulateAppUsageForInstance(&window, kInstanceId, kAppUsageDuration);

// Verify aggregated usage data is persisted in the pref store.
const auto& expected_usage_duration = kAppUsageDuration + kAppUsageDuration;
Expand All @@ -186,56 +214,56 @@ TEST_F(AppUsageObserverTest, ShouldAggregateAppUsageDataOnSubsequentUsage) {

TEST_F(AppUsageObserverTest,
ShouldStopPersistingAppUsageDataWhenSettingDisabled) {
// Enable reporting setting initially.
reporting_settings_.SetBoolean(::ash::kReportDeviceAppInfo, true);
// Allow reporting for specified app type initially.
SetAllowedAppReportingTypes({::ash::reporting::kAppCategoryAndroidApps});

// Create a new window for the app and simulate app usage.
static constexpr base::TimeDelta kAppUsageDuration = base::Minutes(2);
const auto window = std::make_unique<::aura::Window>(nullptr);
window->Init(::ui::LAYER_NOT_DRAWN);
::aura::Window window(nullptr);
window.Init(::ui::LAYER_NOT_DRAWN);
const base::UnguessableToken& kInstanceId = base::UnguessableToken::Create();
SimulateAppUsageForInstance(window.get(), kInstanceId, kAppUsageDuration);
SimulateAppUsageForInstance(&window, kInstanceId, kAppUsageDuration);

// Verify data is persisted to the pref store.
ASSERT_THAT(GetPrefService()->GetDict(::apps::kAppUsageTime).size(), Eq(1UL));
VerifyAppUsageDataInPrefStoreForInstance(kInstanceId, kAppUsageDuration);

// Disable setting and simulate additional app usage.
reporting_settings_.SetBoolean(::ash::kReportDeviceAppInfo, false);
SimulateAppUsageForInstance(window.get(), kInstanceId, kAppUsageDuration);
// Disallow reporting and simulate additional app usage.
SetAllowedAppReportingTypes({});
SimulateAppUsageForInstance(&window, kInstanceId, kAppUsageDuration);

// Verify usage data remains unchanged in the pref store.
ASSERT_THAT(GetPrefService()->GetDict(::apps::kAppUsageTime).size(), Eq(1UL));
VerifyAppUsageDataInPrefStoreForInstance(kInstanceId, kAppUsageDuration);
}

TEST_F(AppUsageObserverTest, ShouldPersistAppUsageDataForNewInstance) {
reporting_settings_.SetBoolean(::ash::kReportDeviceAppInfo, true);
SetAllowedAppReportingTypes({::ash::reporting::kAppCategoryAndroidApps});

// Create a new window for the app and simulate app usage.
static constexpr base::TimeDelta kAppUsageDuration = base::Minutes(2);
const auto window = std::make_unique<::aura::Window>(nullptr);
window->Init(::ui::LAYER_NOT_DRAWN);
::aura::Window window(nullptr);
window.Init(::ui::LAYER_NOT_DRAWN);
const base::UnguessableToken& kInstanceId = base::UnguessableToken::Create();
SimulateAppUsageForInstance(window.get(), kInstanceId, kAppUsageDuration);
SimulateAppUsageForInstance(&window, kInstanceId, kAppUsageDuration);

// Verify data is persisted to the pref store.
ASSERT_THAT(GetPrefService()->GetDict(::apps::kAppUsageTime).size(), Eq(1UL));
VerifyAppUsageDataInPrefStoreForInstance(kInstanceId, kAppUsageDuration);

// Simulate app usage for a new instance of the same app.
const auto window1 = std::make_unique<::aura::Window>(nullptr);
window1->Init(::ui::LAYER_NOT_DRAWN);
::aura::Window window1(nullptr);
window1.Init(::ui::LAYER_NOT_DRAWN);
const base::UnguessableToken& kInstanceId1 = base::UnguessableToken::Create();
SimulateAppUsageForInstance(window1.get(), kInstanceId1, kAppUsageDuration);
SimulateAppUsageForInstance(&window1, kInstanceId1, kAppUsageDuration);

// Verify the component persists usage data for the new instance.
ASSERT_THAT(GetPrefService()->GetDict(::apps::kAppUsageTime).size(), Eq(2UL));
VerifyAppUsageDataInPrefStoreForInstance(kInstanceId1, kAppUsageDuration);
}

TEST_F(AppUsageObserverTest, OnAppPlatformMetricsDestroyed) {
reporting_settings_.SetBoolean(::ash::kReportDeviceAppInfo, true);
SetAllowedAppReportingTypes({::ash::reporting::kAppCategoryAndroidApps});

// Reset `AppPlatformMetricsService` to destroy the `AppPlatformMetrics`
// component.
Expand All @@ -244,10 +272,10 @@ TEST_F(AppUsageObserverTest, OnAppPlatformMetricsDestroyed) {
// Create a new window for the app and simulate app usage to verify the
// component is no longer tracking app usage metric.
static constexpr base::TimeDelta kAppUsageDuration = base::Minutes(2);
const auto window = std::make_unique<::aura::Window>(nullptr);
window->Init(::ui::LAYER_NOT_DRAWN);
::aura::Window window(nullptr);
window.Init(::ui::LAYER_NOT_DRAWN);
const base::UnguessableToken& kInstanceId = base::UnguessableToken::Create();
SimulateAppUsageForInstance(window.get(), kInstanceId, kAppUsageDuration);
SimulateAppUsageForInstance(&window, kInstanceId, kAppUsageDuration);

// Verify there is no data persisted to the pref store.
const auto& usage_dict_pref =
Expand Down

0 comments on commit 7026514

Please sign in to comment.