Commit 87dac882 authored by Alexei Svitkine's avatar Alexei Svitkine Committed by Commit Bot

Add full system profiles in .pma file as part of metrics init.

Prior to this change, only an incomplete system profile, notably
missing field trials and uma_default_state fields, was stored
at this point. So unclean shut downs after this point in time,
but before the initial metrics log was saved would not include
that info. This change fixes that by running the code that
populates all the system profile fields.

Since we're now calling ProvideSystemProfileMetrics() twice, the
logic in FieldTrialsProvider was no longer correct in tracking
the log creation time. Instead, this CL updates the API to have
that be passed in.

Additionally, Chrome OS an Screen Info providers needed updates since
their dependencies are not yet available this early, so they are not
able to provide all the information this early.

Finally, webview unit tests needed updating to init mojo as they are
now exercising the provider code which has a Mojo dependency.

Bug: 992538
TBR: sky@chromium.org
Change-Id: Iae54398065470e390b70507d5b3ac09d5a983c9f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1746909
Commit-Queue: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: default avatarRichard Coles <torne@chromium.org>
Reviewed-by: default avatarRobert Kaplow <rkaplow@chromium.org>
Reviewed-by: default avatarBrian White <bcwhite@chromium.org>
Cr-Commit-Position: refs/heads/master@{#687000}
parent 4f76b773
...@@ -9,6 +9,7 @@ include_rules = [ ...@@ -9,6 +9,7 @@ include_rules = [
"+gin/public", "+gin/public",
"+gin/v8_initializer.h", "+gin/v8_initializer.h",
"+media/base/media_switches.h", # For media command line switches. "+media/base/media_switches.h", # For media command line switches.
"+mojo/core/embedder/embedder.h",
"+ui/events/gesture_detection", "+ui/events/gesture_detection",
"+ui/gl", "+ui/gl",
] ]
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include "base/command_line.h" #include "base/command_line.h"
#include "base/test/test_suite.h" #include "base/test/test_suite.h"
#include "content/public/common/content_switches.h" #include "content/public/common/content_switches.h"
#include "mojo/core/embedder/embedder.h"
#include "ui/gl/gl_surface.h" #include "ui/gl/gl_surface.h"
#include "ui/gl/test/gl_surface_test_support.h" #include "ui/gl/test/gl_surface_test_support.h"
...@@ -14,5 +15,7 @@ int main(int argc, char** argv) { ...@@ -14,5 +15,7 @@ int main(int argc, char** argv) {
switches::kSingleProcess); switches::kSingleProcess);
gl::GLSurfaceTestSupport::InitializeNoExtensionsOneOff(); gl::GLSurfaceTestSupport::InitializeNoExtensionsOneOff();
android_webview::GpuServiceWebView::GetInstance(); android_webview::GpuServiceWebView::GetInstance();
return base::TestSuite(argc, argv).Run(); base::TestSuite test_suite(argc, argv);
mojo::core::Init();
return test_suite.Run();
} }
...@@ -353,6 +353,7 @@ test("android_webview_unittests") { ...@@ -353,6 +353,7 @@ test("android_webview_unittests") {
"//components/prefs:test_support", "//components/prefs:test_support",
"//content:content", "//content:content",
"//content/test:test_support", "//content/test:test_support",
"//mojo/core/embedder",
"//net:net", "//net:net",
"//net:test_support", "//net:test_support",
"//ui/base:ui_base_jni_headers", "//ui/base:ui_base_jni_headers",
......
...@@ -309,6 +309,11 @@ void ChromeOSMetricsProvider::ProvideCurrentSessionData( ...@@ -309,6 +309,11 @@ void ChromeOSMetricsProvider::ProvideCurrentSessionData(
void ChromeOSMetricsProvider::WriteBluetoothProto( void ChromeOSMetricsProvider::WriteBluetoothProto(
metrics::SystemProfileProto* system_profile_proto) { metrics::SystemProfileProto* system_profile_proto) {
// This may be called before the async init task to set |adapter_| is set,
// such as when the persistent system profile gets filled in initially.
if (!adapter_)
return;
metrics::SystemProfileProto::Hardware* hardware = metrics::SystemProfileProto::Hardware* hardware =
system_profile_proto->mutable_hardware(); system_profile_proto->mutable_hardware();
......
...@@ -78,7 +78,8 @@ void StartupData::RecordCoreSystemProfile() { ...@@ -78,7 +78,8 @@ void StartupData::RecordCoreSystemProfile() {
// |field_trial_provider|. // |field_trial_provider|.
variations::FieldTrialsProvider field_trial_provider(nullptr, variations::FieldTrialsProvider field_trial_provider(nullptr,
base::StringPiece()); base::StringPiece());
field_trial_provider.ProvideSystemProfileMetrics(&system_profile); field_trial_provider.ProvideSystemProfileMetricsWithLogCreationTime(
base::TimeTicks(), &system_profile);
// TODO(crbug.com/965482): Records information from other providers. // TODO(crbug.com/965482): Records information from other providers.
metrics::GlobalPersistentSystemProfile::GetInstance()->SetSystemProfile( metrics::GlobalPersistentSystemProfile::GetInstance()->SetSystemProfile(
......
...@@ -63,8 +63,17 @@ bool DelegatingProvider::HasIndependentMetrics() { ...@@ -63,8 +63,17 @@ bool DelegatingProvider::HasIndependentMetrics() {
void DelegatingProvider::ProvideSystemProfileMetrics( void DelegatingProvider::ProvideSystemProfileMetrics(
SystemProfileProto* system_profile_proto) { SystemProfileProto* system_profile_proto) {
for (auto& provider : metrics_providers_) // ProvideSystemProfileMetricsWithLogCreationTime() should be called instead.
provider->ProvideSystemProfileMetrics(system_profile_proto); NOTREACHED();
}
void DelegatingProvider::ProvideSystemProfileMetricsWithLogCreationTime(
base::TimeTicks log_creation_time,
SystemProfileProto* system_profile_proto) {
for (auto& provider : metrics_providers_) {
provider->ProvideSystemProfileMetricsWithLogCreationTime(
log_creation_time, system_profile_proto);
}
} }
bool DelegatingProvider::HasPreviousSessionData() { bool DelegatingProvider::HasPreviousSessionData() {
......
...@@ -36,6 +36,9 @@ class DelegatingProvider final : public MetricsProvider { ...@@ -36,6 +36,9 @@ class DelegatingProvider final : public MetricsProvider {
bool HasIndependentMetrics() override; bool HasIndependentMetrics() override;
void ProvideSystemProfileMetrics( void ProvideSystemProfileMetrics(
SystemProfileProto* system_profile_proto) override; SystemProfileProto* system_profile_proto) override;
void ProvideSystemProfileMetricsWithLogCreationTime(
base::TimeTicks log_creation_time,
SystemProfileProto* system_profile_proto) override;
bool HasPreviousSessionData() override; bool HasPreviousSessionData() override;
void ProvidePreviousSessionData( void ProvidePreviousSessionData(
ChromeUserMetricsExtension* uma_proto) override; ChromeUserMetricsExtension* uma_proto) override;
......
...@@ -36,13 +36,14 @@ void FieldTrialsProvider::GetFieldTrialIds( ...@@ -36,13 +36,14 @@ void FieldTrialsProvider::GetFieldTrialIds(
variations::GetFieldTrialActiveGroupIds(suffix_, field_trial_ids); variations::GetFieldTrialActiveGroupIds(suffix_, field_trial_ids);
} }
void FieldTrialsProvider::OnDidCreateMetricsLog() { void FieldTrialsProvider::ProvideSystemProfileMetrics(
if (registry_) { metrics::SystemProfileProto* system_profile_proto) {
creation_times_.push_back(base::TimeTicks::Now()); // ProvideSystemProfileMetricsWithLogCreationTime() should be called instead.
} NOTREACHED();
} }
void FieldTrialsProvider::ProvideSystemProfileMetrics( void FieldTrialsProvider::ProvideSystemProfileMetricsWithLogCreationTime(
base::TimeTicks log_creation_time,
metrics::SystemProfileProto* system_profile_proto) { metrics::SystemProfileProto* system_profile_proto) {
std::vector<ActiveGroupId> field_trial_ids; std::vector<ActiveGroupId> field_trial_ids;
const std::string& version = variations::GetSeedVersion(); const std::string& version = variations::GetSeedVersion();
...@@ -52,14 +53,8 @@ void FieldTrialsProvider::ProvideSystemProfileMetrics( ...@@ -52,14 +53,8 @@ void FieldTrialsProvider::ProvideSystemProfileMetrics(
WriteFieldTrials(field_trial_ids, system_profile_proto); WriteFieldTrials(field_trial_ids, system_profile_proto);
if (registry_) { if (registry_) {
base::TimeTicks creation_time;
// Should always be true, but don't crash even if there is a bug.
if (!creation_times_.empty()) {
creation_time = creation_times_.back();
creation_times_.pop_back();
}
std::vector<ActiveGroupId> synthetic_trials; std::vector<ActiveGroupId> synthetic_trials;
registry_->GetSyntheticFieldTrialsOlderThan(creation_time, registry_->GetSyntheticFieldTrialsOlderThan(log_creation_time,
&synthetic_trials); &synthetic_trials);
WriteFieldTrials(synthetic_trials, system_profile_proto); WriteFieldTrials(synthetic_trials, system_profile_proto);
} }
......
...@@ -5,8 +5,6 @@ ...@@ -5,8 +5,6 @@
#ifndef COMPONENTS_METRICS_FIELD_TRIALS_PROVIDER_H_ #ifndef COMPONENTS_METRICS_FIELD_TRIALS_PROVIDER_H_
#define COMPONENTS_METRICS_FIELD_TRIALS_PROVIDER_H_ #define COMPONENTS_METRICS_FIELD_TRIALS_PROVIDER_H_
#include <vector>
#include "base/strings/string_piece.h" #include "base/strings/string_piece.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "components/metrics/metrics_provider.h" #include "components/metrics/metrics_provider.h"
...@@ -27,9 +25,11 @@ class FieldTrialsProvider : public metrics::MetricsProvider { ...@@ -27,9 +25,11 @@ class FieldTrialsProvider : public metrics::MetricsProvider {
~FieldTrialsProvider() override; ~FieldTrialsProvider() override;
// metrics::MetricsProvider: // metrics::MetricsProvider:
void OnDidCreateMetricsLog() override;
void ProvideSystemProfileMetrics( void ProvideSystemProfileMetrics(
metrics::SystemProfileProto* system_profile_proto) override; metrics::SystemProfileProto* system_profile_proto) override;
void ProvideSystemProfileMetricsWithLogCreationTime(
base::TimeTicks log_creation_time,
metrics::SystemProfileProto* system_profile_proto) override;
private: private:
// Overrideable for testing. // Overrideable for testing.
...@@ -41,13 +41,7 @@ class FieldTrialsProvider : public metrics::MetricsProvider { ...@@ -41,13 +41,7 @@ class FieldTrialsProvider : public metrics::MetricsProvider {
// Suffix used for the field trial names before they are hashed for uploads. // Suffix used for the field trial names before they are hashed for uploads.
std::string suffix_; std::string suffix_;
// A stack of log creation times. DISALLOW_COPY_AND_ASSIGN(FieldTrialsProvider);
// While the initial metrics log exists, there will be two logs open.
// Use a stack so that we use the right creation time for the first ongoing
// log.
// TODO(crbug/746098): Simplify InitialMetricsLog logic so this is not
// necessary.
std::vector<base::TimeTicks> creation_times_;
}; };
} // namespace variations } // namespace variations
......
...@@ -96,14 +96,17 @@ TEST_F(FieldTrialsProviderTest, ProvideSyntheticTrials) { ...@@ -96,14 +96,17 @@ TEST_F(FieldTrialsProviderTest, ProvideSyntheticTrials) {
// Make sure these trials are older than the log. // Make sure these trials are older than the log.
WaitUntilTimeChanges(base::TimeTicks::Now()); WaitUntilTimeChanges(base::TimeTicks::Now());
provider.OnDidCreateMetricsLog(); // Get the current time and wait for it to change.
base::TimeTicks log_creation_time = base::TimeTicks::Now();
// Make sure that the log is older than the trials that should be excluded. // Make sure that the log is older than the trials that should be excluded.
WaitUntilTimeChanges(base::TimeTicks::Now()); WaitUntilTimeChanges(log_creation_time);
RegisterExtraSyntheticTrial(); RegisterExtraSyntheticTrial();
metrics::SystemProfileProto proto; metrics::SystemProfileProto proto;
provider.ProvideSystemProfileMetrics(&proto); provider.ProvideSystemProfileMetricsWithLogCreationTime(log_creation_time,
&proto);
ASSERT_EQ(base::size(kFieldTrialIds) + base::size(kSyntheticTrials), ASSERT_EQ(base::size(kFieldTrialIds) + base::size(kSyntheticTrials),
static_cast<size_t>(proto.field_trial_size())); static_cast<size_t>(proto.field_trial_size()));
...@@ -115,7 +118,8 @@ TEST_F(FieldTrialsProviderTest, NoSyntheticTrials) { ...@@ -115,7 +118,8 @@ TEST_F(FieldTrialsProviderTest, NoSyntheticTrials) {
TestProvider provider(nullptr, base::StringPiece()); TestProvider provider(nullptr, base::StringPiece());
metrics::SystemProfileProto proto; metrics::SystemProfileProto proto;
provider.ProvideSystemProfileMetrics(&proto); provider.ProvideSystemProfileMetricsWithLogCreationTime(base::TimeTicks(),
&proto);
ASSERT_EQ(base::size(kFieldTrialIds), ASSERT_EQ(base::size(kFieldTrialIds),
static_cast<size_t>(proto.field_trial_size())); static_cast<size_t>(proto.field_trial_size()));
......
...@@ -297,7 +297,10 @@ void MetricsLog::WriteRealtimeStabilityAttributes( ...@@ -297,7 +297,10 @@ void MetricsLog::WriteRealtimeStabilityAttributes(
const SystemProfileProto& MetricsLog::RecordEnvironment( const SystemProfileProto& MetricsLog::RecordEnvironment(
DelegatingProvider* delegating_provider) { DelegatingProvider* delegating_provider) {
DCHECK(!has_environment_); // Clear any previous system profile. This will be present in the case when
// the initial system profile got recorded on start up. Clearing it ensures
// there is no duplication of repeated fields.
uma_proto()->clear_system_profile();
has_environment_ = true; has_environment_ = true;
SystemProfileProto* system_profile = uma_proto()->mutable_system_profile(); SystemProfileProto* system_profile = uma_proto()->mutable_system_profile();
...@@ -309,7 +312,8 @@ const SystemProfileProto& MetricsLog::RecordEnvironment( ...@@ -309,7 +312,8 @@ const SystemProfileProto& MetricsLog::RecordEnvironment(
if (client_->GetBrand(&brand_code)) if (client_->GetBrand(&brand_code))
system_profile->set_brand_code(brand_code); system_profile->set_brand_code(brand_code);
delegating_provider->ProvideSystemProfileMetrics(system_profile); delegating_provider->ProvideSystemProfileMetricsWithLogCreationTime(
creation_time_, system_profile);
return *system_profile; return *system_profile;
} }
......
...@@ -48,7 +48,12 @@ void MetricsProvider::ProvideIndependentMetrics( ...@@ -48,7 +48,12 @@ void MetricsProvider::ProvideIndependentMetrics(
} }
void MetricsProvider::ProvideSystemProfileMetrics( void MetricsProvider::ProvideSystemProfileMetrics(
SystemProfileProto* system_profile_proto) {}
void MetricsProvider::ProvideSystemProfileMetricsWithLogCreationTime(
base::TimeTicks log_creation_time,
SystemProfileProto* system_profile_proto) { SystemProfileProto* system_profile_proto) {
ProvideSystemProfileMetrics(system_profile_proto);
} }
bool MetricsProvider::HasPreviousSessionData() { bool MetricsProvider::HasPreviousSessionData() {
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "base/callback.h" #include "base/callback.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/time/time.h"
namespace base { namespace base {
class HistogramSnapshotManager; class HistogramSnapshotManager;
...@@ -64,10 +65,19 @@ class MetricsProvider { ...@@ -64,10 +65,19 @@ class MetricsProvider {
ChromeUserMetricsExtension* uma_proto, ChromeUserMetricsExtension* uma_proto,
base::HistogramSnapshotManager* snapshot_manager); base::HistogramSnapshotManager* snapshot_manager);
// Provides additional metrics into the system profile. // Provides additional metrics into the system profile. This is a convenience
// method over ProvideSystemProfileMetricsWithLogCreationTime() without the
// |log_creation_time| param. Should not be called directly by services.
virtual void ProvideSystemProfileMetrics( virtual void ProvideSystemProfileMetrics(
SystemProfileProto* system_profile_proto); SystemProfileProto* system_profile_proto);
// Provides additional metrics into the system profile. The log creation
// time param provides a timestamp of when the log was opened, which is needed
// for some metrics providers.
virtual void ProvideSystemProfileMetricsWithLogCreationTime(
base::TimeTicks log_creation_time,
SystemProfileProto* system_profile_proto);
// Called once at startup to see whether this provider has critical data // Called once at startup to see whether this provider has critical data
// to provide about the previous session. // to provide about the previous session.
// Returning true will trigger ProvidePreviousSessionData on all other // Returning true will trigger ProvidePreviousSessionData on all other
......
...@@ -316,16 +316,17 @@ void MetricsService::EnableRecording() { ...@@ -316,16 +316,17 @@ void MetricsService::EnableRecording() {
state_manager_->ForceClientIdCreation(); state_manager_->ForceClientIdCreation();
client_->SetMetricsClientId(state_manager_->client_id()); client_->SetMetricsClientId(state_manager_->client_id());
SystemProfileProto system_profile;
MetricsLog::RecordCoreSystemProfile(client_, &system_profile);
GlobalPersistentSystemProfile::GetInstance()->SetSystemProfile(
system_profile, /*complete=*/false);
if (!log_manager_.current_log()) if (!log_manager_.current_log())
OpenNewLog(); OpenNewLog();
delegating_provider_.OnRecordingEnabled(); delegating_provider_.OnRecordingEnabled();
// Fill in the system profile in the log and persist it (to prefs, .pma and
// crashpad). This includes running the providers so that information like
// field trials and hardware info is provided. If Chrome crashes before this
// log is completed, the .pma file will have this system profile.
RecordCurrentEnvironment(log_manager_.current_log(), /*complete=*/false);
base::RemoveActionCallback(action_callback_); base::RemoveActionCallback(action_callback_);
action_callback_ = base::Bind(&MetricsService::OnUserAction, action_callback_ = base::Bind(&MetricsService::OnUserAction,
base::Unretained(this)); base::Unretained(this));
...@@ -620,7 +621,7 @@ void MetricsService::CloseCurrentLog() { ...@@ -620,7 +621,7 @@ void MetricsService::CloseCurrentLog() {
// MetricsLog class. // MetricsLog class.
MetricsLog* current_log = log_manager_.current_log(); MetricsLog* current_log = log_manager_.current_log();
DCHECK(current_log); DCHECK(current_log);
RecordCurrentEnvironment(current_log); RecordCurrentEnvironment(current_log, /*complete=*/true);
base::TimeDelta incremental_uptime; base::TimeDelta incremental_uptime;
base::TimeDelta uptime; base::TimeDelta uptime;
GetUptimes(local_state_, &incremental_uptime, &uptime); GetUptimes(local_state_, &incremental_uptime, &uptime);
...@@ -752,7 +753,7 @@ bool MetricsService::PrepareInitialStabilityLog( ...@@ -752,7 +753,7 @@ bool MetricsService::PrepareInitialStabilityLog(
void MetricsService::PrepareInitialMetricsLog() { void MetricsService::PrepareInitialMetricsLog() {
DCHECK_EQ(INIT_TASK_DONE, state_); DCHECK_EQ(INIT_TASK_DONE, state_);
RecordCurrentEnvironment(initial_metrics_log_.get()); RecordCurrentEnvironment(initial_metrics_log_.get(), /*complete=*/true);
base::TimeDelta incremental_uptime; base::TimeDelta incremental_uptime;
base::TimeDelta uptime; base::TimeDelta uptime;
GetUptimes(local_state_, &incremental_uptime, &uptime); GetUptimes(local_state_, &incremental_uptime, &uptime);
...@@ -806,6 +807,14 @@ std::unique_ptr<MetricsLog> MetricsService::CreateLog( ...@@ -806,6 +807,14 @@ std::unique_ptr<MetricsLog> MetricsService::CreateLog(
log_type, client_); log_type, client_);
} }
void MetricsService::SetPersistentSystemProfile(
const std::string& serialized_proto,
bool complete) {
GlobalPersistentSystemProfile::GetInstance()->SetSystemProfile(
serialized_proto, complete);
}
// static
std::string MetricsService::RecordCurrentEnvironmentHelper( std::string MetricsService::RecordCurrentEnvironmentHelper(
MetricsLog* log, MetricsLog* log,
PrefService* local_state, PrefService* local_state,
...@@ -816,12 +825,12 @@ std::string MetricsService::RecordCurrentEnvironmentHelper( ...@@ -816,12 +825,12 @@ std::string MetricsService::RecordCurrentEnvironmentHelper(
return recorder.SerializeAndRecordEnvironmentToPrefs(system_profile); return recorder.SerializeAndRecordEnvironmentToPrefs(system_profile);
} }
void MetricsService::RecordCurrentEnvironment(MetricsLog* log) { void MetricsService::RecordCurrentEnvironment(MetricsLog* log, bool complete) {
DCHECK(client_); DCHECK(client_);
std::string serialized_proto = std::string serialized_proto =
RecordCurrentEnvironmentHelper(log, local_state_, &delegating_provider_); RecordCurrentEnvironmentHelper(log, local_state_, &delegating_provider_);
GlobalPersistentSystemProfile::GetInstance()->SetSystemProfile(
serialized_proto, /*complete=*/true); SetPersistentSystemProfile(serialized_proto, complete);
client_->OnEnvironmentUpdate(&serialized_proto); client_->OnEnvironmentUpdate(&serialized_proto);
} }
......
...@@ -180,6 +180,10 @@ class MetricsService : public base::HistogramFlattener { ...@@ -180,6 +180,10 @@ class MetricsService : public base::HistogramFlattener {
return reporting_service_.metrics_log_store(); return reporting_service_.metrics_log_store();
} }
// Sets the persistent system profile. Virtual for tests.
virtual void SetPersistentSystemProfile(const std::string& serialized_proto,
bool complete);
// Records the current environment (system profile) in |log|, and persists // Records the current environment (system profile) in |log|, and persists
// the results in prefs. // the results in prefs.
// Exposed for testing. // Exposed for testing.
...@@ -291,8 +295,7 @@ class MetricsService : public base::HistogramFlattener { ...@@ -291,8 +295,7 @@ class MetricsService : public base::HistogramFlattener {
// Records the current environment (system profile) in |log|, and persists // Records the current environment (system profile) in |log|, and persists
// the results in prefs and GlobalPersistentSystemProfile. // the results in prefs and GlobalPersistentSystemProfile.
// Exposed for testing. void RecordCurrentEnvironment(MetricsLog* log, bool complete);
void RecordCurrentEnvironment(MetricsLog* log);
// Record complete list of histograms into the current log. // Record complete list of histograms into the current log.
// Called when we close a log. // Called when we close a log.
......
...@@ -62,7 +62,24 @@ class TestMetricsService : public MetricsService { ...@@ -62,7 +62,24 @@ class TestMetricsService : public MetricsService {
using MetricsService::log_store; using MetricsService::log_store;
using MetricsService::RecordCurrentEnvironmentHelper; using MetricsService::RecordCurrentEnvironmentHelper;
// MetricsService:
void SetPersistentSystemProfile(const std::string& serialized_proto,
bool complete) override {
persistent_system_profile_provided_ = true;
persistent_system_profile_complete_ = complete;
}
bool persistent_system_profile_provided() const {
return persistent_system_profile_provided_;
}
bool persistent_system_profile_complete() const {
return persistent_system_profile_complete_;
}
private: private:
bool persistent_system_profile_provided_ = false;
bool persistent_system_profile_complete_ = false;
DISALLOW_COPY_AND_ASSIGN(TestMetricsService); DISALLOW_COPY_AND_ASSIGN(TestMetricsService);
}; };
...@@ -345,6 +362,30 @@ TEST_F(MetricsServiceTest, MetricsProvidersInitialized) { ...@@ -345,6 +362,30 @@ TEST_F(MetricsServiceTest, MetricsProvidersInitialized) {
EXPECT_TRUE(test_provider->init_called()); EXPECT_TRUE(test_provider->init_called());
} }
TEST_F(MetricsServiceTest, SystemProfileDataProvidedOnEnableRecording) {
EnableMetricsReporting();
TestMetricsServiceClient client;
TestMetricsService service(GetMetricsStateManager(), &client,
GetLocalState());
TestMetricsProvider* test_provider = new TestMetricsProvider();
service.RegisterMetricsProvider(
std::unique_ptr<MetricsProvider>(test_provider));
service.InitializeMetricsRecordingState();
// ProvideSystemProfileMetrics() shouldn't be called initially.
EXPECT_FALSE(test_provider->provide_system_profile_metrics_called());
EXPECT_FALSE(service.persistent_system_profile_provided());
service.Start();
// Start should call ProvideSystemProfileMetrics().
EXPECT_TRUE(test_provider->provide_system_profile_metrics_called());
EXPECT_TRUE(service.persistent_system_profile_provided());
EXPECT_FALSE(service.persistent_system_profile_complete());
}
TEST_F(MetricsServiceTest, SplitRotation) { TEST_F(MetricsServiceTest, SplitRotation) {
EnableMetricsReporting(); EnableMetricsReporting();
TestMetricsServiceClient client; TestMetricsServiceClient client;
......
...@@ -66,12 +66,17 @@ ScreenInfoMetricsProvider::~ScreenInfoMetricsProvider() { ...@@ -66,12 +66,17 @@ ScreenInfoMetricsProvider::~ScreenInfoMetricsProvider() {
void ScreenInfoMetricsProvider::ProvideSystemProfileMetrics( void ScreenInfoMetricsProvider::ProvideSystemProfileMetrics(
SystemProfileProto* system_profile_proto) { SystemProfileProto* system_profile_proto) {
// This may be called before the screen info has been initialized, such as
// when the persistent system profile gets filled in initially.
const base::Optional<gfx::Size> display_size = GetScreenSize();
if (!display_size.has_value())
return;
SystemProfileProto::Hardware* hardware = SystemProfileProto::Hardware* hardware =
system_profile_proto->mutable_hardware(); system_profile_proto->mutable_hardware();
const gfx::Size display_size = GetScreenSize(); hardware->set_primary_screen_width(display_size->width());
hardware->set_primary_screen_width(display_size.width()); hardware->set_primary_screen_height(display_size->height());
hardware->set_primary_screen_height(display_size.height());
hardware->set_primary_screen_scale_factor(GetScreenDeviceScaleFactor()); hardware->set_primary_screen_scale_factor(GetScreenDeviceScaleFactor());
hardware->set_screen_count(GetScreenCount()); hardware->set_screen_count(GetScreenCount());
...@@ -80,8 +85,11 @@ void ScreenInfoMetricsProvider::ProvideSystemProfileMetrics( ...@@ -80,8 +85,11 @@ void ScreenInfoMetricsProvider::ProvideSystemProfileMetrics(
#endif #endif
} }
gfx::Size ScreenInfoMetricsProvider::GetScreenSize() const { base::Optional<gfx::Size> ScreenInfoMetricsProvider::GetScreenSize() const {
return display::Screen::GetScreen()->GetPrimaryDisplay().GetSizeInPixel(); auto* screen = display::Screen::GetScreen();
if (!screen)
return base::nullopt;
return base::make_optional(screen->GetPrimaryDisplay().GetSizeInPixel());
} }
float ScreenInfoMetricsProvider::GetScreenDeviceScaleFactor() const { float ScreenInfoMetricsProvider::GetScreenDeviceScaleFactor() const {
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#define COMPONENTS_METRICS_UI_SCREEN_INFO_METRICS_PROVIDER_H_ #define COMPONENTS_METRICS_UI_SCREEN_INFO_METRICS_PROVIDER_H_
#include "base/macros.h" #include "base/macros.h"
#include "base/optional.h"
#include "components/metrics/metrics_provider.h" #include "components/metrics/metrics_provider.h"
#include "ui/gfx/geometry/size.h" #include "ui/gfx/geometry/size.h"
...@@ -24,8 +25,8 @@ class ScreenInfoMetricsProvider : public MetricsProvider { ...@@ -24,8 +25,8 @@ class ScreenInfoMetricsProvider : public MetricsProvider {
protected: protected:
// Exposed for the sake of mocking in test code. // Exposed for the sake of mocking in test code.
// Returns the screen size for the primary monitor. // Returns the screen size for the primary monitor if available.
virtual gfx::Size GetScreenSize() const; virtual base::Optional<gfx::Size> GetScreenSize() const;
// Returns the device scale factor for the primary monitor. // Returns the device scale factor for the primary monitor.
virtual float GetScreenDeviceScaleFactor() const; virtual float GetScreenDeviceScaleFactor() const;
......
...@@ -24,8 +24,8 @@ class TestScreenInfoMetricsProvider : public ScreenInfoMetricsProvider { ...@@ -24,8 +24,8 @@ class TestScreenInfoMetricsProvider : public ScreenInfoMetricsProvider {
~TestScreenInfoMetricsProvider() override {} ~TestScreenInfoMetricsProvider() override {}
private: private:
gfx::Size GetScreenSize() const override { base::Optional<gfx::Size> GetScreenSize() const override {
return gfx::Size(kScreenWidth, kScreenHeight); return base::make_optional(gfx::Size(kScreenWidth, kScreenHeight));
} }
float GetScreenDeviceScaleFactor() const override { float GetScreenDeviceScaleFactor() const override {
......
...@@ -78,13 +78,8 @@ UkmService::UkmService(PrefService* pref_service, ...@@ -78,13 +78,8 @@ UkmService::UkmService(PrefService* pref_service,
bool restrict_to_whitelist_entries) bool restrict_to_whitelist_entries)
: pref_service_(pref_service), : pref_service_(pref_service),
restrict_to_whitelist_entries_(restrict_to_whitelist_entries), restrict_to_whitelist_entries_(restrict_to_whitelist_entries),
client_id_(0),
session_id_(0),
report_count_(0),
client_(client), client_(client),
reporting_service_(client, pref_service), reporting_service_(client, pref_service) {
initialize_started_(false),
initialize_complete_(false) {
DCHECK(pref_service_); DCHECK(pref_service_);
DCHECK(client_); DCHECK(client_);
DVLOG(1) << "UkmService::Constructor"; DVLOG(1) << "UkmService::Constructor";
...@@ -131,6 +126,7 @@ void UkmService::EnableReporting() { ...@@ -131,6 +126,7 @@ void UkmService::EnableReporting() {
if (reporting_service_.reporting_active()) if (reporting_service_.reporting_active())
return; return;
log_creation_time_ = base::TimeTicks::Now();
metrics_providers_.OnRecordingEnabled(); metrics_providers_.OnRecordingEnabled();
if (!initialize_started_) if (!initialize_started_)
...@@ -260,8 +256,8 @@ void UkmService::BuildAndStoreLog() { ...@@ -260,8 +256,8 @@ void UkmService::BuildAndStoreLog() {
metrics::MetricsLog::RecordCoreSystemProfile(client_, metrics::MetricsLog::RecordCoreSystemProfile(client_,
report.mutable_system_profile()); report.mutable_system_profile());
metrics_providers_.ProvideSystemProfileMetrics( metrics_providers_.ProvideSystemProfileMetricsWithLogCreationTime(
report.mutable_system_profile()); log_creation_time_, report.mutable_system_profile());
std::string serialized_log; std::string serialized_log;
report.SerializeToString(&serialized_log); report.SerializeToString(&serialized_log);
......
...@@ -128,13 +128,13 @@ class UkmService : public UkmRecorderImpl { ...@@ -128,13 +128,13 @@ class UkmService : public UkmRecorderImpl {
bool restrict_to_whitelist_entries_; bool restrict_to_whitelist_entries_;
// The UKM client id stored in prefs. // The UKM client id stored in prefs.
uint64_t client_id_; uint64_t client_id_ = 0;
// The UKM session id stored in prefs. // The UKM session id stored in prefs.
int32_t session_id_; int32_t session_id_ = 0;
// The number of reports generated this session. // The number of reports generated this session.
int32_t report_count_; int32_t report_count_ = 0;
// Used to interact with the embedder. Weak pointer; must outlive |this| // Used to interact with the embedder. Weak pointer; must outlive |this|
// instance. // instance.
...@@ -149,10 +149,12 @@ class UkmService : public UkmRecorderImpl { ...@@ -149,10 +149,12 @@ class UkmService : public UkmRecorderImpl {
// The scheduler for determining when uploads should happen. // The scheduler for determining when uploads should happen.
std::unique_ptr<metrics::MetricsRotationScheduler> scheduler_; std::unique_ptr<metrics::MetricsRotationScheduler> scheduler_;
SEQUENCE_CHECKER(sequence_checker_); base::TimeTicks log_creation_time_;
bool initialize_started_ = false;
bool initialize_complete_ = false;
bool initialize_started_; SEQUENCE_CHECKER(sequence_checker_);
bool initialize_complete_;
// Weak pointers factory used to post task on different threads. All weak // Weak pointers factory used to post task on different threads. All weak
// pointers managed by this factory have the same lifetime as UkmService. // pointers managed by this factory have the same lifetime as UkmService.
......
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