Skip to content

Commit

Permalink
Media Engagement: Use preloaded data with autoplay
Browse files Browse the repository at this point in the history
This adds a feature flag to use the preloaded data to make
"high engagement" decisions and therefore control autoplay.
There is also fixes a bug in our SetHighMediaEngagement code
where a navigation on the same origin does not properly set
the high engagement bit on the Frame.

BUG=787464

Change-Id: I2a85e6f60cbe58c4d4adcf54c344d8a65705cb77
Reviewed-on: https://chromium-review.googlesource.com/787311
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523741}
  • Loading branch information
beccahughes authored and Commit Bot committed Dec 13, 2017
1 parent bc686cb commit eb5a05f
Show file tree
Hide file tree
Showing 9 changed files with 255 additions and 14 deletions.
138 changes: 137 additions & 1 deletion chrome/browser/media/media_engagement_autoplay_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,17 @@
// found in the LICENSE file.

#include "base/command_line.h"
#include "base/files/file_util.h"
#include "base/json/json_writer.h"
#include "base/path_service.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/scoped_feature_list.h"
#include "base/threading/thread_restrictions.h"
#include "base/values.h"
#include "build/build_config.h"
#include "chrome/browser/content_settings/chrome_content_settings_utils.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/media/media_engagement_preloaded_list.h"
#include "chrome/browser/media/media_engagement_service.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
Expand All @@ -19,12 +26,32 @@

namespace {

base::FilePath GetPythonPath() {
#if defined(OS_WIN)
// Windows bots do not have python installed and available on the PATH.
// Please see infra/doc/users/python.md
base::FilePath bot_path =
base::FilePath(FILE_PATH_LITERAL("c:/infra-system/bin/python.exe"));

if (base::PathExists(bot_path))
return bot_path;
return base::FilePath(FILE_PATH_LITERAL("python.exe"));
#else
return base::FilePath(FILE_PATH_LITERAL("python"));
#endif
}

const base::FilePath kTestDataPath = base::FilePath(
FILE_PATH_LITERAL("chrome/test/data/media/engagement/preload"));

const char kMediaEngagementTestDataPath[] = "chrome/test/data/media/engagement";

const base::string16 kAllowedTitle = base::ASCIIToUTF16("Allowed");

const base::string16 kDeniedTitle = base::ASCIIToUTF16("Denied");

const base::FilePath kEmptyDataPath = kTestDataPath.AppendASCII("empty.pb");

} // namespace

// Class used to test that origins with a high Media Engagement score
Expand All @@ -51,10 +78,14 @@ class MediaEngagementAutoplayBrowserTest : public InProcessBrowserTest {

scoped_feature_list_.InitWithFeatures(
{media::kRecordMediaEngagementScores,
media::kMediaEngagementBypassAutoplayPolicies},
media::kMediaEngagementBypassAutoplayPolicies,
media::kPreloadMediaEngagementData},
{});

InProcessBrowserTest::SetUp();

// Clear any preloaded MEI data.
ApplyEmptyPreloadedList();
};

