Skip to content

Commit b9b9a7c

Browse files
Michael BaiCommit Bot
Michael Bai
authored and
Commit Bot
committed
Support simulation of internal-profile reporting
Record the number of files (and their unsent samples) from previous runs and make those values into stability metrics. The metrics precisely simulate the INTERNAL_PROFILE behavior and are only enabled for WebView. The patch also extends the metrics for used size to M87. Bug: 1076549 Change-Id: I0356899f13b8b89b9823c81d1eb748ef8ca3ee5d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2293321 Reviewed-by: John Abd-El-Malek <jam@chromium.org> Reviewed-by: Brian White <bcwhite@chromium.org> Commit-Queue: Tao Bai <michaelbai@chromium.org> Cr-Commit-Position: refs/heads/master@{#801165}
1 parent c1355d4 commit b9b9a7c

17 files changed

+193
-32
lines changed

android_webview/browser/aw_feature_list_creator.cc

+4
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,15 @@ const char* const kPersistentPrefsAllowlist[] = {
6060
// Random seed value for variation's entropy providers. Used to assign
6161
// experiment groups.
6262
metrics::prefs::kMetricsLowEntropySource,
63+
// File metrics metadata.
64+
metrics::prefs::kMetricsFileMetricsMetadata,
6365
// Logged directly in the ChromeUserMetricsExtension proto.
6466
metrics::prefs::kInstallDate,
6567
metrics::prefs::kMetricsReportingEnabledTimestamp,
6668
metrics::prefs::kMetricsSessionID,
6769
// Logged in system_profile.stability fields.
70+
metrics::prefs::kStabilityFileMetricsUnsentFilesCount,
71+
metrics::prefs::kStabilityFileMetricsUnsentSamplesCount,
6872
metrics::prefs::kStabilityLaunchCount,
6973
metrics::prefs::kStabilityPageLoadCount,
7074
metrics::prefs::kStabilityRendererHangCount,

android_webview/browser/metrics/aw_metrics_service_client.cc

+5
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "android_webview/browser_jni_headers/AwMetricsServiceClient_jni.h"
1212
#include "base/android/jni_android.h"
1313
#include "base/metrics/histogram_functions.h"
14+
#include "base/metrics/persistent_histogram_allocator.h"
1415
#include "base/no_destructor.h"
1516
#include "components/metrics/metrics_pref_names.h"
1617
#include "components/metrics/metrics_service.h"
@@ -132,6 +133,10 @@ void AwMetricsServiceClient::RegisterAdditionalMetricsProviders(
132133
delegate_->RegisterAdditionalMetricsProviders(service);
133134
}
134135

136+
bool AwMetricsServiceClient::IsPersistentHistogramsEnabled() {
137+
return base::FeatureList::IsEnabled(base::kPersistentHistogramsFeature);
138+
}
139+
135140
// static
136141
void JNI_AwMetricsServiceClient_SetHaveMetricsConsent(JNIEnv* env,
137142
jboolean user_consent,

android_webview/browser/metrics/aw_metrics_service_client.h

+1
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ class AwMetricsServiceClient : public ::metrics::AndroidMetricsServiceClient,
134134
int GetPackageNameLimitRatePerMille() override;
135135
void RegisterAdditionalMetricsProviders(
136136
metrics::MetricsService* service) override;
137+
bool IsPersistentHistogramsEnabled() override;
137138

138139
private:
139140
bool app_in_foreground_ = false;

chrome/browser/metrics/chrome_metrics_service_client.cc

+6-5
Original file line numberDiff line numberDiff line change
@@ -210,16 +210,17 @@ base::LazyInstance<std::string>::Leaky g_environment_for_crash_reporter;
210210
#endif // defined(OS_WIN) || defined(OS_MAC) || defined(OS_ANDROID)
211211

212212
void RegisterFileMetricsPreferences(PrefRegistrySimple* registry) {
213-
metrics::FileMetricsProvider::RegisterPrefs(registry, kBrowserMetricsName);
213+
metrics::FileMetricsProvider::RegisterSourcePrefs(registry,
214+
kBrowserMetricsName);
214215

215-
metrics::FileMetricsProvider::RegisterPrefs(registry,
216-
kCrashpadHistogramAllocatorName);
216+
metrics::FileMetricsProvider::RegisterSourcePrefs(
217+
registry, kCrashpadHistogramAllocatorName);
217218

218219
#if defined(OS_WIN)
219-
metrics::FileMetricsProvider::RegisterPrefs(
220+
metrics::FileMetricsProvider::RegisterSourcePrefs(
220221
registry, installer::kSetupHistogramAllocatorName);
221222

222-
metrics::FileMetricsProvider::RegisterPrefs(
223+
metrics::FileMetricsProvider::RegisterSourcePrefs(
223224
registry, notification_helper::kNotificationHelperHistogramAllocatorName);
224225
#endif
225226
}

components/embedder_support/android/metrics/android_metrics_service_client.cc

+18-6
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "base/base_paths_android.h"
1313
#include "base/files/file_util.h"
1414
#include "base/i18n/rtl.h"
15+
#include "base/metrics/field_trial_params.h"
1516
#include "base/metrics/persistent_histogram_allocator.h"
1617
#include "base/metrics/statistics_recorder.h"
1718
#include "base/path_service.h"
@@ -135,6 +136,11 @@ void RegisterOrRemovePreviousRunMetricsFile(
135136
}
136137
}
137138

139+
bool IsSamplesCounterEnabled() {
140+
return base::GetFieldTrialParamByFeatureAsBool(
141+
base::kPersistentHistogramsFeature, "prev_run_metrics_count_only", false);
142+
}
143+
138144
std::unique_ptr<metrics::FileMetricsProvider> CreateFileMetricsProvider(
139145
PrefService* pref_service,
140146
bool metrics_reporting_enabled) {
@@ -158,8 +164,12 @@ std::unique_ptr<metrics::FileMetricsProvider> CreateFileMetricsProvider(
158164
metrics::FileMetricsProvider::Params browser_metrics_params(
159165
browser_metrics_upload_dir,
160166
metrics::FileMetricsProvider::SOURCE_HISTOGRAMS_ATOMIC_DIR,
161-
metrics::FileMetricsProvider::ASSOCIATE_INTERNAL_PROFILE,
167+
IsSamplesCounterEnabled()
168+
? metrics::FileMetricsProvider::
169+
ASSOCIATE_INTERNAL_PROFILE_SAMPLES_COUNTER
170+
: metrics::FileMetricsProvider::ASSOCIATE_INTERNAL_PROFILE,
162171
kBrowserMetricsName);
172+
163173
browser_metrics_params.max_dir_kib = kMaxHistogramStorageKiB;
164174
browser_metrics_params.filter =
165175
base::BindRepeating(FilterBrowserMetricsFiles);
@@ -192,9 +202,11 @@ AndroidMetricsServiceClient::~AndroidMetricsServiceClient() = default;
192202
// static
193203
void AndroidMetricsServiceClient::RegisterPrefs(PrefRegistrySimple* registry) {
194204
metrics::MetricsService::RegisterPrefs(registry);
195-
metrics::FileMetricsProvider::RegisterPrefs(registry, kBrowserMetricsName);
196-
metrics::FileMetricsProvider::RegisterPrefs(registry,
197-
kCrashpadHistogramAllocatorName);
205+
metrics::FileMetricsProvider::RegisterSourcePrefs(registry,
206+
kBrowserMetricsName);
207+
metrics::FileMetricsProvider::RegisterSourcePrefs(
208+
registry, kCrashpadHistogramAllocatorName);
209+
metrics::FileMetricsProvider::RegisterPrefs(registry);
198210
metrics::StabilityMetricsHelper::RegisterPrefs(registry);
199211
ukm::UkmService::RegisterPrefs(registry);
200212
}
@@ -259,7 +271,7 @@ void AndroidMetricsServiceClient::CreateMetricsService(
259271
std::make_unique<CPUMetricsProvider>());
260272
metrics_service_->RegisterMetricsProvider(
261273
std::make_unique<ScreenInfoMetricsProvider>());
262-
if (client->EnablePersistentHistograms()) {
274+
if (client->IsPersistentHistogramsEnabled()) {
263275
metrics_service_->RegisterMetricsProvider(CreateFileMetricsProvider(
264276
pref_service_, metrics_state_manager_->IsMetricsReportingEnabled()));
265277
}
@@ -538,7 +550,7 @@ bool AndroidMetricsServiceClient::IsInPackageNameSample() {
538550
void AndroidMetricsServiceClient::RegisterAdditionalMetricsProviders(
539551
MetricsService* service) {}
540552

541-
bool AndroidMetricsServiceClient::EnablePersistentHistograms() {
553+
bool AndroidMetricsServiceClient::IsPersistentHistogramsEnabled() {
542554
return false;
543555
}
544556

components/embedder_support/android/metrics/android_metrics_service_client.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ class AndroidMetricsServiceClient : public MetricsServiceClient,
212212
// client returns true then its
213213
// variations::PlatformFieldTrials::SetupFieldTrials needs to also call
214214
// InstantiatePersistentHistograms.
215-
virtual bool EnablePersistentHistograms();
215+
virtual bool IsPersistentHistogramsEnabled();
216216

217217
// Returns the embedding application's package name (unconditionally). Virtual
218218
// for testing.

components/metrics/file_metrics_provider.cc

+68-7
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "base/files/memory_mapped_file.h"
1717
#include "base/logging.h"
1818
#include "base/metrics/histogram_base.h"
19+
#include "base/metrics/histogram_functions.h"
1920
#include "base/metrics/histogram_macros.h"
2021
#include "base/metrics/persistent_histogram_allocator.h"
2122
#include "base/metrics/persistent_memory_allocator.h"
@@ -26,12 +27,15 @@
2627
#include "base/task/thread_pool.h"
2728
#include "base/task_runner.h"
2829
#include "base/task_runner_util.h"
30+
#include "base/threading/thread_task_runner_handle.h"
2931
#include "base/time/time.h"
3032
#include "components/metrics/metrics_pref_names.h"
3133
#include "components/metrics/metrics_service.h"
34+
#include "components/metrics/persistent_histograms.h"
3235
#include "components/metrics/persistent_system_profile.h"
3336
#include "components/prefs/pref_registry_simple.h"
3437
#include "components/prefs/pref_service.h"
38+
#include "components/prefs/scoped_user_pref_update.h"
3539

3640
namespace metrics {
3741

@@ -191,7 +195,10 @@ FileMetricsProvider::Params::Params(const base::FilePath& path,
191195
FileMetricsProvider::Params::~Params() {}
192196

193197
FileMetricsProvider::FileMetricsProvider(PrefService* local_state)
194-
: task_runner_(CreateBackgroundTaskRunner()), pref_service_(local_state) {
198+
: task_runner_(CreateBackgroundTaskRunner()),
199+
pref_service_(local_state),
200+
main_task_runner_(base::ThreadTaskRunnerHandle::Get()) {
201+
DCHECK(main_task_runner_);
195202
base::StatisticsRecorder::RegisterHistogramProvider(
196203
weak_factory_.GetWeakPtr());
197204
}
@@ -216,6 +223,7 @@ void FileMetricsProvider::RegisterSource(const Params& params) {
216223
switch (params.association) {
217224
case ASSOCIATE_CURRENT_RUN:
218225
case ASSOCIATE_INTERNAL_PROFILE:
226+
case ASSOCIATE_INTERNAL_PROFILE_SAMPLES_COUNTER:
219227
sources_to_check_.push_back(std::move(source));
220228
break;
221229
case ASSOCIATE_PREVIOUS_RUN:
@@ -227,12 +235,18 @@ void FileMetricsProvider::RegisterSource(const Params& params) {
227235
}
228236

229237
// static
230-
void FileMetricsProvider::RegisterPrefs(PrefRegistrySimple* prefs,
231-
const base::StringPiece prefs_key) {
238+
void FileMetricsProvider::RegisterSourcePrefs(
239+
PrefRegistrySimple* prefs,
240+
const base::StringPiece prefs_key) {
232241
prefs->RegisterInt64Pref(metrics::prefs::kMetricsLastSeenPrefix +
233242
prefs_key.as_string(), 0);
234243
}
235244

245+
// static
246+
void FileMetricsProvider::RegisterPrefs(PrefRegistrySimple* prefs) {
247+
prefs->RegisterListPref(metrics::prefs::kMetricsFileMetricsMetadata);
248+
}
249+
236250
// static
237251
void FileMetricsProvider::SetTaskRunnerForTesting(
238252
const scoped_refptr<base::TaskRunner>& task_runner) {
@@ -373,7 +387,6 @@ void FileMetricsProvider::FinishedWithSource(SourceInfo* source,
373387
}
374388
}
375389

376-
// static
377390
void FileMetricsProvider::CheckAndMergeMetricSourcesOnTaskRunner(
378391
SourceInfoList* sources) {
379392
// This method has all state information passed in |sources| and is intended
@@ -401,7 +414,11 @@ void FileMetricsProvider::CheckAndMergeMetricSourcesOnTaskRunner(
401414
if (source->association == ASSOCIATE_INTERNAL_PROFILE)
402415
break;
403416

404-
MergeHistogramDeltasFromSource(source.get());
417+
if (source->association == ASSOCIATE_INTERNAL_PROFILE_SAMPLES_COUNTER) {
418+
RecordFileMetadataOnTaskRunner(source.get());
419+
} else {
420+
MergeHistogramDeltasFromSource(source.get());
421+
}
405422
DCHECK(source->read_complete);
406423
}
407424

@@ -627,6 +644,25 @@ bool FileMetricsProvider::ProvideIndependentMetricsOnTaskRunner(
627644
return false;
628645
}
629646

647+
void FileMetricsProvider::AppendToSamplesCountPref(size_t samples_count) {
648+
ListPrefUpdate update(pref_service_,
649+
metrics::prefs::kMetricsFileMetricsMetadata);
650+
update->Append(static_cast<int>(samples_count));
651+
}
652+
653+
void FileMetricsProvider::RecordFileMetadataOnTaskRunner(SourceInfo* source) {
654+
base::HistogramBase::Count samples_count = 0;
655+
base::PersistentHistogramAllocator::Iterator it{source->allocator.get()};
656+
std::unique_ptr<base::HistogramBase> histogram;
657+
while ((histogram = it.GetNext()) != nullptr) {
658+
samples_count += histogram->SnapshotFinalDelta()->TotalCount();
659+
}
660+
main_task_runner_->PostTask(
661+
FROM_HERE, base::BindOnce(&FileMetricsProvider::AppendToSamplesCountPref,
662+
base::Unretained(this), samples_count));
663+
source->read_complete = true;
664+
}
665+
630666
void FileMetricsProvider::ScheduleSourcesCheck() {
631667
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
632668

@@ -643,7 +679,7 @@ void FileMetricsProvider::ScheduleSourcesCheck() {
643679
FROM_HERE,
644680
base::BindOnce(
645681
&FileMetricsProvider::CheckAndMergeMetricSourcesOnTaskRunner,
646-
base::Unretained(check_list)),
682+
base::Unretained(this), base::Unretained(check_list)),
647683
base::BindOnce(&FileMetricsProvider::RecordSourcesChecked,
648684
weak_factory_.GetWeakPtr(), base::Owned(check_list)));
649685
}
@@ -723,7 +759,7 @@ void FileMetricsProvider::OnDidCreateMetricsLog() {
723759

724760
bool FileMetricsProvider::HasIndependentMetrics() {
725761
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
726-
return !sources_with_profile_.empty();
762+
return !sources_with_profile_.empty() || SimulateIndependentMetrics();
727763
}
728764

729765
void FileMetricsProvider::ProvideIndependentMetrics(
@@ -844,4 +880,29 @@ void FileMetricsProvider::MergeHistogramDeltas() {
844880
}
845881
}
846882

883+
bool FileMetricsProvider::SimulateIndependentMetrics() {
884+
if (!pref_service_->HasPrefPath(
885+
metrics::prefs::kMetricsFileMetricsMetadata)) {
886+
return false;
887+
}
888+
889+
ListPrefUpdate list_value(pref_service_,
890+
metrics::prefs::kMetricsFileMetricsMetadata);
891+
if (list_value->empty())
892+
return false;
893+
894+
base::Value::ListView mutable_list = list_value->GetList();
895+
size_t count = pref_service_->GetInteger(
896+
metrics::prefs::kStabilityFileMetricsUnsentSamplesCount);
897+
pref_service_->SetInteger(
898+
metrics::prefs::kStabilityFileMetricsUnsentSamplesCount,
899+
mutable_list[0].GetInt() + count);
900+
pref_service_->SetInteger(
901+
metrics::prefs::kStabilityFileMetricsUnsentFilesCount,
902+
list_value->GetSize() - 1);
903+
list_value->EraseListIter(mutable_list.begin());
904+
905+
return true;
906+
}
907+
847908
} // namespace metrics

components/metrics/file_metrics_provider.h

+30-5
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "base/callback_forward.h"
1313
#include "base/files/file_path.h"
1414
#include "base/gtest_prod_util.h"
15+
#include "base/memory/scoped_refptr.h"
1516
#include "base/memory/weak_ptr.h"
1617
#include "base/metrics/statistics_recorder.h"
1718
#include "base/sequence_checker.h"
@@ -97,6 +98,15 @@ class FileMetricsProvider : public MetricsProvider,
9798
// that time even though actual transfer will be delayed if an
9899
// embedded profile is found.
99100
ASSOCIATE_INTERNAL_PROFILE_OR_PREVIOUS_RUN,
101+
102+
// Used to only record the metadata of |ASSOCIATE_INTERNAL_PROFILE| but not
103+
// merge the metrics. Instead, write metadata such as the samples count etc,
104+
// to prefs then delete file. To precisely simulate the
105+
// |ASSOCIATE_INTERNAL_PROFILE| behavior, one file record will be read out
106+
// and added to the stability prefs each time the metrics service requests
107+
// the |ASSOCIATE_INTERNAL_PROFILE| source metrics. Finally, the results
108+
// will be recoreded as stability metrics in the next run.
109+
ASSOCIATE_INTERNAL_PROFILE_SAMPLES_COUNTER,
100110
};
101111

102112
enum FilterAction {
@@ -159,17 +169,19 @@ class FileMetricsProvider : public MetricsProvider,
159169
// Indicates a file or directory to be monitored and how the file or files
160170
// within that directory are used. Because some metadata may need to persist
161171
// across process restarts, preferences entries are used based on the
162-
// |prefs_key| name. Call RegisterPrefs() with the same name to create the
163-
// necessary keys in advance. Set |prefs_key| empty (nullptr will work) if
172+
// |prefs_key| name. Call RegisterSourcePrefs() with the same name to create
173+
// the necessary keys in advance. Set |prefs_key| empty (nullptr will work) if
164174
// no persistence is required. ACTIVE files shouldn't have a pref key as
165175
// they update internal state about what has been previously sent.
166176
void RegisterSource(const Params& params);
167177

168178
// Registers all necessary preferences for maintaining persistent state
169179
// about a monitored file across process restarts. The |prefs_key| is
170180
// typically the filename.
171-
static void RegisterPrefs(PrefRegistrySimple* prefs,
172-
const base::StringPiece prefs_key);
181+
static void RegisterSourcePrefs(PrefRegistrySimple* prefs,
182+
const base::StringPiece prefs_key);
183+
184+
static void RegisterPrefs(PrefRegistrySimple* prefs);
173185

174186
// Sets the task runner to use for testing.
175187
static void SetTaskRunnerForTesting(
@@ -250,7 +262,7 @@ class FileMetricsProvider : public MetricsProvider,
250262

251263
// Checks a list of sources (on a task-runner allowed to do I/O) and merge
252264
// any data found within them.
253-
static void CheckAndMergeMetricSourcesOnTaskRunner(SourceInfoList* sources);
265+
void CheckAndMergeMetricSourcesOnTaskRunner(SourceInfoList* sources);
254266

255267
// Checks a single source and maps it into memory.
256268
static AccessResult CheckAndMapMetricSource(SourceInfo* source);
@@ -273,6 +285,12 @@ class FileMetricsProvider : public MetricsProvider,
273285
SystemProfileProto* system_profile_proto,
274286
base::HistogramSnapshotManager* snapshot_manager);
275287

288+
// Records the metadata of the |source| to perf.
289+
void RecordFileMetadataOnTaskRunner(SourceInfo* source);
290+
291+
// Appends the samples count to pref on UI thread.
292+
void AppendToSamplesCountPref(size_t samples_count);
293+
276294
// Creates a task to check all monitored sources for updates.
277295
void ScheduleSourcesCheck();
278296

@@ -306,6 +324,11 @@ class FileMetricsProvider : public MetricsProvider,
306324
std::unique_ptr<SourceInfo> source,
307325
bool success);
308326

327+
// Simulates the independent metrics to read the first item from
328+
// kMetricsBrowserMetricsMetadata and updates the stability prefs accordingly,
329+
// return true if the pref isn't empty.
330+
bool SimulateIndependentMetrics();
331+
309332
// A task-runner capable of performing I/O.
310333
scoped_refptr<base::TaskRunner> task_runner_;
311334

@@ -326,6 +349,8 @@ class FileMetricsProvider : public MetricsProvider,
326349
// The preferences-service used to store persistent state about sources.
327350
PrefService* pref_service_;
328351

352+
const scoped_refptr<base::TaskRunner> main_task_runner_;
353+
329354
SEQUENCE_CHECKER(sequence_checker_);
330355
base::WeakPtrFactory<FileMetricsProvider> weak_factory_{this};
331356

0 commit comments

Comments
 (0)