Commit f97f98d3 authored by Mounir Lamouri's avatar Mounir Lamouri Committed by Commit Bot

Reland "Media Engagement: only use/record for HTTPS origins."

This is a reland of 15c84b57

Original change's description:
> Media Engagement: only use/record for HTTPS origins.
> 
> Bug: 958364
> Change-Id: Ifc87e6222df9de526f2900f13899af616604750e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1591878
> Reviewed-by: Becca Hughes <beccahughes@chromium.org>
> Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#659971}

Bug: 958364
Change-Id: I2e1c9741946797875fca63524260ad5d066f74cf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1614442
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: default avatarBecca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#660841}
parent 331892eb
...@@ -367,6 +367,16 @@ IN_PROC_BROWSER_TEST_P(MediaEngagementAutoplayBrowserTest, TopFrameNavigation) { ...@@ -367,6 +367,16 @@ IN_PROC_BROWSER_TEST_P(MediaEngagementAutoplayBrowserTest, TopFrameNavigation) {
ExpectAutoplayAllowedIfEnabled(); ExpectAutoplayAllowedIfEnabled();
} }
IN_PROC_BROWSER_TEST_P(MediaEngagementAutoplayBrowserTest,
BypassAutoplayHighEngagement_HTTPSOnly) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(media::kMediaEngagementHTTPSOnly);
SetScores(PrimaryOrigin(), 20, 20);
LoadTestPage("engagement_autoplay_test.html");
ExpectAutoplayDenied();
}
INSTANTIATE_TEST_SUITE_P(/* no prefix */, INSTANTIATE_TEST_SUITE_P(/* no prefix */,
MediaEngagementAutoplayBrowserTest, MediaEngagementAutoplayBrowserTest,
testing::Bool()); testing::Bool());
...@@ -581,6 +581,9 @@ void MediaEngagementContentsObserver::ReadyToCommitNavigation( ...@@ -581,6 +581,9 @@ void MediaEngagementContentsObserver::ReadyToCommitNavigation(
MediaEngagementScore score = service_->CreateEngagementScore(origin); MediaEngagementScore score = service_->CreateEngagementScore(origin);
bool has_high_engagement = score.high_score(); bool has_high_engagement = score.high_score();
if (base::FeatureList::IsEnabled(media::kMediaEngagementHTTPSOnly))
DCHECK(!has_high_engagement || (origin.scheme() == url::kHttpsScheme));
// If the preloaded feature flag is enabled and the number of visits is less // If the preloaded feature flag is enabled and the number of visits is less
// than the number of visits required to have an MEI score we should check the // than the number of visits required to have an MEI score we should check the
// global data. // global data.
......
...@@ -109,6 +109,13 @@ MediaEngagementScore::MediaEngagementScore( ...@@ -109,6 +109,13 @@ MediaEngagementScore::MediaEngagementScore(
if (!score_dict_) if (!score_dict_)
return; return;
// This is to prevent using previously saved data to mark an HTTP website as
// allowed to autoplay.
if (base::FeatureList::IsEnabled(media::kMediaEngagementHTTPSOnly) &&
origin_.scheme() != url::kHttpsScheme) {
return;
}
GetIntegerFromScore(score_dict_.get(), kVisitsKey, &visits_); GetIntegerFromScore(score_dict_.get(), kVisitsKey, &visits_);
GetIntegerFromScore(score_dict_.get(), kMediaPlaybacksKey, &media_playbacks_); GetIntegerFromScore(score_dict_.get(), kMediaPlaybacksKey, &media_playbacks_);
GetIntegerFromScore(score_dict_.get(), kAudiblePlaybacksKey, GetIntegerFromScore(score_dict_.get(), kAudiblePlaybacksKey,
...@@ -209,6 +216,12 @@ bool MediaEngagementScore::UpdateScoreDict() { ...@@ -209,6 +216,12 @@ bool MediaEngagementScore::UpdateScoreDict() {
if (!score_dict_) if (!score_dict_)
return false; return false;
// This is to prevent saving data that we would otherwise not use.
if (base::FeatureList::IsEnabled(media::kMediaEngagementHTTPSOnly) &&
origin_.scheme() != url::kHttpsScheme) {
return false;
}
if (base::Value* value = score_dict_->FindKeyOfType( if (base::Value* value = score_dict_->FindKeyOfType(
kHasHighScoreKey, base::Value::Type::BOOLEAN)) { kHasHighScoreKey, base::Value::Type::BOOLEAN)) {
is_high = value->GetBool(); is_high = value->GetBool();
......
...@@ -46,6 +46,7 @@ struct MediaEngagementConfig { ...@@ -46,6 +46,7 @@ struct MediaEngagementConfig {
bool feature_record_data; bool feature_record_data;
bool feature_bypass_autoplay; bool feature_bypass_autoplay;
bool feature_preload_data; bool feature_preload_data;
bool feature_https_only;
bool feature_autoplay_disable_settings; bool feature_autoplay_disable_settings;
bool feature_autoplay_whitelist_settings; bool feature_autoplay_whitelist_settings;
bool pref_disable_unified_autoplay; bool pref_disable_unified_autoplay;
......
...@@ -108,7 +108,8 @@ class MediaEngagementScoreTest : public ChromeRenderViewHostTestHarness { ...@@ -108,7 +108,8 @@ class MediaEngagementScoreTest : public ChromeRenderViewHostTestHarness {
int visits_with_media_tag, int visits_with_media_tag,
int high_score_changes, int high_score_changes,
int media_element_playbacks, int media_element_playbacks,
int audio_context_playbacks) { int audio_context_playbacks,
bool update_score_expectation) {
MediaEngagementScore* initial_score = MediaEngagementScore* initial_score =
new MediaEngagementScore(&test_clock, url::Origin(), new MediaEngagementScore(&test_clock, url::Origin(),
std::move(score_dict), nullptr /* settings */); std::move(score_dict), nullptr /* settings */);
...@@ -124,7 +125,7 @@ class MediaEngagementScoreTest : public ChromeRenderViewHostTestHarness { ...@@ -124,7 +125,7 @@ class MediaEngagementScoreTest : public ChromeRenderViewHostTestHarness {
// Increment the scores and check that the values were stored correctly. // Increment the scores and check that the values were stored correctly.
UpdateScore(initial_score); UpdateScore(initial_score);
EXPECT_TRUE(initial_score->UpdateScoreDict()); EXPECT_EQ(update_score_expectation, initial_score->UpdateScoreDict());
delete initial_score; delete initial_score;
} }
...@@ -201,7 +202,7 @@ TEST_F(MediaEngagementScoreTest, MojoSerialization) { ...@@ -201,7 +202,7 @@ TEST_F(MediaEngagementScoreTest, MojoSerialization) {
TEST_F(MediaEngagementScoreTest, EmptyDictionary) { TEST_F(MediaEngagementScoreTest, EmptyDictionary) {
std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue()); std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
TestScoreInitializesAndUpdates(std::move(dict), 0, 0, base::Time(), false, 0, TestScoreInitializesAndUpdates(std::move(dict), 0, 0, base::Time(), false, 0,
0, 0, 0, 0, 0); 0, 0, 0, 0, 0, true);
} }
// Test that scores are read / written correctly from / to partially empty // Test that scores are read / written correctly from / to partially empty
...@@ -211,7 +212,7 @@ TEST_F(MediaEngagementScoreTest, PartiallyEmptyDictionary) { ...@@ -211,7 +212,7 @@ TEST_F(MediaEngagementScoreTest, PartiallyEmptyDictionary) {
dict->SetInteger(MediaEngagementScore::kVisitsKey, 2); dict->SetInteger(MediaEngagementScore::kVisitsKey, 2);
TestScoreInitializesAndUpdates(std::move(dict), 2, 0, base::Time(), false, 0, TestScoreInitializesAndUpdates(std::move(dict), 2, 0, base::Time(), false, 0,
0, 0, 0, 0, 0); 0, 0, 0, 0, 0, true);
} }
// Test that scores are read / written correctly from / to populated score // Test that scores are read / written correctly from / to populated score
...@@ -232,7 +233,7 @@ TEST_F(MediaEngagementScoreTest, PopulatedDictionary) { ...@@ -232,7 +233,7 @@ TEST_F(MediaEngagementScoreTest, PopulatedDictionary) {
2); 2);
TestScoreInitializesAndUpdates(std::move(dict), 20, 12, test_clock.Now(), TestScoreInitializesAndUpdates(std::move(dict), 20, 12, test_clock.Now(),
true, 2, 4, 6, 3, 1, 2); true, 2, 4, 6, 3, 1, 2, true);
} }
// Test getting and commiting the score works correctly with different // Test getting and commiting the score works correctly with different
...@@ -722,3 +723,27 @@ TEST_F(MediaEngagementScoreTest, ...@@ -722,3 +723,27 @@ TEST_F(MediaEngagementScoreTest,
EXPECT_EQ(media_element_playbacks, stored_media_element_playbacks); EXPECT_EQ(media_element_playbacks, stored_media_element_playbacks);
} }
} }
// Test that scores are read / written correctly from / to populated score
// dictionaries.
TEST_F(MediaEngagementScoreTest, PopulatedDictionary_HTTPSOnly) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(media::kMediaEngagementHTTPSOnly);
std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
dict->SetInteger(MediaEngagementScore::kVisitsKey, 20);
dict->SetInteger(MediaEngagementScore::kMediaPlaybacksKey, 12);
dict->SetDouble(MediaEngagementScore::kLastMediaPlaybackTimeKey,
test_clock.Now().ToDeltaSinceWindowsEpoch().InMicroseconds());
dict->SetBoolean(MediaEngagementScore::kHasHighScoreKey, true);
dict->SetInteger(MediaEngagementScore::kAudiblePlaybacksKey, 2);
dict->SetInteger(MediaEngagementScore::kSignificantPlaybacksKey, 4);
dict->SetInteger(MediaEngagementScore::kVisitsWithMediaTagKey, 6);
dict->SetInteger(MediaEngagementScore::kHighScoreChanges, 3);
dict->SetInteger(MediaEngagementScore::kSignificantMediaPlaybacksKey, 1);
dict->SetInteger(MediaEngagementScore::kSignificantAudioContextPlaybacksKey,
2);
TestScoreInitializesAndUpdates(std::move(dict), 0, 0, base::Time(), false, 0,
0, 0, 0, 0, 0, false);
}
...@@ -330,6 +330,9 @@ Profile* MediaEngagementService::profile() const { ...@@ -330,6 +330,9 @@ Profile* MediaEngagementService::profile() const {
bool MediaEngagementService::ShouldRecordEngagement( bool MediaEngagementService::ShouldRecordEngagement(
const url::Origin& origin) const { const url::Origin& origin) const {
if (base::FeatureList::IsEnabled(media::kMediaEngagementHTTPSOnly))
return origin.scheme() == url::kHttpsScheme;
return (origin.scheme() == url::kHttpsScheme || return (origin.scheme() == url::kHttpsScheme ||
origin.scheme() == url::kHttpScheme); origin.scheme() == url::kHttpScheme);
} }
...@@ -358,6 +361,11 @@ std::vector<MediaEngagementScore> MediaEngagementService::GetAllStoredScores() ...@@ -358,6 +361,11 @@ std::vector<MediaEngagementScore> MediaEngagementService::GetAllStoredScores()
continue; continue;
} }
if (base::FeatureList::IsEnabled(media::kMediaEngagementHTTPSOnly) &&
origin.scheme() != url::kHttpsScheme) {
continue;
}
const auto& result = filtered_results.find(origin); const auto& result = filtered_results.find(origin);
if (result != filtered_results.end()) { if (result != filtered_results.end()) {
DCHECK(result->second->incognito && !site.incognito); DCHECK(result->second->incognito && !site.incognito);
......
...@@ -135,6 +135,8 @@ function renderConfigTable(config) { ...@@ -135,6 +135,8 @@ function renderConfigTable(config) {
formatFeatureFlag(config.featureBypassAutoplay))); formatFeatureFlag(config.featureBypassAutoplay)));
configTableBody.appendChild(createConfigRow( configTableBody.appendChild(createConfigRow(
'Preload MEI data', formatFeatureFlag(config.featurePreloadData))); 'Preload MEI data', formatFeatureFlag(config.featurePreloadData)));
configTableBody.appendChild(createConfigRow(
'MEI for HTTPS only', formatFeatureFlag(config.featureHttpsOnly)));
configTableBody.appendChild(createConfigRow( configTableBody.appendChild(createConfigRow(
'Autoplay disable settings', 'Autoplay disable settings',
formatFeatureFlag(config.featureAutoplayDisableSettings))); formatFeatureFlag(config.featureAutoplayDisableSettings)));
......
...@@ -78,6 +78,7 @@ class MediaEngagementScoreDetailsProviderImpl ...@@ -78,6 +78,7 @@ class MediaEngagementScoreDetailsProviderImpl
base::FeatureList::IsEnabled( base::FeatureList::IsEnabled(
media::kMediaEngagementBypassAutoplayPolicies), media::kMediaEngagementBypassAutoplayPolicies),
base::FeatureList::IsEnabled(media::kPreloadMediaEngagementData), base::FeatureList::IsEnabled(media::kPreloadMediaEngagementData),
base::FeatureList::IsEnabled(media::kMediaEngagementHTTPSOnly),
base::FeatureList::IsEnabled(media::kAutoplayDisableSettings), base::FeatureList::IsEnabled(media::kAutoplayDisableSettings),
base::FeatureList::IsEnabled(media::kAutoplayWhitelistSettings), base::FeatureList::IsEnabled(media::kAutoplayWhitelistSettings),
GetBlockAutoplayPref(), GetBlockAutoplayPref(),
......
...@@ -481,6 +481,9 @@ const base::Feature kPreloadMediaEngagementData{ ...@@ -481,6 +481,9 @@ const base::Feature kPreloadMediaEngagementData{
"PreloadMediaEngagementData", base::FEATURE_ENABLED_BY_DEFAULT}; "PreloadMediaEngagementData", base::FEATURE_ENABLED_BY_DEFAULT};
#endif #endif
const base::Feature kMediaEngagementHTTPSOnly{
"MediaEngagementHTTPSOnly", base::FEATURE_DISABLED_BY_DEFAULT};
// Enables experimental local learning for media. Adds reporting only; does not // Enables experimental local learning for media. Adds reporting only; does not
// change media behavior. // change media behavior.
const base::Feature kMediaLearningExperiment{"MediaLearningExperiment", const base::Feature kMediaLearningExperiment{"MediaLearningExperiment",
......
...@@ -110,6 +110,7 @@ MEDIA_EXPORT extern const base::Feature kLowDelayVideoRenderingOnLiveStream; ...@@ -110,6 +110,7 @@ MEDIA_EXPORT extern const base::Feature kLowDelayVideoRenderingOnLiveStream;
MEDIA_EXPORT extern const base::Feature kMediaCapabilitiesWithParameters; MEDIA_EXPORT extern const base::Feature kMediaCapabilitiesWithParameters;
MEDIA_EXPORT extern const base::Feature kMediaCastOverlayButton; MEDIA_EXPORT extern const base::Feature kMediaCastOverlayButton;
MEDIA_EXPORT extern const base::Feature kMediaEngagementBypassAutoplayPolicies; MEDIA_EXPORT extern const base::Feature kMediaEngagementBypassAutoplayPolicies;
MEDIA_EXPORT extern const base::Feature kMediaEngagementHTTPSOnly;
MEDIA_EXPORT extern const base::Feature kMediaLearningExperiment; MEDIA_EXPORT extern const base::Feature kMediaLearningExperiment;
MEDIA_EXPORT extern const base::Feature kMemoryPressureBasedSourceBufferGC; MEDIA_EXPORT extern const base::Feature kMemoryPressureBasedSourceBufferGC;
MEDIA_EXPORT extern const base::Feature kNewEncodeCpuLoadEstimator; MEDIA_EXPORT extern const base::Feature kNewEncodeCpuLoadEstimator;
......
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