void LoadTestPage(const std::string& page) {
Expand Down Expand Up @@ -88,6 +119,50 @@ class MediaEngagementAutoplayBrowserTest : public InProcessBrowserTest {

void ExpectAutoplayDenied() { EXPECT_EQ(kDeniedTitle, WaitAndGetTitle()); }

void ApplyPreloadedOrigin(GURL url) {
base::ScopedAllowBlockingForTesting allow_blocking;

// Get two temporary files.
base::FilePath input_path;
base::FilePath output_path;
EXPECT_TRUE(base::CreateTemporaryFile(&input_path));
EXPECT_TRUE(base::CreateTemporaryFile(&output_path));

// Write JSON file with the server origin in it.
base::ListValue list;
list.AppendString(url::Origin::Create(url).Serialize());
std::string json_data;
base::JSONWriter::Write(list, &json_data);
EXPECT_TRUE(
base::WriteFile(input_path, json_data.c_str(), json_data.size()));

// Get the path to the "generator" binary in the module path.
base::FilePath module_dir;
EXPECT_TRUE(PathService::Get(base::DIR_MODULE, &module_dir));

// Launch the generator and wait for it to finish.
base::CommandLine cmd(GetPythonPath());
cmd.AppendArg("tools/media_engagement_preload/make_dafsa.py");
cmd.AppendArgPath(input_path);
cmd.AppendArgPath(output_path);
base::Process process = base::LaunchProcess(cmd, base::LaunchOptions());
EXPECT_TRUE(process.WaitForExit(0));

// Load the preloaded list.
EXPECT_TRUE(
MediaEngagementPreloadedList::GetInstance()->LoadFromFile(output_path));
}

void ApplyEmptyPreloadedList() {
// Get the path relative to the source root.
base::FilePath source_root;
EXPECT_TRUE(PathService::Get(base::DIR_SOURCE_ROOT, &source_root));

base::ScopedAllowBlockingForTesting allow_blocking;
EXPECT_TRUE(MediaEngagementPreloadedList::GetInstance()->LoadFromFile(
source_root.Append(kEmptyDataPath)));
}

private:
base::string16 WaitAndGetTitle() {
content::TitleWatcher title_watcher(GetWebContents(), kAllowedTitle);
Expand Down Expand Up @@ -177,3 +252,64 @@ IN_PROC_BROWSER_TEST_F(MediaEngagementAutoplayBrowserTest,
LoadTestPage("engagement_autoplay_test.html");
ExpectAutoplayAllowed();
}

IN_PROC_BROWSER_TEST_F(MediaEngagementAutoplayBrowserTest,
UsePreloadedData_Allowed) {
// Autoplay should be blocked by default if we have a bad score.
SetScores(PrimaryOrigin(), 0, 0);
LoadTestPage("engagement_autoplay_test.html");
ExpectAutoplayDenied();

// Load the preloaded data and we should now be able to autoplay.
ApplyPreloadedOrigin(PrimaryOrigin());
LoadTestPage("engagement_autoplay_test.html");
ExpectAutoplayAllowed();

// If we now have a high MEI score we should still be allowed to autoplay.
SetScores(PrimaryOrigin(), 10, 10);
LoadTestPage("engagement_autoplay_test.html");
ExpectAutoplayAllowed();

// If we clear the preloaded data we should still be allowed to autoplay.
ApplyEmptyPreloadedList();
LoadTestPage("engagement_autoplay_test.html");
ExpectAutoplayAllowed();
}

IN_PROC_BROWSER_TEST_F(MediaEngagementAutoplayBrowserTest,
UsePreloadedData_Denied) {
// Autoplay should be blocked by default if we have a bad score.
SetScores(PrimaryOrigin(), 0, 0);
LoadTestPage("engagement_autoplay_test.html");
ExpectAutoplayDenied();

// Load the preloaded data but we are not in that dataset so we should not be
// allowed to autoplay.
ApplyPreloadedOrigin(SecondaryOrigin());
LoadTestPage("engagement_autoplay_test.html");
ExpectAutoplayDenied();

// If we now have a high MEI score we should now be allowed to autoplay.
SetScores(PrimaryOrigin(), 10, 10);
LoadTestPage("engagement_autoplay_test.html");
ExpectAutoplayAllowed();

// If we clear the preloaded data we should still be allowed to autoplay.
ApplyEmptyPreloadedList();
LoadTestPage("engagement_autoplay_test.html");
ExpectAutoplayAllowed();
}

IN_PROC_BROWSER_TEST_F(MediaEngagementAutoplayBrowserTest,
PreloadedDataAndHighVisits) {
// Autoplay should be denied due to a low score.
SetScores(PrimaryOrigin(), 10, 0);
LoadTestPage("engagement_autoplay_test.html");
ExpectAutoplayDenied();

// Load the preloaded data and even though we are in the dataset we should not
// be allowed to play.
ApplyPreloadedOrigin(PrimaryOrigin());
LoadTestPage("engagement_autoplay_test.html");
ExpectAutoplayDenied();
}
20 changes: 18 additions & 2 deletions chrome/browser/media/media_engagement_contents_observer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@

#include "base/metrics/histogram.h"
#include "base/metrics/histogram_macros.h"
#include "build/build_config.h"
#include "chrome/browser/media/media_engagement_preloaded_list.h"
#include "chrome/browser/media/media_engagement_service.h"
#include "chrome/browser/media/media_engagement_session.h"
#include "chrome/browser/profiles/profile.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents.h"
#include "media/base/media_switches.h"
#include "third_party/WebKit/common/associated_interfaces/associated_interface_provider.h"
#include "third_party/WebKit/public/platform/media_engagement.mojom.h"

Expand Down Expand Up @@ -490,7 +491,22 @@ void MediaEngagementContentsObserver::ReadyToCommitNavigation(
content::NavigationHandle* handle) {
// TODO(beccahughes): Convert MEI API to using origin.
GURL url = handle->GetWebContents()->GetURL();
if (service_->HasHighEngagement(url)) {
MediaEngagementScore score = service_->CreateEngagementScore(url);
bool has_high_engagement = score.high_score();

// 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
// global data.
if (!has_high_engagement &&
score.visits() < MediaEngagementScore::GetScoreMinVisits() &&
base::FeatureList::IsEnabled(media::kPreloadMediaEngagementData)) {
has_high_engagement =
MediaEngagementPreloadedList::GetInstance()->CheckOriginIsPresent(
url::Origin::Create(url));
}

// If we have high media engagement then we should send that to Blink.
if (has_high_engagement) {
SendEngagementLevelToFrame(url::Origin::Create(handle->GetURL()),
handle->GetRenderFrameHost());
}
Expand Down
30 changes: 28 additions & 2 deletions chrome/browser/media/media_engagement_preloaded_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "chrome/browser/media/media_engagement_preloaded_list.h"

#include "base/files/file_util.h"
#include "base/macros.h"
#include "base/metrics/histogram_macros.h"
#include "base/path_service.h"
#include "base/strings/string_number_conversions.h"
Expand All @@ -20,11 +21,34 @@ const char MediaEngagementPreloadedList::kHistogramCheckResultName[] =
const char MediaEngagementPreloadedList::kHistogramLoadResultName[] =
"Media.Engagement.PreloadedList.LoadResult";

MediaEngagementPreloadedList::MediaEngagementPreloadedList() = default;
// static
MediaEngagementPreloadedList* MediaEngagementPreloadedList::GetInstance() {
CR_DEFINE_STATIC_LOCAL(MediaEngagementPreloadedList, instance, ());
return &instance;
}

MediaEngagementPreloadedList::MediaEngagementPreloadedList() {
// This class is allowed to be instantiated on any thread.
DETACH_FROM_SEQUENCE(sequence_checker_);
}

MediaEngagementPreloadedList::~MediaEngagementPreloadedList() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
}

MediaEngagementPreloadedList::~MediaEngagementPreloadedList() = default;
bool MediaEngagementPreloadedList::loaded() const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
return is_loaded_;
}

bool MediaEngagementPreloadedList::empty() const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
return dafsa_.empty();
}

bool MediaEngagementPreloadedList::LoadFromFile(const base::FilePath& path) {
DETACH_FROM_SEQUENCE(sequence_checker_);

// Check the file exists.
if (!base::PathExists(path)) {
RecordLoadResult(LoadResult::kFileNotFound);
Expand Down Expand Up @@ -57,6 +81,8 @@ bool MediaEngagementPreloadedList::LoadFromFile(const base::FilePath& path) {

bool MediaEngagementPreloadedList::CheckOriginIsPresent(
const url::Origin& origin) const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

// Check if we have loaded the data.
if (!loaded()) {
RecordCheckResult(CheckResult::kListNotLoaded);
Expand Down
9 changes: 7 additions & 2 deletions chrome/browser/media/media_engagement_preloaded_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <vector>

#include "base/macros.h"
#include "base/sequence_checker.h"

namespace base {
class FilePath;
Expand All @@ -20,6 +21,8 @@ class Origin;

class MediaEngagementPreloadedList {
public:
static MediaEngagementPreloadedList* GetInstance();

MediaEngagementPreloadedList();
~MediaEngagementPreloadedList();

Expand All @@ -31,10 +34,10 @@ class MediaEngagementPreloadedList {
bool CheckOriginIsPresent(const url::Origin& origin) const;

// Check whether we have loaded a list.
bool loaded() const { return is_loaded_; }
bool loaded() const;

// Check whether the list we have loaded is empty.
bool empty() const { return dafsa_.empty(); }
bool empty() const;

protected:
friend class MediaEngagementPreloadedListTest;
Expand Down Expand Up @@ -115,6 +118,8 @@ class MediaEngagementPreloadedList {
// If a list has been successfully loaded.
bool is_loaded_ = false;

SEQUENCE_CHECKER(sequence_checker_);

DISALLOW_COPY_AND_ASSIGN(MediaEngagementPreloadedList);
};

Expand Down
1 change: 1 addition & 0 deletions chrome/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@ test("browser_tests") {

data_deps = [
"//testing/buildbot/filters:browser_tests_filters",
"//tools/media_engagement_preload:generator",
]

data = []
Expand Down
7 changes: 0 additions & 7 deletions content/renderer/render_frame_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3029,13 +3029,6 @@ void RenderFrameImpl::SetEngagementLevel(const url::Origin& origin,
// blink::mojom::MediaEngagementClient implementation --------------------------

void RenderFrameImpl::SetHasHighMediaEngagement(const url::Origin& origin) {
// Set the HasHighMediaEngagement bit on |frame| if the origin matches
// the one we were provided.
if (frame_ && url::Origin(frame_->GetSecurityOrigin()) == origin) {
frame_->SetHasHighMediaEngagement(true);
return;
}

high_media_engagement_origin_ = origin;
}

Expand Down
5 changes: 5 additions & 0 deletions media/base/media_switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -366,4 +366,9 @@ const base::Feature kOverflowIconsForMediaControls{
const base::Feature kUseModernMediaControls{"UseModernMediaControls",
base::FEATURE_DISABLED_BY_DEFAULT};

// Allows Media Engagement to use preloaded data to decide whether an origin has
// a high media engagement.
const base::Feature kPreloadMediaEngagementData{
"PreloadMediaEngagementData", base::FEATURE_DISABLED_BY_DEFAULT};

} // namespace media
1 change: 1 addition & 0 deletions media/base/media_switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ MEDIA_EXPORT extern const base::Feature kNewAudioRenderingMixingStrategy;
MEDIA_EXPORT extern const base::Feature kNewRemotePlaybackPipeline;
MEDIA_EXPORT extern const base::Feature kOverflowIconsForMediaControls;
MEDIA_EXPORT extern const base::Feature kOverlayFullscreenVideo;
MEDIA_EXPORT extern const base::Feature kPreloadMediaEngagementData;
MEDIA_EXPORT extern const base::Feature kResumeBackgroundVideo;
MEDIA_EXPORT extern const base::Feature kSpecCompliantCanPlayThrough;
MEDIA_EXPORT extern const base::Feature kSupportExperimentalCdmInterface;
Expand Down
Loading

0 comments on commit eb5a05f

Please sign in to comment.