Commit c14c1467 authored by Robert Kaplow's avatar Robert Kaplow Committed by Commit Bot

Enable initial_url collection in UKM.

Simplify the logic by just enabling and removing the variations param (which was never enabled).


Bug: 869123
Change-Id: Ic98b681e53ca3a4179d4f5a6f2948520f3c9ff6c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1600451Reviewed-by: default avatarBryan McQuade <bmcquade@chromium.org>
Commit-Queue: Robert Kaplow <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#658561}
parent 363b7a34
...@@ -88,12 +88,6 @@ bool HasSupportedScheme(const GURL& url) { ...@@ -88,12 +88,6 @@ bool HasSupportedScheme(const GURL& url) {
url.SchemeIs(kExtensionScheme) || url.SchemeIs(kAppScheme); url.SchemeIs(kExtensionScheme) || url.SchemeIs(kAppScheme);
} }
// True if we should record the initial_url field of the UKM Source proto.
bool ShouldRecordInitialUrl() {
return base::GetFieldTrialParamByFeatureAsBool(kUkmFeature,
"RecordInitialUrl", false);
}
enum class DroppedDataReason { enum class DroppedDataReason {
NOT_DROPPED = 0, NOT_DROPPED = 0,
RECORDING_DISABLED = 1, RECORDING_DISABLED = 1,
...@@ -325,8 +319,6 @@ void UkmRecorderImpl::StoreRecordingsInReport(Report* report) { ...@@ -325,8 +319,6 @@ void UkmRecorderImpl::StoreRecordingsInReport(Report* report) {
} }
Source* proto_source = report->add_sources(); Source* proto_source = report->add_sources();
kv.second->PopulateProto(proto_source); kv.second->PopulateProto(proto_source);
if (!ShouldRecordInitialUrl())
proto_source->clear_initial_url();
serialized_source_type_counts[GetSourceIdType(kv.first)]++; serialized_source_type_counts[GetSourceIdType(kv.first)]++;
} }
......
...@@ -285,7 +285,7 @@ TEST_F(UkmServiceTest, SourceSerialization) { ...@@ -285,7 +285,7 @@ TEST_F(UkmServiceTest, SourceSerialization) {
EXPECT_EQ(id, proto_source.id()); EXPECT_EQ(id, proto_source.id());
EXPECT_EQ(GURL("https://google.com/final").spec(), proto_source.url()); EXPECT_EQ(GURL("https://google.com/final").spec(), proto_source.url());
EXPECT_FALSE(proto_source.has_initial_url()); EXPECT_TRUE(proto_source.has_initial_url());
} }
TEST_F(UkmServiceTest, AddEntryWithEmptyMetrics) { TEST_F(UkmServiceTest, AddEntryWithEmptyMetrics) {
...@@ -442,43 +442,34 @@ TEST_F(UkmServiceTest, GetNewSourceID) { ...@@ -442,43 +442,34 @@ TEST_F(UkmServiceTest, GetNewSourceID) {
} }
TEST_F(UkmServiceTest, RecordInitialUrl) { TEST_F(UkmServiceTest, RecordInitialUrl) {
for (bool should_record_initial_url : {true, false}) { ClearPrefs();
base::FieldTrialList field_trial_list(nullptr /* entropy_provider */); UkmService service(&prefs_, &client_,
ScopedUkmFeatureParams params( true /* restrict_to_whitelisted_entries */);
base::FeatureList::OVERRIDE_ENABLE_FEATURE, TestRecordingHelper recorder(&service);
{{"RecordInitialUrl", should_record_initial_url ? "true" : "false"}}); EXPECT_EQ(GetPersistedLogCount(), 0);
service.Initialize();
ClearPrefs(); task_runner_->RunUntilIdle();
UkmService service(&prefs_, &client_, service.EnableRecording(/*extensions=*/false);
true /* restrict_to_whitelisted_entries */); service.EnableReporting();
TestRecordingHelper recorder(&service);
EXPECT_EQ(GetPersistedLogCount(), 0);
service.Initialize();
task_runner_->RunUntilIdle();
service.EnableRecording(/*extensions=*/false);
service.EnableReporting();
ukm::SourceId id = GetWhitelistedSourceId(0); ukm::SourceId id = GetWhitelistedSourceId(0);
UkmSource::NavigationData navigation_data; UkmSource::NavigationData navigation_data;
navigation_data.urls = {GURL("https://google.com/initial"), navigation_data.urls = {GURL("https://google.com/initial"),
GURL("https://google.com/final")}; GURL("https://google.com/final")};
recorder.RecordNavigation(id, navigation_data); recorder.RecordNavigation(id, navigation_data);
service.Flush(); service.Flush();
EXPECT_EQ(GetPersistedLogCount(), 1); EXPECT_EQ(GetPersistedLogCount(), 1);
Report proto_report = GetPersistedReport(); Report proto_report = GetPersistedReport();
EXPECT_EQ(1, proto_report.sources_size()); EXPECT_EQ(1, proto_report.sources_size());
const Source& proto_source = proto_report.sources(0); const Source& proto_source = proto_report.sources(0);
EXPECT_EQ(id, proto_source.id()); EXPECT_EQ(id, proto_source.id());
EXPECT_EQ(GURL("https://google.com/final").spec(), proto_source.url()); EXPECT_EQ(GURL("https://google.com/final").spec(), proto_source.url());
EXPECT_EQ(should_record_initial_url, proto_source.has_initial_url()); EXPECT_TRUE(proto_source.has_initial_url());
if (should_record_initial_url) { EXPECT_EQ(GURL("https://google.com/initial").spec(),
EXPECT_EQ(GURL("https://google.com/initial").spec(), proto_source.initial_url());
proto_source.initial_url());
}
}
} }
TEST_F(UkmServiceTest, RestrictToWhitelistedSourceIds) { TEST_F(UkmServiceTest, RestrictToWhitelistedSourceIds) {
......
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