Skip to content

Commit

Permalink
Add KeyDataProvider interface to provide project keys.
Browse files Browse the repository at this point in the history
This refactor will be used to decouple the requirement for a user to
login to record and use StructuredMetrics in Chromium by using the
device key.

Bug: b/290096302
Change-Id: I160281df9f9d4c37da56507c652daf63adcac6a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4662392
Reviewed-by: Hirthanan Subenderan <hirthanan@google.com>
Commit-Queue: Jong Ahn <jongahn@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1184868}
  • Loading branch information
Jong Ahn authored and Chromium LUCI CQ committed Aug 17, 2023
1 parent 3c6f47b commit 08828db
Show file tree
Hide file tree
Showing 18 changed files with 537 additions and 121 deletions.
2 changes: 2 additions & 0 deletions chrome/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -4878,6 +4878,8 @@ static_library("browser") {
"metrics/structured/ash_structured_metrics_recorder.h",
"metrics/structured/cros_events_processor.cc",
"metrics/structured/cros_events_processor.h",
"metrics/structured/key_data_provider_ash.cc",
"metrics/structured/key_data_provider_ash.h",
"metrics/structured/metadata_processor_ash.cc",
"metrics/structured/metadata_processor_ash.h",
"metrics/structured/structured_metrics_key_events_observer.cc",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,18 @@ class AshStructuredMetricsRecorderTest : public MixinBasedInProcessBrowserTest {
->SetOnEventsRecordClosure(delegate);
}

void AddProfile() {
structured_metrics_mixin_.GetTestStructuredMetricsProvider()
->AddProfilePath(base::FilePath(FILE_PATH_LITERAL("structured_metrics"))
.Append(FILE_PATH_LITERAL("user_keys")));
}

void WaitUntilInit() {
// Wait for keys to be initialized and state to be propagated async.
structured_metrics_mixin_.GetTestStructuredMetricsProvider()
->WaitUntilReady();
}

protected:
StructuredMetricsMixin structured_metrics_mixin_{&mixin_host_};

Expand All @@ -72,6 +84,11 @@ class AshStructuredMetricsRecorderTest : public MixinBasedInProcessBrowserTest {

IN_PROC_BROWSER_TEST_F(AshStructuredMetricsRecorderTest,
SendValidEventAndSuccessfullyRecords) {
// Add a profile otherwise event will not be hashed. Then wait for
// initialization to occur.
AddProfile();
WaitUntilInit();

events::v2::test_project_one::TestEventOne test_event;
test_event.SetTestMetricOne("hash").SetTestMetricTwo(1);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,18 @@
#include <stdint.h>

#include "base/no_destructor.h"
#include "base/task/sequenced_task_runner.h"
#include "components/metrics/structured/histogram_util.h"
#include "components/metrics/structured/recorder.h"
#include "components/metrics/structured/structured_metrics_features.h"
#include "components/metrics_services_manager/metrics_services_manager.h"

#if BUILDFLAG(IS_CHROMEOS_ASH)
#include "chrome/browser/browser_process.h" // nogncheck
#include "chrome/browser/metrics/structured/ash_structured_metrics_recorder.h" // nogncheck
#include "chrome/browser/metrics/structured/cros_events_processor.h" // nogncheck
#include "chrome/browser/metrics/structured/key_data_provider_ash.h" // nogncheck
#include "chrome/browser/metrics/structured/metadata_processor_ash.h" // nogncheck
#include "components/metrics/structured/structured_metrics_recorder.h" // nogncheck
#include "components/metrics/structured/structured_metrics_service.h" // nogncheck
#elif BUILDFLAG(IS_CHROMEOS_LACROS)
#include "base/task/current_thread.h"
#include "chrome/browser/metrics/structured/lacros_structured_metrics_recorder.h" // nogncheck
Expand Down Expand Up @@ -81,6 +83,12 @@ void ChromeStructuredMetricsRecorder::Initialize() {
cros_event::kResetCounterPath));
}

// Initialize the key data provider.
g_browser_process->GetMetricsServicesManager()
->GetStructuredMetricsService()
->recorder()
->InitializeKeyDataProvider(std::make_unique<KeyDataProviderAsh>());

Recorder::GetInstance()->AddEventsProcessor(
std::make_unique<MetadataProcessorAsh>());

Expand Down
84 changes: 84 additions & 0 deletions chrome/browser/metrics/structured/key_data_provider_ash.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/metrics/structured/key_data_provider_ash.h"

namespace metrics::structured {
namespace {
// The delay period for the PersistentProto.
constexpr int kSaveDelayMs = 1000;

// The path used to store per-profile keys. Relative to the user's
// cryptohome. This file is created by chromium.
constexpr char kProfileKeyPath[] = "structured_metrics/keys";

// The path used to store per-device keys. This file is created by tmpfiles.d
// on start and has its permissions and ownership set such that it is writable
// by chronos.
constexpr char kDeviceKeyPath[] = "/var/lib/metrics/structured/chromium/keys";

} // namespace

KeyDataProviderAsh::KeyDataProviderAsh()
: KeyDataProviderAsh(base::FilePath(kDeviceKeyPath), kSaveDelayMs) {}

KeyDataProviderAsh::KeyDataProviderAsh(const base::FilePath& device_key_path,
int write_delay_ms)
: device_key_path_(device_key_path), write_delay_ms_(write_delay_ms) {}

KeyDataProviderAsh::~KeyDataProviderAsh() = default;

void KeyDataProviderAsh::InitializeDeviceKey(base::OnceClosure callback) {
if (HasDeviceKey()) {
return;
}

device_key_ = std::make_unique<KeyData>(device_key_path_,
base::Milliseconds(write_delay_ms_),
std::move(callback));
}

void KeyDataProviderAsh::InitializeProfileKey(
const base::FilePath& profile_path,
base::OnceClosure callback) {
// Only the primary user's keys should be loaded. If there is already is a
// profile key, no-op.
if (HasProfileKey()) {
return;
}

profile_key_ = std::make_unique<KeyData>(profile_path.Append(kProfileKeyPath),
base::Milliseconds(write_delay_ms_),
std::move(callback));
}

KeyData* KeyDataProviderAsh::GetDeviceKeyData() {
DCHECK(HasDeviceKey());
return device_key_.get();
}

KeyData* KeyDataProviderAsh::GetProfileKeyData() {
DCHECK(HasProfileKey());
return profile_key_.get();
}

void KeyDataProviderAsh::Purge() {
if (HasDeviceKey()) {
GetDeviceKeyData()->Purge();
}

if (HasProfileKey()) {
GetProfileKeyData()->Purge();
}
}

bool KeyDataProviderAsh::HasProfileKey() {
return profile_key_ != nullptr;
}

bool KeyDataProviderAsh::HasDeviceKey() {
return profile_key_ != nullptr;
}

} // namespace metrics::structured
44 changes: 44 additions & 0 deletions chrome/browser/metrics/structured/key_data_provider_ash.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CHROME_BROWSER_METRICS_STRUCTURED_KEY_DATA_PROVIDER_ASH_H_
#define CHROME_BROWSER_METRICS_STRUCTURED_KEY_DATA_PROVIDER_ASH_H_

#include "components/metrics/structured/key_data_provider.h"

namespace metrics::structured {

// Implementation for Ash Chrome to provide keys to StructuredMetricsRecorder.
//
// Device keys are stored in a static path while profile keys are stored in the
// user's cryptohome partition.
//
// InitializeProfileKey should only be called once for the primary user. All
// subsequent calls will no-op.
class KeyDataProviderAsh : public KeyDataProvider {
public:
KeyDataProviderAsh(const base::FilePath& device_key_path, int write_delay_ms);
KeyDataProviderAsh();
~KeyDataProviderAsh() override;

// KeyDataProvider:
void InitializeDeviceKey(base::OnceClosure callback) override;
void InitializeProfileKey(const base::FilePath& profile_path,
base::OnceClosure callback) override;
KeyData* GetDeviceKeyData() override;
KeyData* GetProfileKeyData() override;
void Purge() override;
bool HasProfileKey() override;
bool HasDeviceKey() override;

private:
const base::FilePath device_key_path_;
int write_delay_ms_;

std::unique_ptr<KeyData> device_key_;
std::unique_ptr<KeyData> profile_key_;
};
} // namespace metrics::structured

#endif // CHROME_BROWSER_METRICS_STRUCTURED_KEY_DATA_PROVIDER_ASH_H_
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,19 @@ namespace metrics::structured {

StructuredMetricsMixin::StructuredMetricsMixin(
InProcessBrowserTestMixinHost* host)
: InProcessBrowserTestMixin(host) {
test_structured_metrics_provider_ =
std::make_unique<TestStructuredMetricsProvider>();
}
: InProcessBrowserTestMixin(host) {}

StructuredMetricsMixin::~StructuredMetricsMixin() = default;

void StructuredMetricsMixin::SetUpCommandLine(base::CommandLine* command_line) {
EnableMetricsRecordingOnlyForTesting(command_line);
}

void StructuredMetricsMixin::SetUpOnMainThread() {
test_structured_metrics_provider_ =
std::make_unique<TestStructuredMetricsProvider>();
}

TestStructuredMetricsProvider*
StructuredMetricsMixin::GetTestStructuredMetricsProvider() {
return test_structured_metrics_provider_.get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class StructuredMetricsMixin : public InProcessBrowserTestMixin {

// InProcessBrowserTestMixin:
void SetUpCommandLine(base::CommandLine* command_line) override;
void SetUpOnMainThread() override;

private:
std::unique_ptr<TestStructuredMetricsProvider>
Expand Down
4 changes: 4 additions & 0 deletions components/metrics/structured/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ static_library("structured") {
"external_metrics.h",
"key_data.cc",
"key_data.h",
"key_data_provider.h",
"persistent_proto.cc",
"persistent_proto.h",
"reporting/structured_metrics_log_metrics.cc",
Expand Down Expand Up @@ -226,6 +227,8 @@ static_library("neutrino_logging_util") {
static_library("test_support") {
testonly = true
sources = [
"test/test_key_data_provider.cc",
"test/test_key_data_provider.h",
"test/test_structured_metrics_provider.cc",
"test/test_structured_metrics_provider.h",
]
Expand Down Expand Up @@ -254,6 +257,7 @@ source_set("unit_tests") {
":structured",
":structured_events",
":structured_metrics_validator",
":test_support",
"//base",
"//base/test:test_support",
"//components/metrics",
Expand Down
55 changes: 55 additions & 0 deletions components/metrics/structured/key_data_provider.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef COMPONENTS_METRICS_STRUCTURED_KEY_DATA_PROVIDER_H_
#define COMPONENTS_METRICS_STRUCTURED_KEY_DATA_PROVIDER_H_

#include "components/metrics/structured/key_data.h"
#include "third_party/abseil-cpp/absl/types/optional.h"

namespace metrics::structured {

// Interface to provide key data to be used for hashing projects.
//
// There are two types of keys: device keys and profile keys. Device keys will
// be ready only InitializeDeviceKey has been called while profile keys should
// be ready once InitializeProfileKey has been called.
class KeyDataProvider {
public:
KeyDataProvider() = default;

KeyDataProvider(const KeyDataProvider& key_data_provider) = delete;
KeyDataProvider& operator=(const KeyDataProvider& key_data_provider) = delete;

virtual ~KeyDataProvider() = default;

// Initializes the device key data.
virtual void InitializeDeviceKey(base::OnceClosure callback) = 0;

// Called whenever a profile key should be initialized.
virtual void InitializeProfileKey(const base::FilePath& profile_path,
base::OnceClosure callback) = 0;

// Returns the device key data.
//
// Returns nullptr if InitializeDeviceKey() has not been called or is in
// progress.
virtual KeyData* GetDeviceKeyData() = 0;

// Returns the profile key data, if available. A call to HasProfileKey()
// should guarantee that this value will not be nullptr.
//
// Returns nullptr otherwise.
virtual KeyData* GetProfileKeyData() = 0;

// Deletes all key data associated with the provider.
virtual void Purge() = 0;

virtual bool HasProfileKey() = 0;
virtual bool HasDeviceKey() = 0;
};

} // namespace metrics::structured

#endif // COMPONENTS_METRICS_STRUCTURED_KEY_DATA_PROVIDER_H_
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "components/metrics/structured/structured_events.h"
#include "components/metrics/structured/structured_metrics_features.h"
#include "components/metrics/structured/structured_metrics_recorder.h"
#include "components/metrics/structured/test/test_key_data_provider.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/metrics_proto/chrome_user_metrics_extension.pb.h"

Expand Down Expand Up @@ -101,9 +102,11 @@ class StructuredMetricsProviderTest : public testing::Test {
system_profile_provider_ = std::make_unique<TestSystemProfileProvider>();
// Create a system profile, normally done by ChromeMetricsServiceClient.
structured_metrics_recorder_ = std::unique_ptr<StructuredMetricsRecorder>(
new StructuredMetricsRecorder(DeviceKeyFilePath(),
/*write_delay=*/base::Seconds(0),
new StructuredMetricsRecorder(/*write_delay=*/base::Seconds(0),
system_profile_provider_.get()));
structured_metrics_recorder_->InitializeKeyDataProvider(
std::make_unique<TestKeyDataProvider>(DeviceKeyFilePath(),
ProfileKeyFilePath()));
// Create the provider, normally done by the ChromeMetricsServiceClient.
provider_ = std::unique_ptr<StructuredMetricsProvider>(
new StructuredMetricsProvider(
Expand Down

0 comments on commit 08828db

Please sign in to comment.